Skip to content

Commit 5b2bcd6

Browse files
krissettokrissetto
authored
Merge pull request #596 from krissetto/fix-anthropic-client
Fix tool response sequencing in anthropic beta client
2 parents 8ffa57e + 8ce01c0 commit 5b2bcd6

3 files changed

Lines changed: 264 additions & 22 deletions

File tree

‎pkg/model/provider/anthropic/beta_client.go‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,30 +126,46 @@ func repairAnthropicSequencingBeta(msgs []anthropic.BetaMessageParam) []anthropi
126126
}
127127
repaired := make([]anthropic.BetaMessageParam, 0, len(msgs)+2)
128128
for i := range msgs {
129-
repaired = append(repaired, msgs[i])
130-
131129
m, ok := marshalToMapBeta(msgs[i])
132130
if !ok || m["role"] != "assistant" {
131+
repaired = append(repaired, msgs[i])
133132
continue
134133
}
135134

136135
toolUseIDs := collectToolUseIDs(contentArrayBeta(m))
137136
if len(toolUseIDs) == 0 {
137+
repaired = append(repaired, msgs[i])
138138
continue
139139
}
140140

141+
// Check if the next message is a user message with tool_results
142+
needsSyntheticMessage := true
141143
if i+1 < len(msgs) {
142144
if next, ok := marshalToMapBeta(msgs[i+1]); ok && next["role"] == "user" {
143145
toolResultIDs := collectToolResultIDs(contentArrayBeta(next))
146+
// Remove tool_use IDs that have corresponding tool_results
144147
for id := range toolResultIDs {
145148
delete(toolUseIDs, id)
146149
}
150+
// If all tool_use IDs have results, no synthetic message needed
151+
if len(toolUseIDs) == 0 {
152+
needsSyntheticMessage = false
153+
}
147154
}
148155
}
149156

150-
if len(toolUseIDs) > 0 {
157+
// Append the assistant message first
158+
repaired = append(repaired, msgs[i])
159+
160+
// If there are missing tool_results, insert a synthetic user message immediately after
161+
if needsSyntheticMessage && len(toolUseIDs) > 0 {
162+
slog.Debug("Inserting synthetic user message for missing tool_results",
163+
"assistant_index", i,
164+
"missing_count", len(toolUseIDs))
165+
151166
blocks := make([]anthropic.BetaContentBlockParamUnion, 0, len(toolUseIDs))
152167
for id := range toolUseIDs {
168+
slog.Debug("Creating synthetic tool_result", "tool_use_id", id)
153169
blocks = append(blocks, anthropic.BetaContentBlockParamUnion{
154170
OfToolResult: &anthropic.BetaToolResultBlockParam{
155171
ToolUseID: id,

‎pkg/model/provider/anthropic/beta_converter.go‎

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package anthropic
22

33
import (
44
"encoding/json"
5-
"log/slog"
65
"strings"
76

87
"github.com/anthropics/anthropic-sdk-go"
@@ -16,10 +15,13 @@ import (
1615
// - Thinking blocks can appear anywhere in the conversation (not required to be first)
1716
// - Always include the complete, unmodified thinking block from previous assistant turns
1817
// - interleaved parameter is kept for API compatibility but always true
18+
//
19+
// Important: Anthropic API requires that all tool_result blocks corresponding to tool_use
20+
// blocks from the same assistant message MUST be grouped into a single user message.
1921
func convertBetaMessages(messages []chat.Message) []anthropic.BetaMessageParam {
2022
var betaMessages []anthropic.BetaMessageParam
2123

22-
for i := range messages {
24+
for i := 0; i < len(messages); i++ {
2325
msg := &messages[i]
2426
if msg.Role == chat.MessageRoleSystem {
2527
// System messages handled separately
@@ -137,19 +139,42 @@ func convertBetaMessages(messages []chat.Message) []anthropic.BetaMessageParam {
137139
continue
138140
}
139141
if msg.Role == chat.MessageRoleTool {
140-
betaMessages = append(betaMessages, anthropic.BetaMessageParam{
141-
Role: anthropic.BetaMessageParamRoleUser,
142-
Content: []anthropic.BetaContentBlockParamUnion{
143-
{
144-
OfToolResult: &anthropic.BetaToolResultBlockParam{
145-
ToolUseID: msg.ToolCallID,
146-
Content: []anthropic.BetaToolResultBlockParamContentUnion{
147-
{OfText: &anthropic.BetaTextBlockParam{Text: strings.TrimSpace(msg.Content)}},
148-
},
142+
// Collect consecutive tool messages and merge them into a single user message
143+
// This is required by Anthropic API: all tool_result blocks for tool_use blocks
144+
// from the same assistant message must be in the same user message
145+
toolResultBlocks := []anthropic.BetaContentBlockParamUnion{
146+
{
147+
OfToolResult: &anthropic.BetaToolResultBlockParam{
148+
ToolUseID: msg.ToolCallID,
149+
Content: []anthropic.BetaToolResultBlockParamContentUnion{
150+
{OfText: &anthropic.BetaTextBlockParam{Text: strings.TrimSpace(msg.Content)}},
149151
},
150152
},
151153
},
154+
}
155+
156+
// Look ahead for consecutive tool messages and merge them
157+
j := i + 1
158+
for j < len(messages) && messages[j].Role == chat.MessageRoleTool {
159+
toolResultBlocks = append(toolResultBlocks, anthropic.BetaContentBlockParamUnion{
160+
OfToolResult: &anthropic.BetaToolResultBlockParam{
161+
ToolUseID: messages[j].ToolCallID,
162+
Content: []anthropic.BetaToolResultBlockParamContentUnion{
163+
{OfText: &anthropic.BetaTextBlockParam{Text: strings.TrimSpace(messages[j].Content)}},
164+
},
165+
},
166+
})
167+
j++
168+
}
169+
170+
// Add the merged user message with all tool results
171+
betaMessages = append(betaMessages, anthropic.BetaMessageParam{
172+
Role: anthropic.BetaMessageParamRoleUser,
173+
Content: toolResultBlocks,
152174
})
175+
176+
// Skip the messages we've already processed
177+
i = j - 1
153178
continue
154179
}
155180
}
@@ -168,18 +193,28 @@ func extractBetaSystemBlocks(messages []chat.Message) []anthropic.BetaTextBlockP
168193

169194
// convertBetaTools converts tools to Beta API format
170195
func convertBetaTools(t []tools.Tool) ([]anthropic.BetaToolUnionParam, error) {
171-
regularTools, err := convertTools(t)
172-
if err != nil {
173-
slog.Error("Failed to convert tools for Anthropic Beta request", "error", err)
174-
return nil, err
175-
}
196+
betaTools := make([]anthropic.BetaToolUnionParam, len(t))
176197

177-
betaTools := make([]anthropic.BetaToolUnionParam, len(regularTools))
198+
for i, tool := range t {
199+
inputSchema, err := ConvertParametersToSchema(tool.Parameters)
200+
if err != nil {
201+
return nil, err
202+
}
178203

179-
for i, tool := range regularTools {
180-
if err := tools.ConvertSchema(tool, &betaTools[i]); err != nil {
204+
// Convert to BetaToolInputSchemaParam
205+
var betaInputSchema anthropic.BetaToolInputSchemaParam
206+
if err := tools.ConvertSchema(inputSchema, &betaInputSchema); err != nil {
181207
return nil, err
182208
}
209+
210+
// Create BetaToolParam and wrap it in BetaToolUnionParam
211+
betaTools[i] = anthropic.BetaToolUnionParam{
212+
OfTool: &anthropic.BetaToolParam{
213+
Name: tool.Name,
214+
Description: anthropic.String(tool.Description),
215+
InputSchema: betaInputSchema,
216+
},
217+
}
183218
}
184219

185220
return betaTools, nil

‎pkg/model/provider/anthropic/beta_converter_test.go‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
package anthropic
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
9+
"github.com/docker/cagent/pkg/chat"
10+
"github.com/docker/cagent/pkg/tools"
11+
)
12+
13+
func TestConvertBetaMessages_MergesConsecutiveToolMessages(t *testing.T) {
14+
// Simulates the roast battle scenario where:
15+
// - Assistant message has 2 tool_use blocks (transfer_task calls)
16+
// - Two separate tool messages follow (one for each transfer_task result)
17+
// - These should be merged into a single user message with 2 tool_result blocks
18+
19+
messages := []chat.Message{
20+
{
21+
Role: chat.MessageRoleUser,
22+
Content: "Start the roast battle",
23+
},
24+
{
25+
Role: chat.MessageRoleAssistant,
26+
Content: "Let me transfer tasks to the comedians",
27+
ToolCalls: []tools.ToolCall{
28+
{
29+
ID: "tool_call_1",
30+
Type: "function",
31+
Function: tools.FunctionCall{
32+
Name: "transfer_task",
33+
Arguments: `{"agent":"roaster_a","task":"Write roast"}`,
34+
},
35+
},
36+
{
37+
ID: "tool_call_2",
38+
Type: "function",
39+
Function: tools.FunctionCall{
40+
Name: "transfer_task",
41+
Arguments: `{"agent":"roaster_b","task":"Write counter-roast"}`,
42+
},
43+
},
44+
},
45+
},
46+
{
47+
Role: chat.MessageRoleTool,
48+
Content: "Roast A completed",
49+
ToolCallID: "tool_call_1",
50+
},
51+
{
52+
Role: chat.MessageRoleTool,
53+
Content: "Roast B completed",
54+
ToolCallID: "tool_call_2",
55+
},
56+
{
57+
Role: chat.MessageRoleAssistant,
58+
Content: "Both roasts are complete!",
59+
},
60+
}
61+
62+
// Convert to Beta format
63+
betaMessages := convertBetaMessages(messages)
64+
65+
// Verify structure: User -> Assistant (with 2 tool_use) -> User (with 2 tool_result) -> Assistant
66+
require.Len(t, betaMessages, 4, "Should have 4 messages after conversion")
67+
68+
// Verify roles
69+
msg0Map, _ := marshalToMapBeta(betaMessages[0])
70+
msg1Map, _ := marshalToMapBeta(betaMessages[1])
71+
msg2Map, _ := marshalToMapBeta(betaMessages[2])
72+
msg3Map, _ := marshalToMapBeta(betaMessages[3])
73+
assert.Equal(t, "user", msg0Map["role"])
74+
assert.Equal(t, "assistant", msg1Map["role"])
75+
assert.Equal(t, "user", msg2Map["role"])
76+
assert.Equal(t, "assistant", msg3Map["role"])
77+
78+
// Verify the second user message (tool results) has both tool_result blocks
79+
userMsg2Map, ok := marshalToMapBeta(betaMessages[2])
80+
require.True(t, ok)
81+
content := contentArrayBeta(userMsg2Map)
82+
require.Len(t, content, 2, "User message should have 2 tool_result blocks")
83+
84+
// Verify both tool_result IDs are present
85+
toolResultIDs := collectToolResultIDs(content)
86+
assert.Contains(t, toolResultIDs, "tool_call_1")
87+
assert.Contains(t, toolResultIDs, "tool_call_2")
88+
89+
// Most importantly: validate that the sequence is valid for Anthropic API
90+
err := validateAnthropicSequencingBeta(betaMessages)
91+
require.NoError(t, err, "Converted messages should pass Anthropic sequencing validation")
92+
}
93+
94+
func TestConvertBetaMessages_SingleToolMessage(t *testing.T) {
95+
// When there's only one tool message, it should still work correctly
96+
messages := []chat.Message{
97+
{
98+
Role: chat.MessageRoleUser,
99+
Content: "Test",
100+
},
101+
{
102+
Role: chat.MessageRoleAssistant,
103+
Content: "",
104+
ToolCalls: []tools.ToolCall{
105+
{
106+
ID: "tool_1",
107+
Type: "function",
108+
Function: tools.FunctionCall{
109+
Name: "test_tool",
110+
Arguments: `{}`,
111+
},
112+
},
113+
},
114+
},
115+
{
116+
Role: chat.MessageRoleTool,
117+
Content: "Tool result",
118+
ToolCallID: "tool_1",
119+
},
120+
{
121+
Role: chat.MessageRoleAssistant,
122+
Content: "Done",
123+
},
124+
}
125+
126+
betaMessages := convertBetaMessages(messages)
127+
require.Len(t, betaMessages, 4)
128+
129+
// Validate sequence
130+
err := validateAnthropicSequencingBeta(betaMessages)
131+
require.NoError(t, err)
132+
}
133+
134+
func TestConvertBetaMessages_NonConsecutiveToolMessages(t *testing.T) {
135+
// When tool messages are separated by other messages (edge case)
136+
// Each tool message group should be handled independently
137+
messages := []chat.Message{
138+
{
139+
Role: chat.MessageRoleUser,
140+
Content: "First request",
141+
},
142+
{
143+
Role: chat.MessageRoleAssistant,
144+
Content: "",
145+
ToolCalls: []tools.ToolCall{
146+
{
147+
ID: "tool_1",
148+
Type: "function",
149+
Function: tools.FunctionCall{
150+
Name: "test_tool",
151+
Arguments: `{}`,
152+
},
153+
},
154+
},
155+
},
156+
{
157+
Role: chat.MessageRoleTool,
158+
Content: "Tool result 1",
159+
ToolCallID: "tool_1",
160+
},
161+
{
162+
Role: chat.MessageRoleAssistant,
163+
Content: "Intermediate response",
164+
ToolCalls: []tools.ToolCall{
165+
{
166+
ID: "tool_2",
167+
Type: "function",
168+
Function: tools.FunctionCall{
169+
Name: "test_tool",
170+
Arguments: `{}`,
171+
},
172+
},
173+
},
174+
},
175+
{
176+
Role: chat.MessageRoleTool,
177+
Content: "Tool result 2",
178+
ToolCallID: "tool_2",
179+
},
180+
{
181+
Role: chat.MessageRoleAssistant,
182+
Content: "Final response",
183+
},
184+
}
185+
186+
betaMessages := convertBetaMessages(messages)
187+
188+
// Validate the entire sequence
189+
err := validateAnthropicSequencingBeta(betaMessages)
190+
require.NoError(t, err, "Messages with non-consecutive tool calls should still validate")
191+
}

0 commit comments

Comments
 (0)