feat(context): compact after large responses - #84
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds post-response auto-compaction: after an assistant response the runtime evaluates context usage against a threshold, emits start/completion/failure events, and runs CompactSessionFrom when needed. Refactors compaction message formatting, exposes test helpers, updates runtime gating to avoid redundant compaction, and adds integration/unit tests. ChangesPost-Response Context Auto-Compaction
Sequence Diagram(s)sequenceDiagram
participant Client
participant Runtime
participant SessionRepository
participant Compactor
Client->>Runtime: send continuation prompt / finish response
Runtime->>SessionRepository: GetContextUsage(sessionID)
SessionRepository-->>Runtime: return token usage
Runtime->>Runtime: derive contextBudgetFromUsage
Runtime->>Runtime: shouldAutoCompactAfterResponse(decision)
alt threshold_exceeded
Runtime->>Client: emit post-response compaction start event
Runtime->>Compactor: CompactSessionFrom(parentEntryID)
Compactor-->>SessionRepository: write compaction branch and leaf
Runtime->>Client: emit post-response compaction completion event
else compaction_not_needed
Runtime->>Client: no compaction performed
end
opt compaction_error
Runtime->>Client: emit post-response compaction failure event
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/assistant/context_auto_compaction_test.go (1)
92-92: Add a brief rationale forOutputReserveTokens = 1in this test.File:
internal/assistant/context_auto_compaction_test.go
Lines: 92-92Snippet showing the final state of code at these lines
runtimeConfig.Context.OutputReserveTokens = 1
OutputReserveTokenswas changed from0→1in the same test harness (commit message: “stabilize post-response compaction”). Since this shifts the available context budget by 1 token, add a short comment explaining why reserving 1 token is required here (so future adjustments don’t treat it as an accidental tweak).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/assistant/context_auto_compaction_test.go` at line 92, Add a short inline comment explaining why runtimeConfig.Context.OutputReserveTokens is set to 1 in the test: mention it reserves one token for the model's post-response output/compaction bookkeeping to avoid off-by-one failures in the post-response compaction assertions (this stabilizes the test after changing from 0→1), and place the comment immediately above the OutputReserveTokens assignment so future readers know it is intentional.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/assistant/context_auto_compaction_test.go`:
- Line 92: Add a short inline comment explaining why
runtimeConfig.Context.OutputReserveTokens is set to 1 in the test: mention it
reserves one token for the model's post-response output/compaction bookkeeping
to avoid off-by-one failures in the post-response compaction assertions (this
stabilizes the test after changing from 0→1), and place the comment immediately
above the OutputReserveTokens assignment so future readers know it is
intentional.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5fe6dd04-477e-4d12-9dfc-ab2671113f94
📒 Files selected for processing (2)
internal/assistant/context_auto_compaction_test.gointernal/assistant/context_post_response_auto_compaction_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/assistant/context_post_response_auto_compaction_test.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #84 +/- ##
==========================================
+ Coverage 68.44% 68.61% +0.17%
==========================================
Files 212 212
Lines 18638 18696 +58
==========================================
+ Hits 12756 12829 +73
+ Misses 4736 4725 -11
+ Partials 1146 1142 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/assistant/export_test.go (1)
57-68: 💤 Low valueConsider adding a clarifying comment.
The hardcoded
contextBudgetvalues (InputTokens: 12, UsableInput: 10, etc.) appear arbitrary. A brief comment explaining they're chosen for formatting tests where exact values don't affect the output would improve clarity.📝 Example comment
// AutoCompactionMessageForTest exposes compaction notice formatting for external package tests. +// The contextBudget values are arbitrary; message formatting tests don't depend on specific token counts. func AutoCompactionMessageForTest(entry *database.EntryEntity) string {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/assistant/export_test.go` around lines 57 - 68, Add a short clarifying comment above AutoCompactionMessageForTest explaining that the hardcoded contextBudget values (InputTokens: 12, UsableInput: 10, etc.) are arbitrary and chosen solely to exercise compactionMessage formatting in tests where exact numeric values don't affect the output; reference the function name AutoCompactionMessageForTest, the contextBudget fields, and compactionMessage so future readers understand these values are not semantic but for formatting/testing purposes.internal/assistant/context_auto_compaction_internal_extra_test.go (1)
96-105: ⚡ Quick winConsider extracting event assertion to a helper.
The
assert.Conditionwith inline loop is functional but verbose. For better readability, consider a helper likeassertContainsCompactionErrorEvent(t, events).♻️ Example helper refactor
func assertContainsCompactionErrorEvent(t *testing.T, events []assistant.StreamEvent) { t.Helper() for _, event := range events { if event.Kind == assistant.StreamEventContextCompaction && strings.Contains(event.Text, "context auto-compaction after response failed:") { return } } t.Error("expected context compaction error event not found") }Then replace lines 96-105 with:
- assert.Condition(t, func() bool { - for _, event := range events { - if event.Kind == assistant.StreamEventContextCompaction && - strings.Contains(event.Text, "context auto-compaction after response failed:") { - return true - } - } - - return false - }) + assertContainsCompactionErrorEvent(t, events)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/assistant/context_auto_compaction_internal_extra_test.go` around lines 96 - 105, Extract the inline assert.Condition loop into a helper to improve readability: create a helper function named assertContainsCompactionErrorEvent(t *testing.T, events []assistant.StreamEvent) that calls t.Helper(), iterates over events, checks event.Kind == assistant.StreamEventContextCompaction and strings.Contains(event.Text, "context auto-compaction after response failed:"), returns on match and calls t.Error(...) if not found; then replace the assert.Condition(...) block with a call to assertContainsCompactionErrorEvent(t, events).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/assistant/context_auto_compaction_internal_extra_test.go`:
- Around line 96-105: Extract the inline assert.Condition loop into a helper to
improve readability: create a helper function named
assertContainsCompactionErrorEvent(t *testing.T, events []assistant.StreamEvent)
that calls t.Helper(), iterates over events, checks event.Kind ==
assistant.StreamEventContextCompaction and strings.Contains(event.Text, "context
auto-compaction after response failed:"), returns on match and calls
t.Error(...) if not found; then replace the assert.Condition(...) block with a
call to assertContainsCompactionErrorEvent(t, events).
In `@internal/assistant/export_test.go`:
- Around line 57-68: Add a short clarifying comment above
AutoCompactionMessageForTest explaining that the hardcoded contextBudget values
(InputTokens: 12, UsableInput: 10, etc.) are arbitrary and chosen solely to
exercise compactionMessage formatting in tests where exact numeric values don't
affect the output; reference the function name AutoCompactionMessageForTest, the
contextBudget fields, and compactionMessage so future readers understand these
values are not semantic but for formatting/testing purposes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 230aa9e0-952c-43ba-8caa-3edfc05785f7
📒 Files selected for processing (2)
internal/assistant/context_auto_compaction_internal_extra_test.gointernal/assistant/export_test.go
|
This was referenced Jun 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatically compact sessions after successful assistant turns when estimated context usage reaches the proactive threshold. Emits non-fatal context compaction events and skips duplicate post-response compaction when a request already compacted before sending.
Validation: