Repository navigation
fix(bedrock): backport #41870 and the GPT-6 reasoning gate fix to stable/1.100.x for v1.100.2 #42000
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(bedrock): backport #41870 and the GPT-6 reasoning gate fix to stable/1.100.x for v1.100.2 #42000
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
|
|
||
| import copy | ||
| import json | ||
| import re | ||
| import time | ||
| import types | ||
| from collections.abc import Mapping | ||
|
|
@@ -104,6 +105,7 @@ | |
| "bash_", | ||
| "text_editor_", | ||
| ] | ||
| BEDROCK_OPENAI_COMPAT_MIN_MAX_TOKENS: Final = 16 | ||
|
|
||
| # Beta header patterns that are not supported by Bedrock Converse API | ||
| # These will be filtered out to prevent errors | ||
|
|
@@ -292,6 +294,14 @@ def _validate_request_metadata(self, metadata: dict) -> None: | |
| llm_provider="bedrock", | ||
| ) | ||
|
|
||
| @staticmethod | ||
| def _requires_min_max_tokens(model: str) -> bool: | ||
| return re.search(r"openai\.gpt-\d|xai\.grok-", model) is not None | ||
|
|
||
| @staticmethod | ||
| def _is_openai_gpt_reasoning_model(model: str) -> bool: | ||
| return re.search(r"openai\.gpt-\d", model) is not None | ||
|
|
||
|
Comment on lines
+303
to
+304
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Rule Used: What: Do not hardcode model-specific flags in the codebase. Instead, put them in model_prices_and_context_window.json and then read them in via get_model_info Why: Prevents need for users to upgrade litellm each time a new model supports this featu... (source) 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!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This mirrors main's merged #31884. A stable backport should not diverge from main, and the family pattern covers new GPT releases without a cost-map entry
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That’s fair. For this PR, matching main’s merged #31884 behavior is the correct backport constraint, and the family-level match is intentionally forward-compatible with future |
||
| def _is_nova_2_model(self, model: str) -> bool: | ||
| """ | ||
| Check if the model is a Nova 2 model that supports reasoningConfig. | ||
|
|
@@ -422,14 +432,14 @@ def _handle_reasoning_effort_parameter(self, model: str, reasoning_effort: str, | |
| Handle the reasoning_effort parameter based on the model type. | ||
|
|
||
| - GPT-OSS models: passed through unchanged via additionalModelRequestFields. | ||
| - OpenAI GPT-5.x models: mapped to ``reasoning.effort`` via additionalModelRequestFields. | ||
| - OpenAI GPT-5.x and GPT-6 models: mapped to ``reasoning.effort`` via additionalModelRequestFields. | ||
| - Nova 2 models: transformed to reasoningConfig. | ||
| - Anthropic models: mapped to ``thinking`` (and ``output_config.effort`` on | ||
| adaptive Claude 4.6 / 4.7). | ||
| """ | ||
| if "gpt-oss" in model: | ||
| optional_params["reasoning_effort"] = reasoning_effort | ||
| elif "openai.gpt-5" in model: | ||
| elif self._is_openai_gpt_reasoning_model(model): | ||
| reasoning: Final[BedrockConverseGptReasoningEffortBlock] = {"effort": reasoning_effort} | ||
| optional_params["reasoning"] = reasoning | ||
| elif self._is_nova_2_model(model): | ||
|
|
@@ -563,7 +573,11 @@ def get_supported_openai_params(self, model: str) -> list[str]: | |
| # only anthropic and mistral support tool choice config. otherwise (E.g. cohere) will fail the call - https://docs.aws.amazon.com/bedrock/latest/APIReference/API_runtime_ToolChoice.html | ||
| supported_params.append("tool_choice") | ||
|
|
||
| if "gpt-oss" in model or "openai.gpt-5" in model or "openai.gpt-5" in base_model: | ||
| if ( | ||
| "gpt-oss" in model | ||
| or self._is_openai_gpt_reasoning_model(model) | ||
| or self._is_openai_gpt_reasoning_model(base_model) | ||
| ): | ||
| supported_params.append("reasoning_effort") | ||
| elif self._is_nova_2_model(model): | ||
| # Nova 2 models support reasoning_effort (transformed to reasoningConfig) | ||
|
|
@@ -874,7 +888,11 @@ def map_openai_params( | |
| is_thinking_enabled=is_thinking_enabled, | ||
| ) | ||
| if param == "max_tokens" or param == "max_completion_tokens": | ||
| optional_params["maxTokens"] = value | ||
| optional_params["maxTokens"] = ( | ||
| max(value, BEDROCK_OPENAI_COMPAT_MIN_MAX_TOKENS) | ||
| if isinstance(value, int) and self._requires_min_max_tokens(model) | ||
| else value | ||
| ) | ||
| if param == "stream": | ||
| optional_params["stream"] = value | ||
| if param == "stop": | ||
|
|
@@ -911,7 +929,7 @@ def map_openai_params( | |
| optional_params["_parallel_tool_use_config"] = { | ||
| "tool_choice": {"type": "auto", "disable_parallel_tool_use": not value} | ||
| } | ||
| if param == "thinking" and "openai.gpt-5" not in model: | ||
| if param == "thinking" and not self._is_openai_gpt_reasoning_model(model): | ||
| if ( | ||
| isinstance(value, dict) | ||
| and value.get("type") == "adaptive" | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded matching violates the directive to store model capabilities in metadata and misses opaque profiles, so GPT/Grok requests below 16 are rejected upstream
Rule Used: What: Do not hardcode model-specific flags in the codebase. Instead, put them in model_prices_and_context_window.json and then read them in via get_model_info Why: Prevents need for users to upgrade litellm each time a new model supports this featu... (source)
Knowledge Base Used: Provider adapters and capabilities
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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backport of #41870, where this was withdrawn. The regex clamps ARNs embedding the model id; opaque application profiles carry none, so get_model_info cannot resolve them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. In this backport, matching the model string is intentional: it covers inference-profile ARNs that embed
openai.gpt-*orxai.grok-*, while opaque application-inference-profile ARNs contain no resolvable model identifier forget_model_infoto use. Given that limitation and the behavior established by #41870, this comment does not apply. No change needed.