fix(bedrock): accept tool_choice="any" on the Converse adapter - #38047
fix(bedrock): accept tool_choice="any" on the Converse adapter#38047likalight wants to merge 1 commit into
Conversation
`map_tool_choice_values` mapped "required" to Bedrock's native
`{any: {}}` but rejected the literal string "any" with
`UnsupportedParamsError`, so the one name Bedrock itself uses for "call
some tool" was the one name the adapter refused.
Map "any" alongside "required" and list it in the error message, which is
the only place these values are discoverable.
Fixes BerriAI#37980
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR makes the Bedrock Converse adapter accept
Confidence Score: 4/5The PR appears safe to merge after optionally removing the redundant source comments. The new literal follows the existing "required" mapping through the Converse request transformation, and the only accepted concern is non-blocking comment cleanup. Files Needing Attention: litellm/llms/bedrock/chat/converse_transformation.py; tests/test_litellm/llms/bedrock/chat/test_bedrock_tool_choice_any.py
|
| Filename | Overview |
|---|---|
| litellm/llms/bedrock/chat/converse_transformation.py | Correctly adds the native "any" alias and updates validation messaging, with only redundant explanatory comments needing cleanup. |
| tests/test_litellm/llms/bedrock/chat/test_bedrock_tool_choice_any.py | Adds focused regression tests covering the changed mapping and adjacent behavior; one redundant assertion comment follows the same minor pattern. |
Reviews (1): Last reviewed commit: "fix(bedrock): accept tool_choice="any" o..." | Re-trigger Greptile
| # Bedrock's native value for "call some tool" is literally `any`, | ||
| # so accepting "required" while rejecting "any" was incoherent. |
There was a problem hiding this comment.
Remove redundant mapping comments
The comments here and beside the error-message assertion in the new test restate straightforward code rather than explaining complex behavior, adding maintenance noise that can drift from the implementation.
Context Used: CLAUDE.md (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!
Fixes #37980.
The bug
AmazonConverseConfig.map_tool_choice_valuesalready maps"required"ontoBedrock's native
{any: {}}:but the literal string
"any"falls through to the terminalelseand raises:So the one name Bedrock itself uses for "call some tool" is the one name the
adapter refuses, while
"required"— which silently becomes exactly that value— is accepted. Callers coming from Anthropic's API, where
anyis the spelling,hit this immediately.
The fix
Map
"any"alongside"required", as suggested in the issue, and add it to theerror message — that string is the only place these accepted values are
discoverable at runtime.
Verification
Six tests in
tests/test_litellm/llms/bedrock/chat/test_bedrock_tool_choice_any.py:"required"and"any"both map to{"any": {}}(parametrized, so the twoare pinned as equivalent rather than merely both working)
"auto"still maps to{"auto": {}}{"tool": {"name": ...}}'any'"none"withdrop_params=Truestill returnsNoneWithout the source change the parametrized
"any"case fails with the issue'sexact error, so the test is confirmed to be reacting to this and nothing else.
tests/test_litellm/llms/bedrock/chat/: 378/378 passruff format --checkclean;ruff checkon the touched source file reportsthe same 13 pre-existing findings as the base commit, none added
LIT002 92, LIT006 5, LIT010 81, LIT011 43), so no budget delta
On the red
code-qualitycheck: it is not from this PR. It fails on everyPR against
litellm_internal_stagingright now, including ones that touch noworkflow files, because three unit shards in
.github/workflows/test-unit.ymlcap the job below the startup-safety invariant. Fixed independently in #38046;
this PR needs no change for it.