Skip to content

Commit 962dc7f

Browse files
pseudotensorclaude
andcommitted
test(bedrock): repair a vacuous test, drop a dead condition, pin the hardening
Round 3 of review. The production logic held up -- 6 of 7 mutations killed, no breakage across the full bedrock directory (1089 passed; the 5 errors are a missing pytest-mock in this env and reproduce at base) -- but one round-2 test asserted nothing. test_response_format_injected_tools_still_honour_parallel_tool_calls picked "anthropic.claude-sonnet-4-5-20250929-v1:0", which advertises supports_native_structured_output, so _translate_response_format_param takes the outputConfig branch and injects NO synthetic tool. optional_params had no tools, the list was empty, and the whole assertion sat behind `if listed:` and never ran -- it passed with the round-2 fix reverted, which made it the only coverage for half that fix and worthless. Of the 59 models carrying supports_parallel_tool_use_config exactly one lacks native structured output ("claude-sonnet-4-5-20250929-v1:0", bare id), so the test now uses that and asserts the exact payload unconditionally: toolConfig.tools = [json_tool_call] additionalModelRequestFields.tool_choice = {"type": "tool", "name": "json_tool_call", "disable_parallel_tool_use": True} Verified it now dies under the mutation it exists to catch. Dropped `and "type" in passthrough_tool_choice` from _apply_parallel_tool_use_config: the builder sets type on all three branches, so it could never be false -- the same reasoning round 1 used to delete the unreachable merge branch two lines above. Softened the guard comment, which claimed more than the guard delivers. A non-empty optional_params["tools"] does not imply a non-empty toolConfig.tools: _bedrock_tools_pt later drops tools carrying neither "function" nor "input_schema" (the Responses built-ins, e.g. web_search), so a request whose tools are all of that kind still reaches the provider with a tool_choice and no toolConfig. Measured, and identical before this feature, so it is not a regression -- but the comment should not assert the payload is prevented. Added the one test that pins the non-dict native["tool"] hardening, which no mutation was catching. Confirmed by review that the tool_choice="none" early return is in the right place: Anthropic's ToolChoiceNone carries only `type`, and disable_parallel_tool_use exists on auto/any/tool only -- so emitting the bare flag is the "missing field type" 400 this PR fixes, and {"type":"none", disable_parallel_tool_use} would be rejected as an extra input. Emitting nothing is the only correct option, which is also what litellm's own Anthropic transform does. 174 pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7105fb9 commit 962dc7f

2 files changed

Lines changed: 46 additions & 11 deletions

File tree

litellm/llms/bedrock/chat/converse_transformation.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,10 @@ def _apply_parallel_tool_use_config(
123123
# hybrid such as {"type": "auto", "name": <user>, "disable_parallel_tool_use": true}.
124124
additional_request_params.update(parallel_tool_use_config)
125125

126-
passthrough_tool_choice = parallel_tool_use_config.get("tool_choice")
127-
if isinstance(passthrough_tool_choice, dict) and "type" in passthrough_tool_choice:
126+
# No `"type" in ...` check: _map_parallel_tool_use_config sets type on every
127+
# branch, so that condition could never be false. (Same reasoning that removed
128+
# the unreachable merge branch above.)
129+
if isinstance(parallel_tool_use_config.get("tool_choice"), dict):
128130
inference_params.pop("tool_choice", None)
129131

130132

@@ -1014,10 +1016,17 @@ def _map_parallel_tool_use_config(
10141016
# A truthy non-list ``tools`` (e.g. a bare dict) passes a
10151017
# non_default_params check but is skipped by the mapping loop's
10161018
# isinstance(value, list) guard, so the payload came out with
1017-
# additionalModelRequestFields.tool_choice and NO toolConfig at all --
1018-
# the exact 400 this guard exists to prevent. In the other direction a
1019-
# json_schema response_format injects a synthetic tool into
1020-
# optional_params with nothing in non_default_params.
1019+
# additionalModelRequestFields.tool_choice and NO toolConfig at all. In the
1020+
# other direction a json_schema response_format injects a synthetic tool
1021+
# into optional_params with nothing in non_default_params.
1022+
#
1023+
# This narrows that payload but does not eliminate it: _bedrock_tools_pt
1024+
# later drops tools carrying neither "function" nor "input_schema" (the
1025+
# Responses built-ins such as web_search), so a request whose tools are ALL
1026+
# of that kind still reaches the provider with a tool_choice and no
1027+
# toolConfig. Measured, and unchanged from before this feature -- the only
1028+
# place the answer is known is _transform_request_helper, where
1029+
# bedrock_tools has already been computed.
10211030
if not optional_params.get("tools"):
10221031
return
10231032

tests/test_litellm/llms/bedrock/chat/test_converse_transformation.py

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6107,8 +6107,15 @@ def test_response_format_injected_tools_still_honour_parallel_tool_calls():
61076107
with nothing in non_default_params, so reading the raw value silently ignored
61086108
parallel_tool_calls for it.
61096109
"""
6110+
# The bare id, NOT "anthropic.<id>": of the 59 models carrying
6111+
# supports_parallel_tool_use_config this is the only one WITHOUT
6112+
# supports_native_structured_output, so it is the only one that takes the
6113+
# synthetic-tool branch instead of outputConfig. Picking any other model makes
6114+
# this test vacuous -- no tool is injected, `toolConfig.tools` is empty, and a
6115+
# conditional assertion never runs. (An earlier version of this test did
6116+
# exactly that and passed with the fix reverted.)
61106117
data = _converse_request(
6111-
"anthropic.claude-sonnet-4-5-20250929-v1:0",
6118+
"claude-sonnet-4-5-20250929-v1:0",
61126119
{
61136120
"response_format": {
61146121
"type": "json_schema",
@@ -6120,10 +6127,14 @@ def test_response_format_injected_tools_still_honour_parallel_tool_calls():
61206127
"parallel_tool_calls": False,
61216128
},
61226129
)
6123-
listed = (data.get("toolConfig") or {}).get("tools") or []
6124-
if listed: # only meaningful if litellm injected the synthetic tool
6125-
passthrough = data["additionalModelRequestFields"]["tool_choice"]
6126-
assert passthrough["disable_parallel_tool_use"] is True
6130+
listed = [t["toolSpec"]["name"] for t in (data.get("toolConfig") or {}).get("tools") or []]
6131+
assert listed == ["json_tool_call"], f"expected the synthetic tool, got {listed}"
6132+
passthrough = data["additionalModelRequestFields"]["tool_choice"]
6133+
assert passthrough == {
6134+
"type": "tool",
6135+
"name": "json_tool_call",
6136+
"disable_parallel_tool_use": True,
6137+
}, passthrough
61276138

61286139

61296140
def test_tool_choice_none_is_not_reinvented_as_auto():
@@ -6143,3 +6154,18 @@ def test_tool_choice_none_is_not_reinvented_as_auto():
61436154
drop_params=True,
61446155
)
61456156
assert "tool_choice" not in data.get("additionalModelRequestFields", {})
6157+
6158+
6159+
def test_a_malformed_native_tool_choice_does_not_raise():
6160+
"""Defensive: unreachable through get_optional_params (every writer of
6161+
optional_params["tool_choice"] produces a well-formed block), but the builder
6162+
reads native["tool"].get("name") and would raise AttributeError on a string."""
6163+
config = AmazonConverseConfig()
6164+
optional_params = {"tools": _TOOL_PARAM, "tool_choice": {"tool": "get_weather"}}
6165+
config._map_parallel_tool_use_config(
6166+
non_default_params={"tools": _TOOL_PARAM, "parallel_tool_calls": False},
6167+
optional_params=optional_params,
6168+
)
6169+
tool_choice = optional_params["_parallel_tool_use_config"]["tool_choice"]
6170+
assert tool_choice["type"] == "tool"
6171+
assert "name" not in tool_choice # nothing usable to forward

0 commit comments

Comments
 (0)