From 515b61525acee0c9680c23cfa150c8d2e9b69f19 Mon Sep 17 00:00:00 2001 From: FyZhu97 Date: Sun, 22 Mar 2026 16:16:35 +0800 Subject: [PATCH] fix: correct forceCompression session history handling --- pkg/agent/loop.go | 59 +++++++++++++++++++++------------------- pkg/agent/loop_test.go | 61 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 86 insertions(+), 34 deletions(-) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index ed5c73afc..b0148a3eb 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -1565,46 +1565,49 @@ func (al *AgentLoop) maybeSummarize(agent *AgentInstance, sessionKey, channel, c } // forceCompression aggressively reduces context when the limit is hit. -// It drops the oldest 50% of messages (keeping system prompt and last user message). +// Session history only stores conversation messages; system prompt is rebuilt +// per request by ContextBuilder and must never be persisted into history. func (al *AgentLoop) forceCompression(agent *AgentInstance, sessionKey string) { history := agent.Sessions.GetHistory(sessionKey) if len(history) <= 4 { return } - // Keep system prompt (usually [0]) and the very last message (user's trigger) - // We want to drop the oldest half of the *conversation* - // Assuming [0] is system, [1:] is conversation - conversation := history[1 : len(history)-1] - if len(conversation) == 0 { + // Drop the oldest half of the actual conversation while preserving the most + // recent context window. If historical test data injected system messages + // directly into the session, strip them here rather than treating them as + // special persisted state. + cleanedHistory := make([]providers.Message, 0, len(history)) + for _, msg := range history { + if msg.Role == "system" { + continue + } + cleanedHistory = append(cleanedHistory, msg) + } + if len(cleanedHistory) <= 4 { + if len(cleanedHistory) != len(history) { + agent.Sessions.SetHistory(sessionKey, cleanedHistory) + agent.Sessions.Save(sessionKey) + } return } - // Helper to find the mid-point of the conversation - mid := len(conversation) / 2 + dropCount := len(cleanedHistory) / 2 + if dropCount == 0 { + return + } - // New history structure: - // 1. System Prompt (with compression note appended) - // 2. Second half of conversation - // 3. Last message + newHistory := append([]providers.Message(nil), cleanedHistory[dropCount:]...) - droppedCount := mid - keptConversation := conversation[mid:] - - newHistory := make([]providers.Message, 0, 1+len(keptConversation)+1) - - // Append compression note to the original system prompt instead of adding a new system message - // This avoids having two consecutive system messages which some APIs (like Zhipu) reject compressionNote := fmt.Sprintf( - "\n\n[System Note: Emergency compression dropped %d oldest messages due to context limit]", - droppedCount, + "[Compression note: dropped %d oldest conversation messages due to context limit.]", + dropCount, ) - enhancedSystemPrompt := history[0] - enhancedSystemPrompt.Content = enhancedSystemPrompt.Content + compressionNote - newHistory = append(newHistory, enhancedSystemPrompt) - - newHistory = append(newHistory, keptConversation...) - newHistory = append(newHistory, history[len(history)-1]) // Last message + if summary := strings.TrimSpace(agent.Sessions.GetSummary(sessionKey)); summary != "" { + agent.Sessions.SetSummary(sessionKey, summary+"\n"+compressionNote) + } else { + agent.Sessions.SetSummary(sessionKey, compressionNote) + } // Update session agent.Sessions.SetHistory(sessionKey, newHistory) @@ -1612,7 +1615,7 @@ func (al *AgentLoop) forceCompression(agent *AgentInstance, sessionKey string) { logger.WarnCF("agent", "Forced compression executed", map[string]any{ "session_key": sessionKey, - "dropped_msgs": droppedCount, + "dropped_msgs": dropCount, "new_count": len(newHistory), }) } diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index 28eab03db..2d4176e55 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -1080,9 +1080,9 @@ func TestAgentLoop_ContextExhaustionRetry(t *testing.T) { // Inject some history to simulate a full context sessionKey := "test-session-context" - // Create dummy history + // Create realistic persisted history: session storage contains conversation + // turns only, not the synthetic system prompt built at request time. history := []providers.Message{ - {Role: "system", Content: "System prompt"}, {Role: "user", Content: "Old message 1"}, {Role: "assistant", Content: "Old response 1"}, {Role: "user", Content: "Old message 2"}, @@ -1119,13 +1119,62 @@ func TestAgentLoop_ContextExhaustionRetry(t *testing.T) { // Check final history length finalHistory := defaultAgent.Sessions.GetHistory(sessionKey) + if len(finalHistory) == 0 { + t.Fatal("expected compressed history to be preserved") + } + for _, msg := range finalHistory { + if msg.Role == "system" { + t.Fatalf("persisted history must not contain system messages: %+v", msg) + } + } + if strings.Contains(finalHistory[0].Content, "[System Note:") { + t.Fatalf("first persisted message was incorrectly rewritten as system note: %q", finalHistory[0].Content) + } // We verify that the history has been modified (compressed) - // Original length: 6 + // Original length: 5 // Expected behavior: compression drops ~50% of history (mid slice) // We can assert that the length is NOT what it would be without compression. - // Without compression: 6 + 1 (new user msg) + 1 (assistant msg) = 8 - if len(finalHistory) >= 8 { - t.Errorf("Expected history to be compressed (len < 8), got %d", len(finalHistory)) + // Without compression: 5 + 1 (new user msg) + 1 (assistant msg) = 7 + if len(finalHistory) >= 7 { + t.Errorf("Expected history to be compressed (len < 7), got %d", len(finalHistory)) + } +} + +func TestAgentLoop_ForceCompression_DoesNotTreatFirstMessageAsSystem(t *testing.T) { + al, _, _, _, cleanup := newTestAgentLoop(t) + defer cleanup() + + defaultAgent := al.registry.GetDefaultAgent() + if defaultAgent == nil { + t.Fatal("No default agent found") + } + + sessionKey := "force-compression-no-system" + defaultAgent.Sessions.SetHistory(sessionKey, []providers.Message{ + {Role: "user", Content: "Old message 1"}, + {Role: "assistant", Content: "Old response 1"}, + {Role: "user", Content: "Old message 2"}, + {Role: "assistant", Content: "Old response 2"}, + {Role: "user", Content: "Trigger message"}, + }) + + al.forceCompression(defaultAgent, sessionKey) + + history := defaultAgent.Sessions.GetHistory(sessionKey) + if len(history) != 3 { + t.Fatalf("compressed history len = %d, want 3", len(history)) + } + assertRoles(t, history, "user", "assistant", "user") + if history[0].Content != "Old message 2" { + t.Fatalf("compressed history[0] = %q, want %q", history[0].Content, "Old message 2") + } + if strings.Contains(history[0].Content, "Compression note") { + t.Fatalf("compression note must not be injected into persisted conversation message: %q", history[0].Content) + } + + summary := defaultAgent.Sessions.GetSummary(sessionKey) + if !strings.Contains(summary, "dropped 2 oldest conversation messages") { + t.Fatalf("summary missing compression note: %q", summary) } }