From b92b9ae5043991f037d45919d738862c67163a25 Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Thu, 6 Aug 2026 14:00:11 -0700 Subject: [PATCH] fix(router): warn once per offending config and expose routing_strategy_args in the UI --- litellm/router.py | 70 ++++++++++++------- .../test_model_group_routing_strategy.py | 42 +++++++++++ .../add_model/advanced_settings.tsx | 10 +++ .../handle_add_model_submit.test.tsx | 3 + .../add_model/handle_add_model_submit.tsx | 7 ++ .../src/components/model_info_view.tsx | 32 +++++++++ 6 files changed, 139 insertions(+), 25 deletions(-) diff --git a/litellm/router.py b/litellm/router.py index 979ebc7a8d8..4fd105e4f77 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -715,7 +715,7 @@ class Router: self._init_routing_groups(self._routing_groups_input) self._override_selectors: dict[str, Any] = {} self._override_selectors_lock = threading.Lock() - self._warned_model_group_strategy_models: set[str] = set() # mutable-ok: warn-once misconfig registry + self._warned_model_group_strategy_keys: set[tuple[str, str, str]] = set() # mutable-ok: warn-once registry self.access_groups = None ## USAGE TRACKING ## if isinstance(litellm._async_success_callback, list): @@ -1129,25 +1129,45 @@ class Router: _EMPTY_MAPPING: Final[Mapping[str, object]] = MappingProxyType({}) + def _warn_model_group_strategy_once(self, model: str, kind: str, fingerprint: str, message: str) -> None: + """ + Reports a model-group strategy misconfiguration once per + (model, problem kind, offending config). Distinct problem kinds warn + independently, and a changed config re-warns. + """ + warn_key: Final = (model, kind, fingerprint) + if warn_key in self._warned_model_group_strategy_keys: + return + self._warned_model_group_strategy_keys.add(warn_key) + verbose_router_logger.warning("%s", message) + + def _deployment_strategy_entry(self, idx: int) -> tuple[str | None, Mapping[str, object]] | None: + """ + The (normalized strategy, args) a deployment declares via + `model_info.routing_strategy`, or None when unset. An empty string + counts as unset, so a PATCH (which cannot delete a model_info key) can + still clear the field. + """ + info: Final = self.model_list[idx].get("model_info") or self._EMPTY_MAPPING + raw: Final = info.get("routing_strategy") + if not raw: + return None + normalized: Final = self._normalize_strategy(raw) if isinstance(raw, (str, RoutingStrategy)) else None + return normalized, info.get("routing_strategy_args") or self._EMPTY_MAPPING + def _get_model_group_strategy_config(self, model: str) -> tuple[str, Mapping[str, object]] | None: """ 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 model_name so a bad stored value can never - take down that model's traffic. An empty string counts as unset, so a - PATCH (which cannot delete a model_info key) can still clear the field. + 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 configured: Final = tuple( - ( - self._normalize_strategy(raw) if isinstance(raw, (str, RoutingStrategy)) else None, - info.get("routing_strategy_args") or self._EMPTY_MAPPING, - ) - for idx in indices - if (raw := (info := self.model_list[idx].get("model_info") or self._EMPTY_MAPPING).get("routing_strategy")) + entry for idx in indices if (entry := self._deployment_strategy_entry(idx)) is not None ) if not configured: return None @@ -1158,14 +1178,15 @@ class Router: ) has_invalid: Final = len(valid) < len(configured) has_conflict: Final = len(frozenset(strategy for strategy, _ in valid)) > 1 - if (has_invalid or has_conflict) and model not in self._warned_model_group_strategy_models: - self._warned_model_group_strategy_models.add(model) - verbose_router_logger.warning( - "model_info.routing_strategy for model_group '%s' has %s; using '%s'. Supported strategies: %s.", + if has_invalid or has_conflict: + self._warn_model_group_strategy_once( model, - "an unsupported value" if has_invalid else "conflicting values across deployments", - valid[0][0] if valid else self._normalize_strategy(self.routing_strategy), - sorted(self._OVERRIDABLE_ROUTING_STRATEGIES), + "config", + ",".join(str(strategy) for strategy, _ in configured), + f"model_info.routing_strategy for model_group '{model}' has " + f"{'an unsupported value' if has_invalid else 'conflicting values across deployments'}; using " + f"'{valid[0][0] if valid else self._normalize_strategy(self.routing_strategy)}'. " + f"Supported strategies: {sorted(self._OVERRIDABLE_ROUTING_STRATEGIES)}.", ) return valid[0] if valid else None @@ -1207,14 +1228,13 @@ class Router: if selector_key not in self._override_selectors: built: Final = self._build_model_group_selector(strategy, args) if built is None: - if model not in self._warned_model_group_strategy_models: - self._warned_model_group_strategy_models.add(model) - verbose_router_logger.warning( - "model_info.routing_strategy_args for model_group '%s' cannot initialize strategy " - "'%s'; falling back to the routing-group / top-level strategy.", - model, - strategy, - ) + 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) 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 4be76903d9d..a7da8945871 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,48 @@ def test_invalid_args_fall_back_without_failing_traffic(caplog): assert len(warnings) == 1 +def test_config_and_args_warnings_fire_independently(caplog): + router = _build_router( + [ + _deployment( + "quality", + "openai/gpt-4o", + "d1", + {"routing_strategy": "latency-based-routing", "routing_strategy_args": {"ttl": "bogus"}}, + ), + _deployment("quality", "openai/gpt-4o-mini", "d2", {"routing_strategy": "cost-based-routing"}), + ] + ) + with caplog.at_level(logging.WARNING, logger="LiteLLM Router"): + strategy, _ = router._get_routing_context("quality") + router._get_routing_context("quality") + assert strategy == "simple-shuffle" + conflict_warnings = [r for r in caplog.records if "conflicting values" in r.getMessage()] + args_warnings = [r for r in caplog.records if "cannot initialize strategy" in r.getMessage()] + assert len(conflict_warnings) == 1 + assert len(args_warnings) == 1 + + +def test_changed_bad_args_warn_again(): + router = _build_router( + [ + _deployment( + "quality", + "openai/gpt-4o", + "d1", + {"routing_strategy": "latency-based-routing", "routing_strategy_args": {"ttl": "bogus"}}, + ) + ] + ) + router._get_routing_context("quality") + assert len(router._warned_model_group_strategy_keys) == 1 + + for idx in router.model_name_to_deployment_indices["quality"]: + router.model_list[idx]["model_info"]["routing_strategy_args"] = {"ttl": "still-bogus"} + router._get_routing_context("quality") + assert len(router._warned_model_group_strategy_keys) == 2 + + def test_simple_shuffle_with_args_keeps_shuffle_semantics(caplog): router = _build_router( [ diff --git a/ui/litellm-dashboard/src/components/add_model/advanced_settings.tsx b/ui/litellm-dashboard/src/components/add_model/advanced_settings.tsx index e85ee448f95..b5fc8bda755 100644 --- a/ui/litellm-dashboard/src/components/add_model/advanced_settings.tsx +++ b/ui/litellm-dashboard/src/components/add_model/advanced_settings.tsx @@ -193,6 +193,16 @@ const AdvancedSettings: React.FC = ({