diff --git a/CHANGELOG.md b/CHANGELOG.md index 253cf285..3a4e1f4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed +- Compaction preserves the previous summary when the history bucket is empty, + including mid-turn cuts that only summarize the current turn prefix. Inherited + file-operation lists are refreshed once instead of accumulating duplicate blocks. - Picker hints (`/` commands, `@` files, `?` shortcuts) now start in one shared column instead of trailing each label's own length. diff --git a/internal/session/compaction/compact.go b/internal/session/compaction/compact.go index 05259d64..b071c1f2 100644 --- a/internal/session/compaction/compact.go +++ b/internal/session/compaction/compact.go @@ -15,12 +15,13 @@ import ( // summarize and the file operations captured from the summarized history. // Recent messages stay in the session tree via FirstKeptEntryId. type CompactionPreparation struct { - FirstKeptEntryId string - MessagesToSummarize []llm.Message - TurnPrefixMessages []llm.Message - IsMidTurnCut bool - TokensBefore int - PreviousSummary string + FirstKeptEntryId string + MessagesToSummarize []llm.Message + TurnPrefixMessages []llm.Message + IsMidTurnCut bool + TokensBefore int + PreviousSummary string + PreviousFileOperations string // ReserveTokens is the headroom compaction keeps from the context window; // summaries are capped at a fraction of it. ReserveTokens int @@ -98,10 +99,15 @@ func PrepareCompact( } previousSummary := "" + previousFileOperations := "" var previousPreserveData map[string]any if preCompactionIndex >= 0 { prevCompaction := pathEntries[preCompactionIndex].(session.CompactionEntry) previousSummary = prevCompaction.Compaction.Summary + previousFileOperations = formatFileOperations( + prevCompaction.Compaction.Details.ReadFiles, + prevCompaction.Compaction.Details.ModifiedFiles, + ) previousPreserveData = prevCompaction.Compaction.PreserveData } @@ -121,15 +127,16 @@ func PrepareCompact( tokenBefore := lastUsage.ContextTokens() return &CompactionPreparation{ - FirstKeptEntryId: firstKeptEntryID, - MessagesToSummarize: messagesToSummarize, - TurnPrefixMessages: turnPrefixMessages, - TokensBefore: tokenBefore, - PreviousSummary: previousSummary, - PreviousPreserveData: previousPreserveData, - FileOps: *fileOps, - IsMidTurnCut: cutPoint.isMidTurnCut, - ReserveTokens: settings.reverseTokens, + FirstKeptEntryId: firstKeptEntryID, + MessagesToSummarize: messagesToSummarize, + TurnPrefixMessages: turnPrefixMessages, + TokensBefore: tokenBefore, + PreviousSummary: previousSummary, + PreviousFileOperations: previousFileOperations, + PreviousPreserveData: previousPreserveData, + FileOps: *fileOps, + IsMidTurnCut: cutPoint.isMidTurnCut, + ReserveTokens: settings.reverseTokens, }, nil } @@ -189,17 +196,7 @@ func summarizeMidTurnCut( go func() { defer wg.Done() - if len(preparation.MessagesToSummarize) == 0 { - historySummary = "No prior history." - return - } - historySummary, historySummaryErr = generateSummary( - ctx, - llm, - preparation.MessagesToSummarize, - preparation.PreviousSummary, - summarizationCap(preparation.ReserveTokens, historySummaryRatio), - ) + historySummary, historySummaryErr = summarizeHistory(ctx, preparation, llm) }() go func() { @@ -231,6 +228,10 @@ func summarizeHistory( llm llm.Compactor, ) (string, error) { if len(preparation.MessagesToSummarize) == 0 { + // An empty history bucket can still have context from an earlier compaction. + if preparation.PreviousSummary != "" { + return stripFileOperations(preparation.PreviousSummary, preparation.PreviousFileOperations), nil + } return "No prior history.", nil } return generateSummary( diff --git a/internal/session/compaction/compact_test.go b/internal/session/compaction/compact_test.go index d4f2585c..e9aa54a3 100644 --- a/internal/session/compaction/compact_test.go +++ b/internal/session/compaction/compact_test.go @@ -200,6 +200,66 @@ func TestCompact_TruncatedSummary_ReturnsError(t *testing.T) { assert.Equal(t, []int{13107}, c.maxTokens, "0.8 * reserveTokens") } +func TestCompact_EmptyHistoryRefreshesFileOperations(t *testing.T) { + for _, midTurn := range []bool{false, true} { + name := "history" + if midTurn { + name = "mid-turn" + } + t.Run(name, func(t *testing.T) { + previous := session.Compaction{ + Summary: "Keep the public API unchanged.\n\n\na.go\nnotes.md\n" + + "\n\n\nold.go\n", + Details: session.CompactionDetails{ + ReadFiles: []string{"a.go", "notes.md"}, + ModifiedFiles: []string{"old.go"}, + }, + } + c := &captureCompactor{text: "Current turn context."} + for range 3 { + messages := []llm.Message{{Role: llm.RoleAssistant, ToolCalls: []llm.ToolCall{ + {Function: llm.Function{Name: "edit", Arguments: `{"path":"a.go"}`}}, + {Function: llm.Function{Name: "edit", Arguments: `{"path":"b.go"}`}}, + }}} + fileOps := extractFileOperations(messages, []session.MessageEntry{ + session.CompactionEntry{Compaction: previous}, + }, 0) + prep := CompactionPreparation{ + PreviousSummary: previous.Summary, + PreviousFileOperations: formatFileOperations( + previous.Details.ReadFiles, + previous.Details.ModifiedFiles, + ), + FileOps: *fileOps, + } + if midTurn { + prep.IsMidTurnCut = true + prep.TurnPrefixMessages = messages + } + + comp, err := Compact(t.Context(), prep, c) + + require.NoError(t, err) + assert.Contains(t, comp.Summary, "Keep the public API unchanged.") + assert.Equal(t, 1, strings.Count(comp.Summary, "")) + assert.Equal(t, 1, strings.Count(comp.Summary, "")) + assert.True(t, strings.HasSuffix( + comp.Summary, + "\n\n\nnotes.md\n\n\n\na.go\nb.go\nold.go\n", + )) + assert.Equal(t, []string{"notes.md"}, comp.Details.ReadFiles) + assert.Equal(t, []string{"a.go", "b.go", "old.go"}, comp.Details.ModifiedFiles) + previous = comp + } + wantCalls := 0 + if midTurn { + wantCalls = 3 + } + assert.Len(t, c.prompts, wantCalls) + }) + } +} + // A mid-turn cut drops the turn prefix as well as the history, and both buckets // are summarized away. Every file touched in either bucket has to reach the // file-operation lists: missing the prefix listed the edited files of the cut diff --git a/internal/session/compaction/fileops.go b/internal/session/compaction/fileops.go index e4103551..4863a7ec 100644 --- a/internal/session/compaction/fileops.go +++ b/internal/session/compaction/fileops.go @@ -96,6 +96,15 @@ func formatFileOperations(readFiles, modifiedFiles []string) string { return "\n\n" + strings.Join(sections, "\n\n") } +// Match the suffix reconstructed from the previous Details so similar text +// in older summaries is not mistaken for an appended file-operation block. +func stripFileOperations(summary, previousFileOperations string) string { + if previousFileOperations != "" && strings.HasSuffix(summary, previousFileOperations) { + return strings.TrimSuffix(summary, previousFileOperations) + } + return summary +} + func computeFileLists(fileOps *FileOperation) ([]string, []string) { modifiedSet := make(map[string]struct{}) readFiles := []string{} diff --git a/internal/session/compaction/fileops_test.go b/internal/session/compaction/fileops_test.go index dbb4b912..6a3a80d4 100644 --- a/internal/session/compaction/fileops_test.go +++ b/internal/session/compaction/fileops_test.go @@ -255,3 +255,33 @@ func TestComputeFileLists(t *testing.T) { assert.Empty(t, modifiedFiles) }) } + +func TestStripFileOperations(t *testing.T) { + const readBlock = "\n\n\na.go\n" + const modifiedBlock = "\n\n\nb.go\n" + for _, tt := range []struct { + name string + summary string + previousFileOperations string + want string + }{ + {"no files", "summary\n", "", "summary\n"}, + {"read only", "summary" + readBlock, readBlock, "summary"}, + {"modified only", "summary" + modifiedBlock, modifiedBlock, "summary"}, + {"both", "summary" + readBlock + modifiedBlock, readBlock + modifiedBlock, "summary"}, + { + "embedded example", "example" + readBlock + "\nmore context" + modifiedBlock, modifiedBlock, + "example" + readBlock + "\nmore context", + }, + {"unknown suffix", "summary" + readBlock, modifiedBlock, "summary" + readBlock}, + {"no details", "summary" + readBlock, "", "summary" + readBlock}, + { + "incomplete block", "summary\n\n\na.go", readBlock, + "summary\n\n\na.go", + }, + } { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, stripFileOperations(tt.summary, tt.previousFileOperations)) + }) + } +} diff --git a/internal/session/compaction/reload_test.go b/internal/session/compaction/reload_test.go index f2023a59..fa8c8051 100644 --- a/internal/session/compaction/reload_test.go +++ b/internal/session/compaction/reload_test.go @@ -78,3 +78,49 @@ func TestFindCutIndex_ReloadedEntriesRespectTokenBudget(t *testing.T) { cutIndex := findCutIndex(entries, 0, len(entries), 25, cutPoints) assert.NotEqual(t, cutPoints[0], cutIndex) } + +func TestCompact_EmptyHistoryPreservesSummaryAfterReload(t *testing.T) { + dir := t.TempDir() + m, err := session.NewSessionManager(dir, session.WithSessionDir(dir), session.WithShouldFlush(true)) + require.NoError(t, err) + const previousSummary = "Keep the public API unchanged." + const fileOperations = "\n\n\nnotes.md\n\n\n\na.go\n" + _, err = m.AppendCompaction(session.Compaction{ + Summary: previousSummary + fileOperations, + Details: session.CompactionDetails{ReadFiles: []string{"notes.md"}, ModifiedFiles: []string{"a.go"}}, + }) + require.NoError(t, err) + _, err = m.Append(llm.Message{Role: llm.RoleUser, Content: "fix the parser"}) + require.NoError(t, err) + keptID, err := m.Append(llm.Message{Role: llm.RoleAssistant, Content: sizedContent(30)}) + require.NoError(t, err) + m, err = session.OpenSession(m.File()) + require.NoError(t, err) + + prep, err := PrepareCompact(m.BuildContext(), Settings{keepRecentTokens: 20}) + require.NoError(t, err) + require.True(t, prep.IsMidTurnCut) + require.Empty(t, prep.MessagesToSummarize) + require.NotEmpty(t, prep.TurnPrefixMessages) + require.Equal(t, keptID, prep.FirstKeptEntryId) + + c := &captureCompactor{text: "Current turn context."} + comp, err := Compact(t.Context(), *prep, c) + require.NoError(t, err) + want := previousSummary + "\n\n---\n\n**Turn Context (mid-turn cut):**\n\nCurrent turn context." + fileOperations + assert.Equal(t, want, comp.Summary) + assert.Len(t, c.prompts, 1, "only the turn-prefix summary needs a model request") + _, err = m.AppendCompaction(comp) + require.NoError(t, err) + + m, err = session.OpenSession(m.File()) + require.NoError(t, err) + entries := m.BuildContext() + require.Len(t, entries, 2) + restored, ok := entries[0].(session.CompactionEntry) + require.True(t, ok) + assert.Equal(t, want, restored.Compaction.Summary) + assert.Equal(t, []string{"notes.md"}, restored.Compaction.Details.ReadFiles) + assert.Equal(t, []string{"a.go"}, restored.Compaction.Details.ModifiedFiles) + assert.Equal(t, keptID, entries[1].GetID()) +} diff --git a/internal/session/compaction/summary_test.go b/internal/session/compaction/summary_test.go index f9f77931..f85ba045 100644 --- a/internal/session/compaction/summary_test.go +++ b/internal/session/compaction/summary_test.go @@ -115,3 +115,39 @@ func TestSummarizeMidTurnCut_CapsEachSummary(t *testing.T) { assert.Contains(t, summary, "Turn Context (mid-turn cut)") assert.ElementsMatch(t, []int{13107, 8192}, c.maxTokens) } + +func TestCompact_EmptyHistory(t *testing.T) { + for _, midTurn := range []bool{false, true} { + for _, previousSummary := range []string{"", "Keep the public API unchanged.\nPreserve exact error messages."} { + name := "history" + if midTurn { + name = "mid-turn" + } + if previousSummary != "" { + name += "/previous-summary" + } else { + name += "/no-previous-summary" + } + t.Run(name, func(t *testing.T) { + c := &captureCompactor{text: "Current turn context."} + prep := CompactionPreparation{PreviousSummary: previousSummary, IsMidTurnCut: midTurn} + want := previousSummary + if want == "" { + want = "No prior history." + } + wantCalls := 0 + if midTurn { + prep.TurnPrefixMessages = []llm.Message{{Role: llm.RoleUser, Content: "fix the parser"}} + want += "\n\n---\n\n**Turn Context (mid-turn cut):**\n\nCurrent turn context." + wantCalls = 1 + } + + comp, err := Compact(t.Context(), prep, c) + + require.NoError(t, err) + assert.Equal(t, want, comp.Summary) + assert.Len(t, c.prompts, wantCalls, "an empty history must not trigger a model request") + }) + } + } +}