This commit is contained in:
ollie 2026-06-01 08:05:38 +02:00
parent cb1551fe67
commit c529ebf41a
2 changed files with 72 additions and 1 deletions

View File

@ -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)
}
}
}
}

View File

@ -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