fix(seahorse): pass Budget to Compact for correct condensed threshold
Issue #4 from PR review: When Budget was not passed to seahorse.Compact, it defaulted to `tokensBefore * 0.75`, making `tokensBefore > budget` always true and causing condensed compaction to trigger unnecessarily. Changes: - context_seahorse.go: Forward Budget from CompactRequest to CompactInput - loop.go: Pass Budget (ContextWindow) in all 3 Compact calls - Add test verifying condensed is skipped when tokens < threshold - Fix lint issues in store.go and store_test.go
This commit is contained in:
parent
615c617a61
commit
a2f72f45e6
7 changed files with 141 additions and 7 deletions
|
|
@ -134,7 +134,8 @@ func (m *seahorseContextManager) Compact(ctx context.Context, req *CompactReques
|
|||
}
|
||||
|
||||
_, err := m.engine.Compact(ctx, req.SessionKey, seahorse.CompactInput{
|
||||
Force: req.Reason == ContextCompressReasonRetry,
|
||||
Force: req.Reason == ContextCompressReasonRetry,
|
||||
Budget: &req.Budget,
|
||||
})
|
||||
return err
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import (
|
|||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/sipeed/picoclaw/pkg/bus"
|
||||
"github.com/sipeed/picoclaw/pkg/config"
|
||||
|
|
@ -970,3 +971,116 @@ func TestSeahorseSteeringMessageIngested(t *testing.T) {
|
|||
t.Error("STEERING MESSAGE NOT IN SEAHORSE DB: steering message should be ingested into SQLite")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeahorseSummarizeSkipsCondensedWhenBelowThreshold verifies that when
|
||||
// Summarize is triggered but tokens are below ContextWindow threshold,
|
||||
// condensed compaction should NOT run.
|
||||
func TestSeahorseSummarizeSkipsCondensedWhenBelowThreshold(t *testing.T) {
|
||||
contextWindow := 1000
|
||||
cfg := &config.Config{
|
||||
Agents: config.AgentsConfig{
|
||||
Defaults: config.AgentDefaults{
|
||||
Workspace: t.TempDir(),
|
||||
ModelName: "test-model",
|
||||
MaxTokens: 4096,
|
||||
MaxToolIterations: 10,
|
||||
ContextManager: "seahorse",
|
||||
ContextWindow: contextWindow,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
msgBus := bus.NewMessageBus()
|
||||
provider := &seahorseTestProvider{}
|
||||
al := NewAgentLoop(cfg, msgBus, provider)
|
||||
defaultAgent := al.registry.GetDefaultAgent()
|
||||
if defaultAgent == nil {
|
||||
t.Fatal("expected default agent")
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
sessionKey := "test-summarize-skip-condensed"
|
||||
|
||||
seahorseCM, ok := al.contextManager.(*seahorseContextManager)
|
||||
if !ok {
|
||||
t.Fatal("expected seahorseContextManager")
|
||||
}
|
||||
store := seahorseCM.engine.GetRetrieval().Store()
|
||||
|
||||
conv, err := store.GetOrCreateConversation(ctx, sessionKey)
|
||||
if err != nil {
|
||||
t.Fatalf("GetOrCreateConversation: %v", err)
|
||||
}
|
||||
|
||||
// Insert leaf summaries directly (bypass leaf compaction requirement)
|
||||
for i := 0; i < seahorse.CondensedMinFanout; i++ {
|
||||
now := time.Now().UTC()
|
||||
summary, sumErr := store.CreateSummary(ctx, seahorse.CreateSummaryInput{
|
||||
ConversationID: conv.ConversationID,
|
||||
Kind: seahorse.SummaryKindLeaf,
|
||||
Depth: 0,
|
||||
Content: fmt.Sprintf("leaf summary %d", i),
|
||||
TokenCount: 50,
|
||||
EarliestAt: &now,
|
||||
LatestAt: &now,
|
||||
})
|
||||
if sumErr != nil {
|
||||
t.Fatalf("CreateSummary %d: %v", i, sumErr)
|
||||
}
|
||||
if appendErr := store.AppendContextSummary(ctx, conv.ConversationID, summary.SummaryID); appendErr != nil {
|
||||
t.Fatalf("AppendContextSummary %d: %v", i, appendErr)
|
||||
}
|
||||
}
|
||||
|
||||
// Add fresh messages (required for condensation candidates)
|
||||
for i := 0; i < seahorse.FreshTailCount+1; i++ {
|
||||
m, msgErr := store.AddMessage(ctx, conv.ConversationID, "user", "fresh", 5)
|
||||
if msgErr != nil {
|
||||
t.Fatalf("AddMessage %d: %v", i, msgErr)
|
||||
}
|
||||
if appendErr := store.AppendContextMessage(ctx, conv.ConversationID, m.ID); appendErr != nil {
|
||||
t.Fatalf("AppendContextMessage %d: %v", i, appendErr)
|
||||
}
|
||||
}
|
||||
|
||||
tokensBefore, err := store.GetContextTokenCount(ctx, conv.ConversationID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetContextTokenCount: %v", err)
|
||||
}
|
||||
threshold := int(float64(contextWindow) * seahorse.ContextThreshold)
|
||||
t.Logf("Tokens before: %d, threshold: %d", tokensBefore, threshold)
|
||||
|
||||
// Trigger Summarize
|
||||
_, err = al.runAgentLoop(ctx, defaultAgent, processOptions{
|
||||
SessionKey: sessionKey,
|
||||
Channel: "cli",
|
||||
ChatID: "direct",
|
||||
UserMessage: "trigger",
|
||||
DefaultResponse: defaultResponse,
|
||||
EnableSummary: true,
|
||||
SendResponse: false,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("runAgentLoop: %v", err)
|
||||
}
|
||||
|
||||
time.Sleep(500 * time.Millisecond)
|
||||
|
||||
summaries, err := store.GetSummariesByConversation(ctx, conv.ConversationID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetSummariesByConversation: %v", err)
|
||||
}
|
||||
|
||||
condensedCount := 0
|
||||
for _, sum := range summaries {
|
||||
if sum.Kind == seahorse.SummaryKindCondensed {
|
||||
condensedCount++
|
||||
}
|
||||
}
|
||||
|
||||
t.Logf("Condensed summaries: %d", condensedCount)
|
||||
|
||||
if tokensBefore < threshold && condensedCount > 0 {
|
||||
t.Errorf("BUG: condensed created when tokens (%d) < threshold (%d)", tokensBefore, threshold)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1742,6 +1742,7 @@ func (al *AgentLoop) runTurn(ctx context.Context, ts *turnState) (turnResult, er
|
|||
if err := al.contextManager.Compact(turnCtx, &CompactRequest{
|
||||
SessionKey: ts.sessionKey,
|
||||
Reason: ContextCompressReasonProactive,
|
||||
Budget: ts.agent.ContextWindow,
|
||||
}); err != nil {
|
||||
logger.WarnCF("agent", "Proactive compact failed", map[string]any{
|
||||
"session_key": ts.sessionKey,
|
||||
|
|
@ -2775,7 +2776,7 @@ turnLoop:
|
|||
}
|
||||
}
|
||||
if ts.opts.EnableSummary {
|
||||
al.contextManager.Compact(turnCtx, &CompactRequest{SessionKey: ts.sessionKey, Reason: ContextCompressReasonSummarize})
|
||||
al.contextManager.Compact(turnCtx, &CompactRequest{SessionKey: ts.sessionKey, Reason: ContextCompressReasonSummarize, Budget: ts.agent.ContextWindow})
|
||||
}
|
||||
|
||||
ts.setPhase(TurnPhaseCompleted)
|
||||
|
|
@ -2851,6 +2852,7 @@ turnLoop:
|
|||
&CompactRequest{
|
||||
SessionKey: ts.sessionKey,
|
||||
Reason: ContextCompressReasonSummarize,
|
||||
Budget: ts.agent.ContextWindow,
|
||||
},
|
||||
)
|
||||
}
|
||||
|
|
|
|||
7
pkg/seahorse/.omc/state/last-tool-error.json
Normal file
7
pkg/seahorse/.omc/state/last-tool-error.json
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
{
|
||||
"tool_name": "Bash",
|
||||
"tool_input_preview": "{\"command\":\"cd /home/yliu/repos/picoclaw && make lint 2>&1\",\"timeout\":120000}",
|
||||
"error": "Exit code 2\npkg/agent/context_seahorse_test.go:1027:1: File is not properly formatted (gci)\n\t\t\tEarliestAt: &now,\n^\n1 issues:\n* gci: 1\nmake: *** [Makefile:264: lint] Error 1",
|
||||
"timestamp": "2026-04-04T02:38:32.067Z",
|
||||
"retry_count": 6
|
||||
}
|
||||
|
|
@ -64,6 +64,11 @@ func (e *CompactionEngine) Compact(ctx context.Context, convID int64, input Comp
|
|||
var budget int
|
||||
if input.Budget != nil {
|
||||
budget = *input.Budget
|
||||
if budget == 0 {
|
||||
logger.ErrorCF("seahorse", "Compact: budget is 0, this should not happen", map[string]any{
|
||||
"conv_id": convID,
|
||||
})
|
||||
}
|
||||
} else {
|
||||
budget = int(float64(tokensBefore) * ContextThreshold)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -854,17 +854,19 @@ func (s *Store) ReplaceContextItemsWithSummary(
|
|||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
var ordinals []int
|
||||
for rows.Next() {
|
||||
var ord int
|
||||
if err := rows.Scan(&ord); err != nil {
|
||||
rows.Close()
|
||||
return err
|
||||
if scanErr := rows.Scan(&ord); scanErr != nil {
|
||||
return scanErr
|
||||
}
|
||||
ordinals = append(ordinals, ord)
|
||||
}
|
||||
rows.Close()
|
||||
if err = rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if len(ordinals) == 0 {
|
||||
return nil
|
||||
|
|
|
|||
|
|
@ -1189,7 +1189,10 @@ func TestStoreReplaceContextItemsWithSummary(t *testing.T) {
|
|||
|
||||
// Replace summaries 0 and 1 (not 2) using per-item deletion
|
||||
// This should NOT delete the message at ordinal 200
|
||||
err := s.ReplaceContextItemsWithSummary(ctx, conv.ConversationID, []string{summaries[0], summaries[1]}, newSummary.SummaryID)
|
||||
err := s.ReplaceContextItemsWithSummary(
|
||||
ctx, conv.ConversationID,
|
||||
[]string{summaries[0], summaries[1]},
|
||||
newSummary.SummaryID)
|
||||
if err != nil {
|
||||
t.Fatalf("ReplaceContextItemsWithSummary: %v", err)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue