mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-24 00:52:24 +00:00
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>
This commit is contained in:
parent
3252852b0f
commit
e7f3f58f96
6 changed files with 278 additions and 48 deletions
32
litellm/litellm_core_utils/agentic_followup_kwargs.py
Normal file
32
litellm/litellm_core_utils/agentic_followup_kwargs.py
Normal file
|
|
@ -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),
|
||||
),
|
||||
)
|
||||
}
|
||||
)
|
||||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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"]}
|
||||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue