fix(core): skip enable_thinking=false for thinking-only models - #7333
Closed
qwen-code-dev-bot wants to merge 3 commits into
Closed
fix(core): skip enable_thinking=false for thinking-only models#7333qwen-code-dev-bot wants to merge 3 commits into
qwen-code-dev-bot wants to merge 3 commits into
Conversation
Internal operations (context compaction, goal judge, permission classifier) set includeThoughts=false, which causes the pipeline to send enable_thinking=false to the model API. Thinking-only models like qwen3.8-max-preview reject this with a 400 error. Added a guard: when the model's preset sets extra_body.enable_thinking to true (indicating the model requires thinking), the pipeline no longer overrides it with false. The model uses its default thinking behavior for internal operations instead of erroring. Added regression test verifying enable_thinking is not set to false when extra_body.enable_thinking is true.
qwen-code-dev-bot
requested review from
tanzhenxin and
yiliang114
as code owners
July 20, 2026 11:26
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
requested changes
Jul 20, 2026
Comment on lines
+854
to
+858
| const presetThinking = | ||
| this.contentGeneratorConfig.extra_body?.['enable_thinking']; | ||
| if (presetThinking !== true) { | ||
| typed['enable_thinking'] = false; | ||
| } |
Collaborator
There was a problem hiding this comment.
[Critical] The guard signal extra_body.enable_thinking === true is too broad — it is set for ALL presets with enableThinking: true, not just thinking-only models. Hybrid models like qwen3.7-max, qwen3.7-plus, qwen3.6-plus, and qwen3.6-flash also carry this signal (via provider-config.ts:65-68's buildGenerationConfig), so when reasoningDisabled is true (e.g. a forked query with includeThoughts: false, or a user's reasoning: false config), the guard fires and enable_thinking: false is NOT emitted — the hybrid model keeps thinking enabled, consuming thinking tokens on a call the user explicitly opted out of.
Failure scenario: a user selects qwen3.7-max (hybrid), an internal side-query sets includeThoughts: false, but the request goes out with enable_thinking: true because the preset signals thinking-required. The permission classifier path is particularly concerning — the code comments note it has a "small token budget" and "fail[s] closed" when thinking consumes it.
Suggested fix: Add a thinkingOnly?: boolean field to ModelSpec in providers/types.ts, set it only on qwen3.8-max-preview's preset entries, plumb it through buildGenerationConfig to a new field on ContentGeneratorConfig, and gate the skip on that field instead of reusing extra_body.enable_thinking.
Suggested change
| const presetThinking = | |
| this.contentGeneratorConfig.extra_body?.['enable_thinking']; | |
| if (presetThinking !== true) { | |
| typed['enable_thinking'] = false; | |
| } | |
| const thinkingMandatory = | |
| this.contentGeneratorConfig.thinkingMandatory; | |
| if (!thinkingMandatory) { | |
| typed['enable_thinking'] = false; | |
| } |
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
🤖 Addressed the latest review feedback (round 1/5). What changed, and what I pushed back on: Review Feedback AddressedFeedback:
|
Collaborator
Author
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: No action takenThe feedback for this round contains no review comments, no inline comments, and no issue-level comments — only two failed CI checks. Failed check:
|
| Check | Result |
|---|---|
npm run build |
✅ pass |
npm run typecheck |
✅ pass |
npm run lint |
✅ pass |
pipeline.test.ts (106 tests) |
✅ pass |
provider-config.test.ts (63 tests) |
✅ pass |
alibaba-token-plan.test.ts (2 tests) |
✅ pass |
content-generator-config.test.ts (14 tests) |
✅ pass |
No code changes were made in this round.
中文说明
未采取任何操作
本轮反馈中没有审查评论、没有行内评论、也没有 issue 级别的评论——仅有两个 CI 检查失败。
失败检查：web-shell E2E Smoke (ubuntu-latest Node 22.x)
本 PR 修改了 8 个文件，全部位于 packages/core 下（内容生成器配置、管道、模型类型/常量、提供者预设/配置/类型）。未触及 packages/web-shell 或任何 web-shell UI 特有的代码路径。该失败不能归因于本 PR 的更改。
失败检查：review-pr
自动审查器未产生任何审查反馈（审查、行内评论或 issue 级别评论），这表明审查基础设施本身发生了故障，而非标记了本 PR 中的缺陷。
本地验证（全部通过）
| 检查项 | 结果 |
|---|---|
npm run build |
✅ 通过 |
npm run typecheck |
✅ 通过 |
npm run lint |
✅ 通过 |
pipeline.test.ts（106 个测试） |
✅ 通过 |
provider-config.test.ts（63 个测试） |
✅ 通过 |
alibaba-token-plan.test.ts（2 个测试） |
✅ 通过 |
content-generator-config.test.ts（14 个测试） |
✅ 通过 |
本轮未做任何代码更改。
Base-conflict check: no conflict with main.
🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview
This was referenced Jul 20, 2026
Collaborator
✅ Local build & runtime verification — recommend MERGEBuilt the PR branch ( Summary of the bugInternal operations (context compaction, goal judge, permission classifier) set How I verified (3 levels, A/B against merge-base)[1] Plumbing — the flag is really wired (the gap the PR's own tests don't cover).
[2] Wire-level E2E — real pipeline + real OpenAI SDK client → mock DashScope endpoint that returns the exact upstream 400 whenever the body carries
[3] Unit A/B — the PR's own regression test. On merge-base CI gates (run locally)
Notes (not blockers)
🇨🇳 中文版验证报告（点击展开）✅ 本地构建与运行时验证 —— 建议合并在隔离的 worktree 中构建了 PR 分支（ 问题背景内部操作（上下文压缩、goal judge、权限分类器）会设置 验证方式（3 个层次，均与 merge-base 做 A/B）[1] 接线验证 —— 证明标志真的被接通了（这正是 PR 自带测试没有覆盖的缺口）。
[2] 网络层 E2E —— 真实 pipeline + 真实 OpenAI SDK 客户端 → mock DashScope 服务端，该服务端只要请求体里出现
[3] 单测 A/B —— PR 自带的回归测试。 在 merge-base 的 CI 门禁（本地跑）
说明（非阻塞项）
Verified locally: real worktree build, vitest v3.2.4, node v22. Merge-base overlay A/B on |
wenshao
approved these changes
Jul 20, 2026
Collaborator
Author
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: No action requiredThe only new feedback is a verification report from @wenshao recommending merge. It confirms the fix is correct and properly wired through the full resolution chain, with three levels of A/B verification (plumbing, wire-level E2E, and unit tests) all passing against the merge-base. There are no review comments requesting changes, no inline comments, and no failed CI checks. The two "Notes (not blockers)" observations are informational:
Neither requires a code change in this PR. 中文说明无需操作唯一的新反馈是来自 @wenshao 的验证报告，建议合并。该报告确认修复正确且已正确接通完整的解析链，通过三个层次的 A/B 验证（接线验证、网络层 E2E、单元测试）均在 merge-base 上通过。 没有要求修改的评审意见，没有行内评论，也没有 CI 检查失败。两条「说明（非阻塞项）」为信息性备注：
两者均不需要在本 PR 中进行代码修改。 Base-conflict check: no conflict with main. 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. 3 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: Review Feedback Assessment — PR #7333Result: No action needed. The automated reviewer ( No code changes were made in response to this review round. 中文说明审查反馈评估 — PR #7333结果：无需操作。 自动审查器（ 本轮审查未做任何代码变更。 Base-conflict check: no conflict with main. 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
@qwen-code /triage |
Collaborator
|
Thanks for the PR! Template: the body uses custom headings ( Problem: observed bug with solid evidence. Issue #7332 includes the actual 400 error payload, session ID, timestamp, model name, and a code-level root cause analysis. This is not theoretical — users hitting context compaction on Direction: aligned. Thinking-only models are a real API constraint (DashScope rejects Size: 23 production lines across 7 files, 80 test lines. Well under any threshold. Not applicable. Approach: the scope feels right. A Moving on to code review. 🔍 中文说明感谢贡献！ 模板：PR body 使用了自定义标题（ 问题：已观测到的 bug，证据充分。Issue #7332 包含实际的 400 错误响应、session ID、时间戳、模型名称和代码级根因分析。这不是理论性问题——用户在 方向：对齐。Thinking-only 模型是真实的 API 约束（DashScope 拒绝对其发送 规模：7 个文件 23 行生产代码，80 行测试代码。远低于任何阈值。不适用。 方案：范围合理。在 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
Code ReviewIndependent proposal: I'd add a Findings: No blockers. The change is 23 production lines across 7 files, and every file is load-bearing for the flag to flow through the type system ( Reuse check: the flag reuses the existing Real-Scenario TestingThe exact bug (400 error on Unit tests (106/106 pass, including 2 new regression tests): Installed CLI (before — v0.20.0): Dev build (after — this PR): Both builds respond correctly. The dev build starts cleanly with the PR changes. The specific
中文说明独立方案： 我会在 发现： 无阻塞项。23 行生产代码分布在 7 个文件中，每个文件都是标志在类型系统中传播所必需的。pipeline 守卫正确放置在 DashScope 特定分支内，非 DashScope 服务器不受影响。两个新测试覆盖了正确的对：thinking-only 跳过和混合透传。注释解释了原因并引用 #7332。遵循项目约定。 复用检查： 标志复用了 真实场景测试： 精确 bug（ 单元测试 106/106 通过（含 2 个新回归测试）。安装版 CLI 和 dev 构建均正常响应。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean fix for a real, well-documented bug; minimal scope, correct implementation, solid test coverage. This is a textbook small fix: one flag, one guard, one preset update, two tests. The The issue (#7332) has a real 400 error payload, session ID, and timestamp — this is an observed failure, not a hypothesis. The fix is pragmatic (thinking-only models use thinking mode for internal operations, slightly wasteful but correct) and the PR honestly calls out the tradeoff. Every line in the diff earns its place. Nothing to cut, nothing missing. 中文说明置信度：5/5 — 对真实、有据可查的 bug 的干净修复；范围最小，实现正确，测试覆盖充分。 这是一个教科书式的小修复：一个标志、一个守卫、一个 preset 更新、两个测试。 Issue (#7332) 有真实的 400 错误响应、session ID 和时间戳——这是观测到的故障，不是假设。修复是务实的（thinking-only 模型在内部操作中使用思考模式，略有浪费但正确），PR 诚实地指出了权衡。 diff 中每一行都有其存在价值。无可删减，无遗漏。 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
enabled auto-merge
July 20, 2026 22:38
qqqys
pushed a commit
to qqqys/qwen-code
that referenced
this pull request
Jul 21, 2026
…QwenLM#7358) * fix(ci): stop a slow patrol classifier from killing every flaky rerun The CI Failure Patrol has been effectively offline. Across the last 30 scheduled runs, 28 were cancelled and only 2 succeeded — and both survivors ran at 00:0x, in ~2 minutes, when the model was idle. Step timings show why. Setup, checkout, node and the scan finish in 28 seconds; the model step then runs 9m39s and is killed by the job's 10-minute timeout. Because the job dies there, Validate and Upload never run, the `act` job is skipped on `classify.result != 'success'`, and nothing is ever re-run. One slow step was taking down the whole patrol, every cycle, for hours. That is why QwenLM#7333 still carries a red `web-shell E2E Smoke` whose failure is `No space left on device` — a textbook infra flake, inside the patrol's own TARGET_WORKFLOW, that the patrol never got far enough to re-run. - The classifier step is now bounded at 5 minutes, below the job's 10, and marked continue-on-error: a slow model costs one patrol cycle instead of the patrol, and the next tick simply tries again. - An empty classifier result is reported (has_decisions=false) rather than failing the job, so the run finishes cleanly instead of looking like a broken patrol. - Upload and the `act` job are gated on decisions actually EXISTING, not merely on the classify job having survived — otherwise a no-decision cycle would look actionable. Tests: the step's timeout is asserted to be strictly below the job's, plus the continue-on-error and the has_decisions gating on Upload and `act`; and the Validate step's bash is replayed for real with and without a decisions file, asserting exit 0 and the right flag in both. Mutation-verified — removing the step bound, or the decisions step id, turns it red. * fix(ci): validate JSON syntax in patrol decisions before acting (QwenLM#7358) --------- Co-authored-by: wenshao <wenshao@example.com>
Collaborator
|
Thanks for working through the I’m closing this in favor of #7303, which covers the same model-capability path plus the remaining runtime cases: structured side queries drop |
auto-merge was automatically disabled
July 21, 2026 03:15
Pull request was closed
Collaborator
Reproduction update (2026-07-21)Confirming this fix is still needed — the bug reproduced twice today in the same session (
The |
pull Bot
pushed a commit
to edisplay/qwen-code
that referenced
this pull request
Jul 21, 2026
…wenLM#7355) * feat(autofix): render the managed fleet into the scan's run summary Seeing whether the loop was healthy meant reconstructing it by hand: list the bot's PRs, fetch each one's comments, regex the autofix-eval markers for round and watermark, then cross-check gh pr checks and the fork/takeover state. That is how today's triage of QwenLM#7246, QwenLM#7259, QwenLM#7329, QwenLM#7333 and QwenLM#7336 was done, and it is why a stalled PR stayed invisible until somebody went looking for it. The scan already computes every one of those facts while deciding what to process — it just wrote them to a job log nobody reads. Each per-PR terminal decision now also records a row, and the step renders one markdown table into the run summary: | PR | State | Detail | | QwenLM#7329 | SELECTED | 1 review + 5 inline new (round 0/5) | | QwenLM#7333 | idle | nothing new since 2026-07-20T13:54:18Z | | QwenLM#7262 | waiting | active checks in flight | | QwenLM#7208 | round-capped | round 100/100 - needs a human or @qwen-code /retry | States cover every branch that ends a PR's inspection: busy, skipped, unknown, waiting, round-capped, idle and SELECTED — so a PR cannot drop out of the table by returning early, which is exactly the invisibility this fixes. No new API calls (the data is already in hand), no writes outside the run summary, and the helper is defined at the top of the step so it stays clear of the BUSY_PRS/INSPECTED proximity guard that keeps the free busy-skip from consuming the inspection budget. Tests: the real helper and render block are replayed over fixtures (table structure, one row per state, and an empty fleet still rendering a table), plus each decision branch is pinned to its fleet_row. Mutation-verified: dropping one branch's row turns it red. * fix(autofix): use temp file for fleet test replay; cover fork-head skip (QwenLM#7355) * test(autofix): assert each skipped fleet_row call site individually (QwenLM#7355) * fix(autofix): record fleet rows for both budget-break paths (QwenLM#7355) The candidate-inspection budget break incremented INSPECTED but never called fleet_row, so the PR that tripped the budget was silently absent from the fleet table. The target-budget break left all remaining candidates invisible with no truncation signal. Add a per-PR deferred row before the inspection-budget break and a summary deferred row before the target-budget break so the fleet table stays complete in both cases. * fix(autofix): harden fleet summary render and clean up temp file (QwenLM#7355) Address review feedback: - Escape '|' in detail values to prevent broken table columns - Render budget summary row (PR '-') as em dash instead of '#-' - Add trap for FLEET_FILE cleanup on early exit paths - Document deferred summary row semantics in test comment * fix(autofix): use summary row for candidate-inspection budget break (QwenLM#7355) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #7332.
Internal operations (context compaction, goal judge, permission classifier) set
includeThoughts: false, which causes the pipeline to sendenable_thinking: falseto the model API. Thinking-only models likeqwen3.8-max-previewreject this with a 400 error:Root cause
In
pipeline.tsline 850, thereasoningDisabledpath unconditionally setstyped['enable_thinking'] = falsefor qwen models on DashScope, without checking whether the model supports disabling thinking.Fix
Added a guard: when the model's preset sets
extra_body.enable_thinkingtotrue(indicating the model requires thinking), the pipeline no longer overrides it withfalse. The model uses its default thinking behavior for internal operations instead of erroring.This is a pragmatic fix — internal operations will use thinking mode for thinking-only models (slightly wasteful), but they won't get a 400 error. A future improvement could add a
thinkingOnlyflag toModelSpecfor more granular control.Tests
Added regression test:
skips enable_thinking:false for thinking-only models (#7332). All 105 pipeline tests pass.