test+refactor(compaction): PruneSummarizerDisabled coverage + hoist ctx_max - #226
Merged
Conversation
…ax lookup Follow-ups from the post-merge review of the compaction-improvements stack: - Test gap: context_compacted_reports_compaction_kind covered PruneAndSummary, PruneAndFailedSummary, and PruneOnly but not PruneSummarizerDisabled. The kind_for helper now takes a failure count; a new assertion runs a WOULD-SUCCEED summarizer with failures at the cap and asserts the emitted kind is PruneSummarizerDisabled (proving the kind comes from the breaker being open, not the summarizer's outcome). - context_window_for_model was looked up 3x identically per inner-loop iteration (turn-start fold, snip cap, post-usage decision). The window is constant within an iteration (model only changes at a turn boundary), so hoist it to one binding at the top of the loop body and reuse. Note: the reported 'pending_messages never cleared' concern does not apply to current code — it is cleared unconditionally right after injection (run.rs inner loop), so no infinite-spin path exists.
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
…ew-followups test+refactor(compaction): PruneSummarizerDisabled coverage + hoist ctx_max
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.
Post-merge review follow-ups for the compaction-improvements stack (#220/#225/#222/#223/#224).
MINOR — missing test for
PruneSummarizerDisabledcontext_compacted_reports_compaction_kindcoveredPruneAndSummary,PruneAndFailedSummary, andPruneOnlybut notPruneSummarizerDisabled(the breaker-open kind).kind_fornow takes a failure count; a new assertion runs a would-succeed summarizer with failures at the cap and asserts the emitted kind isPruneSummarizerDisabled— proving the kind derives from the breaker being open, not from the summarizer outcome.TRIVIAL — context window looked up 3× per iteration
context_window_for_modelwas called at the turn-start fold, the snip cap, and the post-usage decision with an identical result. The window is constant within an inner-loop iteration (the model can only change at a turn boundary), so it's hoisted to a single binding at the top of the loop body.Pre-existing 'pending_messages never cleared' — not a bug
Investigated:
pending_messagesis cleared unconditionally right after injection in the inner loop, so the reported infinite-spin path can't occur. The original finding was based on stale line numbers. No change made.Full feature-matrix tests green at
-D warnings(2141 passed).