From 28c42628b058c81e7c377ed8aceedf83f471d747 Mon Sep 17 00:00:00 2001 From: hlg Date: Wed, 22 Apr 2026 10:24:37 +0800 Subject: [PATCH] fix: normalize DashScope reasoning_effort (minimal vs minimum) DashScope rejects the OpenAI-style value "minimal" with `'reasoning_effort.effort' must be one of: 'none', 'minimum', 'low', 'medium', 'high', 'xhigh'`, but nanobot was passing the string through verbatim. Users who tried the documented "minimal" to disable thinking got a 400; users who tried the DashScope-native "minimum" to work around it got `enable_thinking=True` because the internal comparison was a hard string match on "minimal". Introduce a semantic/wire split in `_build_kwargs`: - `semantic_effort` is the internal canonical form (OpenAI vocabulary). "minimum" on the way in is normalized to "minimal" here so both spellings share one meaning. - `wire_effort` is what we actually serialize. For DashScope with semantic_effort == "minimal" we translate to "minimum" on the way out; other providers are unchanged. - `thinking_enabled` and the Kimi thinking branch now compare on `semantic_effort`, so either user spelling correctly disables provider-side thinking. Tests: - Strengthen `test_dashscope_thinking_disabled_for_minimal` to assert the wire value is "minimum" in addition to the extra_body signal; the original version only checked extra_body and let the invalid-value bug slip through. - Add `test_dashscope_thinking_disabled_for_minimum_alias` so a user who read the DashScope docs and configured "minimum" still gets thinking off. - Add `test_non_dashscope_minimal_not_retranslated` to pin down that the DashScope-specific translation does not leak to OpenAI et al. --- nanobot/providers/openai_compat_provider.py | 31 ++++++++++++++++++--- tests/providers/test_litellm_kwargs.py | 22 +++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/nanobot/providers/openai_compat_provider.py b/nanobot/providers/openai_compat_provider.py index e5cac876..457bf4a2 100644 --- a/nanobot/providers/openai_compat_provider.py +++ b/nanobot/providers/openai_compat_provider.py @@ -387,14 +387,37 @@ class OpenAICompatProvider(LLMProvider): kwargs.update(overrides) break - if reasoning_effort: - kwargs["reasoning_effort"] = reasoning_effort + # Semantic vs. wire distinction for reasoning_effort. + # - semantic_effort is nanobot's internal canonical form (OpenAI's + # vocabulary: "minimal" / "low" / "medium" / "high"). It drives + # decisions like whether to disable provider thinking modes. + # - wire_effort is what we actually serialize to the provider; some + # providers (notably DashScope) reject "minimal" and require + # "minimum" instead. We accept either spelling on input and + # always compare on the semantic form so a user who configured + # "minimum" (DashScope's native spelling) still gets thinking + # disabled instead of accidentally enabled. + semantic_effort: str | None = None + if isinstance(reasoning_effort, str): + semantic_effort = reasoning_effort.lower() + if semantic_effort == "minimum": + semantic_effort = "minimal" + + wire_effort = reasoning_effort + if spec and spec.name == "dashscope" and semantic_effort == "minimal": + # DashScope's reasoning_effort.effort enum accepts: none / + # minimum / low / medium / high / xhigh. Literal "minimal" + # returns 400 invalid_value; translate on the outbound side. + wire_effort = "minimum" + + if wire_effort: + kwargs["reasoning_effort"] = wire_effort # Provider-specific thinking parameters. # Only sent when reasoning_effort is explicitly configured so that # the provider default is preserved otherwise. if spec and reasoning_effort is not None: - thinking_enabled = reasoning_effort.lower() != "minimal" + thinking_enabled = semantic_effort != "minimal" extra: dict[str, Any] | None = None if spec.name == "dashscope": extra = {"enable_thinking": thinking_enabled} @@ -415,7 +438,7 @@ class OpenAICompatProvider(LLMProvider): # so that OpenRouter-style names like "moonshotai/kimi-k2.5" are handled # identically to bare names like "kimi-k2.5". if reasoning_effort is not None and _is_kimi_thinking_model(model_name): - thinking_enabled = reasoning_effort.lower() != "minimal" + thinking_enabled = semantic_effort != "minimal" kwargs.setdefault("extra_body", {}).update( {"thinking": {"type": "enabled" if thinking_enabled else "disabled"}} ) diff --git a/tests/providers/test_litellm_kwargs.py b/tests/providers/test_litellm_kwargs.py index a12aa9fb..5067f094 100644 --- a/tests/providers/test_litellm_kwargs.py +++ b/tests/providers/test_litellm_kwargs.py @@ -731,10 +731,32 @@ def test_dashscope_thinking_enabled_with_reasoning_effort() -> None: def test_dashscope_thinking_disabled_for_minimal() -> None: + """OpenAI-style 'minimal' → DashScope wire value 'minimum' + thinking off. + DashScope rejects the literal string 'minimal' (invalid_value), so we + must translate on the outbound side while still honouring the 'no + thinking' intent via extra_body.""" kw = _build_kwargs_for("dashscope", "qwen3-plus", reasoning_effort="minimal") + assert kw["reasoning_effort"] == "minimum" assert kw["extra_body"] == {"enable_thinking": False} +def test_dashscope_thinking_disabled_for_minimum_alias() -> None: + """Users who read DashScope docs may configure the native 'minimum' + spelling. Internally it's the same semantic as 'minimal' → thinking + must still be disabled (not enabled just because the string isn't + literally 'minimal').""" + kw = _build_kwargs_for("dashscope", "qwen3-plus", reasoning_effort="minimum") + assert kw["reasoning_effort"] == "minimum" + assert kw["extra_body"] == {"enable_thinking": False} + + +def test_non_dashscope_minimal_not_retranslated() -> None: + """The DashScope-specific translation must not leak to other providers; + OpenAI / Anthropic / etc. speak 'minimal' natively.""" + kw = _build_kwargs_for("openai", "gpt-5", reasoning_effort="minimal") + assert kw["reasoning_effort"] == "minimal" + + def test_dashscope_no_extra_body_when_reasoning_effort_none() -> None: kw = _build_kwargs_for("dashscope", "qwen-turbo", reasoning_effort=None) assert "extra_body" not in kw