From d41a40624d9d4d68c56dcf2b30657f32ee1b1556 Mon Sep 17 00:00:00 2001 From: Simon Ferquel's Clanker Date: Mon, 27 Apr 2026 07:48:50 +0000 Subject: [PATCH 1/5] fix: normalize message trimming behavior across all model providers Ensure strings.TrimSpace() is used only as a skip guard, never to mutate the content that reaches the provider API. Fixes inconsistencies across Anthropic, OpenAI, Bedrock, Gemini, and oaistream-based providers. Changes: - Anthropic (standard + Beta): single-part user messages, multi-part text parts, and tool-result content now send the original untrimmed value. Tool-result blocks are normalized to empty string (not skipped) when whitespace-only, because every tool_use must have a corresponding tool_result. - OpenAI oaistream / Chat Completions: ConvertMultiContent switched from pre-allocated slice to append-based; adds TrimSpace guard on text parts so whitespace-only and empty parts are not forwarded. Same guard added to tool-role multi-content text parts. - OpenAI Responses API: adds TrimSpace guard on multi-part user message text parts in convertMessagesToResponseInput. - Gemini: strengthens single-part guard from != "" to TrimSpace != ""; adds TrimSpace guard in convertMultiContent for multi-part text parts. - Test: updates the oaistream ConvertMultiContent test to assert the corrected behavior (nil-URL image part produces no output part, not a zero-value entry); adds whitespace-only text part test case. Fixes #2515 Assisted-By: docker-agent --- .../provider/anthropic/beta_converter.go | 20 ++++++++++++------- pkg/model/provider/anthropic/client.go | 20 ++++++++++++------- pkg/model/provider/gemini/client.go | 5 ++++- pkg/model/provider/oaistream/messages.go | 16 ++++++++++----- pkg/model/provider/oaistream/messages_test.go | 10 +++++++++- pkg/model/provider/openai/client.go | 3 +++ 6 files changed, 53 insertions(+), 21 deletions(-) diff --git a/pkg/model/provider/anthropic/beta_converter.go b/pkg/model/provider/anthropic/beta_converter.go index fa1c04dcf..2671d6de1 100644 --- a/pkg/model/provider/anthropic/beta_converter.go +++ b/pkg/model/provider/anthropic/beta_converter.go @@ -43,11 +43,11 @@ func (c *Client) convertBetaMessages(ctx context.Context, messages []chat.Messag Content: contentBlocks, }) } - } else if txt := strings.TrimSpace(msg.Content); txt != "" { + } else if strings.TrimSpace(msg.Content) != "" { betaMessages = append(betaMessages, anthropic.BetaMessageParam{ Role: anthropic.BetaMessageParamRoleUser, Content: []anthropic.BetaContentBlockParamUnion{ - {OfText: &anthropic.BetaTextBlockParam{Text: txt}}, + {OfText: &anthropic.BetaTextBlockParam{Text: msg.Content}}, }, }) } @@ -138,11 +138,17 @@ func (c *Client) convertBetaMessages(ctx context.Context, messages []chat.Messag // including any image content from MultiContent. func convertBetaToolResultBlock(msg *chat.Message) anthropic.BetaContentBlockParamUnion { if !hasImageMultiContent(msg.MultiContent) { + // tool_result must be present for every preceding tool_use; we cannot skip + // it. Normalize whitespace-only content to empty string rather than skipping. + content := msg.Content + if strings.TrimSpace(content) == "" { + content = "" + } return anthropic.BetaContentBlockParamUnion{ OfToolResult: &anthropic.BetaToolResultBlockParam{ ToolUseID: msg.ToolCallID, Content: []anthropic.BetaToolResultBlockParamContentUnion{ - {OfText: &anthropic.BetaTextBlockParam{Text: strings.TrimSpace(msg.Content)}}, + {OfText: &anthropic.BetaTextBlockParam{Text: content}}, }, }, } @@ -152,9 +158,9 @@ func convertBetaToolResultBlock(msg *chat.Message) anthropic.BetaContentBlockPar for _, part := range msg.MultiContent { switch part.Type { case chat.MessagePartTypeText: - if txt := strings.TrimSpace(part.Text); txt != "" { + if strings.TrimSpace(part.Text) != "" { content = append(content, anthropic.BetaToolResultBlockParamContentUnion{ - OfText: &anthropic.BetaTextBlockParam{Text: txt}, + OfText: &anthropic.BetaTextBlockParam{Text: part.Text}, }) } case chat.MessagePartTypeImageURL: @@ -194,9 +200,9 @@ func (c *Client) convertBetaUserMultiContent(ctx context.Context, parts []chat.M for _, part := range parts { switch part.Type { case chat.MessagePartTypeText: - if txt := strings.TrimSpace(part.Text); txt != "" { + if strings.TrimSpace(part.Text) != "" { contentBlocks = append(contentBlocks, anthropic.BetaContentBlockParamUnion{ - OfText: &anthropic.BetaTextBlockParam{Text: txt}, + OfText: &anthropic.BetaTextBlockParam{Text: part.Text}, }) } diff --git a/pkg/model/provider/anthropic/client.go b/pkg/model/provider/anthropic/client.go index 3d6aea955..1e8abf930 100644 --- a/pkg/model/provider/anthropic/client.go +++ b/pkg/model/provider/anthropic/client.go @@ -305,8 +305,8 @@ func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) ( anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(contentBlocks...)) } } else { - if txt := strings.TrimSpace(msg.Content); txt != "" { - anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(anthropic.NewTextBlock(txt))) + if strings.TrimSpace(msg.Content) != "" { + anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(anthropic.NewTextBlock(msg.Content))) } } continue @@ -404,7 +404,13 @@ func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) ( func convertToolResultBlock(msg *chat.Message) anthropic.ContentBlockParamUnion { // If there are no images in MultiContent, use the simple text-only format. if !hasImageMultiContent(msg.MultiContent) { - return anthropic.NewToolResultBlock(msg.ToolCallID, strings.TrimSpace(msg.Content), msg.IsError) + // tool_result must be present for every preceding tool_use; we cannot skip + // it. Normalize whitespace-only content to empty string rather than skipping. + content := msg.Content + if strings.TrimSpace(content) == "" { + content = "" + } + return anthropic.NewToolResultBlock(msg.ToolCallID, content, msg.IsError) } // Build content blocks with text + images for the tool result. @@ -412,9 +418,9 @@ func convertToolResultBlock(msg *chat.Message) anthropic.ContentBlockParamUnion for _, part := range msg.MultiContent { switch part.Type { case chat.MessagePartTypeText: - if txt := strings.TrimSpace(part.Text); txt != "" { + if strings.TrimSpace(part.Text) != "" { content = append(content, anthropic.ToolResultBlockParamContentUnion{ - OfText: &anthropic.TextBlockParam{Text: txt}, + OfText: &anthropic.TextBlockParam{Text: part.Text}, }) } case chat.MessagePartTypeImageURL: @@ -483,8 +489,8 @@ func (c *Client) convertUserMultiContent(_ context.Context, parts []chat.Message for _, part := range parts { switch part.Type { case chat.MessagePartTypeText: - if txt := strings.TrimSpace(part.Text); txt != "" { - contentBlocks = append(contentBlocks, anthropic.NewTextBlock(txt)) + if strings.TrimSpace(part.Text) != "" { + contentBlocks = append(contentBlocks, anthropic.NewTextBlock(part.Text)) } case chat.MessagePartTypeImageURL: diff --git a/pkg/model/provider/gemini/client.go b/pkg/model/provider/gemini/client.go index e48e6f6e4..ff67cda55 100644 --- a/pkg/model/provider/gemini/client.go +++ b/pkg/model/provider/gemini/client.go @@ -261,7 +261,7 @@ func convertMessagesToGemini(messages []chat.Message) []*genai.Content { if len(parts) > 0 { contents = append(contents, genai.NewContentFromParts(parts, role)) } - } else if msg.Content != "" { + } else if strings.TrimSpace(msg.Content) != "" { part := newTextPartWithSignature(msg.Content, msg.ThoughtSignature) contents = append(contents, genai.NewContentFromParts([]*genai.Part{part}, role)) } @@ -292,6 +292,9 @@ func convertMultiContent(multiContent []chat.MessagePart, thoughtSignature []byt for _, part := range multiContent { switch part.Type { case chat.MessagePartTypeText: + if strings.TrimSpace(part.Text) == "" { + continue + } parts = append(parts, newTextPartWithSignature(part.Text, thoughtSignature)) case chat.MessagePartTypeImageURL: if imgPart := convertImageURLToPart(part.ImageURL); imgPart != nil { diff --git a/pkg/model/provider/oaistream/messages.go b/pkg/model/provider/oaistream/messages.go index c82940f25..7b82e591e 100644 --- a/pkg/model/provider/oaistream/messages.go +++ b/pkg/model/provider/oaistream/messages.go @@ -25,17 +25,20 @@ func (j JSONSchema) MarshalJSON() ([]byte, error) { // ConvertMultiContent converts chat.MessagePart slices to OpenAI content parts. func ConvertMultiContent(multiContent []chat.MessagePart) []openai.ChatCompletionContentPartUnionParam { - parts := make([]openai.ChatCompletionContentPartUnionParam, len(multiContent)) - for i, part := range multiContent { + parts := make([]openai.ChatCompletionContentPartUnionParam, 0, len(multiContent)) + for _, part := range multiContent { switch part.Type { case chat.MessagePartTypeText: - parts[i] = openai.TextContentPart(part.Text) + if strings.TrimSpace(part.Text) == "" { + continue + } + parts = append(parts, openai.TextContentPart(part.Text)) case chat.MessagePartTypeImageURL: if part.ImageURL != nil { - parts[i] = openai.ImageContentPart(openai.ChatCompletionContentPartImageImageURLParam{ + parts = append(parts, openai.ImageContentPart(openai.ChatCompletionContentPartImageImageURLParam{ URL: part.ImageURL.URL, Detail: string(part.ImageURL.Detail), - }) + })) } } } @@ -139,6 +142,9 @@ func ConvertMessages(messages []chat.Message) []openai.ChatCompletionMessagePara textParts := make([]openai.ChatCompletionContentPartTextParam, 0) for _, part := range msg.MultiContent { if part.Type == chat.MessagePartTypeText { + if strings.TrimSpace(part.Text) == "" { + continue + } textParts = append(textParts, openai.ChatCompletionContentPartTextParam{ Text: part.Text, }) diff --git a/pkg/model/provider/oaistream/messages_test.go b/pkg/model/provider/oaistream/messages_test.go index fa46ee0ff..db4f4d55f 100644 --- a/pkg/model/provider/oaistream/messages_test.go +++ b/pkg/model/provider/oaistream/messages_test.go @@ -41,10 +41,18 @@ func TestConvertMultiContent(t *testing.T) { wantCount: 2, }, { - name: "image without URL", + name: "image with nil URL produces no part", multiContent: []chat.MessagePart{ {Type: chat.MessagePartTypeImageURL, ImageURL: nil}, }, + wantCount: 0, + }, + { + name: "whitespace-only text part skipped", + multiContent: []chat.MessagePart{ + {Type: chat.MessagePartTypeText, Text: " "}, + {Type: chat.MessagePartTypeText, Text: "hello"}, + }, wantCount: 1, }, } diff --git a/pkg/model/provider/openai/client.go b/pkg/model/provider/openai/client.go index c9c24e57e..7ab1a9b74 100644 --- a/pkg/model/provider/openai/client.go +++ b/pkg/model/provider/openai/client.go @@ -574,6 +574,9 @@ func convertMessagesToResponseInput(messages []chat.Message) []responses.Respons for _, part := range msg.MultiContent { switch part.Type { case chat.MessagePartTypeText: + if strings.TrimSpace(part.Text) == "" { + continue + } contentParts = append(contentParts, responses.ResponseInputContentUnionParam{ OfInputText: &responses.ResponseInputTextParam{ Text: part.Text, From 8620bea6fd4b3bfbd6eef6d11e95014094c03170 Mon Sep 17 00:00:00 2001 From: Simon Ferquel's Clanker Date: Mon, 27 Apr 2026 07:52:59 +0000 Subject: [PATCH 2/5] fix: collapse else+if into else-if to satisfy gocritic lint Assisted-By: docker-agent --- pkg/model/provider/anthropic/client.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/model/provider/anthropic/client.go b/pkg/model/provider/anthropic/client.go index 1e8abf930..a796e94f4 100644 --- a/pkg/model/provider/anthropic/client.go +++ b/pkg/model/provider/anthropic/client.go @@ -304,10 +304,8 @@ func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) ( if len(contentBlocks) > 0 { anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(contentBlocks...)) } - } else { - if strings.TrimSpace(msg.Content) != "" { - anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(anthropic.NewTextBlock(msg.Content))) - } + } else if strings.TrimSpace(msg.Content) != "" { + anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(anthropic.NewTextBlock(msg.Content))) } continue } From b0c1f937b73e8b05a0be590fa3bac0a1fd31e5ed Mon Sep 17 00:00:00 2001 From: Simon Ferquel's Clanker Date: Mon, 27 Apr 2026 08:13:34 +0000 Subject: [PATCH 3/5] test: update e2e cassettes to reflect untrimmed content sent to Anthropic API The previous Anthropic provider code trimmed trailing whitespace before sending content to the API. Now that the original untrimmed content is sent, cassettes need to reflect the actual bodies: - TestDebug_Title/Anthropic*: sessiontitle generator builds prompts with fmt.Fprintf('%d. %s\n', ...) + a trailing '\n\n' in the format string, producing three trailing newlines. The old TrimSpace stripped these; the new code sends them as-is. - TestExec_Anthropic_ToolCall: list_directory tool output ends with '\n' (from fmt.Fprintf 'FILE %s\n'). The old TrimSpace on tool results stripped this; the new code sends the original content. Assisted-By: docker-agent --- e2e/testdata/cassettes/TestDebug_Title/Anthropic.yaml | 2 +- e2e/testdata/cassettes/TestDebug_Title/Anthropic_Opus46.yaml | 2 +- e2e/testdata/cassettes/TestDebug_Title/Anthropic_Sonnet45.yaml | 2 +- e2e/testdata/cassettes/TestExec_Anthropic_ToolCall.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/e2e/testdata/cassettes/TestDebug_Title/Anthropic.yaml b/e2e/testdata/cassettes/TestDebug_Title/Anthropic.yaml index 30a5b46dc..34d66db12 100644 --- a/e2e/testdata/cassettes/TestDebug_Title/Anthropic.yaml +++ b/e2e/testdata/cassettes/TestDebug_Title/Anthropic.yaml @@ -8,7 +8,7 @@ interactions: proto_minor: 1 content_length: 0 host: api.anthropic.com - body: '{"max_tokens":20,"messages":[{"content":[{"text":"Based on the following recent user messages from a conversation with an AI assistant, generate a short, descriptive title (maximum 50 characters) that captures the main topic or purpose of the conversation. Return ONLY the title text on a single line, nothing else. Do not include any newlines, explanations, or formatting.\n\nRecent user messages:\n1. What can you do?","cache_control":{"type":"ephemeral"},"type":"text"}],"role":"user"}],"model":"claude-haiku-4-5","system":[{"text":"You are a helpful AI assistant that generates concise, descriptive titles for conversations. You will be given up to 2 recent user messages and asked to create a single-line title that captures the main topic. Never use newlines or line breaks in your response.","type":"text"}],"tools":[],"stream":true}' + body: '{"max_tokens":20,"messages":[{"content":[{"text":"Based on the following recent user messages from a conversation with an AI assistant, generate a short, descriptive title (maximum 50 characters) that captures the main topic or purpose of the conversation. Return ONLY the title text on a single line, nothing else. Do not include any newlines, explanations, or formatting.\n\nRecent user messages:\n1. What can you do?\n\n\n","cache_control":{"type":"ephemeral"},"type":"text"}],"role":"user"}],"model":"claude-haiku-4-5","system":[{"text":"You are a helpful AI assistant that generates concise, descriptive titles for conversations. You will be given up to 2 recent user messages and asked to create a single-line title that captures the main topic. Never use newlines or line breaks in your response.","type":"text"}],"tools":[],"stream":true}' url: https://api.anthropic.com/v1/messages method: POST response: diff --git a/e2e/testdata/cassettes/TestDebug_Title/Anthropic_Opus46.yaml b/e2e/testdata/cassettes/TestDebug_Title/Anthropic_Opus46.yaml index 9447dce85..53c90c441 100644 --- a/e2e/testdata/cassettes/TestDebug_Title/Anthropic_Opus46.yaml +++ b/e2e/testdata/cassettes/TestDebug_Title/Anthropic_Opus46.yaml @@ -8,7 +8,7 @@ interactions: proto_minor: 1 content_length: 0 host: api.anthropic.com - body: '{"max_tokens":20,"messages":[{"content":[{"text":"Based on the following recent user messages from a conversation with an AI assistant, generate a short, descriptive title (maximum 50 characters) that captures the main topic or purpose of the conversation. Return ONLY the title text on a single line, nothing else. Do not include any newlines, explanations, or formatting.\n\nRecent user messages:\n1. What can you do?","cache_control":{"type":"ephemeral"},"type":"text"}],"role":"user"}],"model":"claude-opus-4-6","system":[{"text":"You are a helpful AI assistant that generates concise, descriptive titles for conversations. You will be given up to 2 recent user messages and asked to create a single-line title that captures the main topic. Never use newlines or line breaks in your response.","type":"text"}],"tools":[],"stream":true}' + body: '{"max_tokens":20,"messages":[{"content":[{"text":"Based on the following recent user messages from a conversation with an AI assistant, generate a short, descriptive title (maximum 50 characters) that captures the main topic or purpose of the conversation. Return ONLY the title text on a single line, nothing else. Do not include any newlines, explanations, or formatting.\n\nRecent user messages:\n1. What can you do?\n\n\n","cache_control":{"type":"ephemeral"},"type":"text"}],"role":"user"}],"model":"claude-opus-4-6","system":[{"text":"You are a helpful AI assistant that generates concise, descriptive titles for conversations. You will be given up to 2 recent user messages and asked to create a single-line title that captures the main topic. Never use newlines or line breaks in your response.","type":"text"}],"tools":[],"stream":true}' url: https://api.anthropic.com/v1/messages method: POST response: diff --git a/e2e/testdata/cassettes/TestDebug_Title/Anthropic_Sonnet45.yaml b/e2e/testdata/cassettes/TestDebug_Title/Anthropic_Sonnet45.yaml index d5f738e00..e1c4a5d9d 100644 --- a/e2e/testdata/cassettes/TestDebug_Title/Anthropic_Sonnet45.yaml +++ b/e2e/testdata/cassettes/TestDebug_Title/Anthropic_Sonnet45.yaml @@ -8,7 +8,7 @@ interactions: proto_minor: 1 content_length: 0 host: api.anthropic.com - body: '{"max_tokens":20,"messages":[{"content":[{"text":"Based on the following recent user messages from a conversation with an AI assistant, generate a short, descriptive title (maximum 50 characters) that captures the main topic or purpose of the conversation. Return ONLY the title text on a single line, nothing else. Do not include any newlines, explanations, or formatting.\n\nRecent user messages:\n1. What can you do?","cache_control":{"type":"ephemeral"},"type":"text"}],"role":"user"}],"model":"claude-sonnet-4-5","system":[{"text":"You are a helpful AI assistant that generates concise, descriptive titles for conversations. You will be given up to 2 recent user messages and asked to create a single-line title that captures the main topic. Never use newlines or line breaks in your response.","type":"text"}],"tools":[],"stream":true}' + body: '{"max_tokens":20,"messages":[{"content":[{"text":"Based on the following recent user messages from a conversation with an AI assistant, generate a short, descriptive title (maximum 50 characters) that captures the main topic or purpose of the conversation. Return ONLY the title text on a single line, nothing else. Do not include any newlines, explanations, or formatting.\n\nRecent user messages:\n1. What can you do?\n\n\n","cache_control":{"type":"ephemeral"},"type":"text"}],"role":"user"}],"model":"claude-sonnet-4-5","system":[{"text":"You are a helpful AI assistant that generates concise, descriptive titles for conversations. You will be given up to 2 recent user messages and asked to create a single-line title that captures the main topic. Never use newlines or line breaks in your response.","type":"text"}],"tools":[],"stream":true}' url: https://api.anthropic.com/v1/messages method: POST response: diff --git a/e2e/testdata/cassettes/TestExec_Anthropic_ToolCall.yaml b/e2e/testdata/cassettes/TestExec_Anthropic_ToolCall.yaml index 016c89df5..425a41a4b 100644 --- a/e2e/testdata/cassettes/TestExec_Anthropic_ToolCall.yaml +++ b/e2e/testdata/cassettes/TestExec_Anthropic_ToolCall.yaml @@ -55,7 +55,7 @@ interactions: proto_minor: 1 content_length: 0 host: api.anthropic.com - body: '{"max_tokens":64000,"messages":[{"content":[{"text":"How many files in testdata/working_dir? Only output the number.","type":"text"}],"role":"user"},{"content":[{"id":"toolu_012gmfqnoTX8c5aV3vMWUnas","input":{"path":"testdata/working_dir"},"name":"list_directory","cache_control":{"type":"ephemeral"},"type":"tool_use"}],"role":"assistant"},{"content":[{"tool_use_id":"toolu_012gmfqnoTX8c5aV3vMWUnas","is_error":false,"cache_control":{"type":"ephemeral"},"content":[{"text":"FILE README.me","type":"text"}],"type":"tool_result"}],"role":"user"}],"model":"claude-sonnet-4-0","system":[{"text":"You are a knowledgeable assistant that helps users with various tasks.\nBe helpful, accurate, and concise in your responses.","type":"text"},{"text":"## Filesystem Tools\n\n- Relative paths resolve from the working directory; absolute paths and \"..\" work as expected\n- Prefer read_multiple_files over sequential read_file calls\n- Use search_files_content to locate code or text across files\n- Use exclude patterns in searches and max_depth in directory_tree to limit output","cache_control":{"type":"ephemeral"},"type":"text"}],"tools":[{"input_schema":{"properties":{"path":{"description":"The directory path to traverse (relative to working directory)","type":"string"}},"required":["path"],"type":"object"},"name":"directory_tree","description":"Get a recursive tree view of files and directories as a JSON structure."},{"input_schema":{"properties":{"edits":{"description":"Array of edit operations","items":{"additionalProperties":false,"properties":{"newText":{"description":"The replacement text","type":"string"},"oldText":{"description":"The exact text to replace","type":"string"}},"required":["oldText","newText"],"type":"object"},"type":["null","array"]},"path":{"description":"The file path to edit","type":"string"}},"required":["path","edits"],"type":"object"},"name":"edit_file","description":"Make line-based edits to a text file. Each edit replaces exact line sequences with new content."},{"input_schema":{"properties":{"path":{"description":"The directory path to list","type":"string"}},"required":["path"],"type":"object"},"name":"list_directory","description":"Get a detailed listing of all files and directories in a specified path."},{"input_schema":{"properties":{"path":{"description":"The file path to read","type":"string"}},"required":["path"],"type":"object"},"name":"read_file","description":"Read the complete contents of a file from the file system. Supports text files and images (jpg, png, gif, webp). Images are returned as image content that you can view directly."},{"input_schema":{"properties":{"json":{"description":"Whether to return the result as JSON","type":"boolean"},"paths":{"description":"Array of file paths to read","items":{"type":"string"},"type":["null","array"]}},"required":["paths"],"type":"object"},"name":"read_multiple_files","description":"Read the contents of multiple files simultaneously."},{"input_schema":{"properties":{"excludePatterns":{"description":"Patterns to exclude from search","items":{"type":"string"},"type":["null","array"]},"is_regex":{"description":"If true, treat query as regex; otherwise literal text","type":"boolean"},"path":{"description":"The starting directory path","type":"string"},"query":{"description":"The text or regex pattern to search for","type":"string"}},"required":["path","query"],"type":"object"},"name":"search_files_content","description":"Searches for text or regex patterns in the content of files matching a GLOB pattern."},{"input_schema":{"properties":{"content":{"description":"The content to write to the file","type":"string"},"path":{"description":"The file path to write","type":"string"}},"required":["path","content"],"type":"object"},"name":"write_file","description":"Create a new file or completely overwrite an existing file with new content."},{"input_schema":{"properties":{"paths":{"description":"Array of directory paths to create","items":{"type":"string"},"type":["null","array"]}},"required":["paths"],"type":"object"},"name":"create_directory","description":"Create one or more new directories or nested directory structures."},{"input_schema":{"properties":{"paths":{"description":"Array of directory paths to remove","items":{"type":"string"},"type":["null","array"]}},"required":["paths"],"type":"object"},"name":"remove_directory","description":"Remove one or more empty directories."}],"stream":true}' + body: '{"max_tokens":64000,"messages":[{"content":[{"text":"How many files in testdata/working_dir? Only output the number.","type":"text"}],"role":"user"},{"content":[{"id":"toolu_012gmfqnoTX8c5aV3vMWUnas","input":{"path":"testdata/working_dir"},"name":"list_directory","cache_control":{"type":"ephemeral"},"type":"tool_use"}],"role":"assistant"},{"content":[{"tool_use_id":"toolu_012gmfqnoTX8c5aV3vMWUnas","is_error":false,"cache_control":{"type":"ephemeral"},"content":[{"text":"FILE README.me\n","type":"text"}],"type":"tool_result"}],"role":"user"}],"model":"claude-sonnet-4-0","system":[{"text":"You are a knowledgeable assistant that helps users with various tasks.\nBe helpful, accurate, and concise in your responses.","type":"text"},{"text":"## Filesystem Tools\n\n- Relative paths resolve from the working directory; absolute paths and \"..\" work as expected\n- Prefer read_multiple_files over sequential read_file calls\n- Use search_files_content to locate code or text across files\n- Use exclude patterns in searches and max_depth in directory_tree to limit output","cache_control":{"type":"ephemeral"},"type":"text"}],"tools":[{"input_schema":{"properties":{"path":{"description":"The directory path to traverse (relative to working directory)","type":"string"}},"required":["path"],"type":"object"},"name":"directory_tree","description":"Get a recursive tree view of files and directories as a JSON structure."},{"input_schema":{"properties":{"edits":{"description":"Array of edit operations","items":{"additionalProperties":false,"properties":{"newText":{"description":"The replacement text","type":"string"},"oldText":{"description":"The exact text to replace","type":"string"}},"required":["oldText","newText"],"type":"object"},"type":["null","array"]},"path":{"description":"The file path to edit","type":"string"}},"required":["path","edits"],"type":"object"},"name":"edit_file","description":"Make line-based edits to a text file. Each edit replaces exact line sequences with new content."},{"input_schema":{"properties":{"path":{"description":"The directory path to list","type":"string"}},"required":["path"],"type":"object"},"name":"list_directory","description":"Get a detailed listing of all files and directories in a specified path."},{"input_schema":{"properties":{"path":{"description":"The file path to read","type":"string"}},"required":["path"],"type":"object"},"name":"read_file","description":"Read the complete contents of a file from the file system. Supports text files and images (jpg, png, gif, webp). Images are returned as image content that you can view directly."},{"input_schema":{"properties":{"json":{"description":"Whether to return the result as JSON","type":"boolean"},"paths":{"description":"Array of file paths to read","items":{"type":"string"},"type":["null","array"]}},"required":["paths"],"type":"object"},"name":"read_multiple_files","description":"Read the contents of multiple files simultaneously."},{"input_schema":{"properties":{"excludePatterns":{"description":"Patterns to exclude from search","items":{"type":"string"},"type":["null","array"]},"is_regex":{"description":"If true, treat query as regex; otherwise literal text","type":"boolean"},"path":{"description":"The starting directory path","type":"string"},"query":{"description":"The text or regex pattern to search for","type":"string"}},"required":["path","query"],"type":"object"},"name":"search_files_content","description":"Searches for text or regex patterns in the content of files matching a GLOB pattern."},{"input_schema":{"properties":{"content":{"description":"The content to write to the file","type":"string"},"path":{"description":"The file path to write","type":"string"}},"required":["path","content"],"type":"object"},"name":"write_file","description":"Create a new file or completely overwrite an existing file with new content."},{"input_schema":{"properties":{"paths":{"description":"Array of directory paths to create","items":{"type":"string"},"type":["null","array"]}},"required":["paths"],"type":"object"},"name":"create_directory","description":"Create one or more new directories or nested directory structures."},{"input_schema":{"properties":{"paths":{"description":"Array of directory paths to remove","items":{"type":"string"},"type":["null","array"]}},"required":["paths"],"type":"object"},"name":"remove_directory","description":"Remove one or more empty directories."}],"stream":true}' url: https://api.anthropic.com/v1/messages method: POST response: From 7a8ce07f0100422f4e290cb68caf4c8b7074f5ac Mon Sep 17 00:00:00 2001 From: Simon Ferquel's Clanker Date: Mon, 27 Apr 2026 08:55:04 +0000 Subject: [PATCH 4/5] refactor: centralise whitespace-only message filtering in session.GetMessages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses reviewer feedback: 'Is it possible to normalise the behaviour in only one place? In the place where we make the slice of messages of the session?' Add normalizeMessageContent() to session.GetMessages as the single, authoritative place where whitespace-only content is pruned before any provider converter sees it. This replaces the patchwork of per-provider TrimSpace skip-guards. Rules applied by normalizeMessageContent: - Tool-result messages (Role=tool) are always forwarded, even when empty, because every tool_use block must have a corresponding tool_result. - Assistant messages with ToolCalls or FunctionCall are always forwarded even when Content is empty — the content field is optional when tool calls are present. - Any other message whose Content is whitespace-only (and has no MultiContent) is dropped. - Within MultiContent, text parts whose Text is whitespace-only are stripped; non-text parts (images, files) are preserved as-is. If all parts are stripped the whole message is dropped. Provider-converter changes: - All per-converter TrimSpace skip-guards for user/assistant/system messages removed (they are now redundant): Anthropic std+Beta convertMessages, oaistream ConvertMultiContent + tool-role multi-content, OpenAI Responses API user multi-content, Gemini convertMessagesToGemini + convertMultiContent. - Pre-existing assistant empty-skip guards in oaistream and openai/client.go are kept: they handle streaming artifacts (max_tokens assistant deltas) that are produced outside of GetMessages and may not pass through normalizeMessageContent. - Anthropic system blocks (extractSystemBlocks / extractBetaSystemBlocks) continue to TrimSpace because YAML literal-block instructions (instruction: |) always append a trailing newline, and the system-block builder is the right place to normalise that for the Anthropic API payload. - The Anthropic send-original correctness fix (tool_result and conversation-message content: send original string, not trimmed value) is unchanged. Tests updated throughout to document the new 'converter passes through, session layer filters' contract. Assisted-By: docker-agent --- .../provider/anthropic/beta_client_test.go | 3 +- .../provider/anthropic/beta_converter.go | 22 ++-- pkg/model/provider/anthropic/client.go | 27 ++-- pkg/model/provider/anthropic/client_test.go | 55 ++++---- pkg/model/provider/bedrock/client_test.go | 8 +- pkg/model/provider/bedrock/convert.go | 16 ++- pkg/model/provider/gemini/client.go | 5 +- pkg/model/provider/oaistream/messages.go | 6 - pkg/model/provider/oaistream/messages_test.go | 6 +- pkg/model/provider/openai/client.go | 3 - pkg/session/session.go | 53 ++++++++ pkg/session/session_test.go | 119 ++++++++++++++++++ 12 files changed, 247 insertions(+), 76 deletions(-) diff --git a/pkg/model/provider/anthropic/beta_client_test.go b/pkg/model/provider/anthropic/beta_client_test.go index 1115a9980..ab105a689 100644 --- a/pkg/model/provider/anthropic/beta_client_test.go +++ b/pkg/model/provider/anthropic/beta_client_test.go @@ -215,7 +215,8 @@ func TestExtractBetaSystemBlocks_MultipleSystemMessages(t *testing.T) { assert.Equal(t, "Be concise", blocks[1].Text) } -// TestExtractBetaSystemBlocks_SkipsEmptyText tests that empty system text is skipped +// TestExtractBetaSystemBlocks_SkipsEmptyText tests that empty system text is skipped. +// System blocks are trimmed because YAML literal-block instructions always append a trailing newline. func TestExtractBetaSystemBlocks_SkipsEmptyText(t *testing.T) { msgs := []chat.Message{ { diff --git a/pkg/model/provider/anthropic/beta_converter.go b/pkg/model/provider/anthropic/beta_converter.go index 2671d6de1..6fd2a6d54 100644 --- a/pkg/model/provider/anthropic/beta_converter.go +++ b/pkg/model/provider/anthropic/beta_converter.go @@ -43,7 +43,7 @@ func (c *Client) convertBetaMessages(ctx context.Context, messages []chat.Messag Content: contentBlocks, }) } - } else if strings.TrimSpace(msg.Content) != "" { + } else { betaMessages = append(betaMessages, anthropic.BetaMessageParam{ Role: anthropic.BetaMessageParamRoleUser, Content: []anthropic.BetaContentBlockParamUnion{ @@ -68,9 +68,9 @@ func (c *Client) convertBetaMessages(ctx context.Context, messages []chat.Messag } // Add text content if present - if txt := strings.TrimSpace(msg.Content); txt != "" { + if msg.Content != "" { contentBlocks = append(contentBlocks, anthropic.BetaContentBlockParamUnion{ - OfText: &anthropic.BetaTextBlockParam{Text: txt}, + OfText: &anthropic.BetaTextBlockParam{Text: msg.Content}, }) } @@ -158,11 +158,9 @@ func convertBetaToolResultBlock(msg *chat.Message) anthropic.BetaContentBlockPar for _, part := range msg.MultiContent { switch part.Type { case chat.MessagePartTypeText: - if strings.TrimSpace(part.Text) != "" { - content = append(content, anthropic.BetaToolResultBlockParamContentUnion{ - OfText: &anthropic.BetaTextBlockParam{Text: part.Text}, - }) - } + content = append(content, anthropic.BetaToolResultBlockParamContentUnion{ + OfText: &anthropic.BetaTextBlockParam{Text: part.Text}, + }) case chat.MessagePartTypeImageURL: if part.ImageURL == nil { continue @@ -200,11 +198,9 @@ func (c *Client) convertBetaUserMultiContent(ctx context.Context, parts []chat.M for _, part := range parts { switch part.Type { case chat.MessagePartTypeText: - if strings.TrimSpace(part.Text) != "" { - contentBlocks = append(contentBlocks, anthropic.BetaContentBlockParamUnion{ - OfText: &anthropic.BetaTextBlockParam{Text: part.Text}, - }) - } + contentBlocks = append(contentBlocks, anthropic.BetaContentBlockParamUnion{ + OfText: &anthropic.BetaTextBlockParam{Text: part.Text}, + }) case chat.MessagePartTypeImageURL: if part.ImageURL == nil { diff --git a/pkg/model/provider/anthropic/client.go b/pkg/model/provider/anthropic/client.go index a796e94f4..266494a66 100644 --- a/pkg/model/provider/anthropic/client.go +++ b/pkg/model/provider/anthropic/client.go @@ -304,7 +304,7 @@ func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) ( if len(contentBlocks) > 0 { anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(contentBlocks...)) } - } else if strings.TrimSpace(msg.Content) != "" { + } else { anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(anthropic.NewTextBlock(msg.Content))) } continue @@ -321,9 +321,8 @@ func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) ( if len(msg.ToolCalls) > 0 { blockLen := len(msg.ToolCalls) - msgContent := strings.TrimSpace(msg.Content) offset := 0 - if msgContent != "" { + if msg.Content != "" { blockLen++ } toolUseBlocks := make([]anthropic.ContentBlockParamUnion, blockLen) @@ -331,8 +330,8 @@ func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) ( if len(contentBlocks) > 0 { toolUseBlocks = append(contentBlocks, toolUseBlocks...) } - if msgContent != "" { - toolUseBlocks[len(contentBlocks)+offset] = anthropic.NewTextBlock(msgContent) + if msg.Content != "" { + toolUseBlocks[len(contentBlocks)+offset] = anthropic.NewTextBlock(msg.Content) offset = 1 } for j, toolCall := range msg.ToolCalls { @@ -352,8 +351,8 @@ func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) ( // Mark that we expect the very next message to be the grouped tool_result blocks. pendingAssistantToolUse = true } else { - if txt := strings.TrimSpace(msg.Content); txt != "" { - contentBlocks = append(contentBlocks, anthropic.NewTextBlock(txt)) + if msg.Content != "" { + contentBlocks = append(contentBlocks, anthropic.NewTextBlock(msg.Content)) } if len(contentBlocks) > 0 { anthropicMessages = append(anthropicMessages, anthropic.NewAssistantMessage(contentBlocks...)) @@ -416,11 +415,9 @@ func convertToolResultBlock(msg *chat.Message) anthropic.ContentBlockParamUnion for _, part := range msg.MultiContent { switch part.Type { case chat.MessagePartTypeText: - if strings.TrimSpace(part.Text) != "" { - content = append(content, anthropic.ToolResultBlockParamContentUnion{ - OfText: &anthropic.TextBlockParam{Text: part.Text}, - }) - } + content = append(content, anthropic.ToolResultBlockParamContentUnion{ + OfText: &anthropic.TextBlockParam{Text: part.Text}, + }) case chat.MessagePartTypeImageURL: if part.ImageURL == nil { continue @@ -487,9 +484,7 @@ func (c *Client) convertUserMultiContent(_ context.Context, parts []chat.Message for _, part := range parts { switch part.Type { case chat.MessagePartTypeText: - if strings.TrimSpace(part.Text) != "" { - contentBlocks = append(contentBlocks, anthropic.NewTextBlock(part.Text)) - } + contentBlocks = append(contentBlocks, anthropic.NewTextBlock(part.Text)) case chat.MessagePartTypeImageURL: if part.ImageURL == nil { @@ -586,6 +581,8 @@ func extractSystemBlocks(messages []chat.Message) []anthropic.TextBlockParam { } } } else if txt := strings.TrimSpace(msg.Content); txt != "" { + // Trim system-message content: YAML literal blocks (instruction: |) always + // append a trailing newline, and we don't want that in API payloads. systemBlocks = append(systemBlocks, anthropic.TextBlockParam{ Text: txt, }) diff --git a/pkg/model/provider/anthropic/client_test.go b/pkg/model/provider/anthropic/client_test.go index e696b0e74..ad52fba70 100644 --- a/pkg/model/provider/anthropic/client_test.go +++ b/pkg/model/provider/anthropic/client_test.go @@ -64,14 +64,6 @@ func TestCreateChatCompletionStream_ErrorOnEmptyMessages(t *testing.T) { {Role: chat.MessageRoleSystem, Content: "You are helpful."}, }, }, - { - name: "only whitespace content", - messages: []chat.Message{ - {Role: chat.MessageRoleSystem, Content: "System prompt"}, - {Role: chat.MessageRoleUser, Content: " "}, - {Role: chat.MessageRoleAssistant, Content: " \t\n "}, - }, - }, } for _, tt := range tests { @@ -83,26 +75,38 @@ func TestCreateChatCompletionStream_ErrorOnEmptyMessages(t *testing.T) { } } +// TestConvertMessages_SkipEmptySystemText documents that the converter no longer +// filters whitespace-only system messages — normalizeMessageContent in the session +// layer does this before messages reach any provider converter. func TestConvertMessages_SkipEmptySystemText(t *testing.T) { msgs := []chat.Message{{ Role: chat.MessageRoleSystem, Content: " \n\t ", }} + // System messages are extracted into system blocks by extractSystemBlocks; + // that helper now passes them through as-is. The session layer is + // responsible for not sending whitespace-only system messages. out, err := testClient().convertMessages(t.Context(), msgs) require.NoError(t, err) + // System messages are not included in the anthropic message list (they go + // to extractSystemBlocks instead), so out is still empty. assert.Empty(t, out) } +// TestConvertMessages_SkipEmptyUserText_NoMultiContent documents that whitespace +// filtering is now the session layer's responsibility, not the converter's. func TestConvertMessages_SkipEmptyUserText_NoMultiContent(t *testing.T) { msgs := []chat.Message{{ Role: chat.MessageRoleUser, Content: " \n\t ", }} + // The converter forwards the message as-is; normalizeMessageContent in the + // session layer drops it before it reaches the converter in real usage. out, err := testClient().convertMessages(t.Context(), msgs) require.NoError(t, err) - assert.Empty(t, out) + assert.Len(t, out, 1) } func TestConvertMessages_UserMultiContent_SkipEmptyText_KeepImage(t *testing.T) { @@ -120,30 +124,30 @@ func TestConvertMessages_UserMultiContent_SkipEmptyText_KeepImage(t *testing.T) b, err := json.Marshal(out[0]) require.NoError(t, err) - // Basic JSON structure checks var m map[string]any require.NoError(t, json.Unmarshal(b, &m)) - // role should be user assert.Equal(t, "user", m["role"]) - // content should contain exactly one block (the image) + // The converter now forwards all parts as-is. normalizeMessageContent in the + // session layer strips whitespace-only text parts before calling the converter + // in real usage, so both parts appear here when tested directly. content, ok := m["content"].([]any) require.True(t, ok) - assert.Len(t, content, 1) - // and it should be an image block - cb, ok := content[0].(map[string]any) - require.True(t, ok) - assert.Equal(t, "image", cb["type"]) + assert.Len(t, content, 2) } +// TestConvertMessages_SkipEmptyAssistantText_NoToolCalls documents that the +// converter no longer filters whitespace-only assistant messages. func TestConvertMessages_SkipEmptyAssistantText_NoToolCalls(t *testing.T) { msgs := []chat.Message{{ Role: chat.MessageRoleAssistant, Content: " \t\n ", }} + // The converter forwards the message; normalizeMessageContent in the session + // layer drops whitespace-only assistant messages before they reach here. out, err := testClient().convertMessages(t.Context(), msgs) require.NoError(t, err) - assert.Empty(t, out) + assert.Len(t, out, 1) } func TestConvertMessages_AssistantToolCalls_NoText_IncludesToolUse(t *testing.T) { @@ -166,8 +170,11 @@ func TestConvertMessages_AssistantToolCalls_NoText_IncludesToolUse(t *testing.T) assert.Equal(t, "assistant", m["role"]) content, ok := m["content"].([]any) require.True(t, ok) - assert.Len(t, content, 1) - cb, ok := content[0].(map[string]any) + // The whitespace text content is now included alongside the tool_use block + // because the converter no longer strips it. The session layer would have + // already cleaned this up in real usage via normalizeMessageContent. + assert.Len(t, content, 2) + cb, ok := content[1].(map[string]any) require.True(t, ok) assert.Equal(t, "tool_use", cb["type"]) } @@ -444,7 +451,9 @@ func TestExtractSystemBlocks_MultipleSystemMessages(t *testing.T) { assert.Equal(t, "Be concise", blocks[1].Text) } -// TestExtractSystemBlocks_SkipsEmptyText tests that empty system text is skipped +// TestExtractSystemBlocks_SkipsEmptyText tests that empty/whitespace-only system text is skipped. +// System blocks are trimmed because YAML literal-block instructions (instruction: |) always +// append a trailing newline that should not be sent to the API. func TestExtractSystemBlocks_SkipsEmptyText(t *testing.T) { msgs := []chat.Message{ { @@ -528,6 +537,8 @@ func TestExtractSystemBlocks_EmptyContentWithCacheControl(t *testing.T) { t.Parallel() // An empty system message with CacheControl must not panic. + // Since extractSystemBlocks now trims system content, an empty/whitespace-only + // message produces no block, so CacheControl has nothing to apply to. msgs := []chat.Message{ { Role: chat.MessageRoleSystem, @@ -536,7 +547,7 @@ func TestExtractSystemBlocks_EmptyContentWithCacheControl(t *testing.T) { }, } - // Before the fix this panicked with index out of range [-1]. + // Must not panic; the empty block is skipped. blocks := extractSystemBlocks(msgs) assert.Empty(t, blocks) } diff --git a/pkg/model/provider/bedrock/client_test.go b/pkg/model/provider/bedrock/client_test.go index f299d3b1e..3c46b3ab1 100644 --- a/pkg/model/provider/bedrock/client_test.go +++ b/pkg/model/provider/bedrock/client_test.go @@ -105,13 +105,19 @@ func TestConvertMessages_ToolResult(t *testing.T) { func TestConvertMessages_EmptyContent(t *testing.T) { t.Parallel() + // Whitespace-only messages are filtered by session.normalizeMessageContent + // before they reach the provider converter, so the converter itself no longer + // needs to guard against them. This test documents that the converter passes + // them through as-is (empty content blocks); it is the session layer's job + // to ensure such messages never arrive here. msgs := []chat.Message{ {Role: chat.MessageRoleUser, Content: ""}, {Role: chat.MessageRoleUser, Content: " "}, } bedrockMsgs, _ := convertMessages(msgs, false) - assert.Empty(t, bedrockMsgs) + // Both messages now produce user turns with empty or whitespace content blocks. + assert.Len(t, bedrockMsgs, 2) } func TestConvertToolConfig(t *testing.T) { diff --git a/pkg/model/provider/bedrock/convert.go b/pkg/model/provider/bedrock/convert.go index 060b7e137..9b5ae957e 100644 --- a/pkg/model/provider/bedrock/convert.go +++ b/pkg/model/provider/bedrock/convert.go @@ -29,13 +29,13 @@ func convertMessages(messages []chat.Message, enableCaching bool) ([]types.Messa // Extract system messages into separate system blocks if len(msg.MultiContent) > 0 { for _, part := range msg.MultiContent { - if part.Type == chat.MessagePartTypeText && strings.TrimSpace(part.Text) != "" { + if part.Type == chat.MessagePartTypeText { systemBlocks = append(systemBlocks, &types.SystemContentBlockMemberText{ Value: part.Text, }) } } - } else if strings.TrimSpace(msg.Content) != "" { + } else { systemBlocks = append(systemBlocks, &types.SystemContentBlockMemberText{ Value: msg.Content, }) @@ -126,11 +126,9 @@ func convertUserContent(msg *chat.Message) []types.ContentBlock { for _, part := range msg.MultiContent { switch part.Type { case chat.MessagePartTypeText: - if strings.TrimSpace(part.Text) != "" { - blocks = append(blocks, &types.ContentBlockMemberText{ - Value: part.Text, - }) - } + blocks = append(blocks, &types.ContentBlockMemberText{ + Value: part.Text, + }) case chat.MessagePartTypeImageURL: if part.ImageURL != nil { if imageBlock := convertImageURL(part.ImageURL); imageBlock != nil { @@ -139,7 +137,7 @@ func convertUserContent(msg *chat.Message) []types.ContentBlock { } } } - } else if strings.TrimSpace(msg.Content) != "" { + } else { blocks = append(blocks, &types.ContentBlockMemberText{ Value: msg.Content, }) @@ -212,7 +210,7 @@ func convertAssistantContent(msg *chat.Message) []types.ContentBlock { } // Add text content if present - if strings.TrimSpace(msg.Content) != "" { + if msg.Content != "" { blocks = append(blocks, &types.ContentBlockMemberText{ Value: msg.Content, }) diff --git a/pkg/model/provider/gemini/client.go b/pkg/model/provider/gemini/client.go index ff67cda55..e48e6f6e4 100644 --- a/pkg/model/provider/gemini/client.go +++ b/pkg/model/provider/gemini/client.go @@ -261,7 +261,7 @@ func convertMessagesToGemini(messages []chat.Message) []*genai.Content { if len(parts) > 0 { contents = append(contents, genai.NewContentFromParts(parts, role)) } - } else if strings.TrimSpace(msg.Content) != "" { + } else if msg.Content != "" { part := newTextPartWithSignature(msg.Content, msg.ThoughtSignature) contents = append(contents, genai.NewContentFromParts([]*genai.Part{part}, role)) } @@ -292,9 +292,6 @@ func convertMultiContent(multiContent []chat.MessagePart, thoughtSignature []byt for _, part := range multiContent { switch part.Type { case chat.MessagePartTypeText: - if strings.TrimSpace(part.Text) == "" { - continue - } parts = append(parts, newTextPartWithSignature(part.Text, thoughtSignature)) case chat.MessagePartTypeImageURL: if imgPart := convertImageURLToPart(part.ImageURL); imgPart != nil { diff --git a/pkg/model/provider/oaistream/messages.go b/pkg/model/provider/oaistream/messages.go index 7b82e591e..0408a48a2 100644 --- a/pkg/model/provider/oaistream/messages.go +++ b/pkg/model/provider/oaistream/messages.go @@ -29,9 +29,6 @@ func ConvertMultiContent(multiContent []chat.MessagePart) []openai.ChatCompletio for _, part := range multiContent { switch part.Type { case chat.MessagePartTypeText: - if strings.TrimSpace(part.Text) == "" { - continue - } parts = append(parts, openai.TextContentPart(part.Text)) case chat.MessagePartTypeImageURL: if part.ImageURL != nil { @@ -142,9 +139,6 @@ func ConvertMessages(messages []chat.Message) []openai.ChatCompletionMessagePara textParts := make([]openai.ChatCompletionContentPartTextParam, 0) for _, part := range msg.MultiContent { if part.Type == chat.MessagePartTypeText { - if strings.TrimSpace(part.Text) == "" { - continue - } textParts = append(textParts, openai.ChatCompletionContentPartTextParam{ Text: part.Text, }) diff --git a/pkg/model/provider/oaistream/messages_test.go b/pkg/model/provider/oaistream/messages_test.go index db4f4d55f..51282dba0 100644 --- a/pkg/model/provider/oaistream/messages_test.go +++ b/pkg/model/provider/oaistream/messages_test.go @@ -48,12 +48,14 @@ func TestConvertMultiContent(t *testing.T) { wantCount: 0, }, { - name: "whitespace-only text part skipped", + // The converter forwards all parts as-is; normalizeMessageContent in the + // session layer strips whitespace-only text parts before real usage. + name: "whitespace-only text part forwarded as-is", multiContent: []chat.MessagePart{ {Type: chat.MessagePartTypeText, Text: " "}, {Type: chat.MessagePartTypeText, Text: "hello"}, }, - wantCount: 1, + wantCount: 2, }, } diff --git a/pkg/model/provider/openai/client.go b/pkg/model/provider/openai/client.go index 7ab1a9b74..c9c24e57e 100644 --- a/pkg/model/provider/openai/client.go +++ b/pkg/model/provider/openai/client.go @@ -574,9 +574,6 @@ func convertMessagesToResponseInput(messages []chat.Message) []responses.Respons for _, part := range msg.MultiContent { switch part.Type { case chat.MessagePartTypeText: - if strings.TrimSpace(part.Text) == "" { - continue - } contentParts = append(contentParts, responses.ResponseInputContentUnionParam{ OfInputText: &responses.ResponseInputTextParam{ Text: part.Text, diff --git a/pkg/session/session.go b/pkg/session/session.go index 51c84259d..0e0602e7b 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -916,6 +916,7 @@ func (s *Session) GetMessages(a *agent.Agent) []chat.Message { messages = truncateOldToolContent(messages, maxOldToolCallTokens) } + messages = normalizeMessageContent(messages) messages = sanitizeToolCalls(messages) systemCount := 0 @@ -1013,6 +1014,58 @@ func trimMessages(messages []chat.Message, maxItems int) []chat.Message { return result } +// normalizeMessageContent strips purely-whitespace content from messages before +// they reach any provider converter. Specifically: +// +// - Non-tool messages whose Content is whitespace-only and have no MultiContent +// are dropped entirely. Tool-result messages are exempt: every tool_use must +// have a corresponding tool_result, so we cannot skip them even when empty. +// - Text parts inside MultiContent whose Text is whitespace-only are removed. +// A non-tool message that becomes part-less after this pruning is also dropped. +// +// This is the single authoritative guard; individual provider converters do not +// need their own whitespace-skip guards for user/system/assistant messages. +func normalizeMessageContent(messages []chat.Message) []chat.Message { + out := messages[:0:0] // reuse underlying array, length 0 + for _, msg := range messages { // Tool results must always be forwarded — even empty — because the API + // requires a tool_result for every preceding tool_use block. + if msg.Role == chat.MessageRoleTool { + out = append(out, msg) + continue + } + + if len(msg.MultiContent) > 0 { + // Filter whitespace-only text parts; preserve image/file parts as-is. + filtered := msg.MultiContent[:0:0] + for _, part := range msg.MultiContent { + if part.Type == chat.MessagePartTypeText && strings.TrimSpace(part.Text) == "" { + continue + } + filtered = append(filtered, part) + } + if len(filtered) == 0 { + // All parts were whitespace-only text — drop the whole message. + continue + } + msg.MultiContent = filtered + out = append(out, msg) + continue + } + + // Single-part: drop messages with whitespace-only Content, but only when + // there are no tool calls or function calls attached. An assistant message + // with an empty text body but tool_use blocks is valid and must be kept. + if strings.TrimSpace(msg.Content) == "" && len(msg.ToolCalls) == 0 && msg.FunctionCall == nil { + continue + } + out = append(out, msg) + } + if len(out) == 0 { + return nil + } + return out +} + // sanitizeToolCalls ensures every tool call in assistant messages has a // corresponding tool-result message. It walks the message list tracking // pending tool calls; when a tool-result message arrives its ID is marked diff --git a/pkg/session/session_test.go b/pkg/session/session_test.go index 55bf1076c..1a87d5d52 100644 --- a/pkg/session/session_test.go +++ b/pkg/session/session_test.go @@ -574,3 +574,122 @@ func TestTransferTaskPromptExcludesParents(t *testing.T) { assert.Contains(t, subAgentMsg, "librarian", "should list librarian as a valid sub-agent") assert.NotContains(t, subAgentMsg, "planner", "should NOT list parent agent planner as a valid transfer target") } + +func TestNormalizeMessageContent(t *testing.T) { + t.Parallel() + + img := chat.MessagePart{Type: chat.MessagePartTypeImageURL, ImageURL: &chat.MessageImageURL{URL: "data:image/png;base64,AAAA"}} + + tests := []struct { + name string + input []chat.Message + want []chat.Message + }{ + { + name: "empty input", + input: nil, + want: nil, + }, + { + name: "whitespace-only user message dropped", + input: []chat.Message{ + {Role: chat.MessageRoleUser, Content: " \n\t "}, + }, + want: nil, + }, + { + name: "whitespace-only system message dropped", + input: []chat.Message{ + {Role: chat.MessageRoleSystem, Content: " "}, + }, + want: nil, + }, + { + name: "whitespace-only assistant message dropped", + input: []chat.Message{ + {Role: chat.MessageRoleAssistant, Content: "\t\n"}, + }, + want: nil, + }, + { + name: "assistant with empty content but tool calls is kept", + input: []chat.Message{ + {Role: chat.MessageRoleAssistant, Content: "", ToolCalls: []tools.ToolCall{{ID: "tc1"}}}, + }, + want: []chat.Message{ + {Role: chat.MessageRoleAssistant, Content: "", ToolCalls: []tools.ToolCall{{ID: "tc1"}}}, + }, + }, + { + name: "tool result always forwarded even if whitespace-only", + input: []chat.Message{ + {Role: chat.MessageRoleTool, Content: " ", ToolCallID: "t1"}, + }, + want: []chat.Message{ + {Role: chat.MessageRoleTool, Content: " ", ToolCallID: "t1"}, + }, + }, + { + name: "non-empty messages preserved verbatim including leading/trailing space", + input: []chat.Message{ + {Role: chat.MessageRoleUser, Content: " hello "}, + }, + want: []chat.Message{ + {Role: chat.MessageRoleUser, Content: " hello "}, + }, + }, + { + name: "whitespace-only text part stripped from MultiContent", + input: []chat.Message{ + {Role: chat.MessageRoleUser, MultiContent: []chat.MessagePart{ + {Type: chat.MessagePartTypeText, Text: " "}, + img, + }}, + }, + want: []chat.Message{ + {Role: chat.MessageRoleUser, MultiContent: []chat.MessagePart{img}}, + }, + }, + { + name: "message dropped when all MultiContent parts are whitespace-only text", + input: []chat.Message{ + {Role: chat.MessageRoleUser, MultiContent: []chat.MessagePart{ + {Type: chat.MessagePartTypeText, Text: " "}, + {Type: chat.MessagePartTypeText, Text: "\t"}, + }}, + }, + want: nil, + }, + { + name: "image-only MultiContent message preserved", + input: []chat.Message{ + {Role: chat.MessageRoleUser, MultiContent: []chat.MessagePart{img}}, + }, + want: []chat.Message{ + {Role: chat.MessageRoleUser, MultiContent: []chat.MessagePart{img}}, + }, + }, + { + name: "mix: valid and whitespace messages", + input: []chat.Message{ + {Role: chat.MessageRoleSystem, Content: "be helpful"}, + {Role: chat.MessageRoleUser, Content: " "}, + {Role: chat.MessageRoleUser, Content: "hello"}, + {Role: chat.MessageRoleTool, Content: "", ToolCallID: "t1"}, + }, + want: []chat.Message{ + {Role: chat.MessageRoleSystem, Content: "be helpful"}, + {Role: chat.MessageRoleUser, Content: "hello"}, + {Role: chat.MessageRoleTool, Content: "", ToolCallID: "t1"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := normalizeMessageContent(tt.input) + assert.Equal(t, tt.want, got) + }) + } +} From a854cbdc4e59049985e3763fe4fa9c2d99c5be05 Mon Sep 17 00:00:00 2001 From: Simon Ferquel's Clanker Date: Mon, 27 Apr 2026 09:04:58 +0000 Subject: [PATCH 5/5] fix: replace tab with space before inline comment to satisfy gci formatter Assisted-By: docker-agent --- pkg/session/session.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/session/session.go b/pkg/session/session.go index 0e0602e7b..095197a8c 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -1026,8 +1026,8 @@ func trimMessages(messages []chat.Message, maxItems int) []chat.Message { // This is the single authoritative guard; individual provider converters do not // need their own whitespace-skip guards for user/system/assistant messages. func normalizeMessageContent(messages []chat.Message) []chat.Message { - out := messages[:0:0] // reuse underlying array, length 0 - for _, msg := range messages { // Tool results must always be forwarded — even empty — because the API + out := messages[:0:0] // reuse underlying array, length 0 + for _, msg := range messages { // Tool results must always be forwarded — even empty — because the API // requires a tool_result for every preceding tool_use block. if msg.Role == chat.MessageRoleTool { out = append(out, msg)