llm/conversation: fix duplicate tool results in insertMissingToolResults
Fix critical bug in insertMissingToolResults function that created duplicate
tool_result blocks when multiple tool uses were missing results, causing
Claude API 400 errors with message "each tool_use must have a single result".
Problem Analysis:
The function was designed to add error tool results for tool uses that
were requested but had no corresponding tool_result in the user message
(e.g., when hitting budget limits). However, when multiple tool uses
were missing results, the function incorrectly duplicated results:
1. First iteration: adds tool1 result to prefix, sets msg.Content = [tool1] + original
2. Second iteration: adds tool2 to prefix, sets msg.Content = [tool1, tool2] + [tool1] + original
This created duplicate tool_result blocks with the same tool_use_id, violating
Claude's API requirements and causing 400 Bad Request errors.
Root Cause:
The msg.Content assignment was inside the loop that builds the prefix:
Solution:
Move the msg.Content assignment outside the loop, after all missing
tool results have been collected:
Testing:
Added comprehensive test coverage for insertMissingToolResults with scenarios:
- Single missing tool result
- Multiple missing tool results (the problematic case)
- No missing results when results already present
- No tool uses in previous message
The test specifically verifies no duplicate tool_use_ids are created
and that all expected tool results are present exactly once.
This fix resolves the "multiple tool_result blocks with id" error
that was occurring in deep conversations with tool usage.
Fixes #121
Co-Authored-By: sketch <hello@sketch.dev>
Change-ID: s07f278af38b745aak
diff --git a/llm/conversation/convo.go b/llm/conversation/convo.go
index d85515c..12c334f 100644
--- a/llm/conversation/convo.go
+++ b/llm/conversation/convo.go
@@ -282,9 +282,9 @@
}},
}
prefix = append(prefix, content)
- msg.Content = append(prefix, msg.Content...)
- mr.Messages[len(mr.Messages)-1].Content = msg.Content
}
+ msg.Content = append(prefix, msg.Content...)
+ mr.Messages[len(mr.Messages)-1].Content = msg.Content
slog.DebugContext(c.Ctx, "inserted missing tool results")
}
@@ -367,7 +367,7 @@
content.ToolError = true
content.ToolResult = []llm.Content{{
Type: llm.ContentTypeText,
- Text: "user canceled this too_use",
+ Text: "user canceled this tool_use",
}}
toolResults = append(toolResults, content)
}
diff --git a/llm/conversation/convo_test.go b/llm/conversation/convo_test.go
index cf4f1c2..8794468 100644
--- a/llm/conversation/convo_test.go
+++ b/llm/conversation/convo_test.go
@@ -5,10 +5,12 @@
"context"
"net/http"
"os"
+ "slices"
"strings"
"testing"
"sketch.dev/httprr"
+ "sketch.dev/llm"
"sketch.dev/llm/ant"
)
@@ -137,3 +139,160 @@
})
}
}
+
+// TestInsertMissingToolResults tests the insertMissingToolResults function
+// to ensure it doesn't create duplicate tool results when multiple tool uses are missing results.
+func TestInsertMissingToolResults(t *testing.T) {
+ tests := []struct {
+ name string
+ messages []llm.Message
+ currentMsg llm.Message
+ expectedCount int
+ expectedToolIDs []string
+ }{
+ {
+ name: "Single missing tool result",
+ messages: []llm.Message{
+ {
+ Role: llm.MessageRoleAssistant,
+ Content: []llm.Content{
+ {
+ Type: llm.ContentTypeToolUse,
+ ID: "tool1",
+ },
+ },
+ },
+ },
+ currentMsg: llm.Message{
+ Role: llm.MessageRoleUser,
+ Content: []llm.Content{},
+ },
+ expectedCount: 1,
+ expectedToolIDs: []string{"tool1"},
+ },
+ {
+ name: "Multiple missing tool results",
+ messages: []llm.Message{
+ {
+ Role: llm.MessageRoleAssistant,
+ Content: []llm.Content{
+ {
+ Type: llm.ContentTypeToolUse,
+ ID: "tool1",
+ },
+ {
+ Type: llm.ContentTypeToolUse,
+ ID: "tool2",
+ },
+ {
+ Type: llm.ContentTypeToolUse,
+ ID: "tool3",
+ },
+ },
+ },
+ },
+ currentMsg: llm.Message{
+ Role: llm.MessageRoleUser,
+ Content: []llm.Content{},
+ },
+ expectedCount: 3,
+ expectedToolIDs: []string{"tool1", "tool2", "tool3"},
+ },
+ {
+ name: "No missing tool results when results already present",
+ messages: []llm.Message{
+ {
+ Role: llm.MessageRoleAssistant,
+ Content: []llm.Content{
+ {
+ Type: llm.ContentTypeToolUse,
+ ID: "tool1",
+ },
+ },
+ },
+ },
+ currentMsg: llm.Message{
+ Role: llm.MessageRoleUser,
+ Content: []llm.Content{
+ {
+ Type: llm.ContentTypeToolResult,
+ ToolUseID: "tool1",
+ },
+ },
+ },
+ expectedCount: 1, // Only the existing one
+ expectedToolIDs: []string{"tool1"},
+ },
+ {
+ name: "No tool uses in previous message",
+ messages: []llm.Message{
+ {
+ Role: llm.MessageRoleAssistant,
+ Content: []llm.Content{
+ {
+ Type: llm.ContentTypeText,
+ Text: "Just some text",
+ },
+ },
+ },
+ },
+ currentMsg: llm.Message{
+ Role: llm.MessageRoleUser,
+ Content: []llm.Content{},
+ },
+ expectedCount: 0,
+ expectedToolIDs: []string{},
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ srv := &ant.Service{}
+ convo := New(context.Background(), srv)
+
+ // Create request with messages
+ req := &llm.Request{
+ Messages: append(tt.messages, tt.currentMsg),
+ }
+
+ // Call insertMissingToolResults
+ msg := tt.currentMsg
+ convo.insertMissingToolResults(req, &msg)
+
+ // Count tool results in the message
+ toolResultCount := 0
+ toolIDs := []string{}
+ for _, content := range msg.Content {
+ if content.Type == llm.ContentTypeToolResult {
+ toolResultCount++
+ toolIDs = append(toolIDs, content.ToolUseID)
+ }
+ }
+
+ // Verify count
+ if toolResultCount != tt.expectedCount {
+ t.Errorf("Expected %d tool results, got %d", tt.expectedCount, toolResultCount)
+ }
+
+ // Verify no duplicates by checking unique tool IDs
+ seenIDs := make(map[string]int)
+ for _, id := range toolIDs {
+ seenIDs[id]++
+ }
+
+ // Check for duplicates
+ for id, count := range seenIDs {
+ if count > 1 {
+ t.Errorf("Duplicate tool result for ID %s: found %d times", id, count)
+ }
+ }
+
+ // Verify all expected tool IDs are present
+ for _, expectedID := range tt.expectedToolIDs {
+ if !slices.Contains(toolIDs, expectedID) {
+ t.Errorf("Expected tool ID %s not found in results", expectedID)
+ }
+ }
+ })
+ }
+}