fix(agent): normalize tool-call history for strict providers
This commit is contained in:
parent
bd56e10bb8
commit
da6a862079
2 changed files with 128 additions and 71 deletions
|
|
@ -681,75 +681,103 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Second pass: ensure every assistant message with tool_calls has matching
|
// Second pass: normalize each assistant tool-call round so strict providers
|
||||||
// tool result messages following it. This is required by strict providers
|
// always receive exactly one tool response for each expected tool_call_id.
|
||||||
// like DeepSeek that enforce: "An assistant message with 'tool_calls' must
|
// This avoids cross-round dedupe bugs when providers reuse tool_call IDs.
|
||||||
// be followed by tool messages responding to each 'tool_call_id'."
|
|
||||||
final := make([]providers.Message, 0, len(sanitized))
|
final := make([]providers.Message, 0, len(sanitized))
|
||||||
seenToolCallID := make(map[string]bool)
|
|
||||||
for i := 0; i < len(sanitized); i++ {
|
for i := 0; i < len(sanitized); i++ {
|
||||||
msg := sanitized[i]
|
msg := sanitized[i]
|
||||||
|
if msg.Role != "assistant" || len(msg.ToolCalls) == 0 {
|
||||||
|
final = append(final, msg)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
// Deduplicate tool results by ToolCallID
|
// Collect expected IDs in stable order while deduplicating duplicates
|
||||||
if msg.Role == "tool" && msg.ToolCallID != "" {
|
// within the same assistant turn.
|
||||||
if seenToolCallID[msg.ToolCallID] {
|
expectedOrder := make([]string, 0, len(msg.ToolCalls))
|
||||||
logger.DebugCF("agent", "Dropping duplicate tool result", map[string]any{
|
expectedSet := make(map[string]struct{}, len(msg.ToolCalls))
|
||||||
"tool_call_id": msg.ToolCallID,
|
for _, tc := range msg.ToolCalls {
|
||||||
|
if tc.ID == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if _, exists := expectedSet[tc.ID]; exists {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
expectedSet[tc.ID] = struct{}{}
|
||||||
|
expectedOrder = append(expectedOrder, tc.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// If provider output is malformed and tool calls have no IDs, keep the
|
||||||
|
// assistant turn as-is and let provider-level validation handle it.
|
||||||
|
if len(expectedOrder) == 0 {
|
||||||
|
final = append(final, msg)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
// Gather the contiguous tool result block for this assistant turn.
|
||||||
|
j := i + 1
|
||||||
|
collectedTools := make([]providers.Message, 0, len(expectedOrder))
|
||||||
|
for ; j < len(sanitized); j++ {
|
||||||
|
if sanitized[j].Role != "tool" {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
collectedTools = append(collectedTools, sanitized[j])
|
||||||
|
}
|
||||||
|
|
||||||
|
// Keep only the first tool result per expected ID and drop stray entries.
|
||||||
|
toolByID := make(map[string]providers.Message, len(expectedOrder))
|
||||||
|
for _, toolMsg := range collectedTools {
|
||||||
|
if toolMsg.ToolCallID == "" {
|
||||||
|
logger.DebugCF("agent", "Dropping tool result without tool_call_id", map[string]any{})
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if _, expected := expectedSet[toolMsg.ToolCallID]; !expected {
|
||||||
|
logger.DebugCF("agent", "Dropping unexpected tool result", map[string]any{
|
||||||
|
"tool_call_id": toolMsg.ToolCallID,
|
||||||
})
|
})
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
seenToolCallID[msg.ToolCallID] = true
|
if _, exists := toolByID[toolMsg.ToolCallID]; exists {
|
||||||
}
|
logger.DebugCF("agent", "Dropping duplicate tool result within turn", map[string]any{
|
||||||
|
"tool_call_id": toolMsg.ToolCallID,
|
||||||
if msg.Role == "assistant" && len(msg.ToolCalls) > 0 {
|
})
|
||||||
// Collect expected tool_call IDs
|
|
||||||
expected := make(map[string]bool, len(msg.ToolCalls))
|
|
||||||
for _, tc := range msg.ToolCalls {
|
|
||||||
expected[tc.ID] = false
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check following messages for matching tool results
|
|
||||||
toolMsgCount := 0
|
|
||||||
for j := i + 1; j < len(sanitized); j++ {
|
|
||||||
if sanitized[j].Role != "tool" {
|
|
||||||
break
|
|
||||||
}
|
|
||||||
toolMsgCount++
|
|
||||||
if _, exists := expected[sanitized[j].ToolCallID]; exists {
|
|
||||||
expected[sanitized[j].ToolCallID] = true
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// If any tool_call_id is missing, drop this assistant message and its partial tool messages
|
|
||||||
allFound := true
|
|
||||||
for toolCallID, found := range expected {
|
|
||||||
if !found {
|
|
||||||
allFound = false
|
|
||||||
logger.DebugCF(
|
|
||||||
"agent",
|
|
||||||
"Dropping assistant message with incomplete tool results",
|
|
||||||
map[string]any{
|
|
||||||
"missing_tool_call_id": toolCallID,
|
|
||||||
"expected_count": len(expected),
|
|
||||||
"found_count": toolMsgCount,
|
|
||||||
},
|
|
||||||
)
|
|
||||||
break
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if !allFound {
|
|
||||||
// Skip this assistant message and its tool messages
|
|
||||||
i += toolMsgCount
|
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
toolByID[toolMsg.ToolCallID] = toolMsg
|
||||||
}
|
}
|
||||||
|
|
||||||
final = append(final, msg)
|
final = append(final, msg)
|
||||||
|
missingCount := 0
|
||||||
|
for _, toolCallID := range expectedOrder {
|
||||||
|
if toolMsg, ok := toolByID[toolCallID]; ok {
|
||||||
|
final = append(final, toolMsg)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
missingCount++
|
||||||
|
final = append(final, syntheticToolResultForMissingID(toolCallID))
|
||||||
|
}
|
||||||
|
if missingCount > 0 {
|
||||||
|
logger.DebugCF("agent", "Inserted synthetic tool results for missing IDs", map[string]any{
|
||||||
|
"missing_count": missingCount,
|
||||||
|
"expected_count": len(expectedOrder),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// Skip the collected tool block; it has already been normalized above.
|
||||||
|
i = j - 1
|
||||||
}
|
}
|
||||||
|
|
||||||
return final
|
return final
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func syntheticToolResultForMissingID(toolCallID string) providers.Message {
|
||||||
|
return providers.Message{
|
||||||
|
Role: "tool",
|
||||||
|
ToolCallID: toolCallID,
|
||||||
|
Content: "Tool result unavailable: this call response was missing in retained history.",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func (cb *ContextBuilder) AddToolResult(
|
func (cb *ContextBuilder) AddToolResult(
|
||||||
messages []providers.Message,
|
messages []providers.Message,
|
||||||
toolCallID, toolName, result string,
|
toolCallID, toolName, result string,
|
||||||
|
|
|
||||||
|
|
@ -249,13 +249,15 @@ func TestSanitizeHistoryForProvider_IncompleteToolResults(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
result := sanitizeHistoryForProvider(history)
|
result := sanitizeHistoryForProvider(history)
|
||||||
// The assistant message with incomplete tool results should be dropped,
|
// The incomplete turn should be normalized, not dropped:
|
||||||
// along with its partial tool result. The remaining messages are:
|
// one synthetic tool result is inserted for missing "B".
|
||||||
// user ("do two things"), user ("next question"), assistant ("answer")
|
if len(result) != 6 {
|
||||||
if len(result) != 3 {
|
t.Fatalf("expected 6 messages, got %d: %+v", len(result), roles(result))
|
||||||
t.Fatalf("expected 3 messages, got %d: %+v", len(result), roles(result))
|
}
|
||||||
|
assertRoles(t, result, "user", "assistant", "tool", "tool", "user", "assistant")
|
||||||
|
if result[3].ToolCallID != "B" {
|
||||||
|
t.Fatalf("expected synthetic tool result for B, got %q", result[3].ToolCallID)
|
||||||
}
|
}
|
||||||
assertRoles(t, result, "user", "user", "assistant")
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestSanitizeHistoryForProvider_MissingAllToolResults tests the case where
|
// TestSanitizeHistoryForProvider_MissingAllToolResults tests the case where
|
||||||
|
|
@ -270,12 +272,14 @@ func TestSanitizeHistoryForProvider_MissingAllToolResults(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
result := sanitizeHistoryForProvider(history)
|
result := sanitizeHistoryForProvider(history)
|
||||||
// The assistant message with no tool results should be dropped.
|
// No real tool result exists, so one synthetic tool result is inserted.
|
||||||
// Remaining: user ("do something"), user ("hello"), assistant ("hi")
|
if len(result) != 5 {
|
||||||
if len(result) != 3 {
|
t.Fatalf("expected 5 messages, got %d: %+v", len(result), roles(result))
|
||||||
t.Fatalf("expected 3 messages, got %d: %+v", len(result), roles(result))
|
}
|
||||||
|
assertRoles(t, result, "user", "assistant", "tool", "user", "assistant")
|
||||||
|
if result[2].ToolCallID != "A" {
|
||||||
|
t.Fatalf("expected synthetic tool result for A, got %q", result[2].ToolCallID)
|
||||||
}
|
}
|
||||||
assertRoles(t, result, "user", "user", "assistant")
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestSanitizeHistoryForProvider_PartialToolResultsInMiddle tests that
|
// TestSanitizeHistoryForProvider_PartialToolResultsInMiddle tests that
|
||||||
|
|
@ -297,12 +301,37 @@ func TestSanitizeHistoryForProvider_PartialToolResultsInMiddle(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
result := sanitizeHistoryForProvider(history)
|
result := sanitizeHistoryForProvider(history)
|
||||||
// First round is complete (user, assistant+tools, tool, assistant),
|
// First and third rounds remain unchanged; second round gets a synthetic
|
||||||
// second round is incomplete and dropped (assistant+tools, partial tool),
|
// tool result for missing "C" instead of being dropped.
|
||||||
// third round is complete (user, assistant+tools, tool, assistant).
|
if len(result) != 12 {
|
||||||
// Remaining: user, assistant, tool, assistant, user, user, assistant, tool, assistant
|
t.Fatalf("expected 12 messages, got %d: %+v", len(result), roles(result))
|
||||||
if len(result) != 9 {
|
}
|
||||||
t.Fatalf("expected 9 messages, got %d: %+v", len(result), roles(result))
|
assertRoles(t, result, "user", "assistant", "tool", "assistant", "user", "assistant", "tool", "tool", "user", "assistant", "tool", "assistant")
|
||||||
|
if result[7].ToolCallID != "C" {
|
||||||
|
t.Fatalf("expected synthetic tool result for C, got %q", result[7].ToolCallID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSanitizeHistoryForProvider_ReusedToolCallIDAcrossRounds ensures
|
||||||
|
// per-turn normalization does not deduplicate tool_call IDs globally.
|
||||||
|
func TestSanitizeHistoryForProvider_ReusedToolCallIDAcrossRounds(t *testing.T) {
|
||||||
|
history := []providers.Message{
|
||||||
|
msg("user", "round one"),
|
||||||
|
assistantWithTools("call_1"),
|
||||||
|
toolResult("call_1"),
|
||||||
|
msg("assistant", "done one"),
|
||||||
|
msg("user", "round two"),
|
||||||
|
assistantWithTools("call_1"), // ID reused by provider in a later round
|
||||||
|
toolResult("call_1"),
|
||||||
|
msg("assistant", "done two"),
|
||||||
|
}
|
||||||
|
|
||||||
|
result := sanitizeHistoryForProvider(history)
|
||||||
|
if len(result) != 8 {
|
||||||
|
t.Fatalf("expected 8 messages, got %d: %+v", len(result), roles(result))
|
||||||
|
}
|
||||||
|
assertRoles(t, result, "user", "assistant", "tool", "assistant", "user", "assistant", "tool", "assistant")
|
||||||
|
if result[2].ToolCallID != "call_1" || result[6].ToolCallID != "call_1" {
|
||||||
|
t.Fatalf("expected both rounds to keep tool_call_id call_1, got %q and %q", result[2].ToolCallID, result[6].ToolCallID)
|
||||||
}
|
}
|
||||||
assertRoles(t, result, "user", "assistant", "tool", "assistant", "user", "user", "assistant", "tool", "assistant")
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue