mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
Merge pull request #36246 from BerriAI/litellm_lit_5307_advisor_router
fix(advisor): resolve the advisor sub-call through the proxy router
This commit is contained in:
commit
d03ef8be03
2 changed files with 363 additions and 16 deletions
|
|
@ -16,7 +16,7 @@ How it works:
|
|||
|
||||
import uuid
|
||||
from collections.abc import AsyncIterator
|
||||
from typing import Any, Final
|
||||
from typing import TYPE_CHECKING, Any, Final
|
||||
|
||||
import litellm
|
||||
import litellm.constants as _c
|
||||
|
|
@ -28,6 +28,9 @@ from litellm.types.llms.anthropic_messages.anthropic_response import (
|
|||
AnthropicMessagesResponse,
|
||||
)
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from litellm.router import Router
|
||||
|
||||
ADVISOR_MAX_USES: Final[int] = _c.ADVISOR_MAX_USES
|
||||
ADVISOR_NATIVE_PROVIDERS: Final[frozenset] = _c.ADVISOR_NATIVE_PROVIDERS
|
||||
ADVISOR_TOOL_DESCRIPTION: Final[str] = _c.ADVISOR_TOOL_DESCRIPTION
|
||||
|
|
@ -97,6 +100,14 @@ class AdvisorOrchestrationHandler(MessagesInterceptor):
|
|||
|
||||
parent_request_id: Final[str] = str(kwargs.pop("litellm_call_id", None) or uuid.uuid4())
|
||||
metadata_base: Final[dict] = dict(kwargs.pop("metadata", None) or {})
|
||||
advisor_metadata: Final = {
|
||||
**metadata_base,
|
||||
"advisor_sub_call": True,
|
||||
"parent_request_id": parent_request_id,
|
||||
}
|
||||
advisor_router: Final = (
|
||||
None if (advisor_api_key or advisor_api_base) else _resolve_advisor_router(advisor_model)
|
||||
)
|
||||
iteration = 0
|
||||
|
||||
while True:
|
||||
|
|
@ -138,20 +149,27 @@ class AdvisorOrchestrationHandler(MessagesInterceptor):
|
|||
|
||||
# --- Advisor sub-call (always non-streaming, no tools) ---
|
||||
try:
|
||||
advisor_response: AnthropicMessagesResponse = await _call_messages_handler(
|
||||
model=advisor_model,
|
||||
messages=advisor_messages,
|
||||
tools=None,
|
||||
stream=False,
|
||||
max_tokens=max_tokens,
|
||||
custom_llm_provider=None, # let litellm resolve from model name
|
||||
metadata={
|
||||
**metadata_base,
|
||||
"advisor_sub_call": True,
|
||||
"parent_request_id": parent_request_id,
|
||||
},
|
||||
api_key=advisor_api_key,
|
||||
api_base=advisor_api_base,
|
||||
advisor_response: AnthropicMessagesResponse = (
|
||||
await advisor_router.aanthropic_messages(
|
||||
model=advisor_model,
|
||||
messages=advisor_messages,
|
||||
tools=None,
|
||||
stream=False,
|
||||
max_tokens=max_tokens,
|
||||
metadata=advisor_metadata,
|
||||
)
|
||||
if advisor_router is not None
|
||||
else await _call_messages_handler(
|
||||
model=advisor_model,
|
||||
messages=advisor_messages,
|
||||
tools=None,
|
||||
stream=False,
|
||||
max_tokens=max_tokens,
|
||||
custom_llm_provider=None,
|
||||
metadata=advisor_metadata,
|
||||
api_key=advisor_api_key,
|
||||
api_base=advisor_api_base,
|
||||
)
|
||||
)
|
||||
except Exception as advisor_sub_call_exception:
|
||||
mark_advisor_orchestration_failure(advisor_sub_call_exception)
|
||||
|
|
@ -284,6 +302,11 @@ def _build_advisor_context(
|
|||
|
||||
tool_use blocks are excluded because Anthropic requires tool_use to be
|
||||
immediately followed by tool_result — not the advisor question.
|
||||
|
||||
In-sequence system rows (e.g. Claude Code SessionStart hook output) are
|
||||
excluded: they are executor-directed, and a trailing one becomes invalid
|
||||
once the question turn is appended after it (a system row must precede an
|
||||
assistant message or end the array).
|
||||
"""
|
||||
question: Final = (advisor_use_block.get("input") or {}).get("question") or (
|
||||
"Please provide guidance on the current task."
|
||||
|
|
@ -295,7 +318,7 @@ def _build_advisor_context(
|
|||
for block in raw_content
|
||||
if isinstance(block, dict) and block.get("type") == "text"
|
||||
]
|
||||
result: Final = list(messages)
|
||||
result: Final = [m for m in messages if m.get("role") != "system"]
|
||||
if executor_text_blocks:
|
||||
result.append({"role": "assistant", "content": executor_text_blocks})
|
||||
result.append({"role": "user", "content": question})
|
||||
|
|
@ -357,6 +380,24 @@ def _inject_max_uses_error(
|
|||
]
|
||||
|
||||
|
||||
def _resolve_advisor_router(advisor_model: str) -> "Router | None":
|
||||
"""Return the proxy router when it serves ``advisor_model`` directly or via a wildcard.
|
||||
|
||||
Returns ``None`` for SDK callers (no proxy router) and for advisor models the router
|
||||
doesn't know about, so those keep resolving through ``litellm.anthropic_messages()``
|
||||
provider inference.
|
||||
"""
|
||||
try:
|
||||
from litellm.proxy.proxy_server import llm_router
|
||||
except (ImportError, ModuleNotFoundError):
|
||||
return None
|
||||
if llm_router is None:
|
||||
return None
|
||||
if llm_router.is_recognized_model(advisor_model) or llm_router.pattern_router.route(advisor_model):
|
||||
return llm_router
|
||||
return None
|
||||
|
||||
|
||||
async def _call_messages_handler(
|
||||
model: str,
|
||||
messages: list[dict],
|
||||
|
|
|
|||
|
|
@ -1041,3 +1041,309 @@ async def test_executor_failure_is_not_tagged():
|
|||
)
|
||||
|
||||
assert is_advisor_orchestration_failure(exc_info.value) is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 15. The advisor sub-call resolves through the proxy router when the advisor
|
||||
# model is configured in model_list, instead of dialing the public
|
||||
# Anthropic API (regression for LIT-5307).
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _router_with_advisor_deployment(
|
||||
recorder, advisor_model="claude-opus-4-8", deployment_model=None, model_group_alias=None
|
||||
):
|
||||
"""Build a Router whose only deployment is the advisor model on Foundry.
|
||||
|
||||
The recorder replaces ``litellm.anthropic_messages`` before construction
|
||||
because Router binds it at init time, so the returned Router exercises the
|
||||
real deployment-resolution path and records what it dispatched.
|
||||
"""
|
||||
import litellm
|
||||
from litellm.router import Router
|
||||
|
||||
with patch("litellm.anthropic_messages", new=recorder):
|
||||
return Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": advisor_model,
|
||||
"litellm_params": {
|
||||
"model": deployment_model or f"azure_ai/{advisor_model}",
|
||||
"api_base": "http://127.0.0.1:1/foundry",
|
||||
"api_key": "fake-foundry-key",
|
||||
},
|
||||
}
|
||||
],
|
||||
model_group_alias=model_group_alias,
|
||||
num_retries=0,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_advisor_sub_call_routes_through_proxy_router():
|
||||
import litellm.proxy.proxy_server as proxy_server
|
||||
from litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor import (
|
||||
AdvisorOrchestrationHandler,
|
||||
)
|
||||
|
||||
router_calls = []
|
||||
|
||||
async def recorder(**kwargs):
|
||||
router_calls.append(kwargs)
|
||||
return _make_text_response("Use trial division.", model="claude-opus-4-8")
|
||||
|
||||
router = _router_with_advisor_deployment(recorder)
|
||||
|
||||
call_count = 0
|
||||
|
||||
async def mock_call(model, messages, tools, stream, max_tokens, **kwargs):
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
if call_count == 1:
|
||||
return _make_advisor_tool_use_response()
|
||||
return _make_text_response("Final answer.")
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor._call_messages_handler",
|
||||
side_effect=mock_call,
|
||||
),
|
||||
patch.object(proxy_server, "llm_router", router),
|
||||
):
|
||||
h = AdvisorOrchestrationHandler()
|
||||
result = await h.handle(
|
||||
model="executor-model",
|
||||
messages=MESSAGES,
|
||||
tools=[{**ADVISOR_TOOL, "model": "claude-opus-4-8"}],
|
||||
stream=False,
|
||||
max_tokens=512,
|
||||
custom_llm_provider="azure_ai",
|
||||
)
|
||||
|
||||
assert call_count == 2
|
||||
assert len(router_calls) == 1
|
||||
assert router_calls[0]["model"] == "azure_ai/claude-opus-4-8"
|
||||
assert router_calls[0]["api_base"] == "http://127.0.0.1:1/foundry"
|
||||
assert router_calls[0]["api_key"] == "fake-foundry-key"
|
||||
assert "Final answer." in result["content"][0]["text"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("router_kwargs", "advisor_model"),
|
||||
[
|
||||
pytest.param({"model_group_alias": {"advisor": "claude-opus-4-8"}}, "advisor", id="model_group_alias"),
|
||||
pytest.param(
|
||||
{"advisor_model": "azure_ai/*", "deployment_model": "azure_ai/*"},
|
||||
"azure_ai/claude-opus-4-8",
|
||||
id="wildcard",
|
||||
),
|
||||
],
|
||||
)
|
||||
async def test_advisor_sub_call_routes_through_router_for_alias_and_wildcard(router_kwargs, advisor_model):
|
||||
"""Alias and wildcard advisor models resolve through the router like exact model_list matches."""
|
||||
import litellm.proxy.proxy_server as proxy_server
|
||||
from litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor import (
|
||||
AdvisorOrchestrationHandler,
|
||||
)
|
||||
|
||||
router_calls = []
|
||||
|
||||
async def recorder(**kwargs):
|
||||
router_calls.append(kwargs)
|
||||
return _make_text_response("Use trial division.", model="claude-opus-4-8")
|
||||
|
||||
router = _router_with_advisor_deployment(recorder, **router_kwargs)
|
||||
|
||||
call_count = 0
|
||||
|
||||
async def mock_call(model, messages, tools, stream, max_tokens, **kwargs):
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
if call_count == 1:
|
||||
return _make_advisor_tool_use_response()
|
||||
return _make_text_response("Final answer.")
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor._call_messages_handler",
|
||||
side_effect=mock_call,
|
||||
),
|
||||
patch.object(proxy_server, "llm_router", router),
|
||||
):
|
||||
h = AdvisorOrchestrationHandler()
|
||||
await h.handle(
|
||||
model="executor-model",
|
||||
messages=MESSAGES,
|
||||
tools=[{**ADVISOR_TOOL, "model": advisor_model}],
|
||||
stream=False,
|
||||
max_tokens=512,
|
||||
custom_llm_provider="azure_ai",
|
||||
)
|
||||
|
||||
assert call_count == 2
|
||||
assert len(router_calls) == 1
|
||||
assert router_calls[0]["model"] == "azure_ai/claude-opus-4-8"
|
||||
assert router_calls[0]["api_base"] == "http://127.0.0.1:1/foundry"
|
||||
assert router_calls[0]["api_key"] == "fake-foundry-key"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_advisor_sub_call_bypasses_router_for_unconfigured_model():
|
||||
"""An advisor model the router doesn't know about keeps the SDK-level path."""
|
||||
import litellm.proxy.proxy_server as proxy_server
|
||||
from litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor import (
|
||||
AdvisorOrchestrationHandler,
|
||||
)
|
||||
|
||||
router_calls = []
|
||||
|
||||
async def recorder(**kwargs):
|
||||
router_calls.append(kwargs)
|
||||
return _make_text_response("should not be used")
|
||||
|
||||
router = _router_with_advisor_deployment(recorder, advisor_model="some-other-model")
|
||||
|
||||
call_count = 0
|
||||
|
||||
async def mock_call(model, messages, tools, stream, max_tokens, **kwargs):
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
if call_count == 1:
|
||||
return _make_advisor_tool_use_response()
|
||||
if tools is None:
|
||||
return _make_text_response("Advice.", model="claude-opus-4-8")
|
||||
return _make_text_response("Final answer.")
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor._call_messages_handler",
|
||||
side_effect=mock_call,
|
||||
),
|
||||
patch.object(proxy_server, "llm_router", router),
|
||||
):
|
||||
h = AdvisorOrchestrationHandler()
|
||||
await h.handle(
|
||||
model="executor-model",
|
||||
messages=MESSAGES,
|
||||
tools=[{**ADVISOR_TOOL, "model": "claude-opus-4-8"}],
|
||||
stream=False,
|
||||
max_tokens=512,
|
||||
custom_llm_provider="azure_ai",
|
||||
)
|
||||
|
||||
assert router_calls == []
|
||||
assert call_count == 3
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_advisor_sub_call_client_override_bypasses_router():
|
||||
"""A caller-supplied api_key/api_base override must not be re-routed."""
|
||||
import litellm
|
||||
import litellm.proxy.proxy_server as proxy_server
|
||||
from litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor import (
|
||||
AdvisorOrchestrationHandler,
|
||||
)
|
||||
|
||||
router_calls = []
|
||||
|
||||
async def recorder(**kwargs):
|
||||
router_calls.append(kwargs)
|
||||
return _make_text_response("should not be used")
|
||||
|
||||
router = _router_with_advisor_deployment(recorder)
|
||||
|
||||
sub_calls = []
|
||||
|
||||
async def mock_call(model, messages, tools, stream, max_tokens, **kwargs):
|
||||
sub_calls.append({"model": model, "tools": tools, **kwargs})
|
||||
if len(sub_calls) == 1:
|
||||
return _make_advisor_tool_use_response()
|
||||
if tools is None:
|
||||
return _make_text_response("Advice.", model="claude-opus-4-8")
|
||||
return _make_text_response("Final answer.")
|
||||
|
||||
advisor_tool = {
|
||||
**ADVISOR_TOOL,
|
||||
"model": "claude-opus-4-8",
|
||||
"api_key": "client-key",
|
||||
"api_base": "https://client.example.com",
|
||||
}
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor._call_messages_handler",
|
||||
side_effect=mock_call,
|
||||
),
|
||||
patch.object(proxy_server, "llm_router", router),
|
||||
patch.dict(proxy_server.general_settings, {"allow_client_side_credentials": True}),
|
||||
patch.object(litellm, "user_url_validation", False),
|
||||
):
|
||||
h = AdvisorOrchestrationHandler()
|
||||
await h.handle(
|
||||
model="executor-model",
|
||||
messages=MESSAGES,
|
||||
tools=[advisor_tool],
|
||||
stream=False,
|
||||
max_tokens=512,
|
||||
custom_llm_provider="azure_ai",
|
||||
)
|
||||
|
||||
assert router_calls == []
|
||||
advisor_sub_calls = [c for c in sub_calls if c["tools"] is None]
|
||||
assert len(advisor_sub_calls) == 1
|
||||
assert advisor_sub_calls[0]["api_key"] == "client-key"
|
||||
assert advisor_sub_calls[0]["api_base"] == "https://client.example.com"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 16. In-sequence system rows (e.g. Claude Code SessionStart hook output) are
|
||||
# excluded from the advisor sub-call context but kept for the executor: a
|
||||
# trailing system row followed by the appended question turn is rejected
|
||||
# upstream ("role 'system' must precede an 'assistant' message or end the
|
||||
# array").
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_advisor_context_excludes_in_sequence_system_rows():
|
||||
from litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor import (
|
||||
AdvisorOrchestrationHandler,
|
||||
)
|
||||
|
||||
messages_with_system_row = [
|
||||
*MESSAGES,
|
||||
{"role": "system", "content": "SessionStart hook output: prefer functional style."},
|
||||
]
|
||||
|
||||
sub_calls = []
|
||||
|
||||
async def mock_call(model, messages, tools, stream, max_tokens, **kwargs):
|
||||
sub_calls.append({"messages": messages, "tools": tools})
|
||||
if len(sub_calls) == 1:
|
||||
return _make_advisor_tool_use_response()
|
||||
if tools is None:
|
||||
return _make_text_response("Advice.", model="claude-opus-4-6")
|
||||
return _make_text_response("Final answer.")
|
||||
|
||||
with patch(
|
||||
"litellm.llms.anthropic.experimental_pass_through.messages.interceptors.advisor._call_messages_handler",
|
||||
side_effect=mock_call,
|
||||
):
|
||||
h = AdvisorOrchestrationHandler()
|
||||
await h.handle(
|
||||
model="openai/gpt-4o-mini",
|
||||
messages=messages_with_system_row,
|
||||
tools=[ADVISOR_TOOL],
|
||||
stream=False,
|
||||
max_tokens=512,
|
||||
custom_llm_provider="openai",
|
||||
)
|
||||
|
||||
assert len(sub_calls) == 3
|
||||
advisor_messages = sub_calls[1]["messages"]
|
||||
assert sub_calls[1]["tools"] is None
|
||||
assert [m["role"] for m in advisor_messages if m["role"] == "system"] == []
|
||||
assert advisor_messages[-1]["role"] == "user"
|
||||
executor_roles = [m["role"] for m in sub_calls[0]["messages"]]
|
||||
assert "system" in executor_roles
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue