Repository navigation
fix(core): honor compaction retained-text budget - #53233
runsonmypc wants to merge 5 commits into
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: |
|
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:
|
40adb05 to
5147c71
Compare
|
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. |
|
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. |
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
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?
v2and pass with the patch.bun run typecheck, changed-file oxlint, and Prettier checks pass.bun run check: lint completes with warnings and no errors; repository typechecking is blocked by missing@tsconfig/node22in the console packages under the filtered dependency install.Screenshots / recordings
Not a UI change.
Checklist