mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
fix: filter empty text/reasoning parts before sending to LLM (#23284)
## Problem Anthropic rejects requests containing empty text content blocks with: ``` messages: text content blocks must be non-empty ``` Empty text parts (`""` or whitespace-only like `" "`) get persisted in the database when a stream sends `TextStart`/`TextEnd` with no `TextDelta` in between. On the next turn, these parts are loaded from the DB and sent to Anthropic, which rejects them. ## Fix Filter empty/whitespace-only text and reasoning parts at the two LLM dispatch boundaries, without modifying persistence (the raw record is preserved): - **`partsToMessageParts()`** in `chatprompt.go` — filters when converting persisted DB messages to fantasy message parts for LLM calls. This is the last gateway before the Anthropic provider creates `TextBlockParam` objects. - **`toResponseMessages()`** in `chatloop.go` — filters when building in-flight conversation messages between steps within a single turn. Note: `flushActiveState()` (the interruption path) already had this guard — the normal `TextEnd` streaming path did not, but since we're not changing persistence, the fix is applied at the dispatch layer.
This commit is contained in:
@@ -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{
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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")
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user