-
Notifications
You must be signed in to change notification settings - Fork 843
feat(preemptive-compaction): add smart multi-phase compaction with DCP and truncation #529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
feat(preemptive-compaction): add smart multi-phase compaction with DCP and truncation #529
Conversation
…P and truncation - Add DCP (Dynamic Context Pruning) as first phase before summarization - Add truncation phase to remove large tool outputs before expensive summarization - Protect recent N messages from truncation (configurable via truncation_protection_messages) - Add compaction logging for debugging and monitoring - Only fall back to full summarization if DCP + truncation don't free enough tokens This reduces API costs by avoiding unnecessary summarization calls when simpler strategies (pruning duplicates, truncating large outputs) are sufficient.
- Add tests for compaction-logger utility (path verification) - Add comprehensive tests for preemptive-compaction hook: - Disabled state behavior - Model filtering (Claude only) - Threshold checking - Cooldown mechanism - Multi-phase flow (DCP + truncation + summarize) - Custom threshold and model limit callbacks - onBeforeSummarize callback
Greptile SummaryThis PR enhances the preemptive compaction system with a smart multi-phase approach that attempts cheaper context reduction strategies before falling back to expensive summarization. When context usage exceeds the threshold (default 85%), the system now runs DCP (Dynamic Context Pruning) first to remove redundant tool outputs, then attempts truncation with configurable message protection, and only triggers summarization if still above threshold. Key Improvements
Implementation Quality
Minor Suggestions
The implementation aligns well with the project's anti-patterns guidelines (no type safety violations, proper error handling, appropriate temperature settings implied by the feature design). Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Hook as Preemptive Compaction Hook
participant DCP as Dynamic Context Pruning
participant Truncate as Tool Output Truncation
participant Session as Session API
participant User as User (Toast)
Note over Hook: message.updated event<br/>usage >= 85% threshold
Hook->>Hook: Check cooldown & validate
Hook->>User: Show "Smart Compaction" toast
Hook->>Hook: Log "triggered" phase
alt DCP Enabled
Hook->>DCP: executeDynamicContextPruning()
DCP-->>Hook: PruningResult (itemsPruned, tokensSaved)
Hook->>Hook: Log "dcp" phase
Hook->>Truncate: truncateUntilTargetTokens(protectedMessages=3)
Truncate->>Truncate: Find tool results by size<br/>(skip protected messages)
Truncate-->>Hook: TruncationResult (truncatedCount, bytesRemoved)
Hook->>Hook: Log "truncation" phase
Hook->>Hook: Calculate new usage ratio
Hook->>Hook: Log "decision" phase
alt Usage < Threshold
Hook->>User: Show "Success" toast
Hook->>Hook: Log "skipped" phase
Hook->>Hook: Exit (no summarization)
else Usage >= Threshold
Hook->>User: Show "Still need to summarize" toast
Hook->>Session: summarize()
Session-->>Hook: Summary complete
Hook->>User: Show "Complete" toast
Hook->>Hook: Log "summarized" phase
Hook->>Session: promptAsync("Continue")
end
else DCP Disabled
Hook->>User: Show "Preemptive Compaction" toast
Hook->>Session: summarize()
Session-->>Hook: Summary complete
Hook->>User: Show "Complete" toast
Hook->>Hook: Log "summarized" phase
Hook->>Session: promptAsync("Continue")
end
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
src/hooks/preemptive-compaction/index.ts, line 39 (link)style: Consider exposing
CHARS_PER_TOKENas a shared constant frompruning-types.tsinstead of duplicating itThe same constant already exists in
src/hooks/anthropic-context-window-limit-recovery/pruning-types.ts:40. To maintain consistency and follow DRY principle, import it from there:Then remove the local definition on line 39.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
8 files reviewed, 1 comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 8 files
Confidence score: 4/5
- Test gap in
src/hooks/preemptive-compaction/index.test.tsleaves the assertion always passing, so regressions in the hook could slip through unnoticed even though runtime behavior isn’t directly broken. - Given the issue is limited to test coverage and production logic remains untouched, overall merge risk stays modest but worth improving soon.
- Pay close attention to
src/hooks/preemptive-compaction/index.test.ts- assertion never verifies behavior, so strengthen the expectation.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/hooks/preemptive-compaction/index.test.ts">
<violation number="1" location="src/hooks/preemptive-compaction/index.test.ts:268">
P2: This assertion is a no-op that always passes and doesn't verify any actual behavior. Consider using `expect(hook.event(...)).resolves.not.toThrow()` or verify specific state changes occurred.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }) | ||
|
|
||
| // No error thrown means success | ||
| expect(true).toBe(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: This assertion is a no-op that always passes and doesn't verify any actual behavior. Consider using expect(hook.event(...)).resolves.not.toThrow() or verify specific state changes occurred.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/preemptive-compaction/index.test.ts, line 268:
<comment>This assertion is a no-op that always passes and doesn't verify any actual behavior. Consider using `expect(hook.event(...)).resolves.not.toThrow()` or verify specific state changes occurred.</comment>
<file context>
@@ -0,0 +1,522 @@
+ })
+
+ // No error thrown means success
+ expect(true).toBe(true)
+ })
+ })
</file context>
Summary
This PR enhances the preemptive compaction system with a smarter multi-phase approach that attempts DCP (Dynamic Context Pruning) and tool output truncation before falling back to summarization.
Changes
New Features
truncation_protection_messagesoption to protect recent messages from truncation (default: 3)compaction-logger.tsutility for detailed compaction event logging tocompaction.logConfiguration
New config options in
experimental:truncation_protection_messages: Number of recent messages to protect from truncation (1-10, default: 3)Files Changed
src/hooks/preemptive-compaction/index.ts- Main hook with multi-phase logicsrc/hooks/preemptive-compaction/compaction-logger.ts- New logging utilitysrc/config/schema.ts- New config optionsrc/hooks/anthropic-context-window-limit-recovery/storage.ts- Updated to support protected messagesTests Added
src/hooks/preemptive-compaction/index.test.ts- Comprehensive hook tests (18 test cases)src/hooks/preemptive-compaction/compaction-logger.test.ts- Logger utility testsTesting
Rationale
The current preemptive compaction immediately triggers summarization when context usage exceeds the threshold. This PR adds a smarter approach:
This reduces the frequency of expensive summarization calls and preserves more context for the model.
Summary by cubic
Adds smart multi-phase preemptive compaction that runs DCP and truncation before falling back to summarization, reducing token usage and unnecessary summarize calls. Includes structured logging and protection for recent messages.
New Features
Migration
Written for commit 3a503f2. Summary will update on new commits.