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: 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 fa1c04dcf..6fd2a6d54 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 { betaMessages = append(betaMessages, anthropic.BetaMessageParam{ Role: anthropic.BetaMessageParamRoleUser, Content: []anthropic.BetaContentBlockParamUnion{ - {OfText: &anthropic.BetaTextBlockParam{Text: txt}}, + {OfText: &anthropic.BetaTextBlockParam{Text: msg.Content}}, }, }) } @@ -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}, }) } @@ -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,11 +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 != "" { - content = append(content, anthropic.BetaToolResultBlockParamContentUnion{ - OfText: &anthropic.BetaTextBlockParam{Text: txt}, - }) - } + content = append(content, anthropic.BetaToolResultBlockParamContentUnion{ + OfText: &anthropic.BetaTextBlockParam{Text: part.Text}, + }) case chat.MessagePartTypeImageURL: if part.ImageURL == nil { continue @@ -194,11 +198,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 != "" { - contentBlocks = append(contentBlocks, anthropic.BetaContentBlockParamUnion{ - OfText: &anthropic.BetaTextBlockParam{Text: txt}, - }) - } + 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 3d6aea955..266494a66 100644 --- a/pkg/model/provider/anthropic/client.go +++ b/pkg/model/provider/anthropic/client.go @@ -305,9 +305,7 @@ 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))) - } + anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(anthropic.NewTextBlock(msg.Content))) } continue } @@ -323,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) @@ -333,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 { @@ -354,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...)) @@ -404,7 +401,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,11 +415,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 != "" { - content = append(content, anthropic.ToolResultBlockParamContentUnion{ - OfText: &anthropic.TextBlockParam{Text: txt}, - }) - } + content = append(content, anthropic.ToolResultBlockParamContentUnion{ + OfText: &anthropic.TextBlockParam{Text: part.Text}, + }) case chat.MessagePartTypeImageURL: if part.ImageURL == nil { continue @@ -483,9 +484,7 @@ 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)) - } + contentBlocks = append(contentBlocks, anthropic.NewTextBlock(part.Text)) case chat.MessagePartTypeImageURL: if part.ImageURL == nil { @@ -582,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/oaistream/messages.go b/pkg/model/provider/oaistream/messages.go index c82940f25..0408a48a2 100644 --- a/pkg/model/provider/oaistream/messages.go +++ b/pkg/model/provider/oaistream/messages.go @@ -25,17 +25,17 @@ 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) + 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), - }) + })) } } } diff --git a/pkg/model/provider/oaistream/messages_test.go b/pkg/model/provider/oaistream/messages_test.go index fa46ee0ff..51282dba0 100644 --- a/pkg/model/provider/oaistream/messages_test.go +++ b/pkg/model/provider/oaistream/messages_test.go @@ -41,11 +41,21 @@ 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: 1, + wantCount: 0, + }, + { + // 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: 2, }, } diff --git a/pkg/session/session.go b/pkg/session/session.go index 51c84259d..095197a8c 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) + }) + } +}