From f77ddbeebfa4f98778a4a0b047f1c0afbab2808d Mon Sep 17 00:00:00 2001 From: Liu Yuan Date: Fri, 3 Apr 2026 12:53:41 +0800 Subject: [PATCH] fix(seahorse): correct 3 adapter bugs in context management MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - TokenCount: use full message (Content+ToolCalls+Media) instead of Content-only - Empty Content: rebuild Content from tool_result Parts when stored empty - Duplicate summaries: summaries only in Summary field, not in History messages - Grep: fix SearchResult.Snippet→Content for summaries - Schema: fix FTS5 SQL uses VIRTUAL TABLE not TEMP TABLE - TestFTS5SQLConstants: verify FTS5 SQL syntax correctness - Test: fix flaky TestCompactLeaf --- pkg/agent/context_seahorse.go | 41 +---- pkg/agent/context_seahorse_test.go | 220 +++++++++++++++++++++----- pkg/seahorse/schema.go | 40 +++-- pkg/seahorse/schema_test.go | 28 ++++ pkg/seahorse/short_assembler.go | 53 ++++--- pkg/seahorse/short_assembler_test.go | 125 +++++++-------- pkg/seahorse/short_compaction.go | 6 + pkg/seahorse/short_compaction_test.go | 11 +- pkg/seahorse/short_engine.go | 7 +- pkg/seahorse/store.go | 4 +- pkg/seahorse/store_test.go | 35 ++++ 11 files changed, 372 insertions(+), 198 deletions(-) diff --git a/pkg/agent/context_seahorse.go b/pkg/agent/context_seahorse.go index cd1d7fdab..367730833 100644 --- a/pkg/agent/context_seahorse.go +++ b/pkg/agent/context_seahorse.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "strings" "github.com/sipeed/picoclaw/pkg/logger" "github.com/sipeed/picoclaw/pkg/providers" @@ -112,44 +111,15 @@ func (m *seahorseContextManager) Assemble(ctx context.Context, req *AssembleRequ return nil, fmt.Errorf("seahorse assemble: %w", err) } - // Convert to provider messages history := seahorseToProviderMessages(result) - // Format ALL summaries as XML with metadata (depth, kind, etc.) - // This allows the LLM to understand the hierarchical structure - var summaryParts []string - for _, sum := range result.Summaries { - if sum.Content != "" { - // Get parent IDs for condensed summaries - parentIDs := getParentSummaryIDs(ctx, m.engine.GetRetrieval().Store(), sum.SummaryID) - xml := seahorse.FormatSummaryXML(&sum, parentIDs) - summaryParts = append(summaryParts, xml) - } - } - var summary string - if len(summaryParts) > 0 { - summary = strings.Join(summaryParts, "\n\n") - } - + // Summary is already formatted as XML with system prompt addition by assembler return &AssembleResponse{ History: history, - Summary: summary, + Summary: result.Summary, }, nil } -// getParentSummaryIDs retrieves parent summary IDs for a summary. -func getParentSummaryIDs(ctx context.Context, store *seahorse.Store, summaryID string) []string { - parents, err := store.GetSummaryParents(ctx, summaryID) - if err != nil { - return nil - } - ids := make([]string, len(parents)) - for i, p := range parents { - ids[i] = p.SummaryID - } - return ids -} - // Compact compresses conversation history via seahorse summarization. func (m *seahorseContextManager) Compact(ctx context.Context, req *CompactRequest) error { if req == nil { @@ -212,7 +182,7 @@ func providerToSeahorseMessage(msg protocoltypes.Message) seahorse.Message { Role: msg.Role, Content: msg.Content, ReasoningContent: msg.ReasoningContent, - TokenCount: tokenizer.EstimateMessageTokens(providers.Message{Content: msg.Content}), + TokenCount: tokenizer.EstimateMessageTokens(msg), } // Convert ToolCalls → MessageParts @@ -249,8 +219,6 @@ func providerToSeahorseMessage(msg protocoltypes.Message) seahorse.Message { } // seahorseToProviderMessages converts a seahorse.AssembleResult to []providers.Message. -// NOTE: Summaries are already included in Messages as XML-formatted messages by the assembler. -// We do NOT convert Summaries separately to avoid double injection. func seahorseToProviderMessages(result *seahorse.AssembleResult) []protocoltypes.Message { messages := make([]protocoltypes.Message, 0, len(result.Messages)) @@ -276,6 +244,9 @@ func seahorseToProviderMessages(result *seahorse.AssembleResult) []protocoltypes } if part.Type == "tool_result" { pm.ToolCallID = part.ToolCallID + if pm.Content == "" && part.Text != "" { + pm.Content = part.Text + } } if part.Type == "media" && part.MediaURI != "" { pm.Media = append(pm.Media, part.MediaURI) diff --git a/pkg/agent/context_seahorse_test.go b/pkg/agent/context_seahorse_test.go index d8e3960a8..fa14c9124 100644 --- a/pkg/agent/context_seahorse_test.go +++ b/pkg/agent/context_seahorse_test.go @@ -221,16 +221,9 @@ func TestSeahorseToProviderMessages(t *testing.T) { Content: "hello", TokenCount: 5, } - summary := seahorse.Summary{ - SummaryID: "sum_test", - Kind: seahorse.SummaryKindLeaf, - Content: "test summary content", - TokenCount: 50, - } result := seahorseToProviderMessages(&seahorse.AssembleResult{ - Summaries: []seahorse.Summary{summary}, // Should be ignored - Messages: []seahorse.Message{summaryMsg, rawMsg}, + Messages: []seahorse.Message{summaryMsg, rawMsg}, }) // Should have exactly 2 messages (from Messages slice only) @@ -516,7 +509,8 @@ func TestSeahorseCompactRetryUsesCompactUntilUnder(t *testing.T) { if result == nil { t.Fatal("expected non-nil assemble result") } - _ = result.TokenCount // compaction attempted — no assertion on exact count since no LLM + // Compaction attempted — no assertion on exact count since no LLM + _ = result.Summary } // TestSeahorseRealLoopNoDuplicateMessages tests the real-world scenario: @@ -693,42 +687,182 @@ func TestSeahorseAssembleReturnsAllSummaries(t *testing.T) { t.Fatalf("engine.Assemble: %v", err) } - t.Logf("Seahorse returned %d summaries", len(result.Summaries)) - for i, sum := range result.Summaries { - contentPreview := sum.Content - if len(contentPreview) > 50 { - contentPreview = contentPreview[:50] + "..." - } - t.Logf(" summary[%d]: id=%s depth=%d tokens=%d content=%s", - i, sum.SummaryID, sum.Depth, sum.TokenCount, contentPreview) - } + t.Logf("Seahorse returned Summary with %d chars", len(result.Summary)) - // The Summary field in AssembleResponse should contain ALL summaries as XML - // with metadata (depth, kind) so the LLM can understand the hierarchy - if len(result.Summaries) > 1 { - // Check if Summary field contains all summaries - if resp.Summary == "" { - t.Error("Summary field is empty but there are multiple summaries") + // The Summary field should contain XML summaries with metadata (depth, kind) + // The assembler generates this from the Summaries list + if len(resp.Summary) > 0 { + // Should contain XML tag + if !strings.Contains(resp.Summary, " 20 { - prefix := sum.Content[:20] - if !strings.Contains(resp.Summary, prefix) { - t.Errorf("Summary[%d] content (prefix %q) not found in Summary", i, prefix) - } - } + // Should contain depth attribute + if !strings.Contains(resp.Summary, `depth="`) { + t.Error("Summary field should contain depth attribute") + } + // Should contain kind attribute + if !strings.Contains(resp.Summary, `kind="`) { + t.Error("Summary field should contain kind attribute") } } } + +func TestProviderToSeahorseMessageTokenCountIncludesAllFields(t *testing.T) { + // Message with only Content + msgContentOnly := protocoltypes.Message{ + Role: "assistant", + Content: "This is a simple response with some text content.", + } + resultContentOnly := providerToSeahorseMessage(msgContentOnly) + + // Message with Content + ToolCalls + msgWithToolCalls := protocoltypes.Message{ + Role: "assistant", + Content: "This is a simple response with some text content.", + ToolCalls: []protocoltypes.ToolCall{ + { + ID: "tc_123", + Function: &protocoltypes.FunctionCall{ + Name: "read_file", + Arguments: `{"path":"/home/user/document.txt"}`, + }, + }, + }, + } + resultWithToolCalls := providerToSeahorseMessage(msgWithToolCalls) + + if resultWithToolCalls.TokenCount <= resultContentOnly.TokenCount { + t.Errorf("TokenCount with ToolCalls = %d, should be > Content-only = %d", + resultWithToolCalls.TokenCount, resultContentOnly.TokenCount) + } + + // Message with ToolCallID + msgWithToolResult := protocoltypes.Message{ + Role: "tool", + Content: "This is a simple response with some text content.", + ToolCallID: "tc_456", + } + resultWithToolResult := providerToSeahorseMessage(msgWithToolResult) + + if resultWithToolResult.TokenCount <= resultContentOnly.TokenCount { + t.Errorf("TokenCount with ToolCallID = %d, should be > Content-only = %d", + resultWithToolResult.TokenCount, resultContentOnly.TokenCount) + } + + // Message with Media + msgWithMedia := protocoltypes.Message{ + Role: "user", + Content: "This is a simple response with some text content.", + Media: []string{"data:image/png;base64,abc123"}, + } + resultWithMedia := providerToSeahorseMessage(msgWithMedia) + + if resultWithMedia.TokenCount <= resultContentOnly.TokenCount { + t.Errorf("TokenCount with Media = %d, should be > Content-only = %d", + resultWithMedia.TokenCount, resultContentOnly.TokenCount) + } +} + +func TestSeahorseToProviderMessagesRebuildsContentFromParts(t *testing.T) { + msg := seahorse.Message{ + Role: "tool", + Content: "", + TokenCount: 50, + Parts: []seahorse.MessagePart{ + { + Type: "tool_result", + ToolCallID: "tc_999", + Text: "This is the actual tool output that should be in Content", + }, + }, + } + + result := seahorseToProviderMessages(&seahorse.AssembleResult{ + Messages: []seahorse.Message{msg}, + }) + + if len(result) != 1 { + t.Fatalf("expected 1 message, got %d", len(result)) + } + + if result[0].Content == "" { + t.Error("Content is empty - tool_result text was not rebuilt into Content") + } + if result[0].Content != "This is the actual tool output that should be in Content" { + t.Errorf("Content = %q, want tool output text from Parts", result[0].Content) + } +} + +func TestSeahorseAssembleSummaryNotInMessages(t *testing.T) { + engine, err := seahorse.NewEngine(seahorse.Config{ + DBPath: t.TempDir() + "/test.db", + }, nil) + if err != nil { + t.Fatalf("NewEngine: %v", err) + } + defer engine.Close() + + ctx := context.Background() + mgr := &seahorseContextManager{engine: engine} + sessionKey := "test-no-dup-summary" + + // Get the store to directly create a summary + store := engine.GetRetrieval().Store() + conv, err := store.GetOrCreateConversation(ctx, sessionKey) + if err != nil { + t.Fatalf("GetOrCreateConversation: %v", err) + } + + // Ingest some messages first + for i := 0; i < 10; i++ { + _ = mgr.Ingest(ctx, &IngestRequest{ + SessionKey: sessionKey, + Message: protocoltypes.Message{Role: "user", Content: fmt.Sprintf("Message %d", i)}, + }) + } + + // Create a summary + input := seahorse.CreateSummaryInput{ + ConversationID: conv.ConversationID, + Kind: seahorse.SummaryKindLeaf, + Depth: 0, + Content: "This is a test summary about the conversation", + TokenCount: 50, + } + summary, err := store.CreateSummary(ctx, input) + if err != nil { + t.Fatalf("CreateSummary: %v", err) + } + err = store.AppendContextSummary(ctx, conv.ConversationID, summary.SummaryID) + if err != nil { + t.Fatalf("AppendContextSummary: %v", err) + } + + // Assemble + resp, err := mgr.Assemble(ctx, &AssembleRequest{ + SessionKey: sessionKey, + Budget: 50000, + MaxTokens: 4096, + }) + if err != nil { + t.Fatalf("Assemble: %v", err) + } + + // Count how many times the summary content appears + summaryContent := "This is a test summary" + countInHistory := 0 + for _, msg := range resp.History { + if strings.Contains(msg.Content, summaryContent) { + countInHistory++ + } + } + + if countInHistory > 0 { + t.Errorf("Summary content appears %d times in History - should be 0", countInHistory) + } + + // Summary should appear in Summary field + if !strings.Contains(resp.Summary, summaryContent) { + t.Error("Summary content should appear in response.Summary field") + } +} diff --git a/pkg/seahorse/schema.go b/pkg/seahorse/schema.go index 056e1b23a..effa6d60d 100644 --- a/pkg/seahorse/schema.go +++ b/pkg/seahorse/schema.go @@ -7,8 +7,26 @@ import ( "github.com/sipeed/picoclaw/pkg/logger" ) +// SQL statements for FTS5 tables with trigram tokenizer. +const ( + sqlCreateSummariesFTS = `CREATE VIRTUAL TABLE IF NOT EXISTS summaries_fts USING fts5( + summary_id, + content, + tokenize="trigram" + )` + sqlCreateMessagesFTS = `CREATE VIRTUAL TABLE IF NOT EXISTS messages_fts USING fts5( + message_id, + content, + tokenize="trigram" + )` + sqlCheckFTS5Available = `CREATE VIRTUAL TABLE IF NOT EXISTS _fts5_check USING fts5(content)` + sqlCheckTrigramAvailable = `CREATE VIRTUAL TABLE IF NOT EXISTS _trigram_check USING fts5(content, tokenize="trigram")` + sqlDropFTS5Check = `DROP TABLE IF EXISTS _fts5_check` + sqlDropTrigramCheck = `DROP TABLE IF EXISTS _trigram_check` +) + // runSchema creates or upgrades the database schema. -// All migrations are idempotent (safe to run multiple times). +// All schemas are idempotent (safe to run multiple times). func runSchema(db *sql.DB) error { // Check FTS5 support before creating tables if err := checkFTS5Support(db); err != nil { @@ -86,18 +104,10 @@ func runSchema(db *sql.DB) error { )`, // FTS5 virtual table with trigram tokenizer for CJK support - `CREATE VIRTUAL TABLE IF NOT EXISTS summaries_fts USING fts5( - summary_id, - content, - tokenize="trigram" - )`, + sqlCreateSummariesFTS, // FTS5 virtual table for message search with trigram tokenizer - `CREATE VIRTUAL TABLE IF NOT EXISTS messages_fts USING fts5( - message_id, - content, - tokenize="trigram" - )`, + sqlCreateMessagesFTS, // Indexes for common query patterns `CREATE INDEX IF NOT EXISTS idx_messages_conversation ON messages(conversation_id)`, @@ -150,25 +160,25 @@ func checkFTS5Support(db *sql.DB) error { if err != nil { // sqlite_compileoption_used might not exist in older SQLite // Try a different approach: create a test FTS5 table - _, testErr := db.Exec(`CREATE TEMP TABLE IF NOT EXISTS _fts5_test USING fts5(content)`) + _, testErr := db.Exec(sqlCheckFTS5Available) if testErr != nil { return fmt.Errorf("SQLite FTS5 not available: %w (required for full-text search)", testErr) } - db.Exec(`DROP TABLE IF EXISTS _fts5_test`) + db.Exec(sqlDropFTS5Check) } else if fts5Enabled == 0 { return fmt.Errorf("SQLite was compiled without FTS5 support (required for full-text search)") } // Check if trigram tokenizer is available by trying to create a test table // Not all SQLite builds include the trigram tokenizer - _, err = db.Exec(`CREATE TEMP TABLE IF NOT EXISTS _trigram_test USING fts5(content, tokenize="trigram")`) + _, err = db.Exec(sqlCheckTrigramAvailable) if err != nil { logger.WarnCF("seahorse", "SQLite trigram tokenizer not available, CJK search may be limited", map[string]any{"error": err.Error()}) // Trigram is not strictly required, just better for CJK // Don't return error, just log warning } else { - db.Exec(`DROP TABLE IF EXISTS _trigram_test`) + db.Exec(sqlDropTrigramCheck) } return nil diff --git a/pkg/seahorse/schema_test.go b/pkg/seahorse/schema_test.go index 006a11f7f..17879f66c 100644 --- a/pkg/seahorse/schema_test.go +++ b/pkg/seahorse/schema_test.go @@ -181,3 +181,31 @@ func TestMigrationSummaryParentsPK(t *testing.T) { t.Error("expected unique constraint violation for duplicate summary_parents link") } } + +func TestFTS5SQLConstants(t *testing.T) { + db := openTestDB(t) + + // Verify FTS5 check SQL executes without error + _, err := db.Exec(sqlCheckFTS5Available) + if err != nil { + t.Errorf("sqlCheckFTS5Available failed: %v", err) + } + + // Verify trigram check SQL executes without error + _, err = db.Exec(sqlCheckTrigramAvailable) + if err != nil { + t.Errorf("sqlCheckTrigramAvailable failed: %v", err) + } + + // Verify summaries_fts SQL executes without error + _, err = db.Exec(sqlCreateSummariesFTS) + if err != nil { + t.Errorf("sqlCreateSummariesFTS failed: %v", err) + } + + // Verify messages_fts SQL executes without error + _, err = db.Exec(sqlCreateMessagesFTS) + if err != nil { + t.Errorf("sqlCreateMessagesFTS failed: %v", err) + } +} diff --git a/pkg/seahorse/short_assembler.go b/pkg/seahorse/short_assembler.go index 45a0d87ce..f0fd323ba 100644 --- a/pkg/seahorse/short_assembler.go +++ b/pkg/seahorse/short_assembler.go @@ -134,24 +134,6 @@ func (a *Assembler) Assemble(ctx context.Context, convID int64, input AssembleIn if r.summary.Kind == SummaryKindCondensed { condensedCount++ } - // Load parent IDs for XML formatting - parentSummaries, err := a.store.GetSummaryParents(ctx, r.summary.SummaryID) - if err != nil { - logger.WarnCF("seahorse", "assemble: get summary parents", map[string]any{ - "summary_id": r.summary.SummaryID, - "error": err.Error(), - }) - } - var parentIDs []string - for _, ps := range parentSummaries { - parentIDs = append(parentIDs, ps.SummaryID) - } - xmlMsg := FormatSummaryXML(r.summary, parentIDs) - messages = append(messages, Message{ - Role: "system", - Content: xmlMsg, - TokenCount: r.tokenCount, - }) } } @@ -168,12 +150,37 @@ func (a *Assembler) Assemble(ctx context.Context, convID int64, input AssembleIn } } + // Build Summary field: all XML summaries + system prompt addition + var summaryParts []string + for _, sum := range summaries { + if sum.Content == "" { + continue + } + // Load parent IDs for XML formatting + parentSummaries, err := a.store.GetSummaryParents(ctx, sum.SummaryID) + if err != nil { + logger.WarnCF("seahorse", "assemble: get summary parents", map[string]any{ + "summary_id": sum.SummaryID, + "error": err.Error(), + }) + } + var parentIDs []string + for _, ps := range parentSummaries { + parentIDs = append(parentIDs, ps.SummaryID) + } + summaryParts = append(summaryParts, FormatSummaryXML(&sum, parentIDs)) + } + summary := strings.Join(summaryParts, "\n\n") + if systemPromptAddition != "" { + if summary != "" { + summary += "\n\n" + } + summary += systemPromptAddition + } + return &AssembleResult{ - Messages: messages, - Summaries: summaries, - TokenCount: totalTokens, - SourceIDs: sourceIDs, - SystemPromptAddition: systemPromptAddition, + Messages: messages, + Summary: summary, }, nil } diff --git a/pkg/seahorse/short_assembler_test.go b/pkg/seahorse/short_assembler_test.go index 3d5ddefa8..88a05e64c 100644 --- a/pkg/seahorse/short_assembler_test.go +++ b/pkg/seahorse/short_assembler_test.go @@ -35,11 +35,8 @@ func TestAssemblerAssembleEmpty(t *testing.T) { if len(result.Messages) != 0 { t.Errorf("Messages = %d, want 0", len(result.Messages)) } - if len(result.Summaries) != 0 { - t.Errorf("Summaries = %d, want 0", len(result.Summaries)) - } - if result.TokenCount != 0 { - t.Errorf("TokenCount = %d, want 0", result.TokenCount) + if result.Summary != "" { + t.Errorf("Summary = %q, want empty", result.Summary) } } @@ -72,8 +69,9 @@ func TestAssemblerAssembleMessagesOnly(t *testing.T) { if result.Messages[1].Content != "world" { t.Errorf("Messages[1].Content = %q, want 'world'", result.Messages[1].Content) } - if result.TokenCount != 10 { - t.Errorf("TokenCount = %d, want 10", result.TokenCount) + // No summaries, so Summary should be empty + if result.Summary != "" { + t.Errorf("Summary = %q, want empty", result.Summary) } } @@ -107,15 +105,19 @@ func TestAssemblerAssembleWithSummary(t *testing.T) { t.Fatalf("Assemble: %v", err) } - if len(result.Summaries) != 1 { - t.Fatalf("Summaries = %d, want 1", len(result.Summaries)) + // Messages = 2 raw messages (summaries are in Summary field, not Messages) + if len(result.Messages) != 2 { + t.Errorf("Messages = %d, want 2 (raw messages only)", len(result.Messages)) } - if result.Summaries[0].SummaryID != summary.SummaryID { - t.Errorf("Summary ID = %q, want %q", result.Summaries[0].SummaryID, summary.SummaryID) + // Summary should contain XML with summary content + if result.Summary == "" { + t.Error("Summary should not be empty when summary exists") } - // Messages = 1 summary-as-user-msg + 2 raw messages = 3 - if len(result.Messages) != 3 { - t.Errorf("Messages = %d, want 3 (1 summary + 2 raw)", len(result.Messages)) + if !strings.Contains(result.Summary, summary.Content) { + t.Errorf("Summary should contain summary content %q", summary.Content) + } + if !strings.Contains(result.Summary, "") { - t.Errorf("BUG: unescaped < in summary content: %q", xmlContent) + if strings.Contains(result.Summary, "") { + t.Errorf("BUG: unescaped < in summary content: %q", result.Summary) } - if strings.Contains(xmlContent, `"hello"`) { - t.Errorf("BUG: unescaped \" in summary content: %q", xmlContent) + if strings.Contains(result.Summary, `"hello"`) { + t.Errorf("BUG: unescaped \" in summary content: %q", result.Summary) } // & should be escaped as & - if strings.Contains(xmlContent, " & ") { - t.Errorf("BUG: unescaped & in summary content: %q", xmlContent) - } -} - -func TestAssemblerSourceIDs(t *testing.T) { - s, convID := setupAssemblerStore(t) - ctx := context.Background() - - // Create messages - msg1, _ := s.AddMessage(ctx, convID, "user", "hello", 5) - msg2, _ := s.AddMessage(ctx, convID, "assistant", "world", 5) - - s.UpsertContextItems(ctx, convID, []ContextItem{ - {Ordinal: 100, ItemType: "message", MessageID: msg1.ID, TokenCount: 5}, - {Ordinal: 200, ItemType: "message", MessageID: msg2.ID, TokenCount: 5}, - }) - - a := &Assembler{store: s, config: Config{}} - result, err := a.Assemble(ctx, convID, AssembleInput{Budget: 100}) - if err != nil { - t.Fatalf("Assemble: %v", err) - } - - // SourceIDs should contain message IDs - if len(result.SourceIDs) != 2 { - t.Errorf("SourceIDs = %d, want 2", len(result.SourceIDs)) + if strings.Contains(result.Summary, " & ") { + t.Errorf("BUG: unescaped & in summary content: %q", result.Summary) } } @@ -340,11 +314,11 @@ func TestAssemblerSummaryXMLWithParents(t *testing.T) { t.Fatalf("Assemble: %v", err) } - // Find the summary message (first message, formatted as XML) - if len(result.Messages) < 1 { - t.Fatal("expected at least 1 message") + // Summary field should contain XML with parent information + if result.Summary == "" { + t.Fatal("Summary should not be empty") } - xmlContent := result.Messages[0].Content + xmlContent := result.Summary // Should contain section with parent ID if !contains(xmlContent, "") { @@ -388,10 +362,10 @@ func TestAssemblerSummaryXMLIncludesDescendantCount(t *testing.T) { t.Fatalf("Assemble: %v", err) } - if len(result.Messages) < 1 { - t.Fatal("expected at least 1 message") + if result.Summary == "" { + t.Fatal("Summary should not be empty") } - xmlContent := result.Messages[0].Content + xmlContent := result.Summary // Should contain descendant_count="8" if !contains(xmlContent, `descendant_count="8"`) { @@ -425,7 +399,10 @@ func TestAssemblerLeafSummaryNoParents(t *testing.T) { t.Fatalf("Assemble: %v", err) } - xmlContent := result.Messages[0].Content + if result.Summary == "" { + t.Fatal("Summary should not be empty") + } + xmlContent := result.Summary // Leaf summary should NOT have section if contains(xmlContent, "") { @@ -472,9 +449,13 @@ func TestAssemblerDepthAwarePrompt(t *testing.T) { t.Fatalf("Assemble: %v", err) } - // Should have a depth-aware prompt - if result.SystemPromptAddition == "" { - t.Error("expected non-empty SystemPromptAddition when depth >= 2") + // Should have a depth-aware prompt in Summary field + if result.Summary == "" { + t.Error("expected non-empty Summary when depth >= 2") + } + // SystemPromptAddition is embedded in Summary field + if !strings.Contains(result.Summary, "multi-level summarization") { + t.Error("Summary should contain system prompt addition about multi-level summarization") } } diff --git a/pkg/seahorse/short_compaction.go b/pkg/seahorse/short_compaction.go index a24bd4db9..97a0795bb 100644 --- a/pkg/seahorse/short_compaction.go +++ b/pkg/seahorse/short_compaction.go @@ -643,6 +643,12 @@ func (e *CompactionEngine) generateCondensedSummary(ctx context.Context, summari func (e *CompactionEngine) runCondensedLoop(ctx context.Context, convID int64) { var prevTokens int for { + select { + case <-ctx.Done(): + return + default: + } + tokensBefore, err := e.store.GetContextTokenCount(ctx, convID) if err != nil { logger.ErrorCF("seahorse", "condensed: get tokens", map[string]any{"error": err.Error()}) diff --git a/pkg/seahorse/short_compaction_test.go b/pkg/seahorse/short_compaction_test.go index 1c32af183..ea7dcb52d 100644 --- a/pkg/seahorse/short_compaction_test.go +++ b/pkg/seahorse/short_compaction_test.go @@ -28,7 +28,11 @@ func waitForCondensed(ce *CompactionEngine, convID int64, timeout time.Duration) func newTestCompactionEngine(t *testing.T) (*CompactionEngine, *Store, int64) { t.Helper() - s := openTestStore(t) + db := openTestDB(t) + if err := runSchema(db); err != nil { + t.Fatalf("migration: %v", err) + } + s := &Store{db: db} ctx := context.Background() conv, _ := s.GetOrCreateConversation(ctx, "test:compact") shutdownCtx, shutdownCancel := context.WithCancel(context.Background()) @@ -40,7 +44,8 @@ func newTestCompactionEngine(t *testing.T) (*CompactionEngine, *Store, int64) { shutdownCancel: shutdownCancel, } convID := conv.ConversationID - // Ensure async goroutines are stopped before database is closed + // Ensure async goroutines are stopped before database is closed. + // Register cleanup here (after openTestDB) so it runs BEFORE openTestDB's db.Close(). t.Cleanup(func() { shutdownCancel() // Wait for async condensed goroutine to finish (poll condensing map) @@ -852,7 +857,7 @@ func TestCompactAsyncDedup(t *testing.T) { ce, cancel := newTestCompactionEngineWithStore(s, slowComplete) t.Cleanup(func() { cancel() - time.Sleep(100 * time.Millisecond) + waitForCondensed(ce, convID, 2*time.Second) }) // Create conditions for condensed compaction diff --git a/pkg/seahorse/short_engine.go b/pkg/seahorse/short_engine.go index 70bd85ab0..11cbe01d7 100644 --- a/pkg/seahorse/short_engine.go +++ b/pkg/seahorse/short_engine.go @@ -46,11 +46,8 @@ type AssembleInput struct { // AssembleResult contains assembled context. type AssembleResult struct { - Messages []Message `json:"messages"` - Summaries []Summary `json:"summaries"` - TokenCount int `json:"tokenCount"` - SourceIDs []string `json:"sourceIds"` - SystemPromptAddition string `json:"systemPromptAddition,omitempty"` + Messages []Message `json:"messages"` + Summary string `json:"summary"` // formatted XML summaries + system prompt addition } const numSessionShards = 256 diff --git a/pkg/seahorse/store.go b/pkg/seahorse/store.go index e09d8cd41..8077f4c75 100644 --- a/pkg/seahorse/store.go +++ b/pkg/seahorse/store.go @@ -1131,13 +1131,13 @@ func (s *Store) scanSearchResults(rows *sql.Rows, withRank bool) ([]SearchResult var kind string if withRank { // FTS5 mode: no TotalCount in query (set by caller after COUNT) - if err := rows.Scan(&r.SummaryID, &r.ConversationID, &kind, &r.Snippet, &createdAt, &r.Rank); err != nil { + if err := rows.Scan(&r.SummaryID, &r.ConversationID, &kind, &r.Content, &createdAt, &r.Rank); err != nil { return nil, err } } else { // LIKE mode: TotalCount from window function if err := rows.Scan(&r.SummaryID, &r.ConversationID, &kind, - &r.Snippet, &createdAt, &r.TotalCount); err != nil { + &r.Content, &createdAt, &r.TotalCount); err != nil { return nil, err } } diff --git a/pkg/seahorse/store_test.go b/pkg/seahorse/store_test.go index d056819f6..22790ebdc 100644 --- a/pkg/seahorse/store_test.go +++ b/pkg/seahorse/store_test.go @@ -1106,3 +1106,38 @@ func TestSearchMessagesWithTimeFilter(t *testing.T) { t.Errorf("Since=1h-future: expected 0 results, got %d", len(results)) } } + +func TestStoreSearchSummariesReturnsContent(t *testing.T) { + s := openTestStore(t) + ctx := context.Background() + + conv, _ := s.GetOrCreateConversation(ctx, "agent:test") + + // Create a summary with known content + s.CreateSummary(ctx, CreateSummaryInput{ + ConversationID: conv.ConversationID, + Kind: SummaryKindLeaf, + Depth: 0, + Content: "This is the summary content for testing", + TokenCount: 10, + }) + + // Search should return the full content, not empty + results, err := s.SearchSummaries(ctx, SearchInput{ + Pattern: "summary content", + Mode: "like", + ConversationID: conv.ConversationID, + }) + if err != nil { + t.Fatalf("SearchSummaries: %v", err) + } + if len(results) != 1 { + t.Fatalf("expected 1 result, got %d", len(results)) + } + if results[0].Content == "" { + t.Error("SearchResult.Content is empty, want full summary content") + } + if results[0].Content != "This is the summary content for testing" { + t.Errorf("SearchResult.Content = %q, want %q", results[0].Content, "This is the summary content for testing") + } +}