mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-15 00:17:27 +00:00
fix(agent): clear RawAssistantContent on dedup + merge consecutive same-role messages
sanitizeHistory dedup rewrites ToolCalls IDs but not RawAssistantContent. Anthropic provider prefers RawAssistantContent → sends stale IDs → HTTP 400. Fix: clear RawAssistantContent when dedup fires. Also merge consecutive same-role messages (user↔user, assistant↔assistant) to fix role alternation errors from session corruption.
This commit is contained in:
1 parent
79cae648e4
commit
d79d738148
2 files changed
+118
-3
No files matched your search
@@ -504,7 +504,8 @@ func sanitizeHistory(msgs []providers.Message) ([]providers.Message, int) {
|
||||
return nil, dropped
|
||||
}
|
||||
|
||||
// 2. Walk through messages ensuring tool_result follows matching tool_use.
|
||||
// 2. Walk through messages ensuring tool_result follows matching tool_use
|
||||
// and that roles alternate correctly (user↔assistant).
|
||||
// Also dedup tool call IDs across the transcript for legacy sessions that
|
||||
// may have persisted duplicates before the live uniquify fix was deployed.
|
||||
var result []providers.Message
|
||||
@@ -524,18 +525,26 @@ func sanitizeHistory(msgs []providers.Message) ([]providers.Message, int) {
|
||||
// results with the same raw ID pair correctly in encounter order.
|
||||
idQueue := make(map[string][]string, len(msg.ToolCalls)) // origID → []newID
|
||||
expectedIDs := make(map[string]bool, len(msg.ToolCalls))
|
||||
didDedup := false
|
||||
for j := range msg.ToolCalls {
|
||||
origID := msg.ToolCalls[j].ID
|
||||
newID := origID
|
||||
if globalSeen[origID] {
|
||||
newID = fmt.Sprintf("%s_dedup_%d", origID, j)
|
||||
slog.Debug("sanitizeHistory: dedup tool call ID", "orig", origID, "new", newID)
|
||||
didDedup = true
|
||||
dropped++ // count as change so cleaned history is persisted back to DB
|
||||
}
|
||||
msg.ToolCalls[j].ID = newID
|
||||
globalSeen[newID] = true
|
||||
idQueue[origID] = append(idQueue[origID], newID)
|
||||
expectedIDs[newID] = true
|
||||
}
|
||||
// When dedup rewrites IDs, clear RawAssistantContent so the provider
|
||||
// uses the corrected ToolCalls instead of raw JSON with stale IDs.
|
||||
if didDedup {
|
||||
msg.RawAssistantContent = nil
|
||||
}
|
||||
|
||||
result = append(result, msg)
|
||||
|
||||
@@ -578,6 +587,28 @@ func sanitizeHistory(msgs []providers.Message) ([]providers.Message, int) {
|
||||
}
|
||||
}
|
||||
|
||||
// 3. Fix role alternation: LLM APIs require user↔assistant alternation.
|
||||
// Merge consecutive same-role messages (e.g. two user messages) into one,
|
||||
// which can happen from bootstrap nudges, inject channel, or session corruption.
|
||||
if len(result) > 1 {
|
||||
merged := make([]providers.Message, 0, len(result))
|
||||
merged = append(merged, result[0])
|
||||
for j := 1; j < len(result); j++ {
|
||||
prev := &merged[len(merged)-1]
|
||||
curr := result[j]
|
||||
// Only merge plain messages (no tool_calls, no tool role)
|
||||
if curr.Role == prev.Role && curr.Role != "tool" && len(curr.ToolCalls) == 0 && len(prev.ToolCalls) == 0 {
|
||||
slog.Debug("sanitizeHistory: merging consecutive same-role messages",
|
||||
"role", curr.Role, "index", j)
|
||||
prev.Content += "\n\n" + curr.Content
|
||||
dropped++
|
||||
} else {
|
||||
merged = append(merged, curr)
|
||||
}
|
||||
}
|
||||
result = merged
|
||||
}
|
||||
|
||||
return result, dropped
|
||||
}
|
||||
|
||||
|
||||
@@ -257,8 +257,8 @@ func TestSanitizeHistory_DedupsDuplicateIDsWithinTurn(t *testing.T) {
|
||||
{Role: "assistant", Content: "done"},
|
||||
}
|
||||
got, dropped := sanitizeHistory(msgs)
|
||||
if dropped != 0 {
|
||||
t.Errorf("expected 0 dropped, got %d", dropped)
|
||||
if dropped != 1 {
|
||||
t.Errorf("expected 1 dropped (dedup counts as change), got %d", dropped)
|
||||
}
|
||||
|
||||
// Both tool results must be present and paired correctly
|
||||
@@ -326,6 +326,90 @@ func TestSanitizeHistory_NoDedupWhenIDsUnique(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeHistory_MergesConsecutiveUserMessages(t *testing.T) {
|
||||
msgs := []providers.Message{
|
||||
{Role: "user", Content: "hello"},
|
||||
{Role: "user", Content: "world"},
|
||||
{Role: "assistant", Content: "hi there"},
|
||||
}
|
||||
got, dropped := sanitizeHistory(msgs)
|
||||
if len(got) != 2 {
|
||||
t.Fatalf("expected 2 messages after merge, got %d", len(got))
|
||||
}
|
||||
if got[0].Content != "hello\n\nworld" {
|
||||
t.Errorf("expected merged content, got %q", got[0].Content)
|
||||
}
|
||||
if dropped != 1 {
|
||||
t.Errorf("expected 1 dropped, got %d", dropped)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeHistory_MergesConsecutiveAssistantMessages(t *testing.T) {
|
||||
msgs := []providers.Message{
|
||||
{Role: "user", Content: "question"},
|
||||
{Role: "assistant", Content: "part 1"},
|
||||
{Role: "assistant", Content: "part 2"},
|
||||
{Role: "user", Content: "follow-up"},
|
||||
}
|
||||
got, dropped := sanitizeHistory(msgs)
|
||||
if len(got) != 3 {
|
||||
t.Fatalf("expected 3 messages after merge, got %d", len(got))
|
||||
}
|
||||
if got[1].Content != "part 1\n\npart 2" {
|
||||
t.Errorf("expected merged assistant content, got %q", got[1].Content)
|
||||
}
|
||||
if dropped != 1 {
|
||||
t.Errorf("expected 1 dropped, got %d", dropped)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeHistory_DedupClearsRawAssistantContent(t *testing.T) {
|
||||
// When dedup rewrites tool call IDs, RawAssistantContent (which has the old IDs)
|
||||
// must be cleared so the provider uses the corrected ToolCalls.
|
||||
rawContent := []byte(`[{"type":"text","text":"ok"},{"type":"tool_use","id":"tool_1","name":"search","input":{}}]`)
|
||||
msgs := []providers.Message{
|
||||
{Role: "user", Content: "first"},
|
||||
{Role: "assistant", ToolCalls: []providers.ToolCall{{ID: "tool_1", Name: "search"}}, RawAssistantContent: rawContent},
|
||||
{Role: "tool", Content: "result1", ToolCallID: "tool_1"},
|
||||
{Role: "assistant", Content: "ok"},
|
||||
{Role: "user", Content: "second"},
|
||||
// Second turn has same tool_1 ID — triggers dedup
|
||||
{Role: "assistant", ToolCalls: []providers.ToolCall{{ID: "tool_1", Name: "search"}}, RawAssistantContent: rawContent},
|
||||
{Role: "tool", Content: "result2", ToolCallID: "tool_1"},
|
||||
{Role: "assistant", Content: "done"},
|
||||
}
|
||||
got, _ := sanitizeHistory(msgs)
|
||||
// First assistant should keep RawAssistantContent (no dedup needed)
|
||||
if got[1].RawAssistantContent == nil {
|
||||
t.Error("first assistant should keep RawAssistantContent")
|
||||
}
|
||||
// Second assistant (index 5) should have RawAssistantContent cleared due to dedup
|
||||
if got[5].RawAssistantContent != nil {
|
||||
t.Error("dedup'd assistant should have RawAssistantContent cleared")
|
||||
}
|
||||
// Tool result for second turn should have dedup'd ID
|
||||
if got[6].ToolCallID == "tool_1" {
|
||||
t.Errorf("expected dedup'd tool_call_id, got %q", got[6].ToolCallID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeHistory_NoMergeForToolCallMessages(t *testing.T) {
|
||||
// Assistant messages WITH tool_calls should NOT be merged
|
||||
msgs := []providers.Message{
|
||||
{Role: "user", Content: "do something"},
|
||||
{Role: "assistant", Content: "ok", ToolCalls: []providers.ToolCall{{ID: "tc1", Name: "test"}}},
|
||||
{Role: "tool", Content: "result", ToolCallID: "tc1"},
|
||||
{Role: "assistant", Content: "done"},
|
||||
}
|
||||
got, dropped := sanitizeHistory(msgs)
|
||||
if len(got) != 4 {
|
||||
t.Fatalf("expected 4 messages (no merge), got %d", len(got))
|
||||
}
|
||||
if dropped != 0 {
|
||||
t.Errorf("expected 0 dropped, got %d", dropped)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUniquifyToolCallIDs(t *testing.T) {
|
||||
runID := "abcdef12-3456-7890-abcd-ef1234567890"
|
||||
|
||||
|
||||
Reference in new issue
Block a user