diff --git a/coderd/chatd/chatloop/chatloop.go b/coderd/chatd/chatloop/chatloop.go index 82002c1755..44feab87b6 100644 --- a/coderd/chatd/chatloop/chatloop.go +++ b/coderd/chatd/chatloop/chatloop.go @@ -127,7 +127,7 @@ func (r stepResult) toResponseMessages() []fantasy.Message { switch c.GetType() { case fantasy.ContentTypeText: text, ok := fantasy.AsContentType[fantasy.TextContent](c) - if !ok { + if !ok || strings.TrimSpace(text.Text) == "" { continue } assistantParts = append(assistantParts, fantasy.TextPart{ @@ -136,7 +136,7 @@ func (r stepResult) toResponseMessages() []fantasy.Message { }) case fantasy.ContentTypeReasoning: reasoning, ok := fantasy.AsContentType[fantasy.ReasoningContent](c) - if !ok { + if !ok || strings.TrimSpace(reasoning.Text) == "" { continue } assistantParts = append(assistantParts, fantasy.ReasoningPart{ diff --git a/coderd/chatd/chatloop/chatloop_test.go b/coderd/chatd/chatloop/chatloop_test.go index db7498ec3e..b0e7e46aa2 100644 --- a/coderd/chatd/chatloop/chatloop_test.go +++ b/coderd/chatd/chatloop/chatloop_test.go @@ -578,6 +578,84 @@ func TestToResponseMessages_ProviderExecutedToolResultInAssistantMessage(t *test assert.False(t, localTR.ProviderExecuted) } +func TestToResponseMessages_FiltersEmptyTextAndReasoningParts(t *testing.T) { + t.Parallel() + + sr := stepResult{ + content: []fantasy.Content{ + // Empty text — should be filtered. + fantasy.TextContent{Text: ""}, + // Whitespace-only text — should be filtered. + fantasy.TextContent{Text: " \t\n"}, + // Empty reasoning — should be filtered. + fantasy.ReasoningContent{Text: ""}, + // Whitespace-only reasoning — should be filtered. + fantasy.ReasoningContent{Text: " \n"}, + // Non-empty text — should pass through. + fantasy.TextContent{Text: "hello world"}, + // Leading/trailing whitespace with content — kept + // with the original value (not trimmed). + fantasy.TextContent{Text: " hello "}, + // Non-empty reasoning — should pass through. + fantasy.ReasoningContent{Text: "let me think"}, + // Tool call — should be unaffected by filtering. + fantasy.ToolCallContent{ + ToolCallID: "tc-1", + ToolName: "read_file", + Input: `{"path":"main.go"}`, + }, + // Local tool result — should be unaffected by filtering. + fantasy.ToolResultContent{ + ToolCallID: "tc-1", + ToolName: "read_file", + Result: fantasy.ToolResultOutputContentText{Text: "file contents"}, + }, + }, + } + + msgs := sr.toResponseMessages() + require.Len(t, msgs, 2, "expected assistant + tool messages") + + // First message: assistant role with non-empty text, reasoning, + // and the tool call. The four empty/whitespace-only parts must + // have been dropped. + assistantMsg := msgs[0] + assert.Equal(t, fantasy.MessageRoleAssistant, assistantMsg.Role) + require.Len(t, assistantMsg.Content, 4, + "assistant message should have 2x TextPart, ReasoningPart, and ToolCallPart") + + // Part 0: non-empty text. + textPart, ok := fantasy.AsMessagePart[fantasy.TextPart](assistantMsg.Content[0]) + require.True(t, ok, "part 0 should be TextPart") + assert.Equal(t, "hello world", textPart.Text) + + // Part 1: padded text — original whitespace preserved. + paddedPart, ok := fantasy.AsMessagePart[fantasy.TextPart](assistantMsg.Content[1]) + require.True(t, ok, "part 1 should be TextPart") + assert.Equal(t, " hello ", paddedPart.Text) + + // Part 2: non-empty reasoning. + reasoningPart, ok := fantasy.AsMessagePart[fantasy.ReasoningPart](assistantMsg.Content[2]) + require.True(t, ok, "part 2 should be ReasoningPart") + assert.Equal(t, "let me think", reasoningPart.Text) + + // Part 3: tool call (unaffected by text/reasoning filtering). + toolCallPart, ok := fantasy.AsMessagePart[fantasy.ToolCallPart](assistantMsg.Content[3]) + require.True(t, ok, "part 3 should be ToolCallPart") + assert.Equal(t, "tc-1", toolCallPart.ToolCallID) + assert.Equal(t, "read_file", toolCallPart.ToolName) + + // Second message: tool role with the local tool result. + toolMsg := msgs[1] + assert.Equal(t, fantasy.MessageRoleTool, toolMsg.Role) + require.Len(t, toolMsg.Content, 1, + "tool message should have only the local ToolResultPart") + + toolResultPart, ok := fantasy.AsMessagePart[fantasy.ToolResultPart](toolMsg.Content[0]) + require.True(t, ok, "tool part should be ToolResultPart") + assert.Equal(t, "tc-1", toolResultPart.ToolCallID) +} + func hasAnthropicEphemeralCacheControl(message fantasy.Message) bool { if len(message.ProviderOptions) == 0 { return false diff --git a/coderd/chatd/chatprompt/chatprompt.go b/coderd/chatd/chatprompt/chatprompt.go index 8530d9f1ce..55d2089945 100644 --- a/coderd/chatd/chatprompt/chatprompt.go +++ b/coderd/chatd/chatprompt/chatprompt.go @@ -139,9 +139,13 @@ func ConvertMessagesWithFiles( }, }) case codersdk.ChatMessageRoleUser: + userParts := partsToMessageParts(logger, pm.parts, resolved) + if len(userParts) == 0 { + continue + } prompt = append(prompt, fantasy.Message{ Role: fantasy.MessageRoleUser, - Content: partsToMessageParts(logger, pm.parts, resolved), + Content: userParts, }) case codersdk.ChatMessageRoleAssistant: fantasyParts := normalizeAssistantToolCallInputs( @@ -153,6 +157,9 @@ func ConvertMessagesWithFiles( } toolNameByCallID[sanitizeToolCallID(toolCall.ToolCallID)] = toolCall.ToolName } + if len(fantasyParts) == 0 { + continue + } prompt = append(prompt, fantasy.Message{ Role: fantasy.MessageRoleAssistant, Content: fantasyParts, @@ -166,9 +173,13 @@ func ConvertMessagesWithFiles( } } } + toolParts := partsToMessageParts(logger, pm.parts, resolved) + if len(toolParts) == 0 { + continue + } prompt = append(prompt, fantasy.Message{ Role: fantasy.MessageRoleTool, - Content: partsToMessageParts(logger, pm.parts, resolved), + Content: toolParts, }) } } @@ -1175,11 +1186,23 @@ func partsToMessageParts( for _, part := range parts { switch part.Type { case codersdk.ChatMessagePartTypeText: + // Anthropic rejects empty text content blocks with + // "text content blocks must be non-empty". Empty parts + // can arise when a stream sends TextStart/TextEnd with + // no delta in between. We filter them here rather than + // at persistence time to preserve the raw record. + if strings.TrimSpace(part.Text) == "" { + continue + } result = append(result, fantasy.TextPart{ Text: part.Text, ProviderOptions: providerMetadataToOptions(logger, part.ProviderMetadata), }) case codersdk.ChatMessagePartTypeReasoning: + // Same guard as text parts above. + if strings.TrimSpace(part.Text) == "" { + continue + } result = append(result, fantasy.ReasoningPart{ Text: part.Text, ProviderOptions: providerMetadataToOptions(logger, part.ProviderMetadata), diff --git a/coderd/chatd/chatprompt/chatprompt_test.go b/coderd/chatd/chatprompt/chatprompt_test.go index 515684c948..b555b60922 100644 --- a/coderd/chatd/chatprompt/chatprompt_test.go +++ b/coderd/chatd/chatprompt/chatprompt_test.go @@ -1646,3 +1646,125 @@ func TestNulEscapeRoundTrip(t *testing.T) { require.Equal(t, "has\x00nul", decoded[1].Text) }) } + +func TestConvertMessagesWithFiles_FiltersEmptyTextAndReasoningParts(t *testing.T) { + t.Parallel() + + // Helper to build a DB message from SDK parts. + makeMsg := func(t *testing.T, role database.ChatMessageRole, parts []codersdk.ChatMessagePart) database.ChatMessage { + t.Helper() + encoded, err := chatprompt.MarshalParts(parts) + require.NoError(t, err) + return database.ChatMessage{ + Role: role, + Visibility: database.ChatMessageVisibilityBoth, + Content: encoded, + ContentVersion: chatprompt.CurrentContentVersion, + } + } + + t.Run("UserRole", func(t *testing.T) { + t.Parallel() + + parts := []codersdk.ChatMessagePart{ + codersdk.ChatMessageText(""), // empty — filtered + codersdk.ChatMessageText(" \t\n "), // whitespace — filtered + codersdk.ChatMessageReasoning(""), // empty — filtered + codersdk.ChatMessageReasoning(" \n"), // whitespace — filtered + codersdk.ChatMessageText("hello"), // kept + codersdk.ChatMessageText(" hello "), // kept with original whitespace + codersdk.ChatMessageReasoning("thinking deeply"), // kept + codersdk.ChatMessageToolCall("call-1", "my_tool", json.RawMessage(`{"x":1}`)), + codersdk.ChatMessageToolResult("call-1", "my_tool", json.RawMessage(`{"ok":true}`), false), + } + + prompt, err := chatprompt.ConvertMessagesWithFiles( + context.Background(), + []database.ChatMessage{makeMsg(t, database.ChatMessageRoleUser, parts)}, + nil, + slogtest.Make(t, nil), + ) + require.NoError(t, err) + require.Len(t, prompt, 1) + + resultParts := prompt[0].Content + require.Len(t, resultParts, 5, "expected 5 parts after filtering empty text/reasoning") + + textPart, ok := fantasy.AsMessagePart[fantasy.TextPart](resultParts[0]) + require.True(t, ok, "expected TextPart at index 0") + require.Equal(t, "hello", textPart.Text) + + // Leading/trailing whitespace is preserved — only + // all-whitespace parts are dropped. + paddedPart, ok := fantasy.AsMessagePart[fantasy.TextPart](resultParts[1]) + require.True(t, ok, "expected TextPart at index 1") + require.Equal(t, " hello ", paddedPart.Text) + + reasoningPart, ok := fantasy.AsMessagePart[fantasy.ReasoningPart](resultParts[2]) + require.True(t, ok, "expected ReasoningPart at index 2") + require.Equal(t, "thinking deeply", reasoningPart.Text) + + toolCallPart, ok := fantasy.AsMessagePart[fantasy.ToolCallPart](resultParts[3]) + require.True(t, ok, "expected ToolCallPart at index 3") + require.Equal(t, "call-1", toolCallPart.ToolCallID) + + toolResultPart, ok := fantasy.AsMessagePart[fantasy.ToolResultPart](resultParts[4]) + require.True(t, ok, "expected ToolResultPart at index 4") + require.Equal(t, "call-1", toolResultPart.ToolCallID) + }) + + t.Run("AssistantRole", func(t *testing.T) { + t.Parallel() + + parts := []codersdk.ChatMessagePart{ + codersdk.ChatMessageText(""), // empty — filtered + codersdk.ChatMessageText(" "), // whitespace — filtered + codersdk.ChatMessageReasoning(""), // empty — filtered + codersdk.ChatMessageText(" reply "), // kept with whitespace + codersdk.ChatMessageToolCall("tc-1", "read_file", json.RawMessage(`{"path":"x"}`)), + } + + prompt, err := chatprompt.ConvertMessagesWithFiles( + context.Background(), + []database.ChatMessage{makeMsg(t, database.ChatMessageRoleAssistant, parts)}, + nil, + slogtest.Make(t, nil), + ) + require.NoError(t, err) + // 2 messages: assistant + synthetic tool result injected + // by injectMissingToolResults for the unmatched tool call. + require.Len(t, prompt, 2) + + resultParts := prompt[0].Content + require.Len(t, resultParts, 2, "expected text + tool-call after filtering") + + textPart, ok := fantasy.AsMessagePart[fantasy.TextPart](resultParts[0]) + require.True(t, ok, "expected TextPart") + require.Equal(t, " reply ", textPart.Text) + + tcPart, ok := fantasy.AsMessagePart[fantasy.ToolCallPart](resultParts[1]) + require.True(t, ok, "expected ToolCallPart") + require.Equal(t, "tc-1", tcPart.ToolCallID) + }) + + t.Run("AllEmptyDropsMessage", func(t *testing.T) { + t.Parallel() + + // When every part is filtered, the message itself should + // be dropped rather than appending an empty-content message. + parts := []codersdk.ChatMessagePart{ + codersdk.ChatMessageText(""), + codersdk.ChatMessageText(" "), + codersdk.ChatMessageReasoning(""), + } + + prompt, err := chatprompt.ConvertMessagesWithFiles( + context.Background(), + []database.ChatMessage{makeMsg(t, database.ChatMessageRoleAssistant, parts)}, + nil, + slogtest.Make(t, nil), + ) + require.NoError(t, err) + require.Empty(t, prompt, "all-empty message should be dropped entirely") + }) +}