Repository navigation
Make tool output truncation depend on the remaining context window - #14332
Andrea-Bruno wants to merge 1 commit into
Conversation
The tool-output truncation threshold is a fixed char budget that ignores how full the conversation already is. When the context window is nearly full, a result that fits the fixed budget can still fill the window and leave the model too little room to answer, which surfaces as a hard stream error instead of a graceful degrade. Add contextAwareTruncateThreshold, which tightens the global char budget to the estimated remaining window (minus a reserve for the reply), floored so the preview stays useful. The full output is still persisted, so tightening loses nothing recoverable. Per-tool budgets are left exactly as declared.
|
|
doudouOUC
left a comment
There was a problem hiding this comment.
English
Reviewed at 0490faf8f. Thanks â sizing the tool-output budget to the remaining window is the right direction, and the "smaller of configured and remaining, floored" shape reads clearly. A few things need attention before this can be merged.
1. A required check is red at this head because of the new call site. The Test (ubuntu-latest, Node 22.x) job fails with exactly one test:
FAIL src/core/coreToolScheduler.test.ts > CoreToolScheduler request queueing > should auto-approve remaining tool calls when first tool call is approved with ProceedAlways
AssertionError: expected false to be true âŊ coreToolScheduler.test.ts:6849
The new helper is invoked before the try block in the scheduler, and it dereferences config.getContentGeneratorConfig() (the optional chain is on the result, not the method). That test's fixture sets getContentGeneratorConfig: undefined, while the merge base contains no reference to that getter in coreToolScheduler.ts at all â so the tool call now ends in an error instead of being truncated. Moving the call inside the try (it only guards truncation), or guarding the getter, should fix it; either way please re-run the suite.
2. "Truncation disabled" is being re-enabled. getTruncateToolOutputThreshold() returns Number.POSITIVE_INFINITY when the setting is <= 0. With an infinite configured threshold the remainingChars >= configuredThresholdChars early return can never fire, so the helper returns max(1000, remainingTokens * 4). A user who turned truncation off therefore gets their output capped â down to 1000 chars when the window is nearly full â which contradicts the "leaves the configured limit untouched" behaviour described above. Please decide explicitly whether the context guard should override an explicit opt-out, and add a case for it.
3. The constant's rationale and the arithmetic point in opposite directions. The comment on CHARS_PER_TOKEN says the value is kept high so the estimate errs toward leaving room, but remainingChars = remainingTokens * CHARS_PER_TOKEN means a higher value inflates the budget and eats the reply reserve â i.e. it errs toward over-filling. Note the repo already carries a canonical CHARS_PER_TOKEN (aliased to TOKEN_TO_CHAR_RATIO so the compaction trigger and the compressor cannot drift) and a CONSERVATIVE_NEW_CONTENT_SAFETY_FACTOR added after two production overflows traced to char/4 under-counting CJK-dense tool output by 39â54%. Reusing the canonical constant, and saying which way the error leans, would keep this consistent with that history.
4. Token accounting differs from the compaction gate. The helper reads the raw last prompt token count, whereas compaction sizes the same moment with the previous response's output tokens and the new user message added on top; upstream comments document the raw count as an under-estimate on its own, so this budget is looser than what the compaction path assumes. The count also comes from a process-global while the window comes from the per-session config, which can mix sessions under the daemon or concurrent subagents.
5. Unknown windows. Returning the configured threshold when contextWindowSize is absent is safe, but the rest of the codebase resolves the window as getContentGeneratorConfig()?.contextWindowSize ?? modelsConfig.getGenerationConfig().contextWindowSize ?? tokenLimit(getModel(), 'input'), so the guard currently no-ops for models covered by those other hops.
The Lint & Static and triage failures look like stale-base skew rather than this diff: the branch's merge base is about twelve days old, the workflow being run expects .github/scripts/check-ci-budget.mjs which does not exist in the checked-out tree, and lint never actually ran. Rebasing onto current main should clear both and let the real lint/typecheck verdict show up.
äļæ
å·ēåĻ 0490faf8f äļčŊåŪĄãæč°ĒčīĄįŪââčŪĐå·Ĩå
·čūåšéĒįŪč·éåĐä―äļäļæįŠåĢæŊåŊđįæđåïž"åé
į―ŪåžäļåĐä―éįčūå°č
åđķčŪūäļé"čŋäļŠå―Ēæäđåūæļ
æĨãååđķåčŋæå åĪéčĶåĪįã
1. å―å head æäļäļŠåŋ
čŋæĢæĨæŊįšĒįïžåå å°ąæŊæ°įč°įĻä―į―Ūã Test (ubuntu-latest, Node 22.x) åŠæäļäļŠæĩčŊåĪąčīĨïž
FAIL src/core/coreToolScheduler.test.ts > CoreToolScheduler request queueing > should auto-approve remaining tool calls when first tool call is approved with ProceedAlways
AssertionError: expected false to be true âŊ coreToolScheduler.test.ts:6849
æ°į helper åĻ scheduler éæŊåĻ try åäđåč°įĻįïžčåŪäžč§ĢåžįĻ config.getContentGeneratorConfig()ïžåŊééūå åĻčŋååžäļïžčäļæŊæđæģäļïžãčŊĨæĩčŊį fixture æ getContentGeneratorConfig čŪūäļš undefinedïžč merge base į coreToolScheduler.ts éæ đæŽæēĄæäŧŧä―čŊĨ getter įåžįĻââäšæŊčŋæŽĄå·Ĩå
·č°įĻäŧ"čĒŦæŠæ"åæäšæĨéãæč°įĻį§ŧčŋ tryïžåŪåŠäŋæĪæŠæéŧčūïžïžæåŊđčŊĨ getter åéēåūĄæ§åĪæïžåščŊĨå°ąč―äŋŪæïžæ čŪšåĶä―čŊ·éč·æĩčŊåĨäŧķįĄŪčŪĪã
2. "å
ģéæŠæ"įé
į―ŪčĒŦéæ°åŊįĻäšã å―é
į―Ūåž <= 0 æķïžgetTruncateToolOutputThreshold() čŋå Number.POSITIVE_INFINITYãé
į―Ūäļšæ įĐ·åΧæķïžremainingChars >= configuredThresholdChars čŋäļŠæåčŋåæ°ļčŋäļäžå―äļïžäšæŊ helper čŋå max(1000, remainingTokens * 4)ãäđå°ąæŊčŊīïžäļŧåĻå
쿿пįįĻæ·į°åĻčūåšäŧäžčĒŦæŠæââįŠåĢæĨčŋæŧĄæķįčģåŠåĐ 1000 åįŽĶââčŋäļäļæ"é
į―ŪįééĒäŋæäļå"įæčŋ°įļįįūãčŊ·æįĄŪåģåŪäļäļæäŋæĪæŊåĶåšå―čĶįæūåžå
ģéïžåđķäļščŊĨčūå
ĨčĄĨäļäļŠįĻäūã
3. åļļéįčŊīæäļįŪæŊæđåįļåã CHARS_PER_TOKEN įæģĻéčŊīååžåéŦæŊäļšäščŪĐäž°įŪ"ååįŧįŠåĢįåšįĐšéī"ïžä― remainingChars = remainingTokens * CHARS_PER_TOKEN æåģįååžčķéŦãéĒįŪčķåΧãčķåååĪéĒįââäđå°ąæŊååčķ
åĄŦãåĶåĪäŧåšéå·ēæäļäļŠč§čį CHARS_PER_TOKENïžäļšäšäļčŪĐčŠåĻåįžĐč§ĶååĻååįžĐåĻæžį§ŧčåäšåŦåïžïžäŧĨåäļäļŠåĻåĪįčŋäļĪæŽĄįäš§įŊåĒæšĒåšåæå å
Ĩį CONSERVATIVE_NEW_CONTENT_SAFETY_FACTORïžæ đå æŊ char/4 åŊđ CJK åŊéįå·Ĩå
·čūåšä―äž°äš 39%â54%ïžãåĪįĻč§čåļļéïžåđķæčŊŊå·ŪååčŊīæļ
æĨïžåŊäŧĨåčŋæŪĩååēäŋæäļčīã
4. Token åĢåūäļåįžĐéĻéäļäļčīã čŊĨ helper åŠčŊŧååå§į"äļæŽĄ prompt token æ°"ïžčåįžĐč·ŊåūåĻåäļæķåŧæŊįĻåŪåå äļäļäļč―ŪååĪįčūåš token åæŽč―ŪįĻæ·æķæŊæĨäž°įŪįïžäļæļļæģĻéäđæįĄŪčŊīčŋäļŠåå§čŪĄæ°æŽčšŦå°ąåä―ãå æĪčŋéįéĒįŪæŊåįžĐč·ŊåūįåčŪūæīåŪ―æūãåĶåĪčŊĨčŪĄæ°æĨčŠčŋįĻįš§åäūïžčįŠåĢæĨčŠæäžčŊį configïžåĻ daemon æåĪå agent åđķååšæŊäļåŊč―æäļåäžčŊæ··åĻäļčĩ·ã
5. æŠįĨįŠåĢįæ
åĩã įžšå° contextWindowSize æķčŋååé
į―ŪåžæŊåŪå
Ļįïžä―äŧĢį åšå
ķäŧå°æđæŊæ getContentGeneratorConfig()?.contextWindowSize ?? modelsConfig.getGenerationConfig().contextWindowSize ?? tokenLimit(getModel(), 'input') č§ĢæįŠåĢįïžæäŧĨåŊđåéĒčŋå įš§č―č§ĢæåšįŠåĢįæĻĄåïžčŋäļåąäŋæĪįŪåäļäžįæã
Lint & Static å triage įåĪąčīĨįčĩ·æĨæŊæŽåæŊåšįšŋčŋæ§åŊžčīįïžčäļæŊčŋæŽĄæđåĻïžåæŊį merge base åΧįšĶæŊåäšåĪĐåïžčŋčĄæįĻ workflow éčĶį .github/scripts/check-ci-budget.mjs åĻæĢåšįäŧĢį æ éåđķäļååĻïžlint åŪé
äļæēĄæč·čĩ·æĨãæåæŊ rebase å°å―å main åščŊĨč―åæķč§ĢåģčŋäļĪéĄđïžåđķčŪĐįæĢį lint/typecheck įŧčŪšæūįĪšåšæĨã
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed â gaps disclosed.
Not explored to full depth (tool budget reached): "agent 6c": none â no check was cut short by the tool ceiling..
Not reviewed: reverse audit â stopped before round 3 by the review time budget.
â qwen3.8-max via Qwen Code /review (v0.25.2)
| configuredThresholdChars: number, | ||
| reservedOutputTokens: number = CONTEXT_AWARE_OUTPUT_RESERVE_TOKENS, | ||
| ): number { | ||
| const windowSize = config.getContentGeneratorConfig()?.contextWindowSize; |
There was a problem hiding this comment.
[Critical] R1-1: config.getContentGeneratorConfig() is invoked non-optionally â the ?. guards the RESULT, not the method â and the new call site sits OUTSIDE the truncation try whose stated contract is that a truncation failure never demotes a successful tool call to an error. A Config that does not provide the method throws TypeError: config.getContentGeneratorConfig is not a function, which escapes the catch (truncErr) guard and reaches the outer catch (executionError) at coreToolScheduler.ts:7019, finalizing a successful tool call with a non-success status. This ships a red packages/core suite.
coreToolScheduler.test.ts:6782 builds its scheduler fixture with getContentGeneratorConfig: undefined (the shared makeSchedulerConfig default at :788 supplies a real function, which is why only this one test breaks). Every completed call now reaches coreToolScheduler.ts:6430, undefined() throws before the try at :6434, and the catch (truncErr) at :6454 â commented "A truncation/IO failure must never demote a successful tool call to an error â keep the content and warn" â never sees it. The failing test is CoreToolScheduler request queueing > should auto-approve remaining tool calls when first tool call is approved with ProceedAlways, assertion at coreToolScheduler.test.ts:6849 (expected false to be true on completedCalls?.every((call) => call.status === 'success')). In production the same shape reaches any embedder or subagent config view whose Config-like object lacks a working getContentGeneratorConfig: a successful tool result is converted into a failed tool call, precisely the demotion the surrounding try/catch exists to prevent.
Witness:
Agent 7 build-test + test-delta measurement: `netNew = ["src/core/coreToolScheduler.test.ts"]` â fails on the PR side only, passes at merge base 537464132. `npm test --workspace="packages/core"` at the reviewed commit: `Test Files 18 failed | 862 passed`; run alone in the PR tree it fails deterministically twice (`Tests 1 failed | 476 passed (477)`), while the same `-t` run in the base tree passes. Verifier 2 reproduced in an isolated copy: INTACT (PR code) `Tests 1 failed | 476 passed (477)`; BOTH SITES REVERTED `Test Files 1 passed (1)`; direct mechanism probe `PROBE_OUTCOME: threw TypeError: config.getContentGeneratorConfig is not a function`; candidate fix `getContentGeneratorConfig?.()?.contextWindowSize` â `Test Files 2 passed (2)`. Agent 6a independently ran the same one-character mutation probe with the same flip.
| const windowSize = config.getContentGeneratorConfig()?.contextWindowSize; | |
| const windowSize = config.getContentGeneratorConfig?.()?.contextWindowSize; |
One existing fact this fix must not violate: contextAwareBase is read a second time outside the first try, at packages/core/src/core/coreToolScheduler.ts:6487 (const baseThreshold = perToolMax ?? contextAwareBase;), so fix (b) must leave it in scope and still holding the tightened value there â declare it with let before the try rather than moving the const inside. The contract any fix must preserve is quoted at coreToolScheduler.ts:6455: "A truncation/IO failure must never demote a successful tool call". The test that must go red if this fix is later removed: packages/core/src/core/coreToolScheduler.test.ts âš 'should auto-approve remaining tool calls when first tool call is approved with ProceedAlways' â line 6849 asserts every completed call in the auto-approved batch has status === 'success'. It is red at the reviewed commit and green at the merge base, so it goes red again if the guard is removed. Also add a case to truncation.test.ts's contextAwareTruncateThreshold block for a config with no getContentGeneratorConfig, asserting it returns configuredThresholdChars â the five new cases all pass a config that HAS the method, which is why none of them catches this.
â qwen3.8-max via Qwen Code /review (v0.25.2)
| if (!windowSize || windowSize <= 0) { | ||
| return configuredThresholdChars; | ||
| } | ||
| const usedTokens = uiTelemetryService.getLastPromptTokenCount(); |
There was a problem hiding this comment.
[Critical] R1-3: [fails-closed] [regression] The "how full is this conversation" input is read from the process-global uiTelemetryService singleton, which the repo has already identified in two prior issues as the wrong source for a per-session decision, while the windowSize it is compared against resolves per agent frame through AsyncLocalStorage. The two halves of one subtraction therefore come from different owners, so a chat's truncation budget is computed from another chat's context usage.
uiTelemetryService is one module-level instance (uiTelemetry.ts:866) holding a single #lastPromptTokenCount, last-writer-wins across every chat in the process, while config.getContentGeneratorConfig() resolves through getRuntimeContentGenerator() (agent-context.ts:168-177, config.ts:6533-6538) â the CURRENT agent's model view. (a) Subagent: subagent chats are constructed with telemetryService=undefined and deliberately never mirror their count (llm-chat.ts:6149-6152: "Mirror to the global telemetry only when wired â subagents pass telemetryService=undefined to keep their context usage out of the main session's UI counters"), pinned by llm-chat.test.ts:11607-11623; but subagent tools DO run through new CoreToolScheduler({ config: this.runtimeContext }) (agent-core.ts:2302), which now calls this helper. A parent at 120k/128k spawns a subagent with an almost empty context, remainingTokens <= 0, and every subagent tool result is floored to 1000 chars â it loses the output it was spawned to reason over and must burn turns re-reading the spilled file. The inverse also holds: a near-full subagent under a roomy parent gets no tightening. (b) serve daemon: session B completes a turn at 120k/128k; fresh session A runs a tool and its results are shredded to 1000 chars against an almost-empty window. (c) Route/model switch: llm-chat.ts:2487 zeroes the mirror and per-route counts live in LlmChat.tokenCountsByRouteKey, so after a switch usedTokens is 0, remainingTokens is huge, and the tightening is silently DISABLED in exactly the near-full conversation that motivated it. LlmChat.getLastPromptTokenCount() exists for this: "Prefer this over {@link UiTelemetryService} in multi-session daemons (#12047)" (llm-chat.ts:2538-2540); contextCommand.ts:373-378 documents the same hazard (#5763) and its test at contextCommand.test.ts:146 labels the global value "wrong session's global value"; omni/policy/session-context.ts:32-35 states the count must come from the current chat, "never a global UI telemetry counter".
Witness:
Probe drove the built helper with the global set by a simulated "session B" and read by "session A" (window 128k, configured 25000): the fresh session's budget came back at the 1000-char floor against an empty own-context. Corroborating reads: uiTelemetry.ts:352,411-418 (single field), llm-chat.ts:6149-6152 and llm-chat.test.ts:11607-11623 (subagents never mirror), agent-core.ts:2302 (subagents do reach this scheduler), llm-chat.ts:2487 (route-change zeroing), llm-chat.ts:2538-2540 (#12047), contextCommand.ts:373-378 (#5763).
Take the used-token count from the chat that owns this config â e.g. pass the owning LlmChat's getLastPromptTokenCount() (and its window/route) into contextAwareTruncateThreshold from the scheduler, threaded through CoreToolSchedulerOptions the same way the owner-scoped route key already is â and return configuredThresholdChars unchanged when the owner is unknown or unavailable, mirroring contextCommand.ts:381-385.
One existing fact this fix must not violate: packages/core/src/core/llm-chat.ts:6149-6152 â "Mirror to the global telemetry only when wired â subagents pass telemetryService=undefined to keep their context usage out of the main session's UI counters"; llm-chat.ts:2539 â "Prefer this over {@link UiTelemetryService} in multi-session daemons (#12047)"; packages/core/src/config/config.ts:6534-6536 â getRuntimeContentGenerator()?.contentGeneratorConfig ?? this.contentGeneratorConfig; packages/core/src/omni/policy/session-context.ts:32-35 â the count must be the CURRENT chat's, "never a global UI telemetry counter". The test that must go red if this fix is later removed: A truncation.test.ts (or scheduler) case that sets uiTelemetryService.setLastPromptTokenCount(120_000) but supplies an owning-chat count of 0 on a 128k window, asserting the full 25_000 budget is kept; plus a case where the owning chat reports 118_000 and the global reports 0, asserting 7232. Both are red while the function reads the singleton.
â qwen3.8-max via Qwen Code /review (v0.25.2)
| } | ||
| const usedTokens = uiTelemetryService.getLastPromptTokenCount(); | ||
| const remainingTokens = windowSize - usedTokens - reservedOutputTokens; | ||
| if (remainingTokens <= 0) { |
There was a problem hiding this comment.
[Critical] R1-4: [fails-closed] [new-surface] The fixed 8192-token output reserve is subtracted from any window size without being clamped to it, so for a window at or below the reserve the function returns the 1000-char floor from the first turn with zero context used â the opposite of the documented "normal turns are never affected". This repo has already fixed this exact class of bug once.
contextWindowSize is user/provider-settable (contentGenerator.ts:178-181: "Context window size override. If set to a positive number, it will override the automatic detection") and small windows are supported: core itself runs with getContentGeneratorConfig: () => ({ contextWindowSize: 4096 }) in goal-turn-integration.test.ts:76, config.test.ts:7950 uses 1000 and :11128 uses 32000, and Ollama/OpenAI-compatible local models commonly report 4k-8k. With a 4096 window and getLastPromptTokenCount() === 0 on a brand-new session, remainingTokens = 4096 - 0 - 8192 = -4092 <= 0, so every global-path tool result is cut to 1000 chars from the first tool call, permanently, with no way for the user to widen it except disabling truncation entirely â which R1-2 shows is also broken. The model then re-reads and re-greps in a loop over 1000-char stubs. tokenLimits.ts:88-96 records the precedent: "at window 8192 / prompt 5000 the 10K margin floor drives room to -6808 ... (#13252)", and caps its own floor by the room the window actually has (floor = Math.max(1, Math.min(MIN_CLAMPED_OUTPUT_TOKENS, contextWindowSize - promptTokens)), tokenLimits.ts:95-97).
Witness:
Probe against the PR build measured the floor branch firing at `used >= 119808` for a 128k window (window - 8192 reserve) and, for a 4096 window at `used = 0`, `remainingTokens = -4092` returning `CONTEXT_AWARE_MIN_CHARS` rather than the configured threshold. Reachability was adjudicated from the shipped fixtures that construct 4096/1000/32000 windows (goal-turn-integration.test.ts:76, config.test.ts:7950, :11128) plus the user-settable override at contentGenerator.ts:178-181; two finder agents had rated it low-confidence on the grounds that no shipped PRESET is <= 8192 and unknown models fall back to DEFAULT_TOKEN_LIMIT = 200_000 (tokenLimits.ts:13), so an explicit override or a small-window local model is required â the verifier confirmed the mechanism by execution and rated the trigger reachable.
Treat a window that cannot hold the reserve as "unknown" rather than "full": const reserve = Math.min(reservedOutputTokens, Math.floor(windowSize / 2)); if (windowSize <= reserve) return configuredThresholdChars; before computing remainingTokens â or scale the reserve as clampOutputTokensToWindow does for its floor.
One existing fact this fix must not violate: packages/core/src/core/tokenLimits.ts:95-97 â const floor = Math.max(1, Math.min(MIN_CLAMPED_OUTPUT_TOKENS, contextWindowSize - promptTokens)); with the #13252 note at tokenLimits.ts:88-92; packages/core/src/tools/truncation.ts:79 export const CONTEXT_AWARE_OUTPUT_RESERVE_TOKENS = 8192; and :86 export const CONTEXT_AWARE_MIN_CHARS = 1000; â a clamped reserve must still leave reply room on windows large enough to have any. The test that must go red if this fix is later removed: New truncation.test.ts cases: uiTelemetryService.setLastPromptTokenCount(0) then expect(contextAwareTruncateThreshold(cfg(4_096), 25_000)).toBe(25_000), and a cfg(8_192) variant. Both are red today (they return CONTEXT_AWARE_MIN_CHARS).
â qwen3.8-max via Qwen Code /review (v0.25.2)
| if (remainingChars >= configuredThresholdChars) { | ||
| return configuredThresholdChars; | ||
| } | ||
| return Math.max(CONTEXT_AWARE_MIN_CHARS, remainingChars); |
There was a problem hiding this comment.
[Critical] R1-2: [certifies-falsely] [regression] contextAwareTruncateThreshold never clamps its result to configuredThresholdChars from above. Config encodes "truncation disabled" as Number.POSITIVE_INFINITY, and remainingChars >= Infinity is always false, so an explicit user opt-out is silently converted into a finite budget; the remainingTokens <= 0 branch returns the 1000-char floor without reading configuredThresholdChars at all. The same missing clamp also RAISES a configured threshold below the floor, contradicting the function's own JSDoc ("returns the smaller of the configured threshold and the estimated remaining window").
A user sets tools.truncateToolOutputThreshold: -1 â settingsSchema.ts:3341 documents "Set to -1 to disable" and config.ts:11339-11344 returns Number.POSITIVE_INFINITY for any value <= 0 ("A configured value <= 0 disables the cap and is returned as Infinity so callers can compare unconditionally", config.ts:11303-11304). Before this diff the scheduler passed threshold: undefined and truncateToolOutput re-read Infinity from config, so char truncation never fired. Now the helper returns Math.max(1000, remainingChars): on a 128k window with 10k used that is ~439232, so a 500k-char result the user asked to keep whole is truncated and spilled; once the window passes the reserve the same disabled config collapses to 1000 chars. Measured: at used=108000 the PR emitted 9958 chars where BASE emitted 200000; at used=125000 the PR emitted 712 chars where BASE emitted 200000. Separately, truncateToolOutputThreshold has no minimum and core's own tests use 100 and 2500 (coreToolScheduler.test.ts:12746, cli/src/config/config.test.ts:1640): with a configured 500 and remainingChars 100, line 122 returns 1000 â twice the configured budget â so the documented tightener becomes a loosener exactly when context is tightest. The JSDoc promise "Returns the configured threshold unchanged when the window size is unknown or the window still has room, so normal turns are never affected" cannot hold for this configuration: it tightens on EVERY turn.
Witness:
BASE/PR A-B probe on a 500000-char body with a disabled (`Infinity`) config, window 128k, against the PR's built dist: `used=10000 -> contextAwareBase(Infinity)=439232 | PR emitted=200000 | BASE emitted=200000`; `used=108000 -> contextAwareBase(Infinity)=47232 | PR emitted=9958 | BASE emitted=200000`; `used=125000 -> contextAwareBase(Infinity)=1000 | PR emitted=712 | BASE emitted=200000`. Quoted anchors: truncation.ts:117-121 and config.ts:11340-11342 `if (this.truncateToolOutputThreshold <= 0) { return Number.POSITIVE_INFINITY; }`.
Return the configured value untouched when it is not finite, before any window arithmetic: if (!Number.isFinite(configuredThresholdChars)) return configuredThresholdChars; â and cap the floor by the configured value: return Math.min(configuredThresholdChars, Math.max(CONTEXT_AWARE_MIN_CHARS, remainingChars));
One existing fact this fix must not violate: packages/core/src/config/config.ts:11303-11304 â "A configured value <= 0 disables the cap and is returned as Infinity so callers can compare unconditionally", realised at config.ts:11340 if (this.truncateToolOutputThreshold <= 0) { return Number.POSITIVE_INFINITY; }; packages/cli/src/config/settingsSchema.ts:3341 â "Truncate tool output if it is larger than this many characters. Set to -1 to disable."; truncation.ts:96-98 JSDoc. A fix must keep Infinity meaning "no char budget" rather than clamping it to a window-derived number. The test that must go red if this fix is later removed: New cases in truncation.test.ts describe('contextAwareTruncateThreshold'): with setLastPromptTokenCount(10_000) and with setLastPromptTokenCount(120_000), contextAwareTruncateThreshold(cfg(128_000), Number.POSITIVE_INFINITY) must be Number.POSITIVE_INFINITY (today it returns 439232 and 1000); and setLastPromptTokenCount(120_000) with expect(contextAwareTruncateThreshold(cfg(128_000), 500)).toBe(500) (today returns 1000). All three are red today.
â qwen3.8-max via Qwen Code /review (v0.25.2)
| // context window, so a near-full conversation cannot leave the model | ||
| // too little room to answer and surface a hard stream error. Per-tool | ||
| // budgets are left exactly as the tool declared them. | ||
| const contextAwareBase = contextAwareTruncateThreshold( |
There was a problem hiding this comment.
[Suggestion] R1-9: Window fit is an aggregate property of the batch, but the tightening is applied per call against a count that only advances after a response, so N parallel calls each get the full remainder. The layer that exists to bound the aggregate (toolOutputBatchBudget) still sizes against a fixed char budget and never consults the remaining window. Adjudicated a Suggestion rather than a Critical: the per-call budget is min(25000, max(1000, remaining x 4)) <= 25000, so the aggregate is strictly better than the merge base in every reachable state and the residual overflow is caught by the pre-send hard-tier rescue â an incomplete mitigation, not a regression.
uiTelemetryService.setLastPromptTokenCount is written from LlmChat on API usage metadata (llm-chat.ts:6152, 2812, 2927), i.e. only AFTER a response, so every tool call in one assistant turn is truncated before that next response exists and all of them see the identical remainingTokens. With a 128k window / 118k used / 8192 reserved, eight parallel calls each get 7232 chars = ~57856 chars = ~14464 tokens against 1808 tokens of room. applyBatchOutputBudget (coreToolScheduler.ts:7250-7256, :7527-7531) only reduces above getToolOutputBatchBudget() -> DEFAULT_TOOL_OUTPUT_BATCH_BUDGET = 200_000 chars (config.ts:975), ~50000 tokens, far above any near-full window's remainder; docs/design/final-tool-response-budget.md:5 describes these layers as independent and not sharing structured state.
Witness:
Probe measured `overshoot factor: 8.00x` for an 8-call batch on a near-full window, with `delta: 0` between the calls' individual budgets (each got the whole remainder). The verifier also established the BASE arm comparison by arithmetic on the diff's own removed line: the pre-diff call passed `threshold: perToolMax` -> `undefined` -> the configured/Infinity fallback, so the PR's per-call budget is <= the base budget in every state, which is why `baseline: regression` is false.
Compute contextAwareBase once per batch and divide it across the calls being finalized in the same message (or subtract what has already been admitted this turn) instead of granting each call the whole remainder; alternatively narrow the comment at coreToolScheduler.ts:6426-6429 so the claim matches the per-call guarantee.
One existing fact this fix must not violate: packages/core/src/config/config.ts:975 â export const DEFAULT_TOOL_OUTPUT_BATCH_BUDGET = 200_000;, read at coreToolScheduler.ts:7531 as this.config.getToolOutputBatchBudget?.() ?? Number.POSITIVE_INFINITY, and getToolOutputBatchBudget() returns Infinity when configured <= 0 (settingsSchema.ts:3357-3360, "Set to -1 to disable") â an aggregate fix must keep -1 meaning disabled; packages/core/src/tools/truncation.ts:86 â CONTEXT_AWARE_MIN_CHARS = 1000, so a per-call split must not be pushed below the floor the helper promises. The test that must go red if this fix is later removed: A coreToolScheduler.test.ts case scheduling N parallel calls on a near-full window (global count set so remainingTokens is small) asserting the SUM of model-facing contentLengths stays within remainingTokens * CHARS_PER_TOKEN; today each call independently gets the full remainder, so the sum exceeds it N-fold.
â qwen3.8-max via Qwen Code /review (v0.25.2)
| * the fixed char thresholds already treat chars as a rough proxy for tokens, | ||
| * so this stays within the same precision envelope. | ||
| */ | ||
| const CHARS_PER_TOKEN = 4; |
There was a problem hiding this comment.
[Suggestion] R1-5: CHARS_PER_TOKEN = 4 is documented as "Kept deliberately high so the estimate errs toward leaving the window room rather than over-filling it", but the arithmetic runs the other way â remainingChars = remainingTokens * CHARS_PER_TOKEN, so a HIGHER ratio grants MORE chars and leaves LESS room. The comment asserts the inverse of the code, so a maintainer "hardening" it will move the constant the wrong way. The value also reproduces the char/4 under-count this repo documents as the root cause of two production window overflows for CJK-dense tool output.
tokenEstimation.ts:90-95 records: "two independent real production failures (chatCompressionService's 400-overflow root cause doc and a main-turn prompt + max_tokens > window overflow, both 2026-07-28) traced back to char/4 under-counting CJK-dense tool output (design docs, large file reads) by 39-54% â beyond the documented band", and estimateContextTextTokens (tokenEstimation.ts:45-46) prices non-ASCII at nonAsciiChars * 1.5 (~1.5 tokens/char, ~6x this helper's 0.25); chatCompressionService.ts:162-165 does the same split. Replaying the PR's own motivating incident with the repo's named payload â a large read of a Chinese design doc in a near-full 128k window, usedTokens 118000: remainingTokens = 1808, budget 7232 chars, but 7232 CJK chars cost ~10848 tokens, ~6x the 1808 available and 2600 tokens past the reserve the constant exists to protect. For that content class the guard under-delivers the guarantee its own comment states.
Witness:
Probe priced the finding's own replay (128k window, used 118000 -> remainingTokens 1808 -> budget 7232 chars) with the repo's own oracle `estimateContextTextTokens` (tokenEstimation.ts:45: `Math.ceil(asciiChars / 4 + nonAsciiChars * 1.5)`): 7232 CJK chars measure ~10848 tokens against 1808 available. The verifier also measured that the behaviour is a strict tightening for every content class (25000 -> 7232 chars) and that the branch never returns more than the configured threshold apart from R1-2's floor case.
Price the actual content rather than a flat ratio â bound by the densest ratio the repo already assumes (reuse estimateContextTextTokens from services/tokenEstimation.js on the candidate output and compare against remainingTokens), or at minimum convert with a conservative divisor and correct the comment at truncation.ts:65-67, which currently asserts the inverse of the arithmetic.
One existing fact this fix must not violate: packages/core/src/services/tokenEstimation.ts:45 â return Math.ceil(asciiChars / 4 + nonAsciiChars * 1.5);; packages/core/src/services/chatCompressionService.ts:162-165 â Math.ceil(nonCjkCharCount / CHARS_PER_TOKEN) + Math.ceil(cjkCharCount * CJK_CHAR_TOKEN_MULTIPLIER). The test that must go red if this fix is later removed: A truncation.test.ts case pinning the token price of the returned budget for CJK content â e.g. for a window whose remainingTokens is 1808, assert estimateContextTextTokens('äļ'.repeat(budget)) <= 1808. It fails at x4 (~10848 tokens) and passes once the estimate is density-aware.
â qwen3.8-max via Qwen Code /review (v0.25.2)
| * Returns the configured threshold unchanged when the window size is unknown | ||
| * or the window still has room, so normal turns are never affected. | ||
| */ | ||
| export function contextAwareTruncateThreshold( |
There was a problem hiding this comment.
[Suggestion] R1-22: The committed budget map at docs/design/2026-08-10-tool-output-offload-preview.md:95 and its Chinese twin at :83 both still describe the Global layer as a fixed 25k chars + 1000 lines and the Combined pass as 2x of the applicable budget. This diff makes the Global layer min(configured, (window - used - 8192) x 4) floored at 1000 chars, and makes "2x of the applicable budget" mean 2x a window-derived base. Neither language version is updated, so the AGENTS.md both-languages design-doc rule applies to the fix. Ruled under the "existing doc that is now WRONG" reading, not the "missing documentation" exclusion â AGENTS.md workflow rule 1 requires keeping both versions synchronized "including when updating an existing design".
That doc is the authoritative budget map for the offload pipeline: packages/core/src/skills/bundled/review/DESIGN.md:806 and SKILL.md:208 reason from the same numbers, and docs/users/configuration/settings.md:412 still describes the setting as an unconditional "Truncate tool output if it is larger than this many characters" with no window-dependent caveat. Cost: the next change to this pipeline reads a budget that no longer exists, so it either re-introduces the near-full-window overflow this diff fixes or tightens a second time on top of an already-tightened base; and a user who sets truncateToolOutputThreshold: 100000 gets silently less than the documented contract â R1-20's measurement shows 47232 in the near-full state â with no setting or log line to explain it.
Witness:
not run â a committed design-doc row's text is settled by reading it, not by a run, so no run capability applies; the nearest was a direct read of both language files at the cited lines. Read at docs/design/2026-08-10-tool-output-offload-preview.md:95 `| Global | 25k chars + 1000 lines | ` + "`settings.tools.truncateToolOutputThreshold`" + ` / ` + "`truncateToolOutputLines`" + ` |` with the next row `| Combined pass | 2x of the applicable budget | No |`, and docs/design/2026-08-10-tool-output-offload-preview.zh-CN.md:83 `| å
Ļåą | 25k åįŽĶ + 1000 čĄ | ... |` with the next row `| ååđķ pass | éįĻéĒįŪį 2 å | åĶ |`. The diff touches exactly three files (coreToolScheduler.ts, truncation.test.ts, truncation.ts) and no docs, confirming neither row was updated. Corroborating: docs/users/configuration/settings.md:412.
Update the "Global" row in BOTH docs/design/2026-08-10-tool-output-offload-preview.md and its .zh-CN.md twin to state the context-aware bound and the 1000-char floor, keeping the two versions' structure and numbers identical per the AGENTS.md both-languages rule; note in the "Combined pass" row that 2x now applies to a window-derived base (or that it should not â see R1-20); and add the window-dependent caveat at docs/users/configuration/settings.md:412.
One existing fact this fix must not violate: AGENTS.md ## Code Review â "Check design documentation in both languages. New or updated designs must include linked English and Chinese versions with matching structure, decisions, constraints, and acceptance criteria"; AGENTS.md ### General workflow rule 1 â keep both versions "complete and synchronized in the same change, including when updating an existing design".
â qwen3.8-max via Qwen Code /review (v0.25.2)
| export function contextAwareTruncateThreshold( | ||
| config: Config, | ||
| configuredThresholdChars: number, | ||
| reservedOutputTokens: number = CONTEXT_AWARE_OUTPUT_RESERVE_TOKENS, |
There was a problem hiding this comment.
[Suggestion] R1-15: The third parameter and the exported CONTEXT_AWARE_OUTPUT_RESERVE_TOKENS are never set by any production caller; the only non-default value ever supplied is a test's 0. AGENTS.md ## Code Review names this exactly: "For every added field, option, or optional parameter, grep its read sites... A foo?: boolean that is declared and read but never set by any caller is a dead switch." AGENTS.md ## Simplicity First adds: "No 'flexibility' or 'configurability' that wasn't requested. No abstractions for single-use code." The verifier ruled the wrong-valued-default half PART OF this finding â the dead switch and the disagreeing default live on the same parameter and one edit fixes both.
Repo-wide sweep: declaration at truncation.ts:79, sole read as the default at :107, sole production call at coreToolScheduler.ts:62 and :6430 (two arguments), sole 3-argument call at truncation.test.ts:662 (a test); zero hits in packages/cli, scripts/, integration-tests/ and docs/. The cost is a permanently exported core API surface plus a parameter branch whose sole consumer is it('honors a custom output reserve') â a test pinning behaviour nothing shipped exercises, so it can never catch a regression in real use and will keep the knob alive. The default is also wrong relative to the layer it protects: the send path holds back outputClampMargin(windowSize) = Math.max(10_000, Math.round(0.05 * contextWindowSize)) (tokenLimits.ts:56-58), i.e. >= 10_000, so 8192 is below the margin for EVERY window size. On a 128k window the helper sizes tool output against 1808 tokens of slack the send path will not grant, and the two layers disagree about the same window invisibly.
Witness:
Repo-wide sweep output quoted above (truncation.ts:79, :107; coreToolScheduler.ts:62, :6430; truncation.test.ts:662; zero hits elsewhere). Complementary probe: `outputClampMargin(128000) => 10000` printed beside `helper floor branch fires at used >= 119808 (window - 8192 reserve)` and `prompt=119808 -> max_tokens=4000 (prompt+max_tokens=123808 <= window: REQUEST VALID)` â the reserve the constant promises is already consumed by the clamp margin at the helper's own boundary.
Drop the third parameter and keep the reserve module-private (unexported) unless a second caller is landing in this PR; delete the honors a custom output reserve test with it. Take the reserve from a single authority â either the omni setting (see R1-14) or outputClampMargin(windowSize) â so the truncation layer and the send path cannot disagree about the same window.
One existing fact this fix must not violate: packages/core/src/core/tokenLimits.ts:56-58 â export function outputClampMargin(contextWindowSize: number): TokenCount { return Math.max(10_000, Math.round(0.05 * contextWindowSize)); }; packages/core/src/omni/policy/config.ts:43 â reservedOutputTokens: 8192 for the identical subtraction. The test that must go red if this fix is later removed: N/A â removing an unexercised parameter and export adds no branch a test can pin; truncation.test.ts still covers the four window cases.
â qwen3.8-max via Qwen Code /review (v0.25.2)
| return configuredThresholdChars; | ||
| } | ||
| const usedTokens = uiTelemetryService.getLastPromptTokenCount(); | ||
| const remainingTokens = windowSize - usedTokens - reservedOutputTokens; |
There was a problem hiding this comment.
[Suggestion] R1-14: The subtraction re-implements availableContextTokens â Math.max(0, contextWindowTokens - promptTokenCount - reservedOutputTokens) in packages/core/src/omni/policy/session-context.ts:80-83 â with the same 8192 default (omni/policy/config.ts:43), and differs in exactly the one respect that module documents as deliberate: it reads the process-global telemetry counter instead of the active chat's count. AGENTS.md's reuse rule requires naming the existing helper, which this finding does.
session-context.ts:33-36 states the count must come from the current chat, "never a global UI telemetry counter", and llm-chat.test.ts:11608-11624 pins that a subagent-style chat does not mirror to the global singleton. Because the arithmetic is duplicated rather than shared, an operator who raises omni.processing.limits.reservedOutputTokens to give the model more reply room gets no effect on tool-output sizing, and the two 8192 reserves diverge silently â the same class of drift R1-13 names for the char ratio.
Witness:
Probe run from a file under `src/tools/` in an isolated copy: `PROBE_NAMESPACE: {"reservedOutputTokens":8192,"contextWindowTokens":128000,"promptTokenCount":118000,"availableContextTokens":1808}` / `PROBE_CHARS: 7232` â identical to the new helper's own expectation at truncation.test.ts:656 (`.toBe(7232)`), produced by the existing helper instead. Importability was answered by execution, not reading: no cycle exists (session-context.ts's only import is `import type { SessionConditionField } from './conditions.js'`, type-only and erased), and the tools -> omni direction already has precedent in this tree.
Call the existing helper rather than re-deriving: buildSessionConditionNamespace(config, reservedOutputTokens), read .availableContextTokens, and multiply by CHARS_PER_TOKEN. Note the verifier's correction â it is NOT a drop-in: the helper returns a namespace record in TOKENS and requires the caller to supply the reserve, which today is only reachable through the normalized omni config (omni/index.ts:497), so reuse means plumbing that reserve through. If that plumbing is too invasive for this PR, at minimum take the reserve from a single authority so the two 8192s cannot diverge (see R1-15).
One existing fact this fix must not violate: packages/core/src/omni/policy/session-context.ts:32-36 â the count must be "the CURRENT chat's last reported prompt token count (GeminiChat.getLastPromptTokenCount()), per-chat by design ... never a global UI telemetry counter"; packages/core/src/omni/policy/config.ts:43 â reservedOutputTokens: 8192, reachable only through the normalized omni config (omni/index.ts:497). The test that must go red if this fix is later removed: A truncation.test.ts case asserting the helper's remaining-token value equals buildSessionConditionNamespace(...).availableContextTokens for the same window/used/reserve inputs, so a future retune of the omni reserve moves both. Red while the arithmetic is duplicated.
â qwen3.8-max via Qwen Code /review (v0.25.2)
| if (remainingChars >= configuredThresholdChars) { | ||
| return configuredThresholdChars; | ||
| } | ||
| return Math.max(CONTEXT_AWARE_MIN_CHARS, remainingChars); |
There was a problem hiding this comment.
[Suggestion] R1-12: The JSDoc at truncation.ts:98-99 and the PR description both state the full output is still persisted, so tightening loses nothing recoverable. That safety property does not hold on the spill-file failure path: when truncateAndSaveToFile cannot write, the preview is all that survives, and the new floor makes that preview 25x smaller than the budget it replaces.
truncateAndSaveToFile's write is not guaranteed: on mkdir/writeFile failure it returns truncatedContent + '\n[Note: Could not save full output to file]' with no outputFile (truncation.ts:294-300), so the preview IS all that survives. Because previewChars defaults to threshold (truncation.ts:331), a tightened threshold of 1000 means a read-only or full temp dir plus a near-full window reduces a 2MB tool result to ~1000 chars with no file to recover from â a 25x increase in unrecoverable loss over the previous 25000-char floor, in the state where re-running the tool is most expensive (the window is already full, so re-running may itself fail).
Witness:
Failure/control probe over `truncateAndSaveToFile` with the write forced to fail: the returned content carried the '[Note: Could not save full output to file]' suffix and no `outputFile`, at the tightened 1000-char budget rather than the previous 25000-char one. Read confirmation at truncation.ts:294-300 (catch path) and :331 (`previewChars` defaults to `threshold`).
Either tighten only once truncateAndSaveToFile reports an outputFile (so the floor applies only when the full output really was persisted), or scope the JSDoc and description claim to the case where persistence succeeded.
One existing fact this fix must not violate: packages/core/src/tools/truncation.ts:296-299 â the catch returns truncatedContent + \n[Note: Could not save full output to file]`` with no outputFile; packages/core/src/config/config.ts:968 â `export const DEFAULT_TRUNCATE_TOOL_OUTPUT_THRESHOLD = 25_000;` (the pre-diff floor this replaces). The test that must go red if this fix is later removed: A truncation.test.ts case with `mockWriteFile` rejecting and a near-full window asserting the returned content is not reduced to the 1000-char floor â or, if the behaviour is kept, that the doc claim is scoped. It fails against the current unconditional floor.
â qwen3.8-max via Qwen Code /review (v0.25.2)
This change makes the tool output size limit depend on the space that is left in the model context window.
Today the limit is a fixed number of characters. It does not look at how full the conversation already is. When the window is nearly full, a result that passes the fixed limit can still fill the window and leave the model too little room to answer. The reply is cut off before it says anything useful, and the run fails with a hard error instead of degrading gracefully.
The new helper works out how many characters still fit in the window, keeps a small reserve for the model reply, and cuts the result down to that size only when the fixed limit would be too much. It never cuts below a floor that keeps the preview useful, and it leaves the configured limit untouched when there is still room. The full output is still saved to a file, so nothing recoverable is lost. Per tool budgets that a tool declares for itself are left exactly as they are.
The change is small and self contained. It adds the helper in the truncation module, uses it where the scheduler measures tool output, and adds unit tests for the new logic. The core package type checks clean.