fix(core): send thinkingBudget, not thinkingLevel, to Gemini 2.5 - #9094
fix(core): send thinkingBudget, not thinkingLevel, to Gemini 2.5#9094axiom-of-choice wants to merge 4 commits into
Conversation
Gemini 2.5 rejects thinkingLevel outright, so every request from the Gemini/Vertex generator 400s on that family: with no effort set the default sent THINKING_LEVEL_UNSPECIFIED, and /effort sent LOW/MEDIUM/HIGH. Only reasoning: false escaped. Map the effort tier to thinkingConfig.thinkingBudget for 2.5 and keep thinkingLevel for Gemini 3, which is the split the unified-effort design doc already specifies. Drop THINKING_LEVEL_UNSPECIFIED from the no-effort default: it means the model decides, which is what omitting the field does. Fixes QwenLM#9019
github-actions
Bot
added
the
review/self-reported
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Thanks @axiom-of-choice — the write-up itself is genuinely thorough, but the PR body doesn't follow the PR template, so the admission gate stops here, before code review.
What's missing:
- The required headings:
## What this PR does,## Why it's needed,## Reviewer Test Plan(with### How to verify,### Evidence (Before & After),### Tested on),## Risk & Scope, and## Linked Issues - The
Tested onOS table - The bilingual
<details>Chinese translation of the body
The good news: everything the template asks for is already in your description — the live 400 reproduction from #9019, the budget table, the verification steps, and the deliberately excluded scope. Please reorganize that same content under the template headings (one long line per paragraph, no hard wrapping), add the OS table and the Chinese <details> section, then re-request review. A maintainer can also re-trigger with @qwen-code /triage once the body is updated.
中文说明
感谢 @axiom-of-choice——PR 描述的内容本身很扎实，但没有按照 PR 模板 组织，所以准入检查在代码审查之前先停在这里。
缺少的部分：
- 必需标题：
## What this PR does、## Why it's needed、## Reviewer Test Plan（含### How to verify、### Evidence (Before & After)、### Tested on）、## Risk & Scope、## Linked Issues Tested on操作系统表格- 正文的双语
<details>中文翻译
好消息是：模板要求的内容在你的描述里都已经有了——#9019 中的 400 实际复现、budget 映射表、验证步骤、以及刻意排除的范围。请把这些内容重新组织到模板标题下（每段写成一整行，不要硬换行），补上操作系统表格和中文 <details> 部分，然后重新请求审查。正文更新后，维护者也可以用 @qwen-code /triage 重新触发。
— Qwen Code · qwen3.8-max
|
Body reorganized under the template headings, with the OS table and the Chinese translation. Same content as before, nothing dropped. @qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. 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.
Not explored to full depth (tool budget reached): "This PR fixes a bug where every Gemini 2.5 model request…": verifying the ceiling constants (32768 Pro / 24576 non-Pro) against authoritative Google documentation — two fetch attempts found no numeric budget tables and …; "This PR fixes a bug where every Gemini 2.5 model request…": full repo-wide test/typecheck — only packages/core was typechecked and only the touched test file was run.; "This PR fixes a bug where every Gemini 2.5 model request…": none — all checks I started completed within the tool budget.; "This PR fixes a bug where every Gemini 2.5 model request…": none — all planned checks completed within budget..
Test Plan (not a blocker): 28 passing — this review observed 19842, 1364, 19466, 1539, 494, 3485, 559 passed.
— qwen3.8-max via Qwen Code /review (v0.21.11)
| case 'xhigh': | ||
| case 'max': | ||
| // Gemini has no tier above HIGH; log the clamp once (mirroring the |
There was a problem hiding this comment.
[Suggestion] The hand-rolled xhigh/max→HIGH clamp and warn-once latch duplicate the shared clampReasoningEffort helper (packages/core/src/core/reasoning-effort.ts:89) — the Anthropic generator already uses it for the identical job (anthropicContentGenerator.ts:976, warn-once latch at 977-984), and the design doc this PR cites says the central rank-based clamp "replaces the ad-hoc per-adapter clamps". reasoning-effort.test.ts:70-74 already codifies exactly this Gemini scenario (['low','medium','high'], xhigh→high, max→high).
Failure scenario: clampReasoningEffort(effort, ['low','medium','high']) produces byte-for-byte this branch's mapping (low/medium/high pass through; xhigh/max fall back to 'high'). As written, the clamp policy lives in two places: a ladder change (new tier, rank change) must be re-derived by hand in this switch while the central helper's tests cover only the one copy, so the two generators' clamp behavior can drift.
const clamped = clampReasoningEffort(reasoning.effort, ['low', 'medium', 'high']);
if (clamped !== reasoning.effort && !this.effortClampWarned) {
debugLogger.warn(
`reasoning.effort='${reasoning.effort}' is not supported by Gemini; clamping to 'HIGH'.`,
);
this.effortClampWarned = true;
}
const thinkingLevel = { low: 'LOW', medium: 'MEDIUM', high: 'HIGH' }[clamped] as ThinkingLevel;(the fix spans the whole switch, so no one-click suggestion block)
— qwen3.8-max via Qwen Code /review (v0.21.11)
| it('omits both knobs when no effort is configured', async () => { | ||
| const gen = generatorFor('gemini-2.5-pro'); |
There was a problem hiding this comment.
[Suggestion] The newly added reasoning.effort === undefined branch (reasoning object present but no effort) has no test coverage: this test — despite its name — constructs the generator with no reasoning object at all, so it exercises the earlier !reasoning → getParameterValue default path instead.
Failure scenario: ContentGeneratorConfig.reasoning is false | { effort?: ReasoningEffort; budget_tokens?: number }, so an effort-less reasoning object ({} or { budget_tokens: 1024 }) is a representable input. Mutation probe: with the if (reasoning.effort === undefined) return { includeThoughts: true }; early return deleted, { model: 'gemini-2.5-pro', reasoning: {} } silently sends { includeThoughts: true, thinkingBudget: 16384 } (non-2.5: thinkingLevel: 'HIGH') instead of letting the model pick its own default — and the full 25-test suite still passes, so that mutant ships green.
Add a case that constructs the generator with a reasoning object lacking effort:
it('omits both knobs when reasoning is set without an effort', async () => {
const gen = generatorFor('gemini-2.5-pro', {});
await gen.generateContent(
{ model: 'gemini-2.5-pro', contents: [] },
'prompt-id',
);
expect(sentThinkingConfig()).toEqual({ includeThoughts: true });
});— qwen3.8-max via Qwen Code /review (v0.21.11)
The hand-rolled xhigh/max clamp duplicated the Anthropic generator's use of the same helper. Switching to it keeps clamp policy in one place so a ladder change doesn't need re-deriving per adapter.
The existing "no effort configured" test passed reasoning: undefined,
which exercises the earlier !reasoning branch, not the new
reasoning.effort === undefined branch added for a reasoning object
present without an effort (e.g. {} or { budget_tokens }).
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — all checks I started were completed within budget.; "PR #9094 changes the Gemini content generator to send…": did not run npm run typecheck repo-wide (verified via test run + source reads instead).; "PR #9094 changes the Gemini content generator to send…": verifying whether the live Gemini API still rejects thinkingBudget >= maxOutputTokens (docs fetches on ai.google.dev no longer document the budget constraint; g….
Test Plan (not a blocker): 28 passing — this review observed 1364, 19469, 1539, 494, 3485, 559 passed.
中文说明
未探索到全部深度（达到工具调用预算）："You are review agent reverse-audit — Reverse audit agent…"：none — all checks I started were completed within budget.；"PR #9094 changes the Gemini content generator to send…"：did not run npm run typecheck repo-wide (verified via test run + source reads instead).；"PR #9094 changes the Gemini content generator to send…"：verifying whether the live Gemini API still rejects thinkingBudget >= maxOutputTokens (docs fetches on ai.google.dev no longer document the budget constraint; g…。
Test Plan（非阻断）：28 passing — this review observed 1364, 19469, 1539, 494, 3485, 559 passed。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const clamped = clampReasoningEffort(reasoning.effort, [ | ||
| 'low', | ||
| 'medium', | ||
| 'high', | ||
| ]); |
There was a problem hiding this comment.
[Critical] R2-1: gemini-3-pro-preview supports only the low/high thinking levels, but this shared ladder sends MEDIUM whenever effort === 'medium' — the same failure class this PR eliminates for Gemini 2.5. Google's thinking docs list per-model supported levels (gemini-3-pro-preview: low, high; Gemini 3 Flash: minimal/low/medium/high), and the repo's own design doc records the same split (Gemini 3 Pro → LOW/HIGH). Verified by probe at the reviewed commit: effort: 'medium' + gemini-3-pro-preview emits thinkingConfig: { includeThoughts: true, thinkingLevel: 'MEDIUM' }, and the retargeted test maps reasoning effort 'medium' to MEDIUM now asserts that unsupported value against exactly this model. The mapping predates this PR, but the PR rewrites this exact function and re-asserts the mapping in the test. — Failure scenario: a user sets /effort medium (or has persisted model.reasoningEffort: medium) on gemini-3-pro-preview → every request carries thinkingLevel: 'MEDIUM' → requests are rejected and the model is unusable at medium effort, reproducing for Gemini 3 the very 400 this PR fixes for 2.5.
Suggested fix: make the ladder per-model — clamp Pro-family Gemini 3 models with ['low', 'high'] (the rank-based clamp then maps medium/xhigh/max → high), keep ['low', 'medium', 'high'] for Gemini 3 Flash / other level-style models, and update the retargeted medium test accordingly.
中文说明
[Critical] R2-1：gemini-3-pro-preview 只支持 low/high 两个 thinking 档位，但这个共享梯级在 effort === 'medium' 时仍会发送 MEDIUM——这与本 PR 要为 Gemini 2.5 消除的失败属于同一类。Google 的 thinking 文档按模型列出了支持的档位（gemini-3-pro-preview：low、high；Gemini 3 Flash：minimal/low/medium/high），仓库自己的设计文档也记录了同样的划分（Gemini 3 Pro → LOW/HIGH）。已在被审 commit 上用探针验证：effort: 'medium' + gemini-3-pro-preview 会发出 thinkingConfig: { includeThoughts: true, thinkingLevel: 'MEDIUM' }，而被重新定向的测试 maps reasoning effort 'medium' to MEDIUM 现在恰恰针对这个模型断言了这个不被支持的值。该映射在本 PR 之前就已存在，但本 PR 重写了这个函数本身，并在测试中重新确认了这一映射。— 失败场景：用户在 gemini-3-pro-preview 上设置 /effort medium（或已持久化 model.reasoningEffort: medium）→ 每个请求都会带上 thinkingLevel: 'MEDIUM' → 请求被拒绝，该模型在 medium 档位下完全不可用，等于在 Gemini 3 上复现了本 PR 要为 2.5 修复的那个 400。
建议修复：把梯级改为按模型区分——对 Gemini 3 Pro 系列用 ['low', 'high'] 做收敛（基于 rank 的收敛会把 medium/xhigh/max 映射到 high），对 Gemini 3 Flash 及其他 level 风格模型保留 ['low', 'medium', 'high']，并相应更新被重新定向的 medium 测试。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const BUDGET_STYLE_MODEL = /gemini-2\.5/; | ||
| const BUDGET_STYLE_PRO = /gemini-2\.5-pro/; |
There was a problem hiding this comment.
[Suggestion] R2-2: The user-facing per-provider reasoning table (docs/users/configuration/model-providers.md, ~line 592) still documents thinkingLevel-only wire behaviour for all Gemini models — thinkingConfig: { includeThoughts: true, thinkingLevel }, the tier→level mapping, and others → THINKING_LEVEL_UNSPECIFIED — all three claims are now wrong for the 2.5 family, which sends thinkingBudget buckets (2048/8192/16384, xhigh/max → 32768 Pro / 24576 other), never sends thinkingLevel, and sends no knob when effort is unset. The table's sibling rows describe current wire behaviour accurately — that is the house convention for it. Also stale: the ContentGeneratorConfig.reasoning doc comment (packages/core/src/core/contentGenerator.ts, ~line 152, "Gemini caps at 'high'"), since 2.5's xhigh/max now map to budget ceilings instead of capping. Verified verbatim at the reviewed commit; this diff updates neither. — Concrete cost: a user configuring reasoning.effort for a gemini-2.5-* model reasons about what is sent from false premises (e.g. expects thinkingLevel: HIGH for /effort max when the wire actually carries thinkingBudget: 32768), producing misdiagnosis and bug reports against behaviour the docs promise but no longer exists.
Suggested fix: update the Gemini row (or split it by family): Gemini 3 → thinkingLevel with low→LOW, medium→MEDIUM, high/xhigh/max→HIGH (one-time clamp warning); Gemini 2.5 → thinkingBudget with low→2048, medium→8192, high→16384, xhigh/max→32768 (Pro) / 24576 (other 2.5); effort unset → neither knob. Add the 2.5 nuance to the reasoning field comment.
中文说明
[Suggestion] R2-2：面向用户的 per-provider reasoning 表格（docs/users/configuration/model-providers.md，约第 592 行）仍然把所有 Gemini 模型的线上行为描述为只发 thinkingLevel——thinkingConfig: { includeThoughts: true, thinkingLevel }、档位→level 映射、以及 others → THINKING_LEVEL_UNSPECIFIED——对 2.5 系列来说，这三条现在全都错了：2.5 系列发送的是 thinkingBudget 分桶（2048/8192/16384，xhigh/max → Pro 为 32768 / 其他为 24576），从不发送 thinkingLevel，未设置 effort 时两个旋钮都不发送。该表格的其他行都准确描述了当前的线上行为——这是这张表的既有约定。同样过时的还有 ContentGeneratorConfig.reasoning 的文档注释（packages/core/src/core/contentGenerator.ts，约第 152 行，"Gemini caps at 'high'"），因为 2.5 的 xhigh/max 现在映射到 budget 上限而不是被压到 high。已在被审 commit 上逐字核实；本 diff 两处都没有更新。— 具体代价：为 gemini-2.5-* 模型配置 reasoning.effort 的用户会基于错误的前提推断实际发送的内容（例如以为 /effort max 会发 thinkingLevel: HIGH，而线上实际带的是 thinkingBudget: 32768），导致误诊断，以及针对文档承诺但已不存在的行为提交 bug 报告。
建议修复：更新 Gemini 这一行（或按系列拆成两行）：Gemini 3 → thinkingLevel，low→LOW、medium→MEDIUM、high/xhigh/max→HIGH（一次性收敛警告）；Gemini 2.5 → thinkingBudget，low→2048、medium→8192、high→16384、xhigh/max→32768（Pro）/ 24576（其他 2.5）；未设置 effort → 两个旋钮都不发。同时在 reasoning 字段的注释里补上 2.5 的差异说明。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const thinkingLevel = ( | ||
| { | ||
| low: 'LOW', |
There was a problem hiding this comment.
[Suggestion] R2-3: The new thinkingLevel record's low: 'LOW' entry is never exercised — no test drives a level-style (non-2.5) model with effort: 'low'; the only 'low' case in the suite is the 2.5 budget path (['low', 2048] in the it.each). Verified by mutation probe: changing low: 'LOW' to low: 'MEDIUM' leaves all 26 tests green, while a temporary probe test (effort 'low' on gemini-3-pro-preview) fails under the mutation and passes against the real code — so the entry is genuinely unexercised, and the probe distinguishes. — Failure scenario: a typo or future regression in the low entry ships with every test green — a user running /effort low against a Gemini 3 model would silently send thinkingLevel: 'MEDIUM' (more thinking tokens and cost than requested), and no test would catch it.
Suggested fix: add one level-path case — e.g. a gemini-3-pro-preview generator with reasoning: { effort: 'low' } asserting thinkingConfig: { includeThoughts: true, thinkingLevel: 'LOW' }.
中文说明
[Suggestion] R2-3：新的 thinkingLevel 映射表中 low: 'LOW' 这一项从未被执行到——没有任何测试用 level 风格（非 2.5）模型跑 effort: 'low'；测试套件里唯一的 'low' 用例是 2.5 的 budget 路径（it.each 里的 ['low', 2048]）。已通过变异探针核实：把 low: 'LOW' 改成 low: 'MEDIUM' 后全部 26 个测试仍然通过，而一个临时探针测试（在 gemini-3-pro-preview 上跑 effort 'low'）在该变异下会失败、在真实代码下会通过——说明这一项确实没有被覆盖，且探针本身是有效的。— 失败场景：low 项里的笔误或未来的回归会在所有测试全绿的情况下溜出去——在 Gemini 3 模型上执行 /effort low 的用户会悄悄发出 thinkingLevel: 'MEDIUM'（比用户要求的消耗更多 thinking token 和成本），而没有任何测试能发现。
建议修复：补一个 level 路径的用例——例如用 gemini-3-pro-preview 构造 generator 并设置 reasoning: { effort: 'low' }，断言 thinkingConfig: { includeThoughts: true, thinkingLevel: 'LOW' }。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| this.buildThinkingConfig(request.model), | ||
| 'thinkingConfig', |
There was a problem hiding this comment.
[Suggestion] R2-4: When a config-level reasoning effort is active, buildThinkingConfig returns a defined value, and getParameterValue's config-over-request precedence discards explicit request-level thinkingConfig opt-outs — verified by probe: config reasoning: { effort: 'high' } + request-level { thinkingBudget: 0, includeThoughts: false } on gemini-2.5-pro ships { includeThoughts: true, thinkingBudget: 16384 }. This affects the four internal sites that pass thinkingBudget: 0 (acp-integration/generation.ts, goals/goalJudge.ts, goals/goal-verifier.ts, goals/goal-checkpoint-verifier.ts): pre-PR the same input failed loudly (400 thinking_level); post-PR it succeeds with thinking enabled — a visible failure becomes a silent override on latency-sensitive paths. The Risk & Scope section deliberately defers this (and reasoning: false not stopping 2.5 thinking), but no open issue tracks either deferral — a search finds only #9019, which this PR closes. — Concrete cost: with /effort set, the ACP fast-generation path's explicit opt-out is overridden and its cheap completion runs with thinking enabled (budget up to 32768) — added latency and token cost on the path that asked for none — and after merge nothing tracks the deferred work once the PR body scrolls out of view.
Suggested fix: file a follow-up issue capturing both deferred items (the precedence clobber of the four budget-0 sites — including the 2.5 Pro 128-minimum wrinkle noted in the issue thread — and reasoning: false not stopping 2.5 thinking) and reference it in Risk & Scope; or, if config-wins is the intended policy, document it in buildThinkingConfig so the next maintainer doesn't discover the discard by debugging a slow fast path.
中文说明
[Suggestion] R2-4：当配置级 reasoning effort 生效时，buildThinkingConfig 返回一个有定义的值，而 getParameterValue 的"配置优先于请求"优先级会丢弃请求级显式传入的 thinkingConfig 退出开关——已用探针验证：配置 reasoning: { effort: 'high' } + 请求级 { thinkingBudget: 0, includeThoughts: false }，在 gemini-2.5-pro 上实际发出 { includeThoughts: true, thinkingBudget: 16384 }。受影响的是四个传 thinkingBudget: 0 的内部调用点（acp-integration/generation.ts、goals/goalJudge.ts、goals/goal-verifier.ts、goals/goal-checkpoint-verifier.ts）：PR 之前同样的输入会响亮地失败（400 thinking_level）；PR 之后请求成功但 thinking 被开启——一个可见的失败变成了对延迟敏感路径上的静默覆盖。Risk & Scope 部分有意推迟了这一问题（以及 reasoning: false 无法停止 2.5 thinking 的问题），但目前没有任何 open issue 跟踪这两个被推迟的事项——搜索只能找到 #9019，而本 PR 会将其关闭。— 具体代价：设置了 /effort 之后，ACP 快速生成路径显式的退出开关被覆盖，本应廉价的补全会带着 thinking 运行（budget 最高可达 32768）——在一个明确要求不思考的路径上平添延迟和 token 成本；而且合并之后，一旦 PR 描述淡出视野，就没有任何东西继续跟踪这些被推迟的工作。
建议修复：开一个后续 issue 记录这两个被推迟的事项（四个 budget-0 调用点被优先级覆盖的问题——包括 issue 讨论中提到的 2.5 Pro 最小 128 的细节——以及 reasoning: false 无法停止 2.5 thinking 的问题），并在 Risk & Scope 中引用它；或者，如果"配置优先"就是预期策略，请在 buildThinkingConfig 中写明，避免下一位维护者靠排查一条变慢的快速路径才发现这个覆盖。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if (clamped !== reasoning.effort && !this.effortClampWarned) { | ||
| debugLogger.warn( |
There was a problem hiding this comment.
[Suggestion] R2-5: The effort-clamp warning guard — including the warn-once effortClampWarned latch — has no test pinning when it must fire: the test-efficacy probe (harness validated by positive control; 3/3 hunk probes killed) forced the condition true and all 26 tests stayed green (mutant-survived). The clamping mapping is gated (its hunk was killed), but the diagnostic path is not. — Failure scenario: a future edit that breaks this condition — inverting the comparison so in-range efforts (low/medium/high) warn spuriously, or dropping the latch so every request re-logs — ships with every test green, silently corrupting the only signal users get that their configured reasoning.effort was downgraded for Gemini 3 models.
Suggested fix: add a test asserting debugLogger.warn is called once when effort is clamped (e.g. xhigh → HIGH), is NOT called for an in-range effort (low/medium/high), and is not called a second time for a repeat clamped request (the latch).
中文说明
[Suggestion] R2-5：effort 收敛的警告守卫——包括只警告一次的 effortClampWarned 闩锁——没有任何测试固定它何时必须触发：测试有效性探针（harness 已通过正向对照验证；3/3 的 hunk 探针被杀死）把该条件强制为真后，全部 26 个测试依旧全绿（mutant-survived）。收敛映射本身是有测试把关的（对应 hunk 被杀死），但这条诊断路径没有。— 失败场景：未来某个破坏该条件的改动——比如把比较写反，导致档位内的 effort（low/medium/high）也误发警告；或者丢掉闩锁，导致每个请求都重复打日志——会在所有测试全绿的情况下溜出去，悄悄破坏用户得知自己配置的 reasoning.effort 在 Gemini 3 模型上被降档的唯一信号。
建议修复：补一个测试，断言 effort 被收敛时（例如 xhigh → HIGH）debugLogger.warn 恰好被调用一次；对档位内的 effort（low/medium/high）不被调用；对重复的收敛请求不再第二次调用（闩锁生效）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
…#9175) * fix(review): repair seven pipeline defects found by live runs Four full reviews (PRs QwenLM#9113, QwenLM#9094, QwenLM#9109, QwenLM#9106) were run headless against qwen3.8-max and watched step by step. Every one of them reached a verdict, and every one of them also exposed a defect in the pipeline itself. Each fix below carries the measurement that found it. The incremental anchor was being withheld for a reason that says nothing about which lines were read. A dimension nobody can run — on this repo, the integration suite CI skips and the local budget cannot fit — capped the verdict, the cap withheld the anchor, and the missing anchor sent the next round over the full diff again: 119 minutes and 34M input tokens on a pull request whose code had not changed a line. The anchor now answers to the coverage evidence alone, recomputed from the harness's own transcripts, so a depth gap no longer costs a range. Two of the four runs had capped and two had not, on identical mechanical facts, which is the second reason this could not stay a judgment call. The build-and-test dimension could not finish, and no amount of reallocation would have made it. One shell call is capped at ten minutes by the tool itself; this repo needs more than that (install, the scoped builds, then a core suite at 106s and a CLI suite measured at 401s). Since the ceiling is per call, the run now continues across calls: a resumed call reuses the installed and compiled tree, runs only the suites the previous call could not reach, and merges into the same report. A suite killed on a deadline the budget had shortened is recorded as provisional rather than as a timeout, so the continuation knows to give it a full window instead of reproducing the kill. The per-command deadline is sized to the slowest measured suite, and the whole-call budget is now derived from the tool's ceiling rather than from the deadline. A machine ledger posted by another account was invisible, which turned off cross-round recovery in precisely the case it was built for: continuous integration posts as a bot and a maintainer runs as themselves. The two halves of that ledger are not the same claim — the findings are a work list every round re-rules against the code, while the reviewed-at commit decides which lines the next round skips — so the list now travels across accounts and the anchor never does. The attribution of test failures was reading a truncated record. The failing file set for the pull request side was re-parsed from a bounded report, which on a live run recovered one file out of eleven; the same set is now measured where the raw output still exists, before the bound is applied. The loss ran in the direction that matters: a file only the pull request side fails is exactly the one that should have been reported. Three smaller repairs round it out. A review launched from the bundle never learned which build it belonged to, so every helper command it shelled out to resolved whatever happened to be installed on the path — one live run died on its second command. A "nothing to disclose" answer written in Chinese was classified as a real gap and published as one, in a body whose own evidence said the opposite. And a count field surrounded by list fields was sent a list twice in four runs, so it now says what to send instead. * fix(review): tell a continuation that ended before its test phase apart from one that finished A run that never reached its test phase — a failed install, the disk-space gate, a budget spent during the build, a deliberate build-only probe — carries neither a test scope nor a test result, so a continuation finds nothing to do and said the run had reached every suite in scope. That is the same shape this branch fixes elsewhere: prose asserting the opposite of the evidence beside it. It now says no suite ran and names the fix, and a test pins the two facts apart. * fix(review): close the nine findings the pipeline raised on this branch The review this branch changes was pointed at this branch, and it filed three blockers and six suggestions. All of them hold; each fix carries the probe that found it. The Chinese placeholder classifier was dropping real disclosures. It claimed to mirror the English branch's narrowing and did not: without an all-done head the span before the completion word swallowed a gap clause, the single-character negation lookbehind let "not yet finished" read as finished, and forty free characters after the completion word swallowed the clause carrying the gap. Eight real sentences were classified as no-answers, which is the direction that certifies depth nobody reached. The branch now requires the same head the English side has always required, extends the lookbehinds, and ends at the completion word with only a budget adverbial after it. A continuation was declaring victory it had not won. A retry admitted late is killed again — the ordinary outcome when an expensive suite gets the tail of a budget — and the merged note still said every suite in scope had run, while a provisional result sat in the report. Retries the budget never reached were dropped from the accounting altogether, because a retry is a command and the not-run list holds workspaces. Both now count, and the note names what is still provisional and why another continuation is worth it. A continuation also dropped the framing an install failure owes the reader. The fresh path prepends it on every return precisely because the structured field alone was judged insufficient; the merge replaced the note wholesale, so a report carrying a non-zero install arrived with nothing saying that failure is infrastructure rather than something to file against the pull request. The framing is now shared between both paths. The rest are smaller. The budget help text described an admission rule this branch had already replaced, contradicting the code, the tests, and the sibling flag's own description. The continuation test could not fail: it searched the whole prompt, so text from the first invocation block and the sibling brief satisfied every assertion, and deleting the block it guards left it green — it now isolates that block, and a mutation confirms it fails when the block goes. The report guard checked one of the three arrays the merge walks, so a partial report died on a raw type error instead of the named refusal that is the guard's whole purpose. Two prose copies of a constant this branch consolidated now interpolate it. And the comment at the ledger recovery site still described the own-account-only model this branch removes, at exactly the boundary a reader auditing the change would consult. * fix(review): answer the second review round's blockers Ten more blockers, from the pipeline reviewing this branch a second time. The sharpest one is about the change this branch makes to the anchor. Exempting an unreviewed dimension from withholding the anchor was too wide. The field carries two different claims: a dimension nobody could RUN, which says nothing about which lines were read, and a lens that whiffed — made some tool calls, opened some files, returned nothing substantive twice — which is a claim about lines that no machine detector produces. The first cut exempted both, so a twice-whiffed security pass could advance the range past the lines it never reviewed, and the fixture that pinned exactly that was deleted along the way. The exemption now rests on a fact the code can check rather than on the field's name: exactly one dimension declares that it reads no diff, and only its gaps are depth. The deleted fixture is restored, and the other direction is pinned beside it. The Chinese no-answer classifier is rewritten as a closed vocabulary. Spelling the completion clause as bounded spans that merely refuse to cross an exception word invited exactly what two review rounds then found: negations the single-character lookbehind could not see, inability modifiers, hedged completions, gap clauses swallowed on either side of the completion word, and a span that slid past a negated completion to a later affirmed one. Every one of those drops a real disclosure, which is the direction this module's own header calls the worse of the two. A closed vocabulary cannot be walked through, because there is nothing to walk: a sentence carrying a gap is built from pieces the clause does not contain, so it fails to match and is kept. All thirteen evasions are now keep-tested against it. A continuation could destroy the report it was asked to continue. When no toolchain applied at the worktree root, the early returns built a fresh report with no reference to the previous one, and the handler wrote it over the file the run had just read — a wrong or pruned worktree path would replace an in-flight report with an empty one, and the chain stayed dead after the path was fixed. A continuation now refuses instead, and the refusal leaves the file untouched. Recovery across accounts could also run the round counter backward. Rounds are an id space, so a recovered round that goes down re-issues ids the pull request already carries against different findings. A bot whose own recovery failed transiently posts a round-one marker after a round-seven one, and ordering by timestamp alone hands the next round a two. The counter only ever advances, so preferring the highest round cannot lose a newer work list. The rest: the report guard now validates the scope the merge walks, not only its arrays; the ledger's one uncapped field is capped now that the read path takes text any account can post; the build-and-test brief names the third shape of unfinished work, a single-package repo whose budget stopped before its only suite; and the continuation test builds its paths the way its neighbours do instead of spelling them for one platform. * fix(review): answer the third round, whose blockers were all mine Every blocker this round came from last round's fixes. That is worth saying plainly: the repairs were made under review pressure and shipped their own defects, and the pipeline caught each one. Two were outright bugs in the report guard. It read a field off the parsed value before checking that value was an object, so a file containing the literal `null` produced the raw type error the guard exists to replace — and it checked that the command lists were lists without checking what was in them, so a list holding a null cleared the gate and died one layer deeper. Both refuse now, with the recovery instruction the sibling branches already carried. The Chinese clause had reintroduced the overlapping-quantifier shape this module's header bans and its linearity test exists for: four optional groups chained across whitespace matchers, in a language that does not put whitespace between those tokens. The whitespace bought nothing but the backtracking. It is gone, and the pathological input that walks that shape is now in the linearity test beside the others. The anchor decision was reading the dimension list after a pre-existing splice had already removed every entry mentioning the review time budget — a splice that exists so the body does not say one gap twice, and that matches on a phrase. An entry whose free-form reason merely mentions the budget was therefore invisible to the decision, and that entry is exactly the line-coverage claim the decision must respect. It now reads the list as disclosed and renders from the spliced one. And the two copies of the continuation rule disagreed with each other inside one prompt about the shape that cannot be continued at all. They now say the same thing: report the dimension unfinished, and do not spend a continuation on a report that has no scope for one to read. * fix(review): answer the fourth round; the anchor list has three writers The dimension list the anchor decision reads is written at three different points — the caller's own entries, the budget-phrase splice that removes some of them, and the deterministic gates that push their machine-owed debts in later — and the last two rounds each fixed one end by breaking the other. The first version read after the splice and missed the entries it removed; the second read before the gates and missed an unlinted script or an unwalked defect layer, either of which is a line-coverage claim, not a dimension nobody could run. It now reads the live list at the decision point plus the entries the splice removed, which is the only view that sees every writer. Two more from making foreign markers authoritative. The round a marker claims is the id space itself, so preferring the highest round hands an unbounded one from any poster a permanent win — and past 2^53 the increment stops advancing, so every later round re-stamps the same ids against different findings. Rounds are now capped on read and mirrored on write, fail-quiet like every other malformation. And a transient identity lookup no longer costs the recovery: it used to degrade to "no ledger", which leaves this machine's side file at the round it last wrote while other accounts post past it, so the next verdict re-issues ids the pull request already carries. The lookup is isolated now, and a failure recovers the work list as foreign — no anchor rides on an identity the run could not confirm. * fix(review): give the report a run identity; keep foreign ids out of the pipeline's namespace The fifth round found the two holes still open in the two boundaries this branch loosened, and both fixes are about identity rather than shape. A continuation trusted any well-shaped report at the out path, and that path is stable across review rounds while nothing sweeps it on an interrupted one — so the report a crashed round leaves behind is exactly what the next round's resume finds. Continuing it keeps the old commit's passing entries on the new round's tree, certifying old-commit passes for the new commit, and skips the install the fresh worktree never had. Every report now records the run it belongs to — the tree it ran in, and the commit its plan fetched — and a continuation refuses anything else, a report with no identity included: one that cannot prove it belongs here reads the same as one that provably does not. And the trust split stripped the anchor from a foreign marker but let its finding ids through, which hands any account the pipeline's own namespace: a marker at round N carrying ids from round N+1 pre-claims exactly the prefix the next compose stamps, splitting one claim across two ids and renumbering every genuinely new finding past the squatted block. A legitimate marker cannot claim an id from a future round — a round stamps its own ids and carries older ones forward — so the parser now drops any finding that does, read-side only, because the writer cannot produce the violation. * fix(review): refuse element-level corruption instead of crashing on it Two crash shapes from the sixth round, both the same lesson one level deeper than the round-three fix that checked the lists: the elements are the payload. A continuation's not-run entries become shell commands, so a list holding a null cleared an arrays-only guard and crashed in the escaper instead of refusing with the named fix. And the failing-file set rides a report file anything may have edited, with exactly one consumer — a set that is not a string array reached the set arithmetic as-is, where the honest reading is the one an absent field has always taken: this seam supplied no measurement, fall back to re-parsing the stored output. * fix(review): answer the seventh round — instance identity, relay independence, and the cap round-trip Five blockers, all in this branch's own additions, three of them in the fixes earlier rounds bought. The run identity was strings where it needed an instance. The path and the sha are identical after fetch-pr destroys and recreates the worktree — which it does every round — so the identity check admitted a continuation onto a bare tree with no installed or compiled state, whose every suite then failed with resolution errors framed as candidate findings against the pull request. The report now records the worktree root's inode and birth time, which a recreated directory cannot keep, and a continuation onto a previous instance refuses with the reason spelled out. No legitimate continuation crosses a recreation; the valid resumes all happen inside one round. The anchor decision was relay-dependent for a budget or round-cap stop: identical machine state carried the anchor when the orchestrator dropped the mandated stop entry and withheld it when the entry was relayed — and the stderr instruction mandates the relay, so every compliant run paid the full-diff re-review this exemption exists to end. The stop's relayed entry now classifies as depth, and only against the marker the machine itself wrote: no marker, no exemption, so stop-shaped prose cannot buy an anchor the state does not support, and a lens entry that mentions the phrase in its reason withholds as before. The caveat gets the cure the note already had. A resume appended to it, so a completed chain still read "still to run" over suites that had just passed, and the dimension brief tells the agent to quote a present caveat as possibly-incomplete scope. Superseded budget-stop and earlier resume segments are retired, live limitations survive verbatim, and a chain that finishes with none ends with the caveat absent — the field's own contract for full coverage. The round cap broke its own round-trip: the stamp was uncapped while the serializer clamped, so at exactly the cap the writer produced a marker whose own parser dropped every finding — invisibly, with the anchor still riding. The one writer now caps its stamp, which also makes the squat filter's premise true again. And the bare "check" noun in the Chinese token made the classifier drop standalone "did not check" lines — a live gap under the brief's own rule that the line is only written when something was cut short. The noun group keeps the two documented placeholder nouns and nothing else. * fix(review): answer the eighth round — exact machine text, anchored retirement, and the local round's edge Four blockers; three sit in the seventh round's fixes and one is the second round's finding still alive under its replacement. The stop exemption matched a head plus a phrase, and that shape also covers a genuine line-coverage claim whose whiffed scope IS the reverse audit — an entry the phrase splice then removes from the rendered body, so the anchor rode past a whiffed audit while the posted review showed only the benign disclosure. The exemption now matches the exact entries the machinery mints, nothing looser: marker-anchored and text-anchored, and an edited or paraphrased relay withholds, which is the safe direction. The caveat retirement had two holes with one shape. Its regex matched the marker phrases anywhere in a segment, and segments interpolate file names from the reviewed diff — so a file named after the phrase retired the live limitation quoting it, untrusted input silently certifying scope. And the resume clause itself emitted the segment separator inside one clause, so a second continuation cut it in half: the head retired, the tail — "N still to run" — survived into a report whose note says everything ran. Retirement now anchors on the producers' own grammar at the segment start, and the clause is one segment. The run identity had no edge a local round crosses. Local plans carry no sha and the project root is never recreated, so every clause compared equal across rounds and an interrupted round's report certified pre-edit results for the edited tree. The plan file is the one thing every round writes afresh in both modes; its mtime now rides the identity, and a report stamped against a previous round's plan refuses with the reason named. The report gate and the delta seam also stop trusting vacuous content: workspace names and commands must be non-empty — an empty workspace resolves npm to the root suite, a different measurement wearing the requested one's name — timed-out entries are strings, and an empty failing-file set reads as no measurement, since the producer omits the field rather than writing one. * fix(review): answer the ninth round — structural caveats, bounded foreign rounds, and the starved suite Four blockers. One had stood since round one, and two survive earlier fixes in sharper forms; the shape of the repairs is the lesson of the round. The caveat is no longer parsed at all. Two attempts at retiring superseded clauses by re-reading rendered prose both lost to PR-authored names — first a filename matching the phrase, then a workspace dir embedding the segment separator plus the clause grammar, which fabricated a boundary and retired the live limitation's honest tail with the fake clause. The fresh run now records the scope's own caveat in a separate field whenever it appends a machine clause; a continuation carries that string through untouched and rebuilds the joined prose from it plus its own current clause. Absent means nothing was appended and the whole caveat is live. Nothing content-matches, so nothing can be talked out of a limitation by a name in the reviewed diff. The resume path now honors the ordering invariant the fresh path documents. Not-run suites are stored alphabetically, and a continuation that consumed them verbatim starved the changed workspace's suite to the budget's worst tail on every continuation — a chain could hit its cap with the one suite the diff changed never run, disclosed but never measured. Retries first, then the affected suites, then the dependents. A foreign round implausibly far past this account's own is no longer adopted. Round-first selection had a fixed point an attacker could pin with one post: a round at the cap outranks every real round forever, the capped stamp holds the counter there, and every later round re-issues the same ids against different findings. Rounds advance one per posted review, so a legitimate interleave sits a handful ahead at most; beyond our own highest plus a headroom of sixty-four, a foreign marker is not a newer work list — it is not a work list at all. Inside the bound, a hostile post can only inflate the counter by a bounded step, which costs numbers and nothing else. And the stop-entry splice reads both languages, because the exemption already admits the Chinese pair as a compliant relay: with an English-only phrase the relayed Chinese entry survived into the whiffed-dimension rendering beside the structural stop line — the same gap said twice, one copy under the wrong cause. The relay-independence test now asserts the strongest form available: the body is byte-identical whether the entry was relayed in English, relayed in Chinese, or dropped. * fix(review): answer the tenth round — the cache reads the marker, and a typecheck slip Two blockers. One is mine in the plainest sense: a test added late in round nine called a helper with too few arguments, and the local gate that would have caught it — the typecheck — was the one step that round skipped. Fixed, and the same test now also relays the Chinese round-cap pair, pinning byte identity across all three relay states for that branch too. The other is the first round's finding come back through the document: the skill's cache rule was a hand-copied condition list, and it aged out of sync with the module the moment the anchor net grew the depth-only distinction — a whiffed-lens round had its sha withheld by the marker and cached by the prose. The rule is now mechanical instead of descriptive: the cache advances exactly when the composed body's marker carries a sha, because the module already computed the net and two copies of it is how the two anchors came to disagree about what a clean round is. From the suggestions: resume with build-only now refuses instead of silently ignoring the flag — the pair names no work at all; the lockfile's merge-collateral peer-flag churn is reverted to main's copy; and three comments now say what is true — the inode does not separate instances on a filesystem that reuses them (the plan mtime is the cross-round floor there), an inside-bound hostile round can still win one recovery's work list (which is re-ruled, like every foreign work list), and a failed identity lookup bounds recovery to the headroom rather than recovering everything. * fix(review): answer the eleventh round — a proven identity, a validated identity, and the bullet the merge ate Three blockers, and the middle one is a regression my own round-four fix introduced. The isolated identity lookup turned a rate-limit blip into proof of absence. With the lookup's failure swallowed into a null login, the recovery walk had no name to look for, recorded "no own review exists" about an identity it never knew, and the deletion arm removed the side file and reset the round counter — the id-space collision this whole recovery redesign exists to prevent, delivered by a transient network error. The pre-isolation code got this right by accident, because the throw reached the outer catch and took the conservative strip path. Deletion now requires a CONFIRMED identity, and the handler-level pin drives the real handler with a throwing lookup and asserts nothing is removed. The identity gate crashed on the one corrupt shape it did not name: the report's run field was never validated, so `tree: null` slipped past a presence-only check and died on a null dereference inside the very gate that exists to refuse with a named fix. The gate now validates everything the identity check walks — root, sha, plan, and the tree fingerprint's own fields — and the crash shapes joined the refusal table. And the merge resolution had eaten the skill's documentation for a state field the module still consumes: the duplicate-drop account's bullet was adjacent to a bullet this branch rewrote, and the conflict resolution kept the rewrite and dropped the neighbour — leaving a channel the verdict arithmetic counts with no documented way to reach it. Restored verbatim. From the suggestions: the lockfile's merge-collateral churn is reverted again — this time after the last npm operation rather than before the next one — and the seven comments the sweep named now describe the shipped mechanisms: the structural caveat carry-through that replaced both parses, the stamp that writes this build's own entry instead of blanking the slot, the truncation flag that fires only on the re-parse fallback and loses in both directions, the depth exemption's full exception set, and the continuation contract's retry-first half. * fix(review): answer the twelfth round — the union that ends displacement, and the gate's last fields Two blockers. A foreign marker one round ahead — deep inside the plausibility bound — could displace this account's entire work list, and displaced entries owed no ruling, so one drive-by comment retired a certified Critical from the marker chain for the rest of the PR's life; the doctored variant copies the own list minus the entry to suppress. A foreign winner is now MERGED over the own latest findings instead of replacing them: own entries are authoritative on an id collision, foreign entries with new ids join after, the merged list re-caps with an honest dropped count, and the round number still advances — the counter is a shared id space, the work list is not a prize. The headroom comment now says what is true because of this, rather than what was hoped. And the continuation's shape gate now validates the last fields the merge walks: a non-iterable affected crashed the ordering seed, a string notBuilt crashed the refusal's join, and a bare true notBuilt — the worst shape — skipped the unbuilt-tree refusal silently and ran suites against packages that were never compiled, manufacturing the exact failures the refusal exists to prevent. The caveat strings are typed too, since the brief quotes them. From the suggestions: the resumed note carries the caveat exactly as the fresh path does, counts build failures into its passed/failed sentence so it cannot contradict the recomputed ok beside it, and the stray duplicated doc-comment opener is gone. * fix(review): answer the thirteenth round — exec evidence, a command grammar, and an empty login Three blockers from the automatic review, all probe-confirmed on the merge head. R1-11 (isUnusableScriptEntry): the gate answered 'usable' by extension enumeration — anything outside .js/.mjs/.cjs returned usable with no isFile/X_OK/shebang check — so the childEnv stamp could hand skill subcommands an entry no shell can exec: a tsx dev launch stamps the 0644 index.ts, and 'node <pkg-dir>' stamps the DIRECTORY (which passes an X_OK probe as search permission). Every '"${QWEN_CODE_CLI:-qwen}"' then died on exit 126 where empty would have fallen back. The gate now demands positive evidence: a regular file with the execute bit, a '#!' header for any known script extension (the TS family included), no shebang needed for native binaries. Both entrances are pinned at the childEnv level and the primitive's own suite covers the three shapes. R3-38 (--resume injection): the continuation re-executed report-stored test[].command strings verbatim under shell: true, while the identity check pins a report to this run's tree — not to this program's authorship — and the gate's own corrupt-report fixtures declare the edited-in-place report the accepted threat model. Stored commands are now held to the emitter's grammar (npm test [--workspace="<dir>"]), the same policy test-delta already applies before re-running report commands; an alien command refuses the resume with a named fix, before anything runs. identityKnown empty-login: currentUser() answering empty-with-exit-0 (a stubbed or proxied gh) set identityKnown=true while recoverLedger reads '' as unknown, so the deletion arm could remove the prev-ledger side file — resetting the round counter — over an identity that was never proven. Empty is now unknown, matching presubmit's own '' handling. Each fix is pinned by a test its reverted mutant fails. * fix(review): answer the fourteenth round — an anonymous recovery cannot swap the list R13-1: during an identity-lookup outage (currentUser throws, or answers empty) every marker walks as FOREIGN — there is no me — so the union that protects the certified work list never has an own side to merge over, and the recovered winner was written to the side file WHOLESALE: a drive-by marker posted at this account's current round (visible to any authenticated user, inside the zero-base headroom) replaced this machine's last known-good list on equal round + larger review id, and the swap persisted — the marker stays on the PR, so every later outage reopened it. The suppression class the union merge exists to kill, reopened through the anonymous path. persistRecoveredLedger now takes identityKnown and gives the anonymous recovery its own outcome: with a readable existing file, a same-round winner changes NOTHING, and a strictly-higher round advances only the round counter (refusing that too would re-expose R4-2's id-space collision — a lagging counter re-issues ids the PR already carries) plus the reviewId tiebreak; the findings stay this machine's own, and sha/commitId are dropped — an anonymous round cannot be re-vouched, and an anchor superseded by rounds this account never certified must not scope the next review (the healthy foreign-winner path strips it at the recovery seam for the same reason). With no readable file there is nothing to protect and the write stays wholesale. The new suite pins the drive-by (file byte-identical), the advance (counter moves, list survives, anchor and age reference gone — with noOwnReview deliberately true so a positional swap of the two booleans deletes the file and fails the test), and the no-file arm; the guard mutant fails exactly the first two.
…g-budget Resolve the geminiContentGenerator -> llm-content-generator split: Main renamed the module and left the old path as a re-export shim while this branch rewrote geminiContentGenerator.ts. Take main's shim and 3-way replay the branch's thinkingBudget work onto llm-content-generator.ts (git merge-file against the merge base); the only non-applying hunk was the observeGeminiStream -> observeLlmStream rename, kept as main spells it. llm-content-generator.test.ts: take this branch's side of each hunk (the existing thinkingLevel cases move to gemini-3-pro-preview, which is the point of the fix) with the constructor renamed to LlmContentGenerator. tsc -p packages/core clean; 25 tests pass.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R2-2 stale per-provider reasoning table + contentGenerator.ts reasoning doc comment — still stands, already reported (comment 3779075219)
- R2-3 thinkingLevel record's low:'LOW' entry never exercised by any test — still stands, already reported (comment 3779075223)
- R2-4 config-level reasoning discards request-level thinkingConfig opt-outs (four budget-0 sites); deferred work tracked by no issue — still stands, already reported (comment 3779075228)
- R2-5 effort-clamp warning guard and warn-once latch have no test pinning when the warning must fire — still stands, already reported (comment 3779075237)
中文说明
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const clamped = clampReasoningEffort(reasoning.effort, [ | ||
| 'low', | ||
| 'medium', | ||
| 'high', | ||
| ]); |
There was a problem hiding this comment.
[Critical] R2-1: [fails-closed] gemini-3-pro-preview supports only the low/high thinking levels, but this shared ladder sends MEDIUM whenever effort === 'medium' — the same failure class this PR eliminates for Gemini 2.5. Google's thinking docs list per-model supported levels (gemini-3-pro-preview: low, high), and the repo's own design doc records the same split (docs/design/2026-06-30-unified-reasoning-effort-cli.md: "Gemini 3 Pro → LOW/HIGH, Flash → MINIMAL/LOW/MEDIUM/HIGH"). A user who sets /effort medium (or has persisted model.reasoningEffort: medium) on gemini-3-pro-preview gets thinkingConfig: { includeThoughts: true, thinkingLevel: 'MEDIUM' } on every request; if the live API's per-model support matches the doc, the requests are rejected and the model is unusable at medium effort — reproducing for Gemini 3 the very 400 this PR fixes for 2.5. The mapping predates this PR, but this PR rewrites this exact function and re-asserts the mapping: the retargeted test maps reasoning effort 'medium' to MEDIUM now asserts the unsupported value against exactly this model. This is the still-standing round-2 Critical, re-verified by probe at this commit.
Witness:
probe at the reviewed commit (mocked @google/genai, recorded generateContent config):
gemini-3-pro-preview + effort:'medium' -> { includeThoughts: true, thinkingLevel: 'MEDIUM' }
control: gemini-3-pro-preview + effort:'xhigh' -> thinkingLevel: 'HIGH' (the rank clamp itself works)
control: gemini-2.5-pro + effort:'medium' -> { includeThoughts: true, thinkingBudget: 8192 }
flip (Pro ladder clamped to ['low','high']): gemini-3-pro-preview + effort:'medium' -> thinkingLevel: 'HIGH'
Suggested fix: make the ladder per-model — clamp Gemini 3 Pro-family models with ['low', 'high'] (the rank-based clamp then maps medium/xhigh/max → high), keep ['low', 'medium', 'high'] for Gemini 3 Flash and other level-style models, and update the retargeted medium test accordingly.
The fix must respect two facts: the design doc (docs/design/2026-06-30-unified-reasoning-effort-cli.md:108-110) records "Gemini 3 Pro → LOW/HIGH", and clampReasoningEffort (packages/core/src/core/reasoning-effort.ts) prefers the next-stronger supported tier — under ['low','high'] medium clamps UP to high, so keep the clamp rather than hand-mapping. Add a test asserting effort: 'medium' on gemini-3-pro-preview emits thinkingLevel: 'HIGH' (the retargeted test currently asserts the opposite and must flip), and prove it by removing the per-model ladder and watching that test go red.
中文说明
[Critical] R2-1：gemini-3-pro-preview 只支持 low/high 两个 thinking 档位，但这个共享梯级在 effort === 'medium' 时仍会发送 MEDIUM——这与本 PR 要为 Gemini 2.5 消除的失败属于同一类。Google 的 thinking 文档按模型列出了支持的档位（gemini-3-pro-preview：low、high），仓库自己的设计文档也记录了同样的划分（docs/design/2026-06-30-unified-reasoning-effort-cli.md："Gemini 3 Pro → LOW/HIGH，Flash → MINIMAL/LOW/MEDIUM/HIGH"）。用户在 gemini-3-pro-preview 上设置 /effort medium（或已持久化 model.reasoningEffort: medium）后，每个请求都会带上 thinkingConfig: { includeThoughts: true, thinkingLevel: 'MEDIUM' }；如果线上 API 的按模型支持与该文档一致，请求会被拒绝，该模型在 medium 档位下完全不可用——等于在 Gemini 3 上复现了本 PR 要为 2.5 修复的那个 400。该映射在本 PR 之前就已存在，但本 PR 重写了这个函数本身，并在测试中重新确认了这一映射：被重新定向的测试 maps reasoning effort 'medium' to MEDIUM 现在恰恰针对这个模型断言了这个不被支持的值。这是第二轮遗留、至今仍然成立的 Critical，本轮已在被审 commit 上用探针重新验证。
证据：在被审 commit 上用探针验证（mock @google/genai，记录 generateContent 的 config）：gemini-3-pro-preview + effort:'medium' 发出 { includeThoughts: true, thinkingLevel: 'MEDIUM' }；对照 gemini-3-pro-preview + effort:'xhigh' 发出 thinkingLevel: 'HIGH'（基于 rank 的收敛本身正常）；对照 gemini-2.5-pro + effort:'medium' 发出 { includeThoughts: true, thinkingBudget: 8192 }；把 Pro 梯级收敛到 ['low','high'] 后，同一行翻转为 thinkingLevel: 'HIGH'。
建议修复：把梯级改为按模型区分——对 Gemini 3 Pro 系列用 ['low', 'high'] 做收敛（基于 rank 的收敛会把 medium/xhigh/max 映射到 high），对 Gemini 3 Flash 及其他 level 风格模型保留 ['low', 'medium', 'high']，并相应更新被重新定向的 medium 测试。
修复必须尊重两个既有事实：设计文档（docs/design/2026-06-30-unified-reasoning-effort-cli.md:108-110）记录了 "Gemini 3 Pro → LOW/HIGH"；且 clampReasoningEffort（packages/core/src/core/reasoning-effort.ts）优先选择下一个更强的受支持档位——在 ['low','high'] 下 medium 会向上收敛到 high，因此应保留收敛逻辑而不是手工映射。补一个测试，断言 gemini-3-pro-preview 上 effort: 'medium' 发出 thinkingLevel: 'HIGH'（被重新定向的测试目前断言了相反的值，必须翻转），并在移除按模型区分的梯级后确认该测试变红，以此证明测试有效。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| case 'xhigh': | ||
| case 'max': | ||
| return BUDGET_STYLE_PRO.test(model) | ||
| ? THINKING_BUDGET_CEILING_PRO | ||
| : THINKING_BUDGET_CEILING; |
There was a problem hiding this comment.
[Suggestion] R3-1: this diff makes the shipped setting description "Gemini caps at high" false for the 2.5 family, in user-facing places the round-2 docs finding (R2-2) did not list: packages/cli/src/config/settingsSchema.ts:1568 (the model.reasoningEffort description, showInDialog: true, so it renders in the /settings dialog) and its mirror docs/users/configuration/settings.md:176; packages/vscode-ide-companion/schemas/settings.schema.json:690 carries the same sentence and is generated from the schema. Pre-PR the claim was accurate — every Gemini model clamped xhigh/max to thinkingLevel: 'HIGH'. Post-PR, a gemini-2.5-* model maps xhigh/max to thinkingBudget 32768 (Pro) / 24576 (other), strictly above high's 16384 bucket: a user on a 2.5 model who reads the settings dialog or the settings reference concludes /effort xhigh|max is pointless on Gemini, never selects it, and silently misses the doubled thinking budget this PR exists to deliver — or files "max does nothing on Gemini" as a bug when the wire payload in fact grows.
Witness:
probe at the reviewed commit:
gemini-2.5-pro effort=xhigh -> {"includeThoughts":true,"thinkingBudget":32768}
gemini-2.5-pro effort=max -> {"includeThoughts":true,"thinkingBudget":32768}
gemini-2.5-flash effort=max -> {"includeThoughts":true,"thinkingBudget":24576}
gemini-2.5-pro effort=high -> {"includeThoughts":true,"thinkingBudget":16384}
gemini-3-pro-preview effort=xhigh -> {"includeThoughts":true,"thinkingLevel":"HIGH"}
wording at packages/cli/src/config/settingsSchema.ts:1568 (showInDialog: true, rendered in /settings):
"...Each provider maps and clamps this to what the active model supports (e.g. Gemini caps at "high"; Anthropic clamps tiers a model lacks)..."
Suggested fix: reword both copies — e.g. Each provider maps and clamps this to what the active model supports (e.g. Gemini 3 caps at "high"; Gemini 2.5 maps xhigh/max to its thinking-budget ceiling; Anthropic clamps tiers a model lacks). — edit settingsSchema.ts first, update docs/users/configuration/settings.md:176 by hand, then regenerate packages/vscode-ide-companion/schemas/settings.schema.json via npm run generate:settings-schema.
中文说明
[Suggestion] R3-1：本 diff 使得随代码发布的设置描述 "Gemini caps at high"（Gemini 最高到 high）对 2.5 系列不再成立，涉及两处第二轮文档发现（R2-2）没有列出的用户可见位置：packages/cli/src/config/settingsSchema.ts:1568（model.reasoningEffort 的描述，showInDialog: true，会渲染在 /settings 对话框中）及其镜像 docs/users/configuration/settings.md:176；packages/vscode-ide-companion/schemas/settings.schema.json:690 带有同一句话，由 schema 生成。PR 之前这一说法是准确的——所有 Gemini 模型都会把 xhigh/max 收敛到 thinkingLevel: 'HIGH'。PR 之后，gemini-2.5-* 模型把 xhigh/max 映射为 thinkingBudget 32768（Pro）/ 24576（其他），严格高于 high 的 16384 分桶：在 2.5 模型上读到设置对话框或设置文档的用户会以为 /effort xhigh|max 在 Gemini 上没有意义，从而永远不去选择它，悄悄错过本 PR 正要提供的翻倍 thinking 预算——或者在线上 payload 实际变大的情况下，把 "max 在 Gemini 上没用" 当成 bug 提出来。
证据：在被审 commit 上用探针验证：gemini-2.5-pro effort=xhigh 发出 {"includeThoughts":true,"thinkingBudget":32768}、effort=max 同样为 32768、gemini-2.5-flash effort=max 为 24576，而 effort=high 为 16384；gemini-3 一侧 effort=xhigh 仍收敛为 thinkingLevel:"HIGH"。settingsSchema.ts:1568 处的描述（showInDialog: true，渲染于 /settings）仍写着 "Gemini caps at "high""。
建议修复：改写这两处描述——例如 Each provider maps and clamps this to what the active model supports (e.g. Gemini 3 caps at "high"; Gemini 2.5 maps xhigh/max to its thinking-budget ceiling; Anthropic clamps tiers a model lacks).——先改 settingsSchema.ts，手工更新 docs/users/configuration/settings.md:176，再用 npm run generate:settings-schema 重新生成 packages/vscode-ide-companion/schemas/settings.schema.json。
— qwen3.8-max via Qwen Code /review (v0.22.3)
4 participants
What this PR does
Gemini 2.5 takes a thinking budget, not a thinking level, so the Gemini/Vertex generator now sends
thinkingConfig.thinkingBudgetfor that family and keepsthinkingLevelfor Gemini 3. When no reasoning effort is configured it sends{ includeThoughts: true }and no level at all, instead of theTHINKING_LEVEL_UNSPECIFIEDplaceholder.Effort tiers map to budgets by inverting the budget → level thresholds the design doc already records:
thinkingBudgetSay the word if you want different numbers, it is one function.
One side effect worth calling out:
xhighandmaxnow mean something on 2.5 instead of clamping to HIGH, because budgets reach higher than the Gemini 3 ladder can express.Why it's needed
Every request to a Gemini 2.5 model fails with
400 thinking_level is not supported by this model, which makes the whole family unusable. Two paths, same failure: with no effort set the default carriedTHINKING_LEVEL_UNSPECIFIED, and with/effortset it carriedLOW/MEDIUM/HIGH. The API rejects the field itself rather than the value, soreasoning: falsewas the only escape. The placeholder also bought nothing on models that do accept the field:THINKING_LEVEL_UNSPECIFIEDmeans "the model decides", which is exactly what omitting the field does.This is not new design.
docs/design/2026-06-30-unified-reasoning-effort-cli.mdalready specifiestoGeminiThinking(tier, model)asthinking_levelfor Gemini 3 orthinkingConfig.thinkingBudgetfor Gemini 2.5, and its header states "Nothing is deferred" even though the 2.5 branch was never implemented. This PR implements that branch.Reviewer Test Plan
How to verify
Unit tests:
cd packages/core && npx vitest run src/core/geminiContentGeneratorgives 28 passing, including one case per tier on 2.5 Pro, the Flash ceiling, an assertion thatthinkingLevelis never sent to a 2.5 model, and one that an explicit request-levelthinkingConfigstill wins over the default (this last one protects the internal call sites that passthinkingBudget: 0).Full checks:
npm run buildandnpm run typecheckare clean.Live, which I could not run and which a reviewer with 2.5 access should confirm: select
gemini-2.5-proon Vertex or the Gemini API, send any prompt, and confirm the request completes instead of returning400 thinking_level is not supported by this model. Then/effort highand confirm it still completes. On a Gemini 3 model, confirm thought summaries still appear, since the no-effort default changed shape there too.Evidence (Before & After)
Not user-visible, so no screenshots. The change is at the wire level, for
gemini-2.5-pro:{ includeThoughts: true, thinkingLevel: 'THINKING_LEVEL_UNSPECIFIED' }→ 400{ includeThoughts: true }/effort high{ includeThoughts: true, thinkingLevel: 'HIGH' }→ 400{ includeThoughts: true, thinkingBudget: 16384 }/effort max{ includeThoughts: true, thinkingLevel: 'HIGH' }→ 400{ includeThoughts: true, thinkingBudget: 32768 }{ includeThoughts: true, thinkingLevel: 'HIGH' }thinkingConfigreasoningis setThe 400 itself was reproduced live on Vertex; the transcript is in #9019.
Tested on
Environment (optional)
Unit tests,
npm run buildandnpm run typecheckon macOS. No live API call for the fix itself.Risk & Scope
thinkingLevelon Gemini 3 either. The semantics are identical per the API, andincludeThoughts: trueis preserved, so thought summaries are unaffected. The bucket numbers are a judgement call and easy to change.thinkingBudget: 0(goalJudge.ts,goal-verifier.ts,goal-checkpoint-verifier.ts,acp-integration/generation.ts) are still overridden whenever a config-levelreasoningis set; they stop returning 400, they just think when they asked not to, and fixing it means changinggetParameterValueprecedence, which affects every field it handles. Second,reasoning: falseon 2.5 sendsincludeThoughts: falsealone, which does not actually stop thinking on that family, and 2.5 Pro cannot disable it at all. Happy to take either as a follow-up.Linked Issues
Fixes #9019
中文说明
这个 PR 做了什么
Gemini 2.5 使用的是 thinking budget，而不是 thinking level，因此 Gemini/Vertex 生成器现在对该系列发送
thinkingConfig.thinkingBudget，而对 Gemini 3 保留thinkingLevel。未配置 reasoning effort 时，发送{ includeThoughts: true }，完全不带 level，不再发送THINKING_LEVEL_UNSPECIFIED占位值。各档位到 budget 的映射，来自反向套用设计文档中已记录的 budget → level 阈值：
thinkingBudget如果希望换成别的数值，请直接说，只需要改一个函数。
一个值得说明的副作用：
xhigh和max在 2.5 上现在有了实际意义，不再被压到 HIGH，因为 budget 能表达的范围超出 Gemini 3 的档位阶梯。为什么需要
任何发往 Gemini 2.5 模型的请求都会失败并返回
400 thinking_level is not supported by this model，导致整个系列不可用。两条路径、同一个失败：未设置 effort 时，默认值带上THINKING_LEVEL_UNSPECIFIED；设置了/effort时，带上LOW/MEDIUM/HIGH。API 拒绝的是字段本身而不是取值，所以唯一的例外是reasoning: false。而且这个占位值对接受该字段的模型也毫无收益：THINKING_LEVEL_UNSPECIFIED的含义是"由模型自行决定"，这与省略该字段完全等价。这并不是新的设计。
docs/design/2026-06-30-unified-reasoning-effort-cli.md已经把toGeminiThinking(tier, model)规定为：Gemini 3 用thinking_level，Gemini 2.5 用thinkingConfig.thinkingBudget；该文档头部写着 "Nothing is deferred"，但 2.5 这一分支从未实现。本 PR 实现了该分支。审阅者测试计划
如何验证
单元测试：
cd packages/core && npx vitest run src/core/geminiContentGenerator，28 项通过，其中包括 2.5 Pro 上每个档位各一例、Flash 的上限、断言绝不向 2.5 模型发送thinkingLevel，以及断言请求级显式传入的thinkingConfig仍然优先于默认值（最后这一条保护了那些传thinkingBudget: 0的内部调用点）。完整检查：
npm run build与npm run typecheck均通过。线上验证，这部分我无法执行，建议有 2.5 权限的审阅者确认：在 Vertex 或 Gemini API 上选择
gemini-2.5-pro，发送任意提示，确认请求能够完成，而不是返回400 thinking_level is not supported by this model；随后执行/effort high并确认仍能完成。在 Gemini 3 模型上，确认 thought summaries 仍然显示，因为无 effort 时的默认结构在那里也发生了变化。证据（前后对比）
该改动对用户不可见，因此没有截图。变化发生在请求层面，以
gemini-2.5-pro为例：{ includeThoughts: true, thinkingLevel: 'THINKING_LEVEL_UNSPECIFIED' }→ 400{ includeThoughts: true }/effort high{ includeThoughts: true, thinkingLevel: 'HIGH' }→ 400{ includeThoughts: true, thinkingBudget: 16384 }/effort max{ includeThoughts: true, thinkingLevel: 'HIGH' }→ 400{ includeThoughts: true, thinkingBudget: 32768 }{ includeThoughts: true, thinkingLevel: 'HIGH' }thinkingConfigreasoning时生效400 本身已在 Vertex 上实际复现，记录见 #9019。
测试环境
运行环境（可选）
在 macOS 上运行单元测试、
npm run build与npm run typecheck。修复本身没有进行线上 API 调用。风险与范围
thinkingLevel。按照 API 定义两者语义相同，且保留了includeThoughts: true，因此 thought summaries 不受影响。budget 的具体数值属于判断取舍，很容易调整。thinkingBudget: 0的内部调用点（goalJudge.ts、goal-verifier.ts、goal-checkpoint-verifier.ts、acp-integration/generation.ts）在设置了配置级reasoning时仍会被覆盖；它们不再返回 400，只是在要求不思考时仍然思考，而修复它需要改动getParameterValue的优先级，会影响它处理的每一个字段。其二，在 2.5 上reasoning: false只发送includeThoughts: false，并不能真正关闭该系列的 thinking，而 2.5 Pro 根本无法关闭。这两项我都乐意作为后续 PR 处理。关联 Issue
Fixes #9019