Repository navigation
fix(compaction): Ignore stale usage in replacement estimates - #2648
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Author
|
|
thomasjiangcy
approved these changes
Nov 4, 2026
A carried assistant message holds provider usage for the whole request before the swap. The replacement estimate took that usage as its base, so prepared summaries that carried 2+ messages were scored at the old size and discarded as over_target. Estimate each replacement message by its own size, and add the system prompt and tool definitions, which the usage-based trigger already counts. Co-Authored-By: Thomas Jiang <20501235+thomasjiangcy@users.noreply.github.com>
sentry-junior
Bot
force-pushed
the
fix/compaction-replacement-estimate
branch
from
November 4, 2026 07:40
d0b0809 to
be59d28
Compare
Contributor
|
6 screenshot changes — 6 changed · 0 added · 0 removed Review screenshots in Frameshift
|
This was referenced Nov 4, 2026
thomasjiangcy
added a commit
that referenced
this pull request
Nov 5, 2026
…2743) A background summary was requested only after the agent loop ended, so a single long Turn that crossed the compaction threshold mid-run never had one ready. Since #2648 shipped, prod traces show no `over_target` discards. The inline compactions that remain are `active` ones (~20s inside the Turn), and mid-Turn swaps almost never apply. The request now runs in `prepareNextTurnWithContext`, after every model call. That covers the final reply too, so the Turn-end call is removed. Pi waits for `turn_end` listeners before this hook, so the tool results are already committed when the mark is taken, and the mark lands on a safe cut after a tool result. The request is skipped when the hook just replaced history, because the last usage then counts the old history. The hook runs before every model call, so the check above the mark now reads only the newest `compaction_requested`/`compaction_prepared` fact first. It loads the full history only when no request is pending and no summary became ready in this slice. `loadLatestBackgroundCompaction` moves from the swap module into `background-compaction.ts`, so both use one copy. The mid-Turn swap stays at the threshold. Swapping as soon as a summary is ready would break the warm prompt cache in the middle of a Turn. **Test coverage gap:** I verified the hook wiring with a real-agent integration test (Slack harness, scripted model stream). It fails on main and passes here. I did not commit it, because `test-architecture:check` does not allow tests in `packages/junior/tests` to run the agent. An integration eval would have to tune the context window to real token counts, and I couldn't run evals in this environment. Only the request rule is covered: a pending request blocks another one. Decide whether you want an eval before merge. <!-- junior-request-attribution:start --> via @thomasjiangcy. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:slack%3AC0CCUMX7ET0%3A1793877933.460399 --> -- [View Session](https://junior-prod.sentry.dev/conversations/slack%3AC0CCUMX7ET0%3A1793877933.460399) [[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AC0CCUMX7ET0%3A1793877933.460399/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Thomas Jiang <20501235+thomasjiangcy@users.noreply.github.com> Co-authored-by: Thomas Jiang <thomas.jiang@sentry.io>
This branch was successfully deployed
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.
Over the last 24h, 58 of 128 prepared-summary swaps that had a ready summary were thrown away as
over_target. Every discard carried 2 or more messages after the mark.The cause is
estimateHistoryTokens→ piestimateContextTokens. It takes the provider usage on the last assistant message as its base. A carried assistant message still holds the usage of its own model call, and that call saw the full history from before the swap. So the replacement scored at about the old size (~309k in one case, against a 288k target) no matter how short the summary was. That conversation then discarded the same summary on each turn for ~2h, until an active compaction ran inside the turn (~28s).Changes:
estimateTokensand ignore usage. The pre-swap estimate still uses usage, because that history really is that big.estimateRequestOverheadTokens, chars/4 over the prompt plus{ name, description, parameters }of each tool). The usage-based trigger already counts them, so without this the check compared a full-request number with a history-only number.The chars/4 heuristic itself stays as it is. Against prod usage it undercounts by ~1.6× (and leaves out ~36k of overhead before this change). That makes the target check lenient, but it's safe: the next model call reports real usage, and the normal compaction still runs at the threshold.
via @thomasjiangcy.
--
View Session [Sentry]