Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
import httpx

from litellm.llms.anthropic.chat.transformation import AnthropicConfig
from litellm.llms.bedrock.chat.converse_transformation import (
AmazonConverseConfig,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cross-class use of a private static method creates fragile coupling

AmazonConverseConfig._add_additional_properties_to_schema is a _-prefixed (private) static method on the Converse transformer. Calling it directly from the Invoke transformer creates an implicit dependency between two sibling classes: if the method is ever renamed, moved, or its contract changed, the Invoke path will break silently at runtime rather than at import time.

The cleaner fix is to extract the utility into a shared location (e.g., litellm/llms/bedrock/common_utils.py) so both transformers can import it without either depending on the other.

# In bedrock/common_utils.py
def add_additional_properties_to_schema(schema: dict) -> dict:
    """Recursively ensure all object types have additionalProperties: false."""
    ...

Then update both converse_transformation.py and anthropic_claude3_transformation.py to import from the shared location.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

from litellm.llms.bedrock.chat.invoke_transformations.base_invoke_transformation import (
AmazonInvokeConfig,
)
Expand All @@ -21,6 +24,18 @@
else:
LiteLLMLoggingObj = Any

# Anthropic Claude models that support native structured outputs on Bedrock InvokeModel.
# Maintained separately from the Converse path's BEDROCK_NATIVE_STRUCTURED_OUTPUT_MODELS
# because Invoke and Converse have independent feature rollouts.
# Ref: https://docs.aws.amazon.com/bedrock/latest/userguide/structured-output.html
BEDROCK_INVOKE_NATIVE_STRUCTURED_OUTPUT_MODELS = {
"claude-haiku-4-5",
"claude-sonnet-4-5",
"claude-sonnet-4-6",
"claude-opus-4-5",
"claude-opus-4-6",
}
Comment on lines +29 to +35

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded model list violates project policy

BEDROCK_INVOKE_NATIVE_STRUCTURED_OUTPUT_MODELS is a hardcoded set of model name substrings, which means every time AWS adds a new Claude model that supports native structured outputs on the Invoke API, users must upgrade LiteLLM to get support.

The project convention (also violated by the existing BEDROCK_NATIVE_STRUCTURED_OUTPUT_MODELS in converse_transformation.py) is to store these flags in model_prices_and_context_window.json and read them via get_model_info. This lets users pick up new model support without an SDK upgrade.

The recommended fix is to:

  1. Add a "supports_bedrock_invoke_structured_outputs": true key to each model entry in model_prices_and_context_window.json
  2. Replace _supports_native_structured_outputs with a lookup through get_model_info (similar to how supports_reasoning is used for the reasoning effort feature)
# Instead of:
BEDROCK_INVOKE_NATIVE_STRUCTURED_OUTPUT_MODELS = {
    "claude-haiku-4-5",
    "claude-sonnet-4-5",
    ...
}

# Do something like:
from litellm.utils import get_model_info

def _supports_native_structured_outputs(model: str) -> bool:
    try:
        info = get_model_info(model=model, custom_llm_provider="bedrock")
        return bool(info.get("supports_bedrock_invoke_structured_outputs"))
    except Exception:
        return False

Note: the same issue exists in converse_transformation.py's BEDROCK_NATIVE_STRUCTURED_OUTPUT_MODELS, but that's pre-existing. Fixing it here would be a good opportunity to align with the policy.

Rule Used: What: Do not hardcode model-specific flags in the ... (source)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid point. The model_prices_and_context_window.json approach would be cleaner long-term, but there are a couple of blockers for this PR:

  1. The Invoke path doesn't have its own model entries in the JSON -- there's only one bedrock/invoke/ entry and it's for an old model. The lookup would need to strip the bedrock/invoke/ prefix, handle inference profile IDs (us.anthropic.claude-sonnet-4-6), and fall back to the base Bedrock entry. That model ID resolution logic doesn't exist yet.
  2. The existing Converse path (BEDROCK_NATIVE_STRUCTURED_OUTPUT_MODELS) uses the same hardcoded set pattern.

Happy to follow up with a separate PR to migrate both Invoke and Converse sets to JSON lookups if the maintainers prefer that approach.

Comment on lines +29 to +35

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded model list should live in model_prices_and_context_window.json

Per project convention, model-capability flags should be stored in model_prices_and_context_window.json and read via get_model_info, not hardcoded here. Hardcoding means users must upgrade LiteLLM every time AWS adds a new Claude model to the Invoke native structured-output feature set.

The same pattern exists for the Converse path (BEDROCK_NATIVE_STRUCTURED_OUTPUT_MODELS); both should eventually be migrated. The PR author has already noted this as a follow-up concern in the discussion thread.

Rule Used: What: Do not hardcode model-specific flags in the ... (source)



class AmazonAnthropicClaudeConfig(AmazonInvokeConfig, AnthropicConfig):
"""
Expand Down Expand Up @@ -49,34 +64,49 @@ def custom_llm_provider(self) -> Optional[str]:
def get_supported_openai_params(self, model: str) -> List[str]:
return AnthropicConfig.get_supported_openai_params(self, model)

@staticmethod
def _supports_native_structured_outputs(model: str) -> bool:
"""Check if the Bedrock Invoke model supports native structured outputs."""
return any(substring in model for substring in BEDROCK_INVOKE_NATIVE_STRUCTURED_OUTPUT_MODELS)

def map_openai_params(
self,
non_default_params: dict,
optional_params: dict,
model: str,
drop_params: bool,
) -> dict:
# Force tool-based structured outputs for Bedrock Invoke
# (similar to VertexAI fix in #19201)
# Bedrock Invoke doesn't support output_format parameter
original_model = model
if "response_format" in non_default_params:
# Use a model name that forces tool-based approach
response_format = non_default_params.get("response_format")

# Native path: build output_format directly for Bedrock-supported models
# (includes haiku-4-5 which the Anthropic parent doesn't know about).
if isinstance(response_format, dict) and self._supports_native_structured_outputs(model):
_output_format = self.map_response_format_to_anthropic_output_format(response_format)
if _output_format is not None:
optional_params["output_format"] = _output_format
optional_params["json_mode"] = True
remaining = {k: v for k, v in non_default_params.items() if k != "response_format"}
return AnthropicConfig.map_openai_params(
self,
remaining,
optional_params,
model,
drop_params,
)

# Fallback: force tool-based structured outputs for unsupported models
# (or json_object without schema on a supported model).
if response_format is not None:
model = "claude-3-sonnet-20240229"
Comment on lines +106 to 109

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fallback silently overrides model for tool injection

Setting model = "claude-3-sonnet-20240229" is a local variable override — it tricks the parent's map_openai_params into choosing the tool-based path by selecting an old model name that is known not to support native outputs. This is a subtle and fragile approach.

If the parent's native-supported model set ever changes (e.g., adds "claude-3-sonnet-20240229" to the native list — unlikely but possible), this fallback would break silently. A more explicit approach would be to directly call map_response_format_to_anthropic_tool on the fallback path rather than relying on an opaque model override.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed this is fragile. It's the pre-existing pattern from the code this PR refactored -- the previous implementation also overrode the model name the same way. Calling map_response_format_to_anthropic_tool directly would be cleaner, but that method also handles tool_choice injection and thinking-mode checks that are coupled to the parent's internal state. Extracting just the tool-based path without duplicating logic would require refactoring the parent class, which is out of scope here.

Open to revisiting if the maintainers want to refactor the parent class.


optional_params = AnthropicConfig.map_openai_params(
return AnthropicConfig.map_openai_params(
self,
non_default_params,
optional_params,
model,
drop_params,
)

# Restore original model name
model = original_model

return optional_params

def transform_request(
self,
model: str,
Expand All @@ -87,11 +117,7 @@ def transform_request(
) -> dict:
# Filter out AWS authentication parameters before passing to Anthropic transformation
# AWS params should only be used for signing requests, not included in request body
filtered_params = {
k: v
for k, v in optional_params.items()
if k not in self.aws_authentication_params
}
filtered_params = {k: v for k, v in optional_params.items() if k not in self.aws_authentication_params}
filtered_params = self._normalize_bedrock_tool_search_tools(filtered_params)

_anthropic_request = AnthropicConfig.transform_request(
Expand All @@ -105,11 +131,26 @@ def transform_request(

_anthropic_request.pop("model", None)
_anthropic_request.pop("stream", None)
# Bedrock Invoke doesn't support output_format parameter
_anthropic_request.pop("output_format", None)
# Bedrock Invoke doesn't support output_config parameter
# Fixes: https://github.com/BerriAI/litellm/issues/22797
_anthropic_request.pop("output_config", None)

# Convert Anthropic output_format to Bedrock InvokeModel output_config.format
output_format = _anthropic_request.pop("output_format", None)
if output_format and isinstance(output_format, dict) and output_format.get("type") == "json_schema":
schema = output_format.get("schema", {})
normalized_schema = AmazonConverseConfig._add_additional_properties_to_schema(schema)
# Preserve existing output_config keys (e.g. effort from reasoning_effort)
output_config = _anthropic_request.get("output_config") or {}
output_config["format"] = {
"type": "json_schema",
"schema": normalized_schema,
}
_anthropic_request["output_config"] = output_config
else:
# Non-native path: strip output_config entirely.
# Bedrock Invoke rejects the key itself (not just sub-keys) with
# "extraneous key [output_config] is not permitted" for models
# that don't support native structured outputs.
# Fixes: https://github.com/BerriAI/litellm/issues/22797
_anthropic_request.pop("output_config", None)
Comment on lines +164 to +170

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

output_config stripped unconditionally on non-native path

The else branch removes output_config from the request regardless of what put it there. This means that if a user passes both reasoning_effort (which sets output_config.effort) and a response_format on a model that does not support native structured outputs, the output_config (with the effort key) will be silently dropped.

Concretely, if Bedrock ever supports output_config.effort on the Invoke API, this else-branch will silently discard it for any mixed-mode request. Even today, if AnthropicConfig.transform_request populates output_config from other optional params, it would be wiped here.

The if branch already handles the merge case correctly (it calls _anthropic_request.get("output_config") or {} and merges format in), so the else branch should at minimum avoid stripping keys it didn't set. Consider only removing the format key (or the whole output_config only when it came from an output_format source):

else:
    # Non-native path: remove only the format key that we never populated.
    # Leave any other output_config keys (e.g. effort) intact.
    output_config = _anthropic_request.get("output_config")
    if output_config and "format" in output_config:
        output_config.pop("format")
        if not output_config:
            _anthropic_request.pop("output_config", None)

If the intent is truly that Bedrock Invoke does not support output_config at all (as fixed in #22797), then at least add a comment explaining that any future output_config keys (like effort) should be handled here explicitly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggestion to only strip the format key won't work here. Bedrock Invoke rejects the output_config key itself for unsupported models -- not just sub-keys inside it. The error from #22797 is: extraneous key [output_config] is not permitted. So any output_config content (including effort) would cause a 400 on unsupported models.

Added a comment explaining this.

if "anthropic_version" not in _anthropic_request:
_anthropic_request["anthropic_version"] = self.anthropic_version

Expand All @@ -135,9 +176,7 @@ def transform_request(
)
beta_set.update(auto_betas)

if tool_search_used and not (
programmatic_tool_calling_used or input_examples_used
):
if tool_search_used and not (programmatic_tool_calling_used or input_examples_used):
beta_set.discard(ANTHROPIC_TOOL_SEARCH_BETA_HEADER)
if "opus-4" in model.lower() or "opus_4" in model.lower():
beta_set.add("tool-search-tool-2025-10-19")
Expand Down Expand Up @@ -166,9 +205,7 @@ def _normalize_bedrock_tool_search_tools(self, optional_params: dict) -> dict:
if tool_type == "tool_search_tool_regex_20251119":
normalized_tool = tool.copy()
normalized_tool["type"] = "tool_search_tool_regex"
normalized_tool["name"] = normalized_tool.get(
"name", "tool_search_tool_regex"
)
normalized_tool["name"] = normalized_tool.get("name", "tool_search_tool_regex")
normalized_tools.append(normalized_tool)
continue
normalized_tools.append(tool)
Expand Down
Loading
Loading