Skip to content

fix(core): resolve Qwen 3.8 reasoning budget conflicts - #8525

Merged
DragonnZhang merged 26 commits into
QwenLM:mainfrom
DragonnZhang:dragon/fix-qwen38-thinking-budget-precedence
Aug 12, 2026
Merged

fix(core): resolve Qwen 3.8 reasoning budget conflicts#8525
DragonnZhang merged 26 commits into
QwenLM:mainfrom
DragonnZhang:dragon/fix-qwen38-thinking-budget-precedence

Conversation

@DragonnZhang

Copy link
Copy Markdown
Collaborator

What this PR does

This change prevents DashScope Qwen 3.8 requests from carrying both reasoning_effort and thinking_budget when those settings come from different configuration layers. It applies the existing extra_body > request sampling parameters > reasoning precedence, keeps unrelated thinking settings intact, and documents how conflicts and explicit same-layer pairs are handled.

Why it's needed

Qwen 3.8 effort support introduced a flat reasoning_effort field, but existing configurations may already set thinking_budget. DashScope rejects requests that contain both fields, so selecting an effort could turn a previously valid configuration into an invalid API request. This follow-up to #8472 preserves the higher-priority user setting instead of emitting the conflicting pair.

Reviewer Test Plan

How to verify

Configure a DashScope Qwen 3.8 model with a nested reasoning effort and a higher-priority thinking_budget in either samplingParams or extra_body. Confirm that the resulting request retains the budget, omits both flat and nested effort fields, and preserves enable_thinking when configured. Then place reasoning_effort in extra_body with a lower-priority request-level budget and confirm that the request retains only the effort. Existing Qwen 3.8 effort-only requests should continue to pass all five effort tiers through unchanged.

Evidence (Before & After)

N/A — request-construction behavior with no UI change. Before this fix the conflicting configurations emitted both fields; after this fix the higher-priority field is the only one emitted.

Tested on

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

Environment (optional)

Local Node.js workspace with the focused DashScope provider and OpenAI pipeline Vitest suites, package build, typecheck, ESLint, Prettier, and diff checks.

Risk & Scope

  • Main risk or tradeoff: conflict resolution is intentionally limited to Qwen 3.8 tiered-effort requests and only removes a field when configuration-layer precedence is unambiguous.
  • Not validated / out of scope: no live Qwen 3.8 Preview API request was sent; explicitly setting both conflicting fields in the same configuration layer remains a user configuration error.
  • Breaking changes / migration notes: none.

Linked Issues

Follow-up to #8472.

中文说明

这个 PR 做了什么

当 DashScope Qwen 3.8 的 reasoning_effortthinking_budget 来自不同配置层时,这个改动会防止请求同时携带两个字段。它按现有的 extra_body > 请求采样参数 > reasoning 优先级处理冲突,保留无关的思考设置,并记录冲突和同层显式配置的处理方式。

为什么需要这个改动

Qwen 3.8 effort 支持引入了顶层 reasoning_effort 字段,但现有配置可能已经设置 thinking_budget。DashScope 会拒绝同时包含两个字段的请求,因此选择 effort 可能使之前有效的配置变成无效 API 请求。作为 #8472 的后续修复,这个改动会保留优先级更高的用户设置,而不是发送冲突字段对。

Reviewer 测试计划

如何验证

为 DashScope Qwen 3.8 模型配置嵌套 reasoning effort,并在 samplingParamsextra_body 中设置优先级更高的 thinking_budget。确认最终请求保留 budget,删除顶层和嵌套 effort 字段,并在已配置时保留 enable_thinking。然后在 extra_body 中设置 reasoning_effort,同时在优先级更低的请求层设置 budget,确认请求只保留 effort。现有只配置 effort 的 Qwen 3.8 请求应继续原样传递全部五个 effort 档位。

证据(修复前与修复后)

不适用——这是没有 UI 变化的请求构建行为。修复前,冲突配置会同时发送两个字段;修复后,只会发送优先级更高的字段。

测试平台

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

环境(可选)

本地 Node.js 工作区;已运行 DashScope provider 和 OpenAI pipeline 的定向 Vitest 测试、package build、typecheck、ESLint、Prettier 和 diff 检查。

风险与范围

  • 主要风险或取舍:冲突处理特意限制在 Qwen 3.8 分档 effort 请求中,只有在配置层优先级毫无疑义时才会删除字段。
  • 未验证或超出范围:未向真实 Qwen 3.8 Preview API 发送请求;在同一配置层显式设置两个冲突字段仍属于用户配置错误。
  • 破坏性变更或迁移说明:无。

关联 Issue

#8472 的后续修复。

# Conflicts:
#	docs/users/configuration/model-providers.md
#	packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts
#	packages/core/src/core/openaiContentGenerator/provider/dashscope.ts
# Conflicts:
#	docs/users/configuration/model-providers.md
#	packages/core/src/core/openaiContentGenerator/provider/dashscope.ts
@DragonnZhang
DragonnZhang marked this pull request as ready for review August 5, 2026 02:18
@DragonnZhang
DragonnZhang enabled auto-merge August 5, 2026 02:18
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

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

@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.

@DragonnZhang Thanks for the follow-up work here — but the problem this PR describes was already fixed on main before this review ran.

The stated problem no longer exists on the current base. The PR says "selecting an effort could turn a previously valid configuration into an invalid API request" and that before this fix "the conflicting configurations emitted both fields." That was true in the window between #8472 and #8488 — but #8488 (fix(core): harden Qwen 3.8 reasoning effort wire shape, merged 2026-08-04) already landed: dropConflictingThinkingKnobs guarantees a tiered Qwen 3.8 request never ships reasoning_effort + thinking_budget together (the tier ships alone; every dropped knob is warn-logged). Your diff sits on top of #8488 and rewrites its tests — yet the PR body still describes the pre-#8488 world and never mentions #8488.

What remains is a proposal to flip two deliberate design choices that just shipped:

  • Cross-layer conflicts#8488 resolves them tier-wins (the losing knob is dropped with a warn log). This PR resolves them by layer precedence instead, and both of its resolution paths delete the losing field before the warn logic runs — so conflicts #8488 logs become silent. In particular, a configured extra_body.thinking_budget would make /effort selections quietly no-op.
  • Explicit same-layer pairs#8488 resolves them into a valid request. This PR preserves the pair, so configurations that work today would start returning DashScope 400s on principle.

Both are defensible design opinions, but they contradict a decision that went through a full review cycle hours before this triage ran. Reversing a just-merged design needs to argue against that design explicitly — with a concrete configuration harmed by #8488's behavior — not against a conflict shape that no longer ships.

Could you restate the problem against current main? Concretely: a real configuration where #8488's tier-wins + warn behavior hurts a user, and why a silent /effort no-op and a deliberate 400 on same-layer pairs are better outcomes. If the core concern is precedence consistency, an issue discussing the resolution policy with the maintainer who shipped #8488 is the right venue before more code.

中文说明

@DragonnZhang 感谢跟进——但本 PR 描述的问题在本次审查运行前已被 main 修复。

所述问题在当前基线上已不存在。 PR 说"选择 effort 可能使之前有效的配置变成无效 API 请求"、"修复前冲突配置会同时发送两个字段"。这在 #8472#8488 之间的窗口内属实——但 #8488fix(core): harden Qwen 3.8 reasoning effort wire shape,2026-08-04 合并)已经落地:dropConflictingThinkingKnobs 已保证 Qwen 3.8 分档请求绝不会同时携带 reasoning_effort + thinking_budget(tier 单独发送,每个被丢弃的字段都记录 warn 日志)。你的 diff 正是基于 #8488 之上并重写了它的测试——但 PR 正文仍在描述 #8488 之前的世界,且通篇未提及 #8488

剩下的部分实际上是提议翻转两个刚刚发布的刻意设计选择:

  • 跨层冲突——#8488 裁决 tier 优先(落选字段被丢弃并记录 warn)。本 PR 改为按配置层优先级裁决,且两条裁决路径都在 warn 逻辑运行之前删除落选字段——于是 #8488 会记录日志的冲突变成静默丢弃。特别地,配置了 extra_body.thinking_budget 的用户使用 /effort 会悄悄失效。
  • 同层显式成对配置——#8488 会将其解析为有效请求。本 PR 保留成对字段,使得今天能正常工作的配置开始收到 DashScope 400——出于原则。

两者都是可以成立的设计观点,但它们与数小时前刚经过完整审查周期并合并的决定相矛盾。翻转刚合并的设计,需要明确针对那个设计提出论证——给出一个被 #8488 行为实际损害的具体配置——而不是针对一个已不再被发送的冲突形态。

能否基于当前 main 重新陈述问题?具体说:一个 #8488 的 tier 优先 + warn 行为确实伤害用户的真实配置,以及为什么静默的 /effort 失效和同层成对配置刻意 400 是更好的结果。如果核心关切是优先级一致性,先开一个 issue 与发布 #8488 的 maintainer 讨论解析策略,再写更多代码。

Qwen Code · qwen3.8-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: @qwen-code /review --timeout=360. See workflow logs.

1 similar comment
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: @qwen-code /review --timeout=360. See workflow logs.

@wenshao

wenshao commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Review — fix(core): resolve Qwen 3.8 reasoning budget conflicts

Reviewed at 5386e37 (net diff vs merge-base 3ad4fbd). The precedence idea is sound, but three things in the current shape look wrong on the wire, and one premise is unverified.

1. tool_choice: 'required' guard goes blind when thinking_budget wins

pipeline.ts:978-993 strips tool_choice: 'required' only when it can see thinking is on:

thinkingMandatory ||
  (isQwenFamilyWireModel(model) &&
    (typed['enable_thinking'] === true ||
      (typeof reasoningEffort === 'string' && reasoningEffort !== 'none')))

After thinkingBudgetWins deletes reasoning_effort (dashscope.ts:369), the request still has thinking enabled — via thinking_budget — but none of those three signals is present. On a preset-configured qwen3.8-max-preview this is covered by thinkingMandatory: true (alibaba-token-plan.ts:39). On a custom provider config for the same family (no preset ⇒ thinkingMandatory false) with extra_body.thinking_budget set, every forced-tool side query (auto-mode permission classifier, structured side queries) now ships tool_choice: 'required' in thinking mode and takes the 400 "The tool_choice parameter does not support being set to required or object in thinking mode". On main the tier won, reasoning_effort shipped, and the guard fired.

If thinking_budget is going to be a first-class way to turn thinking on, the guard needs to recognise it too (numeric thinking_budget on a qwen wire model ⇒ thinking mode).

2. The new deletes bypass the 'none' disable sentinel

dropConflictingThinkingKnobs uses a deliberate value check — dashscope.ts:438-443, "'none' is an explicit disable that stays on the wire". The new delete merged['reasoning_effort'] at :369 runs before it and has no such guard.

extra_body: { thinking_budget: 4096 } + samplingParams: { reasoning_effort: 'none' } on qwen3.8-maxextraBodyHasBudget && !extraBodyHasEffort ⇒ the 'none' is deleted and the wire body is { thinking_budget: 4096 }: thinking is back on at 4096 tokens instead of disabled. It also disarms the required-thinking retry learning at pipeline.ts:1183, which keys off wireRequest['reasoning_effort'] === 'none'.

(The reasoning: false path is safe — it runs after buildRequest and re-injects 'none' while deleting thinking_budget. It's the explicit samplingParams/extra_body 'none' that gets eaten.)

Suggested fix: skip both new deletes when the surviving value is 'none', mirroring :442.

3. preserveThinkingBudget deliberately ships the pair DashScope rejects

hasExplicitSameLayerConflict ⇒ both fields survive the merge (:457). For extra_body: { reasoning_effort: 'high', thinking_budget: 1024 } — the exact case dashscope.test.ts:1026 now locks in — every request 400s, where on main the tier won and the request succeeded. The PR's stated goal is that "selecting an effort could turn a previously valid configuration into an invalid API request"; this branch makes that permanent for the same-layer shape.

Two inconsistencies inside that branch:

  • preserveThinkingBudget only guards thinking_budget. enable_thinking in the same extra_body is still dropped, so "left untouched" isn't what happens — and it burns the once-per-generator warn budget, so a later genuinely different conflict logs nothing.
  • If preserving an ambiguous pair is the intent, the user gets an opaque provider 400 with no local signal. A warn ("both knobs set in the same layer; DashScope will reject this") would at least name the cause.

4. Nothing is logged when a user-selected tier is discarded

/effort writes contentGeneratorConfig.reasoning, which is the lowest layer here. A thinking_budget sitting in settings.json therefore silently deletes the tier on every request: dropConflictingThinkingKnobs early-returns (effort now undefined), so its existing debugLogger.warn never fires, and dashscope.test.ts:1001 asserts expect(mockDebugLogger.warn).not.toHaveBeenCalled(). Meanwhile config.getReasoningEffort() still returns the tier, so /effort and the footer keep reporting a value that never reaches the wire.

Wire-parameter precedence (extra_body > samplingParams > reasoning) is the right rule for two static config layers, but reasoning is where a just-issued interactive command lands — losing it silently to months-old config is a surprising outcome. At minimum it should warn.

5. Unverified premise: does qwen3.8-max read thinking_budget at all?

The JSDoc this PR edits says the opposite of what the change assumes — "Only the qwen3.8-max family reads reasoning_effort itself" / "Older qwen hybrids read enable_thinking / thinking_budget, not reasoning_effort" (dashscope.ts:414-431). The PR body notes no live Qwen 3.8 Preview request was sent. If that family ignores thinking_budget, the user's /effort selection is thrown away for a field the model doesn't read; if it rejects the unknown field, every request 400s. No unit test can distinguish either outcome from the intended one — worth one real request before landing.

Smaller items

  • Lost coverage. Repurposing drops both conflicting knobs when a tier ships alongside enable_thinking and thinking_budget removed the only test of the multi-key delete loop (for (const key of dropped), :466). Trace the remaining paths: extra_body budget ⇒ effort deleted ⇒ early return; request budget + extra_body effort ⇒ budget already deleted; same-layer ⇒ preserveThinkingBudget. The only surviving two-element case is extra_body.enable_thinking === false plus a thinking_budget, and no test covers it — a mutation that clears only dropped[0] passes the suite.
  • Presence vs value checks disagree. The resolution uses !== undefined while dropConflictingThinkingKnobs uses in (and :245 uses in for the sibling copy). extra_body: { thinking_budget: undefined } ⇒ the resolution says "no conflict" and keeps reasoning_effort, but the spread puts the key in merged, so :457 fires and warns about a knob the resolution just declared absent.
  • Dead default. preserveThinkingBudget = false (:436) can never be taken — the sole call site (:374-378) always passes it.
  • Shape. The three 8-line booleans encode one mutually-exclusive 3-way decision with isTieredQwenModel && !extraBodyDisablesThinking and the !extraBodyHasBudget && !extraBodyHasEffort fallthrough copy-pasted into each; hasExplicitSameLayerConflict is fully derivable from the other two, so it's redundant state that can drift. Resolving the winning layer once ((extraBodyHasEffort || extraBodyHasBudget) ? extraBody : requestParams) and switching on which field it carries collapses all three.
  • Docs (model-providers.md:588). "Explicit same-layer pairs are left untouched and remain invalid" is inaccurate on both halves: enable_thinking is still dropped, and "remain invalid" understates a hard 400 — the same rejection the preceding sentence gives as the reason the resolution exists.

@DragonnZhang

Copy link
Copy Markdown
Collaborator Author

Addressed the latest review in acfebe2:

  • Numeric thinking_budget now counts as active thinking when guarding tool_choice=required.
  • DashScope precedence is resolved through one preferred knob; same-layer conflicts retain reasoning_effort, while lower-priority conflicting knobs are removed with a once-per-generator warning.
  • An explicit reasoning_effort=none sentinel is preserved when a higher-priority budget wins.
  • Added coverage for lower-layer drops, the none sentinel, multi-key deletion, same-layer conflicts, undefined budgets, and the required-tool guard; updated provider documentation.
  • Validation: 242 targeted tests passed, plus core typecheck, ESLint, and Prettier.

I could not verify live qwen3.8-max acceptance of thinking_budget because this environment has no DashScope API credentials. That external API validation remains explicitly unverified rather than inferred from the local adapter tests.

@wenshao

wenshao commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Review: fix(core): resolve Qwen 3.8 reasoning budget conflicts

Overview

Two changes:

  1. dashscope.ts — before merging, buildRequest now resolves which of reasoning_effort / thinking_budget "wins" by picking the highest-priority configuration layer (extra_body > request sampling params > reasoning) and deleting the loser in mergeExtraBodyAndResolveKnobs. Same-layer explicit pairs keep reasoning_effort. dropConflictingThinkingKnobs also switches from 'thinking_budget' in merged to merged['thinking_budget'] !== undefined, and the warn is factored into warnConflictingKnobDrop.
  2. pipeline.ts — a numeric thinking_budget now also counts as "thinking is on" for the DashScope tool_choice: 'required' drop.

The precedence model is coherent and the comments are unusually good. Below are the things I'd want resolved before merge.


1. /effort <tier> becomes a silent no-op when a thinking_budget is configured (behavior reversal)

/effort writes into ContentGeneratorConfig.reasoning.effort (config.tssetReasoningEffort), i.e. the lowest-priority layer in the new ordering. So for a user whose provider config carries extra_body: { thinking_budget: 1024 } (or samplingParams.thinking_budget), every /effort selection is now deleted on the wire.

This is a direct reversal of main, visible in the two rewritten tests:

-    it('drops both conflicting knobs when a tier ships alongside enable_thinking and thinking_budget', () => {
-      expect(result['reasoning_effort']).toBe('high');
+    it('keeps higher-priority extra_body thinking knobs over a configured tier', () => {
+      expect(result['reasoning_effort']).toBeUndefined();

The docs claim this is mitigated:

Dropped knobs are warn-logged once per generator so interactive /effort selections are not silently masked by older configuration.

That mitigation does not hold. createDebugLogger().warn (packages/core/src/utils/debugLogger.ts:222-226) is:

warn: (...args: unknown[]) => {
  const session = getActiveSession();
  if (!session) return;      // ← no debug session ⇒ no-op
  writeLog(session, 'WARN', tag, args);
},

With no active debug-log session — the normal case — nothing is emitted anywhere. The TUI acknowledges /effort max, /effort read-back still reports max, and the wire body carries neither.

The root issue is that the precedence model treats "a tier the user just selected interactively" and "a tier persisted in generationConfig.reasoning" as the same layer. They aren't. Options:

  • give an explicit runtime /effort selection its own precedence above a static thinking_budget; or
  • keep the ordering but make the conflict user-visible — e.g. have /effort check the resolved config for a conflicting thinking_budget and return a warning message, instead of relying on debugLogger.warn.

Either way the doc sentence about warn-logging should go, since it describes a signal users can't see.

2. reasoning_effort: 'none' + thinking_budget still ships the pair DashScope rejects

The cross-layer drop excludes 'none' (dashscope.ts:352-355) and dropConflictingThinkingKnobs early-returns on 'none' (:431), so this new test asserts both fields on the wire:

it('preserves a request-level none sentinel under a higher-priority budget', () => {
  // extra_body: { thinking_budget: 4096 }, request: reasoning_effort: 'none'
  expect(result['reasoning_effort']).toBe('none');
  expect(result['thinking_budget']).toBe(4096);
});

But pipeline.ts:887-894 asserts the opposite invariant and deletes the budget precisely when it emits the 'none' sentinel:

// The tier-native family reads reasoning_effort, not the boolean: emit the
// canonical disable in the knob it reads (the strip below preserves 'none').
// Drop a user-supplied thinking_budget too — DashScope rejects it alongside
// reasoning_effort.
delete typed['enable_thinking'];
delete typed['thinking_budget'];
typed['reasoning_effort'] = 'none';

So samplingParams: { reasoning_effort: 'none' } (or reasoning: { effort: 'none' }) plus an extra_body.thinking_budget on qwen3.8-max produces exactly the 400 this PR exists to prevent — reachable without going through the pipeline's disable path, since that path only fires on reasoning: false / thinkingConfig.includeThoughts: false.

This isn't a regression, but the PR now codifies it in a test. Either the pipeline comment is wrong (in which case relax it there and say so), or 'none' should be excluded only from being dropped, not from dropping the budget. Given the PR body says no live API request was made, it's worth confirming against DashScope which of the two beliefs is correct — they can't both be.

3. pipeline.ts: a thinking_budget now suppresses tool_choice: 'required' even when thinking is explicitly off

(typed['enable_thinking'] === true ||
  typeof thinkingBudget === 'number' ||          // ← new, ORed
  (typeof reasoningEffort === 'string' && reasoningEffort !== 'none'))

{ enable_thinking: false, thinking_budget: 4096 } satisfies the second clause, so tool_choice is dropped although thinking is disabled. The pipeline itself produces that shape: the reasoning-disabled branch for non-tiered qwen sets typed['enable_thinking'] = false (:896) and — unlike the tiered branch at :893 — does not delete thinking_budget.

Concrete failure: legacy qwen model (e.g. qwen3-max) + extra_body.thinking_budget: 4096 + a forced-tool side query with thinkingConfig.includeThoughts: false (the AUTO-mode permission classifier's respond_in_schema, fcMode === 'ANY') → tool_choice: 'required' is stripped, the classifier may free-text instead of calling the tool, and per the existing comment it "fails closed". That's the degradation the existing preserve required tool selection when reasoning effort is none case guards against, reintroduced via a different field.

Suggested guard:

(typeof thinkingBudget === 'number' && typed['enable_thinking'] !== false)

plus a pipeline table case for the enable_thinking: false + thinking_budget shape.

4. The warn latch now swallows the second drop set

warnConflictingKnobDrop is called twice per request (cross-layer pass, then dropConflictingThinkingKnobs) but latches conflictingKnobDropWarned on the first call. With extra_body: { reasoning_effort: 'high', enable_thinking: true, thinking_budget: 1024 } the log reports dropped: ['thinking_budget'] and never mentions the dropped enable_thinking — on main both arrived in one array.

Related: the warn reads merged['reasoning_effort'] after deletion, so when the effort is the dropped field the log shows reasoningEffort: undefined (asserted in the new tests) — you can't tell which tier was discarded, which is the one thing you'd want from that line.

Both go away if the two passes accumulate into a single dropped list and warn once, capturing the effort value before deletion.

5. Structure

  • The 26-line preferredThinkingKnob block (dashscope.ts:251-276) lives in buildRequest but is used only by mergeExtraBodyAndResolveKnobs. A private resolvePreferredThinkingKnob(model, extraBody, requestParams) would keep buildRequest readable, drop the param-threading through two call sites, and be directly unit-testable. The extraBodyHasEffort / requestHasBudget intermediates are single-use and inline fine.
  • Resolution for one wire field now lives in two places (cross-layer in mergeExtraBodyAndResolveKnobs, same-layer in dropConflictingThinkingKnobs), and correctness depends on their ordering plus the shared warn latch. A single pass that computes the winner and deletes the losers would be materially easier to reason about — and would fix Are you interested in AI Terminal? #4 for free.
  • merged['thinking_budget'] !== undefined is the right fix for the spurious warning, but the explicitly-undefined key now survives in the request object. Harmless on the wire (JSON.stringify drops it), just worth a one-line comment so the next reader doesn't "fix" it back to in.

6. Tests

  • The it.each table uses toBe(undefined), which passes for both "absent" and "present but undefined" — toBeUndefined() reads better and the distinction matters here given the in!== undefined change.
  • Cases 2 and 3 set reasoning: { effort: 'low' } on both the config and the request object. Only the config layer feeds buildQwenEffortConfig; the request-level copy obscures which layer is actually under test.
  • Missing coverage: cross-layer + same-layer drops in one request (Are you interested in AI Terminal? #4); enable_thinking: false + thinking_budget in the pipeline table (如何自定义密钥文件 .env可能与其他文件冲突 #3); the tiered path through the vision branch — preferredThinkingKnob is threaded into isVisionModel()'s branch but every vision test uses legacy qwen-vl-*, so that argument is never exercised there. If no tiered model is ever a vision model, a comment saying so is better than untested threading.

7. CI is not running

The ubuntu Test job fails at checkout, not on your code:

Can't find 'action.yml', 'action.yaml' or 'Dockerfile' under
'.../.github/actions/verify-checkout-head'

The branch is 39 commits behind main (merge base 3ad4fbdb7d, 2026-08-04) and predates that composite action, so all three Test legs skip/fail and there's no green signal for this change yet. Please rebase.


Summary

The core idea — resolve the conflict once, by configuration layer — is the right shape, and the in!== undefined fix plus the thinking_budget gap in the tool_choice guard are genuine improvements. Blocking items, in order: #3 (concrete forced-tool regression, one-line fix), #2 (the PR's own test asserts the rejected pair ships), and #1 (needs a product decision on whether an interactive /effort may be silently overridden — and the doc sentence corrected either way). #4#6 are cleanups. Rebase for CI.

@DragonnZhang

Copy link
Copy Markdown
Collaborator Author

Addressed the deterministic review findings in 858147c0dba2caf290354c1f128517ceac0edf5c:

Validation passed: both focused Core suites (255 tests), Core TypeScript typecheck, targeted ESLint, Prettier, and the repository pre-commit hook.

I intentionally did not guess on #1 because it changes product semantics. Which behavior should the interactive command have when static samplingParams or extra_body already supplies thinking_budget?

  1. runtime /effort overrides the static budget; or
  2. static precedence remains, but /effort returns a user-visible conflict warning.

Once that policy is confirmed I can implement the matching behavior.

Comment thread packages/core/src/core/openaiContentGenerator/provider/dashscope.ts Outdated
Comment thread packages/core/src/core/openaiContentGenerator/provider/dashscope.ts Outdated
Comment thread packages/core/src/core/openaiContentGenerator/provider/dashscope.ts Outdated
Comment thread packages/core/src/core/openaiContentGenerator/provider/dashscope.ts Outdated
@wenshao

wenshao commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Aug 7, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@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.

Reviewed. Suggestions are inline.

Not reviewed: reverse audit — stopped at the 5-round hard cap without two consecutive dry rounds; convergence not established (rounds 2-5 each reported findings — all verified, or excluded as re-reports of threads already on the PR).

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI; the touched integration file (cli/acp-integration.test.ts, formatting-only change) ran locally green (11/11), but the broader integration suite did not run locally.

Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — finished within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks completed within budget..

中文说明

已审查。 建议见行内评论。

未审查:reverse audit — stopped at the 5-round hard cap without two consecutive dry rounds; convergence not established (rounds 2-5 each reported findings — all verified, or excluded as re-reports of threads already on the PR)。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI; the touched integration file (cli/acp-integration.test.ts, formatting-only change) ran locally green (11/11), but the broader integration suite did not run locally。

未探索到全部深度(达到工具调用预算):You are review agent reverse-audit — Reverse audit agen...:none — finished within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all checks above completed within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all checks completed within budget.

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +380 to +383
// qwen3.8 rejects reasoning_effort with thinking_budget. Resolve the
// highest-priority layer once; when both fields are explicit in that
// layer, reasoning_effort keeps the pre-existing provider behavior.
const selectedThinkingKnob = isTieredQwenModel

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] R9-1: The ACP surface (session/set_config_optionreasoning_effort, packages/cli/src/acp-integration/acpAgent.ts:5451) was not updated for this PR's wire-precedence flip: it reports unconditional success as soon as the config accepts the tier, even when a higher-priority thinking_budget now shadows it on the wire — the exact misreport this PR fixed in the /effort command, the picker hook, and both SDKs via getReasoningEffortOverride. — Failure scenario: config with model: qwen3.8-max + samplingParams.thinking_budget: 2048 (or extra_body.thinking_budget); the user sets effort to xhigh from an ACP client → the handler returns success with currentValue === 'xhigh', but after this flip selectDashScopeThinkingKnob picks the budget and deletes reasoning_effort, so every request ships thinking_budget: 2048 and the tier never reaches the wire, while the client keeps showing xhigh as the active setting. Probe-verified at this head: success returned, getReasoningEffortOverride called 0 times.

Suggested fix (in acpAgent.ts, not anchored here):

// setConfigOption, reasoning_effort case, after applyReasoningEffort succeeds:
const override = session.getConfig().getReasoningEffortOverride?.();
// surface `override` in the response (warning note / config-option metadata)
// instead of silent success, mirroring formatEffortChangeMessage / handleSetEffort
中文说明

R9-1: ACP 入口(session/set_config_optionreasoning_effortpackages/cli/src/acp-integration/acpAgent.ts:5451)没有随本 PR 的 wire 优先级翻转更新:配置接受档位即无条件返回成功,即使更高优先级的 thinking_budget 现在会在 wire 上遮蔽该档位——这正是本 PR 在 /effort 命令、选择器 hook 和两个 SDK 中通过 getReasoningEffortOverride 修复的同类误报。失败场景:配置 model: qwen3.8-max + samplingParams.thinking_budget: 2048(或 extra_body.thinking_budget);用户通过 ACP 客户端设置 effort 为 xhigh → handler 返回成功且 currentValue === 'xhigh',但翻转后 selectDashScopeThinkingKnob 选中 budget 并删除 reasoning_effort,每个请求都发送 thinking_budget: 2048,档位永远到不了 wire,而客户端仍显示 xhigh 为生效设置。已在当前 head 探针验证:返回成功且 getReasoningEffortOverride 被调用 0 次。建议修复:在 applyReasoningEffort 成功后调用 getReasoningEffortOverride() 并在响应中呈现,对齐 formatEffortChangeMessage / handleSetEffort

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +17 to +19
return t(
'Reasoning effort set to {{tier}}, but thinking is currently disabled; after thinking is re-enabled, {{source}}.{{field}} will still have higher priority.',
{ tier, source: override.source, field: override.field },

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] R9-2: The disabled-branch override message assumes the override is a value knob (effort/budget), but getReasoningEffortOverride() also returns field: 'enable_thinking' for an enable_thinking: false off-switch, producing self-contradictory guidance. — Failure scenario: tiered model + reasoning: false + extra_body: { enable_thinking: false } (thinking hard-disabled in two places), user runs /effort high → the message says "after thinking is re-enabled, extra_body.enable_thinking will still have higher priority" — but that off-switch is the documented escape hatch translated to reasoning_effort: 'none' on the wire, so re-enabling reasoning can never re-enable thinking while that exact knob is present: the message names as the future competitor the very knob that makes its precondition unreachable. Probe rendered the message verbatim at this head; with a value knob (thinking_budget) the same sentence is coherent.

Suggested change
return t(
'Reasoning effort set to {{tier}}, but thinking is currently disabled; after thinking is re-enabled, {{source}}.{{field}} will still have higher priority.',
{ tier, source: override.source, field: override.field },
if (override.field === 'enable_thinking') {
return t(
'Reasoning effort set to {{tier}}, but thinking is currently disabled; {{source}}.{{field}} keeps thinking off and takes precedence over the tier — remove it to let the tier take effect.',
{ tier, source: override.source, field: override.field },
);
}
return t(
'Reasoning effort set to {{tier}}, but thinking is currently disabled; after thinking is re-enabled, {{source}}.{{field}} will still have higher priority.',
{ tier, source: override.source, field: override.field },
中文说明

R9-2: thinking 已禁用分支的覆盖提示假定覆盖项是值旋钮(effort/budget),但 getReasoningEffortOverride()enable_thinking: false 关闭开关也会返回 field: 'enable_thinking',产生自相矛盾的指引。失败场景:分档模型 + reasoning: false + extra_body: { enable_thinking: false }(两处硬禁用思考),用户执行 /effort high → 消息说"after thinking is re-enabled, extra_body.enable_thinking will still have higher priority"——但该关闭开关正是文档化的逃生通道,wire 上被转换为 reasoning_effort: 'none',只要它存在,重新开启 reasoning 永远无法恢复思考:消息把令其前提不可达的旋钮说成未来的竞争者。已在本 head 探针逐字复现该消息;值旋钮(thinking_budget)场景下同一句子是成立的。修复:在该分支特判 override.field === 'enable_thinking',改为提示该旋钮保持思考关闭且优先于档位(删除它档位才能生效)。

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +4302 to +4303
describe('reasoning effort override', () => {
it('reports a higher-priority DashScope knob that shadows reasoning effort', () => {

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] R9-3: Config.getReasoningEffortOverride() is never tested with currentEffort === undefined (reasoning: false or absent) — every case in this new describe block sets a tier, yet the no-tier state is exactly what the new "thinking disabled + static override" UI path reads. — Failure scenario: with qwen3.8-max, reasoning: false, and extra_body: { thinking_budget: 4096 }, /effort high calls the real getter with currentEffort === undefined; a plausible future simplification ("no active tier → no override", an early return when getReasoningEffort() is undefined) leaves every test in this block green — all have a tier set — and /effort silently regresses to the plain "it will take effect when thinking is re-enabled" message, re-introducing the misleading confirmation this PR exists to remove. Probe-verified: the suggested cases pass on this head; applying the feared early return fails only the new cases while all 14 pre-existing cases stay green.

Suggested fix:

it('reports a static override when thinking is disabled (no active tier)', () => {
  // config: reasoning: false, extra_body: { thinking_budget: 4096 }
  // expect getReasoningEffortOverride() →
  //   { source: 'extra_body', field: 'thinking_budget' }
});
// plus a variant with `reasoning` omitted entirely
中文说明

R9-3: Config.getReasoningEffortOverride() 从未在 currentEffort === undefinedreasoning: false 或缺省)下被测试——这个新 describe 块的所有用例都设置了档位,而无档位状态恰是新的"thinking 已禁用 + 静态覆盖"UI 路径所读取的。失败场景:qwen3.8-max + reasoning: false + extra_body: { thinking_budget: 4096 } 时,/effort high 会以 currentEffort === undefined 调用真实 getter;未来可能的简化("无激活档位 → 无覆盖",即 getReasoningEffort() 为 undefined 时提前返回)会让本块所有测试保持绿色——它们都设置了档位——/effort 将静默回退到普通的"it will take effect when thinking is re-enabled"消息,重新引入本 PR 要消除的误导性确认。已探针验证:建议的用例在当前 head 通过;施加担心的提前返回后仅新用例失败,14 个既有用例仍为绿。

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +1299 to +1301
expect(result['reasoning_effort']).toBeUndefined();
expect(result['enable_thinking']).toBeUndefined();
expect(result['thinking_budget']).toBe(4096);

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] R9-4: The new budget-selection drop branch in mergeExtraBodyAndResolveKnobs can drop two fields at once, but no test exercises both drops together — the two inner conditions are only ever tested in isolation. — Failure scenario: the branch is reachable with all three knobs present: config reasoning: { effort: 'high' } + extra_body: { thinking_budget: 4096 } (selection = budget) + request-level enable_thinking: false. Mutation-verified at this head: changing the two inner ifs to if/else if ships enable_thinking: false alongside the winning budget while the entire existing suite passes 132/132 — the suite provably cannot distinguish that regression from correct code.

Suggested fix:

it('drops both a configured tier and a request-level disable when a budget wins', () => {
  // config: { reasoning: { effort: 'high' }, extra_body: { thinking_budget: 4096 } }
  // request-level: enable_thinking: false
  // expect reasoning_effort undefined, enable_thinking undefined,
  // thinking_budget 4096, warn dropped: ['reasoning_effort', 'enable_thinking']
});
中文说明

R9-4: mergeExtraBodyAndResolveKnobs 中新增的 budget 获胜删除分支可以一次删除两个字段,但没有任何测试同时触发两个删除——两个内部条件从未被一起测试。失败场景:三个旋钮同时存在时该分支可达:配置 reasoning: { effort: 'high' } + extra_body: { thinking_budget: 4096 }(选中 budget)+ 请求层 enable_thinking: false。已在当前 head 做变异验证:把两个 if 改成 if/else if 后,enable_thinking: false 会与获胜 budget 一起发送,而现有套件 132/132 全部通过——该套件从证明上无法区分这一回归与正确代码。

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +1359 to +1361
it('keeps a legacy Qwen budget over an opaque none effort', () => {
const generator = new DashScopeOpenAICompatibleProvider(
{

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] R9-5: The legacy branch of dropConflictingThinkingKnobs changed in this diff from a presence check ('thinking_budget' in merged — verified at merge base 7425e42fc7) to a value check (merged['thinking_budget'] === undefined), flipping behavior for undefined-valued budget keys on legacy models — but no test exercises a legacy model with an undefined-valued thinking_budget key (legacy models skip withoutNullishThinkingKnobs). — Failure scenario: qwen3.7-max + extra_body: { thinking_budget: undefined, reasoning_effort: 'max' } — the exact shape the tiered undefined-budget test proves occurs in practice. Mutation-verified: restoring the pre-PR presence check passes the entire suite (132/132) while silently dropping the user's reasoning_effort: 'max' for a placeholder key that serializes to nothing — the wire loses the override and gains nothing.

Suggested fix:

it('keeps a legacy reasoning_effort over an undefined-valued budget key', () => {
  // model: 'qwen3.7-max'
  // extra_body: { thinking_budget: undefined, reasoning_effort: 'max' }
  // expect result['reasoning_effort'] === 'max', no warn fires
});
中文说明

R9-5: dropConflictingThinkingKnobs 的 legacy 分支在本 diff 中从存在性检查('thinking_budget' in merged——已在 merge base 7425e42fc7 核实)改为值检查(merged['thinking_budget'] === undefined),翻转了 legacy 模型上 undefined 值 budget 键的行为——但没有测试覆盖 legacy 模型 + undefined 值 thinking_budget 键(legacy 模型跳过 withoutNullishThinkingKnobs 净化)。失败场景:qwen3.7-max + extra_body: { thinking_budget: undefined, reasoning_effort: 'max' }——正是分档 undefined-budget 测试证明实际会出现的形态。变异验证:恢复 PR 前的存在性检查后整套测试(132/132)依然通过,却会为一个序列化后不存在的占位键静默丢弃用户的 reasoning_effort: 'max'——wire 失去了覆盖值而一无所获。

— qwen3.8-max via Qwen Code /review (v0.21.9)

{ effort },
);
return Boolean((response as Record<string, unknown> | null)?.applied);
return parseEffortStatus(response) ?? { applied: false, override: null };

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] R9-6: The new setEffortStatus fallback for a set_effort response lacking a boolean applied has no covering test in either SDK (Python mirror: packages/sdk-python/src/qwen_code_sdk/query.py:515). — Failure scenario: paired with a CLI whose set_effort success response omits applied (a pre-PR CLI or future protocol drift), parseEffortStatus returns undefined and this ?? fallback is the only thing keeping setEffort from throwing a TypeError on .applied of undefined instead of returning the documented graceful false (the pre-diff code defended against exactly this with Boolean(response?.applied) / response.get("applied", False)). Probe-verified in both SDKs at this head: the new tests pass on PR code; dropping the fallback fails the probe in TS (expected undefined to deeply equal {applied: false, override: null}) and Python (assert None == {'applied': False, 'override': None}) while all existing effort tests stay green.

Suggested fix:

// Query.test.ts: answer the set_effort control request with {} (no `applied`);
// assert setEffortStatus resolves to { applied: false, override: null }.
// Parity test in test_query_core.py for set_effort_status.
中文说明

R9-6: 新的 setEffortStatus 对缺少布尔 appliedset_effort 响应的兜底在两个 SDK 中都没有覆盖测试(Python 对应:packages/sdk-python/src/qwen_code_sdk/query.py:515)。失败场景:与 set_effort 成功响应缺少 applied 的 CLI(PR 前的 CLI 或未来协议漂移)配对时,parseEffortStatus 返回 undefined,这个 ?? 兜底是避免 setEffortundefined.appliedTypeError(而非返回文档承诺的优雅 false)的唯一防线(diff 前代码正是用 Boolean(response?.applied) / response.get("applied", False) 防御这一形态)。已在当前 head 对两个 SDK 探针验证:新测试在 PR 代码上通过;删除兜底后 TS 探针失败(expected undefined to deeply equal {applied: false, override: null})、Python 探针失败(assert None == {'applied': False, 'override': None}),而现有 effort 测试全部保持绿色。

— qwen3.8-max via Qwen Code /review (v0.21.9)

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round: no action required (PR #8525)

This round found nothing actionable:

  • Reviews: none newer than the last evaluation.
  • Inline comments: none newer than the last evaluation.
  • Issue-level comments: none newer than the last evaluation.
  • Failed checks: none — all CI checks are SUCCESS or intentionally SKIPPED for this PR/event.
  • Base conflicts: none reported (--conflict false), and no merge of origin/main was performed.

The PR is in Critical-only mode after five change-producing rounds. The deferred non-Critical items listed by the workflow (one review plus six inline findings from the automated reviewer) were excluded from this round's actionable scope by the deterministic filter; per policy they remain open for human follow-up and were intentionally left untouched — no code changes, no thread resolutions, and no comment replies for them.

No code changes were made and no commit was created this round. The branch head remains ea91a351a9.

中文说明

Autofix 评审轮次:无需处理(PR #8525

本轮没有发现任何可处理的内容:

  • 评审(Reviews): 自上次评估以来没有新的评审。
  • 行内评论(Inline comments): 自上次评估以来没有新的行内评论。
  • Issue 级评论: 自上次评估以来没有新的 Issue 级评论。
  • 失败的检查: 无 —— 所有 CI 检查均为 SUCCESS,或因本 PR/事件而按预期 SKIPPED
  • 与基础分支的冲突: 未报告(--conflict false),本轮也没有执行 origin/main 的合并。

该 PR 在经历五个产生改动的轮次后已进入仅处理 Critical 的模式。工作流列出的被延后的非 Critical 条目(一条评审,以及来自自动评审者的六条行内发现)已被确定性过滤器排除在本轮可处理范围之外;按策略它们保持开放、留待人工跟进,本轮有意不做任何处理 —— 不修改代码、不解决任何讨论串、也不对这些条目撰写评论回复。

本轮没有修改任何代码,也没有创建任何提交。分支头部仍为 ea91a351a9

Deferred non-Critical feedback

Critical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (@qwen-code /retry starts a fresh counting window.)

中文说明

完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 @qwen-code /retry 可开启新的计数窗口。)

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@wenshao

wenshao commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Maintainer verification — real local stack, BEFORE/AFTER A/B

I built both sides from source and exercised them against a real DashScope-compatible endpoint that enforces the documented rule this PR exists for (reasoning_effort + thinking_budget together → HTTP 400 InvalidParameter). Every result below is a recorded wire body or a real TUI/SDK run, not a unit-test assertion.

BuildsBEFORE = merge-base main @ 7425e42, AFTER = PR head ea91a35. Both npm install && npm run build && npm run bundle, driven as dist/cli.js. Isolated QWEN_HOME per scenario; provider selected via DASHSCOPE_PROXY_BASE_URL so the DashScope provider path is genuinely active.

1. Wire bodies — 17 configurations, same settings, two builds

wire A/B

Four configurations shipped an illegal field pair before this PR and now do not (ex=1 is the CLI exiting non-zero on the endpoint's 400):

# config BEFORE AFTER
A7 samplingParams: { reasoning_effort: 'none', thinking_budget: 2048 } effort="none" budget=2048400 effort="none" → 200
A8 extra_body.reasoning_effort='none' + samplingParams.thinking_budget effort="none" budget=2048400 effort="none" → 200
A9 samplingParams: { reasoning_effort: 4, thinking_budget: 2048 } effort=4 budget=2048400 effort=4 → 200
A12 legacy qwen3-max, reasoning_effort='none' + budget effort="none" budget=2048400 budget=2048 → 200

These are exactly the holes in the old guard: it returned early on effort === 'none' and on any non-string effort, so both fields survived. That is the concrete pre-existing bug, and it is fixed.

Precedence now follows the documented layer order — A1 (/effort high + samplingParams.thinking_budget) and A2 (/effort high + extra_body.thinking_budget) previously shipped reasoning_effort and silently discarded the user's explicit budget; they now ship the budget. A3/A4 (effort wins, same-layer pair) are unchanged.

thinkingMandatory — A11 (qwen3.8-max-preview shape: thinkingMandatory: true + extra_body.enable_thinking: false) shipped reasoning_effort: 'none' before, which that family rejects; it now ships no thinking knob at all. The new pipeline strip is load-bearing.

No regression on the common paths — all five effort tiers with no conflicting knob (B1–B5) are byte-identical between builds, as are the legacy qwen3-max on-switch mapping (A6) and the preset-injected extra_body.enable_thinking: true + tier case (verified separately: both builds ship reasoning_effort alone).

The once-per-generator warning fires correctly — one [WARN] DashScope: dropped conflicting thinking knobs { model: 'qwen3.8-max', reasoningEffort: 'high', dropped: [ 'reasoning_effort' ] } across a two-request session.

2. /effort in the real TUI

/effort A/B

Real tmux TUI, extra_body.enable_thinking: false configured, same keystrokes. BEFORE reported Reasoning effort: high (requested; …) while the wire actually shipped reasoning_effort: 'none' — a pre-existing misreport, now corrected to name the winning field. With no conflicting knob configured, the message is unchanged.

3. SDK effort status

SDK A/B

TypeScript SDK driving the real CLI with samplingParams.thinking_budget=2048 and query({ effort: 'high' }). BEFORE: setEffort() returned true while the tier never reached the wire, and no status surface existed. AFTER: getInitialEffortStatus() / setEffortStatus() return applied: false with override: { source: 'samplingParams', field: 'thinking_budget' } and a human-readable reason.

4. Suites and checks (PR head)

Check Result
packages/core — dashscope + pipeline + config 799 passed
packages/cli — effort-command, use-effort-command, systemController 37 passed, 2 failed
packages/sdk-typescriptQuery.test.ts 66 passed
packages/sdk-python — full unit suite (py3.12 container) 148 passed
npm run typecheck pass
npm run lint pass
prettier --check on changed files pass

The 2 CLI failures are get_usage_info › returns dashboard … timeouts; they fail identically on the merge-base, so they are pre-existing and unrelated to this PR.

Notes for the merge decision

Nothing here blocks the merge. Three things worth a second look:

  1. Behaviour change beyond "avoid an invalid request". For a valid pre-existing config like samplingParams.thinking_budget + /effort high, main sent the tier; this PR sends the budget instead. That is the correct precedence, but it means /effort becomes a no-op on the wire for those users. The new /effort message and SDK status are what make that discoverable — good — but it is a semantic change, not purely a rejection fix, and the PR description currently frames it only as the latter.
  2. PR description does not cover the whole diff. The body describes only the DashScope request-construction change, while the diff also adds public SDK surface (EffortStatus / EffortOverride / setEffortStatus / getInitialEffortStatus in TS, EffortStatus / set_effort_status / initial_effort_status in Python), the effort_status field on the control-protocol initialize response, and the /effort message rework. New public API deserves to be in the description and release notes.
  3. Minor SDK asymmetry. The CLI emits reason on effort_status, and the TS EffortStatus carries it, but the Python EffortStatus TypedDict and _parse_effort_status drop it — Python callers can only reconstruct a reason from override. Also integration-tests/cli/acp-integration.test.ts contains a formatting-only reflow unrelated to this change.

Verdict: verified. The illegal field pair is eliminated in every configuration I could construct that produced it, precedence matches the documented order, all five effort tiers and the legacy Qwen paths are unchanged, and the reporting surfaces now tell the truth.

中文版本

维护者验证 —— 本地真实环境,BEFORE/AFTER 双臂对比

我从源码分别构建了两侧,并针对一个真实的 DashScope 兼容端点进行验证,该端点强制执行本 PR 所针对的既有规则(reasoning_effortthinking_budget 同时出现 → HTTP 400 InvalidParameter)。下面每条结论都来自记录到的真实请求体或真实的 TUI / SDK 运行,而非单测断言。

构建BEFORE = merge-base main @ 7425e42AFTER = PR head ea91a35。两侧均执行 npm install && npm run build && npm run bundle,以 dist/cli.js 驱动。每个场景使用隔离的 QWEN_HOME;通过 DASHSCOPE_PROXY_BASE_URL 选中 provider,确保真正走 DashScope 代码路径。

1. 线上请求体 —— 17 组配置,相同设置,两个构建

修复前有四组配置会发出非法字段对,现在不会了ex=1 表示 CLI 因端点返回 400 而非零退出):

# 配置 BEFORE AFTER
A7 samplingParams: { reasoning_effort: 'none', thinking_budget: 2048 } effort="none" budget=2048400 effort="none" → 200
A8 extra_body.reasoning_effort='none' + samplingParams.thinking_budget effort="none" budget=2048400 effort="none" → 200
A9 samplingParams: { reasoning_effort: 4, thinking_budget: 2048 } effort=4 budget=2048400 effort=4 → 200
A12 旧模型 qwen3-maxreasoning_effort='none' + budget effort="none" budget=2048400 budget=2048 → 200

这正是旧守卫的漏洞所在:它在 effort === 'none' 以及任何非字符串 effort 时提前返回,导致两个字段一起发出。这是真实存在的既有缺陷,已被修复。

优先级现在符合文档中的层级顺序:A1(/effort high + samplingParams.thinking_budget)与 A2(/effort high + extra_body.thinking_budget)此前发出 reasoning_effort 并静默丢弃用户显式配置的 budget,现在改为发出 budget。A3/A4(effort 胜出、同层显式配对)行为不变。

thinkingMandatory:A11(qwen3.8-max-preview 形态:thinkingMandatory: true + extra_body.enable_thinking: false)此前会发出该系列会拒绝的 reasoning_effort: 'none',现在完全不发送任何 thinking 字段。pipeline 中新增的剥离逻辑是必要的。

常见路径无回归:五个 effort 档位在无冲突字段时(B1–B5)两侧完全一致;旧版 qwen3-max 的 on-switch 映射(A6)以及 preset 注入 extra_body.enable_thinking: true + 档位的场景(单独验证)也都一致,两侧均只发 reasoning_effort

一次性告警行为正确:两次请求的会话中只出现一条 [WARN] DashScope: dropped conflicting thinking knobs { model: 'qwen3.8-max', reasoningEffort: 'high', dropped: [ 'reasoning_effort' ] }

2. 真实 TUI 中的 /effort

真实 tmux TUI,配置 extra_body.enable_thinking: false,相同按键。BEFORE 显示 Reasoning effort: high (requested; …),但实际线上发出的是 reasoning_effort: 'none' —— 这是既有的误报,现在会正确指出真正生效的字段。在没有冲突配置时,提示文案保持不变。

3. SDK effort 状态

TypeScript SDK 驱动真实 CLI,配置 samplingParams.thinking_budget=2048,调用 query({ effort: 'high' })。BEFORE:setEffort() 返回 true,但该档位从未到达线上,且没有任何状态查询接口。AFTER:getInitialEffortStatus() / setEffortStatus() 返回 applied: false,并带有 override: { source: 'samplingParams', field: 'thinking_budget' } 及可读的 reason

4. 测试与静态检查(PR head)

检查项 结果
packages/core —— dashscope + pipeline + config 799 通过
packages/cli —— effort-command、use-effort-command、systemController 37 通过,2 失败
packages/sdk-typescript —— Query.test.ts 66 通过
packages/sdk-python —— 完整单测(py3.12 容器) 148 通过
npm run typecheck 通过
npm run lint 通过
改动文件 prettier --check 通过

2 个 CLI 失败是 get_usage_info › returns dashboard … 超时,在 merge-base 上同样失败,属于既有问题,与本 PR 无关。

合并决策参考

没有阻塞项。三点值得再看一眼:

  1. 行为变更超出了「避免非法请求」的范围。 对于 samplingParams.thinking_budget + /effort high 这类原本合法的配置,main 发送的是档位,本 PR 改为发送 budget。这个优先级是对的,但意味着这些用户的 /effort 在线上变成了空操作。新的 /effort 提示和 SDK 状态让这一点可被发现(很好),但这是语义变更,而不仅仅是修复拒绝问题,而当前 PR 描述只按后者来陈述。
  2. PR 描述未覆盖完整改动。 正文只描述了 DashScope 请求构建的改动,而 diff 还新增了公开 SDK 接口(TS 侧 EffortStatus / EffortOverride / setEffortStatus / getInitialEffortStatus,Python 侧 EffortStatus / set_effort_status / initial_effort_status)、控制协议 initialize 响应上的 effort_status 字段,以及 /effort 文案改造。新增公开 API 应写进描述和 release notes。
  3. SDK 存在小的不对称。 CLI 在 effort_status 中输出 reason,TS 的 EffortStatus 保留了该字段,但 Python 的 EffortStatus TypedDict 与 _parse_effort_status 丢弃了它 —— Python 调用方只能从 override 反推原因。另外 integration-tests/cli/acp-integration.test.ts 中包含一处与本次改动无关的纯格式重排。

结论:验证通过。 我能构造出的所有会产生非法字段对的配置都已消除,优先级符合文档顺序,五个 effort 档位与旧版 Qwen 路径保持不变,提示与状态接口现在如实反映实际生效的配置。

The CLI emits a human-readable reason on effort_status and the
TypeScript SDK surfaces it, but the Python EffortStatus TypedDict and
_parse_effort_status dropped it, leaving Python callers to reconstruct
the reason from override. Add reason as an optional field and pass it
through, mirroring the TypeScript parser.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 6/100 轮)。改动内容与我反驳保留之处如下:

Address-review round summary — PR #8525

Round input: one issue-level maintainer verification comment (@wenshao, verdict "verified — nothing blocks the merge") carrying three follow-up points. No inline comments, no reviews, no failed checks were actionable this round; Critical-only mode is active. Dispositions below.

Feedback points and decisions

1. Minor SDK asymmetry — Python drops the effort-status reasonFixed

The CLI emits a human-readable reason on effort_status (initialize response) and the TypeScript SDK's EffortStatus carries it, but the Python EffortStatus TypedDict and _parse_effort_status dropped it, so Python callers could only reconstruct a reason from override. Fixed by mirroring the TypeScript parser exactly:

  • packages/sdk-python/src/qwen_code_sdk/types.py — added reason: NotRequired[str] to EffortStatus (mirrors TS reason?: string).
  • packages/sdk-python/src/qwen_code_sdk/query.py_parse_effort_status now keeps reason when it is a string and omits the key otherwise (mirrors the TS typeof record.reason === 'string' guard).
  • packages/sdk-python/tests/unit/test_query_core.pytest_initialize_sends_effort fixture/assertion now carry the CLI's actual reason format (samplingParams.enable_thinking takes precedence, matching systemController's ${source}.${field} takes precedence). The existing test_set_effort_status_returns_override exact-equality assertion already pins the "no reason key when the CLI sends none" case.

Note: the CLI only attaches reason to the initialize-response effort_status; the set_effort response carries no reason today, identically for both SDKs — so parity is complete with this change and no CLI-side change was needed.

2. Formatting-only reflow in integration-tests/cli/acp-integration.test.tsDeclined with evidence (revert attempted, mechanically impossible through the normal path)

The observation is correct that the reflow is unrelated to this PR's purpose, and I attempted to revert the file to origin/main's content. Evidence gathered while doing so:

  • origin/main's version of the file fails prettier --check under the repo's own .prettierrc.json (an 88-char destructuring line exceeds printWidth: 80); the PR's reflowed version is what passes. The pre-existing unformatted state lives on main (main's CI stays green only because CI's prettier step runs prettier --write . with no dirty-tree check afterwards).
  • I staged the revert and committed; the repo's pre-commit hook (lint-staged → prettier --write on staged .ts files) re-wrapped the line, the revert became an empty diff, and it was silently dropped from the commit. Bypassing the hook (--no-verify) is not permitted.

So the reflow is prettier-mandated formatting that the repo's own commit pipeline re-creates on any commit touching this file; keeping it is the only tooling-compatible state. The root cause (the file landed on main prettier-dirty) can only be cleaned by the very same reflow, so there is nothing to remove from this PR. Recording this here so the point is answered, not dropped.

3. PR description frames the change only as a rejection fix / does not cover the new public API surface → Escalated to maintainer (not a code change)

Updating the PR description is a GitHub write, which this flow cannot perform (no credentials in the agent; the address-review path consumes no PR-body file). Release notes need no repo change either: CHANGELOG.md is generated from GitHub Releases ("Do not edit it by hand — run npm run changelog to regenerate"). Decision needed: how to extend the PR body before merge. Options: (a) a maintainer edits the PR body directly — suggested text below; (b) leave the description as is and rely on the commit history. Recommendation: (a).

Suggested additions to the PR description:

  • Semantic change, not only a rejection fix. For valid pre-existing configs where an explicit higher-priority thinking knob coexists with /effort (e.g. samplingParams.thinking_budget + /effort high), the explicit knob now wins on the wire per the documented layer order — main sent the tier and silently discarded the knob. /effort becomes a wire no-op in that situation; the reworked /effort message and the SDK effort-status surfaces are what make that discoverable.
  • New public API surface. TypeScript SDK: EffortStatus, EffortOverride, EffortTier, Query.setEffortStatus(), Query.getInitialEffortStatus(). Python SDK: EffortStatus, EffortOverride, Query.set_effort_status(), Query.initial_effort_status. Control protocol: new effort_status field on the initialize response. CLI: /effort message rework naming the winning field.

Changes

One commit: 868b25c32dfix(sdk-python): expose effort status reason from CLI (#8525) (3 files: types.py, query.py, test_query_core.py; 8 insertions, 1 deletion). No conflict: --conflict false, no merge performed.

Verification

Commands actually run and their results:

  • npm run build — passed (exit 0)
  • npm run typecheck — passed (exit 0)
  • npm run lint (eslint . --ext .ts,.tsx && eslint integration-tests) — passed (exit 0)
  • python3 -m py_compile on all three changed Python files — passed
  • Logic surrogate: AST-extracted the exact shipped _parse_effort_status and executed 6 cases (reason kept; reason absent → key absent; applied/no-override; non-string reason dropped; non-dict → None; non-bool applied → None) — all passed
  • Line-length audit of changed Python files against ruff's 88-char limit — no violations
  • Unavailable on this runner: the packages/sdk-python CI suite (ruff check, ruff format --check, mypy strict, pytest on py3.10/3.11/3.12 per .github/workflows/sdk-python.yml) — no pip/Docker here and networked package commands are forbidden for this flow. That workflow triggers on packages/sdk-python/** changes when this push lands and remains the final gate for the Python side.
  • No focused Vitest run was needed: no TypeScript source changed this round; the integration-test file revert was dropped by the pre-commit hook, leaving the file byte-identical to the pre-round HEAD.
中文说明

评审反馈处理轮次总结 — PR #8525

本轮输入:一条 issue 级维护者验证评论(@wenshao,结论为“验证通过——无合并阻塞项”),其中带三个后续关注点。本轮没有可处理的行内评论、评审意见或失败检查;当前处于仅处理 Critical 的模式。各项处置如下。

反馈点与处置

1. SDK 小的不对称 —— Python 丢弃 effort-status 的 reason已修复

CLI 会在 effort_status(initialize 响应)中输出人类可读的 reason,TypeScript SDK 的 EffortStatus 保留了该字段,但 Python 的 EffortStatus TypedDict 与 _parse_effort_status 将其丢弃,导致 Python 调用方只能从 override 反推原因。已按与 TypeScript 解析器完全一致的方式修复:

  • packages/sdk-python/src/qwen_code_sdk/types.py —— 为 EffortStatus 增加 reason: NotRequired[str](对应 TS 的 reason?: string)。
  • packages/sdk-python/src/qwen_code_sdk/query.py —— _parse_effort_status 现在在 reason 为字符串时保留该字段,否则不含该键(对应 TS 的 typeof record.reason === 'string' 判断)。
  • packages/sdk-python/tests/unit/test_query_core.py —— test_initialize_sends_effort 的 fixture 与断言现在携带 CLI 真实的 reason 格式(samplingParams.enable_thinking takes precedence,与 systemController${source}.${field} takes precedence 一致)。既有的 test_set_effort_status_returns_override 精确相等断言已经固化了“CLI 未发送 reason 时结果中不含 reason 键”这一情形。

说明:CLI 目前只在 initialize 响应的 effort_status 中附带 reasonset_effort 响应本身不携带 reason,两个 SDK 面对的行为完全一致 —— 因此本改动已实现完整对齐,无需 CLI 侧改动。

2. integration-tests/cli/acp-integration.test.ts 中的纯格式重排 → 有证据地拒绝(已尝试还原,但在正常路径下机制上不可行)

“该重排与本 PR 目的无关”这一观察是正确的,我也尝试将文件还原为 origin/main 的内容。过程中收集到的证据:

  • origin/main 版本的该文件在仓库自身的 .prettierrc.json无法通过 prettier --check(一行 88 字符的解构赋值超过 printWidth: 80);本 PR 重排后的版本才是符合格式的状态。未格式化的既有状态存在于 main 上(main 的 CI 之所以是绿的,仅因为 CI 的 prettier 步骤执行的是 prettier --write .,之后没有检查改动)。
  • 我将还原改动加入暂存并提交;仓库的 pre-commit 钩子(lint-staged → 对暂存的 .ts 文件执行 prettier --write)把该行重新折行,还原变成空 diff,被静默移出提交。绕过钩子(--no-verify)不被允许。

因此该重排是 prettier 强制的格式,仓库自身的提交流水线在任何触及该文件的提交中都会重新生成它;保留它是唯一与工具链兼容的状态。根因(该文件以 prettier 不合规的状态进入 main)只能通过同样的重排来清理,所以本 PR 中没有可移除的内容。在此记录,确保该反馈点得到回应而非被遗漏。

3. PR 描述仅将改动表述为避免非法请求 / 未覆盖新增公开 API → 升级给维护者(非代码改动)

更新 PR 描述属于 GitHub 写操作,本流程无法执行(agent 无凭据;address-review 路径不消费 PR 正文文件)。Release notes 也无需改动仓库文件:CHANGELOG.md 由 GitHub Releases 自动生成(“请勿手动编辑 —— 运行 npm run changelog 重新生成”)。需要决策:合并前如何扩充 PR 正文。选项:(a) 由维护者直接编辑 PR 正文 —— 建议文案见下;(b) 保持现有描述,依赖提交历史。建议:(a)。

建议补充进 PR 描述的内容:

  • 这是语义变更,而不仅是避免非法请求的修复。 对于显式高优先级 thinking 字段与 /effort 并存的既有合法配置(例如 samplingParams.thinking_budget + /effort high),现在按文档中的层级顺序由显式字段在线上生效 —— main 此前发送的是档位并静默丢弃显式字段。此时 /effort 在线上成为空操作;改造后的 /effort 提示与 SDK effort 状态接口正是为了让这一点可被发现。
  • 新增公开 API。 TypeScript SDK:EffortStatusEffortOverrideEffortTierQuery.setEffortStatus()Query.getInitialEffortStatus()。Python SDK:EffortStatusEffortOverrideQuery.set_effort_status()Query.initial_effort_status。控制协议:initialize 响应新增 effort_status 字段。CLI:/effort 文案改造,指出实际生效的字段。

改动

单个提交:868b25c32d —— fix(sdk-python): expose effort status reason from CLI (#8525)(3 个文件:types.pyquery.pytest_query_core.py;8 行新增,1 行删除)。无冲突:--conflict false,未执行合并。

验证

实际执行的命令及结果:

  • npm run build —— 通过(exit 0)
  • npm run typecheck —— 通过(exit 0)
  • npm run linteslint . --ext .ts,.tsx && eslint integration-tests)—— 通过(exit 0)
  • 对三个改动的 Python 文件执行 python3 -m py_compile —— 通过
  • 逻辑替身验证:用 AST 提取实际提交的 _parse_effort_status 并执行 6 组用例(reason 保留;reason 缺失 → 无该键;applied 且无 override;非字符串 reason 被丢弃;非 dict → None;applied 非布尔 → None)—— 全部通过
  • 按 ruff 的 88 字符限制审查改动的 Python 文件行宽 —— 无超限
  • 本 runner 不可用:packages/sdk-python 的 CI 套件(ruff check、ruff format --check、mypy strict、py3.10/3.11/3.12 上的 pytest,见 .github/workflows/sdk-python.yml)—— 本机无 pip/Docker,且本流程禁止联网包命令。该 workflow 由本次推送中的 packages/sdk-python/** 改动触发,仍是 Python 侧的最终验证门。
  • 未运行定向 Vitest:本轮没有改动任何 TypeScript 源码;集成测试文件的还原被 pre-commit 钩子移除,该文件与本轮开始前的 HEAD 逐字节一致。

Deferred non-Critical feedback

Critical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (@qwen-code /retry starts a fresh counting window.)

中文说明

完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 @qwen-code /retry 可开启新的计数窗口。)

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@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 issues found. LGTM! ✅

Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — I did not run the sdk-typescript vitest suite, but I filed no finding that depends on it; my conclusion rests on static tracing of the diff against its p…; PR #8525 resolves DashScope Qwen 3.8 conflicts between r...: none — all planned checks completed within budget.`.

中文说明

未发现问题。LGTM!✅

未探索到全部深度(达到工具调用预算):You are review agent reverse-audit — Reverse audit agen...:none — I did not run the sdk-typescript vitest suite, but I filed no finding that depends on it; my conclusion rests on static tracing of the diff against its p…;PR #8525 resolves DashScope Qwen 3.8 conflicts between r...:none — all planned checks completed within budget.`。

— qwen3.8-max via Qwen Code /review (v0.21.10)

@doudouOUC doudouOUC 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.

Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.

中文说明

已审查——无阻断问题。 建议见行内评论。 1 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。

— deepseek-v4-flash via Qwen Code /review (v0.21.8)

* same-layer precedence as the request builder. Keeping this decision shared
* lets UI reporters describe the value that will actually reach the wire.
*/
export function selectDashScopeThinkingKnob(

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] Add direct unit tests for selectDashScopeThinkingKnob

selectDashScopeThinkingKnob is exported and contains complex multi-layer resolution logic (4 nested helpers, 3-layer precedence with on-switch special case) but has no direct unit test. It is exercised indirectly through buildRequest() tests and getReasoningEffortOverride() tests, which are comprehensive, but a direct test suite would be more robust against future refactors.

Suggested change
export function selectDashScopeThinkingKnob(
Add a `describe('selectDashScopeThinkingKnob', ...)` block with direct test cases for each major resolution path: extra_body disable, extra_body budget, extra_body on-switch, samplingParams disable, samplingParams budget, same-layer effort/budget conflict, and all three on-switch sub-paths.
中文说明

建议:selectDashScopeThinkingKnob 添加直接单元测试。

selectDashScopeThinkingKnob 是导出的函数,包含复杂多层解析逻辑(4 个嵌套辅助函数,3 层优先级含 on-switch 特例),但没有直接单元测试。它通过 buildRequest()getReasoningEffortOverride() 的测试间接覆盖,覆盖全面,但直接测试套件在后续重构时会更健壮。

— deepseek-v4-flash via Qwen Code /review (v0.21.8)

…enLM#8525)

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 7/100 轮)。改动内容与我反驳保留之处如下:

Autofix review-response summary — PR #8525

Feedback points

[Suggestion] Add direct unit tests for selectDashScopeThinkingKnob (rc:3762292432) — Implemented.

The function is exported and has two independent consumers: the DashScope
provider's request builder and Config.getReasoningEffortOverride() (the
interactive /effort reporter). It encodes this PR's core precedence
contract, and its return shape (source/field/value) is observable only
through a direct call — the existing buildRequest() tests can assert the
final wire payload but not which layer/field won. Direct tests pin the
resolution logic against future refactors at both call sites.

No other actionable feedback this round (no failed checks; the deferred
non-Critical section listed no items).

Changes

One commit, test-only — no production code touched:

  • Added a top-level describe('selectDashScopeThinkingKnob', ...) to
    packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts
    (16 tests) covering: the tiered-model gate (non-tiered and missing model),
    case-insensitive model matching, no-knob fallback, reasoning-tier fallback,
    extra_body disable winning over same-layer values and lower layers,
    extra_body budget winning over lower layers, same-layer effort-over-budget
    conflict in both layers, nullish extra_body knobs being ignored (the
    config reporter passes extra_body through unsanitized), all three
    extra_body on-switch sub-paths (samplingParams value decides / reasoning
    tier decides / on-switch itself) plus its blocking of a lower-priority
    samplingParams disable, and the samplingParams disable, budget, on-switch
    (with and without a reasoning tier) paths.

Conflict notes

None (--conflict false; no merge performed).

Verification

Commands actually run, in order:

  • vitest run src/core/openaiContentGenerator/provider/dashscope.test.ts (in packages/core) — passed, 148/148 tests (132 pre-existing + 16 new)
  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint (ESLint) — passed
  • npx prettier --write on the touched test file — reported "unchanged" (already conforming)

Integration tests were not run: the change adds unit tests only and exercises
no behavior that exists solely in the bundled CLI. No settings source
changed, so no settings-schema regeneration was needed.

中文说明

Autofix 审查响应总结 — PR #8525

反馈条目

[建议] 为 selectDashScopeThinkingKnob 添加直接单元测试(rc:3762292432) — 已实现。

该函数是导出的,且有两个独立的使用方:DashScope provider 的请求构建器和
Config.getReasoningEffortOverride()(交互式 /effort 报告器)。它承载了本 PR
的核心优先级契约,且其返回结构(source/field/value)只能通过直接调用观察到——
现有的 buildRequest() 测试只能断言最终的 wire 载荷,无法断言是哪一层/哪个字段胜出。
直接测试能在两个调用点上为未来重构固定住解析逻辑。

本轮无其他可执行反馈(无失败检查;延后的非 Critical 区域未列出任何条目)。

改动

单个提交,纯测试——未触碰任何生产代码:

  • packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts
    中新增顶层 describe('selectDashScopeThinkingKnob', ...)(16 个测试),覆盖:
    分层模型门槛(非分层模型与缺失模型)、模型名大小写不敏感匹配、无任何旋钮时的回退、
    reasoning 档位回退、extra_body 关闭开关胜过同层取值与更低优先级层、extra_body budget
    胜过更低优先级层、两层中同层 effort 优先于 budget 的冲突、nullish extra_body 旋钮被忽略
    (config 报告器直接透传未经清理的 extra_body)、extra_body on-switch 的全部三个子路径
    (samplingParams 取值决定 / reasoning 档位决定 / on-switch 自身生效)及其对更低优先级
    samplingParams 关闭开关的阻断,以及 samplingParams 关闭开关、budget、on-switch
    (有/无 reasoning 档位)路径。

冲突说明

无(--conflict false;未执行合并)。

验证

实际执行的命令(按顺序):

  • vitest run src/core/openaiContentGenerator/provider/dashscope.test.ts(在 packages/core 目录下)— 通过,148/148 个测试(132 个原有 + 16 个新增)
  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint(ESLint)— 通过
  • 对改动的测试文件执行 npx prettier --write — 报告 "unchanged"(本就符合格式)

未运行集成测试:本次改动仅新增单元测试,不涉及仅存在于打包后 CLI 中的行为。
未改动任何 settings 源,因此无需重新生成 settings schema。

Deferred non-Critical feedback

Critical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (@qwen-code /retry starts a fresh counting window.)

中文说明

完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 @qwen-code /retry 可开启新的计数窗口。)

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@wenshao

wenshao commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 281 passed · 0 failed · 281 total

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:281 通过 · 0 失败 · 281 总计

Verification report

PR 8525 — fix(core): resolve Qwen 3.8 reasoning budget conflicts (round 5)

Verdict: merge-ready — 281/281 scripted assertions passed, 0 unexpected failures. Verified head: 3c745444681ab5f5530a7fcc38151ff96bc72458 (merge-ref base a64d1291d2f6298f67763d0953b1653cf7b34060).

Follow-up round at a moved head AND moved base again. Round 4 verified head 282079c0 over base 95e17691a9; this round's parents are both different (delta since round 4: a2703ebd merge of upstream/main with conflict resolutions in the four CLI effort files, 868b25c3 sdk-python effort_status.reason passthrough, 3c745444 direct unit tests for selectDashScopeThinkingKnob, plus bot main-merges that moved the base to the Agent-Plugins-v1 tip). No measurement was carried forward: the base worktree was rebuilt from the new HEAD^1, every harness re-driven, every suite re-run, the MC2 census re-executed on a rebuilt dist, and the live premise re-sent. Per the snapshot's own commits array this is a 26-commit PR; the depth-2 graft makes only the aggregate diff exercisable (scripted check in the gates summary; see Not covered).

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与新 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

  • 结论:merge-ready。脚本化断言 281 通过 · 0 失败(A/B 矩阵 179、线路 oracle 32、CLI 全管线 E2E 18、真实 API 前提 8、变异矩阵判定 10、门禁+闭包 23、Python 探针 11)。
  • A/B 结论:35 个配置组合在 PR 构建与新 base(a64d1291d2)构建上驱动真实编译产物 buildRequest()。HEAD 全部按 extra_body > samplingParams > reasoning 优先级只保留高优先级字段,任何组合都不上线路被 API 拒绝的 pair;base 在 14 个组合上输出错误形状,其余 21 个均等组合不受影响。环回 HTTP 抓包(真实 OpenAI SDK 序列化)、全管线 CLI E2E(head bundle,用户可见 settings 形状)、真实 qwen3.8-max API 实测(pair → 400「cannot be set simultaneously」,HEAD 各形状含 delta 形状 → 200)闭环。
  • 本轮增量验证3c745444 的 16 个新 selector 直测非空跑——M5 变异(去掉 on-switch 注册)的 5 个红测试中有 3 个是这些新测试;dashscope 套件 132→148。868b25c3 的 Python reason 透传经 AST 编译出的真实函数体 8 用例功能验证(容器无 pip/typing_extensions,pytest 本体仍不可跑,与历轮 A/A)。CLI effort 四文件的主线合并冲突解决经 39/39 套件与 E2E 复核。
  • 上轮发现复核(全部重新测量):规范化守卫 value === false 依旧活着且被钉住(M6p 恰好 1 红,同一测试);on-switch except-path 依旧成立——在 384 州普查中占 4 个状态(全部 currentEffort === undefined,CLI 不可达),删除后 returnedDefined 334→338(恰 +4)、514 套件仍绿、报告器在两侧构建上都无一句谎言(dishonest=0);boolean/0 等垃圾 thinking knob 直通依旧成立且被真实 API 证实(boolean budget → 400「must be a positive integer」);null-budget 冗余守卫依旧成立。无新增阻塞项。
  • 未覆盖:Python SDK pytest 本体(无 pip,A/A);CLI E2E 的 base 臂;逐 commit 归因(depth-2,1/26 可达);TUI 渲染新 override 消息;D14 number effort 与 CLI 路径的垃圾 knob 活测。

Previous-finding status (round 4 → round 5)

# Round-4 finding/observation Severity Status at 3c745444 (re-measured this round)
1a Canonicalize guard value === false (replacement of the round-3 dead guard) live and pinned informational Stands. M6p (widen to value !== undefined) re-run: killed by exactly one test, the same one — "keeps a higher-priority extra_body enable_thinking over a samplingParams disable without a tier" (expected undefined to be true). See 04-mutation-matrix.png.
1b getReasoningEffortOverride on-switch except-path: live but CLI-unreachable, unpinned informational Stands, re-measured at the new head. Census over an explicit 8×8×6 = 384-state space (layer shapes × currentEffort incl. undefined): 4 states have an on-switch selection, all with currentEffort === undefined (no CLI call site produces that — the reporter runs right after setReasoningEffort). MC2 rebuilt dist: returnedDefined 334 → 338 (exactly +4, the on-switch states; 59 → 63 overrides at undefined effort) and the reporter never lies on either build (dishonest = 0/384 both arms: no state reports an override while the requested tier ships through unchanged). 514-test config suite green with the branch deleted. Round 4 quoted 313→319 (+6) on a differently-shaped layer space; same conclusion on this round's explicit space. Classification unchanged: live defense for CLI-unreachable states, unpinned by tests — informational, not a merge condition.
2 Boolean thinking_budget: false passes through (D12) informational (nit) Stands, live-proven again. D12 re-measured in the matrix (head ships {"thinking_budget":false}, base shipped the accepted tier); live probe L8: the real API rejects a boolean budget with HTTP 400 "thinking_budget parameter must be a positive integer and not greater than 262144". Same class: the falsy 0 budget (cell C12) also ships verbatim on head (thinking_budget: 0) and hits the same validation. Severity stays nit — the input is invalid configuration either way; a typeof value === 'number' || typeof value === 'string' filter in the thinking_budget arm of selectValueFromLayer would close both.
3 Null-budget direction redundantly guarded informational Stands. C5b/C6 parity cells re-measured on both arms; M3 (sanitization disabled) is killed by exactly the 4 null-reasoning_effort/null-enable_thinking cells, while the null-budget cells stay green because the selector's != null and the merge tail still drop them. Coverage redundancy, not a gap.
4 Correction to the 9f27e190 commit message ("value-aware drop branch" attribution) correction Stands as history. The underlying property — "an on-switch never rewrites the tier" — re-measured at this head: 0/384 census states co-exist an on-switch selection with a shipping effort tier. Still a description correction, not a code-change request.
Round-4 verdict merge-ready at 282079c0 Superseded by this round: head moved (3 substantive commits), base moved; everything re-verified at the new pair.

Central claim and A/B proof

Central claim: for tiered qwen3.8-max* DashScope models, when reasoning_effort / thinking_budget / enable_thinking come from different configuration layers, the outgoing request carries only the documented-precedence outcome (extra_body > samplingParams > reasoning) — never the API-rejected pair — and a budget beneath an enable_thinking: true on-switch survives with the on-switch beside it.

Harness harness/ab-knob-matrix.mjs drives the real compiled provider (head packages/core/dist vs a base worktree rebuilt from the new HEAD^1) over 35 cells × 2 arms, asserting the exact knob projection per arm (base expectations encode the predicted pre-PR shape as control cells), plus a head-side invariant that no tiered cell ships the rejected pair. Witness: 01-ab-matrix-head-vs-base.png. Re-ran green after the MC2 restore (coherence).

Cells Shape HEAD wire BASE wire Flip?
C1 / C2 / C12 budget in extra_body / request / falsy 0 + config tier budget only effort tier (drops user budget) yes
C4 extra_body on-switch+budget + tier max budget + enable_thinking: true effort max (drops both user knobs) yes
C5 extra_body reasoning_effort: null + tier low tier low reasoning_effort: null on wire yes
C7 request 'none' + extra_body budget budget only the rejected pair ('none'+4096) yes
C8 / C9 same-layer extra_body / request pair 'none'+budget effort 'none' kept, budget dropped the rejected pair on wire yes
C11 extra_body off-switch, no tier canonical reasoning_effort: 'none' enable_thinking: false verbatim yes (intended canonicalization)
D2 extra_body on-switch + request budget + lower request off-switch + tier budget + enable_thinking: true effort tier (drops both) yes
D3 extra_body on-switch + request budget + tier budget + enable_thinking: true effort tier yes
D8 extra_body on-switch+budget + lower off-switch + tier budget + enable_thinking: true effort tier yes
D12 boolean budget false (type boundary) thinking_budget: false passes through effort tier yes — see Findings
D13 string budget '4096' budget verbatim effort tier yes
C3 / C10 same-layer explicit effort+budget / off-switch escape hatch + tier effort kept / canonical 'none' same parity
C5b / C6 / D14 null request budget / null extra_body budget / number effort unchanged behavior same parity
D4–D7, D9, D10 on-switch and off-switch precedence siblings documented outcome same wire parity
P1–P5 tiers low…max, no conflict tier verbatim same parity
P6 no knobs at all neither knob same parity
P7 / P8 legacy qwen hybrid (budget+injected on-switch / opaque effort+budget) unchanged behavior same parity
P9 non-qwen model on the endpoint opaque knob pair kept (by design) same parity
P10 nested reasoning object strip tier only, reasoning dropped same parity

Result: 179/179 arm assertions. Head flips exactly the 14 cells where base ships a wrong shape and perturbs none of the 21 parity cells; 32 tiered no-pair invariants, 35 preserve_thinking markers, 2 P10 strip checks, and 4 closure controls all green. Closure controls: base dist lacks selectDashScopeThinkingKnob; zero @qwen-code/* imports in the base provider dist (the workspace-symlink trap, named and defused — node_modules/@qwen-code/qwen-code-core realpath points into the head tree, so the harness imports base dist by path, and packages/core has no workspace deps); third-party openai realpath identical across arms; lockfile untouched.

Wire oracle (harness/wire-capture.mjs, real OpenAI SDK client serializing buildRequest() output over a loopback socket, 4 cells × 2 arms × 4 asserts = 32/32): head bodies {thinking_budget:4096} / {budget, enable_thinking:true} / {budget} under a request 'none'; base bodies {reasoning_effort:'high'} and the rejected pair {reasoning_effort:'none', thinking_budget:4096}; preserve_thinking:true on every body proves the DashScope provider path; peer-side and caller-side asserted on all 8 calls. Witness: 02-wire-capture-base-vs-head.png.

Full-pipeline CLI E2E (harness/e2e-cli.mjs, head bundle dist/cli.js, user-facing settings shape modelProviders.openai[].generationConfig.{extra_body,samplingParams} + model.reasoningEffort, fresh QWEN_HOME, loopback SSE capture server matched via DASHSCOPE_PROXY_BASE_URL, --auth-type openai): E1 ships {thinking_budget:4096} with no effort field; E2 ships {reasoning_effort:'high'}; E3 (delta shape) — extra_body on-switch + samplingParams budget + tier — ships {thinking_budget:2048, enable_thinking:true} with no effort. No rejected pair on ANY captured request (each scenario produced 2 requests — initial turn plus one follow-up; knobs and the no-pair invariant asserted on all). 18/18. Witness: 03-cli-e2e-full-pipeline.png.

Live premise (harness/live-premise.mjs, real qwen3.8-max via the lane proxy, max_tokens=8): pair high+budget → 400 'reasoning_effort' and 'thinking_budget' cannot be set simultaneously; pair 'none'+budget → same 400; budget alone, effort alone, 'none' alone, string budget, and the delta shape budget+enable_thinking:true all → 200; boolean budget → 400 "must be a positive integer". 8/8. Witness: 07-live-api-premise.png. Chain closed: base ships shapes the real API rejects; every head shape — including the delta shape — is accepted.

Reviewer Test Plan walk-through

Plan step Result
Nested/config effort + higher-priority budget in samplingParams or extra_body → request retains budget, omits flat and nested effort, preserves enable_thinking when configured PASS — C1, C2, C4, D2, D3, D8 (provider); W1/W2 (wire); E1/E3 (full pipeline)
reasoning_effort in extra_body + lower-priority request budget → request retains only effort PASS — C3, C8 (both arms); C7 (higher-priority budget wins over request 'none')
Existing effort-only requests pass all five tiers unchanged PASS — P1–P5 (both arms) + E2 (full pipeline)

Delta verification (commits since round 4)

  • a2703ebd merge-conflict resolutions (systemController.ts, effort-command.ts, use-effort-command.{ts,test.ts}) — the effective diff at this head was reviewed file by file: the CLI surface matches the round-4-verified shape (effort_status with applied/override/reason on initialize, override on set_effort, shared formatEffortChangeMessage in the new effort-utils.ts), with no base-side logic dropped. Behavior re-proven by the 39-test CLI gate and the MS1 mutant (1 red, "returns the higher-priority wire override").
  • 868b25c3 sdk-python effort_status.reason — the container cannot import the module (typing_extensions missing; no pip — A/A with rounds 2–4), so _parse_effort_status was compiled verbatim from the real source via ast and driven through 8 scripted cases: reason passthrough, reason omitted (NotRequired honored), non-string reason dropped, malformed payloads (applied non-bool / missing / non-dict) → None. 11/11 incl. AST parses of all three changed files. The TS mirror (parseEffortStatus) is pinned by the 66-test sdk-typescript suite.
  • 3c745444 direct unit tests for selectDashScopeThinkingKnob (dashscope suite 132 → 148) — proven non-vacuous by mutation: the M5 kill set (on-switch registration removed) contains 3 of the new tests ("is itself the selection when nothing below carries a value", "blocks a lower-priority samplingParams disable", "keeps a lone samplingParams on-switch as the selection"). The unmutated control is green, so the kills are meaningful.

Findings

No blocking findings.

Observations (informational, not blocking)

  1. getReasoningEffortOverride on-switch except-path: live but CLI-unreachable, unpinned (carried from round 3, re-measured — status table row 1b). Deleting the branch changes reporter output in exactly 4/384 census states (334 → 338 returnedDefined), all with currentEffort === undefined; the reporter is honest on both builds (0/384 states claim an override while the requested tier ships unchanged). A reviewer may keep it as defense-in-depth or delete it; neither changes CLI-reachable behavior.
  2. Garbage thinking-knob values pass through selectValueFromLayer (carried, sharpened — status table row 2). Boolean budget false (D12) and falsy 0 (C12) ship verbatim on head; the live API rejects a boolean budget with 400 "must be a positive integer", and 0 hits the same validation. Base accepted the tier for the same garbage config, so the PR converts previously-accepted invalid configs into rejected requests — still a nit (invalid configuration either way). Candidate fix (not applied this round): admit only typeof value === 'number' \|\| typeof value === 'string' in the thinking_budget arm; note the live API accepted the string '4096' (L7), so a string filter stays compatible.
  3. Null-budget direction redundantly guarded (carried — status table row 3).

Mutation matrix (vacuity of the PR's own tests)

Witness: 04-mutation-matrix.png. All 9 mutants behaved as predicted (dashscope-suite rows assert the round-4 kill counts as lower bounds — the 16 new selector tests can only add kills); every red is a behavioral expected-vs-actual AssertionError; git status --porcelain clean after each restore (scripted in the gates summary).

Mutant Guard Suite result Verdict
control unmutated dashscope suite green, exit 0 baseline
M0 positive control: legacy-qwen effort-vs-budget drop 4 red KILLED
M3 nullish knob sanitization 4 red — the null-reasoning_effort/enable_thinking cells KILLED
M5 on-switch registration in selectFromLayer 5 red (round 4: 2) — incl. 3 of the new direct selector tests KILLED
M6p canonicalize guard widened (value === falsevalue !== undefined) 1 red — exactly the no-tier on-switch test KILLED
M7 enable_thinking:true under winning budget dropped 3 red — the budget-beneath-on-switch tests KILLED
M4b canonical 'none' rewrite removed 6 red — the escape-hatch tests KILLED
MC2 on-switch except-path removed from reporter (dist rebuilt) config suite 514/514 green SURVIVOR — adjudicated: suppresses exactly 4 phantom overrides in CLI-unreachable states (row 1b, 05-override-census-head-mc2-restored.png)
MP2 budget clause of the tool_choice drop (pipeline.ts) 2 red KILLED-AS-PREDICTED
MP3 thinking-mandatory 'none' strip removed (pipeline.ts) 2 red incl. end-to-end KILLED-AS-PREDICTED
MS1 override === null clause dropped from systemController set_effort applied 1 red — "returns the higher-priority wire override" KILLED-AS-PREDICTED

Targeted gates

Witness: 06-gates-and-closure.png (fresh re-runs in harness/gates-summary.mjs).

  • packages/core: dashscope.test.ts 148/148 (round 4: 132; +16 new selector unit tests), pipeline.test.ts 153/153, config.test.ts 514/514 — 815 total.
  • packages/cli: effort-command + use-effort-command + systemController → 39/39.
  • packages/sdk-typescript: Query.test.ts → 66/66.
  • npm run typecheck: all workspaces, exit 0. Liveness proven: planted private verifyLiveProbe: number = 'not-a-number' in the PR-touched config.ts → exit 2 with src/config/config.ts(4237,11): error TS2322 at the planted line; restored → clean (scripted checks over the saved logs).
  • Input-closure identity: HEAD^2 == snapshot headRefOid == 3c745444…; HEAD^1 == snapshot baseRefOid == a64d1291d2; lockfile untouched (shared node_modules is a clean control); working tree clean after every mutation/restore cycle (scripted).

Not covered

  • packages/sdk-python pytest suite — container has Python 3.11.2 but no pip (re-verified: python3 -m pip --version → "No module named pip") and no typing_extensions, so the module cannot even be imported; agent is non-root. Environmental, A/A with rounds 2–4. Mitigation: the changed function was driven functionally via its real source compiled through ast (above).
  • Base arm of the CLI E2E — base behavior at the full-pipeline level is covered by the provider-level matrix and wire capture (both arms); the base bundle was not built.
  • Per-commit attribution — depth-2 graft: git rev-list --count HEAD^1..HEAD^2 yields 1 while the snapshot lists 26 commits; git rev-parse --is-shallow-repository = true (scripted gate row). The aggregate HEAD^1..HEAD diff was verified; the delta commits' behavior is isolated by the E3/D-cells, the M5 new-test kills, MS1, and the Python probe.
  • Interactive TUI rendering of the new override message — unit tests only (the 39-test CLI gate covers formatEffortChangeMessage through effort-command/use-effort-command).
  • Live matrix for remaining garbage shapes — boolean budget was sent live (400 quoted); number effort (D14, parity on both arms) and 0/boolean budgets through the CLI path were not sent live (they reach the same API validation).
  • The E2E follow-up turn — every scenario emitted a second chat-completion request; knob projections and the no-pair invariant were asserted on ALL captured requests and the shape was identical across scenarios; its trigger was not identified (same observation as round 4, environmental, outside this PR's surface).
  • Census space shape differs from round 4's — this round's space is explicitly enumerated (8 layer shapes × 8 × 6 efforts incl. undefined); absolute counts (334/338, 4 states) therefore differ from round 4's (313/319, 6 states) while the conclusion is identical. Not a carry-forward of round 4's numbers.
  • Live probes consumed ~64 tokens of the lane proxy budget (max_tokens=8 × 8 requests).

Methodology

Environment: CI merge-ref checkout (HEAD = merge of 3c745444 into a64d1291d2, depth 2); head build pre-built by the workflow (dist/cli.js bundle + per-package dist/, selector presence asserted). Base control: scratch worktree at the new HEAD^1, packages/core compiled there with tsc --build; the first base build failed because the worktree lacked the nested per-package node_modules (root-hoisted mime is v2.6.0 while the workspace pins 4.0.7 nested under packages/core/node_modules) — symlinked the nested dirs plus root node_modules into the worktree, rebuild green; realpath purity quoted in the gates summary (openai realpath shared, @qwen-code/qwen-code-core would resolve into the head tree — avoided by path-importing base dist, and packages/core has zero workspace deps). Worktree removed with git worktree remove --force after the A/B cells were captured. The MC2 measurement rebuilt the head dist with the branch removed, re-ran the census, then restored the source and rebuilt — the restored dist/src/config/config.js is sha256-identical to the pre-mutation one, and coherence re-runs of the matrix (179) and wire oracle (32) on the restored dist were green. Harnesses (lib.mjs, ab-knob-matrix.mjs, wire-capture.mjs, e2e-cli.mjs, mutation-matrix.mjs, override-census.mjs, live-premise.mjs, gates-summary.mjs, python-probe.py) live in harness/ of this artifact dir; raw logs in logs/ (including override-census-{head,mc2,restored}.log, mutation-matrix.log, typecheck{,-planted}.log); captures in evidence/ via scripts/verify-capture.mjs (01/02/03 live re-runs; 04/05/06/07 faithful teed logs). E2E ran the bundle in a clean env (fresh HOME/QWEN_HOME, no inherited OPENAI_* vars, --auth-type openai — one bring-up fix: headless mode refuses to start without an explicit auth type; the cited run is the fixed one) against an in-process loopback SSE capture server. Assertion totals in assertions.json: A/B matrix 179, wire oracle 32, CLI E2E 18, live premise 8, mutation-matrix verdict rows 9 + unmutated control 1, gates + closure summary 23, Python probe 11 (= 281); each number maps to exactly one scripted check in the named harness.

Evidence images

01-ab-matrix-head-vs-base

02-wire-capture-base-vs-head

03-cli-e2e-full-pipeline

04-mutation-matrix

05-override-census-head-mc2-restored

06-gates-and-closure

07-live-api-premise

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@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.

Reviewed. Suggestions are inline.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the touched integration file (cli/acp-integration.test.ts, formatting-only change) was manually run green (11/11), but the broader integration suite went unexercised.

Not reviewed: reverse audit — stopped at the 5-round hard cap without two consecutive dry rounds; convergence not established (rounds 3 and 5 fully dry; round 4's two findings resolved to one probe-rejected and one low-confidence terminal-only).

Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all planned checks completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks I started were completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks I started were completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all planned checks completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all planned checks completed within budget., and 2 more.

中文说明

已审查。 建议见行内评论。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the touched integration file (cli/acp-integration.test.ts, formatting-only change) was manually run green (11/11), but the broader integration suite went unexercised。

未审查:reverse audit — stopped at the 5-round hard cap without two consecutive dry rounds; convergence not established (rounds 3 and 5 fully dry; round 4's two findings resolved to one probe-rejected and one low-confidence terminal-only)。

未探索到全部深度(达到工具调用预算):You are review agent reverse-audit — Reverse audit agen...:none — all planned checks completed within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all checks I started were completed within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all checks I started were completed within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all planned checks completed within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all planned checks completed within budget.,另有 2 条。

— qwen3.8-max via Qwen Code /review (v0.21.10)

Comment on lines +1052 to +1053
/** Return the server-reported status for the initial effort request. */
getInitialEffortStatus(): EffortStatus | undefined {

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] R11-1: getInitialEffortStatus() is a synchronous getter populated only when the async initialize() completes, but neither its JSDoc nor the package README states that callers must await query.initialized first — and undefined already means two other things ("no effort requested", "legacy CLI"), so the unready state is silently ambiguous. The Python twin initial_effort_status (packages/sdk-python/src/qwen_code_sdk/query.py:525-526) has the same undocumented precondition. — Failure scenario: a consumer following the README's construct-then-iterate pattern checks q.getInitialEffortStatus()?.applied === false right after construction to surface a fallback for a shadowed effort tier; initialize() yields at its first await before the control round-trip starts, so the getter returns undefined — indistinguishable from "no override" — and the shadowed-effort branch never fires. Probe-verified: undefined synchronously after construction and while initialize is in flight; populated only after await query.initialized.

Suggested change
/** Return the server-reported status for the initial effort request. */
getInitialEffortStatus(): EffortStatus | undefined {
/** Return the server-reported status for the initial effort request.
* Only meaningful after `initialized` resolves; returns `undefined` until
* then, and also when no effort was requested or the CLI predates the field. */
getInitialEffortStatus(): EffortStatus | undefined {
中文说明

R11-1: getInitialEffortStatus() 是一个同步 getter,其值只有在异步 initialize() 完成后才会填充,但它的 JSDoc 和包 README 都没有说明调用方必须先 await query.initialized——而 undefined 已经表示另外两种含义("未请求 effort"、"旧版 CLI"),因此"尚未就绪"状态被静默地混入其中。Python 对应物 initial_effort_statuspackages/sdk-python/src/qwen_code_sdk/query.py:525-526)存在同样的未文档化前置条件。失败场景:使用者按 README 的"构造后直接迭代"模式,在构造后立即检查 q.getInitialEffortStatus()?.applied === false,以决定是否提示被遮蔽的 effort 档位;initialize() 在控制协议往返开始前的第一个 await 处就让出,getter 此时返回 undefined——与"无覆盖"无法区分——遮蔽提示分支永远不会触发。已用探针验证:构造后同步及 initialize 进行中均为 undefined,仅在 await query.initialized 后才有值。

— qwen3.8-max via Qwen Code /review (v0.21.10)

Comment on lines +794 to +801
const {
sendRequest,
cleanup,
stderr,
sessionUpdates,
permissionRequests,
agent,
} = setupAcpTest(rig, {

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] R11-2: This PR edits an integration test that no PR-gating CI job ever collects — the required PR gate test:integration:no-ak:sandbox:none runs a fixed 12-file list that excludes this file; ci.yml's integration_cli job is merge_group-only (it never triggers on pull_request while the merge queue is disabled); e2e.yml collects it but runs only post-merge / nightly / on workflow_dispatch. — Failure scenario: a regression in ACP set_config_option reasoning-effort handling — the surface this PR's behavior change rides on — or a broken edit to this test merges with zero PR-time signal and surfaces only at the post-merge e2e push run or the nightly regression. Nothing is currently broken: this file ran green in this review (11/11, bundled CLI + fake OpenAI server) — this is a gate-coverage gap, not a defect.

Suggested fix: add ./cli/acp-integration.test.ts to the test:integration:no-ak:sandbox:none file list (it uses only the fake OpenAI server, so it fits the no-AK gate), or explicitly accept post-merge-only coverage for it.

中文说明

R11-2: 本 PR 修改了一个不被任何 PR 门禁 CI 任务收集的集成测试——必需的 PR 门禁 test:integration:no-ak:sandbox:none 运行一个固定的 12 文件列表,其中不包含本文件;ci.ymlintegration_cli 任务仅属于 merge_group(在 merge queue 未启用时不会在 pull_request 上触发);e2e.yml 会收集它,但只在合并后 push / 每日定时 / workflow_dispatch 时运行。失败场景:ACP set_config_option reasoning-effort 处理(本 PR 行为变更所依赖的表面)出现回归,或对本测试的破坏性修改,将在 PR 阶段毫无信号地合并,直到合并后的 e2e push 运行或每日回归才暴露。当前没有破坏:本审查中该文件运行全绿(11/11,打包 CLI + fake OpenAI server)——这是门禁覆盖缺口,不是缺陷。

建议修复:将 ./cli/acp-integration.test.ts 加入 test:integration:no-ak:sandbox:none 的文件列表(它只使用 fake OpenAI server,符合 no-AK 门禁),或明确接受对它仅做合并后覆盖。

— qwen3.8-max via Qwen Code /review (v0.21.10)

@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. ✅

@DragonnZhang
DragonnZhang added this pull request to the merge queue Aug 12, 2026
Merged via the queue into QwenLM:main with commit ac78acd Aug 12, 2026
89 of 90 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants