fix(core): resolve DashScope thinking-knob conflicts by family (#8488 round 2)

[wenshao]

What this PR does

Second review round on #8488 (merged). Resolves the remaining thinking-knob conflicts on the DashScope wire shape by model family:

1. Explicit opt-out honoured: on the qwen3.8-max family, an explicit extra_body.enable_thinking: false no longer gets silently deleted (which turned thinking back on) — it is translated into the family's canonical disable, reasoning_effort: 'none', which the pipeline's disable strip already preserves.
2. Legacy hybrids keep the knobs they read: on older qwen models, a reasoning_effort override conflicting with a thinking_budget now drops the inert reasoning_effort instead of the meaningful budget; enable_thinking and thinking_budget survive.
3. tool_choice gate made consistent: the enable_thinking === true clause is family-gated like its reasoning_effort sibling. On non-qwen models sharing the endpoint the field is an opaque no-op (GLM reads thinking.enabled, DeepSeek thinking.type) that does not put the request in thinking mode, so stripping tool_choice: 'required' there only degraded their forced-tool side queries — the exact harm fix(core): harden Qwen 3.8 reasoning effort wire shape #8488 set out to fix. thinkingMandatory stays ungated.
4. Family-consistent disable shape: the DashScope disable path for the tier-native family emits reasoning_effort: 'none' (the knob it reads) instead of enable_thinking: false (the boolean it doesn't), and also drops a conflicting thinking_budget. The required-thinking retry trigger recognises the new shape so runtime learning still fires.
5. Observability & layering: user extra_body knob drops now warn once per generator (they discard explicit user config); the wire-model family predicates moved to modalityDefaults.ts (single source, also used by the pipeline's non-DashScope branch), and the provider's vision/text merge tails share one helper.
6. Docs: the vendor rejection is attributed to thinking_budget only (the enable_thinking + reasoning_effort pair was live-accepted in feat(core): support Qwen 3.8 reasoning effort #8472's E2E), and the extra_body exceptions are documented.

Why it's needed

The round-1 shape still silently discarded explicit user configuration in two places (findings 1–2 of the #8488 second review) and left the two tool_choice clauses on opposite sides of the family-gating argument (finding 3) — non-qwen Token Plan presets (glm-5.*, kimi-k2.*) ship enable_thinking: true, so their forced-tool side queries kept losing tool_choice: 'required'.

Reviewer Test Plan

How to verify

All changes are covered by unit tests; every new/flipped test was verified load-bearing by targeted mutation (reverting the corresponding fix turns exactly its tests red, nothing else):

cd packages/core && npx vitest run src/core/openaiContentGenerator src/core/modalityDefaults.test.ts

• qwen3.8-max + effort tier + extra_body: { enable_thinking: false } → wire carries reasoning_effort: 'none' alone.
• qwen3.7-max + extra_body: { thinking_budget, reasoning_effort } → wire keeps enable_thinking/thinking_budget, drops the inert reasoning_effort (text and vision branches).
• glm-5.2 + preset shape end-to-end through the real provider: tool_choice: 'required' survives.
• Disable path: qwen3.8-max opt-outs ship reasoning_effort: 'none'; the retry tests re-pin runtime learning with the new shape.
• Multi-knob drop, snapshot/alias drop, and non-qwen budget-gate cases (the three inline suggestions from the round-1 review).

Evidence (Before & After)

N/A (wire-shape change, not user-visible; unit-test + mutation evidence above).

Tested on

OS	Status
🍏 macOS	✅
🪟 Windows	N/A
🐧 Linux	N/A

Environment (optional)

Unit tests only; full npm run typecheck and targeted eslint/prettier --check clean.

Risk & Scope

• Main risk or tradeoff: if the DashScope gateway reads enable_thinking on kimi presets and rejects required in thinking mode, kimi side queries would 400 instead of silently degrading — no codebase or live evidence supports that reading, while the status quo observably discarded forced tools for non-qwen presets. Stated in the fix(core): harden Qwen 3.8 reasoning effort wire shape #8488 thread for the record.
• Not validated / out of scope: live DashScope acceptance of reasoning_effort: 'none' as a disable on qwen3.8-max (no API access); the non-DashScope (chat_template_kwargs) disable branch is unchanged.
• Breaking changes / migration notes: none.

Linked Issues

Follow-up to #8488 (second-round review findings); original user request #2876.

中文说明

本 PR 做了什么

#8488（已合并）的第二轮 review 处理。按模型家族消解 DashScope 出网形态上剩余的思考旋钮冲突：

1. 尊重显式关闭：qwen3.8-max 家族上，显式的 extra_body.enable_thinking: false 不再被静默删除（那会把思考重新打开），而是翻译为家族的标准禁用形态 reasoning_effort: 'none'——pipeline 的 disable 剥除本就保留该值。
2. 旧 hybrid 保留其读取的旋钮：旧 qwen 模型上，与 thinking_budget 冲突的 reasoning_effort 覆盖现在剥掉惰性的 reasoning_effort 而非有意义的 budget；enable_thinking 与 thinking_budget 保留。
3. tool_choice 门控一致化：enable_thinking === true 条款与 reasoning_effort 兄弟条款一样加上家族门控。对共享端点的非 qwen 模型，该字段是不透明的空操作（GLM 读 thinking.enabled、DeepSeek 读 thinking.type），不会把请求置入思考模式，在那里剥掉 tool_choice: 'required' 只会退化其强制工具侧查询——正是 fix(core): harden Qwen 3.8 reasoning effort wire shape #8488 要修复的损害。thinkingMandatory 保持不加门控。
4. 家族一致的 disable 形态：DashScope 上 tier-native 家族的 disable 路径发出 reasoning_effort: 'none'（家族读取的旋钮）而非 enable_thinking: false（家族不读的布尔），并同时剥掉冲突的 thinking_budget。required-thinking 重试触发条件识别新形态，运行时学习照常触发。
5. 可观测性与层析：剥除用户 extra_body 旋钮现在每 generator 记录一次 warn（丢弃的是用户显式配置）；wire 模型家族谓词移入 modalityDefaults.ts（单源，pipeline 非 DashScope 分支也在使用）；provider 的 vision/文本合并尾部共享同一 helper。
6. 文档：供应商拒绝只归因于 thinking_budget（enable_thinking + reasoning_effort 组合在 feat(core): support Qwen 3.8 reasoning effort #8472 的真实 E2E 中被接受），并补充 extra_body 例外说明。

为什么需要

第 1 轮形态仍在两处静默丢弃用户显式配置（#8488 第二轮 review 的发现 1–2），且两个 tool_choice 条款在家族门控上立场相反（发现 3）——非 qwen Token Plan 预设（glm-5.*、kimi-k2.*）都携带 enable_thinking: true，其强制工具侧查询持续丢失 tool_choice: 'required'。

Reviewer 测试计划

如何验证

所有改动均有单元测试覆盖；每个新增/反转测试都经定向变异验证承重（还原对应修复恰好使其变红，不波及其他）：

cd packages/core && npx vitest run src/core/openaiContentGenerator src/core/modalityDefaults.test.ts

• qwen3.8-max + effort 档位 + extra_body: { enable_thinking: false } → 出网仅携带 reasoning_effort: 'none'。
• qwen3.7-max + extra_body: { thinking_budget, reasoning_effort } → 出网保留 enable_thinking/thinking_budget，剥掉惰性 reasoning_effort（文本与视觉分支）。
• glm-5.2 预设形态经真实 provider 端到端：tool_choice: 'required' 保留。
• Disable 路径：qwen3.8-max opt-out 发出 reasoning_effort: 'none'；重试测试以新形态重新钉住运行时学习。
• 多旋钮剥除、快照/别名剥除、非 qwen budget 门控用例（第 1 轮 review 的三条行内建议）。

前后对比证据

N/A（出网形态变更，非用户可见；证据为上述单测与变异验证）。

测试环境

OS	状态
🍏 macOS	✅
🪟 Windows	N/A
🐧 Linux	N/A

环境（可选）

仅单元测试；全仓 npm run typecheck 与定向 eslint/prettier --check 干净。

风险与范围

• 主要风险或权衡：若 DashScope 网关在 kimi 预设上读取 enable_thinking 并在思考模式下拒绝 required，kimi 侧查询将以 400 替代静默退化——代码库与实测均无证据支持该解读，而现状可观察地在非 qwen 预设上丢弃强制工具。已在 fix(core): harden Qwen 3.8 reasoning effort wire shape #8488 线程中明确记录。
• 未验证 / 超出范围：reasoning_effort: 'none' 作为 qwen3.8-max 禁用形态的真实 DashScope 接受性（无 API 访问）；非 DashScope（chat_template_kwargs）disable 分支未改动。
• 破坏性变更 / 迁移说明：无。

关联 Issue

#8488 的后续（第二轮 review 发现）；原始用户请求 #2876。

[qwen-code-ci-bot]

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

[qwen-code-ci-bot]

Thanks for the PR — second round on #8488.

Template ✓ all required sections present.

Problem: real and verified, not theoretical. I traced all three defects in the merged round-1 code myself: an explicit extra_body: { enable_thinking: false } opt-out on the qwen3.8-max family is dropped by the knob-conflict logic (silently re-enabling thinking); on legacy hybrids the drop keeps the inert reasoning_effort and deletes the meaningful thinking_budget; and the pipeline's tool_choice gate strips required for non-qwen presets (glm-5.*, kimi-k2.*) because its enable_thinking === true clause is not family-gated while its reasoning_effort sibling is. Findings 1–3 of the #8488 second review check out.

Direction: aligned — this completes the fix #8488 set out to make: stop silently discarding explicit user configuration, and stop degrading forced-tool side queries for non-qwen models on the shared endpoint. No claude-code CHANGELOG signal for this area, which is expected for a vendor-specific DashScope wire shape; the area is squarely qwen-code's own mission.

Size: core paths (openaiContentGenerator, modalityDefaults). 226 production lines vs 210 test lines vs 16 docs lines — under every threshold, author is a maintainer (fork is the workflow, not a trust signal).

Approach: scope matches the three findings plus the three inline suggestions from round-1 review; nothing unrelated rides along. The consolidation (family predicates moved to modalityDefaults.ts, shared vision/text merge tail) directly serves the single-source goal — the round-1 bug class was exactly "two clauses on opposite sides of the family-gating argument".

Risk: revert-history signal matches high-risk paths: openaiContentGenerator/pipeline.ts and provider/dashscope.ts. That means full review depth and CI evidence before approval; a sandboxed lane is named in the review comment.

Moving on to code review. 🔍

中文说明

感谢贡献——#8488 的第二轮处理。

模板 ✓ 各必填部分齐全。

问题： 真实存在且已核实，并非理论性问题。我在已合并的第 1 轮代码中逐一确认了三处缺陷：qwen3.8-max 家族上显式的 extra_body: { enable_thinking: false } 关闭会被旋钮冲突逻辑删除（思考被静默重新打开）；旧 hybrid 模型上剥除逻辑保留了惰性的 reasoning_effort、却删掉了有意义的 thinking_budget；pipeline 的 tool_choice 门控会剥掉非 qwen 预设（glm-5.*、kimi-k2.*）的 required，因为其 enable_thinking === true 条款没有家族门控而 reasoning_effort 兄弟条款有。#8488 第二轮 review 的发现 1–3 全部属实。

方向： 对齐——这是在完成 #8488 的既定目标：不再静默丢弃用户显式配置，不再退化共享端点上非 qwen 模型的强制工具侧查询。claude-code CHANGELOG 无此领域信号，对 DashScope 供应商专属出网形态属预期之内；该领域正是 qwen-code 自己的核心使命。

规模： 触及核心路径（openaiContentGenerator、modalityDefaults）。生产 226 行、测试 210 行、文档 16 行——低于所有阈值；作者是维护者（fork 只是工作流，不是信任信号问题）。

方案： 范围与三项发现外加第 1 轮 review 的三条行内建议匹配，无夹带改动。收拢（家族谓词移入 modalityDefaults.ts、vision/文本合并尾部共享）直接服务于单源目标——第 1 轮的 bug 类别恰恰是"两个条款在家族门控上立场相反"。

风险： revert 历史信号命中高风险路径：openaiContentGenerator/pipeline.ts 与 provider/dashscope.ts。这意味着批准前需完整 review 深度与 CI 证据；沙箱验证通道已在审查评论中点名。

进入代码审查 🔍

— Qwen Code · qwen3.8-max

_{Reviewed at 440047a9280b3e6f1ed3e18d785e1f43f1ef4ec0 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review

Before reading the diff I sketched my own fix for the three round-2 findings from the PR description alone; the implementation matches that independent proposal point for point, so the assessment below isn't anchored-on-arrival.

What I verified by reading the code:

• Opt-out honoured (finding 1): in dropConflictingThinkingKnobs, an explicit enable_thinking: false on the tier-native family now becomes reasoning_effort: 'none' — and 'none' survives all three downstream guards (the provider's own 'none' early-return, the pipeline's disable-strip exception, the tool-choice gate's !== 'none' clause). Explicit user intent wins over the config tier, and nothing silently re-enables thinking.
• Legacy priority flipped (finding 2): the legacy branch drops the inert reasoning_effort only when a meaningful thinking_budget is present; enable_thinking/thinking_budget survive. With no budget present, reasoning_effort still passes through — round-1 behavior preserved there, and the vision branch resolves conflicts the same way via the shared tail.
• tool_choice gate (finding 3): both field clauses are now family-gated. I checked the qwen-family behavior is unchanged versus round 1, and thinkingMandatory correctly stays ungated (it is explicit "thinking is on" knowledge, model-agnostic by design). The glm-5.2 end-to-end test through the real provider flips tool_choice from dropped to 'required'.
• Disable shape + retry (finding 4): the DashScope disable path emits the tier-native reasoning_effort: 'none' (dropping the conflicting knobs first), and the required-thinking retry trigger recognises the new shape — I walked the retry tests' wire calls and the unchanged assertions still hold under the new shape. thinkingMandatory models still skip the disable path entirely; the non-DashScope chat_template_kwargs branch is untouched.
• Relocation: all four call sites of the moved family predicate are updated; modalityDefaults.ts imports only types and tokenLimits, so no new import cycle; no other consumer anywhere in the repo depends on the old enable_thinking: false wire shape for the tier-native family — the remaining references are config-level preset injection, ACP config mapping, and error-message matching, all unaffected.

No critical blockers found. One cosmetic nit, non-blocking: the once-per-generator warn reads merged['reasoning_effort'] after the delete loop, so on the legacy branch it logs undefined (the dropped value is gone before it is logged), and in the opt-out translation case the "dropped … that conflict" wording fires even though the user's intent was honoured rather than discarded.

Test evidence (the PR's own CI, fetched via API — no PR code was run in this review)

The Linux leg of Qwen Code CI ran the full lint suite, npm run test:ci (full unit suite), and the serve fast-path bundle closure check on the reviewed commit — green. The macOS/Windows legs and CLI integration tests report skipped at PR stage per this repo's CI policy (they are merge-queue/main legs), which is the normal shape for this workflow, not a gap caused by this PR. The mutation-testing evidence in the PR body (every flipped test load-bearing) is the author's claim — CI confirms the suite passes, not that the tests pin the change.

Check	Conclusion
Qwen Code CI / precheck-pr / precheck	success
Qwen Code CI / Test (ubuntu-latest, Node 22.x)	success
Qwen Code CI / Desktop Shell (ubuntu-22.04)	success
Qwen Code CI / web-shell E2E Smoke (ubuntu-latest, Node 22.x)	success
Qwen Code CI / Test (macos-latest, Node 22.x)	skipped
Qwen Code CI / Test (windows-latest, Node 22.x)	skipped
Qwen Code CI / Integration Tests (CLI, No Sandbox)	skipped

Sandboxed verification would settle the remaining gap: @qwen-code /verify — that the flipped and new assertions are load-bearing (the suite goes red against the base build with each fix reverted) is currently the author's mutation claim, and this A/B against the base build is exactly what would prove it. Note that live DashScope acceptance of reasoning_effort: 'none' as a disable is outside what any lane can settle (no API access) — that residual stays a maintainer judgement call, as the PR body itself states.

中文说明

代码审查

读 diff 之前，我先仅凭 PR 描述独立草拟了三项发现的修法；实现与独立方案逐点一致，因此以下评估不是先入为主的确认。

逐条核实：

• 尊重显式关闭（发现 1）：dropConflictingThinkingKnobs 中，tier-native 家族上显式的 enable_thinking: false 现在翻译为 reasoning_effort: 'none'——而 'none' 能穿过全部三道下游守卫（provider 自身的 'none' 提前返回、pipeline disable 剥除的例外、tool_choice 门控的 !== 'none' 条款）。用户显式意图压过配置档位，思考不会被静默重开。
• 旧模型优先级翻转（发现 2）：旧家族分支仅当存在有意义的 thinking_budget 时才剥惰性的 reasoning_effort；enable_thinking/thinking_budget 保留。无 budget 时 reasoning_effort 照常透传——保持第 1 轮行为；vision 分支经共享尾部以同样方式消解冲突。
• tool_choice 门控（发现 3）：两个字段条款都加上了家族门控。核对过 qwen 家族行为与第 1 轮一致，thinkingMandatory 正确地保持不加门控（它是显式的"思考开启"知识，按设计与模型无关）。glm-5.2 经真实 provider 的端到端测试把 tool_choice 从被剥翻转为保留 required。
• disable 形态与重试（发现 4）：DashScope disable 路径对 tier-native 家族发出 reasoning_effort: 'none'（先剥冲突旋钮），required-thinking 重试触发条件识别新形态——走查了重试测试的出网调用，未改动的断言在新形态下依然成立。thinkingMandatory 模型仍完全跳过 disable 路径；非 DashScope 的 chat_template_kwargs 分支未动。
• 搬迁：被移动家族谓词的全部四处调用点均已更新；modalityDefaults.ts 只导入类型与 tokenLimits，无新循环依赖；全仓再无其他消费者依赖 tier-native 家族的旧 enable_thinking: false 出网形态——其余引用均为配置层预设注入、ACP 配置映射、错误消息匹配，均不受影响。

未发现阻断性问题。 一处非阻断的小瑕疵：每 generator 一次的 warn 在删除循环之后读取 merged['reasoning_effort']，旧家族分支上会记到 undefined（被剥的值在打日志前已删除）；且 opt-out 翻译场景下"dropped … that conflict"措辞会在用户意图被尊重（而非丢弃）时也触发。

测试证据（PR 自身 CI，经 API 获取——本审查未运行任何 PR 代码）

Qwen Code CI 的 Linux 段在被审 commit 上跑完完整 lint、npm run test:ci（全量单测）与 serve fast-path bundle closure——绿。macOS/Windows 段与 CLI 集成测试按本仓 CI 策略在 PR 阶段报 skipped（属 merge-queue/main 段），是该 workflow 的正常形态而非本 PR 造成的缺口。PR 正文中的变异验证（每个翻转测试承重）是作者声明——CI 证明套件通过，不证明测试钉住了改动。

（CI 明细见上表）

沙箱验证可以补齐剩余缺口：@qwen-code /verify——"翻转/新增断言承重（还原任一修复后套件相对 base 构建变红）"目前是作者的变异声明，针对 base 构建的 A/B 正是证明这一点的手段。注意 reasoning_effort: 'none' 作为禁用形态的真实 DashScope 接受性不在任何通道能验证的范围内（无 API 访问）——该残留风险如 PR 正文所述，留待维护者判断。

— Qwen Code · qwen3.8-max

_{Reviewed at 440047a9280b3e6f1ed3e18d785e1f43f1ef4ec0 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 4/5 — clean, focused second round; the problem was verified in the merged code before reading the diff; the two open items are a cosmetic warn-log nit and a vendor-side fact no sandbox here can settle.

Stepping back: this is a follow-up whose motivation is auditable — each of the three defects is observable in the merged round-1 code, so the PR is not fixing a phantom. The implementation is what I would have written: translate rather than delete where user intent can be preserved, flip the drop priority toward the knob the model actually reads, and put both tool_choice clauses under the same family gate so they cannot drift apart again. The predicate relocation is not cleanup for its own sake — it removes the "same fact, two homes" drift that produced finding 3. Tests pin the wire body at the client boundary and cover every flipped behavior, including the three round-1 inline suggestions; CI on the reviewed commit is green.

Reservations, stated plainly: (1) the warn-log nit from the review — payload timing on the legacy branch and wording on the translation path; cosmetic, fixable later; (2) live DashScope acceptance of reasoning_effort: 'none' as the disable shape remains unverified for lack of API access — the author says so in the PR body, and the harm this PR removes is observed while the new risk is theoretical. I'm approving because every code-level question resolves from the diff and tests, and the residual is a vendor fact a maintainer with API access can confirm post-merge.

Approving, pinned to the reviewed commit. The Stage 1 high-risk-path flag stands: worth a maintainer's glance at the new disable shape if a live qwen3.8-max session is cheap to run.

中文说明

置信度：4/5 —— 干净、聚焦的第二轮处理；问题在读 diff 之前已在已合并代码中核实；两个未决项是一处 warn 日志的小瑕疵，和一个本环境沙箱无法验证的供应商侧事实。

退一步看：这是一份动机可审计的后续——三个缺陷在已合并的第 1 轮代码中均可观察到，不是在修一个幽灵。实现与我独立草拟的方案一致：能保留用户意图就翻译而非删除、剥除优先级倒向模型真正读取的旋钮、两个 tool_choice 条款置于同一家族门控之下使二者不会再分叉。谓词搬迁不是为清理而清理——它消除了"同一事实、两处存放"的漂移，而发现 3 正是这种漂移的产物。测试在 client 边界钉住出网体，覆盖每个翻转行为，含第 1 轮的三条行内建议；被审 commit 的 CI 为绿。

保留意见，直说：（1）审查中指出的 warn 日志小瑕疵——旧家族分支的载荷时序与翻译路径的措辞；外观问题，可后续修；（2）reasoning_effort: 'none' 作为禁用形态的真实 DashScope 接受性因无 API 访问仍未验证——作者在 PR 正文中如实声明，且本 PR 消除的损害是已观测的，新风险是理论性的。批准的理由是代码层面的所有问题都能从 diff 与测试中得到解答，残留是一个有 API 访问权限的维护者合并后即可确认的供应商事实。

已按被审 commit 钉住批准。第 1 阶段的高风险路径标记保持有效：若方便跑一次真实 qwen3.8-max 会话，值得维护者看一眼新的 disable 形态。

— Qwen Code · qwen3.8-max

_{Reviewed at 440047a9280b3e6f1ed3e18d785e1f43f1ef4ec0 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[yiliang114]

LGTM — round-2 fixes are correct: enable_thinking:false → reasoning_effort:'none' translation honours explicit opt-out; legacy hybrids correctly drop inert reasoning_effort and keep thinking_budget; tool_choice enable_thinking clause now family-gated consistent with its sibling (GLM/DeepSeek read different fields, stripping tool_choice only degraded their side queries); disable shape emits the knob the family actually reads; family predicates centralized in modalityDefaults.ts (single source). Warn-once for user extra_body drops is good observability. Tests cover all six fixes including edge cases (undefined input, case-insensitivity, snapshot/alias matching).

[qwen-code-ci-bot]

Released in v0.21.6.