From 6df125ff6d42b0455d5a431e33fbfc20925f12cd Mon Sep 17 00:00:00 2001 From: abhi Date: Sat, 15 Aug 2026 11:09:14 -0700 Subject: [PATCH] fix: address CI failures on canonical model resolution Three CI gates failed on the initial push; all three were real: 1. lint (strict-rule budget): BLE001 +5, PERF401 +1 over base. The defensive 'except Exception' catches are intentional (an unplaceable name or a malformed deployment must never fail a request or brick a router), so they now carry '# noqa: BLE001' with justifications, matching the convention already used in router.py and common_request_processing.py. The candidate -spelling loop becomes a generator passed to list.extend (PERF401). Both rules are now back at base parity. 2. code-quality (router_code_coverage): the checker only scans test files whose filename contains 'router', so tests/.../test_canonical_model_resolution.py was invisible to it and both new Router methods read as untested. Renamed to test_router_canonical_model_resolution.py and added direct coverage for _get_canonical_model_index (memoization + fenced build failure). Checker now reports untested_perc: 0.0. 3. proxy-infra (test_route_non_a2a_model_raises_error_if_not_in_router): a real regression. The test drives route_request with a Mock() router, so resolve_canonical_model_name returned a truthy Mock and the hook treated it as a resolved target instead of raising. Both hooks (routing and auth) now require an actual non-empty str before acting on a resolution -- correct hardening independent of the test: a stub or partially-initialised router must never be read as a grant. Full affected suite: 517 passed. basedpyright: new module 0 errors. Co-Authored-By: Claude --- litellm/proxy/auth/auth_checks.py | 6 ++++-- litellm/proxy/route_llm_request.py | 8 ++++--- litellm/router.py | 4 +--- .../canonical_model_resolution.py | 14 +++++++------ ...test_router_canonical_model_resolution.py} | 21 +++++++++++++++++++ 5 files changed, 39 insertions(+), 14 deletions(-) rename tests/test_litellm/router_utils/{test_canonical_model_resolution.py => test_router_canonical_model_resolution.py} (90%) diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index d80232bbab3..8a44a7bab33 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -3332,11 +3332,13 @@ def _can_object_call_model( # allowed ["stale-deployment-name"] from gaining access to a different # deployment via a canonical rewrite. if llm_router and model not in (llm_router.model_group_alias or {}): - canonical_target: Final = llm_router.resolve_canonical_model_name( + canonical_target: Final[object] = llm_router.resolve_canonical_model_name( model=model, request_team_id=team_id, ) - if canonical_target is not None and _check_model_access_helper( + # Require a real model-group name; a non-string from a router stub must + # never be treated as a grant. + if isinstance(canonical_target, str) and canonical_target and _check_model_access_helper( model=canonical_target, llm_router=llm_router, models=models, diff --git a/litellm/proxy/route_llm_request.py b/litellm/proxy/route_llm_request.py index 00500a45360..da75dc9d506 100644 --- a/litellm/proxy/route_llm_request.py +++ b/litellm/proxy/route_llm_request.py @@ -672,11 +672,13 @@ async def route_request( # turn a hard failure into a success, never re-point working traffic. requested_model: Final[object] = data.get("model") # pyright: ignore[reportUnknownMemberType, reportUnknownVariableType] - data is an untyped request dict if llm_router is not None and isinstance(requested_model, str): - canonical_target: Final = llm_router.resolve_canonical_model_name( + canonical_target: Final[object] = llm_router.resolve_canonical_model_name( model=requested_model, request_team_id=team_id, ) - if canonical_target is not None: + # Require a real model-group name: a router stub that returns a + # non-string (e.g. a test double) must not be read as "resolved". + if isinstance(canonical_target, str) and canonical_target: # AND-on-target: the caller must be allowed to call the *resolved* # group. The requested spelling passing the earlier auth check is # not enough -- without this, a key whose allowlist holds only a @@ -698,7 +700,7 @@ async def route_request( valid_token=user_api_key_dict, llm_router=llm_router, ) - except Exception: + except Exception: # noqa: BLE001 # any auth failure declines the rewrite; never widens access target_allowed = False if target_allowed: # Preserve the client's spelling for spend logs / debugging -- diff --git a/litellm/router.py b/litellm/router.py index 2d4f7491803..8f02a5255e1 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -10719,9 +10719,7 @@ class Router: self._canonical_model_index = build_canonical_index( cast("list[DeploymentTypedDict]", self.model_list) ) - except Exception as exc: - # Never let index construction brick a router: degrade to - # 'strict' behaviour instead. + except Exception as exc: # noqa: BLE001 # index construction must never brick a router; degrade to 'strict' verbose_router_logger.error("canonical-resolution: index build failed, disabling feature: %s", exc) self._canonical_model_index = {} self._canonical_model_index_cost_generation = cost_generation diff --git a/litellm/router_utils/canonical_model_resolution.py b/litellm/router_utils/canonical_model_resolution.py index e4e215d019c..ebb4688aeb1 100644 --- a/litellm/router_utils/canonical_model_resolution.py +++ b/litellm/router_utils/canonical_model_resolution.py @@ -88,7 +88,7 @@ def _infer_provider(model: str) -> str | None: """ try: _, custom_llm_provider, _, _ = litellm.get_llm_provider(model=model) - except Exception: + except Exception: # noqa: BLE001 # an unplaceable name simply never resolves; never fail the request return None return custom_llm_provider or None @@ -112,7 +112,7 @@ def canonicalize(model: str) -> tuple[str, str] | None: # is exactly the normalization wanted here. try: stripped, _, _, _ = litellm.get_llm_provider(model=model) - except Exception: + except Exception: # noqa: BLE001 # an unplaceable name simply never resolves; never fail the request return None return (provider, stripped or model) @@ -172,7 +172,7 @@ def build_canonical_index( group_identity[model_group] = None continue group_identity.setdefault(model_group, identity) - except Exception as exc: # pragma: no cover - defensive + except Exception as exc: # noqa: BLE001 # pragma: no cover - a malformed deployment must not abort the index build verbose_router_logger.debug("canonical-resolution: skipping deployment: %s", exc) continue @@ -183,9 +183,11 @@ def build_canonical_index( # Index the canonical spelling plus any dated<->undated sibling the cost # map attests is the same model. spellings: list[str] = [canonical_name] - for candidate in _undated_variants(canonical_name): - if _same_model_per_cost_map(canonical_name, candidate): - spellings.append(candidate) + spellings.extend( + candidate + for candidate in _undated_variants(canonical_name) + if _same_model_per_cost_map(canonical_name, candidate) + ) for dated, entry in litellm.model_cost.items(): if not isinstance(dated, str) or not isinstance(entry, Mapping) or dated in spellings: continue diff --git a/tests/test_litellm/router_utils/test_canonical_model_resolution.py b/tests/test_litellm/router_utils/test_router_canonical_model_resolution.py similarity index 90% rename from tests/test_litellm/router_utils/test_canonical_model_resolution.py rename to tests/test_litellm/router_utils/test_router_canonical_model_resolution.py index 0e282af8516..c9b06888312 100644 --- a/tests/test_litellm/router_utils/test_canonical_model_resolution.py +++ b/tests/test_litellm/router_utils/test_router_canonical_model_resolution.py @@ -231,6 +231,27 @@ class TestRouterResolveCanonicalModelName: ) assert router.resolve_canonical_model_name(DATED) is None + def test_get_canonical_model_index_builds_and_caches(self, anthropic_router: Router): + """The index is built on demand, memoized, and keyed by identity.""" + index = anthropic_router._get_canonical_model_index() + assert index[("anthropic", UNDATED)] == ANTHROPIC_GROUP + # Second call returns the same memoized object (no rebuild). + assert anthropic_router._get_canonical_model_index() is index + + def test_get_canonical_model_index_survives_build_failure( + self, anthropic_router: Router, monkeypatch: pytest.MonkeyPatch + ): + """A failing index build degrades to 'strict', never raises.""" + import litellm.router as router_module + + def boom(*_args: object, **_kwargs: object) -> dict: + raise RuntimeError("cost map exploded") + + monkeypatch.setattr(router_module, "build_canonical_index", boom) + anthropic_router._canonical_model_index = None + assert anthropic_router._get_canonical_model_index() == {} + assert anthropic_router.resolve_canonical_model_name(DATED) is None + def test_index_rebuilds_after_model_list_change(self, anthropic_router: Router): assert anthropic_router.resolve_canonical_model_name(DATED) == ANTHROPIC_GROUP anthropic_router.set_model_list(