Repository navigation
fix(core): bound session title generation with a small output limit - #54411
eatmoreduck wants to merge 2 commits into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
This bounds the title request as intended: with the #54401 reproduction (fake LM Studio server, dev from source), the title request went from no max_completion_tokens on the base to 512 with this change. The main request is unchanged. Title, generate and compaction tests (30/30) and typecheck in packages/core pass. The public API is unchanged.
The concern is that 512 is too low as a ceiling for every title request, not just local thinking models (see the inline comment). Requests with a thinking budget can be rejected or come back empty. Since the fallback to the session model goes through the same title path with the same cap, those sessions may end up with no title at all. A higher ceiling that still stops the runaway, or a cap that applies only when the title model's reasoning is off, would avoid this.
| const SUMMARY_OUTPUT_MAX = 32_000 | ||
| // A title never needs more; without a bound a local thinking model can reason in circles | ||
| // for minutes on the background title request while the user waits (#54401). | ||
| const TITLE_OUTPUT_MAX = 512 |
There was a problem hiding this comment.
512 is below what several providers need when the title model has reasoning on. selectTitle only picks a none/minimal/low variant when the small model advertises one. Otherwise, and on the fallback attempt in SessionTitle.attempt (same title kind, same cap), the request uses the session's variant:
- Anthropic and Bedrock Claude with a budget variant:
fitThinkingBudget(budget, 512, 1024)returns 1024, which is not belowmax_tokens: 512, so the API should reject the request. Before this change Anthropic titles went out with the protocol default of 32,000. - OpenRouter: it applies the same 1,024 minimum to
reasoning.max_tokens. - Gemini: its minimum thinking budget is 512, which equals
maxOutputTokensand leaves nothing for the answer. - OpenAI reasoning and adaptive Claude at default effort: fix(core): generate titles at the lowest model variant #51939 measured about 1,300–1,450 reasoning tokens on a title, so 512 would often end before any text.
In each case both attempts fail and the session keeps no title. A ceiling safely above the 1,024 minimum plus room for the answer (2–4k tokens) would still stop the runaway in #54401. Another option is to apply the small cap only when the selected title model has no thinking budget or its reasoning is off.
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
The background title request went out with no max_tokens, so on a local thinking model it reasoned in an unbounded loop for minutes while competing with the main request for CPU (fixes anomalyco#54401). Titles now get a 512-token ceiling through the same outputLimit path as primary and compaction requests; hooks can still change or remove the limit.
8ceaa06 to
843746d
Compare
There was a problem hiding this comment.
The title request is still bounded as intended. Re-running the #54401 reproduction from source, the title request goes from no max_completion_tokens on the base to 512 with this change. Title, generate and compaction tests (44/44) and typecheck in packages/core pass. The public API is unchanged.
The new commits rebase the change and move the cap inline, but the earlier point about 512 being too low when the title model reasons is not addressed. The newer base helps, since selectTitle now picks a lower variant when one exists. But the low variant still reasons. The fallback attempt uses the session's selected variant and gets the same 512 cap. Thinking-budget providers (Claude with a budget, OpenRouter, Gemini) can still reject the title request or return it empty.
512 rejected title requests outright on providers where max_completion_tokens must exceed the thinking budget (Claude budget, OpenRouter, Gemini). 4096 still cuts the circular-reasoning loop from anomalyco#54401 (minutes of tokens) in seconds while leaving a typical budget room to finish.
|
Addressed in 3f70b0d: the cap moves from 512 to 4,096, sized to leave room for a typical thinking budget so budget-aware providers (Claude budget, OpenRouter, Gemini) no longer reject or empty the background title request. The #54401 pathology — a local model reasoning in circles for minutes — burns tokens by the tens of thousands, so 4k still cuts it off within seconds. Title tests (21/21) and the full typecheck suite pass. |
There was a problem hiding this comment.
The earlier point is addressed. With a 4,096-token ceiling, thinking budgets are fitted to 2,048 tokens. That is above the 1,024 minimum for Anthropic, Bedrock and OpenRouter and below max_tokens, so those title requests are no longer rejected or left empty, and runaway local reasoning is still bounded. Re-running the #54401 reproduction from source, the title request goes from no max_completion_tokens on the base to 4096 with this change. Title, generate and compaction tests (44/44) and typecheck in packages/core pass. The public API is unchanged. No further concerns.
Issue for this PR
Closes #54401
Type of change
What does this PR do?
The background session-title request goes out with no output limit:
outputLimitonly coversprimaryandcompaction, so titles get the provider default. On a local thinking model the title generation reasons in circles for minutes while the user waits (#54401).This gives title requests a 512-token ceiling (
TITLE_OUTPUT_MAX) through the sameoutputLimitpath as compaction, min'd with the model's catalog output limit. 512 is generous for a one-line title but caps the runaway. The value still passes through thesession.titlehook, which can change or remove it;generatekeeps the provider default as before.How did you verify your code works?
packages/core/test/session-title.test.ts: the title request now carriesmaxTokens <= 512(red ondev, green with this change)session-title16/16 (including the existing hook test that overridesmaxTokensto 32), plussession-generate/session-compaction/model-resolver61/61, no regressionsbun run typecheckandoxlintclean on changed filesScreenshots / recordings
Not a UI change.
Checklist
AI disclosure: fix and tests written with AI assistance under my direction; I reviewed the diff and ran the verification above locally.
Added by the opencode bot (AI-generated).
Related: #51939