fix(core): guard against CJK-driven char/4 under-count in output clamp - #7963
Conversation
estimatePromptTokens's steady-state branch uses char/4 to estimate the size of newly-added content (e.g. tool results just appended to history). char/4 is a flat ratio that does not account for CJK text density, and two independent real production failures (both 2026-07-28, on KAT-Coder-V2.5-Dev and Nex-N2-mini) traced back to this: large CJK-dense tool results (parallel read_file calls returning design docs) were under-counted by 39-54%, so clampOutputTokensToWindow computed a maxOutputTokens that was too large, and prompt + max_tokens exceeded the window by ~1 token in production, triggering a 400 that propagates past reactive compression (which correctly detects it has no room left, but can't undo an already-oversized request). Add an opt-in `conservative` parameter to estimatePromptTokens that inflates only the new-content term by 1.5x (ceil'd) — calibrated against the two observed under-count ratios with headroom, applied only at the two call sites that feed clampOutputTokensToWindow (where under-counting is dangerous), left off at the cheap-gate/compaction-trigger call sites (where the existing code comments already document under-counting as safe — it only makes compaction trigger earlier, never skip it).
github-actions
Bot
added
the
review/self-reported
|
Thanks for the PR! Template: headings differ from the repo template (Summary/Fix/Verification vs What this PR does/Why it's needed/Reviewer Test Plan), but the content is thorough and covers everything the template asks for — not worth a round-trip. ✓ Problem: observed bug, well-evidenced. The linked issue (#7961) describes three independent reproductions of the ~1-token window overflow on CJK-heavy content, including one from a different CLI client (OpenCode) against the same backend. A maintainer confirmed the root cause on the issue thread. The code itself acknowledges the risk in the clampOutputTokensToWindow docstring ("a fresh chars/4 estimate under-counts CJK and tool-heavy prompts, which is the one way a residual 400 could return") — this PR closes that documented gap on the steady-state and recovery paths. Direction: aligned. This fixes a real 400-error class that the existing clamp was built to prevent (#5950). The first-send path already has FIRST_SEND_CLAMP_OVERHEAD_PAD = 20,000 for exactly this reason; the steady-state path lacked an equivalent. CHANGELOG: no direct reference, but the area (token management / output clamp) is clearly within scope. Size: 26 production lines (2 in geminiChat.ts, 24 in tokenEstimation.ts) + 80 test lines. Well under any threshold. Approach: the scope is tight — one new parameter with a safe default, applied at exactly the two call sites that feed maxOutputTokens to the backend. The two "cheap gate" compaction-trigger call sites and the chatCompressionService call site are correctly left unchanged. No unrelated changes, no drive-by refactors. The 1.5x factor is acknowledged as a starting point rather than a rigorously derived constant, which is honest — and the docstring documents the observed 39-54% under-count it covers. Risk: geminiChat.ts is a high-risk path (correlated with post-merge reverts in this repo's history). The change here is two lines adding a parameter to existing calls, so the blast radius is small, but I'm flagging it for reviewer awareness and requiring full CI evidence before approval. Moving on to code review. 🔍 中文说明感谢贡献！ 模板：标题与仓库模板不同（Summary/Fix/Verification vs What this PR does/Why it's needed/Reviewer Test Plan），但内容详尽，覆盖了模板要求的所有内容——不值得为此打回。✓ 问题：已观测到的 bug，证据充分。关联 issue（#7961）描述了三次独立的 CJK 密集内容窗口溢出复现，包括来自不同 CLI 客户端（OpenCode）的复现。维护者在 issue 线程中确认了根因。代码本身在 clampOutputTokensToWindow 文档字符串中已承认此风险——本 PR 在稳态和恢复路径上关闭了这个已记录的缺口。 方向：对齐。修复了现有 clamp 本应防止的真实 400 错误类别（#5950）。首次发送路径已有 FIRST_SEND_CLAMP_OVERHEAD_PAD = 20,000；稳态路径缺少等效填充。 规模：26 行生产代码 + 80 行测试代码。远低于任何阈值。 方案：范围精准——一个新参数（安全默认值），仅应用于向后台发送 maxOutputTokens 的两个调用点。两个"廉价门控"压缩触发调用点和 chatCompressionService 调用点正确地保持不变。无无关改动。1.5x 系数被诚实地标注为起点而非严格推导的常数。 风险：geminiChat.ts 是高风险路径（与本仓库历史中的合并后回退相关）。此处改动仅为两行参数添加，影响范围小，但标记以供审阅者注意，并在批准前要求完整 CI 证据。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: given the problem (char/4 under-counts CJK text in the output clamp path, causing ~1-token window overflows), I would add a conservative multiplier to the new-content estimate at the two clamp call sites, keep it backward compatible with a default-off parameter, and leave the compaction-trigger call sites unchanged (under-counting there is safe — it triggers compaction earlier). I'd put the constant in tokenEstimation.ts alongside the existing CHARS_PER_TOKEN. Comparison with the diff: the PR matches this almost exactly. The implementation is clean and minimal:
No correctness bugs, no security concerns, no convention violations found. The test changes are well-structured: the geminiChat.test.ts expectation bumps (19999→19998, 49722→49721) are correctly explained by the 1.5x inflation on tiny messages (char/4("hi") = 1 token → ceil(1 * 1.5) = 2), and the new conservative-mode test suite in tokenEstimation.test.ts covers the default-off behavior, the selective inflation, and a realistic CJK-heavy scenario. CI Test EvidenceUbuntu unit tests pass. macOS/Windows tests and integration tests were skipped (not failed) — this is the repo's normal CI routing for fork PRs.
Not verified: the author's E2E claim (27 turns / ~53.8 min / ~1.12M tokens, zero recurrence) is self-reported and not independently re-run here. The unit tests pin the mathematical correctness of the conservative mode (removing it would fail the new tests), but the end-to-end behavioral claim — that 1.5x is the right factor to prevent real 400s on CJK-heavy content without materially eating into the output budget — rests on the author's reproduction. Sandboxed verification would settle this: 中文说明独立方案： 给定问题（char/4 在输出 clamp 路径中低估 CJK 文本，导致约 1 token 的窗口溢出），我会在两个 clamp 调用点添加保守乘数，保持向后兼容（默认关闭），并不改动压缩触发调用点。 与 diff 的比较： PR 几乎完全匹配此方案。实现干净且最小化：conservative 参数默认 false，乘数仅应用于 new-content 项而非 API 权威计数，所有五个下游消费者均已核实。未发现正确性 bug、安全问题或规范违反。 CI 证据： Ubuntu 单元测试通过。macOS/Windows 测试和集成测试被跳过（非失败）——这是 fork PR 的正常 CI 路由。 未验证： 作者的 E2E 声明（27 轮 / ~53.8 分钟 / ~112 万 tokens，零复发）为自我报告，未在此独立重跑。单元测试固定了 conservative 模式的数学正确性，但端到端行为声明依赖作者的复现。沙箱验证可以解决此问题： — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean, minimal fix for a confirmed bug; the only reservation is that the 1.5x factor is empirically chosen rather than derived, and the E2E evidence is author-reported. This is exactly the kind of PR I like to see. The problem is real (three independent reproductions, maintainer-confirmed root cause), the fix is surgical (one parameter, two call sites, backward compatible), and the author was honest about what the 1.5x constant is — a starting point covering the observed 39-54% under-count, not a number pulled from a tokenizer study. The docstring on the constant is the right kind of comment: it explains why this number and not another, with references to the specific production failures. My independent proposal matched the PR's approach almost exactly. I looked for a simpler path and didn't find one — the alternative (a proper tokenizer-based estimate) would be a much larger change with its own dependency concerns, and the author already flags it as a possible follow-up. The diff carries nothing beyond what the stated goal needs. The geminiChat.ts high-risk path flag is noted, but the actual change there is two lines adding a parameter to existing calls — about as low-risk as a touch to that file gets. The unit tests pin the math correctly, and the existing test expectation bumps are well-explained. Non-blocking nits for the author's awareness: the 6th positional parameter on estimatePromptTokens is getting unwieldy — an options object might be worth considering in a follow-up if more parameters accumulate. And the 1.5x factor is worth revisiting if a lightweight tokenizer-based estimate becomes feasible. LGTM, approving. ✅ 中文说明置信度：4/5 —— 干净、最小化的修复，针对已确认的 bug；唯一保留意见是 1.5x 系数为经验选择而非推导得出，E2E 证据为作者自我报告。 这正是我希望看到的 PR。问题真实（三次独立复现，维护者确认根因），修复精准（一个参数，两个调用点，向后兼容），作者对 1.5x 常数的定位诚实——覆盖观测到的 39-54% 低估的起点，而非来自分词器研究的数字。 我的独立方案与 PR 方案几乎完全一致。我寻找了更简路径但未找到——替代方案（基于分词器的估算）会是更大的改动。diff 不包含超出目标所需的任何内容。 geminiChat.ts 高风险路径标记已注意，但实际改动仅为两行参数添加——对该文件来说风险最低。单元测试正确固定了数学逻辑。 非阻塞建议：estimatePromptTokens 的第 6 个位置参数开始变得笨拙——如果继续积累参数，可考虑在后续 PR 中使用选项对象。1.5x 系数在轻量级分词器估算可行时值得重新审视。 LGTM，批准。✅ — Qwen Code · qwen3.8-max-preview Reviewed at |
|
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
| /** | ||
| * Multiplier applied to the char/4 estimate of NEWLY-added content when | ||
| * `conservative` is set. char/4 is documented (see file header) as varying |
There was a problem hiding this comment.
[Suggestion] The JSDoc block for estimatePromptTokens is orphaned — CONSERVATIVE_NEW_CONTENT_SAFETY_FACTOR and its own JSDoc were inserted between the function's doc comment and its declaration. TypeScript attaches only the immediately preceding /** */ to a declaration, so estimatePromptTokens loses its JSDoc (including the important WARNING about using the estimate to trigger earlier, never to skip).
Move the constant declaration (with its JSDoc) above the function's JSDoc so the function's documentation stays adjacent to its declaration.
— qwen3.7-max via Qwen Code /review
| * reads) by 39-54% — beyond the documented band. 1.5x covers both observed | ||
| * cases with headroom without materially eating into the output budget for |
There was a problem hiding this comment.
[Suggestion] The comment claims 1.5× "covers both observed cases with headroom," but the documented upper bound is 54% under-count (real = char/4 × 1.54). The 1.5× factor leaves a ~2.6% residual gap on the new-content term — for a large CJK tool result (char/4 ≈ 10K tokens), this is ~400 tokens of overflow that the reactive handler catches but the proactive clamp doesn't prevent.
Consider bumping to 1.6× (covers 54% with headroom) or correcting the comment to name what covers the 54% case.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /verify |
|
Sandboxed verification not started — the PR author does not have write access to this repository, and 沙箱验证未启动 —— 该 PR 作者不具备本仓库写权限，而 |
Local maintainer verification — real build + real tokenizer
Verdict: merge-ready. The fix is real and load-bearing — it converts a reproducible window overflow into a fitting request, is monotonically safe (it can only ever shrink 1. End-to-end: does it actually stop the overflow?I built a harness that drives the real The
A is the money row: base sends a request that is 984 tokens over the window — the exact Auto-compaction is stubbed to 2. Is
|
| mutation | result |
|---|---|
| M0 pristine PR head | 278 passed |
| M1 PR tests vs. base source (both files reverted) | 4 failed — 2 new unit tests + both updated geminiChat expectations |
M2 remove only the steady-state call site (geminiChat.ts:2357) |
2 failed → killed ✅ |
M3 remove only the recovery-path call site (geminiChat.ts:3072) |
278 passed → survived |
The third new unit test (defaults to non-conservative) passes both ways — that is a correct regression guard, not a vacuous test.
M3 is a genuine coverage gap. Half of the production change is unverified: the one recovery-path assertion (geminiChat.test.ts:10368) expects 4_000, which is MIN_CLAMPED_OUTPUT_TOKENS — the floor is hit with and without the flag, so the assertion cannot distinguish them. countBasedRecoveryEstimate appears nowhere else in the repo, so no other suite covers it either. A recovery case sized so the clamp lands between the floor and the ceiling would close this.
4. Regressions, gates, scope
- Full
packages/coresuite, both arms: PR18132 passed / 6 failed, base18129 passed / 6 failed(the +3 is exactly the 3 new unit tests). The failing sets are byte-identical between the two arms — all 11 failing testcases are local-environment artifacts (turndownabsent from the donornode_modules,@qwen-code/qwen-code-coredist not built, and the knownmemoryLifecyclemachine artifact). Zero PR-attributable failures. eslintclean,prettier --checkclean on all 4 changed files.tsc --noEmitclean apart from the same environmentalturndownerror present on base.- Risk is one-directional.
Math.ceil(x * 1.5) ≥ x, so the estimate only ever grows andmaxOutputTokensonly ever shrinks, bounded below by the 4,000 floor. No configuration can produce a larger request than today, so this cannot introduce new 400s. The worst case is moreMAX_TOKENStruncation near the compaction threshold, which the existing escalation-on-truncation retry already handles. - Scope note (not a regression): two other estimates that feed
clampOutputTokensToWindoware still raw char/4 and are untouched by this PR — the first-send fallback (effectiveTokens + FIRST_SEND_CLAMP_OVERHEAD_PAD,geminiChat.ts:2359) andwalkRecoveryEstimate(geminiChat.ts:3081-3085, a whole-history walk, so the largest CJK exposure of the four). Scenario F shows a resumed CJK session overflowing identically on both arms via the first-send branch. The PR is correctly scoped to what Main-turn output-token clamp can under-count CJK-heavy new content by ~chars/4, occasionally overflowing the context window by a few tokens #7961 reports; I mention it so the bug class isn't assumed closed.
Recommendation
Merge. The fix is verified end-to-end on the real send path, monotonically safe, and regression-free.
Follow-ups, in priority order:
- In this PR (2-line comment edit): soften the
CONSERVATIVE_NEW_CONTENT_SAFETY_FACTORcomment — "1.5x covers both observed cases with headroom" is not supported by the real tokenizer. Something like "1.5× substantially raises the payload size required to overflow; it is not a full correction for CJK density" matches what the constant actually does. - Follow-up: a recovery-path test that isn't floor-bound, to kill the M3 mutant.
- Follow-up: density-aware estimation instead of a blanket factor, which fixes row B and refunds row D.
Reproduction notes
- Worktrees:
pr7963-verify@ff05f78869and detached @0c0ca5fed0;node_modulesAPFS-cloned from a known-good tree; everynode_modules/@qwen-code/*entry asserted torealpathinside its own worktree before any run (otherwise the "base" arm silently compiles PR source). - Harness: a
describeblock appended topackages/core/src/core/geminiChat.test.tsin both trees so the two arms run byte-identical test code against different production source. Run with--silent=false(this package setssilent: trueinvitest.config.ts, which swallowsconsole.log). - Tokenizer:
Qwen/Qwen3-8Btokenizer.json(11,422,654 bytes) +tokenizers0.22.2. Both fixtures verified exactly linear on repeat ('设计文档章节内容。' ×2000= 18,000 chars = 10,000 tokens; the English unit = 14.002 tokens/unit at n=600), sorealTokens = units × tokensPerUnitis exact, not interpolated. - The
functionResponseJSON wrapper adds a few more real tokens on top of the countedoutputstring; omitting it makes every verdict above conservative in the PR's favour.
中文版本
维护者本地验证 —— 真实构建 + 真实分词器
/verify 拒绝了本 PR（fork 作者无仓库写权限），因此我在本地完成了验证。验证对象为 head ff05f78869，对照 base 0c0ca5fed0，在两个各自独立 node_modules 的隔离 worktree 中进行。
结论：可以合并。 该修复是真实且起作用的——它把一个可复现的上下文窗口溢出请求变成了合规请求；变更方向单调安全（只会缩小 maxOutputTokens，绝不会放大）；在完整的 packages/core 测试套件中零回归。下面列出三项后续事项，其中第一项建议直接并入本 PR，因为它只是两行注释的修改。
1. 端到端：是否真的阻止了溢出？
我构建了一个测试装置，驱动真实的 GeminiChat.sendMessageStream 路径，输入一个 CJK 密集的 read_file 工具结果，捕获生产代码实际发到线上的 maxOutputTokens，然后校验钳位逻辑本应保证的不变式：
真实提示词 token 数 + maxOutputTokens ≤ 上下文窗口
其中"真实" token 数来自真实的 Qwen3 分词器（Qwen/Qwen3-8B 的 tokenizer.json，词表 151669，通过 tokenizers 0.22.2）——离线测得后内嵌，因为样本是确定性的且在重复时严格线性分词。钳位算术、估算值、以及发出的请求本身没有任何一处是模拟的。
| 场景 | base maxOut |
PR maxOut |
base | PR |
|---|---|---|---|---|
| A —— 36,000 CJK 字符，锚定 88K | 24,056 | 19,548 | 400（超 +984） | OK（−3,524）✅ |
| B —— 72,000 CJK 字符，锚定 70K | 32,000 | 24,048 | 400（+10,928） | 400（+2,976） |
| C —— 9,000 CJK 字符，锚定 100K | 18,806 | 17,673 | OK | OK |
| D —— 39,600 英文字符，锚定 88K | 23,156 | 18,198 | OK | OK（预算 −4,958） |
| E —— 极小消息（受上限约束） | 32,000 | 32,000 | OK | OK |
| F —— 首次发送 / 恢复会话，历史 198K CJK 字符 | 32,000 | 32,000 | 400（+11,428） | 400（+11,428） |
A 行是关键证据：base 发出的请求超出窗口 984 个 token——正是本钳位逻辑要防止的那类 prompt + max_tokens > window 400 错误——而 PR 使其合规。这是修复在真实发送路径上生效，而非纸面推演。
装置中自动压缩被打桩为 NOOP（与 PR 自带测试一致），以便单独测量钳位行为。
2. 1.5 这个常量取值合适吗？
CONSERVATIVE_NEW_CONTENT_SAFETY_FACTOR 的注释声称 char/4 对 CJK 低估 "39-54%"，且 "1.5x covers both observed cases with headroom"（1.5 倍有余量地覆盖了两个已观测案例）。我用真实分词器直接检验了这一说法：
- PR 自己的测试样本（
'设计文档章节内容。'.repeat(2000)）被低估 55.0%——char/4 给出 4,500 token，真实值是 10,000。要让估算成为上界，所需系数是 2.22×。在 1.5× 下，估算仍然少了 3,250 个 token。 - 真实中文技术文档：需要 2.30×；日文 2.64×；韩文 2.80×。
- 注释里的 "54%" 无论按哪种读法——
est = 0.46 × real（需 2.17×）或real = 1.54 × est（需 1.54×）——1.5× 都无法"有余量地覆盖"。该注释夸大了这个常量的实际作用。 - 反过来，char/4 对英文本就是高估的（0.87–0.93×），所以一刀切的 1.5× 会让英文估算比真实值高出约 72%。
这正是 B 行仍然 400、而 D 行白白损失 4,958 个输出预算 token（−21%）的原因。
我建议不要简单调大该常量——那只会加重对英文的惩罚。按字符密度区分的估算既更贴合，对 ASCII 又完全无损。把 CJK 码点按约 1.4 字符/token 计费（实测：中文 1.73、日文 1.50、韩文 1.42），在同一语料上得到真实值的 0.80×–1.29×，而 char/4 × 1.5 是 0.54×–1.73×；同时英文保持与今天完全一致：
// 约 5 行，无新依赖，对纯 ASCII 内容零影响
const cjk = countCjkCodepoints(text);
return Math.ceil(cjk / 1.4 + (text.length - cjk) / 4);这个可以作为后续项，不阻塞本 PR。
3. 测试是否真正起作用？
| 变异 | 结果 |
|---|---|
| M0 原始 PR head | 278 通过 |
| M1 PR 的测试跑在 base 源码上（两个源文件都还原） | 4 个失败 —— 2 个新增单测 + 2 个被更新的 geminiChat 断言 |
M2 只移除稳态调用点（geminiChat.ts:2357） |
2 个失败 → 已杀死 ✅ |
M3 只移除恢复路径调用点（geminiChat.ts:3072） |
278 通过 → 存活 |
第三个新增单测（defaults to non-conservative）两边都通过——这是正确的回归护栏，不是空测试。
M3 是真实的覆盖缺口。 生产变更有一半未被验证：唯一的恢复路径断言（geminiChat.test.ts:10368）期望值是 4_000，即 MIN_CLAMPED_OUTPUT_TOKENS 下限——加不加该标志都会触到下限，因此断言无法区分两者。countBasedRecoveryEstimate 在仓库其他地方均未出现，所以也没有别的套件覆盖它。补一个让钳位结果落在下限与上限之间的恢复场景即可闭合。
4. 回归、门禁与范围
- 完整
packages/core套件，两侧对比： PR18132 通过 / 6 失败，base18129 通过 / 6 失败（相差的 3 个正好是新增的 3 个单测）。两侧的失败集合完全一致——全部 11 个失败用例均为本地环境产物（供体node_modules缺turndown、@qwen-code/qwen-code-core的 dist 未构建，以及已知的memoryLifecycle机器产物）。没有任何可归因于 PR 的失败。 - 4 个改动文件的
eslint与prettier --check均通过。tsc --noEmit除了 base 上同样存在的环境性turndown报错外无其他问题。 - 风险是单向的。
Math.ceil(x * 1.5) ≥ x，因此估算只增不减、maxOutputTokens只减不增，且下有 4,000 的地板兜底。任何配置都不可能产生比今天更大的请求，所以不可能引入新的 400。最坏情况是在接近压缩阈值时更容易触发MAX_TOKENS截断，而现有的截断升级重试已经覆盖了这一点。 - 范围说明（非回归）： 还有另外两处喂给
clampOutputTokensToWindow的估算仍是裸 char/4，本 PR 未触及——首次发送兜底（effectiveTokens + FIRST_SEND_CLAMP_OVERHEAD_PAD，geminiChat.ts:2359）以及walkRecoveryEstimate（geminiChat.ts:3081-3085，它遍历整个历史，因此是四处中 CJK 暴露面最大的一处）。场景 F 展示了一个恢复的 CJK 会话通过首次发送分支在两侧同样溢出。本 PR 的范围与 Main-turn output-token clamp can under-count CJK-heavy new content by ~chars/4, occasionally overflowing the context window by a few tokens #7961 所报告的问题是吻合的；我在此指出只是为了避免误以为这一类缺陷已经彻底关闭。
建议
合并。 修复已在真实发送路径上端到端验证，方向单调安全，且无回归。
后续事项，按优先级：
- 在本 PR 内（改 2 行注释）： 弱化
CONSERVATIVE_NEW_CONTENT_SAFETY_FACTOR的注释——"1.5x covers both observed cases with headroom" 得不到真实分词器的支持。改成类似 "1.5× 大幅提高了触发溢出所需的载荷体量；它并不是对 CJK 密度的完整修正" 更符合该常量的实际作用。 - 后续： 补一个不受下限约束的恢复路径测试，杀死 M3 变异体。
- 后续： 用按密度区分的估算替代一刀切系数，可修好 B 行并把 D 行的预算还回去。
复现说明
- worktree：
pr7963-verify@ff05f78869与 detached @0c0ca5fed0；node_modules由已知健康的树 APFS 克隆而来；每个node_modules/@qwen-code/*条目在任何运行前都断言其realpath落在各自 worktree 内（否则 "base" 一侧会悄悄编译 PR 的源码）。 - 装置：把一个
describe块追加到两棵树各自的packages/core/src/core/geminiChat.test.ts，使两侧运行逐字节相同的测试代码、面向不同的生产源码。运行时需加--silent=false（该包在vitest.config.ts中设了silent: true，会吞掉console.log）。 - 分词器：
Qwen/Qwen3-8B的tokenizer.json（11,422,654 字节）+tokenizers0.22.2。两个样本均已验证在重复时严格线性（'设计文档章节内容。' ×2000= 18,000 字符 = 10,000 token；英文单元在 n=600 时为 14.002 token/单元），因此realTokens = units × tokensPerUnit是精确值而非插值。 functionResponse的 JSON 外壳会在所统计的output字符串之外再增加若干真实 token；忽略它使上述所有判定都偏向于对 PR 有利的保守方向。
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.2. |
4 participants
Closes #7961.
Summary
clampOutputTokensToWindow's steady-state and recovery-path branches compute the remaining output budget usingestimatePromptTokens(...), which estimates new content (not yet covered by the API's own authoritativelastPromptTokenCount) with a flatchars / 4heuristic. That heuristic under-counts CJK-dense text — a single CJK character is very often close to one token by itself, nowhere near 4 chars/token — so when the new content since the last confirmed count is CJK-heavy, the computed remaining budget comes out slightly too generous, and the real request can end up a handful of tokens over the window, triggering the same class of400the clamp is meant to prevent.Fix
conservativeparameter (defaultfalse, backward compatible) toestimatePromptTokens(). Whentrue, only the new-content term (estimateContentTokens([userMessage])) is inflated byCONSERVATIVE_NEW_CONTENT_SAFETY_FACTOR = 1.5— the API-authoritativelastPromptTokenCount/lastOutputTokenCountterms are left untouched, since they don't need padding.conservative: trueat the two call sites that feedclampOutputTokensToWindow(the steady-statepromptTokensForClampcomputation and the recovery-pathcountBasedRecoveryEstimate). Left the two earlier "cheap gate / should we compact at all" call sites unchanged — under-counting there is already documented as safe (it only makes compaction trigger a little earlier, never skips it).Verification
tokenEstimation.test.ts: added aconservative modetest suite (3 tests), including one reproducing the real CJK-undercount scenario with a synthetic Chinese-heavyfunctionResponse.geminiChat.test.ts: 2 existing tests' expectedmaxOutputTokensbumped by 1 (19999→19998, 49722→49721) to reflect the inflation on tiny messages (e.g. "hi"), with comments explaining why.Screenshot / Demo
N/A — no user-facing change. This adjusts an internal token-budget safety margin (
estimatePromptTokens's conservative mode); there is no new command, flag, or visible CLI behavior to demonstrate. See the Verification section above for the reproduction cases and updated test expectations.Notes for reviewers