scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-12 23:01:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 87

fix(quality_router): resolve Greptile review feedback

Browse source 
Addresses four P1 findings from PR review plus test coverage:

1. set_model_list missing quality_routers reset
   - Hot-reloading the Router would leave stale QualityRouter instances
     pointing at the old model_list. `set_model_list` now clears
     `self.quality_routers` alongside the other indices.

2. Round-down fallback before default_model
   - `_resolve_model_for_quality_tier` now rounds DOWN to the closest
     lower tier after round-up fails, before falling back to
     `default_model`. Degrades gracefully rather than jumping straight
     off-tier.

3. RoutingPreferences validation bypass
   - `_build_tier_index` now instantiates `RoutingPreferences(**prefs)`
     so invalid shapes (e.g. non-int quality_tier) raise a clear
     ValueError instead of silently succeeding.

4. Config-ordering dependency
   - `_tier_to_models` is now built lazily on first access. Previously,
     eager construction in `__init__` meant a QualityRouter deployment
     had to appear AFTER all its referenced models in config.yaml,
     because `Router._create_deployment` populates `model_list`
     incrementally. Any `available_models` defined after the router
     entry would silently be reported as missing.

Also adds 6 new tests covering each fix:
- test_invalid_quality_tier_type_raises_clear_error
- test_router_can_be_instantiated_before_its_targets_exist
- test_set_model_list_clears_quality_routers_registry
- test_rounds_down_when_no_higher_tier_exists
- test_rounds_down_prefers_closest_lower_tier
- test_prefers_round_up_over_round_down

Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
This commit is contained in:
Krrish Dholakia 2026-04-18 11:45:50 -07:00
parent 671551b039
commit 69f9de727c
3 changed files with 233 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
litellm/router.py
View file

@ -7025,6 +7025,9 @@ class Router:
self.model_id_to_deployment_index_map = {} # Reset the index
self.model_name_to_deployment_indices = {} # Reset the model_name index
self.team_model_to_deployment_indices = {} # Reset the team_model index
# Reset per-strategy router registries so hot-reload doesn't leave
# stale routers pointing at the old model_list.
self.quality_routers = {}
self._invalidate_model_group_info_cache()
self._invalidate_access_groups_cache()
# we add api_base/api_key each model so load balancing between azure/gpt on api_base1 and api_base2 works

101
litellm/router_strategy/quality_router/quality_router.py
View file

