From 2306816d40f9fd720c12cc8acff3e6b19e0cae84 Mon Sep 17 00:00:00 2001 From: tin-berri Date: Thu, 27 Aug 2026 18:44:44 -0700 Subject: [PATCH] fix(shadow_eval): refuse a judge model that also serves one of the arms it grades (#38589) A shadow eval whose judge_model is one of the router's tier models, the router's default model, or a reverse job's baseline_model was accepted with no warning. An LLM judge scores its own output higher than a rival's, so that tier's win rate measures the judge instead of the models, and the job's whole budget buys a result that has to be thrown away. start_shadow_eval now rejects it with a 400 naming the colliding arm. `judge_target` is the single answer to "where does a call to this name go for this caller, and what answers it", and the resolvability gate, the collision gate and the judge dispatch all read it. It has three outcomes and no others: the router serves the name, the SDK serves it, or nothing does. Splitting that question is what every bug here came from, so `router_resolves_model` and `answering_models` are gone rather than joined by a third. Two spellings of one model are one identity. A name is compared by what would answer it, resolved through every channel `get_model_list` composes and then put in the provider-qualified form litellm itself uses, so a judge given as `gpt-4o` collides with a tier deployment serving `openai/gpt-4o`, and a judge given as `openai/gpt-4o` collides with a deployment configured as bare `gpt-4o`. Both ends are normalised because an admin writes them at different times. Answering is also per-caller. The shadow and judge calls carry the shadowed key's `user_api_key_team_id`, which is what the router selects deployments with, so the endpoint derives the job's teams once from the keys it already looks up and every check runs under them, and the judge dispatch picks its arm under the same team. A team's public model name resolves to nothing for everyone else and a team's own deployment resolves for nobody else, so a check that omits the team answers for a caller who does not exist. A collision under any one team fails the job, because every key's verdicts land in the same win rates. Three sites were separately re-deriving "the provider models this name resolves to", with unexplained divergence in whether they fell back to the literal name. `Router.resolved_litellm_models` is now the one owner; the routing-plugin candidate list and the stream-options check both delegate to it, and `_deployment_litellm_model` is gone. The router's arms come from `strategy_router_dependencies`, the same enumeration the health check reads. Only the roles that serve are arms: a classifier or embedding model picks the tier and never produces a response anyone judges. A semantic auto-router keeps its routes in an opaque config blob, so only its default model is enumerable and the guard is incomplete there by design, able to miss a collision but never to invent one The two regenerated artifacts carry `presidio_analyze_chunk_size_bytes` from alters the spec; the sync gate runs on any PR touching litellm/proxy, so this one has to carry the base's drift to go green --- litellm/integrations/shadow_eval_logger.py | 9 + litellm/litellm_core_utils/llm_judge.py | 69 +++- litellm/proxy/common_request_processing.py | 16 +- .../auto_router_endpoints.py | 146 +++++++-- litellm/router.py | 24 +- .../integrations/test_shadow_eval_logger.py | 33 ++ .../litellm_core_utils/test_llm_judge.py | 86 ++++- .../test_auto_router_endpoints.py | 297 +++++++++++++++++- tests/test_litellm/test_router.py | 35 +++ 9 files changed, 643 insertions(+), 72 deletions(-) diff --git a/litellm/integrations/shadow_eval_logger.py b/litellm/integrations/shadow_eval_logger.py index 5f4e7c71395..c021014e249 100644 --- a/litellm/integrations/shadow_eval_logger.py +++ b/litellm/integrations/shadow_eval_logger.py @@ -452,6 +452,14 @@ async def _key_or_team_is_over_budget(metadata: Mapping[str, object]) -> bool: return False +def _forwarded_team_id(metadata: Mapping[str, object]) -> str | None: + """The shadowed key's team, the identity the judge call already carries in its metadata + and the router already selects deployments with. Read here too so the arm choice, which + happens before the router sees the call, is made under the same team.""" + team_id: Final = metadata.get("user_api_key_team_id") + return team_id if isinstance(team_id, str) and team_id else None + + def _routing_decision(metadata: Mapping[str, object]) -> Mapping[str, object]: """The routing decision a pre-routing strategy wrote to a call's metadata, empty when a plain model served it. Read off the sampled request for the control arm, and off the @@ -915,6 +923,7 @@ class ShadowEvalLogger(CustomLogger): self._router_provider(), judge_model, judge_messages, # pyright: ignore[reportArgumentType] # plain SDK message dicts + team_id=_forwarded_team_id(parent_metadata), temperature=0, max_tokens=JUDGE_MAX_OUTPUT_TOKENS, response_format=PAIRWISE_JUDGE_RESPONSE_FORMAT, diff --git a/litellm/litellm_core_utils/llm_judge.py b/litellm/litellm_core_utils/llm_judge.py index 4ad8d719402..b632d3a9af9 100644 --- a/litellm/litellm_core_utils/llm_judge.py +++ b/litellm/litellm_core_utils/llm_judge.py @@ -4,7 +4,9 @@ from __future__ import annotations import json import re -from typing import TYPE_CHECKING, Final +from dataclasses import dataclass +from functools import lru_cache +from typing import TYPE_CHECKING, Final, Literal import litellm @@ -56,17 +58,62 @@ def extract_text_from_content(content: object) -> str: return "" -def router_resolves_model(router: Router | None, model: str) -> bool: - """Whether the model name resolves through the proxy's router (configured deployment - or model-group alias), the same check the judge dispatch itself makes, so start-time - validation cannot accept a name the call path then fails on.""" - return router is not None and bool(model in router.model_group_alias or router.get_model_list(model_name=model)) +@lru_cache(maxsize=512) +def _provider_qualified(model: str) -> str | None: + """`model` in the one spelling litellm itself resolves it to, or None if it maps to no + provider. + + A deployment may be configured as `openai/gpt-4o` and a judge given as `gpt-4o`; both + reach the same model, so an identity that keeps them apart reports two models where + there is one. None is a different answer from "unchanged": a name that is already + provider-qualified normalises to itself, and reading that as a failure would call every + correctly-spelled public model unresolvable. + """ + try: + stripped, provider, _, _ = litellm.get_llm_provider(model=model) + except Exception: # noqa: BLE001 # an unmapped name has no provider, which is the answer + return None + return f"{provider}/{stripped}" if provider and stripped else None + + +@dataclass(frozen=True, slots=True) +class JudgeTarget: + """Where a call to one model name goes for one caller, and what answers it. + + The single answer to that question: the resolvability gate, the judge-vs-candidate + gate and the dispatch all read it, so none of them can decide it differently. Splitting + it is what let start-time validation accept a team's own model while dispatch sent the + literal name to the SDK. + """ + + via: Literal["router", "sdk", "nothing"] + models: frozenset[str] + + +def judge_target(router: Router | None, model: str, team_id: str | None = None) -> JudgeTarget: + """Resolve `model` the way a call from `team_id` would be. + + Three outcomes and no others: the router serves it (a deployment, a team-public name, + an alias, a routing group or a wildcard, exactly the channels `get_model_list` + composes); the SDK serves it because litellm recognises the provider; or nothing does, + which is the only case a caller may refuse on. + + `team_id` is part of the question, not a refinement of it. A team-public name resolves + only for its own team and a team's own deployment resolves for nobody else, so asking + without it answers for a caller who does not exist. + """ + served: Final = router.resolved_litellm_models(model, team_id=team_id) if router is not None else () + if served: + return JudgeTarget("router", frozenset(_provider_qualified(m) or m for m in served)) + qualified: Final = _provider_qualified(model) + return JudgeTarget("sdk", frozenset({qualified})) if qualified is not None else JudgeTarget("nothing", frozenset()) async def judge_acompletion( router: Router | None, judge_model: str, messages: list[AllMessageValues], # mutable-ok: the SDK acompletion signature takes a list + team_id: str | None = None, **params: object, ) -> ModelResponse: """Dispatch a judge call through the proxy's router when the judge model is a @@ -74,9 +121,13 @@ async def judge_acompletion( provider-qualified public names. The router path never retries or falls back: a failed judge call is the caller's counted failure, not a spend multiplier. Sampling preferences are advisory: models that removed sampling params (e.g. - claude-sonnet-5) drop them instead of rejecting the judge call.""" - if router_resolves_model(router, judge_model): - return await router.acompletion( # pyright: ignore[reportOptionalMemberAccess] # router_resolves_model implies router is not None + claude-sonnet-5) drop them instead of rejecting the judge call. + + The arm is chosen by `judge_target` under the caller's own team, the same call + start-time validation makes, so a judge a team can reach cannot be validated as a + deployment and then dispatched as a public name the SDK has never heard of.""" + if judge_target(router, judge_model, team_id).via == "router": + return await router.acompletion( # pyright: ignore[reportOptionalMemberAccess] # a router target implies router is not None model=judge_model, messages=messages, num_retries=0, diff --git a/litellm/proxy/common_request_processing.py b/litellm/proxy/common_request_processing.py index 315fbcba310..f555966b76b 100644 --- a/litellm/proxy/common_request_processing.py +++ b/litellm/proxy/common_request_processing.py @@ -417,15 +417,6 @@ def _litellm_model_supports_stream_options(litellm_model: str) -> bool: return supported_params is not None and "stream_options" in supported_params -def _deployment_litellm_model(deployment: Mapping[str, object]) -> str | None: - litellm_params: Final = deployment.get("litellm_params") - if isinstance(litellm_params, Mapping): - litellm_model = litellm_params.get("model") - else: - litellm_model = getattr(litellm_params, "model", None) - return litellm_model if isinstance(litellm_model, str) else None - - def _model_deployments_support_stream_options( model: object, llm_router: Router | None, @@ -433,11 +424,8 @@ def _model_deployments_support_stream_options( ) -> bool: if not isinstance(model, str): return False - deployments = llm_router.get_model_list(model_name=model, team_id=team_id) if llm_router is not None else None - deployment_models: Final = tuple( - litellm_model - for deployment in deployments or () - if (litellm_model := _deployment_litellm_model(deployment)) is not None + deployment_models: Final = ( + llm_router.resolved_litellm_models(model, team_id=team_id) if llm_router is not None else () ) candidate_models: Final = deployment_models if deployment_models else (model,) return all(_litellm_model_supports_stream_options(m) for m in candidate_models) diff --git a/litellm/proxy/management_endpoints/auto_router_endpoints.py b/litellm/proxy/management_endpoints/auto_router_endpoints.py index 1e459bb9b61..9f30ec01940 100644 --- a/litellm/proxy/management_endpoints/auto_router_endpoints.py +++ b/litellm/proxy/management_endpoints/auto_router_endpoints.py @@ -17,7 +17,7 @@ from pydantic import BaseModel, ConfigDict, TypeAdapter, field_validator from litellm._logging import verbose_proxy_logger from litellm.exceptions import BudgetExceededError -from litellm.litellm_core_utils.llm_judge import router_resolves_model +from litellm.litellm_core_utils.llm_judge import judge_target from litellm.proxy._types import ( CommonProxyErrors, LiteLLM_TeamTable, @@ -39,7 +39,11 @@ from litellm.proxy.litellm_pre_call_utils import ( from litellm.repositories.base_repository import SupportsModelDump from litellm.repositories.team_repository import TeamRepository from litellm.router_strategy.complexity_router import ComplexityRouter -from litellm.router_utils.auto_router_model_naming import classify_strategy_router_model +from litellm.router_utils.auto_router_model_naming import ( + StrategyRouterDependencyRole, + classify_strategy_router_model, + strategy_router_dependencies, +) from litellm.types.management_endpoints.auto_router_endpoints import ( SHADOW_EVAL_TURN_VALVE, AutoRouterBenchmarkGroup, @@ -89,6 +93,9 @@ class _VerificationTokenRow(Protocol): @property def key_name(self) -> str | None: ... + @property + def team_id(self) -> str | None: ... + class _VerificationTokenTable(Protocol): async def find_unique(self, *, where: Mapping[str, object]) -> _VerificationTokenRow | None: ... @@ -671,30 +678,126 @@ def _is_configured_pre_routing_strategy(llm_router: "Router", router_name: str) ) -def _validate_plain_model(llm_router: "Router | None", model: str, field_name: str) -> None: +def _validate_plain_model( + llm_router: "Router | None", model: str, field_name: str, team_ids: Sequence[str | None] +) -> None: """Reject a model the dispatch path cannot resolve, at start rather than as a silently growing error count once the job is already sampling and billing. Both the judge and a reverse job's baseline must be plain models: an auto-router in either slot would - re-route per turn, so the comparison would have no fixed arm to attribute results to.""" + re-route per turn, so the comparison would have no fixed arm to attribute results to. + + Resolvability is asked once per team the job samples for, because that is the identity + the call carries: a name only one team can reach fails every turn for the other keys, + which is the growing error count this check exists to prevent.""" if llm_router is not None and _is_configured_pre_routing_strategy(llm_router, model): raise HTTPException( status_code=400, detail=f"{field_name} '{model}' is an auto-router; it must be a plain model", ) - if router_resolves_model(llm_router, model): + unreachable: Final = tuple(team for team in team_ids if judge_target(llm_router, model, team).via == "nothing") + if not unreachable: return - import litellm + raise HTTPException( + status_code=400, + detail=( + f"{field_name} '{model}' is neither a model configured on this proxy nor a " + "provider-qualified public model name (e.g. 'anthropic/claude-sonnet-5')" + _for_teams(unreachable) + ), + ) - try: - litellm.get_llm_provider(model=model) - except Exception as e: - raise HTTPException( - status_code=400, - detail=( - f"{field_name} '{model}' is neither a model configured on this proxy nor a " - "provider-qualified public model name (e.g. 'anthropic/claude-sonnet-5')" - ), - ) from e + +def _for_teams(team_ids: Sequence[str | None]) -> str: + """Name the teams a fault applies to, when it does not apply to every key alike.""" + named: Final = tuple(sorted(team for team in team_ids if team is not None)) + return f" for team {', '.join(named)}" if named else "" + + +_JUDGED_ROLES: Final[frozenset[StrategyRouterDependencyRole]] = frozenset({"tier", "default"}) + + +def _router_arm_models(llm_router: "Router | None", router_name: str) -> tuple[tuple[str, str], ...]: + """``(role, model_name)`` for every model the router under evaluation can answer with. + + Drawn from ``strategy_router_dependencies``, the single answer to "what does this router + call", so this cannot disagree with the health check's reading of the same deployment. + Only the roles that SERVE are arms: the classifier and embedding models pick the tier, + they never produce a response anyone judges, so a judge sharing them carries no + self-preference. + + A semantic auto-router keeps its routes in an opaque config blob or a file, so only its + default model is enumerable and the guard below is incomplete for it. That direction is + deliberate: it can miss a collision, never invent one. + + Which tiers a router declares is a property of its config and not of who is calling, so + this lookup is unscoped; what each tier NAME resolves to is the team-dependent half, and + it belongs to the caller that compares them. + """ + deployments: Final = llm_router.get_model_list(model_name=router_name) if llm_router is not None else None + return tuple( + dict.fromkeys( + (dependency.role, dependency.model_name) + for deployment in deployments or () + for dependency in strategy_router_dependencies(deployment["litellm_params"]) + if dependency.role in _JUDGED_ROLES + ) + ) + + +def _judge_collisions_for_team( + llm_router: "Router | None", data: StartShadowEvalRequest, team_id: str | None +) -> tuple[tuple[str, str], ...]: + """``(role, model_name)`` for each arm the judge would also be, as one team's keys see it. + + Both sides resolve under the SAME team, since two names are the same model only for a + caller who can reach both; resolving the judge for one team against an arm for another + invents a collision no request could produce. + """ + judge: Final = judge_target(llm_router, data.judge_model, team_id).models + return tuple( + (role, model) + for role, model in ( + *_router_arm_models(llm_router, data.router_name), + *((("baseline", data.baseline_model),) if data.baseline_model is not None else ()), + ) + if judge & judge_target(llm_router, model, team_id).models + ) + + +def _validate_judge_is_not_a_candidate( + llm_router: "Router | None", data: StartShadowEvalRequest, team_ids: Sequence[str | None] +) -> None: + """Reject a judge that is one of the two arms it grades. + + A judge scores its own output higher than a rival's, so a run whose judge also serves an + arm reports a win rate for that arm that measures the judge rather than the models, and + the whole job's spend buys a result that has to be discarded. Both arms are in scope: the + router answers with a tier or default model in either direction, and a reverse job's + ``baseline_model`` is the fixed arm the router is compared against. + + Names are compared by what would ANSWER them, not by spelling: the shipped default judge + ``anthropic/claude-sonnet-5`` collides with a tier deployment an admin named + ``sonnet-tier``, and an alias collides with its target, neither of which a string + comparison sees. + + A collision for ONE team is a collision for the job, because the verdicts every key + produces land in the same win rates. + """ + collisions: Final = tuple( + dict.fromkeys( + collision for team_id in team_ids for collision in _judge_collisions_for_team(llm_router, data, team_id) + ) + ) + if not collisions: + return + raise HTTPException( + status_code=400, + detail=( + f"judge_model '{data.judge_model}' is also an arm this job would judge: " + + ", ".join(f"{role} model '{model}'" for role, model in collisions) + + ". A judge scores its own answers higher than a rival's, so the win rates would " + "measure the judge; pick a judge that serves neither arm" + ), + ) def _is_unique_violation(error: Exception) -> bool: @@ -1029,9 +1132,6 @@ async def start_shadow_eval( raise HTTPException(status_code=500, detail=CommonProxyErrors.db_not_connected_error.value) if llm_router is None or not _is_configured_pre_routing_strategy(llm_router, data.router_name): raise HTTPException(status_code=400, detail=f"'{data.router_name}' is not a configured auto-router") - _validate_plain_model(llm_router, data.judge_model, "judge_model") - if data.baseline_model is not None: - _validate_plain_model(llm_router, data.baseline_model, "baseline_model") token_rows: Final = await _verification_tokens(prisma_client).find_many( where={"token": {"in": list(data.api_key_ids)}} # mutable-ok: Prisma filter ) @@ -1045,6 +1145,14 @@ async def start_shadow_eval( ), ) + # Every model check below runs once per team the job samples for, since that is the + # identity the shadow and judge calls carry and therefore what the router selects on. + team_ids: Final = tuple(dict.fromkeys(row.team_id for row in token_rows or ())) + _validate_plain_model(llm_router, data.judge_model, "judge_model", team_ids) + if data.baseline_model is not None: + _validate_plain_model(llm_router, data.baseline_model, "baseline_model", team_ids) + _validate_judge_is_not_a_candidate(llm_router, data, team_ids) + # A job whose window passed or whose budget ran out stopped sampling on its own, # but its legs still hold their slots in the per-key, per-direction partial unique index # until stamped; free them so a new eval can start. Sweeping both directions is deliberate. diff --git a/litellm/router.py b/litellm/router.py index 714463e6953..02027c9bd48 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -10810,6 +10810,25 @@ class Router: return returned_models + def resolved_litellm_models(self, model_name: str, team_id: str | None = None) -> tuple[str, ...]: + """The provider model strings `model_name` can actually be served by on this proxy. + + `get_model_list` composes every channel the request path itself uses (exact name, + model_group_alias, routing groups, wildcards), so this answers "which models will + answer a call to this name" rather than "what did the admin call it": the deployment + name is admin-arbitrary, and two names over one provider model are one model. + + Empty when the name resolves to no deployment. That is not the same fact as "the + call will fail" - a provider-qualified public name is served by the SDK with no + deployment behind it - so the fallback for an empty result is the caller's policy, + never this function's. + """ + return tuple( + litellm_model + for deployment in self.get_model_list(model_name=model_name, team_id=team_id) or () + if isinstance(litellm_model := deployment.get("litellm_params", {}).get("model"), str) and litellm_model + ) + def _invalidate_model_group_info_cache(self) -> None: """Invalidate the cached model group info. @@ -12033,10 +12052,7 @@ class Router: resolve_structured_messages, ) - deployments: Final = self.get_model_list(model_name=model) or [] - candidate_models: Final = [ - d["litellm_params"]["model"] for d in deployments if d.get("litellm_params", {}).get("model") - ] + candidate_models: Final = list(self.resolved_litellm_models(model)) metadata_key: Final = self._get_metadata_variable_name_from_kwargs(request_kwargs) metadata: Final = request_kwargs.setdefault(metadata_key, {}) diff --git a/tests/test_litellm/integrations/test_shadow_eval_logger.py b/tests/test_litellm/integrations/test_shadow_eval_logger.py index 4f6fea7b710..5459e545a71 100644 --- a/tests/test_litellm/integrations/test_shadow_eval_logger.py +++ b/tests/test_litellm/integrations/test_shadow_eval_logger.py @@ -1238,3 +1238,36 @@ def _failing_router(): router.get_model_list = MagicMock(return_value=None) router.acompletion = AsyncMock(side_effect=RuntimeError("provider exploded")) return router + + +@pytest.mark.asyncio +async def test_judge_call_resolves_its_arm_under_the_shadowed_keys_team(monkeypatch: pytest.MonkeyPatch) -> None: + """Start-time validation resolves the judge under the key's team, so the dispatch has to + as well or the two disagree about the same name. + + A team-public judge resolves to a real deployment for its own team and to nothing for + anybody else. Choosing the arm without the team sends the literal name to the SDK, which + has never heard of it, so every judge call fails on a job validation just accepted. + """ + import litellm + from litellm.litellm_core_utils.llm_judge import judge_acompletion + + router = litellm.Router( + model_list=[ + { + "model_name": "row_team_a", + "litellm_params": {"model": "anthropic/claude-sonnet-5", "api_key": "fake"}, + "model_info": {"team_id": "team-a", "team_public_model_name": "house-judge"}, + } + ] + ) + router.acompletion = AsyncMock( # pyright: ignore[reportAttributeAccessIssue] # fake the call, not the resolution + return_value={"choices": [{"message": {"content": "router answer"}}]} + ) + sdk = AsyncMock(return_value={"choices": [{"message": {"content": "sdk answer"}}]}) + monkeypatch.setattr(litellm, "acompletion", sdk) + + await judge_acompletion(router, "house-judge", [{"role": "user", "content": "hi"}], team_id="team-a") + + router.acompletion.assert_awaited_once() + sdk.assert_not_called() diff --git a/tests/test_litellm/litellm_core_utils/test_llm_judge.py b/tests/test_litellm/litellm_core_utils/test_llm_judge.py index a0a2311914b..3bcfde76450 100644 --- a/tests/test_litellm/litellm_core_utils/test_llm_judge.py +++ b/tests/test_litellm/litellm_core_utils/test_llm_judge.py @@ -5,11 +5,12 @@ from unittest.mock import AsyncMock, MagicMock import pytest +import litellm from litellm.litellm_core_utils.llm_judge import ( extract_text_from_content, judge_acompletion, + judge_target, parse_json_verdict, - router_resolves_model, ) @@ -46,27 +47,40 @@ def test_extract_text_from_content(content, expected): assert extract_text_from_content(content) == expected -def _router(alias=(), deployments=False) -> MagicMock: - router = MagicMock() - router.model_group_alias = dict.fromkeys(alias, "x") - router.get_model_list = MagicMock( - return_value=[{"litellm_params": {"model": "openai/gpt-4o"}}] if deployments else None +def _router(alias: tuple[str, ...] = (), deployments: bool = False) -> litellm.Router: + """A real Router, so name resolution is the product's own. + + Only the network call is faked: a resolution fake has to be kept in step with every + channel the real one composes, and the one that was here answered a stubbed + `get_model_list` while the code under test asked a different method, so every arm-choice + assertion passed on a truthy Mock. + """ + router = litellm.Router( + model_list=[ + {"model_name": name, "litellm_params": {"model": "openai/gpt-4o", "api_key": "fake"}} + for name in (("gpt-4o",) if deployments else ()) + (("alias-target",) if alias else ()) + ], + model_group_alias=dict.fromkeys(alias, "alias-target"), + ) + router.acompletion = AsyncMock( # pyright: ignore[reportAttributeAccessIssue] # fake only the call, not the resolution + return_value={"choices": [{"message": {"content": "router answer"}}]} ) - router.acompletion = AsyncMock(return_value={"choices": [{"message": {"content": "router answer"}}]}) return router -def test_router_resolves_model_matrix(): - assert router_resolves_model(None, "gpt-4o") is False - assert router_resolves_model(_router(), "gpt-4o") is False - assert router_resolves_model(_router(alias=("gpt-4o",)), "gpt-4o") is True - assert router_resolves_model(_router(deployments=True), "gpt-4o") is True +def test_judge_target_matrix() -> None: + """Every name lands in exactly one of the three outcomes the dispatch branches on.""" + assert judge_target(None, "gpt-4o").via == "sdk" + assert judge_target(_router(), "gpt-4o").via == "sdk" + assert judge_target(_router(alias=("gpt-4o",)), "gpt-4o").via == "router" + assert judge_target(_router(deployments=True), "gpt-4o").via == "router" + assert judge_target(_router(), "not/a real model!").via == "nothing" @pytest.mark.asyncio async def test_judge_acompletion_prefers_router_and_disables_retries(): router = _router(deployments=True) - response = await judge_acompletion(router, "judge-model", [{"role": "user", "content": "hi"}], temperature=0) + response = await judge_acompletion(router, "gpt-4o", [{"role": "user", "content": "hi"}], temperature=0) assert response == {"choices": [{"message": {"content": "router answer"}}]} _, kwargs = router.acompletion.call_args assert kwargs["num_retries"] == 0 @@ -90,3 +104,49 @@ async def test_judge_acompletion_falls_back_to_sdk_for_unconfigured_model(monkey assert sdk.call_args.kwargs["model"] == "anthropic/claude-sonnet-5" assert sdk.call_args.kwargs["num_retries"] == 0 assert sdk.call_args.kwargs["drop_params"] is True + + +@pytest.mark.parametrize( + "model,expected", + [ + ("named-deployment", frozenset({"anthropic/claude-sonnet-5"})), + ("alias-for-it", frozenset({"anthropic/claude-sonnet-5"})), + ("anthropic/claude-sonnet-5", frozenset({"anthropic/claude-sonnet-5"})), + ("anthropic/claude-opus-4-5", frozenset({"anthropic/claude-opus-4-5"})), + ], + ids=["deployment", "alias", "the-public-name-the-deployment-serves", "nothing-configured"], +) +def test_judge_target_identifies_a_name_by_what_would_serve_it(model: str, expected: frozenset[str]) -> None: + """Three spellings of one model must come back as one identity, or a caller comparing + two names by their answering models would call the same model two different ones. + + The last case is the fallback: nothing on the proxy serves it, so the SDK gets the name + verbatim and the name is the identity. + """ + router = litellm.Router( + model_list=[ + { + "model_name": "named-deployment", + "litellm_params": {"model": "anthropic/claude-sonnet-5", "api_key": "fake"}, + } + ], + model_group_alias={"alias-for-it": "named-deployment"}, + ) + + assert judge_target(router, model).models == expected + + +def test_judge_target_without_a_router_is_the_public_name_the_sdk_would_call() -> None: + target = judge_target(None, "anthropic/claude-sonnet-5") + assert (target.via, target.models) == ("sdk", frozenset({"anthropic/claude-sonnet-5"})) + + +def test_judge_target_gives_one_identity_to_a_bare_public_name_and_a_prefixed_deployment() -> None: + """`gpt-4o` and a deployment serving `openai/gpt-4o` are one model, so a judge named the + first must collide with a tier named the second. Comparing the spellings finds nothing + and the job runs with the judge grading itself.""" + router = litellm.Router( + model_list=[{"model_name": "fast-tier", "litellm_params": {"model": "openai/gpt-4o", "api_key": "fake"}}] + ) + + assert judge_target(router, "gpt-4o").models == judge_target(router, "fast-tier").models diff --git a/tests/test_litellm/proxy/management_endpoints/test_auto_router_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_auto_router_endpoints.py index 7e6c4488a7d..f5251fd82d0 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_auto_router_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_auto_router_endpoints.py @@ -809,15 +809,66 @@ VIEWER = UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY, api_ke NON_ADMIN = UserAPIKeyAuth(user_role=LitellmUserRoles.INTERNAL_USER, api_key="sk-user", user_id="user") -def _shadow_router() -> MagicMock: - router = MagicMock() - router.auto_routers = {} - router.complexity_routers = {"my-router": [MagicMock()]} - router.adaptive_routers = {} - router.quality_routers = {} - router.model_group_alias = {} - router.get_model_list = MagicMock(return_value=None) - return router +def _complexity_router_deployment( + model_name: str, tiers: dict[str, str], default: str, classifier: str = "cheap" +) -> dict[str, object]: + return { + "model_name": model_name, + "litellm_params": { + "model": "auto_router/complexity_router", + "complexity_router_default_model": default, + "complexity_router_config": { + "tiers": tiers, + "classifier_type": "llm", + "classifier_llm_config": {"model": classifier}, + "session_affinity": False, + }, + }, + } + + +def _shadow_router() -> Router: + """A real Router, so the endpoint's model checks run against real resolution. + + `sonnet-router` exists to keep the judge-vs-candidate cases honest: its tiers are + deployments named nothing like the shipped default judge, yet one of them serves + `anthropic/claude-sonnet-5`, so only a check that resolves names finds the collision. + `my-router` deliberately serves none of it, since the default judge has to stay valid + for every other test in this file. + """ + return Router( + model_list=[ + {"model_name": "cheap", "litellm_params": {"model": "openai/gpt-4o-mini", "api_key": "fake"}}, + {"model_name": "mid", "litellm_params": {"model": "openai/gpt-4o", "api_key": "fake"}}, + {"model_name": "pricey", "litellm_params": {"model": "openai/o3", "api_key": "fake"}}, + {"model_name": "prefixed-tier", "litellm_params": {"model": "openai/gpt-4o", "api_key": "fake"}}, + {"model_name": "bare-tier", "litellm_params": {"model": "gpt-4o", "api_key": "fake"}}, + {"model_name": "house-sonnet", "litellm_params": {"model": "anthropic/claude-sonnet-5", "api_key": "fake"}}, + { + "model_name": "model_name_team-a_x", + "litellm_params": {"model": "anthropic/claude-sonnet-5", "api_key": "fake"}, + "model_info": {"team_id": "team-a", "team_public_model_name": "house-judge"}, + }, + { + "model_name": "model_name_team-b_y", + "litellm_params": {"model": "anthropic/claude-sonnet-5", "api_key": "fake"}, + "model_info": {"team_id": "team-b", "team_public_model_name": "b-tier"}, + }, + _complexity_router_deployment( + "my-router", {"SIMPLE": "cheap", "MEDIUM": "mid", "COMPLEX": "pricey"}, "mid" + ), + _complexity_router_deployment( + "sonnet-router", {"SIMPLE": "cheap", "MEDIUM": "house-sonnet"}, "cheap" + ), + _complexity_router_deployment( + "classifier-router", {"SIMPLE": "cheap"}, "cheap", classifier="pricey" + ), + _complexity_router_deployment("b-team-router", {"SIMPLE": "cheap", "MEDIUM": "b-tier"}, "cheap"), + _complexity_router_deployment("prefixed-router", {"SIMPLE": "prefixed-tier"}, "prefixed-tier"), + _complexity_router_deployment("bare-router", {"SIMPLE": "bare-tier"}, "bare-tier"), + ], + model_group_alias={"judge-alias": "pricey"}, + ) def _leg_record(**overrides: object) -> MagicMock: @@ -847,22 +898,37 @@ def _leg_record(**overrides: object) -> MagicMock: def _key_record( - token: str = "key-hash", key_alias: str | None = "prod-alpha", key_name: str | None = "sk-...lpha" + token: str = "key-hash", + key_alias: str | None = "prod-alpha", + key_name: str | None = "sk-...lpha", + team_id: str | None = None, ) -> MagicMock: - record = MagicMock(spec=["token", "key_alias", "key_name"]) + record = MagicMock(spec=["token", "key_alias", "key_name", "team_id"]) record.token = token record.key_alias = key_alias record.key_name = key_name + record.team_id = team_id return record -def _shadow_prisma(legs=(), agg_rows=None, by_leg_rows=None, known_keys=("key-hash", "key-hash-2")) -> MagicMock: +def _shadow_prisma( + legs=(), agg_rows=None, by_leg_rows=None, known_keys=("key-hash", "key-hash-2"), key_teams=None +) -> MagicMock: """The job-table fake honours the filters it is handed, so a read that forgets stopped_at sees rows the partial index would have released, one that forgets direction sees the opposite-direction legs a key may hold at the same time, and a group read that matched on a leg id would come back empty.""" prisma = MagicMock() - prisma.db.litellm_verificationtoken.find_many = AsyncMock(return_value=[_key_record(token) for token in known_keys]) + teams: Final = key_teams or {} + + async def find_tokens(*, where): + """Honours the token filter, like the job-table fake below: the endpoint derives the + job's teams from these rows, so a fake returning keys the request never named would + validate against a team no leg of the job runs under.""" + requested = where["token"]["in"] + return [_key_record(t, team_id=teams.get(t)) for t in known_keys if t in requested] + + prisma.db.litellm_verificationtoken.find_many = AsyncMock(side_effect=find_tokens) async def execute_raw(sql: str, *params: object): if "SET stopped_by" in sql: @@ -1022,6 +1088,11 @@ async def test_start_shadow_eval_writes_one_leg_per_key_in_one_statement(monkeyp (ADMIN, {"direction": "reverse", "baseline_model": "my-router"}, (), 400), (ADMIN, {"direction": "reverse", "baseline_model": "not/a real model!"}, (), 400), (ADMIN, {"direction": "reverse", "baseline_model": "openai/gpt-4o", "router_name": "not-a-router"}, (), 400), + (ADMIN, {"judge_model": "pricey"}, (), 400), + (ADMIN, {"judge_model": "mid"}, (), 400), + (ADMIN, {"judge_model": "judge-alias"}, (), 400), + (ADMIN, {"router_name": "sonnet-router"}, (), 400), + (ADMIN, {"direction": "reverse", "baseline_model": "house-sonnet"}, (), 400), ], ids=[ "non-admin", @@ -1034,6 +1105,11 @@ async def test_start_shadow_eval_writes_one_leg_per_key_in_one_statement(monkeyp "router-as-baseline", "unresolvable-baseline", "reverse-still-needs-an-auto-router", + "judge-is-a-tier-model", + "judge-is-the-routers-default-model", + "judge-alias-resolves-to-a-tier-model", + "default-judge-is-what-a-tier-deployment-serves", + "judge-is-what-the-reverse-baseline-serves", ], ) async def test_start_shadow_eval_rejections( @@ -1051,6 +1127,68 @@ async def test_start_shadow_eval_rejections( prisma.db.litellm_shadowevaljob.create_many.assert_not_called() +@pytest.mark.asyncio +@pytest.mark.parametrize( + "request_overrides", + [ + {"judge_model": "house-sonnet"}, + {"judge_model": "anthropic/claude-opus-4-5"}, + {"router_name": "sonnet-router", "judge_model": "pricey"}, + {"router_name": "classifier-router", "judge_model": "pricey"}, + {"direction": "reverse", "baseline_model": "house-sonnet", "judge_model": "openai/gpt-4.1"}, + ], + ids=[ + "judge-serves-a-model-no-tier-serves", + "judge-is-an-unconfigured-public-name", + "judge-is-a-tier-of-a-DIFFERENT-router", + "judge-is-only-the-routers-classifier", + "reverse-judge-differs-from-both-arms", + ], +) +async def test_start_shadow_eval_accepts_a_judge_that_serves_neither_arm( + monkeypatch: pytest.MonkeyPatch, request_overrides: dict[str, object] +) -> None: + """The negative class of the judge-as-candidate gate. + + Without these, a gate that refused every judge would pass the rejection table above + while making the endpoint useless. + """ + import litellm.proxy.proxy_server as proxy_server + + prisma = _shadow_prisma() + monkeypatch.setattr(proxy_server, "prisma_client", prisma) + monkeypatch.setattr(proxy_server, "llm_router", _shadow_router()) + + response = await start_shadow_eval(_start_request(**request_overrides), ADMIN) + + assert response.job_id + prisma.db.litellm_shadowevaljob.create_many.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_start_shadow_eval_names_the_colliding_arm_by_the_deployment_the_admin_configured( + monkeypatch: pytest.MonkeyPatch, +): + """The gate compares what would ANSWER each name, not the names themselves. + + `anthropic/claude-sonnet-5` shares no substring with the deployment `house-sonnet` that + serves it, so a spelling comparison accepts this job and the run's whole budget buys a + result that has to be discarded. The detail has to name the deployment, since that is + the thing the admin can go and change. + """ + import litellm.proxy.proxy_server as proxy_server + + monkeypatch.setattr(proxy_server, "prisma_client", _shadow_prisma()) + monkeypatch.setattr(proxy_server, "llm_router", _shadow_router()) + + with pytest.raises(HTTPException) as exc: + await start_shadow_eval(_start_request(router_name="sonnet-router"), ADMIN) + + assert exc.value.status_code == 400 + assert "house-sonnet" in str(exc.value.detail) + assert "anthropic/claude-sonnet-5" in str(exc.value.detail) + + @pytest.mark.asyncio async def test_start_shadow_eval_names_the_busy_key_and_its_job(monkeypatch: pytest.MonkeyPatch): """A key busy elsewhere blocks the whole start rather than being silently dropped from @@ -1805,3 +1943,136 @@ async def test_two_racing_stops_produce_exactly_one_winner(monkeypatch: pytest.M await stop_shadow_eval_job("job-1", ADMIN) assert exc.value.status_code == 400 assert "already stopped" in exc.value.detail + + +@pytest.mark.asyncio +async def test_start_shadow_eval_finds_a_collision_only_the_keys_team_can_see(monkeypatch: pytest.MonkeyPatch): + """The shadow and judge calls carry the shadowed key's team, so the router selects + deployments with it and an unscoped check answers for a caller that does not exist. + + `house-judge` is team-a's public name for a deployment serving anthropic/claude-sonnet-5, + which is also what the router's MEDIUM tier `house-sonnet` serves. Resolved without the + team it matches no deployment at all, so the judge reads as the literal string, nothing + collides, and the job runs a week producing win rates its own judge authored. + """ + import litellm.proxy.proxy_server as proxy_server + + prisma = _shadow_prisma(key_teams={"key-hash": "team-a"}) + monkeypatch.setattr(proxy_server, "prisma_client", prisma) + monkeypatch.setattr(proxy_server, "llm_router", _shadow_router()) + + with pytest.raises(HTTPException) as exc: + await start_shadow_eval(_start_request(router_name="sonnet-router", judge_model="house-judge"), ADMIN) + + assert exc.value.status_code == 400 + assert "house-sonnet" in str(exc.value.detail) + prisma.db.litellm_shadowevaljob.create_many.assert_not_called() + + +@pytest.mark.asyncio +async def test_start_shadow_eval_refuses_when_only_one_of_several_teams_collides(monkeypatch: pytest.MonkeyPatch): + """Every key's verdicts land in the same win rates, so one team's biased judge is enough + to spoil the job. team-b cannot reach `house-judge` at all; team-a can, and collides.""" + import litellm.proxy.proxy_server as proxy_server + + prisma = _shadow_prisma(key_teams={"key-hash": "team-b", "key-hash-2": "team-a"}) + monkeypatch.setattr(proxy_server, "prisma_client", prisma) + monkeypatch.setattr(proxy_server, "llm_router", _shadow_router()) + + with pytest.raises(HTTPException) as exc: + await start_shadow_eval( + _start_request( + api_key_ids=("key-hash", "key-hash-2"), router_name="sonnet-router", judge_model="house-judge" + ), + ADMIN, + ) + + assert exc.value.status_code == 400 + + +@pytest.mark.asyncio +async def test_start_shadow_eval_sees_a_collision_hidden_behind_the_second_teams_tier( + monkeypatch: pytest.MonkeyPatch, +): + """The arm side is team-scoped too, and the same job is valid or not depending on which + keys it samples for. + + `b-team-router`'s MEDIUM tier is team-b's own deployment, serving the model the judge + `house-sonnet` also serves. A team-a key can never be routed to it, so that job is fine; + add a team-b key and the judge starts grading its own answers. The pair is one test + because either half alone would pass against a check that ignored teams in the direction + it does not exercise. + """ + import litellm.proxy.proxy_server as proxy_server + + monkeypatch.setattr(proxy_server, "llm_router", _shadow_router()) + + monkeypatch.setattr(proxy_server, "prisma_client", _shadow_prisma(key_teams={"key-hash": "team-a"})) + accepted = await start_shadow_eval( + _start_request(router_name="b-team-router", judge_model="house-sonnet"), ADMIN + ) + assert accepted.job_id + + monkeypatch.setattr( + proxy_server, "prisma_client", _shadow_prisma(key_teams={"key-hash": "team-a", "key-hash-2": "team-b"}) + ) + with pytest.raises(HTTPException) as exc: + await start_shadow_eval( + _start_request( + api_key_ids=("key-hash", "key-hash-2"), router_name="b-team-router", judge_model="house-sonnet" + ), + ADMIN, + ) + + assert exc.value.status_code == 400 + assert "b-tier" in str(exc.value.detail) + + +@pytest.mark.asyncio +async def test_start_shadow_eval_matches_a_bare_public_judge_name_to_a_prefixed_tier( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`gpt-4o` and a tier deployment serving `openai/gpt-4o` are one model. + + The judge is not configured on the proxy, so it is served by the SDK under the name + litellm resolves it to; the tier is served by its deployment under the name the admin + configured. Comparing those two spellings finds nothing, and the job runs a week with + the judge grading its own answers, which is the whole defect this endpoint guards. + """ + import litellm.proxy.proxy_server as proxy_server + + prisma = _shadow_prisma() + monkeypatch.setattr(proxy_server, "prisma_client", prisma) + monkeypatch.setattr(proxy_server, "llm_router", _shadow_router()) + + with pytest.raises(HTTPException) as exc: + await start_shadow_eval(_start_request(router_name="prefixed-router", judge_model="gpt-4o"), ADMIN) + + assert exc.value.status_code == 400 + assert "prefixed-tier" in str(exc.value.detail) + prisma.db.litellm_shadowevaljob.create_many.assert_not_called() + + +@pytest.mark.asyncio +async def test_start_shadow_eval_matches_a_prefixed_judge_name_to_a_bare_tier_deployment( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The mirror of the case above, and the reason BOTH sides are normalised. + + An admin may configure a deployment as plain `gpt-4o` and litellm infers the provider. + Normalising only the judge would leave that tier spelled differently from the judge that + is the same model, so the collision would be missed for exactly the configs that spell + the two ends differently, which is every config this guard exists for. + """ + import litellm.proxy.proxy_server as proxy_server + + prisma = _shadow_prisma() + monkeypatch.setattr(proxy_server, "prisma_client", prisma) + monkeypatch.setattr(proxy_server, "llm_router", _shadow_router()) + + with pytest.raises(HTTPException) as exc: + await start_shadow_eval(_start_request(router_name="bare-router", judge_model="openai/gpt-4o"), ADMIN) + + assert exc.value.status_code == 400 + assert "bare-tier" in str(exc.value.detail) + prisma.db.litellm_shadowevaljob.create_many.assert_not_called() diff --git a/tests/test_litellm/test_router.py b/tests/test_litellm/test_router.py index 99ad2699f96..779f0bd8e56 100644 --- a/tests/test_litellm/test_router.py +++ b/tests/test_litellm/test_router.py @@ -10947,3 +10947,38 @@ async def test_router_without_fallback_access_check_attempts_every_config_fallba response = await router.acompletion(model="primary", messages=[{"role": "user", "content": "hi"}]) assert response.choices[0].message.content == "served by secret-fallback" + + +def _resolution_router() -> Router: + return Router( + model_list=[ + {"model_name": "pinned", "litellm_params": {"model": "openai/gpt-4o", "api_key": "sk-test"}}, + {"model_name": "pooled", "litellm_params": {"model": "openai/gpt-4o-mini", "api_key": "sk-test"}}, + {"model_name": "pooled", "litellm_params": {"model": "anthropic/claude-haiku-4-5", "api_key": "sk-test"}}, + {"model_name": "bedrock/*", "litellm_params": {"model": "bedrock/*", "api_key": "sk-test"}}, + ], + model_group_alias={"nickname": "pinned"}, + ) + + +@pytest.mark.parametrize( + "model_name,expected", + [ + ("pinned", ("openai/gpt-4o",)), + ("nickname", ("openai/gpt-4o",)), + ("pooled", ("openai/gpt-4o-mini", "anthropic/claude-haiku-4-5")), + ("bedrock/anthropic.claude-3-5-sonnet", ("bedrock/anthropic.claude-3-5-sonnet",)), + ("never-configured", ()), + ], + ids=["exact-name", "model-group-alias", "every-member-of-a-pool", "wildcard-expands", "resolves-to-nothing"], +) +def test_resolved_litellm_models_answers_through_every_channel_a_request_uses( + model_name: str, expected: tuple[str, ...] +) -> None: + """A caller comparing two names by what serves them needs each channel the request path + composes, since the deployment name an admin picked carries no information on its own. + + `resolves-to-nothing` is the contract that keeps the fallback out of here: an empty + result is not "the call fails", so what to do about it stays each caller's policy. + """ + assert set(_resolution_router().resolved_litellm_models(model_name)) == set(expected)