mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-04 02:31:27 +00:00
fix: address Greptile review feedback on PR #26439
Three concerns raised by bot reviewers, all addressed:
1. CodeQL cyclic-import warning
``experimental_pass_through/transformation.py`` imported from the
parent ``..transformation`` module, which CodeQL flagged as a
potential cycle. Extracted the helper into a new leaf module
``vertex_ai_partner_models/anthropic/output_params_utils.py`` that
has no heavy imports of its own. Both transformation files now
import from it cleanly. Renamed the helper from the underscore-
prefixed ``_sanitize_vertex_anthropic_output_params`` to the
public ``sanitize_vertex_anthropic_output_params`` since it is now
shared across modules.
2. Greptile P2: redundant ``None`` guard on ``extra_kwargs``
``handler.py`` had two ``extra_kwargs = extra_kwargs if ... else {}``
coercions; the second was a no-op because line 220 already
coerced. Removed the second one and added a NOTE comment so future
readers understand ``extra_kwargs`` is guaranteed non-None at the
point of use.
3. Greptile P2: misleading "already translated" docstring
The docstring claimed the translator above mapped
``output_config.format`` to ``response_format``, but Greptile
correctly traced the code and found that only the legacy top-level
``output_format`` was being translated — ``output_config.format``
was being silently dropped on the adapter path. Two-part fix:
a. Code: extended ``_translate_output_format_to_openai`` to accept
both shapes (top-level ``output_format`` AND
``output_config.format`` sub-key). Top-level still takes
precedence when both are supplied. This means callers using the
newer Anthropic Structured Outputs API now have their schema
properly forwarded to non-Anthropic backends as
``response_format``.
b. Tests: rewrote the misleading docstring to describe what
actually happens, plus added two new tests:
* ``test_output_format_top_level_still_translates`` —
regression guard for the legacy path
* ``test_output_format_takes_precedence_over_output_config_format``
— documents the precedence rule explicitly
Tests: 28/28 pass (was 26/26 before; +2 for the new translation
behavior + precedence). All run in ~0.5s, no real network calls.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
b9e46cbdb7
commit
79517bc628
7 changed files with 156 additions and 65 deletions
|
|
@ -254,7 +254,8 @@ class LiteLLMMessagesToCompletionTransformationHandler:
|
|||
# ``AnthropicMessagesRequestOptionalParams``, also extend
|
||||
# ``ANTHROPIC_ONLY_REQUEST_KEYS`` here so it doesn't silently leak.
|
||||
excluded_keys = ANTHROPIC_ONLY_REQUEST_KEYS | {"anthropic_messages"}
|
||||
extra_kwargs = extra_kwargs if extra_kwargs is not None else {}
|
||||
# NOTE: extra_kwargs was already coerced from None to {} at the top of
|
||||
# this method (line ~220). It is guaranteed to be a dict here.
|
||||
for key, value in extra_kwargs.items():
|
||||
if (
|
||||
key == "litellm_logging_obj"
|
||||
|
|
|
|||
|
|
@ -664,7 +664,7 @@ class LiteLLMAnthropicMessagesAdapter:
|
|||
|
||||
@staticmethod
|
||||
def translate_anthropic_thinking_to_reasoning_effort(
|
||||
thinking: Dict[str, Any]
|
||||
thinking: Dict[str, Any],
|
||||
) -> Optional[str]:
|
||||
"""
|
||||
Translate Anthropic's thinking parameter to OpenAI's reasoning_effort.
|
||||
|
|
@ -1081,10 +1081,23 @@ class LiteLLMAnthropicMessagesAdapter:
|
|||
anthropic_message_request: AnthropicMessagesRequest,
|
||||
new_kwargs: ChatCompletionRequest,
|
||||
) -> None:
|
||||
"""Translate output_format to response_format when applicable."""
|
||||
if "output_format" not in anthropic_message_request:
|
||||
return
|
||||
output_format = anthropic_message_request["output_format"]
|
||||
"""Translate Anthropic structured-output config to OpenAI ``response_format``.
|
||||
|
||||
Accepts either the legacy top-level ``output_format`` field OR the
|
||||
newer ``output_config.format`` (sub-key on ``output_config``) so that
|
||||
both shapes flow through to non-Anthropic backends as
|
||||
``response_format``. Without the ``output_config.format`` branch,
|
||||
callers using the new Anthropic Structured Outputs API would have
|
||||
their schema silently dropped on the adapter path — only the legacy
|
||||
top-level ``output_format`` was being mapped.
|
||||
|
||||
``output_format`` takes precedence when both are provided.
|
||||
"""
|
||||
output_format: Any = anthropic_message_request.get("output_format")
|
||||
if not output_format:
|
||||
output_config = anthropic_message_request.get("output_config")
|
||||
if isinstance(output_config, dict):
|
||||
output_format = output_config.get("format")
|
||||
if not output_format:
|
||||
return
|
||||
response_format = self.translate_anthropic_output_format_to_openai(
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ from litellm.types.llms.vertex_ai import VertexPartnerProvider
|
|||
from litellm.types.router import GenericLiteLLMParams
|
||||
|
||||
from ....vertex_llm_base import VertexBase
|
||||
from ..transformation import _sanitize_vertex_anthropic_output_params
|
||||
from ..output_params_utils import sanitize_vertex_anthropic_output_params
|
||||
|
||||
|
||||
class VertexAIPartnerModelsAnthropicMessagesConfig(AnthropicMessagesConfig, VertexBase):
|
||||
|
|
@ -163,6 +163,6 @@ class VertexAIPartnerModelsAnthropicMessagesConfig(AnthropicMessagesConfig, Vert
|
|||
# and ``output_format``, but rejects ``output_config.effort`` with 400
|
||||
# "Extra inputs are not permitted". Sanitize in place so the supported
|
||||
# bits flow through.
|
||||
_sanitize_vertex_anthropic_output_params(anthropic_messages_request)
|
||||
sanitize_vertex_anthropic_output_params(anthropic_messages_request)
|
||||
|
||||
return anthropic_messages_request
|
||||
|
|
|
|||
|
|
@ -0,0 +1,50 @@
|
|||
"""
|
||||
Shared sanitization for ``output_config`` / ``output_format`` on Vertex AI
|
||||
Claude. Lives in its own module so both the chat-completion transformation
|
||||
(``transformation.py``) and the Messages pass-through transformation
|
||||
(``experimental_pass_through/transformation.py``) can import it without
|
||||
forming a cycle through the parent module's heavier imports.
|
||||
|
||||
CodeQL flagged the ``..transformation`` import path as a potential cyclic
|
||||
import; extracting the helper into a leaf module resolves the warning and
|
||||
keeps the parent module's import surface narrow.
|
||||
"""
|
||||
|
||||
# Keys inside ``output_config`` that Vertex AI Claude does not accept.
|
||||
# Today only ``effort`` triggers "Extra inputs are not permitted"; add new
|
||||
# entries here as Vertex parity drifts. Keep this list narrow — anything
|
||||
# Vertex DOES accept (e.g. ``format`` for structured outputs) must be
|
||||
# preserved so callers can rely on Anthropic-native features.
|
||||
VERTEX_UNSUPPORTED_OUTPUT_CONFIG_KEYS: frozenset = frozenset({"effort"})
|
||||
|
||||
|
||||
def sanitize_vertex_anthropic_output_params(data: dict) -> None:
|
||||
"""
|
||||
Strip Vertex-unsupported keys from ``output_config`` /
|
||||
``output_format`` in-place; forward whatever remains.
|
||||
|
||||
Behavior:
|
||||
* ``output_config`` containing only unsupported keys (e.g. ``effort``
|
||||
alone) is removed entirely so the request body has no empty dict.
|
||||
* ``output_config`` containing a mix of supported + unsupported keys
|
||||
has the unsupported subset filtered out and the rest forwarded.
|
||||
* ``output_config`` that is supported in full passes through unchanged.
|
||||
* ``output_format`` is forwarded as-is (Vertex AI Claude accepts it).
|
||||
* Non-dict values for ``output_config`` are dropped to avoid sending
|
||||
malformed payloads downstream.
|
||||
"""
|
||||
output_config = data.get("output_config")
|
||||
if output_config is None:
|
||||
return
|
||||
if not isinstance(output_config, dict):
|
||||
data.pop("output_config", None)
|
||||
return
|
||||
sanitized = {
|
||||
k: v
|
||||
for k, v in output_config.items()
|
||||
if k not in VERTEX_UNSUPPORTED_OUTPUT_CONFIG_KEYS
|
||||
}
|
||||
if sanitized:
|
||||
data["output_config"] = sanitized
|
||||
else:
|
||||
data.pop("output_config", None)
|
||||
|
|
@ -10,45 +10,7 @@ from litellm.types.llms.openai import AllMessageValues
|
|||
from litellm.types.utils import ModelResponse
|
||||
|
||||
from ....anthropic.chat.transformation import AnthropicConfig
|
||||
|
||||
# Keys inside ``output_config`` that Vertex AI Claude does not accept.
|
||||
# Today only ``effort`` triggers "Extra inputs are not permitted"; add new
|
||||
# entries here as Vertex parity drifts. Keep this list narrow — anything
|
||||
# Vertex DOES accept (e.g. ``format`` for structured outputs) must be
|
||||
# preserved so callers can rely on Anthropic-native features.
|
||||
_VERTEX_UNSUPPORTED_OUTPUT_CONFIG_KEYS = frozenset({"effort"})
|
||||
|
||||
|
||||
def _sanitize_vertex_anthropic_output_params(data: dict) -> None:
|
||||
"""
|
||||
Strip Vertex-unsupported keys from ``output_config`` / ``output_format``
|
||||
in-place; forward whatever remains.
|
||||
|
||||
Behavior:
|
||||
* ``output_config`` containing only unsupported keys (e.g. ``effort``
|
||||
alone) is removed entirely so the request body has no empty dict.
|
||||
* ``output_config`` containing a mix of supported + unsupported keys has
|
||||
the unsupported subset filtered out and the rest forwarded.
|
||||
* ``output_config`` that is supported in full passes through unchanged.
|
||||
* ``output_format`` is forwarded as-is (Vertex AI Claude accepts it).
|
||||
* Non-dict values for ``output_config`` are dropped to avoid sending
|
||||
malformed payloads downstream.
|
||||
"""
|
||||
output_config = data.get("output_config")
|
||||
if output_config is None:
|
||||
return
|
||||
if not isinstance(output_config, dict):
|
||||
data.pop("output_config", None)
|
||||
return
|
||||
sanitized = {
|
||||
k: v
|
||||
for k, v in output_config.items()
|
||||
if k not in _VERTEX_UNSUPPORTED_OUTPUT_CONFIG_KEYS
|
||||
}
|
||||
if sanitized:
|
||||
data["output_config"] = sanitized
|
||||
else:
|
||||
data.pop("output_config", None)
|
||||
from .output_params_utils import sanitize_vertex_anthropic_output_params
|
||||
|
||||
|
||||
class VertexAIError(Exception):
|
||||
|
|
@ -149,7 +111,7 @@ class VertexAIAnthropicConfig(AnthropicConfig):
|
|||
# Vertex returns 400 "Extra inputs are not permitted". Sanitize in place:
|
||||
# forward the structured-output bits, drop the unsupported keys.
|
||||
# Same treatment for the legacy top-level ``output_format`` field.
|
||||
_sanitize_vertex_anthropic_output_params(data)
|
||||
sanitize_vertex_anthropic_output_params(data)
|
||||
|
||||
tools = optional_params.get("tools")
|
||||
tool_search_used = self.is_tool_search_used(tools)
|
||||
|
|
|
|||
|
|
@ -46,10 +46,13 @@ from litellm.llms.anthropic.experimental_pass_through.adapters.handler import (
|
|||
MESSAGES = [{"role": "user", "content": "hello"}]
|
||||
|
||||
|
||||
def _call_prepare(extra_kwargs, model="gpt-4o", **overrides):
|
||||
def _call_prepare(extra_kwargs, model="gpt-4o", output_format=None, **overrides):
|
||||
"""
|
||||
Drive ``_prepare_completion_kwargs`` with the minimum scaffolding needed.
|
||||
|
||||
``output_format`` is a top-level parameter on the function, so callers
|
||||
pass it explicitly here rather than tucking it into ``extra_kwargs``.
|
||||
|
||||
Uses an explicit-None check on ``extra_kwargs`` so callers can test the
|
||||
falsy-empty-dict path. The fallback ``or {}`` pattern PR #22727 used here
|
||||
masked the no-extra-kwargs case from ever exercising the test's intent.
|
||||
|
|
@ -68,7 +71,7 @@ def _call_prepare(extra_kwargs, model="gpt-4o", **overrides):
|
|||
tools=None,
|
||||
top_k=None,
|
||||
top_p=None,
|
||||
output_format=None,
|
||||
output_format=output_format,
|
||||
extra_kwargs=extra_kwargs,
|
||||
)
|
||||
|
||||
|
|
@ -107,22 +110,84 @@ class TestOutputConfigStrippedFromCompletionKwargs:
|
|||
"reject it with 400 'Extra inputs are not permitted'"
|
||||
)
|
||||
|
||||
def test_output_config_with_format_is_stripped_format_already_translated(self):
|
||||
"""Even when ``output_config`` carries useful structured-output info,
|
||||
the raw key must be excluded — the translator above has already mapped
|
||||
``output_config.format`` to ``response_format`` (the OpenAI-shaped key
|
||||
the downstream backend understands)."""
|
||||
def test_output_config_format_translated_to_response_format(self):
|
||||
"""When ``output_config`` carries structured-output ``format``, the
|
||||
translator now maps it to OpenAI's ``response_format`` so non-Anthropic
|
||||
backends see the schema in their native shape. The raw
|
||||
``output_config`` key is still stripped from ``completion_kwargs`` —
|
||||
only the translated ``response_format`` survives.
|
||||
|
||||
Before this PR, only the legacy top-level ``output_format`` was
|
||||
translated; ``output_config.format`` was silently dropped on the
|
||||
adapter path even when the schema was correctly supplied (issue
|
||||
flagged by Greptile review of the initial fix).
|
||||
"""
|
||||
schema = {
|
||||
"type": "object",
|
||||
"additionalProperties": False,
|
||||
"properties": {"name": {"type": "string"}},
|
||||
}
|
||||
extra_kwargs = {
|
||||
"custom_llm_provider": "azure",
|
||||
"output_config": {
|
||||
"format": {"type": "json_schema", "schema": {"type": "object"}}
|
||||
},
|
||||
"output_config": {"format": {"type": "json_schema", "schema": schema}},
|
||||
}
|
||||
|
||||
result = _call_prepare(extra_kwargs=extra_kwargs)
|
||||
completion_kwargs = result[0] if isinstance(result, tuple) else result
|
||||
|
||||
# Raw Anthropic-shaped key is gone (would 400 on non-Anthropic backends).
|
||||
assert "output_config" not in completion_kwargs
|
||||
# Translated OpenAI-shaped key is present so the schema actually
|
||||
# reaches the downstream backend.
|
||||
assert "response_format" in completion_kwargs, (
|
||||
"output_config.format must be translated to response_format — "
|
||||
"without this, structured-output schemas are silently dropped on "
|
||||
"the adapter path"
|
||||
)
|
||||
|
||||
def test_output_format_top_level_still_translates(self):
|
||||
"""Regression guard: the legacy top-level ``output_format`` field must
|
||||
continue to translate to ``response_format``. The new
|
||||
``output_config.format`` path must not break this existing behavior."""
|
||||
schema = {"type": "object", "properties": {"name": {"type": "string"}}}
|
||||
result = _call_prepare(
|
||||
extra_kwargs={"custom_llm_provider": "azure"},
|
||||
output_format={"type": "json_schema", "schema": schema},
|
||||
)
|
||||
completion_kwargs = result[0] if isinstance(result, tuple) else result
|
||||
|
||||
assert "response_format" in completion_kwargs
|
||||
|
||||
def test_output_format_takes_precedence_over_output_config_format(self):
|
||||
"""When both top-level ``output_format`` and ``output_config.format``
|
||||
are present, the legacy top-level ``output_format`` wins. Documents
|
||||
which one the translator picks rather than leaving it implementation-
|
||||
defined."""
|
||||
winning_schema = {
|
||||
"type": "object",
|
||||
"properties": {"top_level": {"type": "string"}},
|
||||
}
|
||||
losing_schema = {
|
||||
"type": "object",
|
||||
"properties": {"nested": {"type": "string"}},
|
||||
}
|
||||
result = _call_prepare(
|
||||
extra_kwargs={
|
||||
"custom_llm_provider": "azure",
|
||||
"output_config": {
|
||||
"format": {"type": "json_schema", "schema": losing_schema}
|
||||
},
|
||||
},
|
||||
output_format={"type": "json_schema", "schema": winning_schema},
|
||||
)
|
||||
completion_kwargs = result[0] if isinstance(result, tuple) else result
|
||||
|
||||
assert "response_format" in completion_kwargs
|
||||
# Verify the winning_schema (top-level output_format) was used,
|
||||
# not the losing one nested under output_config.
|
||||
rendered = str(completion_kwargs["response_format"])
|
||||
assert "top_level" in rendered
|
||||
assert "nested" not in rendered
|
||||
|
||||
def test_other_extra_kwargs_still_passed_through(self):
|
||||
"""Regression guard: the strip must be narrow. Unrelated fields like
|
||||
|
|
|
|||
|
|
@ -693,32 +693,32 @@ def test_sanitize_vertex_anthropic_output_params_unit():
|
|||
"""Direct unit coverage for the helper itself (used by both Vertex
|
||||
Anthropic transformation paths). Mirrors the integration assertions
|
||||
above without going through the full ``transform_request`` stack."""
|
||||
from litellm.llms.vertex_ai.vertex_ai_partner_models.anthropic.transformation import (
|
||||
_sanitize_vertex_anthropic_output_params,
|
||||
from litellm.llms.vertex_ai.vertex_ai_partner_models.anthropic.output_params_utils import (
|
||||
sanitize_vertex_anthropic_output_params,
|
||||
)
|
||||
|
||||
# No-op when output_config absent.
|
||||
data: dict = {"max_tokens": 8}
|
||||
_sanitize_vertex_anthropic_output_params(data)
|
||||
sanitize_vertex_anthropic_output_params(data)
|
||||
assert data == {"max_tokens": 8}
|
||||
|
||||
# Effort-only → dropped entirely.
|
||||
data = {"output_config": {"effort": "high"}}
|
||||
_sanitize_vertex_anthropic_output_params(data)
|
||||
sanitize_vertex_anthropic_output_params(data)
|
||||
assert "output_config" not in data
|
||||
|
||||
# Format-only → preserved unchanged.
|
||||
fmt = {"format": {"type": "json_schema", "schema": {"type": "object"}}}
|
||||
data = {"output_config": dict(fmt)}
|
||||
_sanitize_vertex_anthropic_output_params(data)
|
||||
sanitize_vertex_anthropic_output_params(data)
|
||||
assert data["output_config"] == fmt
|
||||
|
||||
# Mixed → effort filtered, format kept.
|
||||
data = {"output_config": {"format": fmt["format"], "effort": "high"}}
|
||||
_sanitize_vertex_anthropic_output_params(data)
|
||||
sanitize_vertex_anthropic_output_params(data)
|
||||
assert data["output_config"] == fmt
|
||||
|
||||
# Non-dict → dropped defensively.
|
||||
data = {"output_config": "garbage"}
|
||||
_sanitize_vertex_anthropic_output_params(data)
|
||||
sanitize_vertex_anthropic_output_params(data)
|
||||
assert "output_config" not in data
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue