fix(runtime): reach the standalone compaction call site with the retreat - #4671
Conversation
apache#4667 wired the proven-boundary retreat into the mid-turn and pre-turn call site and missed the standalone one, which is the site manual compaction uses. Without `runHeaders` and `acceptedRoute`, `acceptedInputBoundary` returns nothing on its first line, so the first `input_too_large` fails open. That path retreated before: its coverage gate admitted every halving step, so the loop walked down until a span was accepted. Four entries reach it — CLI `/compact`, Desktop `sessions:compact`, sub-agent compaction from supervisor wake, and the pre-turn fallback — and its first attempt covers the whole prior session with no reserved tail, which is the span most likely to be rejected. So the regression lands on the ordinary long session, not an edge. Both values are already in hand at that call site. The test drives `compactHistory` rather than the planner: the planner tests hand the route in directly, so they would have stayed green with both call sites deleted, which is exactly how this got through. Reported by @Astro-Han on apache#4667. Refs apache#4559 Generated-by: Claude Code Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
|
Ready for review, and small: two values passed at one call site, plus the test that would have caught the gap. @Astro-Han — this is your finding from #4667. I confirmed the standalone site retreated before, so it is a regression that PR introduced rather than a gap it left, and the test drives @sylvesterkaczmarek — this reuses the route-aware boundary from your review on #4667 at the second call site, with no new derivation, so there is one rule and two call sites.
|
Astro-Han
left a comment
There was a problem hiding this comment.
Exactly the two lines, identical to the mid-turn site, so there is one rule at two call sites rather than a second derivation. The test earns its place: it drives compactHistory rather than the planner, and asserts both attempts by coverage, with the retreat stopping where the newest reply on this route begins. Without the route reaching the planner there is no second attempt, so it fails on the previous commit, which is the obligation the planner tests could not carry.
Declining the mid-turn item here is the right call too, and your reading of it matches mine: pre-existing, and it needs its own change to the gate.
…eat (apache#4671) apache#4667 wired the proven-boundary retreat into the mid-turn and pre-turn call site and missed the standalone one, which is the site manual compaction uses. Without `runHeaders` and `acceptedRoute`, `acceptedInputBoundary` returns nothing on its first line, so the first `input_too_large` fails open. That path retreated before: its coverage gate admitted every halving step, so the loop walked down until a span was accepted. Four entries reach it: CLI `/compact`, Desktop `sessions:compact`, sub-agent compaction from supervisor wake, and the pre-turn fallback. Its first attempt covers the whole prior session with no reserved tail, which is the span most likely to be rejected, so the regression landed on the ordinary long session rather than an edge. The first three entries reported failure with a `context_compaction_failed_open` note; the fourth sent the oversized history and the turn died with `context_overflow`. The fix passes the same two values the other call site already passes, so there is one rule and two call sites rather than two rules. The test drives `compactHistory` rather than the planner: the planner tests hand the route in directly, so they would have stayed green with both call sites deleted, which is exactly how this got through. Mid-turn still cannot reach a retreat, because `priorRunHeaders` excludes the current turn so the proven index lands at or below `headAnchorIndex` while the gate wants it above. That is a pre-existing limit rather than something apache#4667 changed, and it needs its own change to the gate, so it is not in this PR. No protocol or schema change. Refs apache#4559, apache#4667 Generated-by: Claude Code Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J Generated-by: GLM-5.3-Flash (ZCode)
Summary
Follow-up to #4667, reported by @Astro-Han there. That PR wired the proven-boundary retreat into the mid-turn and pre-turn call site and missed the standalone one, which is the site manual compaction uses.
Without
runHeadersandacceptedRoute,acceptedInputBoundaryreturns nothing on its first line, so the firstinput_too_largefails open with no retreat at all. That path retreated before: its coverage gate admitted every halving step, so the loop walked down until a span was accepted. So this is a regression #4667 introduced, not a pre-existing gap.Four entries reach the standalone site — CLI
/compact, Desktopsessions:compact, sub-agent compaction from supervisor wake, and the pre-turn fallback inai-sdk-backend.ts. The first three end as failed with acontext_compaction_failed_opennote; the fourth sends the oversized history and the turn dies withcontext_overflow. Its first attempt covers the whole prior session with no reserved tail, which is the span most likely to be rejected, so the regression lands on the ordinary long session rather than an edge.Both values are already in hand at that call site:
input.runtimeContextRunHeadersis used ten lines below, and the route is the same one the mid-turn site passes.Refs #4559, #4667
Verification
The test drives
compactHistory, notplanHistoryCompaction. That distinction is the point: the planner tests handacceptedRoutein directly, so they would have stayed green with both call sites deleted, which is exactly how this got through. "manual compactHistory retreats to a span this route has accepted" asserts both attempts — the full span, then the retreat stopping where the newest reply this route produced begins — and fails on the previous commit, where there is no second attempt.ai-sdk-backend214/214,history-compaction24/24,mid-turn-capacity-backend73/73; typecheck, lint and format clean.Self-review
priorRunHeadersexcludes the current turn so the proven index lands at or belowheadAnchorIndexwhile the gate wants it above. I confirmed that reading. It is a pre-existing limit rather than something fix(runtime): retreat a rejected fold to a span the provider has accepted #4667 changed (halving usually undershot the same gate), and it needs its own change to the gate, so it is not in this PR.AI use
Select exactly one:
Tool(s) and scope: Claude Code — implementation; reviewed and verified by the author.
Checklist
Does this PR entail a change in behavior?
https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J