refactor(anthropic): extract _validate_effort_for_model to prevent drift
The chat completion path (`_apply_output_config`) and the /v1/messages pass-through (`AnthropicMessagesConfig._translate_reasoning_effort_to_anthropic`) both gate `max` / `xhigh` per model. The two sites had diverged from near-identical copies into separately maintained blocks, creating a real drift risk when a new model tier (e.g. Claude 4.8) lands -- a contributor could update one site and miss the other. Centralise the gating in `AnthropicConfig._validate_effort_for_model`, which returns an error message string or `None`. Each call site keeps its own provider-appropriate exception type (`BadRequestError` for the chat path, `AnthropicError` for the /v1/messages pass-through) but the gating decision now comes from one place. Net -11 LOC. Adds a parametrised unit test exercising the helper directly across 4.5 / 4.6 / 4.7 model families and `max` / `xhigh` / lower-effort inputs. Existing tests at both call sites continue to pass unchanged. Addresses Greptile finding on PR #27074.
This commit is contained in:
parent
4f9a3a5c9f
commit
f8f07c5cb7
@ -275,6 +275,41 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig):
|
||||
pass
|
||||
return False
|
||||
|
||||
@staticmethod
|
||||
def _validate_effort_for_model(model: str, effort: Optional[str]) -> Optional[str]:
|
||||
"""Return ``None`` if ``effort`` is allowed on ``model``, else an error message.
|
||||
|
||||
Centralises per-model gating for ``max`` and ``xhigh`` so the chat
|
||||
completion path (``_apply_output_config``) and the /v1/messages
|
||||
pass-through (``AnthropicMessagesConfig._translate_reasoning_effort_to_anthropic``)
|
||||
can't drift when a new model tier is added. Caller raises the
|
||||
provider-appropriate exception type using the returned message.
|
||||
|
||||
``max`` is supported on Claude 4.6 (Opus + Sonnet) and Claude 4.7
|
||||
adaptive-thinking models per
|
||||
https://platform.claude.com/docs/en/build-with-claude/effort. The
|
||||
data-driven ``supports_max_reasoning_effort`` flag in
|
||||
``model_prices_and_context_window.json`` is the source of truth;
|
||||
family-level ``_is_claude_4_6_model`` / ``_is_claude_4_7_model``
|
||||
checks remain as a fallback for OpenRouter / GitHub Copilot /
|
||||
Vercel / Bedrock variants whose model-map entries don't yet carry
|
||||
the flag.
|
||||
|
||||
``xhigh`` is purely data-driven via ``supports_xhigh_reasoning_effort``
|
||||
so enabling it for a new model is a model-map-only change.
|
||||
"""
|
||||
if effort == "max" and not (
|
||||
AnthropicConfig._is_claude_4_6_model(model)
|
||||
or AnthropicConfig._is_claude_4_7_model(model)
|
||||
or AnthropicConfig._supports_effort_level(model, "max")
|
||||
):
|
||||
return f"effort='max' is not supported by this model. Got model: {model}"
|
||||
if effort == "xhigh" and not AnthropicConfig._supports_effort_level(
|
||||
model, "xhigh"
|
||||
):
|
||||
return f"effort='xhigh' is not supported by this model. Got model: {model}"
|
||||
return None
|
||||
|
||||
@staticmethod
|
||||
def _model_supports_effort_param(model: str) -> bool:
|
||||
"""Whether the model accepts ``output_config.effort`` at all.
|
||||
@ -1710,36 +1745,14 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig):
|
||||
model=model,
|
||||
llm_provider=self.custom_llm_provider or "anthropic",
|
||||
)
|
||||
# ``max`` is supported on Claude 4.6 (Opus + Sonnet) and Claude 4.7
|
||||
# adaptive-thinking models (per
|
||||
# https://platform.claude.com/docs/en/build-with-claude/effort).
|
||||
# Prefer the data-driven ``supports_max_reasoning_effort`` flag in
|
||||
# ``model_prices_and_context_window.json`` so new variants only
|
||||
# require a model-map update. Family-level ``_is_claude_4_6_model``
|
||||
# / ``_is_claude_4_7_model`` checks remain as a fallback for
|
||||
# OpenRouter/GitHub Copilot/Vercel/Bedrock variants whose entries
|
||||
# don't yet carry the flag.
|
||||
if effort == "max" and not (
|
||||
self._is_claude_4_6_model(model)
|
||||
or self._is_claude_4_7_model(model)
|
||||
or self._supports_effort_level(model, "max")
|
||||
):
|
||||
# Per-model gating for ``max`` / ``xhigh`` is centralised in
|
||||
# ``_validate_effort_for_model`` so the chat path and the
|
||||
# /v1/messages pass-through stay in lock-step when a new model
|
||||
# tier lands.
|
||||
gate_error = self._validate_effort_for_model(model, effort)
|
||||
if gate_error is not None:
|
||||
raise litellm.exceptions.BadRequestError(
|
||||
message=(
|
||||
f"effort='max' is not supported by this model. "
|
||||
f"Got model: {model}"
|
||||
),
|
||||
model=model,
|
||||
llm_provider=self.custom_llm_provider or "anthropic",
|
||||
)
|
||||
# ``xhigh`` is data-driven via ``supports_xhigh_reasoning_effort`` so
|
||||
# enabling it for a new model is a pure model-map change.
|
||||
if effort == "xhigh" and not self._supports_effort_level(model, "xhigh"):
|
||||
raise litellm.exceptions.BadRequestError(
|
||||
message=(
|
||||
f"effort='xhigh' is not supported by this model. "
|
||||
f"Got model: {model}"
|
||||
),
|
||||
message=gate_error,
|
||||
model=model,
|
||||
llm_provider=self.custom_llm_provider or "anthropic",
|
||||
)
|
||||
|
||||
@ -239,39 +239,15 @@ class AnthropicMessagesConfig(BaseAnthropicMessagesConfig):
|
||||
),
|
||||
status_code=400,
|
||||
)
|
||||
# Per-model gating: ``xhigh`` and ``max`` are only valid on
|
||||
# specific tiers (Opus 4.6/4.7 for max; data-driven for xhigh).
|
||||
# The chat completion path enforces this via
|
||||
# ``_apply_output_config``; mirror it here so /v1/messages
|
||||
# callers see a clean 400 instead of a provider-side error.
|
||||
# ``max`` is supported on Claude 4.6 (Opus + Sonnet) and Claude
|
||||
# 4.7 adaptive-thinking models. Prefer the data-driven
|
||||
# ``supports_max_reasoning_effort`` flag in
|
||||
# ``model_prices_and_context_window.json``; family-level checks
|
||||
# are a fallback for variants whose entries don't yet carry the
|
||||
# flag.
|
||||
if mapped_effort == "max" and not (
|
||||
AnthropicConfig._is_claude_4_6_model(model)
|
||||
or AnthropicConfig._is_claude_4_7_model(model)
|
||||
or AnthropicConfig._supports_effort_level(model, "max")
|
||||
):
|
||||
raise AnthropicError(
|
||||
message=(
|
||||
f"effort='max' is not supported by this model. "
|
||||
f"Got model: {model}"
|
||||
),
|
||||
status_code=400,
|
||||
)
|
||||
if mapped_effort == "xhigh" and not AnthropicConfig._supports_effort_level(
|
||||
model, "xhigh"
|
||||
):
|
||||
raise AnthropicError(
|
||||
message=(
|
||||
f"effort='xhigh' is not supported by this model. "
|
||||
f"Got model: {model}"
|
||||
),
|
||||
status_code=400,
|
||||
)
|
||||
# Per-model gating for ``max`` / ``xhigh`` is centralised in
|
||||
# ``AnthropicConfig._validate_effort_for_model`` so the chat
|
||||
# completion path and this /v1/messages pass-through stay in
|
||||
# lock-step when a new model tier lands.
|
||||
gate_error = AnthropicConfig._validate_effort_for_model(
|
||||
model, mapped_effort
|
||||
)
|
||||
if gate_error is not None:
|
||||
raise AnthropicError(message=gate_error, status_code=400)
|
||||
existing_output_config = optional_params.get("output_config")
|
||||
if not isinstance(existing_output_config, dict):
|
||||
existing_output_config = {}
|
||||
|
||||
@ -2101,6 +2101,44 @@ def test_supports_effort_level_handles_provider_prefixes(model, level, expected)
|
||||
assert AnthropicConfig._supports_effort_level(model, level) is expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"model,effort,expect_error",
|
||||
[
|
||||
# ``max`` accepted on 4.6 / 4.7 (family fallback) and rejected on 4.5.
|
||||
("claude-opus-4-6", "max", False),
|
||||
("claude-sonnet-4-6", "max", False),
|
||||
("claude-opus-4-7", "max", False),
|
||||
("claude-opus-4-5-20251101", "max", True),
|
||||
("claude-sonnet-4-5", "max", True),
|
||||
# ``xhigh`` data-driven; only 4.7 carries the flag in the model map.
|
||||
("claude-opus-4-7", "xhigh", False),
|
||||
("claude-opus-4-6", "xhigh", True),
|
||||
("claude-sonnet-4-6", "xhigh", True),
|
||||
# Lower efforts and ``None`` always pass the gate.
|
||||
("claude-opus-4-5-20251101", "high", False),
|
||||
("claude-haiku-4-5", "low", False),
|
||||
("claude-opus-4-5-20251101", None, False),
|
||||
],
|
||||
)
|
||||
def test_validate_effort_for_model_centralises_per_model_gating(
|
||||
model, effort, expect_error
|
||||
):
|
||||
"""``_validate_effort_for_model`` is the single source of truth for the
|
||||
per-model ``max`` / ``xhigh`` gating that ``_apply_output_config`` (chat
|
||||
completion path) and ``AnthropicMessagesConfig._translate_reasoning_effort_to_anthropic``
|
||||
(/v1/messages pass-through) both rely on. Both call sites raise their
|
||||
own provider-appropriate exception, but the gating decision must come
|
||||
from one place to prevent drift when a new model tier lands.
|
||||
"""
|
||||
err = AnthropicConfig._validate_effort_for_model(model, effort)
|
||||
if expect_error:
|
||||
assert err is not None
|
||||
assert effort in err
|
||||
assert model in err
|
||||
else:
|
||||
assert err is None
|
||||
|
||||
|
||||
def test_transform_request_uses_dynamic_max_tokens():
|
||||
"""
|
||||
Test that transform_request uses dynamic max_tokens based on model
|
||||
|
||||
Loading…
Reference in New Issue
Block a user