Skip to content

fix(core): honor compaction retained-text budget - #53233

Open
runsonmypc wants to merge 5 commits into
anomalyco:devfrom
runsonmypc:compaction-keep-bound
Open

runsonmypc wants to merge 5 commits into
anomalyco:devfrom
runsonmypc:compaction-keep-bound

Conversation

@runsonmypc

@runsonmypc runsonmypc commented Oct 4, 2026 •

Copy link
Copy Markdown

Issue for this PR

Fixes #43250. Enforces the reported retained-text budget invariant; when no complete exchange fits, retaining no recent tail is permitted by the issue. Coordination: #43250 (comment)

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Compaction currently walks backward from the retention budget to a user boundary, which can retain an entire oversized exchange. Select the next complete user exchange within the allowance instead; if none fits, summarize everything. Include separator and rounding overhead, and honor keep.tokens: 0.

The regression retains 76,099 characters with a 15k allowance before this change, then fails to continue after switching to a smaller model. With the fix, it summarizes the exchange and continues. Original messages remain stored.

Keeping whole exchanges can leave no verbatim tail even when individual assistant messages would fit. This change does not guarantee that the summary plus retained text and reply reserve fit every model, or repair an existing oversized checkpoint.

How did you verify your code works?

  • Both oversized-exchange regressions fail on current v2 and pass with the patch.
  • 259 tests pass across compaction, configuration, runner, provider-context, native-compaction, and compaction-transport suites. Covers zero retention, separator rounding, assistant-only history after a checkpoint, and continuation. Uses offline deterministic model responses, not live provider validation.
  • Core bun run typecheck, changed-file oxlint, and Prettier checks pass.
  • Root bun run check: lint completes with warnings and no errors; repository typechecking is blocked by missing @tsconfig/node22 in the console packages under the filtered dependency install.

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

@nkoynov

nkoynov commented Oct 7, 2026

Copy link
Copy Markdown

Tested on the v2.0.24 tag: it applies cleanly, and the compaction (19) and runner (223) tests pass. In a VM A/B it does what it says: the first compaction summarizes the whole autonomous turn, including a rule from its opening message, and the prompt drops from ~70K to ~36.5K instead of growing to ~93K. Numbers and a production case: #43250 (comment)

Two things I noticed:

  1. When the newest exchange alone exceeds the target, flattenAndDropOldest returns undefined and the first attempt sends prepared.request unflattened, which is larger than its text form. With this PR the whole autonomous turn is that newest exchange; in the VM the summary input was 93.6K tokens against a 70K target. On a real window that becomes a rejection, then "cannot be reduced further". Sending the text form, or splitting the newest group at assistant-message boundaries, would avoid it.
  2. When the newest exchange is over budget, no verbatim tail is kept. Falling back to the newest whole assistant messages within keep (each carries its own tool calls and results) would keep the last few steps when compaction lands mid-task.

@runsonmypc
runsonmypc force-pushed the compaction-keep-bound branch from 40adb05 to 5147c71 Compare October 22, 2026 22:17
@runsonmypc

Copy link
Copy Markdown
Author

Thanks for testing this and for the numbers. I rebased onto dev and fixed point 1 in 5147c71. After #58015, dev already shortens a newest exchange that starts with a user message down to its user text, so the case left was a newest exchange with no user message, like a turn that continues after a checkpoint. When the transcript can't be shortened enough, the first attempt now sends it whole as text instead of the unchanged request, without reasoning, media or tool definitions. There's a test for it. I'm leaving point 2 for a follow-up PR.

@nkoynov

nkoynov commented Oct 22, 2026

Copy link
Copy Markdown

Confirmed on 5147c71: the new test fails without the change, and the compaction and runner suites pass (276/276).

One case is still open: a newest exchange with no user message that's still over the target even with tool outputs removed (a long turn continuing after a checkpoint). The first attempt sends the uncut text, and since the retry targets are fractions of that size, all four retries send the same outputs-cut transcript and never the outputs-removed one. In my probe a 40-step turn that would have fit with outputs removed failed after five requests. Trying the shortest version once before giving up, or splitting that exchange at assistant messages, would cover it. A follow-up is fine.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

2 participants