mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
address review (P2): delegate reasoning_effort='disable' opt-in to provider configs
Greptile P2 (PR #26642): the previous fix used a hardcoded provider allowlist inside `_think_to_reasoning_effort` in `litellm/utils.py`, which violates the project rule that provider-specific knowledge belongs in `llms/<provider>/`. This commit replaces the allowlist with a per-provider class-level capability flag, so each provider's config decides for itself whether `reasoning_effort='disable'` is a valid value upstream: - `BaseConfig.supports_reasoning_disable: bool = False` (new, default false) - `VertexGeminiConfig.supports_reasoning_disable = True` (also covers `gemini/...` via the `GoogleAIStudioGeminiConfig` subclass -- Gemini maps "disable" to `thinkingConfig.thinkingBudget = 0`) - `OllamaConfig.supports_reasoning_disable = True` - `OllamaChatConfig.supports_reasoning_disable = True` (Ollama natively understands `think=False`; existing `reasoning_effort="disable"` -> native `think=False` mapping is unchanged) `_think_to_reasoning_effort` now accepts the already-resolved `provider_config` (passed through from `get_optional_params`, where it is already looked up via `ProviderConfigManager.get_provider_chat_config`) and checks `getattr(provider_config, "supports_reasoning_disable", False)` instead of comparing `custom_llm_provider` against a string set. Behavior for providers like Anthropic / Bedrock / OpenAI o-series is unchanged: their configs inherit the `False` default, so `think:false` is silently dropped (with a debug log) rather than injecting an unmapped "disable" value that would raise `ValueError` / `BadRequestError`. Adding a new "disable"-capable provider is now a one-line change in that provider's config, with no need to touch `utils.py`. Tests ----- - New `TestProviderConfigsOptInToReasoningDisable` asserts the four configs above opt in (`True`) while `AnthropicConfig` and `AmazonConverseConfig` keep the safe `False` default -- guards against accidental regressions. - Existing helper-level tests updated to pass small fake config stand-ins (`_FakeConfigSupportsDisable`, `_FakeConfigRejectsDisable`) instead of relying on a hardcoded provider-name match. - Added `test_think_false_dropped_when_no_provider_config_passed` for the conservative no-config code path. `python -m pytest tests/test_litellm/test_think_param_normalization.py -q` => 21 passed.
This commit is contained in:
skgandikota
parent
b953bfe243
commit
f451082541
6 changed files with 121 additions and 33 deletions
|
|
@ -79,6 +79,19 @@ class BaseLLMException(Exception):
|
|||
|
||||
|
||||
class BaseConfig(ABC):
|
||||
# Per-provider capability flag: ``True`` means this provider's
|
||||
# ``map_openai_params`` / ``_map_reasoning_effort`` (or equivalent)
|
||||
# accepts the literal value ``reasoning_effort="disable"`` and translates
|
||||
# it into a real "no reasoning" signal upstream (e.g. Gemini's
|
||||
# ``thinkingConfig.thinkingBudget = 0``, Ollama's native ``think=False``).
|
||||
# Providers that only accept ``low|medium|high|minimal|none|None`` and
|
||||
# would raise on ``"disable"`` (Anthropic, Bedrock, OpenAI o-series, ...)
|
||||
# must leave this as ``False``. Used by
|
||||
# ``litellm.utils._think_to_reasoning_effort`` to decide whether the
|
||||
# Ollama-style ``"think": false`` request flag should be translated to
|
||||
# ``reasoning_effort="disable"`` for this provider, or silently dropped.
|
||||
supports_reasoning_disable: bool = False
|
||||
|
||||
def __init__(self):
|
||||
pass
|
||||
|
||||
|
|
|
|||
|
|
@ -47,6 +47,9 @@ else:
|
|||
|
||||
|
||||
class OllamaChatConfig(BaseConfig):
|
||||
# See OllamaConfig: `reasoning_effort="disable"` is mapped to the native
|
||||
# `think=False` parameter upstream, so it is safe to inject for `think:false`.
|
||||
supports_reasoning_disable: bool = True
|
||||
"""
|
||||
Reference: https://github.com/ollama/ollama/blob/main/docs/api.md#parameters
|
||||
|
||||
|
|
|
|||
|
|
@ -40,6 +40,10 @@ else:
|
|||
|
||||
|
||||
class OllamaConfig(BaseConfig):
|
||||
# Ollama natively understands `think=False`, and litellm's existing
|
||||
# `reasoning_effort="disable"` -> `think=False` mapping turns the OpenAI-
|
||||
# style flag into the native one upstream. Safe to inject "disable".
|
||||
supports_reasoning_disable: bool = True
|
||||
"""
|
||||
Reference: https://github.com/ollama/ollama/blob/main/docs/api.md#parameters
|
||||
|
||||
|
|
|
|||
|
|
@ -168,6 +168,10 @@ class VertexAIBaseConfig:
|
|||
|
||||
|
||||
class VertexGeminiConfig(VertexAIBaseConfig, BaseConfig):
|
||||
# Gemini's `thinkingConfig.thinkingBudget = 0` translates the OpenAI-style
|
||||
# `reasoning_effort="disable"` into a real "no reasoning" upstream signal,
|
||||
# so it is safe to inject "disable" for `think:false` requests.
|
||||
supports_reasoning_disable: bool = True
|
||||
"""
|
||||
Reference: https://cloud.google.com/vertex-ai/docs/generative-ai/chat/test-chat-prompts
|
||||
Reference: https://cloud.google.com/vertex-ai/generative-ai/docs/model-reference/inference
|
||||
|
|
|
|||
|
|
@ -3683,6 +3683,7 @@ def _think_to_reasoning_effort(
|
|||
think_value: Any,
|
||||
non_default_params: dict,
|
||||
supported_params: List[str],
|
||||
provider_config: Optional[Any] = None,
|
||||
custom_llm_provider: Optional[str] = None,
|
||||
) -> None:
|
||||
"""
|
||||
|
|
@ -3701,12 +3702,10 @@ def _think_to_reasoning_effort(
|
|||
set ``reasoning_effort = "disable"`` on ``non_default_params`` -- but only
|
||||
when (a) the caller has not already supplied ``reasoning_effort``, (b) the
|
||||
provider lists ``reasoning_effort`` as a supported param, and (c) the
|
||||
provider's transformation layer is known to recognise the literal value
|
||||
``"disable"`` (currently Gemini / Vertex AI / Ollama). Other providers
|
||||
(Anthropic, Bedrock, OpenAI o-series, ...) advertise ``reasoning_effort``
|
||||
but only accept ``low|medium|high|minimal|none|None``; passing
|
||||
``"disable"`` to them raises ``ValueError`` / ``BadRequestError``. For
|
||||
those providers, ``think`` is silently dropped (with a debug log).
|
||||
provider's config opts in via the ``supports_reasoning_disable`` class
|
||||
flag (currently Gemini / Vertex AI / Ollama). For other providers (whose
|
||||
``_map_reasoning_effort`` raises on the literal ``"disable"``),
|
||||
``think:false`` is silently dropped (with a debug log).
|
||||
- ``think`` truthy: ``reasoning_effort`` is left untouched (callers that
|
||||
want to enable reasoning should pass ``reasoning_effort`` explicitly).
|
||||
- ``think`` is an unrecognized string: ignored.
|
||||
|
|
@ -3725,30 +3724,23 @@ def _think_to_reasoning_effort(
|
|||
else:
|
||||
think_bool = bool(think_value)
|
||||
|
||||
# Providers whose `_map_reasoning_effort` (or equivalent) accepts the
|
||||
# literal "disable" value. Adding to this list is safe; omitting a provider
|
||||
# that *does* accept it just means `think:false` is silently ignored there.
|
||||
_PROVIDERS_SUPPORTING_REASONING_DISABLE = {
|
||||
"gemini",
|
||||
"vertex_ai",
|
||||
"vertex_ai_beta",
|
||||
"ollama",
|
||||
"ollama_chat",
|
||||
}
|
||||
provider_supports_disable = bool(
|
||||
getattr(provider_config, "supports_reasoning_disable", False)
|
||||
)
|
||||
|
||||
if (
|
||||
think_bool is False
|
||||
and "reasoning_effort" not in non_default_params
|
||||
and "reasoning_effort" in supported_params
|
||||
and (custom_llm_provider or "") in _PROVIDERS_SUPPORTING_REASONING_DISABLE
|
||||
and provider_supports_disable
|
||||
):
|
||||
non_default_params["reasoning_effort"] = "disable"
|
||||
elif think_bool is False:
|
||||
verbose_logger.debug(
|
||||
"litellm: 'think=False' was passed but provider %r does not accept "
|
||||
"reasoning_effort='disable'; dropping silently. Pass "
|
||||
"reasoning_effort explicitly if your provider supports a "
|
||||
"'disable'-equivalent value.",
|
||||
"litellm: 'think=False' was passed but provider %r does not opt "
|
||||
"in to reasoning_effort='disable' (supports_reasoning_disable=False); "
|
||||
"dropping silently. Pass reasoning_effort explicitly if your "
|
||||
"provider supports a 'disable'-equivalent value.",
|
||||
custom_llm_provider,
|
||||
)
|
||||
elif think_bool is True:
|
||||
|
|
@ -4158,6 +4150,7 @@ def get_optional_params( # noqa: PLR0915
|
|||
think_value=_think_value,
|
||||
non_default_params=non_default_params,
|
||||
supported_params=supported_params,
|
||||
provider_config=provider_config,
|
||||
custom_llm_provider=custom_llm_provider,
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -15,6 +15,23 @@ sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..",
|
|||
from litellm.utils import _think_to_reasoning_effort, get_optional_params
|
||||
|
||||
|
||||
class _FakeConfigSupportsDisable:
|
||||
"""Stand-in for a provider config (e.g. ``GeminiConfig`` /
|
||||
``OllamaConfig``) that opts in to ``reasoning_effort='disable'`` via the
|
||||
``supports_reasoning_disable`` class flag."""
|
||||
|
||||
supports_reasoning_disable = True
|
||||
|
||||
|
||||
class _FakeConfigRejectsDisable:
|
||||
"""Stand-in for a provider config (e.g. ``AnthropicConfig`` /
|
||||
``BedrockConverseConfig``) whose ``_map_reasoning_effort`` would raise on
|
||||
the literal ``'disable'``; default value of the ``BaseConfig`` class flag
|
||||
is ``False``."""
|
||||
|
||||
supports_reasoning_disable = False
|
||||
|
||||
|
||||
class TestThinkToReasoningEffortHelper:
|
||||
def test_think_false_bool_sets_reasoning_effort_disable(self):
|
||||
non_default_params: dict = {}
|
||||
|
|
@ -24,6 +41,7 @@ class TestThinkToReasoningEffortHelper:
|
|||
think_value=False,
|
||||
non_default_params=non_default_params,
|
||||
supported_params=supported_params,
|
||||
provider_config=_FakeConfigSupportsDisable(),
|
||||
custom_llm_provider="gemini",
|
||||
)
|
||||
|
||||
|
|
@ -37,6 +55,7 @@ class TestThinkToReasoningEffortHelper:
|
|||
think_value=falsy,
|
||||
non_default_params=non_default_params,
|
||||
supported_params=["reasoning_effort"],
|
||||
provider_config=_FakeConfigSupportsDisable(),
|
||||
custom_llm_provider="gemini",
|
||||
)
|
||||
|
||||
|
|
@ -49,6 +68,7 @@ class TestThinkToReasoningEffortHelper:
|
|||
think_value=True,
|
||||
non_default_params=non_default_params,
|
||||
supported_params=["reasoning_effort"],
|
||||
provider_config=_FakeConfigSupportsDisable(),
|
||||
custom_llm_provider="gemini",
|
||||
)
|
||||
|
||||
|
|
@ -63,6 +83,7 @@ class TestThinkToReasoningEffortHelper:
|
|||
think_value=False,
|
||||
non_default_params=non_default_params,
|
||||
supported_params=["temperature", "top_p"],
|
||||
provider_config=_FakeConfigRejectsDisable(),
|
||||
custom_llm_provider="cohere_chat",
|
||||
)
|
||||
|
||||
|
|
@ -75,6 +96,7 @@ class TestThinkToReasoningEffortHelper:
|
|||
think_value=False,
|
||||
non_default_params=non_default_params,
|
||||
supported_params=["reasoning_effort"],
|
||||
provider_config=_FakeConfigSupportsDisable(),
|
||||
custom_llm_provider="gemini",
|
||||
)
|
||||
|
||||
|
|
@ -90,29 +112,40 @@ class TestThinkToReasoningEffortHelper:
|
|||
think_value=False,
|
||||
non_default_params=non_default_params,
|
||||
supported_params=["reasoning_effort"],
|
||||
provider_config=_FakeConfigSupportsDisable(),
|
||||
custom_llm_provider="gemini",
|
||||
)
|
||||
|
||||
assert non_default_params["reasoning_effort"] == ""
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"provider",
|
||||
["anthropic", "bedrock", "openai", "azure", "vertex_ai_anthropic"],
|
||||
)
|
||||
def test_think_false_dropped_for_providers_that_reject_disable(self, provider):
|
||||
"""Providers whose `_map_reasoning_effort` does not accept the literal
|
||||
``"disable"`` (Anthropic, Bedrock, OpenAI o-series, ...) would raise a
|
||||
runtime ``ValueError`` / ``BadRequestError`` if we injected
|
||||
``reasoning_effort="disable"``. For these providers, ``think:false``
|
||||
must be silently dropped instead. Regression test for the P1 review
|
||||
comment on PR #26642."""
|
||||
def test_think_false_dropped_when_provider_config_rejects_disable(self):
|
||||
"""Providers whose config has ``supports_reasoning_disable=False`` (the
|
||||
``BaseConfig`` default — Anthropic, Bedrock, OpenAI o-series, ...)
|
||||
would raise on ``reasoning_effort='disable'``. ``think:false`` must be
|
||||
silently dropped instead. Regression for the P1 review on PR #26642."""
|
||||
non_default_params: dict = {}
|
||||
|
||||
_think_to_reasoning_effort(
|
||||
think_value=False,
|
||||
non_default_params=non_default_params,
|
||||
supported_params=["reasoning_effort"],
|
||||
custom_llm_provider=provider,
|
||||
provider_config=_FakeConfigRejectsDisable(),
|
||||
custom_llm_provider="anthropic",
|
||||
)
|
||||
|
||||
assert "reasoning_effort" not in non_default_params
|
||||
|
||||
def test_think_false_dropped_when_no_provider_config_passed(self):
|
||||
"""If we cannot determine the provider's capability (no config), be
|
||||
conservative and drop rather than risk a runtime ValueError."""
|
||||
non_default_params: dict = {}
|
||||
|
||||
_think_to_reasoning_effort(
|
||||
think_value=False,
|
||||
non_default_params=non_default_params,
|
||||
supported_params=["reasoning_effort"],
|
||||
provider_config=None,
|
||||
custom_llm_provider="some_unknown_provider",
|
||||
)
|
||||
|
||||
assert "reasoning_effort" not in non_default_params
|
||||
|
|
@ -124,12 +157,50 @@ class TestThinkToReasoningEffortHelper:
|
|||
think_value="maybe",
|
||||
non_default_params=non_default_params,
|
||||
supported_params=["reasoning_effort"],
|
||||
provider_config=_FakeConfigSupportsDisable(),
|
||||
custom_llm_provider="gemini",
|
||||
)
|
||||
|
||||
assert "reasoning_effort" not in non_default_params
|
||||
|
||||
|
||||
class TestProviderConfigsOptInToReasoningDisable:
|
||||
"""Verifies the per-provider opt-in flags wired up on the actual config
|
||||
classes — these are what the real ``get_optional_params`` flow consults."""
|
||||
|
||||
def test_gemini_configs_opt_in(self):
|
||||
from litellm.llms.gemini.chat.transformation import GoogleAIStudioGeminiConfig
|
||||
from litellm.llms.vertex_ai.gemini.vertex_and_google_ai_studio_gemini import (
|
||||
VertexGeminiConfig,
|
||||
)
|
||||
|
||||
assert VertexGeminiConfig.supports_reasoning_disable is True
|
||||
assert GoogleAIStudioGeminiConfig.supports_reasoning_disable is True
|
||||
|
||||
def test_ollama_configs_opt_in(self):
|
||||
from litellm.llms.ollama.chat.transformation import OllamaChatConfig
|
||||
from litellm.llms.ollama.completion.transformation import OllamaConfig
|
||||
|
||||
assert OllamaConfig.supports_reasoning_disable is True
|
||||
assert OllamaChatConfig.supports_reasoning_disable is True
|
||||
|
||||
def test_anthropic_config_does_not_opt_in(self):
|
||||
"""Anthropic's ``_map_reasoning_effort`` raises on ``'disable'``, so it
|
||||
must keep the ``BaseConfig`` default of ``False``."""
|
||||
from litellm.llms.anthropic.chat.transformation import AnthropicConfig
|
||||
|
||||
assert AnthropicConfig.supports_reasoning_disable is False
|
||||
|
||||
def test_bedrock_config_does_not_opt_in(self):
|
||||
"""Bedrock Converse rejects any ``reasoning_effort`` value outside
|
||||
``low|medium|high``, so it must not opt in."""
|
||||
from litellm.llms.bedrock.chat.converse_transformation import (
|
||||
AmazonConverseConfig,
|
||||
)
|
||||
|
||||
assert AmazonConverseConfig.supports_reasoning_disable is False
|
||||
|
||||
|
||||
class TestGetOptionalParamsThinkFalse:
|
||||
"""End-to-end tests through ``get_optional_params`` against real provider
|
||||
configs -- no network calls."""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue