mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-17 23:51:30 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
d952e5e87e
commit
6df125ff6d
5 changed files with 39 additions and 14 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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 --
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
Loading…
Add table
Reference in a new issue