Repository navigation
feat(model): make DashScope thinking_enable tri-state - #3192
Conversation
…er force-disables thinking Parameters.thinking_enable was a non-optional bool defaulting to False, so the is not None guard in _call_api was always true and every request carried extra_body[enable_thinking]=False even when the caller never configured it — silently disabling thinking on Qwen3+ hybrid-reasoning models. Declare the field bool | None = None, matching the volcengine model, so unset omits the flag while explicit True/False still take effect. Fixes agentscope-ai#3191
github-actions
Bot
added
the
pr/awaiting-review
|
Thanks for the pull request. One thing worth sorting out before it is reviewed:
If you think it belongs here anyway, say so and a maintainer will take a look. |
@qbc2016 PTAL. Context for the guard above: I filed #3191 (Oct 9) and opened this PR the same day — five days before the claim. The claimant @Winoren withdrew on Oct 14, noting this PR "already provides this exact tri-state fix with regression tests" (see their comment on the issue). The assignment also looks like a claim-bot mix-up (the grant was addressed to a different username), and it auto-releases on Given the issue carries |
github-actions
Bot
removed
the
pr/awaiting-review
qbc2016
changed the title
AgentScope Version
2.0.10dev (main @
72f3f6f)Description
DashScopeChatModel.Parameters.thinking_enableis declared as a non-optionalboolwithdefault=False(src/agentscope/model/_dashscope/_model.py:53), while the injection guard in_call_apiisis not None(:258) — which is always true for a plainbool. As a result, every request carriesextra_body["enable_thinking"] = Falseeven when the caller never configured anything, silently disabling thinking mode on Qwen3+ hybrid-reasoning models whose server-side default is thinking enabled.Changes — the field becomes tri-state, matching the volcengine model in this repo (
model/_volcengine/_model.py:40already declaresthinking_enable: bool \| None = Field(default=None, ...)with the identicalis not Noneguard, which needs no change):None)enable_thinking=FalseFalseenable_thinking=FalseTrueenable_thinking=TrueOn prior art: #2346 previously tried a similar direction but chose "only send when enabled", which — as @qbc2016 pointed out — removes the ability to explicitly disable thinking on models where the server default is thinking enabled. Tri-state keeps every explicit configuration working and only stops misinterpreting "caller said nothing" as "disable". Callers who relied on the old implicit default (nothing configured → thinking disabled) must now pass
thinking_enable=Falseexplicitly; this restores parity with a bare DashScope API call, the semantics the volcengine model already follows.How to test
Regression tests in
tests/model_dashscope_thinking_enable_test.py:test_dashscope_thinking_enable_not_sent_when_unset— fails onmain(AssertionError: enable_thinking was force-injected ... : {'enable_thinking': False}), passes with this fix;test_dashscope_thinking_enable_explicit_false_is_sent— pins the explicit-disable behavior so the fix cannot regress it.python -m pytest tests/model_dashscope_thinking_enable_test.py tests/model_dashscope_test.py # 2 + 16 passed.Fixes #3191
Checklist