feat(compaction): report user turns the retention budget could not hold - #8424
Merged
Merged
Conversation
Retention landed with no way to see it fail. A turn past the budget still folds into the digest, and the projection reads exactly the same either way — so the one case where compaction hands a user's own words to the summarizer was the one case nobody could observe. That also left [[keep]] useless in practice: an escape hatch only helps someone told it exists at the moment it is needed. keepUserTurns now returns a userTurnRetention count that rides the existing telemetry to CompactionTelemetry (user_kept / user_dropped), and a committed checkpoint that dropped a turn emits a warning naming [[keep]]. The notice fires after commit, never on a rejected candidate: a checkpoint that was not installed folded nothing. The retention code moves to compact_user_turns.go — compact.go was 705 lines against the 800 ceiling, and this is one responsibility. partitionFoldForProjection drops its early/carried return values, which have been nil since content-driven summary replaced the hoist, rather than growing to five results. Coverage follows the two gaps this series left open: - A boot-level effect test drives the real Build assembly through a fold and asserts the constraint reaches the provider request, with a summarizer whose digest deliberately records nothing. Verified by mutation: stubbing keepUserTurns to a no-op turns it red. - Sub-agents inherit retention through subagentOptions, the single construction point they share; a test pins that inheritance and the child-scoped budget, since a parent's instructions are the child's constraints and nothing else in the child transcript records them. partitionCoversRegion additionally asserts the conservation law: every user turn in a region is either kept or counted as dropped, so a turn cannot go missing from the accounting the way it previously could from the projection.
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
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.
Follow-up to #8419, which made compaction keep folded user turns verbatim within
a budget. This closes the two gaps that PR left open.
The blind spot
Retention shipped with no way to see it fail. A turn past the budget still folds
into the digest, and the projection reads exactly the same whether every
constraint survived or one was dropped. So the single case where compaction
still hands a user's own words to the summarizer was the single case nobody
could observe — the same shape of failure the original change set out to fix,
one level up.
It also left
[[keep]]useless in practice. An escape hatch documented only inSPEC.md does not help the person who needed it at the moment they needed it.
Change
keepUserTurnsreturns auserTurnRetentioncount that rides the existingtelemetry path into
CompactionTelemetryasuser_kept/user_dropped, and acommitted checkpoint that had to drop a turn emits a warning naming
[[keep]]. The notice fires after commit and never on a rejected candidate — acheckpoint that was not installed folded nothing, so warning there would be a
false alarm.
Two structural cleanups ride along, both in service of the change rather than
beside it:
compact_user_turns.go.compact.gowas at 705lines against the 800-line ceiling, and this is one responsibility.
partitionFoldForProjectiondrops itsearly/carriedreturn values,which have been
nilsince content-driven summary replaced the early-user-turnhoist. Threading retention through would otherwise have made it five results.
Coverage
Effect test at the final boundary (
internal/boot). Drives the realBuildassembly through an actual fold and asserts the constraint reaches the provider
request — with a summarizer whose digest deliberately records nothing, so a
digest cannot be what carries it. The constraint is the second user turn on
purpose: the first is pinned into the stable prefix and would not exercise
fold-region retention at all.
This test was verified by mutation, not just by passing: stubbing
keepUserTurnsto return early turns it red withconstraint absent from the post-compaction request. Restored and re-verified green.Sub-agent inheritance. Sub-agents build from
subagentOptions, the singleconstruction point
task/read_only_task/parallel_taskschildren share,which already forwards
ContextWindow,KeepPolicyandCompactRatio— soretention is inherited with no new code. A test pins that inheritance and the
child-scoped budget, because a parent's instructions are the child's
constraints and nothing else in the child transcript records them.
Conservation law.
partitionCoversRegionnow also asserts that every userturn in a region is either kept or counted as dropped. A turn can no longer go
missing from the accounting the way it previously could from the projection, and
every test using that helper inherits the check.
Verification
Cache-impact: low - the stable system-prompt prefix is untouched; this adds counters to existing compaction telemetry and one post-commit notice. No change to which messages compose the projection, so the provider-visible bytes for a given transcript are identical to #8419.
Cache-guard: internal/boot/compaction_retention_effect_test.go asserts the post-compaction provider request through the real Build assembly (mutation-verified), plus the unchanged internal/agent/cachehit_e2e_test.go.
System-prompt-review: esengine - the only internal/boot change is a new test file; no system-prompt text, tool schema, memory content, or boot wiring is modified.
Documentation-impact: updated - docs/SPEC.md "What survives a fold" now states that a drop is reported rather than silent, with the matching text in docs/SPEC.zh-CN.md.