@ -24,7 +24,7 @@ from litellm.router_strategy.complexity_router.complexity_router import (
ComplexityRouter,
)
from .config import QualityRouterConfig
from .config import QualityRouterConfig, RoutingPreferences
if TYPE_CHECKING:
from litellm.router import Router
@ -75,16 +75,27 @@ class QualityRouter(CustomLogger):
self._model_cost: Dict[str, Optional[float]] = {}
self._model_order: Dict[str, Optional[int]] = {}
# Pre-built tier → models index for O(1) tier resolution.
self._tier_to_models: Dict[int, List[str]] = self._build_tier_index()
# Tier → models index. Built lazily on first access so the QualityRouter
# deployment does NOT need to appear after all its referenced models in
# the config — when `_build_tier_index` runs eagerly in `__init__`, the
# router instance's `model_list` is still being assembled incrementally
# by `_create_deployment`, and any `available_models` defined AFTER the
# router entry in config.yaml would silently be reported as missing.
self._tier_to_models_cache: Optional[Dict[int, List[str]]] = None
verbose_router_logger.debug(
f"QualityRouter initialized for {model_name} with "
f"available_models={self.config.available_models}, "
f"default_model={self.config.default_model}, "
f"tier_index={self._tier_to_models}"
f"default_model={self.config.default_model}"
)
@property
def _tier_to_models(self) -> Dict[int, List[str]]:
"""Lazy tier→models index; built on first access."""
if self._tier_to_models_cache is None:
self._tier_to_models_cache = self._build_tier_index()
return self._tier_to_models_cache
def _get_routing_preferences(self, deployment: Any) -> Optional[Dict[str, Any]]:
"""
Extract litellm_routing_preferences from a deployment, handling both
@ -156,38 +167,43 @@ class QualityRouter(CustomLogger):
if name is None or name not in available:
continue
prefs = self._get_routing_preferences(deployment)
if prefs is None:
raw_prefs = self._get_routing_preferences(deployment)
if raw_prefs is None:
raise ValueError(
f"QualityRouter: model '{name}' is listed in available_models "
f"but has no model_info.litellm_routing_preferences"
)
# Accept dict or Pydantic-shaped prefs.
if isinstance(prefs, dict):
tier = prefs.get("quality_tier")
keywords = prefs.get("keywords") or []
order = prefs.get("order")
else:
tier = getattr(prefs, "quality_tier", None)
keywords = getattr(prefs, "keywords", None) or []
order = getattr(prefs, "order", None)
if tier is None:
# Validate via the Pydantic model so we get a clear error for
# missing quality_tier, wrong types, etc. This also means
# `RoutingPreferences` is the single source of truth for the
# accepted shape — readers relied on raw dicts before.
try:
if isinstance(raw_prefs, RoutingPreferences):
prefs = raw_prefs
elif isinstance(raw_prefs, dict):
prefs = RoutingPreferences(**raw_prefs)
else:
# A Pydantic object of some other shape — coerce via its dict.
prefs = RoutingPreferences(
**(
raw_prefs.model_dump()
if hasattr(raw_prefs, "model_dump")
else dict(raw_prefs)
)
)
except Exception as e:
raise ValueError(
f"QualityRouter: model '{name}' has litellm_routing_preferences "
f"but no quality_tier field"
)
f"QualityRouter: model '{name}' has invalid "
f"litellm_routing_preferences: {e}"
) from e
tier_int = int(tier)
tier_int = int(prefs.quality_tier)
tier_to_models.setdefault(tier_int, []).append(name)
self._model_keywords[name] = [str(k).lower() for k in keywords if k]
self._model_keywords[name] = [str(k).lower() for k in prefs.keywords if k]
self._model_quality[name] = tier_int
self._model_cost[name] = self._get_deployment_input_cost(deployment)
try:
self._model_order[name] = int(order) if order is not None else None
except (TypeError, ValueError):
self._model_order[name] = None
self._model_order[name] = prefs.order
seen[name] = True
missing = [name for name, found in seen.items() if not found]
@ -228,6 +244,10 @@ class QualityRouter(CustomLogger):
3. input_cost_per_token ASC (unpriced = +inf so priced wins)
4. model_name ASC (deterministic stability)
"""
# Touch the lazy index so `_model_keywords` / `_model_quality` /
# `_model_cost` / `_model_order` are populated.
_ = self._tier_to_models
text = user_message.lower()
matches: List[Tuple[str, str]] = [] # (model_name, matched_keyword)
@ -258,16 +278,29 @@ class QualityRouter(CustomLogger):
Strategy:
1. Exact tier match → first model registered at that tier.
2. Otherwise round up to the next higher tier that has a model.
3. Otherwise fall back to `config.default_model`.
2. Round UP to the next higher tier that has a model (closer to a
request we might lack capacity for).
3. Round DOWN to the closest lower tier that has a model (degrade
gracefully instead of jumping straight to `default_model`,
which may be off-tier).
4. Fall back to `config.default_model`.
5. Otherwise raise.
"""
if tier in self._tier_to_models and self._tier_to_models[tier]:
return self._tier_to_models[tier][0]
tier_index = self._tier_to_models
if tier in tier_index and tier_index[tier]:
return tier_index[tier][0]
higher_tiers = sorted(t for t in self._tier_to_models if t > tier)
# Round up.
higher_tiers = sorted(t for t in tier_index if t > tier)
for t in higher_tiers:
if self._tier_to_models[t]:
return self._tier_to_models[t][0]
if tier_index[t]:
return tier_index[t][0]
# Round down — closest lower tier first.
lower_tiers = sorted((t for t in tier_index if t < tier), reverse=True)
for t in lower_tiers:
if tier_index[t]:
return tier_index[t][0]
if self.config.default_model:
return self.config.default_model

172
tests/test_litellm/router_strategy/test_quality_router.py
View file

@ -137,13 +137,17 @@ class TestTierIndex:
router = MagicMock()
router.model_list = ml
# Construction succeeds (tier index is lazy); the error surfaces on
# first use so the router entry doesn't have to appear after all of
# its referenced models in config.yaml.
qr = QualityRouter(
model_name="qr",
litellm_router_instance=router,
default_model="haiku",
quality_router_config={"available_models": ["haiku", "sonnet"]},
)
with pytest.raises(ValueError, match="sonnet"):
QualityRouter(
model_name="qr",
litellm_router_instance=router,
default_model="haiku",
quality_router_config={"available_models": ["haiku", "sonnet"]},
)
_ = qr._tier_to_models
# ─── Resolve model for quality tier ─────────────────────────────────────────
@ -173,8 +177,9 @@ class TestResolveModelForQualityTier:
assert qr._resolve_model_for_quality_tier(2) == "opus"
def test_falls_back_to_default_when_nothing_higher_exists(self):
# Only tier 1 available. Asking for tier 4 should fall back to default.
def test_rounds_down_when_no_higher_tier_exists(self):
# Only tier 1 available. Asking for tier 4 rounds up (nothing), then
# rounds DOWN to the closest lower tier — tier 1.
spec = [{"model_name": "haiku", "quality_tier": 1}]
router = MagicMock()
router.model_list = _make_model_list(spec)
@ -186,7 +191,156 @@ class TestResolveModelForQualityTier:
quality_router_config={"available_models": ["haiku"]},
)
assert qr._resolve_model_for_quality_tier(4) == "emergency-default"
assert qr._resolve_model_for_quality_tier(4) == "haiku"
def test_rounds_down_prefers_closest_lower_tier(self):
# Available: 1, 2. Asking for 4 rounds down to tier 2 (not tier 1).
spec = [
{"model_name": "haiku", "quality_tier": 1},
{"model_name": "sonnet", "quality_tier": 2},
]
router = MagicMock()
router.model_list = _make_model_list(spec)
qr = QualityRouter(
model_name="qr",
litellm_router_instance=router,
default_model="emergency-default",
quality_router_config={"available_models": ["haiku", "sonnet"]},
)
assert qr._resolve_model_for_quality_tier(4) == "sonnet"
def test_prefers_round_up_over_round_down(self):
# Available: 1, 3. Asking for 2 rounds UP to 3, not DOWN to 1.
spec = [
{"model_name": "haiku", "quality_tier": 1},
{"model_name": "opus", "quality_tier": 3},
]
router = MagicMock()
router.model_list = _make_model_list(spec)
qr = QualityRouter(
model_name="qr",
litellm_router_instance=router,
default_model="emergency-default",
quality_router_config={"available_models": ["haiku", "opus"]},
)
assert qr._resolve_model_for_quality_tier(2) == "opus"
# ─── RoutingPreferences validation ─────────────────────────────────────────
class TestRoutingPreferencesValidation:
def test_invalid_quality_tier_type_raises_clear_error(self):
# quality_tier must be an int — pass a non-coercible string.
ml = [
{
"model_name": "haiku",
"litellm_params": {"model": "openai/gpt-4o-mini"},
"model_info": {
"id": "id-haiku",
"litellm_routing_preferences": {"quality_tier": "not-an-int"},
},
}
]
router = MagicMock()
router.model_list = ml
qr = QualityRouter(
model_name="qr",
litellm_router_instance=router,
default_model="haiku",
quality_router_config={"available_models": ["haiku"]},
)
with pytest.raises(ValueError, match="invalid litellm_routing_preferences"):
_ = qr._tier_to_models
# ─── Config-ordering independence (lazy index build) ───────────────────────
class TestConfigOrderingIndependence:
def test_router_can_be_instantiated_before_its_targets_exist(self):
# Build a router instance whose referenced model_list is EMPTY at
# construction time (simulating a config where the router entry
# appears before its target deployments). The tier index must not be
# built eagerly — it's deferred until first use.
router = MagicMock()
router.model_list = [] # <- targets haven't been added yet
qr = QualityRouter(
model_name="qr",
litellm_router_instance=router,
default_model="haiku",
quality_router_config={"available_models": ["haiku", "sonnet", "opus"]},
)
# Now the targets come online. This mirrors the incremental add by
# `Router._create_deployment`.
router.model_list = _make_model_list(
[
{"model_name": "haiku", "quality_tier": 1},
{"model_name": "sonnet", "quality_tier": 2},
{"model_name": "opus", "quality_tier": 3},
]
)
# First access triggers the index build and sees the full list.
assert qr._tier_to_models == {
1: ["haiku"],
2: ["sonnet"],
3: ["opus"],
}
# ─── Router.set_model_list resets quality_routers (hot reload) ─────────────
class TestSetModelListResetsQualityRouters:
def test_set_model_list_clears_quality_routers_registry(self):
from litellm.router import Router
router = Router(
model_list=[
{
"model_name": "haiku",
"litellm_params": {
"model": "openai/gpt-4o-mini",
"api_key": "sk-test",
},
"model_info": {"litellm_routing_preferences": {"quality_tier": 1}},
},
{
"model_name": "my-qr",
"litellm_params": {
"model": "auto_router/quality_router",
"quality_router_default_model": "haiku",
"quality_router_config": {"available_models": ["haiku"]},
},
},
]
)
assert "my-qr" in router.quality_routers
# Hot-reload with a new model_list that doesn't define the router.
router.set_model_list(
[
{
"model_name": "haiku",
"litellm_params": {
"model": "openai/gpt-4o-mini",
"api_key": "sk-test",
},
}
]
)
# Stale router from before must be cleared.
assert "my-qr" not in router.quality_routers
# ─── Pre-routing hook ───────────────────────────────────────────────────────