Skip to content

Conversation

@harshav167
Copy link
Contributor

@harshav167 harshav167 commented Jan 6, 2026

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

  • Multi-phase compaction flow: DCP → Truncation → Decision → Summarize (if needed)
  • Protected messages: Configurable truncation_protection_messages option to protect recent messages from truncation (default: 3)
  • Compaction logging: New compaction-logger.ts utility for detailed compaction event logging to compaction.log
  • Smart decision logic: Skip summarization if DCP + truncation reduce usage below threshold

Configuration

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 logic
  • src/hooks/preemptive-compaction/compaction-logger.ts - New logging utility
  • src/config/schema.ts - New config option
  • src/hooks/anthropic-context-window-limit-recovery/storage.ts - Updated to support protected messages

Tests Added

  • src/hooks/preemptive-compaction/index.test.ts - Comprehensive hook tests (18 test cases)
  • src/hooks/preemptive-compaction/compaction-logger.test.ts - Logger utility tests

Testing

bun test src/hooks/preemptive-compaction/
# 18 pass, 0 fail

Rationale

The current preemptive compaction immediately triggers summarization when context usage exceeds the threshold. This PR adds a smarter approach:

  1. First, try DCP - Remove redundant/stale tool outputs (deduplication, superseded writes, error purging)
  2. Then, try truncation - Truncate large tool outputs while protecting recent messages
  3. Only then, if still above threshold - Trigger summarization

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

    • Multi-phase flow: DCP → truncation → decision → summarize only if still above threshold.
    • Protect recent messages from truncation via experimental.truncation_protection_messages (1–10, default 3).
    • Compaction logging with phase details written to compaction.log.
    • Smart decision logic to skip summarization when pruning lowers usage below the threshold.
  • Migration

    • Optionally enable experimental.dcp_for_compaction or dynamic_context_pruning.enabled.
    • Tune experimental.truncation_protection_messages as needed.
    • No changes required if defaults are acceptable.

Written for commit 3a503f2. Summary will update on new commits.

…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-apps
Copy link

greptile-apps bot commented Jan 6, 2026

Greptile Summary

This 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

  • Multi-phase flow: DCP → Truncation → Decision → Summarize (only if needed)
  • Protected messages: New truncation_protection_messages config (default: 3) protects recent messages from truncation by sorting messages by mtime
  • Compaction logging: New compaction-logger.ts utility provides detailed logging to compaction.log with formatted output tracking each phase
  • Smart decision logic: Skips summarization if DCP + truncation successfully reduce usage below threshold, reducing expensive LLM calls

Implementation Quality

  • Excellent test coverage: 18 comprehensive test cases covering all flows, edge cases, and configuration options
  • Proper TDD approach: Tests follow BDD patterns with clear #given, #when, #then comments as per project conventions
  • Clean architecture: Well-separated concerns with dedicated logger utility and reuse of existing DCP/truncation functions
  • Backward compatibility: DCP is opt-in via config flags (dynamic_context_pruning.enabled or dcp_for_compaction)

Minor Suggestions

  • Consider deduplicating the CHARS_PER_TOKEN constant (exists in both files)

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

  • This PR is safe to merge with minimal risk
  • Score reflects exceptional code quality: comprehensive test coverage (18 test cases), clean architecture following project conventions (TDD with BDD patterns), proper error handling (silent fail for logging), backward compatibility (DCP is opt-in), and smart design that reduces expensive summarization calls. The only minor issue is a duplicated constant, which is a style concern rather than a functional problem.
  • No files require special attention

Important Files Changed

Filename Overview
src/hooks/preemptive-compaction/index.ts Core multi-phase compaction logic with DCP → truncation → summarization flow. Well-structured with comprehensive logging and smart decision logic to skip summarization when possible.
src/hooks/preemptive-compaction/index.test.ts Comprehensive test suite with 18 test cases covering all multi-phase flows, edge cases, and configuration options. Excellent test quality following BDD patterns.
src/config/schema.ts Added truncation_protection_messages config with appropriate validation (1-10 range). Clean schema change.
src/hooks/anthropic-context-window-limit-recovery/storage.ts Enhanced findToolResultsBySize and truncateUntilTargetTokens to support protected message count. Correctly sorts by mtime for accurate message ordering.

Sequence Diagram

sequenceDiagram
    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
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (1)

  1. src/hooks/preemptive-compaction/index.ts, line 39 (link)

    style: Consider exposing CHARS_PER_TOKEN as a shared constant from pruning-types.ts instead of duplicating it

    The 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

Edit Code Review Agent Settings | Greptile

Copy link

@cubic-dev-ai cubic-dev-ai bot left a 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.ts leaves 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&#39;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)
Copy link

@cubic-dev-ai cubic-dev-ai bot Jan 6, 2026

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&#39;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>
Fix with Cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant