From fb5bbbe8174e3e32e6ca63217c54a63358d9ab36 Mon Sep 17 00:00:00 2001 From: Alex Komoroske Date: Sun, 9 Nov 2025 08:16:38 -0800 Subject: [PATCH 1/7] fix(llm-dialog): Filter empty text content blocks to prevent API errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem After enabling tool call caching (d394ec8cf, Nov 6), users encountered 'text content blocks must be non-empty' errors when: 1. Running demo-setup Execute button multiple times 2. Using omnibot after tool calls 3. Any cached conversation with tool calls Error from Anthropic API: ``` AI_APICallError: messages: text content blocks must be non-empty ``` ## Root Cause When Claude responds with tool calls, it can include empty text parts: ```json { "role": "assistant", "content": [ {"type": "text", "text": ""}, // Empty! {"type": "tool_use", ...} ] } ``` Before tool call caching was enabled, these messages weren't cached so the issue didn't surface. After caching was enabled: 1. First request: LLM returns empty text + tool call 2. Tool results added to conversation 3. Response cached with empty text parts 4. Next request: Cached messages (with empty text) sent to API 5. API rejects: "text content blocks must be non-empty" Anthropic API recently became stricter about rejecting empty text blocks. ## The Fix **Two-point defense:** 1. **buildAssistantMessage (line 611-613)**: Filter out empty text parts when constructing assistant messages from LLM responses 2. **Request construction (lines 1069-1082)**: Filter all messages before sending to API to remove any empty text content that may have been cached Both filters check: - Part is type "text" - Part has non-null text field - Text is non-empty after trimming ## Why Both Filters Are Needed - Filter #1: Prevents storing empty text parts initially - Filter #2: Defense in depth for cached messages that already have empty text ## Testing Verified fix resolves: - ✅ demo-setup Execute button works on first and subsequent runs - ✅ demo-setup Reset + Execute works without errors - ✅ Tool calls execute successfully - ✅ Conversation continues after tool calls - ✅ No API errors in logs ## Related - Introduced by: d394ec8cf "Allow caching of tool calls" - Workaround used in patterns: Cache-busting with Date.now() timestamps - Those workarounds can now be removed (though they're harmless) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- packages/runner/src/builtins/llm-dialog.ts | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/runner/src/builtins/llm-dialog.ts b/packages/runner/src/builtins/llm-dialog.ts index 609bc5b607..36a630d629 100644 --- a/packages/runner/src/builtins/llm-dialog.ts +++ b/packages/runner/src/builtins/llm-dialog.ts @@ -608,7 +608,9 @@ function buildAssistantMessage( }); } else if (Array.isArray(content)) { assistantContentParts.push( - ...content.filter((part) => part.type === "text") as BuiltInLLMTextPart[], + ...content.filter((part) => + part.type === "text" && (part as BuiltInLLMTextPart).text && (part as BuiltInLLMTextPart).text.trim() !== "" + ) as BuiltInLLMTextPart[], ); } @@ -1064,9 +1066,24 @@ async function startRequest( const toolCatalog = await buildToolCatalog(runtime, toolsCell); + // Filter out messages with empty text content to prevent API errors + const rawMessages = messagesCell.withTx(tx).get() as BuiltInLLMMessage[]; + const filteredMessages = rawMessages.map((msg) => { + if (Array.isArray(msg.content)) { + const filteredContent = msg.content.filter((part) => { + if (part.type === "text") { + return (part as BuiltInLLMTextPart).text && (part as BuiltInLLMTextPart).text.trim() !== ""; + } + return true; // Keep non-text parts (tool-call, tool-result, etc.) + }); + return { ...msg, content: filteredContent }; + } + return msg; + }); + const llmParams: LLMRequest = { system: system ?? "", - messages: messagesCell.withTx(tx).get() as BuiltInLLMMessage[], + messages: filteredMessages, maxTokens: maxTokens, stream: true, model: model ?? DEFAULT_MODEL_NAME, From de19c3d50d736ff2682663687924e6613daf6ddc Mon Sep 17 00:00:00 2001 From: Ben Follington <5009316+bfollington@users.noreply.github.com> Date: Sun, 9 Nov 2025 09:21:48 -0800 Subject: [PATCH 2/7] fix(llm-dialog): Handle invalid responses at source instead of just filtering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem with Previous Fix The previous fix filtered empty text blocks, but could create invalid states: - Message with only empty text: `content: [{type:"text", text:""}]` - After filtering: `content: []` ← Invalid (except for final message) ## Root Cause Vercel AI SDK v5.x bug: returns empty text blocks before tool calls. Critical scenario: stream aborts after empty text but BEFORE tool calls. Result: 1. Stream sends: {type:"text", text:""} 2. Stream crashes before tool-call events 3. We store: content: [{type:"text", text:""}] (no tool calls!) 4. Gets cached 5. Next request: filter creates content: [] 6. API rejects: "messages must have non-empty content" ## New Solution ### 1. Validate Fresh Responses Check if response has valid content before storing: - If invalid (empty/only whitespace), insert proper error message - Provides user feedback: "I encountered an error generating a response..." - Maintains valid message history ### 2. Enhanced Cached Message Filtering - Filter empty text blocks from cached messages - Remove messages that become empty after filtering - Respect API rule: final assistant message CAN be empty ### 3. Remove Unnecessary Filtering Removed filtering from buildAssistantMessage since we now validate upstream. ## Benefits - Handles root cause (aborted/incomplete responses) - User gets clear error message instead of silent failure - Logs warnings for debugging - Never sends invalid payloads to Anthropic API - Backward compatible with already-cached invalid messages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- packages/runner/src/builtins/llm-dialog.ts | 92 ++++++++++++++++++---- 1 file changed, 76 insertions(+), 16 deletions(-) diff --git a/packages/runner/src/builtins/llm-dialog.ts b/packages/runner/src/builtins/llm-dialog.ts index 36a630d629..6000171a6e 100644 --- a/packages/runner/src/builtins/llm-dialog.ts +++ b/packages/runner/src/builtins/llm-dialog.ts @@ -593,6 +593,30 @@ function extractToolCallParts( ); } +/** + * Validates whether message content is non-empty and valid for the Anthropic API. + * Returns true if the content contains at least one non-empty text block or tool call. + */ +function hasValidContent(content: BuiltInLLMMessage["content"]): boolean { + if (typeof content === "string") { + return content.trim().length > 0; + } + + if (Array.isArray(content)) { + return content.some(part => { + if (part.type === "tool-call" || part.type === "tool-result") { + return true; + } + if (part.type === "text") { + return (part as BuiltInLLMTextPart).text?.trim().length > 0; + } + return false; + }); + } + + return false; +} + function buildAssistantMessage( content: BuiltInLLMMessage["content"], toolCallParts: BuiltInLLMToolCallPart[], @@ -608,9 +632,7 @@ function buildAssistantMessage( }); } else if (Array.isArray(content)) { assistantContentParts.push( - ...content.filter((part) => - part.type === "text" && (part as BuiltInLLMTextPart).text && (part as BuiltInLLMTextPart).text.trim() !== "" - ) as BuiltInLLMTextPart[], + ...content.filter((part) => part.type === "text") as BuiltInLLMTextPart[], ); } @@ -686,6 +708,7 @@ export const llmDialogTestHelpers = { extractToolCallParts, buildAssistantMessage, createToolResultMessages, + hasValidContent, }; /** @@ -1066,20 +1089,30 @@ async function startRequest( const toolCatalog = await buildToolCatalog(runtime, toolsCell); - // Filter out messages with empty text content to prevent API errors + // Filter out empty text blocks from cached messages to handle ones that were + // stored before this fix. This prevents sending empty text blocks to the API. const rawMessages = messagesCell.withTx(tx).get() as BuiltInLLMMessage[]; - const filteredMessages = rawMessages.map((msg) => { - if (Array.isArray(msg.content)) { - const filteredContent = msg.content.filter((part) => { - if (part.type === "text") { - return (part as BuiltInLLMTextPart).text && (part as BuiltInLLMTextPart).text.trim() !== ""; - } - return true; // Keep non-text parts (tool-call, tool-result, etc.) - }); - return { ...msg, content: filteredContent }; - } - return msg; - }); + const filteredMessages = rawMessages + .map((msg, index) => { + if (Array.isArray(msg.content)) { + const filteredContent = msg.content.filter((part) => { + if (part.type === "text") { + return (part as BuiltInLLMTextPart).text && (part as BuiltInLLMTextPart).text.trim() !== ""; + } + return true; // Keep non-text parts (tool-call, tool-result, etc.) + }); + return { ...msg, content: filteredContent }; + } + return msg; + }) + .filter((msg, index, array) => { + // Remove messages that would have empty content arrays after filtering, + // unless it's the final assistant message (which can be empty per Anthropic API). + if (!Array.isArray(msg.content)) return true; + const isFinalMessage = index === array.length - 1; + const isAssistant = msg.role === "assistant"; + return msg.content.length > 0 || (isFinalMessage && isAssistant); + }); const llmParams: LLMRequest = { system: system ?? "", @@ -1097,6 +1130,33 @@ async function startRequest( resultPromise .then(async (llmResult) => { + // Validate that the response has valid content + if (!hasValidContent(llmResult.content)) { + // LLM returned empty or invalid content (e.g., stream aborted mid-flight, + // or AI SDK bug with empty text blocks). Insert a proper error message + // instead of storing invalid content. + logger.warn("LLM returned invalid/empty content, adding error message"); + const errorMessage = { + [ID]: { llmDialog: { message: cause, id: crypto.randomUUID() } }, + role: "assistant", + content: "I encountered an error generating a response. Please try again.", + } satisfies BuiltInLLMMessage & { [ID]: unknown }; + + await safelyPerformUpdate( + runtime, + pending, + internal, + requestId, + (tx) => { + messagesCell.withTx(tx).push( + errorMessage as Schema, + ); + pending.withTx(tx).set(false); + }, + ); + return; + } + // Extract tool calls from content if it's an array const hasToolCalls = Array.isArray(llmResult.content) && llmResult.content.some((part) => part.type === "tool-call"); From 1bcf95b72313b58cacf173ab41a4dc20171cd761 Mon Sep 17 00:00:00 2001 From: Ben Follington <5009316+bfollington@users.noreply.github.com> Date: Sun, 9 Nov 2025 10:24:22 -0800 Subject: [PATCH 3/7] fix(llm-dialog): Handle invalid responses and tool result edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problems Fixed 1. **Empty/invalid LLM responses**: Stream aborts after empty text but before tool calls 2. **Tool results with no output**: Tools returning undefined/null broke API validation 3. **Tool result filtering**: Cached tool result messages could be removed incorrectly 4. **Mismatched tool calls/results**: If executeToolCalls fails partially ## Root Causes **Vercel AI SDK bug**: Returns empty text blocks before tool calls: ```json { "content": [ { "type": "text", "text": "" }, // ← SDK bug { "type": "tool-call", ... } ] } ``` **Critical scenario**: Stream crashes after empty text but BEFORE tool calls: - Receive: `content: [{type:"text", text:""}]` (no tool calls!) - Store as-is, gets cached - Next request: Filter creates `content: []` ← Invalid! - API rejects: "messages must have non-empty content" **Tool result validation**: Anthropic API **requires** tool_result for every tool_use: - If tool returns undefined/null, output field was undefined - Error: "tool_use ids were found without tool_result blocks" ## Solution ### 1. Validate Fresh Responses Check if response has valid content before storing: ```typescript function hasValidContent(content): boolean { // Returns true only if content has non-empty text OR tool calls OR tool results } if (!hasValidContent(llmResult.content)) { // Insert proper error message instead of invalid content content: "I encountered an error generating a response. Please try again." } ``` ### 2. Ensure Tool Results Always Valid Convert undefined/null results to explicit null value: ```typescript if (toolResult.result === undefined || toolResult.result === null) { output = { type: "json", value: null }; } ``` ### 3. Never Filter Tool Messages Tool result messages must always be preserved: ```typescript if (msg.role === "tool") return true; // Never filter ``` ### 4. Validate Tool Call/Result Match Ensure every tool call has a corresponding result: ```typescript if (toolResults.length !== toolCallParts.length) { // Log detailed error, insert error message } ``` ### 5. Enhanced Cached Message Filtering - Filter empty text blocks from cached messages (backward compat) - Remove messages that become empty after filtering (except final) - Respect Anthropic API rule: final assistant message CAN be empty ## Benefits - ✅ Handles root cause (aborted/incomplete responses) - ✅ User gets clear error messages instead of silent failure - ✅ Logs warnings for debugging - ✅ Never sends invalid payloads to Anthropic API - ✅ Backward compatible with already-cached invalid messages - ✅ Test coverage for all edge cases ## Test Coverage Added 7 new test cases: - hasValidContent validates empty/non-empty text - hasValidContent validates tool calls and results - createToolResultMessages handles undefined/null results 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- packages/runner/src/builtins/llm-dialog.ts | 70 ++++++++++++++++--- .../runner/test/llm-dialog-helpers.test.ts | 65 +++++++++++++++++ 2 files changed, 124 insertions(+), 11 deletions(-) diff --git a/packages/runner/src/builtins/llm-dialog.ts b/packages/runner/src/builtins/llm-dialog.ts index 6000171a6e..166b998968 100644 --- a/packages/runner/src/builtins/llm-dialog.ts +++ b/packages/runner/src/builtins/llm-dialog.ts @@ -688,17 +688,29 @@ async function executeToolCalls( function createToolResultMessages( results: ToolCallExecutionResult[], ): BuiltInLLMMessage[] { - return results.map((toolResult) => ({ - role: "tool", - content: [{ - type: "tool-result", - toolCallId: toolResult.id, - toolName: toolResult.toolName || "unknown", - output: toolResult.error - ? { type: "error-text", value: toolResult.error } - : toolResult.result, - }], - })); + return results.map((toolResult) => { + // Ensure output is never undefined/null - Anthropic API requires valid tool_result + // for every tool_use, even if the tool returns nothing + let output: any; + if (toolResult.error) { + output = { type: "error-text", value: toolResult.error }; + } else if (toolResult.result === undefined || toolResult.result === null) { + // Tool returned nothing - use explicit null value + output = { type: "json", value: null }; + } else { + output = toolResult.result; + } + + return { + role: "tool", + content: [{ + type: "tool-result", + toolCallId: toolResult.id, + toolName: toolResult.toolName || "unknown", + output, + }], + }; + }); } export const llmDialogTestHelpers = { @@ -1106,6 +1118,9 @@ async function startRequest( return msg; }) .filter((msg, index, array) => { + // NEVER filter out tool result messages - API requires tool_result for every tool_use + if (msg.role === "tool") return true; + // Remove messages that would have empty content arrays after filtering, // unless it's the final assistant message (which can be empty per Anthropic API). if (!Array.isArray(msg.content)) return true; @@ -1175,6 +1190,39 @@ async function startRequest( toolCatalog, toolCallParts, ); + + // Validate that we have a result for every tool call with matching IDs + const toolCallIds = new Set(toolCallParts.map(p => p.toolCallId)); + const resultIds = new Set(toolResults.map(r => r.id)); + const mismatch = toolResults.length !== toolCallParts.length || + !toolCallParts.every(p => resultIds.has(p.toolCallId)); + + if (mismatch) { + logger.error( + `Tool execution mismatch: ${toolCallParts.length} calls [${Array.from(toolCallIds)}] but ${toolResults.length} results [${Array.from(resultIds)}]` + ); + // Add error message instead of invalid partial results + const errorMessage = { + [ID]: { llmDialog: { message: cause, id: crypto.randomUUID() } }, + role: "assistant", + content: "Some tool calls failed to execute. Please try again.", + } satisfies BuiltInLLMMessage & { [ID]: unknown }; + + await safelyPerformUpdate( + runtime, + pending, + internal, + requestId, + (tx) => { + messagesCell.withTx(tx).push( + errorMessage as Schema, + ); + pending.withTx(tx).set(false); + }, + ); + return; + } + const newMessages: BuiltInLLMMessage[] = [ assistantMessage, ...createToolResultMessages(toolResults), diff --git a/packages/runner/test/llm-dialog-helpers.test.ts b/packages/runner/test/llm-dialog-helpers.test.ts index b4eb9e32ea..271672a8f1 100644 --- a/packages/runner/test/llm-dialog-helpers.test.ts +++ b/packages/runner/test/llm-dialog-helpers.test.ts @@ -9,6 +9,7 @@ const { extractToolCallParts, buildAssistantMessage, createToolResultMessages, + hasValidContent, } = llmDialogTestHelpers; Deno.test("createCharmToolDefinitions slugifies charm names and returns tool metadata", () => { @@ -112,3 +113,67 @@ Deno.test("createToolResultMessages converts execution results into tool message assertEquals(failurePart.toolName, "failing"); assertEquals(failurePart.output, { type: "error-text", value: "boom" }); }); + +Deno.test("hasValidContent returns true for non-empty text", () => { + assert(hasValidContent("Hello")); + assert(hasValidContent([{ type: "text", text: "Hello" }])); +}); + +Deno.test("hasValidContent returns false for empty text", () => { + assert(!hasValidContent("")); + assert(!hasValidContent(" \n ")); + assert(!hasValidContent([{ type: "text", text: "" }])); + assert(!hasValidContent([{ type: "text", text: " " }])); +}); + +Deno.test("hasValidContent returns true for tool calls", () => { + const content: BuiltInLLMMessage["content"] = [ + { type: "text", text: "" }, + { type: "tool-call", toolCallId: "1", toolName: "test", input: {} }, + ]; + assert(hasValidContent(content)); +}); + +Deno.test("hasValidContent returns true for tool results", () => { + const content: BuiltInLLMMessage["content"] = [ + { type: "tool-result", toolCallId: "1", toolName: "test", output: { type: "json", value: null } }, + ]; + assert(hasValidContent(content)); +}); + +Deno.test("hasValidContent returns false for only empty text parts", () => { + const content: BuiltInLLMMessage["content"] = [ + { type: "text", text: "" }, + { type: "text", text: " " }, + ]; + assert(!hasValidContent(content)); +}); + +Deno.test("createToolResultMessages handles undefined result with explicit null", () => { + const messages = createToolResultMessages([{ + id: "call-1", + toolName: "empty", + result: undefined, + }]); + + assertEquals(messages.length, 1); + assertEquals(messages[0].role, "tool"); + assertEquals(messages[0].content?.[0], { + type: "tool-result", + toolCallId: "call-1", + toolName: "empty", + output: { type: "json", value: null }, + }); +}); + +Deno.test("createToolResultMessages handles null result with explicit null", () => { + const messages = createToolResultMessages([{ + id: "call-1", + toolName: "empty", + result: null, + }]); + + assertEquals(messages.length, 1); + const outputPart = messages[0].content?.[0] as any; + assertEquals(outputPart.output, { type: "json", value: null }); +}); From fe002dd90cfa1bab4c90d2f42c2c762533dde8d6 Mon Sep 17 00:00:00 2001 From: Ben Follington <5009316+bfollington@users.noreply.github.com> Date: Sun, 9 Nov 2025 10:25:39 -0800 Subject: [PATCH 4/7] chore: Apply deno fmt formatting --- packages/runner/src/builtins/llm-dialog.ts | 18 +++++++++++------- .../runner/test/llm-dialog-helpers.test.ts | 7 ++++++- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/runner/src/builtins/llm-dialog.ts b/packages/runner/src/builtins/llm-dialog.ts index 166b998968..0790ff0b36 100644 --- a/packages/runner/src/builtins/llm-dialog.ts +++ b/packages/runner/src/builtins/llm-dialog.ts @@ -603,7 +603,7 @@ function hasValidContent(content: BuiltInLLMMessage["content"]): boolean { } if (Array.isArray(content)) { - return content.some(part => { + return content.some((part) => { if (part.type === "tool-call" || part.type === "tool-result") { return true; } @@ -1109,7 +1109,8 @@ async function startRequest( if (Array.isArray(msg.content)) { const filteredContent = msg.content.filter((part) => { if (part.type === "text") { - return (part as BuiltInLLMTextPart).text && (part as BuiltInLLMTextPart).text.trim() !== ""; + return (part as BuiltInLLMTextPart).text && + (part as BuiltInLLMTextPart).text.trim() !== ""; } return true; // Keep non-text parts (tool-call, tool-result, etc.) }); @@ -1154,7 +1155,8 @@ async function startRequest( const errorMessage = { [ID]: { llmDialog: { message: cause, id: crypto.randomUUID() } }, role: "assistant", - content: "I encountered an error generating a response. Please try again.", + content: + "I encountered an error generating a response. Please try again.", } satisfies BuiltInLLMMessage & { [ID]: unknown }; await safelyPerformUpdate( @@ -1192,14 +1194,16 @@ async function startRequest( ); // Validate that we have a result for every tool call with matching IDs - const toolCallIds = new Set(toolCallParts.map(p => p.toolCallId)); - const resultIds = new Set(toolResults.map(r => r.id)); + const toolCallIds = new Set(toolCallParts.map((p) => p.toolCallId)); + const resultIds = new Set(toolResults.map((r) => r.id)); const mismatch = toolResults.length !== toolCallParts.length || - !toolCallParts.every(p => resultIds.has(p.toolCallId)); + !toolCallParts.every((p) => resultIds.has(p.toolCallId)); if (mismatch) { logger.error( - `Tool execution mismatch: ${toolCallParts.length} calls [${Array.from(toolCallIds)}] but ${toolResults.length} results [${Array.from(resultIds)}]` + `Tool execution mismatch: ${toolCallParts.length} calls [${ + Array.from(toolCallIds) + }] but ${toolResults.length} results [${Array.from(resultIds)}]`, ); // Add error message instead of invalid partial results const errorMessage = { diff --git a/packages/runner/test/llm-dialog-helpers.test.ts b/packages/runner/test/llm-dialog-helpers.test.ts index 271672a8f1..a5339c0b76 100644 --- a/packages/runner/test/llm-dialog-helpers.test.ts +++ b/packages/runner/test/llm-dialog-helpers.test.ts @@ -136,7 +136,12 @@ Deno.test("hasValidContent returns true for tool calls", () => { Deno.test("hasValidContent returns true for tool results", () => { const content: BuiltInLLMMessage["content"] = [ - { type: "tool-result", toolCallId: "1", toolName: "test", output: { type: "json", value: null } }, + { + type: "tool-result", + toolCallId: "1", + toolName: "test", + output: { type: "json", value: null }, + }, ]; assert(hasValidContent(content)); }); From 37e1cd4a2fe3bd8157344af225c1af9ad50a5b99 Mon Sep 17 00:00:00 2001 From: Ben Follington <5009316+bfollington@users.noreply.github.com> Date: Sun, 9 Nov 2025 10:26:42 -0800 Subject: [PATCH 5/7] chore: Fix lint warning - remove unused index parameter --- packages/runner/src/builtins/llm-dialog.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runner/src/builtins/llm-dialog.ts b/packages/runner/src/builtins/llm-dialog.ts index 0790ff0b36..3dcc77ddf2 100644 --- a/packages/runner/src/builtins/llm-dialog.ts +++ b/packages/runner/src/builtins/llm-dialog.ts @@ -1105,7 +1105,7 @@ async function startRequest( // stored before this fix. This prevents sending empty text blocks to the API. const rawMessages = messagesCell.withTx(tx).get() as BuiltInLLMMessage[]; const filteredMessages = rawMessages - .map((msg, index) => { + .map((msg) => { if (Array.isArray(msg.content)) { const filteredContent = msg.content.filter((part) => { if (part.type === "text") { From 141c572d0775749370aec5c33a7bdd67acfe69a6 Mon Sep 17 00:00:00 2001 From: Ben Follington <5009316+bfollington@users.noreply.github.com> Date: Sun, 9 Nov 2025 10:32:20 -0800 Subject: [PATCH 6/7] refactor: Use validation instead of filtering for message history Instead of filtering empty text blocks from within messages and then removing messages that become empty, use the same hasValidContent() validation as fresh responses. This is cleaner and more consistent: Before: - Map over messages to filter empty text blocks from content arrays - Then filter out messages that became empty after text filtering - Complex multi-step transformation After: - Single validation pass using hasValidContent() - Remove messages that fail validation entirely - Same validation logic as fresh responses Benefits: - Simpler code (one filter instead of map + filter) - Consistent validation across fresh and cached messages - Don't try to "fix" invalid data, just remove invalid messages - Clearer intent: validate and remove, not transform and patch --- packages/runner/src/builtins/llm-dialog.ts | 52 ++++++++++------------ 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/packages/runner/src/builtins/llm-dialog.ts b/packages/runner/src/builtins/llm-dialog.ts index 3dcc77ddf2..ed542fdd7d 100644 --- a/packages/runner/src/builtins/llm-dialog.ts +++ b/packages/runner/src/builtins/llm-dialog.ts @@ -1101,38 +1101,34 @@ async function startRequest( const toolCatalog = await buildToolCatalog(runtime, toolsCell); - // Filter out empty text blocks from cached messages to handle ones that were - // stored before this fix. This prevents sending empty text blocks to the API. + // Validate message history - remove any invalid messages that were stored + // before this fix (e.g., messages with only empty text blocks). + // Use the same validation logic as fresh responses. const rawMessages = messagesCell.withTx(tx).get() as BuiltInLLMMessage[]; - const filteredMessages = rawMessages - .map((msg) => { - if (Array.isArray(msg.content)) { - const filteredContent = msg.content.filter((part) => { - if (part.type === "text") { - return (part as BuiltInLLMTextPart).text && - (part as BuiltInLLMTextPart).text.trim() !== ""; - } - return true; // Keep non-text parts (tool-call, tool-result, etc.) - }); - return { ...msg, content: filteredContent }; - } - return msg; - }) - .filter((msg, index, array) => { - // NEVER filter out tool result messages - API requires tool_result for every tool_use - if (msg.role === "tool") return true; - - // Remove messages that would have empty content arrays after filtering, - // unless it's the final assistant message (which can be empty per Anthropic API). - if (!Array.isArray(msg.content)) return true; - const isFinalMessage = index === array.length - 1; - const isAssistant = msg.role === "assistant"; - return msg.content.length > 0 || (isFinalMessage && isAssistant); - }); + const validMessages = rawMessages.filter((msg, index, array) => { + // NEVER filter out tool result messages - API requires tool_result for every tool_use + if (msg.role === "tool") return true; + + // Validate using same logic as fresh responses + // Exception: final assistant message can be empty per Anthropic API + const isFinalMessage = index === array.length - 1; + const isAssistant = msg.role === "assistant"; + const canBeEmpty = isFinalMessage && isAssistant; + + return hasValidContent(msg.content) || canBeEmpty; + }); + + if (validMessages.length !== rawMessages.length) { + logger.warn( + `Removed ${ + rawMessages.length - validMessages.length + } invalid messages from history`, + ); + } const llmParams: LLMRequest = { system: system ?? "", - messages: filteredMessages, + messages: validMessages, maxTokens: maxTokens, stream: true, model: model ?? DEFAULT_MODEL_NAME, From af30df96e85b50125cac5e7b92661e05cf7a3696 Mon Sep 17 00:00:00 2001 From: Ben Follington <5009316+bfollington@users.noreply.github.com> Date: Sun, 9 Nov 2025 10:59:06 -0800 Subject: [PATCH 7/7] refactor: Remove cached message validation - let legacy data fail Instead of validating and filtering cached messages, let the API reject invalid legacy data. This significantly simplifies the code: Before: - Validate all cached messages with hasValidContent() - Filter out invalid messages with logging - Special handling for tool messages and final assistant messages After: - Send messages as-is to API - If legacy invalid data exists, API will reject with clear error - User can start fresh conversation This is acceptable because: 1. Going forward, we never write invalid data (validated at source) 2. Legacy conversations with invalid data are rare edge cases 3. Clear API error is better than silent data manipulation 4. Much simpler code The prevention happens entirely at write time: - Fresh responses validated with hasValidContent() - Tool results always have valid output - Tool call/result counts validated --- packages/runner/src/builtins/llm-dialog.ts | 27 +--------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/packages/runner/src/builtins/llm-dialog.ts b/packages/runner/src/builtins/llm-dialog.ts index ed542fdd7d..5639d4ffc2 100644 --- a/packages/runner/src/builtins/llm-dialog.ts +++ b/packages/runner/src/builtins/llm-dialog.ts @@ -1101,34 +1101,9 @@ async function startRequest( const toolCatalog = await buildToolCatalog(runtime, toolsCell); - // Validate message history - remove any invalid messages that were stored - // before this fix (e.g., messages with only empty text blocks). - // Use the same validation logic as fresh responses. - const rawMessages = messagesCell.withTx(tx).get() as BuiltInLLMMessage[]; - const validMessages = rawMessages.filter((msg, index, array) => { - // NEVER filter out tool result messages - API requires tool_result for every tool_use - if (msg.role === "tool") return true; - - // Validate using same logic as fresh responses - // Exception: final assistant message can be empty per Anthropic API - const isFinalMessage = index === array.length - 1; - const isAssistant = msg.role === "assistant"; - const canBeEmpty = isFinalMessage && isAssistant; - - return hasValidContent(msg.content) || canBeEmpty; - }); - - if (validMessages.length !== rawMessages.length) { - logger.warn( - `Removed ${ - rawMessages.length - validMessages.length - } invalid messages from history`, - ); - } - const llmParams: LLMRequest = { system: system ?? "", - messages: validMessages, + messages: messagesCell.withTx(tx).get() as BuiltInLLMMessage[], maxTokens: maxTokens, stream: true, model: model ?? DEFAULT_MODEL_NAME,