From e7f3f58f961197e3285a4f956dbfba62c6078030 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Mon, 21 Sep 2026 20:06:01 -0700 Subject: [PATCH] refactor(agentic-loop): build follow-up kwargs in one place so no executor can repeat a request param (#42307) * refactor(agentic-loop): build follow-up kwargs in one place so no executor can repeat a request param The Responses and both chat completions follow-up executors each rebuilt the follow-up kwargs by hand and then expanded them next to the request params, so a plan whose kwargs repeated a request param raised a duplicate keyword TypeError. They now share build_agentic_followup_kwargs, which drops any key already sent as a request param (and the explicitly passed model/input/messages) from both the request kwargs and the plan kwargs. Each executor keeps its own internal-key filter unchanged, and the /v1/messages executor is untouched because it merges into a single dict and cannot hit this. * test(agentic-loop): move follow-up regressions into their mapped test files Greptile review: the executor regressions belong in test_llm_http_handler.py and test_chat_completion_agentic_loop.py rather than a split-off file, and the builder test helper returned a read-only mapping while promising a dict. The Responses overlap test is dropped because #41560 already added the same one to the mapped file. --------- Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com> --- .../agentic_followup_kwargs.py | 32 ++++++++ .../chat_completion_agentic_loop.py | 46 +++++++---- litellm/llms/custom_httpx/llm_http_handler.py | 68 ++++++++-------- .../test_agentic_followup_kwargs.py | 66 +++++++++++++++ .../test_chat_completion_agentic_loop.py | 34 ++++++++ .../custom_httpx/test_llm_http_handler.py | 80 +++++++++++++++++++ 6 files changed, 278 insertions(+), 48 deletions(-) create mode 100644 litellm/litellm_core_utils/agentic_followup_kwargs.py create mode 100644 tests/test_litellm/litellm_core_utils/test_agentic_followup_kwargs.py diff --git a/litellm/litellm_core_utils/agentic_followup_kwargs.py b/litellm/litellm_core_utils/agentic_followup_kwargs.py new file mode 100644 index 00000000000..50ec19f62c4 --- /dev/null +++ b/litellm/litellm_core_utils/agentic_followup_kwargs.py @@ -0,0 +1,32 @@ +from collections.abc import Collection, Mapping, Sequence +from itertools import chain +from types import MappingProxyType +from typing import Final + + +def build_agentic_followup_kwargs( + *, + request_kwargs: Mapping[str, object], + patch_kwargs: Mapping[str, object], + request_params: Collection[str], + depth: int, + max_loops: int, + fingerprints: Sequence[str], + fingerprint: str, +) -> Mapping[str, object]: + """Kwargs for an agentic follow-up call: the request's kwargs overlaid by the plan's, never repeating a key already sent as a request param""" + seen: Final = [*fingerprints, fingerprint] # mutable-ok: the chat loop's settings reader only accepts a list + return MappingProxyType( + { + key: value + for key, value in chain( + ((k, v) for k, v in request_kwargs.items() if k not in request_params), + ((k, v) for k, v in patch_kwargs.items() if k not in request_params), + ( + ("_agentic_loop_depth", depth + 1), + ("max_agentic_loops", max_loops), + ("_agentic_loop_fingerprints", seen), + ), + ) + } + ) diff --git a/litellm/litellm_core_utils/chat_completion_agentic_loop.py b/litellm/litellm_core_utils/chat_completion_agentic_loop.py index c8e9e2583ba..e0bd85a7937 100644 --- a/litellm/litellm_core_utils/chat_completion_agentic_loop.py +++ b/litellm/litellm_core_utils/chat_completion_agentic_loop.py @@ -2,10 +2,13 @@ import json from collections.abc import Mapping +from itertools import chain +from types import MappingProxyType from typing import Final, cast from litellm._logging import verbose_logger from litellm.integrations.custom_logger import CustomLogger +from litellm.litellm_core_utils.agentic_followup_kwargs import build_agentic_followup_kwargs from litellm.litellm_core_utils.agentic_loop_settings import ( DEFAULT_MAX_AGENTIC_LOOPS, validated_max_agentic_loops, @@ -117,13 +120,25 @@ def _wrap_response_as_fake_stream( ) -def _add_agentic_loop_metadata(kwargs_for_followup: dict[str, object]) -> None: - metadata = kwargs_for_followup.get("litellm_metadata") - metadata = dict(metadata) if isinstance(metadata, dict) else {} - for key, value in kwargs_for_followup.items(): - if key.startswith("_agentic_loop") or key == "max_agentic_loops" or is_interception_internal_key(key): - metadata[key] = value - kwargs_for_followup["litellm_metadata"] = metadata +def _with_agentic_loop_metadata(kwargs_for_followup: Mapping[str, object]) -> Mapping[str, object]: + metadata: Final = kwargs_for_followup.get("litellm_metadata") + return MappingProxyType( + { + **kwargs_for_followup, + "litellm_metadata": dict( # mutable-ok: the follow-up call's logging and proxy hooks write into litellm_metadata in place + chain( + metadata.items() if isinstance(metadata, dict) else (), + ( + (key, value) + for key, value in kwargs_for_followup.items() + if key.startswith("_agentic_loop") + or key == "max_agentic_loops" + or is_interception_internal_key(key) + ), + ) + ), + } + ) def _filter_followup_kwargs(source: dict[str, object]) -> dict[str, object]: @@ -165,14 +180,17 @@ async def _execute_chat_completion_agentic_plan( if "tool_choice" not in patch.optional_params: optional_params_for_followup.pop("tool_choice", None) - kwargs_for_followup: Final = _filter_followup_kwargs(kwargs) - kwargs_for_followup.update( - {k: v for k, v in _filter_followup_kwargs(patch.kwargs).items() if k not in optional_params_for_followup} + kwargs_for_followup: Final = _with_agentic_loop_metadata( + build_agentic_followup_kwargs( + request_kwargs=_filter_followup_kwargs(kwargs), + patch_kwargs=_filter_followup_kwargs(patch.kwargs), + request_params=frozenset((*optional_params_for_followup, "model", "messages")), + depth=depth, + max_loops=max_loops, + fingerprints=fingerprints, + fingerprint=fingerprint, + ) ) - kwargs_for_followup["_agentic_loop_depth"] = depth + 1 - kwargs_for_followup["max_agentic_loops"] = max_loops - kwargs_for_followup["_agentic_loop_fingerprints"] = fingerprints + [fingerprint] - _add_agentic_loop_metadata(kwargs_for_followup) try: response_followup = await litellm.acompletion( diff --git a/litellm/llms/custom_httpx/llm_http_handler.py b/litellm/llms/custom_httpx/llm_http_handler.py index db821f42a90..2a105301521 100644 --- a/litellm/llms/custom_httpx/llm_http_handler.py +++ b/litellm/llms/custom_httpx/llm_http_handler.py @@ -4,7 +4,6 @@ import ssl from collections.abc import AsyncGenerator, AsyncIterator, Coroutine, Iterator, Mapping, Sequence from contextlib import asynccontextmanager from functools import lru_cache -from itertools import chain from types import MappingProxyType, ModuleType from typing import ( TYPE_CHECKING, @@ -34,6 +33,7 @@ from litellm._logging import _redact_string, verbose_logger from litellm.anthropic_beta_headers_manager import update_headers_with_filtered_beta from litellm.constants import MAX_FILE_LIST_LIMIT, REALTIME_WEBSOCKET_MAX_MESSAGE_SIZE_BYTES from litellm.files.types import FileContentStreamingResult +from litellm.litellm_core_utils.agentic_followup_kwargs import build_agentic_followup_kwargs from litellm.litellm_core_utils.agentic_loop_settings import ( DEFAULT_MAX_AGENTIC_LOOPS, validated_max_agentic_loops, @@ -5614,28 +5614,22 @@ class BaseLLMHTTPHandler: } internal_keys: Final = {"litellm_logging_obj"} - kwargs_for_followup: Final = MappingProxyType( - { - key: value - for key, value in chain( - ( - (k, v) - for k, v in kwargs.items() - if not is_interception_internal_key( - k, prefixes=NON_CODE_INTERPRETER_INTERCEPTION_INTERNAL_PREFIXES - ) - and k != "_code_interpreter_interception_converted_stream" - and k not in internal_keys - and k not in optional_params - ), - ((k, v) for k, v in patch.kwargs.items() if k not in optional_params), - ( - ("_agentic_loop_depth", depth + 1), - ("max_agentic_loops", max_loops), - ("_agentic_loop_fingerprints", fingerprints + [fingerprint]), - ), - ) - } + kwargs_for_followup: Final = build_agentic_followup_kwargs( + request_kwargs=MappingProxyType( + { + k: v + for k, v in kwargs.items() + if not is_interception_internal_key(k, prefixes=NON_CODE_INTERPRETER_INTERCEPTION_INTERNAL_PREFIXES) + and k != "_code_interpreter_interception_converted_stream" + and k not in internal_keys + } + ), + patch_kwargs=patch.kwargs, + request_params=frozenset((*optional_params, "model", "input")), + depth=depth, + max_loops=max_loops, + fingerprints=fingerprints, + fingerprint=fingerprint, ) try: @@ -5756,17 +5750,23 @@ class BaseLLMHTTPHandler: "stream_response", "custom_prompt_dict", } - kwargs_for_followup: Final = { - k: v - for k, v in kwargs.items() - if not k.startswith("_websearch_interception") - and not k.startswith("_compression_interception") - and k not in internal_params - } - kwargs_for_followup.update(patch.kwargs) - kwargs_for_followup["_agentic_loop_depth"] = depth + 1 - kwargs_for_followup["max_agentic_loops"] = max_loops - kwargs_for_followup["_agentic_loop_fingerprints"] = fingerprints + [fingerprint] + kwargs_for_followup: Final = build_agentic_followup_kwargs( + request_kwargs=MappingProxyType( + { + k: v + for k, v in kwargs.items() + if not k.startswith("_websearch_interception") + and not k.startswith("_compression_interception") + and k not in internal_params + } + ), + patch_kwargs=patch.kwargs, + request_params=frozenset((*optional_params_for_followup, "model", "messages")), + depth=depth, + max_loops=max_loops, + fingerprints=fingerprints, + fingerprint=fingerprint, + ) return await litellm.acompletion( model=full_model_name, diff --git a/tests/test_litellm/litellm_core_utils/test_agentic_followup_kwargs.py b/tests/test_litellm/litellm_core_utils/test_agentic_followup_kwargs.py new file mode 100644 index 00000000000..af0fbdcc35b --- /dev/null +++ b/tests/test_litellm/litellm_core_utils/test_agentic_followup_kwargs.py @@ -0,0 +1,66 @@ +from collections.abc import Mapping +from typing import Final + +from litellm.litellm_core_utils.agentic_followup_kwargs import build_agentic_followup_kwargs + + +def _build( + *, + request_kwargs: dict[str, object], + patch_kwargs: dict[str, object], + request_params: set[str], + fingerprints: list[str] | None = None, +) -> Mapping[str, object]: + return build_agentic_followup_kwargs( + request_kwargs=request_kwargs, + patch_kwargs=patch_kwargs, + request_params=request_params, + depth=0, + max_loops=3, + fingerprints=fingerprints if fingerprints is not None else [], + fingerprint="fp", + ) + + +def test_followup_kwargs_never_repeat_a_request_param(): + """Neither source may re-add a key the caller already sends as a request param, or the follow-up call raises a duplicate keyword""" + followup: Final = _build( + request_kwargs={"prompt_cache_key": "thread-1", "api_base": "https://a"}, + patch_kwargs={"prompt_cache_key": "thread-1", "metadata": {"user": "u1"}}, + request_params={"prompt_cache_key", "model", "input"}, + ) + + assert followup.keys().isdisjoint({"prompt_cache_key", "model", "input"}) + assert followup["api_base"] == "https://a" + assert followup["metadata"] == {"user": "u1"} + + +def test_followup_kwargs_let_the_plan_override_the_request(): + followup: Final = _build( + request_kwargs={"api_base": "https://request", "timeout": 5}, + patch_kwargs={"api_base": "https://plan"}, + request_params=set(), + ) + + assert followup["api_base"] == "https://plan" + assert followup["timeout"] == 5 + + +def test_followup_kwargs_carry_the_loop_bookkeeping_without_touching_the_inputs(): + fingerprints: Final = ["earlier"] + request_kwargs: Final = {"_agentic_loop_depth": 0, "max_agentic_loops": 9} + patch_kwargs: Final = {"_agentic_loop_fingerprints": ["stale"]} + + followup: Final = _build( + request_kwargs=request_kwargs, + patch_kwargs=patch_kwargs, + request_params=set(), + fingerprints=fingerprints, + ) + + assert followup["_agentic_loop_depth"] == 1 + assert followup["max_agentic_loops"] == 3 + assert followup["_agentic_loop_fingerprints"] == ["earlier", "fp"] + assert fingerprints == ["earlier"] + assert request_kwargs == {"_agentic_loop_depth": 0, "max_agentic_loops": 9} + assert patch_kwargs == {"_agentic_loop_fingerprints": ["stale"]} diff --git a/tests/test_litellm/litellm_core_utils/test_chat_completion_agentic_loop.py b/tests/test_litellm/litellm_core_utils/test_chat_completion_agentic_loop.py index 434daab6ab5..0d7f735e6e5 100644 --- a/tests/test_litellm/litellm_core_utils/test_chat_completion_agentic_loop.py +++ b/tests/test_litellm/litellm_core_utils/test_chat_completion_agentic_loop.py @@ -343,6 +343,40 @@ async def test_dispatcher_runs_followup_with_incremented_depth_and_patched_messa assert logger.cleanup_calls == 1 +@pytest.mark.asyncio +async def test_dispatcher_followup_does_not_repeat_a_request_param_found_in_request_kwargs( + restore_callbacks, +): + """Request kwargs that repeat a request param must not crash the follow-up + with a duplicate keyword, whether or not the plan copies them too.""" + followup = _plain_model_response("done") + request_kwargs = {"temperature": 0.2, "api_base": "https://a"} + plan = AgenticLoopPlan( + run_agentic_loop=True, + request_patch=AgenticLoopRequestPatch(messages=_patched_messages(), kwargs=dict(request_kwargs)), + ) + litellm.callbacks = [_GateOnlyLogger(plan=plan, tool_calls={"tool_calls": [{"id": "call_abc"}]})] + + acompletion_mock = AsyncMock(return_value=followup) + with patch.object(litellm, "acompletion", acompletion_mock): + result = await maybe_run_chat_completion_agentic_loop( + response=_tool_call_model_response(), + model="gpt-4o-mini", + messages=[{"role": "user", "content": "what is 6*7?"}], + optional_params={"temperature": 0.2}, + kwargs=dict(request_kwargs), + logging_obj=_LoggingStub(), + custom_llm_provider="openai", + stream=False, + ) + + assert result is followup + acompletion_mock.assert_awaited_once() + call_kwargs = acompletion_mock.await_args.kwargs + assert call_kwargs["temperature"] == 0.2 + assert call_kwargs["api_base"] == "https://a" + + @pytest.mark.asyncio async def test_dispatcher_raises_when_depth_reaches_max_agentic_loops( restore_callbacks, diff --git a/tests/test_litellm/llms/custom_httpx/test_llm_http_handler.py b/tests/test_litellm/llms/custom_httpx/test_llm_http_handler.py index fa4b7439dd8..67a8d045036 100644 --- a/tests/test_litellm/llms/custom_httpx/test_llm_http_handler.py +++ b/tests/test_litellm/llms/custom_httpx/test_llm_http_handler.py @@ -4051,3 +4051,83 @@ async def test_responses_agentic_followup_does_not_repeat_request_params_from_pl assert followup_calls[0]["prompt_cache_key"] == "thread-1" assert followup_calls[0]["metadata"] == {"user": "u1"} assert followup_calls[0]["_agentic_loop_depth"] == 1 + + +@pytest.mark.asyncio +async def test_responses_agentic_followup_sends_the_plans_request_param_over_a_stale_kwargs_copy(monkeypatch): + from litellm.integrations.custom_logger import CustomLogger + from litellm.types.integrations.custom_logger import AgenticLoopPlan, AgenticLoopRequestPatch + + followup_calls: list[dict[str, object]] = [] + + async def fake_aresponses(**kwargs: object) -> str: + followup_calls.append(kwargs) + return "followup-response" + + monkeypatch.setattr(litellm, "aresponses", fake_aresponses) + + await BaseLLMHTTPHandler()._execute_responses_agentic_plan( + plan=AgenticLoopPlan( + run_agentic_loop=True, + request_patch=AgenticLoopRequestPatch( + model="gpt-5", + messages=[{"role": "user", "content": "x"}], + optional_params={"prompt_cache_key": "from-plan-params"}, + kwargs={"prompt_cache_key": "stale-copy"}, + ), + ), + model="gpt-5", + response_api_optional_request_params={"prompt_cache_key": "from-request"}, + logging_obj=Mock(litellm_call_id="call-1"), + kwargs={}, + depth=0, + max_loops=3, + fingerprints=[], + fingerprint="fp", + callback=CustomLogger(), + ) + + assert followup_calls[0]["prompt_cache_key"] == "from-plan-params" + + +@pytest.mark.asyncio +async def test_chat_completion_agentic_followup_does_not_repeat_request_params_from_plan_kwargs(monkeypatch): + """A plan whose kwargs repeat a request param, or the explicitly passed model, must not crash the chat follow-up with a duplicate keyword""" + from litellm.types.integrations.custom_logger import AgenticLoopPlan, AgenticLoopRequestPatch + + followup_calls: list[dict[str, object]] = [] + + async def fake_acompletion(**kwargs: object) -> str: + followup_calls.append(kwargs) + return "followup-response" + + monkeypatch.setattr(litellm, "acompletion", fake_acompletion) + request_kwargs: Final = {"temperature": 0.2, "api_base": "https://a", "model": "gpt-5"} + plan: Final = AgenticLoopPlan( + run_agentic_loop=True, + request_patch=AgenticLoopRequestPatch( + model="gpt-5", + messages=[{"role": "user", "content": "x"}], + optional_params={"temperature": 0.2}, + kwargs=dict(request_kwargs), + ), + ) + + response: Final = await BaseLLMHTTPHandler()._execute_chat_completion_agentic_plan( + plan=plan, + model="gpt-5", + messages=[{"role": "user", "content": "x"}], + optional_params={"temperature": 0.2}, + kwargs=dict(request_kwargs), + custom_llm_provider="openai", + depth=0, + max_loops=3, + fingerprints=[], + fingerprint="fp", + ) + + assert response == "followup-response" + assert len(followup_calls) == 1 + assert followup_calls[0]["temperature"] == 0.2 + assert followup_calls[0]["api_base"] == "https://a" + assert followup_calls[0]["model"] == "openai/gpt-5"