mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-30 06:43:27 +00:00
fix(responses): only merge repeated Codex tools when drop_params is on (#49112)
Without drop_params, every copy of a tool from additional_tools input items is forwarded as sent, so a provider that rejects duplicate tools returns its own 400. Codex keeps the merge because the proxy turns drop_params on for it.
This commit is contained in:
parent
62b8ee78de
commit
d884167bf2
7 changed files with 68 additions and 17 deletions
|
|
@ -22,6 +22,7 @@ import httpx
|
|||
|
||||
import litellm
|
||||
from litellm._logging import verbose_logger
|
||||
from litellm.litellm_core_utils.core_helpers import drops_unsupported_params
|
||||
from litellm.litellm_core_utils.url_utils import join_api_base
|
||||
from litellm.llms.base_llm.chat.transformation import BaseLLMException
|
||||
from litellm.llms.base_llm.responses.codex_compat import drop_unsupported_tools, normalize_codex_input_items
|
||||
|
|
@ -196,7 +197,9 @@ class BedrockMantleResponsesAPIConfig(BedrockMantleAuthMixin, OpenAIResponsesAPI
|
|||
params: Final = cast( # cast-ok: the base signature leaves the params dict untyped
|
||||
"ResponsesAPIOptionalRequestParams", response_api_optional_request_params
|
||||
)
|
||||
hoisted: Final = hoist_additional_tools(input, params.get("tools"))
|
||||
hoisted: Final = hoist_additional_tools(
|
||||
input, params.get("tools"), drop_params=drops_unsupported_params(litellm_params.drop_params)
|
||||
)
|
||||
normalized_input, rewritten_types = normalize_codex_input_items(hoisted.input)
|
||||
if rewritten_types:
|
||||
verbose_logger.warning(
|
||||
|
|
|
|||
|
|
@ -89,6 +89,8 @@ def _unique_tools(tools: Sequence[Mapping[str, object]]) -> tuple[Mapping[str, o
|
|||
def hoist_additional_tools(
|
||||
input: str | ResponseInputParam,
|
||||
tools: Sequence[ALL_RESPONSES_API_TOOL_PARAMS] | None,
|
||||
*,
|
||||
drop_params: bool,
|
||||
) -> HoistedAdditionalTools:
|
||||
existing: Final = tuple(tools or ())
|
||||
if isinstance(input, str):
|
||||
|
|
@ -103,6 +105,8 @@ def hoist_additional_tools(
|
|||
len(items),
|
||||
)
|
||||
remaining_input: Final = [item for item in input if not _is_additional_tools_item(item)]
|
||||
if not drop_params:
|
||||
return HoistedAdditionalTools(input=remaining_input, tools=(*existing, *hoisted), hoisted=hoisted)
|
||||
unique: Final = tuple(
|
||||
cast("ALL_RESPONSES_API_TOOL_PARAMS", tool) # cast-ok: a merged namespace is rebuilt from the same tool dicts
|
||||
for tool in _unique_tools((*existing, *hoisted))
|
||||
|
|
|
|||
|
|
@ -114,7 +114,7 @@ def with_provider_file_ids(
|
|||
}
|
||||
|
||||
|
||||
def _with_hoisted_additional_tools(responses_request: Mapping[str, object]) -> Mapping[str, object]:
|
||||
def _with_hoisted_additional_tools(responses_request: Mapping[str, object], drop_params: bool) -> Mapping[str, object]:
|
||||
responses_input: Final = responses_request.get("input")
|
||||
if not isinstance(responses_input, list):
|
||||
return responses_request
|
||||
|
|
@ -122,7 +122,7 @@ def _with_hoisted_additional_tools(responses_request: Mapping[str, object]) -> M
|
|||
top_level_tools: Final = cast( # cast-ok: tools are forwarded as sent, as inference does
|
||||
"tuple[ALL_RESPONSES_API_TOOL_PARAMS, ...]", _ITEMS.validate_python(responses_request.get("tools") or ())
|
||||
)
|
||||
hoisted: Final = hoist_additional_tools(input_items, top_level_tools)
|
||||
hoisted: Final = hoist_additional_tools(input_items, top_level_tools, drop_params=drop_params)
|
||||
if not hoisted.hoisted:
|
||||
return responses_request
|
||||
return {**responses_request, "input": hoisted.input, "tools": list(hoisted.tools)}
|
||||
|
|
@ -196,7 +196,7 @@ async def chat_count_request(
|
|||
chat_body: Final = await _with_previous_turns(
|
||||
responses_batch_body_to_chat_body(
|
||||
_with_compaction_as_summary_text(
|
||||
_with_emulated_file_search(_with_hoisted_additional_tools(responses_request))
|
||||
_with_emulated_file_search(_with_hoisted_additional_tools(responses_request, policy.drop_params))
|
||||
),
|
||||
custom_llm_provider,
|
||||
policy.drop_params,
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ from typing import Final
|
|||
from pydantic import TypeAdapter
|
||||
|
||||
import litellm
|
||||
from litellm.litellm_core_utils.core_helpers import drops_unsupported_params
|
||||
from litellm.responses.additional_tools import HoistedAdditionalTools, hoist_additional_tools
|
||||
from litellm.responses.litellm_completion_transformation.client_tool_search import bridge_client_tool_search
|
||||
from litellm.responses.litellm_completion_transformation.compaction_items import holds_compaction_summary
|
||||
|
|
@ -73,7 +74,11 @@ class LiteLLMCompletionTransformationHandler:
|
|||
| BaseResponsesAPIStreamingIterator
|
||||
| Coroutine[object, object, ResponsesAPIResponse | BaseResponsesAPIStreamingIterator]
|
||||
):
|
||||
hoisted: Final = hoist_additional_tools(input, responses_api_request.get("tools"))
|
||||
hoisted: Final = hoist_additional_tools(
|
||||
input,
|
||||
responses_api_request.get("tools"),
|
||||
drop_params=drops_unsupported_params(_REQUEST_KWARGS_ADAPTER.validate_python(kwargs).get("drop_params")),
|
||||
)
|
||||
previous_response_id: Final = responses_api_request.get("previous_response_id")
|
||||
if _is_async and previous_response_id and has_client_tool_search(hoisted.tools):
|
||||
return self._async_response_api_handler_after_earlier_searches(
|
||||
|
|
|
|||
|
|
@ -70,7 +70,9 @@ def bedrock_converse_peer(request: Request) -> Reply:
|
|||
)
|
||||
|
||||
|
||||
def send_to_bedrock_converse(gateway: Gateway, history: list[JsonValue]) -> tuple[httpx.Response, tuple[Request, ...]]:
|
||||
def send_to_bedrock_converse(
|
||||
gateway: Gateway, history: list[JsonValue], drop_params: bool = True
|
||||
) -> tuple[httpx.Response, tuple[Request, ...]]:
|
||||
with wire_server(bedrock_converse_peer) as wire, gateway.scenario() as scenario:
|
||||
model: Final = scenario.model(
|
||||
model=f"bedrock/converse/{BEDROCK_GPT}",
|
||||
|
|
@ -79,7 +81,15 @@ def send_to_bedrock_converse(gateway: Gateway, history: list[JsonValue]) -> tupl
|
|||
aws_bedrock_runtime_endpoint=wire.url,
|
||||
)
|
||||
response: Final = gateway.request(
|
||||
"POST", "/v1/responses", {"model": model, "input": history, "store": False, "cache": {"no-cache": True}}
|
||||
"POST",
|
||||
"/v1/responses",
|
||||
{
|
||||
"model": model,
|
||||
"input": history,
|
||||
"store": False,
|
||||
"cache": {"no-cache": True},
|
||||
"drop_params": drop_params,
|
||||
},
|
||||
)
|
||||
return response, wire.drain()
|
||||
|
||||
|
|
@ -113,6 +123,20 @@ def test_codex_tool_list_repeated_on_every_turn_reaches_bedrock_converse_once(ga
|
|||
assert sorted(names) == ["collaboration__followup_task", "exec_command"], names
|
||||
|
||||
|
||||
def test_codex_tool_list_repeated_on_every_turn_reaches_bedrock_converse_as_sent_without_drop_params(
|
||||
gateway: Gateway,
|
||||
) -> None:
|
||||
marker: Final = uuid.uuid4().hex
|
||||
tools: Final = tool_list_item(function_tool("exec_command"))
|
||||
history: Final[list[JsonValue]] = [tools, user_turn(f"first {marker}"), assistant_turn("ok"), tools]
|
||||
|
||||
response, forwarded = send_to_bedrock_converse(gateway, history, drop_params=False)
|
||||
|
||||
assert response.status_code == 400, response.text
|
||||
assert "already defined" in response.text, response.text
|
||||
assert [str(spec["name"]) for spec in converse_tool_specs(forwarded[0])] == ["exec_command", "exec_command"]
|
||||
|
||||
|
||||
def test_newest_copy_of_a_repeated_codex_tool_is_what_bedrock_converse_receives(gateway: Gateway) -> None:
|
||||
marker: Final = uuid.uuid4().hex
|
||||
history: Final[list[JsonValue]] = [
|
||||
|
|
@ -195,6 +219,7 @@ def test_codex_tool_list_repeated_on_every_turn_reaches_bedrock_mantle_once(gate
|
|||
user_turn(f"second {marker}"),
|
||||
],
|
||||
"store": False,
|
||||
"drop_params": True,
|
||||
},
|
||||
)
|
||||
forwarded: Final = wire.drain()
|
||||
|
|
|
|||
|
|
@ -95,8 +95,8 @@ async def test_claude_count_request_counts_additional_tools_input_items_as_tools
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_claude_count_request_counts_a_tool_list_repeated_on_every_turn_once() -> None:
|
||||
policy: Final = DropPolicy(drop_params=False, additional_drop_params=frozenset())
|
||||
async def test_claude_count_request_counts_a_tool_list_repeated_on_every_turn_once_under_drop_params() -> None:
|
||||
policy: Final = DropPolicy(drop_params=True, additional_drop_params=frozenset())
|
||||
tool_list: Final[dict[str, JsonValue]] = {"type": "additional_tools", "tools": [_WEATHER_TOOL]}
|
||||
|
||||
repeated: Final = await _claude_count(
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ _USER_MESSAGE = {"type": "message", "role": "user", "content": "Run ls"}
|
|||
|
||||
|
||||
def test_string_input_passes_through_with_existing_tools():
|
||||
hoisted = hoist_additional_tools("hello", [_TOP_LEVEL_TOOL])
|
||||
hoisted = hoist_additional_tools("hello", [_TOP_LEVEL_TOOL], drop_params=False)
|
||||
|
||||
assert hoisted.input == "hello"
|
||||
assert hoisted.tools == (_TOP_LEVEL_TOOL,)
|
||||
|
|
@ -25,7 +25,7 @@ def test_string_input_passes_through_with_existing_tools():
|
|||
def test_input_without_additional_tools_items_is_returned_untouched():
|
||||
request_input = [_USER_MESSAGE]
|
||||
|
||||
hoisted = hoist_additional_tools(request_input, None)
|
||||
hoisted = hoist_additional_tools(request_input, None, drop_params=False)
|
||||
|
||||
assert hoisted.input is request_input
|
||||
assert hoisted.tools == ()
|
||||
|
|
@ -39,7 +39,7 @@ def test_additional_tools_items_are_stripped_and_appended_after_top_level_tools_
|
|||
{"type": "additional_tools", "id": "at_2", "role": "developer", "tools": [_WAIT_TOOL]},
|
||||
]
|
||||
|
||||
hoisted = hoist_additional_tools(request_input, [_TOP_LEVEL_TOOL])
|
||||
hoisted = hoist_additional_tools(request_input, [_TOP_LEVEL_TOOL], drop_params=False)
|
||||
|
||||
assert hoisted.input == [_USER_MESSAGE]
|
||||
assert hoisted.tools == (_TOP_LEVEL_TOOL, _EXEC_TOOL, _WAIT_TOOL)
|
||||
|
|
@ -49,7 +49,7 @@ def test_additional_tools_items_are_stripped_and_appended_after_top_level_tools_
|
|||
def test_additional_tools_item_without_a_tools_list_is_stripped_and_contributes_nothing():
|
||||
request_input = [{"type": "additional_tools", "id": "at_1", "role": "developer", "tools": "exec"}, _USER_MESSAGE]
|
||||
|
||||
hoisted = hoist_additional_tools(request_input, None)
|
||||
hoisted = hoist_additional_tools(request_input, None, drop_params=False)
|
||||
|
||||
assert hoisted.input == [_USER_MESSAGE]
|
||||
assert hoisted.tools == ()
|
||||
|
|
@ -77,7 +77,7 @@ def test_tool_list_repeated_on_every_turn_keeps_each_tool_once_with_the_newest_d
|
|||
_USER_MESSAGE,
|
||||
]
|
||||
|
||||
hoisted: Final = hoist_additional_tools(request_input, None)
|
||||
hoisted: Final = hoist_additional_tools(request_input, None, drop_params=True)
|
||||
|
||||
assert hoisted.tools == (collaboration, _function("exec", "new"))
|
||||
|
||||
|
|
@ -89,7 +89,7 @@ def test_namespace_sent_in_several_turns_keeps_every_distinct_member_once() -> N
|
|||
_tool_list(_namespace("mcp__docs__", _function("fetch", "new"), _function("list"))),
|
||||
]
|
||||
|
||||
hoisted: Final = hoist_additional_tools(request_input, None)
|
||||
hoisted: Final = hoist_additional_tools(request_input, None, drop_params=True)
|
||||
|
||||
assert hoisted.tools == (
|
||||
_namespace("mcp__docs__", _function("search"), _function("fetch", "new"), _function("list")),
|
||||
|
|
@ -109,7 +109,7 @@ def test_unnamed_tools_collapse_only_when_identical() -> None:
|
|||
}
|
||||
request_input: Final = [_tool_list(docs_server, wiki_server), _USER_MESSAGE, _tool_list(docs_server)]
|
||||
|
||||
hoisted: Final = hoist_additional_tools(request_input, None)
|
||||
hoisted: Final = hoist_additional_tools(request_input, None, drop_params=True)
|
||||
|
||||
assert hoisted.tools == (docs_server, wiki_server)
|
||||
|
||||
|
|
@ -117,6 +117,20 @@ def test_unnamed_tools_collapse_only_when_identical() -> None:
|
|||
def test_top_level_tool_repeated_in_a_tool_list_item_is_kept_once() -> None:
|
||||
request_input: Final = [_tool_list(_TOP_LEVEL_TOOL, _WAIT_TOOL), _USER_MESSAGE]
|
||||
|
||||
hoisted: Final = hoist_additional_tools(request_input, [_TOP_LEVEL_TOOL])
|
||||
hoisted: Final = hoist_additional_tools(request_input, [_TOP_LEVEL_TOOL], drop_params=True)
|
||||
|
||||
assert hoisted.tools == (_TOP_LEVEL_TOOL, _WAIT_TOOL)
|
||||
|
||||
|
||||
def test_tool_list_repeated_on_every_turn_is_sent_as_is_without_drop_params() -> None:
|
||||
request_input: Final = [
|
||||
_tool_list(_function("exec", "old")),
|
||||
_USER_MESSAGE,
|
||||
_tool_list(_function("exec", "new")),
|
||||
_USER_MESSAGE,
|
||||
]
|
||||
|
||||
hoisted: Final = hoist_additional_tools(request_input, [_TOP_LEVEL_TOOL], drop_params=False)
|
||||
|
||||
assert hoisted.tools == (_TOP_LEVEL_TOOL, _function("exec", "old"), _function("exec", "new"))
|
||||
assert hoisted.input == [_USER_MESSAGE, _USER_MESSAGE]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue