fix(seahorse): derive readable content from Parts and cap CompactUntilUnder iterations
- Derive readable content from MessageParts in AddMessageWithParts so FTS5 indexing and summary formatting can access tool call information - formatMessagesForSummary and truncateSummary now fall back to Parts when Content is empty, fixing blank summaries for Part-based messages - Add MaxCompactIterations (20) to prevent CompactUntilUnder infinite loops; exceeded iterations are logged as warnings
This commit is contained in:
parent
02c040ba9e
commit
f40e989c2e
5 changed files with 258 additions and 4 deletions
58
pkg/seahorse/compact_until_under_test.go
Normal file
58
pkg/seahorse/compact_until_under_test.go
Normal file
|
|
@ -0,0 +1,58 @@
|
|||
package seahorse
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// =============================================================================
|
||||
// CompactUntilUnder iteration cap
|
||||
// =============================================================================
|
||||
|
||||
func TestCompactUntilUnderIterationCap(t *testing.T) {
|
||||
// Setup: create a conversation with so many tokens that compaction
|
||||
// will never reach the budget. The iteration cap prevents infinite loops.
|
||||
//
|
||||
// We use a mock CompleteFn that always returns the same content,
|
||||
// and a budget of 0 which tokens can never reach.
|
||||
// Without the cap, this would loop forever.
|
||||
|
||||
db := openTestDB(t)
|
||||
if err := runSchema(db); err != nil {
|
||||
t.Fatalf("migration: %v", err)
|
||||
}
|
||||
s := &Store{db: db}
|
||||
|
||||
conv, _ := s.GetOrCreateConversation(context.Background(), "agent:iter-cap")
|
||||
convID := conv.ConversationID
|
||||
|
||||
// Add many messages to ensure there's plenty to compact
|
||||
for i := 0; i < 40; i++ {
|
||||
m, _ := s.AddMessage(context.Background(), convID, "user",
|
||||
"this is a long message with lots of tokens to push context over budget", 100)
|
||||
s.AppendContextMessage(context.Background(), convID, m.ID)
|
||||
}
|
||||
|
||||
// A completeFn that always succeeds but returns non-reducing content
|
||||
mockComplete := func(ctx context.Context, prompt string, opts CompleteOptions) (string, error) {
|
||||
return "Summary that doesn't reduce tokens much.", nil
|
||||
}
|
||||
|
||||
ce, cancel := newTestCompactionEngineWithStore(s, mockComplete)
|
||||
defer cancel()
|
||||
|
||||
// Use budget=1 so tokens can never reach budget
|
||||
// (each message is 100 tokens, so 40 messages = 4000 tokens, budget 1 is unreachable)
|
||||
// The function should stop after maxCompactIterations, not loop forever
|
||||
ce.config = Config{} // ensure defaults
|
||||
|
||||
result, err := ce.CompactUntilUnder(context.Background(), convID, 1)
|
||||
if err != nil {
|
||||
// Should not error — should stop gracefully
|
||||
t.Fatalf("CompactUntilUnder with budget=0: %v", err)
|
||||
}
|
||||
|
||||
// The function should have completed within reasonable time
|
||||
// If it exceeded the cap, it would still return (not hang)
|
||||
_ = result
|
||||
}
|
||||
144
pkg/seahorse/parts_roundtrip_test.go
Normal file
144
pkg/seahorse/parts_roundtrip_test.go
Normal file
|
|
@ -0,0 +1,144 @@
|
|||
package seahorse
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// =============================================================================
|
||||
// Bug 1: formatMessagesForSummary ignores Parts
|
||||
// - formatMessagesForSummary only reads m.Content, empty for Part-based messages
|
||||
// - truncateSummary has same issue
|
||||
// =============================================================================
|
||||
|
||||
func TestFormatMessagesForSummaryIncludesParts(t *testing.T) {
|
||||
ts := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)
|
||||
|
||||
messages := []Message{
|
||||
{ID: 1, Role: "user", Content: "hello world", CreatedAt: ts},
|
||||
{
|
||||
ID: 2,
|
||||
Role: "assistant",
|
||||
Content: "", // empty — real content is in Parts
|
||||
Parts: []MessagePart{
|
||||
{Type: "text", Text: "I will run a command"},
|
||||
{Type: "tool_use", Name: "bash", Arguments: `{"command":"ls -la"}`, ToolCallID: "call_1"},
|
||||
},
|
||||
CreatedAt: ts.Add(time.Minute),
|
||||
},
|
||||
{
|
||||
ID: 3,
|
||||
Role: "tool",
|
||||
Content: "", // empty — real content is in Parts
|
||||
Parts: []MessagePart{
|
||||
{Type: "tool_result", Text: "file1.txt\nfile2.txt", ToolCallID: "call_1"},
|
||||
},
|
||||
CreatedAt: ts.Add(2 * time.Minute),
|
||||
},
|
||||
}
|
||||
|
||||
result := formatMessagesForSummary(messages)
|
||||
|
||||
// Must contain the plain text message
|
||||
if !contains(result, "hello world") {
|
||||
t.Error("formatMessagesForSummary: missing plain text content")
|
||||
}
|
||||
|
||||
// Must contain tool_use info (not blank)
|
||||
if !contains(result, "bash") || !contains(result, "ls -la") {
|
||||
t.Errorf("formatMessagesForSummary: tool_use info missing from Parts.\nGot:\n%s", result)
|
||||
}
|
||||
|
||||
// Must contain tool_result info (not blank)
|
||||
if !contains(result, "file1.txt") {
|
||||
t.Errorf("formatMessagesForSummary: tool_result text missing from Parts.\nGot:\n%s", result)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTruncateSummaryIncludesParts(t *testing.T) {
|
||||
messages := []Message{
|
||||
{ID: 1, Role: "user", Content: "run the tests", CreatedAt: time.Now()},
|
||||
{
|
||||
ID: 2,
|
||||
Role: "assistant",
|
||||
Content: "", // empty
|
||||
Parts: []MessagePart{
|
||||
{Type: "tool_use", Name: "bash", Arguments: `{"command":"go test ./..."}`, ToolCallID: "call_1"},
|
||||
},
|
||||
CreatedAt: time.Now(),
|
||||
},
|
||||
{
|
||||
ID: 3,
|
||||
Role: "tool",
|
||||
Content: "", // empty
|
||||
Parts: []MessagePart{
|
||||
{Type: "tool_result", Text: "PASS\nok 3.2s", ToolCallID: "call_1"},
|
||||
},
|
||||
CreatedAt: time.Now(),
|
||||
},
|
||||
}
|
||||
|
||||
result := truncateSummary(messages)
|
||||
|
||||
// Must contain plain text
|
||||
if !contains(result, "run the tests") {
|
||||
t.Error("truncateSummary: missing plain text content")
|
||||
}
|
||||
|
||||
// Must contain tool info from Parts (not blank)
|
||||
if !contains(result, "bash") || !contains(result, "go test") {
|
||||
t.Errorf("truncateSummary: tool_use info missing from Parts.\nGot:\n%s", result)
|
||||
}
|
||||
|
||||
// Must contain tool_result from Parts
|
||||
if !contains(result, "PASS") {
|
||||
t.Errorf("truncateSummary: tool_result text missing from Parts.\nGot:\n%s", result)
|
||||
}
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Bug 2: SearchMessages cannot find Part-based messages
|
||||
// - FTS5 indexes empty content, LIKE queries empty content
|
||||
// =============================================================================
|
||||
|
||||
func TestSearchMessagesFindsPartBasedMessages(t *testing.T) {
|
||||
s := openTestStore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
conv, _ := s.GetOrCreateConversation(ctx, "agent:search-parts")
|
||||
convID := conv.ConversationID
|
||||
|
||||
// Add a plain message (searchable)
|
||||
s.AddMessage(ctx, convID, "user", "list the files please", 5)
|
||||
|
||||
// Add a Part-based message (tool_use) — currently NOT searchable
|
||||
parts := []MessagePart{
|
||||
{Type: "tool_use", Name: "bash", Arguments: `{"command":"grep -r TODO ."}`, ToolCallID: "call_1"},
|
||||
}
|
||||
s.AddMessageWithParts(ctx, convID, "assistant", parts, 10)
|
||||
|
||||
// Add a Part-based message (tool_result) — currently NOT searchable
|
||||
resultParts := []MessagePart{
|
||||
{Type: "tool_result", Text: "main.go:42: TODO fix this bug", ToolCallID: "call_1"},
|
||||
}
|
||||
s.AddMessageWithParts(ctx, convID, "tool", resultParts, 10)
|
||||
|
||||
// Search for "grep" — should find the tool_use message
|
||||
results, err := s.SearchMessages(ctx, SearchInput{Pattern: "grep"})
|
||||
if err != nil {
|
||||
t.Fatalf("SearchMessages: %v", err)
|
||||
}
|
||||
if len(results) == 0 {
|
||||
t.Error("SearchMessages: 'grep' not found — Part-based messages are invisible to search")
|
||||
}
|
||||
|
||||
// Search for "TODO fix" — should find the tool_result message
|
||||
results2, err := s.SearchMessages(ctx, SearchInput{Pattern: "TODO fix"})
|
||||
if err != nil {
|
||||
t.Fatalf("SearchMessages: %v", err)
|
||||
}
|
||||
if len(results2) == 0 {
|
||||
t.Error("SearchMessages: 'TODO fix' not found — tool_result messages are invisible to search")
|
||||
}
|
||||
}
|
||||
|
|
@ -98,7 +98,7 @@ func (e *CompactionEngine) CompactUntilUnder(ctx context.Context, convID int64,
|
|||
prevTokens := 0
|
||||
logger.InfoCF("seahorse", "compact_until_under: start", map[string]any{"conv_id": convID, "budget": budget})
|
||||
|
||||
for {
|
||||
for iter := 0; iter < MaxCompactIterations; iter++ {
|
||||
tokens, err := e.store.GetContextTokenCount(ctx, convID)
|
||||
if err != nil {
|
||||
return result, fmt.Errorf("get tokens: %w", err)
|
||||
|
|
@ -155,6 +155,15 @@ func (e *CompactionEngine) CompactUntilUnder(ctx context.Context, convID int64,
|
|||
}
|
||||
prevTokens = newTokens
|
||||
}
|
||||
|
||||
// Safety cap exceeded — see MaxCompactIterations doc for rationale.
|
||||
logger.WarnCF("seahorse", "compact_until_under: exceeded max iterations", map[string]any{
|
||||
"conv_id": convID,
|
||||
"budget": budget,
|
||||
"iterations": MaxCompactIterations,
|
||||
"tokens": prevTokens,
|
||||
})
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// compactLeaf compresses the oldest contiguous message chunk into a leaf summary.
|
||||
|
|
@ -738,7 +747,11 @@ func formatMessagesForSummary(messages []Message) string {
|
|||
var result string
|
||||
for _, m := range messages {
|
||||
ts := m.CreatedAt.Format("2006-01-02 15:04 MST")
|
||||
result += fmt.Sprintf("[%s]\n%s\n\n", ts, m.Content)
|
||||
content := m.Content
|
||||
if content == "" && len(m.Parts) > 0 {
|
||||
content = partsToReadableContent(m.Parts)
|
||||
}
|
||||
result += fmt.Sprintf("[%s]\n%s\n\n", ts, content)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
|
@ -836,7 +849,11 @@ Output requirements:
|
|||
func truncateSummary(messages []Message) string {
|
||||
content := ""
|
||||
for _, m := range messages {
|
||||
content += m.Content + "\n"
|
||||
c := m.Content
|
||||
if c == "" && len(m.Parts) > 0 {
|
||||
c = partsToReadableContent(m.Parts)
|
||||
}
|
||||
content += c + "\n"
|
||||
}
|
||||
if len(content) > 2048 {
|
||||
content = content[:2048]
|
||||
|
|
|
|||
|
|
@ -21,4 +21,10 @@ const (
|
|||
LeafTargetTokens int = 1200 // Target tokens for leaf summaries
|
||||
CondensedTargetTokens int = 2000 // Target tokens for condensed summaries
|
||||
MaxExpandTokens int = 4000 // Token cap for expansion queries
|
||||
|
||||
// MaxCompactIterations caps CompactUntilUnder to prevent infinite loops.
|
||||
// Each iteration reduces ~4x tokens via leaf (8:1) or condensed (4:1) compaction.
|
||||
// With a 200k token context window and 75% threshold, ~20 iterations is enough
|
||||
// for any realistic scenario. If exceeded, the issue is logged as a warning.
|
||||
MaxCompactIterations int = 20
|
||||
)
|
||||
|
|
|
|||
|
|
@ -179,6 +179,32 @@ func (s *Store) AddMessage(ctx context.Context, convID int64, role, content stri
|
|||
}, nil
|
||||
}
|
||||
|
||||
// partsToReadableContent derives a readable text summary from message parts.
|
||||
// This ensures FTS5 indexing and summary formatting can access tool call information.
|
||||
func partsToReadableContent(parts []MessagePart) string {
|
||||
var b strings.Builder
|
||||
for i, p := range parts {
|
||||
if i > 0 {
|
||||
b.WriteString("\n")
|
||||
}
|
||||
switch p.Type {
|
||||
case "text":
|
||||
b.WriteString(p.Text)
|
||||
case "tool_use":
|
||||
fmt.Fprintf(&b, "[tool_use: %s, args: %s]", p.Name, p.Arguments)
|
||||
case "tool_result":
|
||||
fmt.Fprintf(&b, "[tool_result for %s: %s]", p.ToolCallID, p.Text)
|
||||
case "media":
|
||||
fmt.Fprintf(&b, "[media: %s (%s)]", p.MediaURI, p.MimeType)
|
||||
default:
|
||||
if p.Text != "" {
|
||||
b.WriteString(p.Text)
|
||||
}
|
||||
}
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
// AddMessageWithParts adds a message with structured parts.
|
||||
func (s *Store) AddMessageWithParts(
|
||||
ctx context.Context,
|
||||
|
|
@ -193,9 +219,12 @@ func (s *Store) AddMessageWithParts(
|
|||
}
|
||||
defer tx.Rollback()
|
||||
|
||||
// Derive readable content from Parts for FTS5 indexing and summary formatting
|
||||
readableContent := partsToReadableContent(parts)
|
||||
|
||||
result, err := tx.ExecContext(ctx,
|
||||
"INSERT INTO messages (conversation_id, role, content, token_count) VALUES (?, ?, ?, ?)",
|
||||
convID, role, "", tokenCount,
|
||||
convID, role, readableContent, tokenCount,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("add message: %w", err)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue