Skip to content

fix(core): disable qwen thinking via chat_template_kwargs on non-DashScope servers - #6271

Merged
wenshao merged 3 commits into
QwenLM:mainfrom
beantownbytes:fix/qwen-thinking-non-dashscope
Jul 4, 2026
Merged

fix(core): disable qwen thinking via chat_template_kwargs on non-DashScope servers#6271
wenshao merged 3 commits into
QwenLM:mainfrom
beantownbytes:fix/qwen-thinking-non-dashscope

Conversation

@beantownbytes

@beantownbytes beantownbytes commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

What this PR does

For hybrid-thinking qwen models, the "disable thinking" switch is now delivered in a way that self-hosted OpenAI-compatible servers actually honour. When the wire model is a qwen* model or coder-model and the provider is not DashScope, the pipeline sets chat_template_kwargs.enable_thinking = false (merged with any existing chat_template_kwargs) instead of the top-level enable_thinking field, and strips any top-level enable_thinking a provider preset injected via extra_body so the two signals cannot contradict. The DashScope path is unchanged.

Why it's needed

Previously enable_thinking: false was only emitted for DashScope providers, and always as a top-level request field. Self-hosted OpenAI-compatible servers (vLLM, SGLang) render the chat template server-side and read the thinking switch from chat_template_kwargs; they silently ignore a top-level enable_thinking. So on a self-hosted qwen endpoint, thinking is never actually disabled.

The user-facing failure is that auto-approval mode is unusable against such an endpoint. The permission classifier issues short structured-output calls with a small token budget; because thinking stays on, the model spends that budget emitting <think>, returns empty or truncated JSON, and every tool call fails closed.

Reviewer Test Plan

How to verify

Point qwen-code at a self-hosted vLLM (or SGLang) server serving a hybrid-thinking qwen model such as Qwen3.6-27B, configured as an OpenAI-compatible provider (non-DashScope base URL), and enable auto-approval mode.

Before this change: the auto-mode permission classifier calls come back with content: null (the model consumes the token budget on <think>), and auto mode fails closed on every tool call. After this change: the classifier returns valid JSON and auto mode works, because the server sees chat_template_kwargs.enable_thinking = false and stops emitting reasoning.

Unit coverage: npx vitest run packages/core/src/core/openaiContentGenerator/pipeline.test.ts (83 passing). New/updated cases assert that a qwen* model and a coder-model on a non-DashScope endpoint emit chat_template_kwargs: { enable_thinking: false } with no top-level enable_thinking, and that a top-level enable_thinking: true injected via extra_body is stripped on this path.

Evidence (Before & After)

Not a TUI change. Observable request/response difference on a self-hosted vLLM endpoint: before, the classifier response is content: null; after, it is valid JSON (e.g. {"decision":"allow"}). Behavior is pinned by the unit tests above.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

macOS: unit tests + live verification against a vLLM v0.22.1 server. Windows/Linux: not run locally; the change is pure request-shaping logic covered by cross-platform unit tests in CI.

Environment (optional)

Local unit tests via npx vitest; live check against vLLM v0.22.1 serving Qwen3.6-27B NVFP4.

Risk & Scope

  • Main risk or tradeoff: the non-DashScope branch now sends chat_template_kwargs on every non-DashScope qwen request. Servers that do not recognise the field ignore it (harmless no-op); this is the same convention vLLM and SGLang already use. It also deletes a top-level enable_thinking on this path, which is intentional (the field is qwen-specific and should not leak top-level to non-DashScope servers).
  • Not validated / out of scope: live-verified only against vLLM v0.22.1; SGLang, LiteLLM proxies, and Ollama are covered by unit tests but not exercised live. Intentionally not gated on a known-server allowlist: self-hosted endpoint URLs are arbitrary, so an allowlist would be fragile and would exclude legitimate deployments while providing little benefit over relying on unknown-field tolerance.
  • Breaking changes / migration notes: none. The DashScope path is byte-for-byte unchanged; the non-DashScope path previously delivered no working thinking-disable at all.

Linked Issues

Related to #4676 (auto-mode classifier timeouts / stage-2 thinking), which routes through the same DashScope-only path and does not reach self-hosted servers. No closing keyword; this is a distinct fix.

中文说明

这个 PR 做了什么

对于混合思考(hybrid-thinking)的 qwen 模型,现在以自托管的 OpenAI 兼容服务器能真正识别的方式来传递"禁用思考"开关。当线上(wire)模型是 qwen* 模型或 coder-model 且提供方不是 DashScope 时,pipeline 会设置 chat_template_kwargs.enable_thinking = false(与已有的 chat_template_kwargs 合并),而不是使用顶层的 enable_thinking 字段;同时会移除提供方预设通过 extra_body 注入的任何顶层 enable_thinking,以免两个信号相互矛盾。DashScope 路径保持不变。

为什么需要它

此前 enable_thinking: false 只对 DashScope 提供方发送,并且总是作为顶层请求字段。自托管的 OpenAI 兼容服务器(vLLM、SGLang)在服务端渲染聊天模板,并从 chat_template_kwargs 读取思考开关;它们会静默忽略顶层的 enable_thinking。因此在自托管的 qwen 端点上,思考实际上从未被禁用。

面向用户的故障是:自动批准(auto-approval)模式在这类端点上无法使用。权限分类器(permission classifier)会发出预算很小的短结构化输出请求;由于思考仍然开启,模型把预算花在输出 <think> 上,返回空的或被截断的 JSON,导致每次工具调用都失败关闭(fail closed)。

审查者测试计划

如何验证

将 qwen-code 指向一个自托管的 vLLM(或 SGLang)服务器,服务一个混合思考的 qwen 模型(如 Qwen3.6-27B),配置为 OpenAI 兼容提供方(非 DashScope 的 base URL),并启用自动批准模式。

改动之前:自动模式的权限分类器请求返回 content: null(模型把 token 预算消耗在 <think> 上),每次工具调用都失败关闭。改动之后:分类器返回有效的 JSON,自动模式正常工作,因为服务器看到 chat_template_kwargs.enable_thinking = false 并停止输出推理内容。

单元测试覆盖:npx vitest run packages/core/src/core/openaiContentGenerator/pipeline.test.ts(83 个通过)。新增/更新的用例断言:非 DashScope 端点上的 qwen* 模型和 coder-model 会发送 chat_template_kwargs: { enable_thinking: false } 且没有顶层 enable_thinking;并且通过 extra_body 注入的顶层 enable_thinking: true 会在该路径上被移除。

证据(前后对比)

不是 TUI 改动。在自托管 vLLM 端点上可观察到的请求/响应差异:之前分类器响应为 content: null;之后为有效 JSON(例如 {"decision":"allow"})。该行为由上述单元测试固定。

测试平台

操作系统 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

macOS:单元测试 + 针对 vLLM v0.22.1 服务器的实机验证。Windows/Linux:本地未运行;该改动是纯请求构造逻辑,由 CI 中的跨平台单元测试覆盖。

环境(可选)

通过 npx vitest 运行本地单元测试;针对服务 Qwen3.6-27B NVFP4 的 vLLM v0.22.1 做了实机检查。

风险与范围

  • 主要风险或权衡:非 DashScope 分支现在会在每个非 DashScope 的 qwen 请求上发送 chat_template_kwargs。不识别该字段的服务器会忽略它(无害的空操作);这正是 vLLM 和 SGLang 已经采用的约定。它还会在该路径上删除顶层的 enable_thinking,这是有意为之(该字段是 qwen 专有的,不应以顶层形式泄漏到非 DashScope 服务器)。
  • 未验证 / 超出范围:仅针对 vLLM v0.22.1 做了实机验证;SGLang、LiteLLM 代理和 Ollama 由单元测试覆盖但未做实机验证。有意不基于已知服务器白名单来门控:自托管端点 URL 是任意的,白名单会很脆弱,会把合法部署排除在外,且相比依赖"忽略未知字段"的容错几乎没有额外收益。
  • 破坏性变更 / 迁移说明:无。DashScope 路径逐字节保持不变;非 DashScope 路径此前根本没有可用的禁用思考功能。

关联 Issue

#4676(自动模式分类器超时 / 第二阶段思考)相关,该 issue 走的是同一条仅限 DashScope 的路径,无法到达自托管服务器。不使用关闭关键字;这是一个独立的修复。

…Scope servers

For hybrid-thinking qwen models, enable_thinking:false was only emitted for
DashScope providers, always as a top-level request field. Self-hosted
OpenAI-compatible servers (vLLM, SGLang) render the chat template server-side
and read the switch from chat_template_kwargs; they silently ignore a
top-level enable_thinking.

The visible failure is that auto-approval mode is unusable against a
self-hosted qwen endpoint: the permission classifier issues short
structured-output calls with a small token budget, and because thinking is
never disabled the model spends that budget emitting <think>, returns
truncated JSON, and every tool call fails closed.

When the model is a qwen/coder-model and the provider is not DashScope, set
chat_template_kwargs.enable_thinking = false (merged with any existing
chat_template_kwargs) instead of the top-level field. DashScope behavior is
unchanged. Adds a unit test for the non-DashScope path.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @beantownbytes, thanks for the PR — the fix itself looks well-targeted, but the PR body doesn't follow our pull request template.

The body uses custom headings (Problem / Fix / Notes) instead of the template's required sections. The missing ones are:

  • What this PR does / Why it's needed — your Problem/Fix sections cover this content, but we need the standard headings for consistency
  • Reviewer Test Plan — this is the big one. The template requires "How to verify", "Evidence (Before & After)", and a "Tested on" table. Your mention of vLLM verification is helpful context but doesn't substitute for a structured test plan that reviewers can follow
  • Risk & Scope — main risk/tradeoff, what's out of scope, breaking changes
  • Linked Issues — reference related issues (you mention #4676 in Notes, which should go here)
  • 中文说明 — the Chinese translation in the <details> block

Could you restructure the body to match the template? The content is there — it just needs the right format so maintainers can review efficiently.

中文说明

你好 @beantownbytes,感谢你的 PR!修复方向很精准,但 PR 正文没有使用我们的 PR 模板

目前使用的是自定义标题(Problem / Fix / Notes),缺少模板要求的以下部分:

  • What this PR does / Why it's needed — 你的 Problem/Fix 部分已覆盖这些内容,但需要使用标准标题以保持一致性
  • Reviewer Test Plan — 这是最重要的部分。模板要求包含"How to verify"、"Evidence (Before & After)"以及"Tested on"表格。你提到了 vLLM 验证,这是有帮助的背景信息,但不能替代一个结构化的、审查者可以跟随的测试计划
  • Risk & Scope — 主要风险/权衡、不在范围内的内容、破坏性变更
  • Linked Issues — 引用相关 issue(你在 Notes 中提到了 #4676,应该放在这里)
  • 中文说明<details> 块中的中文翻译

能否按照模板重新组织正文?内容都在——只是需要正确的格式,以便维护者高效审查。

Qwen Code · qwen3.7-max

)
) {
typed['enable_thinking'] = false;
} else {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] The non-DashScope branch sets chat_template_kwargs: { enable_thinking: false } but does not strip any top-level enable_thinking that extra_body may have already injected into the request.

When a provider preset (e.g. ModelScope's Qwen/Qwen3.5-397B-A17B) has enableThinking: true, provider-config.ts compiles it into extra_body: { enable_thinking: true }, and DefaultOpenAICompatibleProvider.buildRequest spreads that into the top-level request. The resulting wire request then carries both "enable_thinking": true (top-level) and "chat_template_kwargs": { "enable_thinking": false } — contradictory signals.

While vLLM/SGLang may ignore the top-level field (as the comment notes), other OpenAI-compatible servers (ModelScope, LiteLLM proxies) that read both could keep thinking enabled despite the explicit opt-out. The new test doesn't catch this because its mock doesn't inject enable_thinking: true via extra_body, so expect(apiCall.enable_thinking).toBeUndefined() passes trivially.

Suggested change
} else {
} else {
// Non-DashScope OpenAI-compatible servers (vLLM, SGLang, ...) render
// the model's chat template server-side and read the thinking switch
// from `chat_template_kwargs`, not a top-level `enable_thinking`
// (which they silently ignore). Send it there so hybrid qwen models
// actually stop emitting <think> when reasoning is disabled — e.g.
// the auto-mode permission classifier's short structured-output
// calls, which otherwise spend their small token budget on thinking
// and fail closed.
// Strip any top-level enable_thinking injected by extra_body
// (provider-config.ts sets it for models with enableThinking: true).
delete typed['enable_thinking'];
const existing = (typed['chat_template_kwargs'] ?? {}) as Record<
string,
unknown
>;
typed['chat_template_kwargs'] = {
...existing,
enable_thinking: false,
};
}

— qwen3.7-max via Qwen Code /review

expect(apiCall.enable_thinking).toBeUndefined();
});

it('disables qwen thinking via chat_template_kwargs on a non-DashScope endpoint (vLLM/SGLang)', async () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The only coder-model test (line ~986) uses AuthType.QWEN_OAUTH, which causes isDashScopeProvider to return true. The || model === 'coder-model' branch inside the non-DashScope else arm is therefore never exercised in tests.

Consider adding a test with model: 'coder-model', baseUrl: 'https://llm.example.com/v1' (non-DashScope), includeThoughts: false, asserting chat_template_kwargs equals { enable_thinking: false }.

— qwen3.7-max via Qwen Code /review

) {
typed['enable_thinking'] = false;
} else {
// Non-DashScope OpenAI-compatible servers (vLLM, SGLang, ...) render

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] chat_template_kwargs is a vLLM/SGLang convention, but this else-branch fires for every non-DashScope endpoint — including LiteLLM proxies, Ollama, and other OpenAI-compatible servers. Most will silently ignore unknown fields (harmless), but servers that validate request bodies strictly could reject the request.

Consider documenting this assumption in the comment, or gating the field on a known server list.

— qwen3.7-max via Qwen Code /review

…cope path

Address review: when a qwen model runs against a non-DashScope endpoint and a
provider preset injected enable_thinking:true via extra_body, emitting only
chat_template_kwargs.enable_thinking=false left the contradictory top-level
field in place. Servers that honour both signals could keep thinking enabled
despite the opt-out. Delete the top-level field on this path so the nested
switch is authoritative, matching the codebase's rule of not leaking the
qwen-specific enable_thinking field to non-DashScope servers.

Also covers the coder-model non-DashScope arm with a dedicated test, and
strengthens the vLLM test to inject a top-level enable_thinking:true and
assert it is stripped.
@beantownbytes

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Addressed in 652ab68, and the PR body is restructured to the template.

[Critical] top-level enable_thinking not stripped - fixed. The non-DashScope branch now delete typed['enable_thinking'] before setting chat_template_kwargs, so an extra_body-injected enable_thinking: true (from a preset with enableThinking: true) can't contradict the opt-out. This also keeps the path from leaking the qwen-specific top-level field to non-DashScope servers, consistent with the existing gate rationale. The vLLM test now injects enable_thinking: true via mockProvider.buildRequest and asserts it is stripped, so the assertion is no longer trivially true.

[Suggestion] coder-model non-DashScope arm untested - added a dedicated test: model: 'coder-model' on https://llm.example.com/v1 with includeThoughts: false, asserting chat_template_kwargs equals { enable_thinking: false } and no top-level enable_thinking.

[Suggestion] gate chat_template_kwargs on a known-server list - documented the assumption in the comment (unknown field is ignored by servers that don't recognise it), but intentionally not adding an allowlist. Self-hosted endpoint URLs are arbitrary, so a hostname/server allowlist would be fragile and would exclude legitimate vLLM/SGLang deployments while adding little over relying on unknown-field tolerance. If a strict-validation server ever needs an opt-out, a per-provider config flag would be the right lever, not a hardcoded list. Open to changing this if you'd prefer a gate.

wenshao
wenshao previously approved these changes Jul 3, 2026
);
(mockClient.chat.completions.create as Mock).mockResolvedValue({
id: 'r',
choices: [{ message: { content: 'ok' }, finish_reason: 'stop' }],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The test now verifies that a top-level enable_thinking: true injected via extra_body gets stripped — nice improvement. However, the chat_template_kwargs merge with pre-existing keys (other than enable_thinking) is still untested.

The production code at pipeline.ts spreads ...existing before appending enable_thinking: false. Consider adding a test case where mockProvider.buildRequest also returns chat_template_kwargs: { apply_chat_template: true } and asserting:

expect(apiCall.chat_template_kwargs).toEqual({ apply_chat_template: true, enable_thinking: false });

This guards against a future refactor that drops the ...existing spread and silently loses user-configured kwargs.

— qwen3.7-max via Qwen Code /review

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No review findings. The fix correctly restructures the thinking-disable logic for non-DashScope servers and is well-covered by tests (83 passing). Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

Address review: assert the else-branch spreads pre-existing
chat_template_kwargs before appending enable_thinking:false, so a refactor
that drops the spread and loses user-configured kwargs is caught. Injects
chat_template_kwargs:{apply_chat_template:true} via buildRequest and expects
the merged {apply_chat_template:true, enable_thinking:false}.
@beantownbytes

Copy link
Copy Markdown
Contributor Author

Good call - added in 874553a. New test injects chat_template_kwargs: { apply_chat_template: true } via mockProvider.buildRequest and asserts the merged { apply_chat_template: true, enable_thinking: false }, so a refactor that drops the ...existing spread and loses user-configured kwargs would fail. 84 tests passing. Thanks for the review and the approval.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No review findings. The fix correctly restructures the thinking-disable logic for non-DashScope servers: chat_template_kwargs: { enable_thinking: false } replaces the top-level field that vLLM/SGLang ignore, the delete typed['enable_thinking'] properly strips any contradictory provider-preset injection, and the merge with pre-existing chat_template_kwargs preserves user-configured keys. Test coverage is solid (84 passing) — qwen model, coder-model alias, and kwargs merge are all exercised on the non-DashScope path. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR, @beantownbytes!

Template looks good ✓

Problem: This is a real, observed bug — not theoretical hardening. Self-hosted OpenAI-compatible servers (vLLM, SGLang) read the thinking-disable switch from chat_template_kwargs, silently ignoring the top-level enable_thinking field. The PR describes the concrete failure: auto-approval mode's permission classifier sends short structured-output calls with a small token budget, the model spends it all on <think>, and every tool call fails closed with content: null. Related to #4676.

Direction: Clearly aligned with qwen-code's core mission — users pointing the CLI at self-hosted qwen endpoints expect auto-mode to work. The DashScope path already had this; extending it to non-DashScope servers closes a real gap. No CHANGELOG reference (this is qwen-specific plumbing), but the area is clearly in-scope.

Approach: Scope is tight — 2 files, one focused change. The restructuring is the minimal fix: split the existing if (DashScope && qwen) gate into a DashScope/non-DashScope fork, each emitting the field in the shape its server ecosystem expects. The delete typed['enable_thinking'] on the non-DashScope path is a good defensive touch (prevents a preset with enableThinking: true from contradicting the opt-out via extra_body). Tests cover all three arms: vLLM/SGLang, coder-model, and merge with pre-existing kwargs.

One note: the inline comments in the non-DashScope branch are thorough but verbose (~15 lines of prose for ~8 lines of code). Not a blocker — just flagging for the author's awareness in case they want to tighten it up.

Moving on to code review. 🔍

中文说明

感谢 PR!

模板完整 ✓

问题: 这是一个真实存在的 bug,不是理论性加固。自托管的 OpenAI 兼容服务器(vLLM、SGLang)从 chat_template_kwargs 读取禁用思考的开关,会静默忽略顶层的 enable_thinking 字段。PR 描述了具体故障:自动批准模式的权限分类器发出 token 预算很小的短结构化输出请求,模型把预算全花在 <think> 上,每次工具调用都以 content: null 失败关闭。与 #4676 相关。

方向: 与 qwen-code 的核心使命明确对齐——用户把 CLI 指向自托管 qwen 端点时,期望自动模式能正常工作。DashScope 路径已经有了这个功能;扩展到非 DashScope 服务器填补了一个真实的缺口。CHANGELOG 无直接参考(这是 qwen 特有的底层逻辑),但该领域明确在范围内。

方案: 范围紧凑——2 个文件,一个聚焦的改动。重构是最小修复:将现有的 if (DashScope && qwen) 门控拆分为 DashScope/非 DashScope 两个分支,各自以其服务器生态期望的格式发送字段。非 DashScope 路径上的 delete typed['enable_thinking'] 是一个好的防御性处理(防止 enableThinking: true 的预设通过 extra_body 注入矛盾的顶层字段)。测试覆盖了三个场景:vLLM/SGLang、coder-model、以及与已有 kwargs 的合并。

一个观察:非 DashScope 分支中的内联注释详尽但冗长(约 15 行文字对应约 8 行代码)。不阻塞——仅提醒作者考虑是否可以精简。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

The diff restructures the thinking-disable logic in pipeline.ts cleanly. The existing if (DashScope && qwen_model) gate becomes a two-arm if/else, each emitting the field in the shape its server ecosystem expects:

  • DashScope arm (unchanged): top-level enable_thinking = false
  • Non-DashScope arm (new): chat_template_kwargs.enable_thinking = false, with delete typed['enable_thinking'] to strip any contradictory top-level field injected via extra_body

The delete before the spread-merge is important — a provider preset with enableThinking: true would otherwise inject a top-level enable_thinking: true that contradicts the kwargs opt-out on servers that honour both. The existing-kwargs spread (...existing) is safe and tested.

Reuse check: the non-DashScope branch builds chat_template_kwargs inline. No existing shared utility covers this — the shape is provider-specific and the code is adjacent to the DashScope arm it mirrors. No reuse concern.

No blockers found. The logic is correct, well-scoped, and consistent with the surrounding code patterns.

Test Results

Before (main code + PR's new tests)

Tests run against main branch production code with the PR's 3 new test cases applied. All 3 new tests fail — chat_template_kwargs is undefined because the non-DashScope path never set it:

 ❯ ContentGenerationPipeline > execute (84 tests | 3 failed)
   × disables qwen thinking via chat_template_kwargs on a non-DashScope endpoint (vLLM/SGLang) 12ms
     → expected undefined to deeply equal { enable_thinking: false }
   × disables coder-model thinking via chat_template_kwargs on a non-DashScope endpoint 1ms
     → expected undefined to deeply equal { enable_thinking: false }
   × merges enable_thinking into pre-existing chat_template_kwargs on a non-DashScope endpoint 2ms
     → expected { apply_chat_template: true } to deeply equal { apply_chat_template: true, …(1) }

 Test Files  1 failed (1)
      Tests  3 failed | 81 passed (84)

After (this PR)

All 84 tests pass with the PR's production code change:

 ✓ src/core/openaiContentGenerator/pipeline.test.ts (84 tests) 95ms

 Test Files  1 passed (1)
      Tests  84 passed (84)

Note: This is a request-shaping fix, not a TUI change. Live verification requires a self-hosted vLLM/SGLang endpoint, which is not available in this CI environment. The unit tests pin the exact request body shape and are sufficient to verify the fix.

中文说明

代码审查

diff 清晰地重构了 pipeline.ts 中的禁用思考逻辑。现有的 if (DashScope && qwen_model) 门控变为两个分支的 if/else,各自以其服务器生态期望的格式发送字段:

  • DashScope 分支(未变):顶层 enable_thinking = false
  • 非 DashScope 分支(新增):chat_template_kwargs.enable_thinking = false,并通过 delete typed['enable_thinking'] 移除任何通过 extra_body 注入的矛盾顶层字段

delete 在 spread 合并之前很重要——带有 enableThinking: true 的提供方预设会注入矛盾的顶层 enable_thinking: true。已有 kwargs 的 spread(...existing)是安全的且已测试覆盖。

复用检查: 非 DashScope 分支内联构建 chat_template_kwargs。没有现有共享工具覆盖此场景——该格式是提供方特定的,代码与它镜像的 DashScope 分支相邻。无复用问题。

未发现阻塞问题。 逻辑正确,范围合理,与周围代码模式一致。

测试结果

改动前(main 代码 + PR 新测试)

在 main 分支的生产代码上运行 PR 的 3 个新测试用例。全部 3 个新测试失败——chat_template_kwargsundefined,因为非 DashScope 路径从未设置它:

 ❯ ContentGenerationPipeline > execute (84 tests | 3 failed)
   × disables qwen thinking via chat_template_kwargs on a non-DashScope endpoint (vLLM/SGLang)
   × disables coder-model thinking via chat_template_kwargs on a non-DashScope endpoint
   × merges enable_thinking into pre-existing chat_template_kwargs on a non-DashScope endpoint

改动后(本 PR)

所有 84 个测试通过:

 ✓ src/core/openaiContentGenerator/pipeline.test.ts (84 tests) 95ms

注: 这是请求格式修复,不是 TUI 改动。实时验证需要自托管 vLLM/SGLang 端点,CI 环境不具备此条件。单元测试固定了精确的请求体格式,足以验证修复。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

This PR does exactly what it says: fixes a real bug where auto-mode is broken against self-hosted qwen endpoints because the thinking-disable switch was only being sent in the shape DashScope understands, not the shape vLLM/SGLang reads.

My independent proposal for this fix would have been identical — restructure the existing if (DashScope && qwen) gate into a two-arm fork, one per server ecosystem. The PR goes one step further by also stripping any contradictory top-level enable_thinking that a provider preset might inject, which I would have missed. That's a good defensive touch.

The before/after is clean: 3 new tests fail on main (the non-DashScope path never set chat_template_kwargs), and all 84 pass with the fix. The code is straightforward, the comments are thorough (slightly verbose, but not a concern), and the scope is tight.

The author has been responsive to prior review feedback — addressed the critical item (top-level stripping), added the missing coder-model test, and documented the no-allowlist design decision with clear reasoning.

Verdict: Approve. The fix is correct, focused, and well-tested. Ships a real user-facing improvement with no risk to the existing DashScope path. ✅

中文说明

这个 PR 完成了它声称的功能:修复了一个真实的 bug——自动模式在自托管 qwen 端点上不可用,因为禁用思考的开关只以 DashScope 理解的格式发送,而不是 vLLM/SGLang 读取的格式。

我对这个修复的独立方案会完全相同——将现有的 if (DashScope && qwen) 门控重构为两个分支,每个服务器生态一个。PR 更进一步,还移除了提供方预设可能注入的任何矛盾的顶层 enable_thinking,这是我可能会遗漏的。这是一个好的防御性处理。

Before/after 很清晰:3 个新测试在 main 上失败(非 DashScope 路径从未设置 chat_template_kwargs),修复后全部 84 个通过。代码直白,注释详尽(稍微冗长,但不成问题),范围紧凑。

作者对先前的审查反馈响应及时——解决了关键问题(顶层字段移除)、添加了缺失的 coder-model 测试、并用清晰的推理记录了不做白名单的设计决策。

结论: 批准。修复正确、聚焦、测试充分。带来了真实的用户侧改进,对现有 DashScope 路径无风险。✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@wenshao
wenshao added this pull request to the merge queue Jul 4, 2026
Merged via the queue into QwenLM:main with commit 015ee42 Jul 4, 2026
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants