feat(sdk): add reasoning effort option to CLI and both SDKs - #6464
Conversation
Add support for controlling the model's reasoning effort level (low/medium/high/xhigh/max) via: - CLI: handle `set_effort` control request, accept `effort` in initialize payload, report `can_set_effort` capability - Python SDK: `effort` option in QueryOptions, `set_effort()` method - TypeScript SDK: `effort` option in QueryOptions/TransportOptions, `setEffort()` method, `SET_EFFORT` control request type
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Hey @juhuan — thanks for the PR! The feature itself (reasoning effort control via SDK) is a reasonable addition and the direction aligns with existing set_model / setModel patterns. But the PR body doesn't follow the required template.
Template: the body uses ## Summary and ## Test plan, but the PR template requires specific sections: ## What this PR does, ## Why it's needed, ## Reviewer Test Plan (with How to verify, Evidence (Before & After), Tested on matrix), ## Risk & Scope, ## Linked Issues, and a Chinese <details> translation block. Please restructure the body to match.
A few things I noticed while skimming the diff — worth addressing alongside the template:
- Protocol type weakness:
effortis typed asstringinCLIControlSetEffortRequest,CLIControlInitializeRequest, and the Pythonprotocol.pyTypedDict. The core already hasReasoningEffort = 'low' | 'medium' | 'high' | 'xhigh' | 'max'— the protocol types should use the constrained type (or aLiteralin Python) rather than a barestring, so invalid values fail at the type boundary, not at runtime. - Silent failure in initialize: when
setReasoningEffortthrows duringhandleInitialize, the error is caught and logged but the initialize response still succeeds. This means a bad effort value at startup is silently ignored. The runtimehandleSetEffortcorrectly throws. Consider whether the initialize path should also propagate the error (or at minimum validate and reject before callingsetReasoningEffort). - No tests in the diff: the PR body claims 58 Python tests passed and 1163 TypeScript tests passed, but no test files appear in the diff. Were new tests added for
set_effort/setEffort, or were these just existing suites confirming no regressions? If no new tests, consider adding at least a validation test and a control-request round-trip test.
Please update the body to follow the template and address the type/validation notes. Happy to re-run the review after that.
中文说明
@juhuan 你好，感谢提交 PR！功能本身（通过 SDK 控制 reasoning effort）是合理的扩展，方向也与现有的 set_model / setModel 模式一致。但 PR 正文未遵循模板要求。
模板：正文使用了 ## Summary 和 ## Test plan，但 PR 模板 要求特定的章节：## What this PR does、## Why it's needed、## Reviewer Test Plan（含 How to verify、Evidence (Before & After)、Tested on 矩阵）、## Risk & Scope、## Linked Issues，以及中文 <details> 翻译块。请按模板重构正文。
浏览 diff 时发现几个值得一起修改的问题：
- 协议类型偏弱：
CLIControlSetEffortRequest、CLIControlInitializeRequest和 Pythonprotocol.pyTypedDict 中effort的类型为string。核心代码已有ReasoningEffort = 'low' | 'medium' | 'high' | 'xhigh' | 'max'——协议类型应使用约束类型（Python 中用Literal），而非裸string，这样无效值在类型层面就会报错，而非等到运行时。 - 初始化时静默失败：
handleInitialize中setReasoningEffort抛异常时，错误被捕获并记录，但 initialize 响应仍然成功。这意味着启动时的无效 effort 值被静默忽略。运行时handleSetEffort正确地抛出异常。请考虑 initialize 路径是否也应传播错误（或至少在调用前做校验并拒绝）。 - diff 中没有测试文件：PR 正文声称 58 个 Python 测试通过、1163 个 TypeScript 测试通过，但 diff 中未出现测试文件。是否有为
set_effort/setEffort新增测试，还是仅跑了已有测试确认无回归？如果没有新测试，建议至少添加一个验证测试和一个 control-request 往返测试。
请按模板更新正文并处理上述类型/验证问题。更新后可以重新触发审查。
— Qwen Code · qwen3.7-max
|
Hey @juhuan, thanks for working on SDK completeness — these are all useful additions. I noticed you've opened 5 separate PRs today (#6458, #6460, #6461, #6463, #6464), each exposing a single CLI feature to the SDKs. Looking at the diffs, the total changes across all 5 PRs are around 400 lines, and most of them touch overlapping file sets (e.g., Would it make more sense to consolidate these into a single PR? A few reasons:
If there's a specific reason to keep them separate (e.g., different reviewers, staged rollout, or one depends on another), that'd be helpful to understand. Otherwise, I'd suggest squashing them into one PR for a cleaner review and merge. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Needs Human Review
Possibly: The initialize handler (lines 263-278) silently swallows setReasoningEffort errors via try/catch, while handleSetEffort re-throws. This matches the existing convention for other optional init steps (MCP servers, subagents), but the inconsistency means an invalid effort at init time is silently ignored while the same value at runtime correctly throws. Consider surfacing the failure in the initialize response for consistency.
Possibly: The TypeScript SDK repeats the effort union type 'low' | 'medium' | 'high' | 'xhigh' | 'max' inline in 3+ locations (TransportOptions, QueryOptions, setEffort parameter) without a shared type alias. The Python SDK defined Effort: TypeAlias and reused it — consider doing the same for the TS SDK.
— qwen3.7-max via Qwen Code /review
|
|
||
| try { | ||
| this.context.config.setReasoningEffort( | ||
| effort as 'low' | 'medium' | 'high' | 'xhigh' | 'max', |
There was a problem hiding this comment.
[Critical] Missing server-side validation for effort value. handleSetEffort accepts any non-empty string and unsafely casts it to 'low' | 'medium' | 'high' | 'xhigh' | 'max' — but the as cast is erased at runtime. An arbitrary string like "banana" or "ultra" is passed directly to config.setReasoningEffort(), which stores it verbatim into the content generator config and eventually sends it to model API providers.
The existing normalizeReasoningEffort() utility in packages/core/src/core/reasoning-effort.ts handles aliases and rejects invalid values, but is never called here. The same unsafe cast also exists in the initialize handler at line 267.
| effort as 'low' | 'medium' | 'high' | 'xhigh' | 'max', | |
| const normalized = normalizeReasoningEffort(effort); | |
| if (!normalized) { | |
| throw new Error( | |
| `Invalid effort: "${effort}". Expected one of: low, medium, high, xhigh, max.`, | |
| ); | |
| } | |
| this.context.config.setReasoningEffort(normalized); |
— qwen3.7-max via Qwen Code /review
| case 'interrupt': | ||
| case 'continue_last_turn': | ||
| case 'set_model': | ||
| case 'set_effort': |
There was a problem hiding this comment.
[Critical] Zero tests for the new set_effort functionality across all three packages:
- CLI: No tests for
handleSetEffort(success, invalid effort, aborted signal), the initialize-with-effort branch, or thecase 'set_effort':routing inControlDispatcher - TypeScript SDK: No test for
Query.setEffort()— the existingsetModel()test inQuery.test.tshas full coverage butsetEfforthas none - Python SDK: No test for
Query.set_effort(), no test for the neweffortvalidation invalidation.py, and the integration test fake CLI inconftest.pydoesn't handleset_effort
Every new control-plane subtype should have at minimum: a routing test, a handler success/error test, and an SDK method test. The set_model tests provide a good template.
— qwen3.7-max via Qwen Code /review
| throw new Error('Request aborted'); | ||
| } | ||
|
|
||
| const effort = payload.effort; |
There was a problem hiding this comment.
[Suggestion] The interactive /effort command routes input through normalizeReasoningEffort() which accepts aliases like "x-high", "maximum", "med", and is case-insensitive. The SDK path bypasses this entirely, doing a bare effort as ReasoningEffort cast.
Concrete downstream consequence: in clampReasoningEffort(), unrecognized strings have REASONING_EFFORT_RANKS[unknown] === undefined, so the rank comparison loop never matches and the function falls through to ranked[ranked.length - 1] — silently escalating to max (the most expensive tier). A user sending "x-high" through the SDK would get maximum cost without any feedback.
| const effort = payload.effort; | |
| const effort = normalizeReasoningEffort(payload.effort); | |
| if (!effort) { | |
| throw new Error( | |
| `Invalid effort: ${JSON.stringify(payload.effort)}. Expected one of: low, medium, high, xhigh, max.`, | |
| ); | |
| } |
— qwen3.7-max via Qwen Code /review
| } | ||
|
|
||
| const effort = payload.effort; | ||
| if (typeof effort !== 'string' || effort.trim() === '') { |
There was a problem hiding this comment.
[Suggestion] config.setReasoningEffort(undefined) is a documented operation that clears the effort override back to the model default. But this validation rejects empty/null, and neither SDK exposes a clear path. Once setEffort('high') is called, the user cannot revert to the model/provider default without creating a new session.
Consider accepting a sentinel value (e.g., "default") that maps to setReasoningEffort(undefined), or accept null/undefined as a valid effort in the protocol types.
— qwen3.7-max via Qwen Code /review
| // Apply initial effort if provided | ||
| if (payload.effort) { | ||
| try { | ||
| this.context.config.setReasoningEffort( |
There was a problem hiding this comment.
[Suggestion] config.setReasoningEffort() early-returns without effect when reasoning === false (config.ts:3264: if (!cfg || cfg.reasoning === false) { return; }). But this code still logs "Initial effort set to: ..." and the initialize response includes can_set_effort: true in capabilities. The SDK consumer proceeds under the false assumption that their effort level is active.
Consider checking this.context.config.contentGeneratorConfig?.reasoning === false before calling and logging a warning, or having setReasoningEffort return a boolean indicating whether it was actually applied.
— qwen3.7-max via Qwen Code /review
| await self._ensure_started() | ||
| await self._send_control_request("set_model", {"model": model}) | ||
|
|
||
| async def set_effort(self, effort: str) -> None: |
There was a problem hiding this comment.
[Suggestion] validate_query_options() checks effort against _VALID_EFFORTS, but this runtime method accepts bare str with no validation. Users get a ValidationError for invalid effort in QueryOptions but can pass the same invalid value through set_effort() and only discover the problem when the server silently misconfigures (see clampReasoningEffort behavior with unknown strings).
| async def set_effort(self, effort: str) -> None: | |
| async def set_effort(self, effort: str) -> None: | |
| if effort not in _VALID_EFFORTS: | |
| raise ValidationError( | |
| f"Invalid effort: {effort!r}. " | |
| "Expected one of: low, medium, high, xhigh, max." | |
| ) | |
| await self._ensure_started() | |
| await self._send_control_request("set_effort", {"effort": effort}) |
— qwen3.7-max via Qwen Code /review
💡 Suggestion: Consolidate SDK PRsHi @juhuan, thanks for the comprehensive SDK work! We noticed you have 15 open PRs that all modify the same core files ( The problem
Suggestion: regroup into 2 PRsWe recommend closing the current 15 PRs and reopening them as 2 consolidated PRs: PR 1 — Covers pure SDK-side option additions (~9 current PRs):
PR 2 — Covers features that also involve CLI-side
This keeps a reasonable separation of concerns while eliminating the merge-conflict chain and making review much more manageable. /cc @juhuan |
|
Closing in favor of consolidated PRs (see suggestion comment above). Please reopen as 2 grouped PRs. |
5 participants
Summary
set_effortcontrol request to CLI'sSystemController, with routing inControlDispatchereffortin theinitializepayload so it can be set at startupcan_set_effortcapability in the initialize responseeffortfield inQueryOptions(with validation),Efforttype alias,set_effort()runtime method,CLIControlSetEffortRequestprotocol typeeffortfield inQueryOptions/TransportOptions(with Zod schema),setEffort()runtime method,SET_EFFORTinControlRequestTypeenum,CLIControlSetEffortRequestprotocol interfaceValid effort values:
low,medium,high,xhigh,max.Test plan
pytest— 58 passed)tsc --noEmit)vitest run— 1163 passed)effortis applied when set viaQueryOptionsset_effort()/setEffort()changes effort at runtime