fix(ai): stop compacting the chat on a context window we never measured - #286
Merged
Conversation
Every turn began by measuring the history against `DEFAULT_BUDGET_TOKENS = 80_000` and, past 70% of it, blocking on a whole extra summarizer call before the user's request was even sent — a call that also pays the reasoning budget, so on a thinking model it is not a cheap one. That 80k was invented. The app cannot ask a provider how big its context window is, so the number could not be right for anything: it discards context at 5% fill on a 1M-token Gemini, and would be far too generous elsewhere. It is the same mistake `getTranscript` made with its 800 segments, and it gets the same answer — a guessed limit is deleted, not retuned. Until the app can learn a real window, the only honest trigger is a person deciding they want one, which is the button that already exists. The same guess also gated the manual path: `compactSessionNow` went through the identical heuristic, so on any ordinary conversation pressing Compact did nothing at all, silently. Pressing it is now the decision. The one refusal left is "fewer than 4 messages", which is not a guess about anyone's context window — folding a single exchange into a summary cannot make it shorter — plus the existing measured guard that refuses a summary no smaller than what it replaces. `compactionBlocked` goes with it: it existed only to stop the automatic path re-buying the same useless summary, and nothing retries on its own any more. `DEFAULT_BUDGET_TOKENS` survives as the context pill's denominator and nothing else; both it and its renderer twin now say in writing that they must never regain a decision. Verified by ablation: re-adding the automatic trigger turns four of the six compaction tests red, including the one that pins that a turn never summarizes anything by itself.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughChat compaction is now caller-controlled. Automatic pre-turn compaction was removed. Split planning no longer uses token budgets. Manual compaction validates summaries and preserves session state when compaction does not reduce history. ChangesChat compaction behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
Every turn began by measuring the history against
DEFAULT_BUDGET_TOKENS = 80_000and, past 70% of it, blocking on a whole extra summarizer call beforethe user's request was even sent — a call that also pays the reasoning budget,
so on a thinking model it is not a cheap one.
That 80k was invented. The app cannot ask a provider how big its context
window is, so the number could not be right for anything: it discards context
at 5% fill on a 1M-token Gemini, and would be far too generous elsewhere. It
is the same mistake
getTranscriptmade with its 800 segments, and it getsthe same answer — a guessed limit is deleted, not retuned. Until the app can
learn a real window, the only honest trigger is a person deciding they want
one, which is the button that already exists.
The same guess also gated the manual path:
compactSessionNowwent throughthe identical heuristic, so on any ordinary conversation pressing Compact did
nothing at all, silently. Pressing it is now the decision. The one refusal
left is "fewer than 4 messages", which is not a guess about anyone's context
window — folding a single exchange into a summary cannot make it shorter —
plus the existing measured guard that refuses a summary no smaller than what
it replaces.
compactionBlockedgoes with it: it existed only to stop the automatic pathre-buying the same useless summary, and nothing retries on its own any more.
DEFAULT_BUDGET_TOKENSsurvives as the context pill's denominator and nothingelse; both it and its renderer twin now say in writing that they must never
regain a decision.
Verified by ablation: re-adding the automatic trigger turns four of the six
compaction tests red, including the one that pins that a turn never summarizes
anything by itself.
Summary by CodeRabbit
New Features
Bug Fixes