From a29bf2791371b2ac7d6c9b84d86c5eaa5cbdb059 Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Thu, 6 Aug 2026 14:07:23 -0700 Subject: [PATCH] fix(router): stable conflict winner, evict selectors on model-list changes, diff UI clears against live state --- litellm/router.py | 65 +++++++++++++------ .../test_model_group_routing_strategy.py | 59 +++++++++++++++++ .../src/components/model_info_view.tsx | 4 +- 3 files changed, 106 insertions(+), 22 deletions(-) diff --git a/litellm/router.py b/litellm/router.py index 4fd105e4f77..0ddce162065 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -1159,15 +1159,21 @@ class Router: """ Reads `model_info.routing_strategy` (+ `routing_strategy_args`) off the deployments of `model`. When deployments of the same model_name disagree, - the first deployment in model_list order wins; invalid or conflicting - values are reported once per offending config so a bad stored value can - never take down that model's traffic. + the configured deployment with the smallest deployment id wins, which is + stable across edits and upserts (those re-append the deployment, so + model_list order is not); invalid or conflicting values are reported + once per offending config so a bad stored value can never take down + that model's traffic. """ indices: Final = self.model_name_to_deployment_indices.get(model) if not indices: return None + ordered: Final = sorted( + indices, + key=lambda idx: str((self.model_list[idx].get("model_info") or self._EMPTY_MAPPING).get("id") or ""), + ) configured: Final = tuple( - entry for idx in indices if (entry := self._deployment_strategy_entry(idx)) is not None + entry for idx in ordered if (entry := self._deployment_strategy_entry(idx)) is not None ) if not configured: return None @@ -1210,6 +1216,19 @@ class Router: if args and strategy != "simple-shuffle" ) + def _evict_stale_model_group_selectors(self) -> None: + """ + Drops cached model-group selectors that no deployment's `model_info` + strategy config references anymore (after edits, deletions, or args + going invalid), unregistering them from litellm's callback lists. + """ + if not getattr(self, "_override_selectors", None): + return + live_keys: Final = self._live_model_group_selector_keys() + with self._override_selectors_lock: + stale: Final = tuple(k for k in self._override_selectors if "|" in k and k not in live_keys) + self._unregister_router_selectors(tuple(self._override_selectors.pop(k) for k in stale)) + def _resolve_model_group_context( self, model: str, strategy: str, args: Mapping[str, object] ) -> tuple[str, object | None] | None: @@ -1225,23 +1244,27 @@ class Router: return strategy, self._get_override_strategy_selector(strategy) selector_key: Final = f"{strategy}|{json.dumps(args, sort_keys=True, default=str)}" with self._override_selectors_lock: - if selector_key not in self._override_selectors: - built: Final = self._build_model_group_selector(strategy, args) - if built is None: - self._warn_model_group_strategy_once( - model, - "args", - selector_key, - f"model_info.routing_strategy_args for model_group '{model}' cannot initialize strategy " - f"'{strategy}'; falling back to the routing-group / top-level strategy.", - ) - return None - live_keys: Final = self._live_model_group_selector_keys() - stale: Final = tuple(k for k in self._override_selectors if "|" in k and k not in live_keys) - self._unregister_router_selectors(tuple(self._override_selectors.pop(k) for k in stale)) - self._override_selectors[selector_key] = built + cached: Final = self._override_selectors.get(selector_key) + if cached is not None: verbose_router_logger.debug("routing_group=model-info model=%s strategy=%s", model, strategy) - return strategy, self._override_selectors[selector_key] + return strategy, cached + self._evict_stale_model_group_selectors() + built: Final = self._build_model_group_selector(strategy, args) + if built is None: + self._warn_model_group_strategy_once( + model, + "args", + selector_key, + f"model_info.routing_strategy_args for model_group '{model}' cannot initialize strategy " + f"'{strategy}'; falling back to the routing-group / top-level strategy.", + ) + return None + with self._override_selectors_lock: + winner: Final = self._override_selectors.setdefault(selector_key, built) + if winner is not built: + self._unregister_router_selectors((built,)) + verbose_router_logger.debug("routing_group=model-info model=%s strategy=%s", model, strategy) + return strategy, winner def _get_routing_context(self, model: str, request_kwargs: dict | None = None) -> tuple[str | None, Any | None]: """ @@ -8139,6 +8162,7 @@ class Router: verbose_router_logger.debug("\nInitialized Model List %s", self.get_model_names()) self.model_names = {m["model_name"] for m in model_list} + self._evict_stale_model_group_selectors() # Note: model_name_to_deployment_indices is already built incrementally # by _create_deployment -> _add_model_to_list_and_index_map @@ -8413,6 +8437,7 @@ class Router: self.team_public_model_names = frozenset( public_model_name for _, public_model_name in self.team_model_to_deployment_indices ) + self._evict_stale_model_group_selectors() for team_id in list(self.team_pattern_routers.keys()): team_pattern_router = self.team_pattern_routers[team_id] diff --git a/tests/test_litellm/router_strategy/test_model_group_routing_strategy.py b/tests/test_litellm/router_strategy/test_model_group_routing_strategy.py index a7da8945871..c3f7e9b8c10 100644 --- a/tests/test_litellm/router_strategy/test_model_group_routing_strategy.py +++ b/tests/test_litellm/router_strategy/test_model_group_routing_strategy.py @@ -159,6 +159,65 @@ def test_invalid_args_fall_back_without_failing_traffic(caplog): assert len(warnings) == 1 +def test_conflict_winner_is_stable_across_model_list_order(): + ordered = _build_router( + [ + _deployment("quality", "openai/gpt-4o", "a-first", {"routing_strategy": "cost-based-routing"}), + _deployment("quality", "openai/gpt-4o-mini", "b-second", {"routing_strategy": "latency-based-routing"}), + ] + ) + reversed_router = _build_router( + [ + _deployment("quality", "openai/gpt-4o-mini", "b-second", {"routing_strategy": "latency-based-routing"}), + _deployment("quality", "openai/gpt-4o", "a-first", {"routing_strategy": "cost-based-routing"}), + ] + ) + assert ordered._get_routing_context("quality")[0] == "cost-based-routing" + assert reversed_router._get_routing_context("quality")[0] == "cost-based-routing" + + +def test_invalid_args_evict_previously_cached_selector(): + router = _build_router( + [ + _deployment( + "quality", + "openai/gpt-4o", + "d1", + {"routing_strategy": "latency-based-routing", "routing_strategy_args": {"ttl": 120}}, + ) + ] + ) + _, old_selector = router._get_routing_context("quality") + assert any("|" in k for k in router._override_selectors) + + for idx in router.model_name_to_deployment_indices["quality"]: + router.model_list[idx]["model_info"]["routing_strategy_args"] = {"ttl": "bogus"} + + strategy, _ = router._get_routing_context("quality") + assert strategy == "simple-shuffle" + assert not any("|" in k for k in router._override_selectors) + assert all(c is not old_selector for c in litellm.callbacks) + + +def test_deleting_deployment_evicts_its_selector(): + router = _build_router( + [ + _deployment( + "quality", + "openai/gpt-4o", + "d1", + {"routing_strategy": "latency-based-routing", "routing_strategy_args": {"ttl": 120}}, + ) + ] + ) + _, selector = router._get_routing_context("quality") + assert any("|" in k for k in router._override_selectors) + + router.delete_deployment(id="d1") + assert not any("|" in k for k in router._override_selectors) + assert all(c is not selector for c in litellm.callbacks) + + def test_config_and_args_warnings_fire_independently(caplog): router = _build_router( [ diff --git a/ui/litellm-dashboard/src/components/model_info_view.tsx b/ui/litellm-dashboard/src/components/model_info_view.tsx index d3210c4e7dc..e20b64260aa 100644 --- a/ui/litellm-dashboard/src/components/model_info_view.tsx +++ b/ui/litellm-dashboard/src/components/model_info_view.tsx @@ -429,7 +429,7 @@ export default function ModelInfoView({ }; } const formRoutingStrategy = values.routing_strategy ?? ""; - if (formRoutingStrategy !== (modelData.model_info?.routing_strategy ?? "")) { + if (formRoutingStrategy !== (localModelData.model_info?.routing_strategy ?? "")) { updatedModelInfo = { ...updatedModelInfo, routing_strategy: formRoutingStrategy, @@ -437,7 +437,7 @@ export default function ModelInfoView({ } if (values.routing_strategy_args !== undefined) { const parsedArgs = values.routing_strategy_args ? JSON.parse(values.routing_strategy_args) : {}; - const storedArgs = modelData.model_info?.routing_strategy_args ?? {}; + const storedArgs = localModelData.model_info?.routing_strategy_args ?? {}; if (JSON.stringify(parsedArgs) !== JSON.stringify(storedArgs)) { updatedModelInfo = { ...updatedModelInfo,