From c529ebf41a023de0164185398be825dbbdc54ec6 Mon Sep 17 00:00:00 2001 From: ollie Date: Mon, 1 Jun 2026 08:05:38 +0200 Subject: [PATCH] updates --- pkg/agent/compaction_test.go | 63 +++++++++++++++++++++++++++++++++++- pkg/agent/session.go | 10 ++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/pkg/agent/compaction_test.go b/pkg/agent/compaction_test.go index 324f450..1931a7f 100644 --- a/pkg/agent/compaction_test.go +++ b/pkg/agent/compaction_test.go @@ -1,8 +1,11 @@ package agent import ( + "encoding/json" "os" "testing" + + "ollie/pkg/backend" ) type mockBackendForCompact struct { @@ -38,7 +41,7 @@ func TestResolveCompactionModel_BackendDefault(t *testing.T) { }{ {"anthropic", "claude-3-5-haiku-latest"}, {"openai", "gpt-4o-mini"}, - {"openrouter", "gpt-4o-mini"}, + {"openrouter", "deepseek-v4-flash"}, {"gemini", "gemini-2.0-flash"}, {"ollama", ""}, } @@ -50,3 +53,61 @@ func TestResolveCompactionModel_BackendDefault(t *testing.T) { } } } + +func TestBuildCompactedHistory_OrphanedToolMessage(t *testing.T) { + // Simulate a history where the hot zone boundary (total - hotTailSize) + // lands on a tool message whose preceding assistant+tool_calls is outside + // the hot zone. Without the fix, this produces an orphaned tool message + // that causes OpenAI 400 errors. + var msgs []backend.Message + + // Pad with enough messages so the boundary falls on the tool message. + // We need total - hotTailSize to land on the tool result. + // hotTailSize = 8, so we need the tool msg at index total-8. + // Build: 10 user/assistant pairs (20 msgs), then assistant+tool_calls, tool result, then 7 more messages. + for i := range 10 { + msgs = append(msgs, + backend.Message{Role: "user", Content: "q" + string(rune('0'+i))}, + backend.Message{Role: "assistant", Content: "a" + string(rune('0'+i))}, + ) + } + // assistant with tool_calls at index 20 + msgs = append(msgs, backend.Message{ + Role: "assistant", + ToolCalls: []backend.ToolCall{{ID: "call_orphan", Name: "test_tool", Arguments: json.RawMessage(`{}`)}}, + }) + // tool result at index 21 — this is where hotStart would land without the fix + msgs = append(msgs, backend.Message{ + Role: "tool", + Content: "tool output", + ToolCallID: "call_orphan", + }) + // 7 more messages to fill the rest of the hot zone (indices 22-28) + for i := range 3 { + msgs = append(msgs, + backend.Message{Role: "user", Content: "follow " + string(rune('0'+i))}, + backend.Message{Role: "assistant", Content: "reply " + string(rune('0'+i))}, + ) + } + msgs = append(msgs, backend.Message{Role: "user", Content: "final"}) + // total = 29, hotStart = 29 - 8 = 21 (the tool message) + + ts := TaskState{Objective: "test"} + result := buildCompactedHistory(ts, msgs) + + // Verify no tool message appears without a preceding assistant+tool_calls. + for i, m := range result { + if m.Role == "tool" { + if i == 0 { + t.Fatalf("result[0] is a tool message — no preceding assistant") + } + prev := result[i-1] + if prev.Role != "assistant" && prev.Role != "tool" { + t.Fatalf("result[%d] is tool but result[%d] is %q (want assistant or tool)", i, i-1, prev.Role) + } + if prev.Role == "assistant" && len(prev.ToolCalls) == 0 { + t.Fatalf("result[%d] is tool but preceding assistant has no tool_calls", i) + } + } + } +} diff --git a/pkg/agent/session.go b/pkg/agent/session.go index 48a0151..7eca9da 100644 --- a/pkg/agent/session.go +++ b/pkg/agent/session.go @@ -348,6 +348,16 @@ func buildCompactedHistory(ts TaskState, allMessages []backend.Message) []backen if hotStart < 0 { hotStart = 0 } + + // If hotStart lands on a tool message, walk backward to include the + // preceding assistant message with tool_calls. Without this, the hot + // zone starts with an orphaned tool result which violates the OpenAI + // API constraint that tool messages must follow an assistant message + // containing the corresponding tool_calls. + for hotStart > 0 && allMessages[hotStart].Role == "tool" { + hotStart-- + } + warmStart := hotStart - warmIndexSize if warmStart < 0 { warmStart = 0