mirror of
https://github.com/sipeed/picoclaw.git
synced 2026-06-12 18:08:54 +00:00
fix(provider): deduplicate tool results and merge consecutive tool_result blocks for Anthropic API (#1793)
Anthropic API returns 400 when multiple tool_result blocks share the same tool_use_id, or when consecutive tool results are sent as separate user messages. This fix: 1. Adds ToolCallID deduplication in sanitizeHistoryForProvider (context.go) to drop duplicate tool results before sending to any provider. 2. Merges consecutive tool result messages into a single user message with multiple tool_result content blocks in Anthropic's buildRequestBody, for both "user" (with ToolCallID) and "tool" role messages. 3. Adds tests for both behaviors. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -678,8 +678,21 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message
|
||||
// like DeepSeek that enforce: "An assistant message with 'tool_calls' must
|
||||
// be followed by tool messages responding to each 'tool_call_id'."
|
||||
final := make([]providers.Message, 0, len(sanitized))
|
||||
seenToolCallID := make(map[string]bool)
|
||||
for i := 0; i < len(sanitized); i++ {
|
||||
msg := sanitized[i]
|
||||
|
||||
// Deduplicate tool results by ToolCallID
|
||||
if msg.Role == "tool" && msg.ToolCallID != "" {
|
||||
if seenToolCallID[msg.ToolCallID] {
|
||||
logger.DebugCF("agent", "Dropping duplicate tool result", map[string]any{
|
||||
"tool_call_id": msg.ToolCallID,
|
||||
})
|
||||
continue
|
||||
}
|
||||
seenToolCallID[msg.ToolCallID] = true
|
||||
}
|
||||
|
||||
if msg.Role == "assistant" && len(msg.ToolCalls) > 0 {
|
||||
// Collect expected tool_call IDs
|
||||
expected := make(map[string]bool, len(msg.ToolCalls))
|
||||
|
||||
@@ -188,6 +188,31 @@ func TestSanitizeHistoryForProvider_PlainConversation(t *testing.T) {
|
||||
assertRoles(t, result, "user", "assistant", "user", "assistant")
|
||||
}
|
||||
|
||||
func TestSanitizeHistoryForProvider_DuplicateToolResults(t *testing.T) {
|
||||
history := []providers.Message{
|
||||
msg("user", "do something"),
|
||||
assistantWithTools("A", "B"),
|
||||
toolResult("A"),
|
||||
toolResult("B"),
|
||||
toolResult("A"), // duplicate
|
||||
toolResult("B"), // duplicate
|
||||
msg("assistant", "done"),
|
||||
}
|
||||
|
||||
result := sanitizeHistoryForProvider(history)
|
||||
if len(result) != 5 {
|
||||
t.Fatalf("expected 5 messages, got %d: %+v", len(result), roles(result))
|
||||
}
|
||||
assertRoles(t, result, "user", "assistant", "tool", "tool", "assistant")
|
||||
// Verify the kept tool results have the correct IDs
|
||||
if result[2].ToolCallID != "A" {
|
||||
t.Errorf("expected tool result A, got %q", result[2].ToolCallID)
|
||||
}
|
||||
if result[3].ToolCallID != "B" {
|
||||
t.Errorf("expected tool result B, got %q", result[3].ToolCallID)
|
||||
}
|
||||
}
|
||||
|
||||
func roles(msgs []providers.Message) []string {
|
||||
r := make([]string, len(msgs))
|
||||
for i, m := range msgs {
|
||||
|
||||
@@ -188,17 +188,23 @@ func buildRequestBody(
|
||||
|
||||
case "user":
|
||||
if msg.ToolCallID != "" {
|
||||
// Tool result message
|
||||
content := []map[string]any{
|
||||
{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": msg.ToolCallID,
|
||||
"content": msg.Content,
|
||||
},
|
||||
// Tool result message — merge into previous user message if it contains tool_results
|
||||
toolResultBlock := map[string]any{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": msg.ToolCallID,
|
||||
"content": msg.Content,
|
||||
}
|
||||
if len(apiMessages) > 0 {
|
||||
if prev, ok := apiMessages[len(apiMessages)-1].(map[string]any); ok && prev["role"] == "user" {
|
||||
if content, ok := prev["content"].([]map[string]any); ok {
|
||||
prev["content"] = append(content, toolResultBlock)
|
||||
continue
|
||||
}
|
||||
}
|
||||
}
|
||||
apiMessages = append(apiMessages, map[string]any{
|
||||
"role": "user",
|
||||
"content": content,
|
||||
"content": []map[string]any{toolResultBlock},
|
||||
})
|
||||
} else {
|
||||
// Regular user message
|
||||
@@ -246,17 +252,23 @@ func buildRequestBody(
|
||||
})
|
||||
|
||||
case "tool":
|
||||
// Tool result (alternative format)
|
||||
content := []map[string]any{
|
||||
{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": msg.ToolCallID,
|
||||
"content": msg.Content,
|
||||
},
|
||||
// Tool result (alternative format) — merge into previous user message if it contains tool_results
|
||||
toolResultBlock := map[string]any{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": msg.ToolCallID,
|
||||
"content": msg.Content,
|
||||
}
|
||||
if len(apiMessages) > 0 {
|
||||
if prev, ok := apiMessages[len(apiMessages)-1].(map[string]any); ok && prev["role"] == "user" {
|
||||
if content, ok := prev["content"].([]map[string]any); ok {
|
||||
prev["content"] = append(content, toolResultBlock)
|
||||
continue
|
||||
}
|
||||
}
|
||||
}
|
||||
apiMessages = append(apiMessages, map[string]any{
|
||||
"role": "user",
|
||||
"content": content,
|
||||
"content": []map[string]any{toolResultBlock},
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -562,6 +562,96 @@ func TestBuildRequestBodyEdgeCases(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildRequestBody_ConsecutiveToolResultsMerged(t *testing.T) {
|
||||
// Consecutive tool results (role "tool") should be merged into a single "user" message
|
||||
messages := []Message{
|
||||
{Role: "user", Content: "Use tools"},
|
||||
{Role: "assistant", Content: "", ToolCalls: []ToolCall{
|
||||
{ID: "t1", Name: "tool_a", Arguments: map[string]any{"x": 1}},
|
||||
{ID: "t2", Name: "tool_b", Arguments: map[string]any{"y": 2}},
|
||||
}},
|
||||
{Role: "tool", ToolCallID: "t1", Content: "result1"},
|
||||
{Role: "tool", ToolCallID: "t2", Content: "result2"},
|
||||
}
|
||||
|
||||
got, err := buildRequestBody(messages, nil, "test-model", map[string]any{"max_tokens": 8192})
|
||||
if err != nil {
|
||||
t.Fatalf("buildRequestBody() error: %v", err)
|
||||
}
|
||||
|
||||
apiMessages, ok := got["messages"].([]any)
|
||||
if !ok {
|
||||
t.Fatalf("messages is not []any")
|
||||
}
|
||||
|
||||
// Expect: user, assistant, user (merged tool results)
|
||||
if len(apiMessages) != 3 {
|
||||
for i, m := range apiMessages {
|
||||
t.Logf("message[%d]: %+v", i, m)
|
||||
}
|
||||
t.Fatalf("expected 3 API messages, got %d", len(apiMessages))
|
||||
}
|
||||
|
||||
// The third message should be a user message with 2 tool_result blocks
|
||||
toolResultMsg, ok := apiMessages[2].(map[string]any)
|
||||
if !ok {
|
||||
t.Fatalf("tool result message is not map[string]any")
|
||||
}
|
||||
if toolResultMsg["role"] != "user" {
|
||||
t.Errorf("expected role 'user', got %v", toolResultMsg["role"])
|
||||
}
|
||||
content, ok := toolResultMsg["content"].([]map[string]any)
|
||||
if !ok {
|
||||
t.Fatalf("content is not []map[string]any: %T", toolResultMsg["content"])
|
||||
}
|
||||
if len(content) != 2 {
|
||||
t.Fatalf("expected 2 tool_result blocks, got %d", len(content))
|
||||
}
|
||||
if content[0]["tool_use_id"] != "t1" {
|
||||
t.Errorf("first tool_result tool_use_id = %v, want t1", content[0]["tool_use_id"])
|
||||
}
|
||||
if content[1]["tool_use_id"] != "t2" {
|
||||
t.Errorf("second tool_result tool_use_id = %v, want t2", content[1]["tool_use_id"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildRequestBody_UserToolResultsMerged(t *testing.T) {
|
||||
// Consecutive tool results using role "user" with ToolCallID should also be merged
|
||||
messages := []Message{
|
||||
{Role: "user", Content: "Use tools"},
|
||||
{Role: "assistant", Content: "", ToolCalls: []ToolCall{
|
||||
{ID: "t1", Name: "tool_a", Arguments: map[string]any{"x": 1}},
|
||||
{ID: "t2", Name: "tool_b", Arguments: map[string]any{"y": 2}},
|
||||
}},
|
||||
{Role: "user", ToolCallID: "t1", Content: "result1"},
|
||||
{Role: "user", ToolCallID: "t2", Content: "result2"},
|
||||
}
|
||||
|
||||
got, err := buildRequestBody(messages, nil, "test-model", map[string]any{"max_tokens": 8192})
|
||||
if err != nil {
|
||||
t.Fatalf("buildRequestBody() error: %v", err)
|
||||
}
|
||||
|
||||
apiMessages, ok := got["messages"].([]any)
|
||||
if !ok {
|
||||
t.Fatalf("messages is not []any")
|
||||
}
|
||||
|
||||
// Expect: user, assistant, user (merged tool results)
|
||||
if len(apiMessages) != 3 {
|
||||
t.Fatalf("expected 3 API messages, got %d", len(apiMessages))
|
||||
}
|
||||
|
||||
toolResultMsg := apiMessages[2].(map[string]any)
|
||||
content, ok := toolResultMsg["content"].([]map[string]any)
|
||||
if !ok {
|
||||
t.Fatalf("content is not []map[string]any: %T", toolResultMsg["content"])
|
||||
}
|
||||
if len(content) != 2 {
|
||||
t.Fatalf("expected 2 tool_result blocks, got %d", len(content))
|
||||
}
|
||||
}
|
||||
|
||||
// TestParseResponseBodyEdgeCases tests edge cases for parseResponseBody.
|
||||
func TestParseResponseBodyEdgeCases(t *testing.T) {
|
||||
tests := []struct {
|
||||
|
||||
Reference in New Issue
Block a user