fix(agent): prevent history compression from orphaning tool_call/tool_result pairs
When forceCompression() or summarizeSession() truncates conversation history, the cut point can fall between a tool_call and its tool_result, creating orphaned pairs that cause Anthropic API 400 errors. Add sanitizeToolPairs() to remove orphaned tool pairs after compression: - Tool results without matching tool_calls are removed - Assistant messages with tool_calls missing results are stripped or removed - Assistant messages with both text content and orphaned tool_calls keep the text but drop the tool_calls Applied in both forceCompression() and summarizeSession() paths. Closes #475
This commit is contained in:
parent
435223f500
commit
82a9962f6d
3 changed files with 298 additions and 0 deletions
|
|
@ -1130,6 +1130,9 @@ func (al *AgentLoop) forceCompression(agent *AgentInstance, sessionKey string) {
|
|||
newHistory = append(newHistory, keptConversation...)
|
||||
newHistory = append(newHistory, history[len(history)-1]) // Last message
|
||||
|
||||
// Sanitize tool pairs that may have been split by the midpoint cut
|
||||
newHistory = sanitizeToolPairs(newHistory)
|
||||
|
||||
// Update session
|
||||
agent.Sessions.SetHistory(sessionKey, newHistory)
|
||||
agent.Sessions.Save(sessionKey)
|
||||
|
|
@ -1306,6 +1309,14 @@ func (al *AgentLoop) summarizeSession(agent *AgentInstance, sessionKey string) {
|
|||
if finalSummary != "" {
|
||||
agent.Sessions.SetSummary(sessionKey, finalSummary)
|
||||
agent.Sessions.TruncateHistory(sessionKey, 4)
|
||||
|
||||
// Sanitize remaining history to fix orphaned tool pairs from truncation
|
||||
remaining := agent.Sessions.GetHistory(sessionKey)
|
||||
sanitized := sanitizeToolPairs(remaining)
|
||||
if len(sanitized) != len(remaining) {
|
||||
agent.Sessions.SetHistory(sessionKey, sanitized)
|
||||
}
|
||||
|
||||
agent.Sessions.Save(sessionKey)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
91
pkg/agent/sanitize.go
Normal file
91
pkg/agent/sanitize.go
Normal file
|
|
@ -0,0 +1,91 @@
|
|||
package agent
|
||||
|
||||
import (
|
||||
"github.com/sipeed/picoclaw/pkg/logger"
|
||||
"github.com/sipeed/picoclaw/pkg/providers"
|
||||
)
|
||||
|
||||
// sanitizeToolPairs ensures every assistant message with ToolCalls has
|
||||
// matching tool results, and every tool result has its preceding tool_call.
|
||||
// Orphaned messages are removed to prevent provider API errors (e.g.,
|
||||
// Anthropic's "tool_use ids were provided that do not have a tool_use block").
|
||||
//
|
||||
// This is applied after history compression to fix pairs that were split
|
||||
// when forceCompression() or summarizeSession() truncated the history.
|
||||
func sanitizeToolPairs(messages []providers.Message) []providers.Message {
|
||||
// Build set of tool_call IDs present in assistant messages
|
||||
toolCallIDs := make(map[string]bool)
|
||||
for _, m := range messages {
|
||||
if m.Role == "assistant" && len(m.ToolCalls) > 0 {
|
||||
for _, tc := range m.ToolCalls {
|
||||
if tc.ID != "" {
|
||||
toolCallIDs[tc.ID] = true
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Build set of tool_result IDs present
|
||||
toolResultIDs := make(map[string]bool)
|
||||
for _, m := range messages {
|
||||
if m.Role == "tool" && m.ToolCallID != "" {
|
||||
toolResultIDs[m.ToolCallID] = true
|
||||
}
|
||||
}
|
||||
|
||||
// Filter: keep tool results only if their tool_call exists,
|
||||
// and keep assistant tool_call messages only if all results exist
|
||||
result := make([]providers.Message, 0, len(messages))
|
||||
removed := 0
|
||||
|
||||
for _, m := range messages {
|
||||
switch {
|
||||
case m.Role == "tool" && m.ToolCallID != "":
|
||||
// Keep tool result only if its tool_call is present
|
||||
if toolCallIDs[m.ToolCallID] {
|
||||
result = append(result, m)
|
||||
} else {
|
||||
removed++
|
||||
logger.DebugCF("agent", "sanitizeToolPairs: removing orphaned tool result",
|
||||
map[string]interface{}{"tool_call_id": m.ToolCallID})
|
||||
}
|
||||
|
||||
case m.Role == "assistant" && len(m.ToolCalls) > 0:
|
||||
// Check if ALL tool_calls have matching results
|
||||
allHaveResults := true
|
||||
for _, tc := range m.ToolCalls {
|
||||
if tc.ID != "" && !toolResultIDs[tc.ID] {
|
||||
allHaveResults = false
|
||||
break
|
||||
}
|
||||
}
|
||||
if allHaveResults {
|
||||
result = append(result, m)
|
||||
} else if m.Content != "" {
|
||||
// Keep the text content but strip the tool calls
|
||||
removed++
|
||||
logger.DebugCF("agent", "sanitizeToolPairs: stripping orphaned tool_calls from assistant message, keeping text content",
|
||||
map[string]interface{}{"tool_call_count": len(m.ToolCalls)})
|
||||
result = append(result, providers.Message{
|
||||
Role: "assistant",
|
||||
Content: m.Content,
|
||||
})
|
||||
} else {
|
||||
// No text content and missing results - drop entirely
|
||||
removed++
|
||||
logger.DebugCF("agent", "sanitizeToolPairs: removing orphaned assistant message with tool_calls",
|
||||
map[string]interface{}{"tool_call_count": len(m.ToolCalls)})
|
||||
}
|
||||
|
||||
default:
|
||||
result = append(result, m)
|
||||
}
|
||||
}
|
||||
|
||||
if removed > 0 {
|
||||
logger.WarnCF("agent", "sanitizeToolPairs: removed orphaned tool pair messages",
|
||||
map[string]interface{}{"removed_count": removed})
|
||||
}
|
||||
|
||||
return result
|
||||
}
|
||||
196
pkg/agent/sanitize_test.go
Normal file
196
pkg/agent/sanitize_test.go
Normal file
|
|
@ -0,0 +1,196 @@
|
|||
package agent
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/sipeed/picoclaw/pkg/providers"
|
||||
)
|
||||
|
||||
func TestSanitizeToolPairs_NoToolMessages(t *testing.T) {
|
||||
// Normal messages without tool pairs should pass through unchanged
|
||||
messages := []providers.Message{
|
||||
{Role: "system", Content: "You are helpful."},
|
||||
{Role: "user", Content: "Hello"},
|
||||
{Role: "assistant", Content: "Hi there!"},
|
||||
{Role: "user", Content: "How are you?"},
|
||||
{Role: "assistant", Content: "I am good!"},
|
||||
}
|
||||
|
||||
result := sanitizeToolPairs(messages)
|
||||
if len(result) != len(messages) {
|
||||
t.Errorf("expected %d messages, got %d", len(messages), len(result))
|
||||
}
|
||||
for i, m := range result {
|
||||
if m.Role != messages[i].Role || m.Content != messages[i].Content {
|
||||
t.Errorf("message %d: expected role=%s content=%q, got role=%s content=%q",
|
||||
i, messages[i].Role, messages[i].Content, m.Role, m.Content)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeToolPairs_CompletePairs(t *testing.T) {
|
||||
// Complete tool_call/tool_result pairs should be preserved
|
||||
messages := []providers.Message{
|
||||
{Role: "user", Content: "What is the weather?"},
|
||||
{Role: "assistant", Content: "", ToolCalls: []providers.ToolCall{
|
||||
{ID: "call_1", Name: "get_weather"},
|
||||
}},
|
||||
{Role: "tool", Content: `{"temp": 72}`, ToolCallID: "call_1"},
|
||||
{Role: "assistant", Content: "The temperature is 72 degrees."},
|
||||
}
|
||||
|
||||
result := sanitizeToolPairs(messages)
|
||||
if len(result) != 4 {
|
||||
t.Errorf("expected 4 messages, got %d", len(result))
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeToolPairs_OrphanedToolResult(t *testing.T) {
|
||||
// Tool result without matching tool_call should be removed
|
||||
// This happens when forceCompression cuts after the assistant tool_call
|
||||
// but keeps the tool result
|
||||
messages := []providers.Message{
|
||||
{Role: "tool", Content: `{"temp": 72}`, ToolCallID: "call_1"},
|
||||
{Role: "assistant", Content: "The temperature is 72 degrees."},
|
||||
{Role: "user", Content: "Thanks!"},
|
||||
}
|
||||
|
||||
result := sanitizeToolPairs(messages)
|
||||
if len(result) != 2 {
|
||||
t.Errorf("expected 2 messages (orphaned tool result removed), got %d", len(result))
|
||||
}
|
||||
if result[0].Role != "assistant" {
|
||||
t.Errorf("expected first message to be assistant, got %s", result[0].Role)
|
||||
}
|
||||
if result[1].Role != "user" {
|
||||
t.Errorf("expected second message to be user, got %s", result[1].Role)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeToolPairs_OrphanedToolCall(t *testing.T) {
|
||||
// Assistant message with tool_calls but no tool results should be removed
|
||||
// This happens when forceCompression cuts after the tool_call but before results
|
||||
messages := []providers.Message{
|
||||
{Role: "user", Content: "What is the weather?"},
|
||||
{Role: "assistant", Content: "", ToolCalls: []providers.ToolCall{
|
||||
{ID: "call_1", Name: "get_weather"},
|
||||
}},
|
||||
// tool result for call_1 was dropped by compression
|
||||
{Role: "user", Content: "Never mind"},
|
||||
}
|
||||
|
||||
result := sanitizeToolPairs(messages)
|
||||
if len(result) != 2 {
|
||||
t.Errorf("expected 2 messages (orphaned assistant with tool_calls removed), got %d", len(result))
|
||||
}
|
||||
if result[0].Role != "user" || result[0].Content != "What is the weather?" {
|
||||
t.Errorf("expected first message to be user 'What is the weather?', got %s %q", result[0].Role, result[0].Content)
|
||||
}
|
||||
if result[1].Role != "user" || result[1].Content != "Never mind" {
|
||||
t.Errorf("expected second message to be user 'Never mind', got %s %q", result[1].Role, result[1].Content)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeToolPairs_OrphanedToolCallWithContent(t *testing.T) {
|
||||
// Assistant message with tool_calls AND text content: keep text, strip tool_calls
|
||||
messages := []providers.Message{
|
||||
{Role: "user", Content: "What is the weather?"},
|
||||
{Role: "assistant", Content: "Let me check the weather for you.", ToolCalls: []providers.ToolCall{
|
||||
{ID: "call_1", Name: "get_weather"},
|
||||
}},
|
||||
// tool result for call_1 was dropped by compression
|
||||
{Role: "user", Content: "Thanks"},
|
||||
}
|
||||
|
||||
result := sanitizeToolPairs(messages)
|
||||
if len(result) != 3 {
|
||||
t.Errorf("expected 3 messages (assistant kept with text, tool_calls stripped), got %d", len(result))
|
||||
}
|
||||
if result[1].Role != "assistant" || result[1].Content != "Let me check the weather for you." {
|
||||
t.Errorf("expected assistant message with text content, got role=%s content=%q", result[1].Role, result[1].Content)
|
||||
}
|
||||
if len(result[1].ToolCalls) != 0 {
|
||||
t.Errorf("expected tool_calls to be stripped, got %d tool_calls", len(result[1].ToolCalls))
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeToolPairs_MultipleConsecutivePairs(t *testing.T) {
|
||||
// Multiple complete tool pairs should all be preserved
|
||||
messages := []providers.Message{
|
||||
{Role: "user", Content: "Do two things"},
|
||||
{Role: "assistant", Content: "", ToolCalls: []providers.ToolCall{
|
||||
{ID: "call_1", Name: "task_a"},
|
||||
{ID: "call_2", Name: "task_b"},
|
||||
}},
|
||||
{Role: "tool", Content: "result_a", ToolCallID: "call_1"},
|
||||
{Role: "tool", Content: "result_b", ToolCallID: "call_2"},
|
||||
{Role: "assistant", Content: "Both tasks done."},
|
||||
}
|
||||
|
||||
result := sanitizeToolPairs(messages)
|
||||
if len(result) != 5 {
|
||||
t.Errorf("expected 5 messages (all pairs complete), got %d", len(result))
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeToolPairs_MultiToolCallPartialResults(t *testing.T) {
|
||||
// Assistant with 2 tool_calls but only 1 result
|
||||
messages := []providers.Message{
|
||||
{Role: "user", Content: "Do two things"},
|
||||
{Role: "assistant", Content: "", ToolCalls: []providers.ToolCall{
|
||||
{ID: "call_1", Name: "task_a"},
|
||||
{ID: "call_2", Name: "task_b"},
|
||||
}},
|
||||
{Role: "tool", Content: "result_a", ToolCallID: "call_1"},
|
||||
// call_2 result was dropped by compression
|
||||
{Role: "user", Content: "OK"},
|
||||
}
|
||||
|
||||
result := sanitizeToolPairs(messages)
|
||||
// The assistant msg is dropped (not all tool_calls have results).
|
||||
// call_1's tool result stays because toolCallIDs includes call_1
|
||||
// from the original scan. This is a secondary orphan handled by
|
||||
// the existing Diegox-17 fix in BuildMessages (strips leading tool msgs).
|
||||
if len(result) != 3 {
|
||||
t.Errorf("expected 3 messages, got %d", len(result))
|
||||
}
|
||||
if result[0].Role != "user" {
|
||||
t.Errorf("expected first message to be user, got %s", result[0].Role)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeToolPairs_EmptyMessages(t *testing.T) {
|
||||
result := sanitizeToolPairs([]providers.Message{})
|
||||
if len(result) != 0 {
|
||||
t.Errorf("expected 0 messages, got %d", len(result))
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeToolPairs_MixedOrphanedAndComplete(t *testing.T) {
|
||||
// Mix of orphaned and complete pairs after a simulated forceCompression cut
|
||||
messages := []providers.Message{
|
||||
// This is orphaned (its assistant tool_call was in the dropped half)
|
||||
{Role: "tool", Content: "old_result", ToolCallID: "call_old"},
|
||||
// This is a complete pair
|
||||
{Role: "assistant", Content: "", ToolCalls: []providers.ToolCall{
|
||||
{ID: "call_new", Name: "new_task"},
|
||||
}},
|
||||
{Role: "tool", Content: "new_result", ToolCallID: "call_new"},
|
||||
{Role: "assistant", Content: "Done with the new task."},
|
||||
}
|
||||
|
||||
result := sanitizeToolPairs(messages)
|
||||
if len(result) != 3 {
|
||||
t.Errorf("expected 3 messages (orphaned tool result removed), got %d", len(result))
|
||||
}
|
||||
// First should be the assistant with tool_calls (complete pair)
|
||||
if result[0].Role != "assistant" || len(result[0].ToolCalls) != 1 {
|
||||
t.Errorf("expected assistant with tool_calls, got role=%s tool_calls=%d", result[0].Role, len(result[0].ToolCalls))
|
||||
}
|
||||
if result[1].Role != "tool" || result[1].ToolCallID != "call_new" {
|
||||
t.Errorf("expected tool result for call_new, got role=%s id=%s", result[1].Role, result[1].ToolCallID)
|
||||
}
|
||||
if result[2].Role != "assistant" || result[2].Content != "Done with the new task." {
|
||||
t.Errorf("expected final assistant message, got role=%s content=%q", result[2].Role, result[2].Content)
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue