diff --git a/litellm/router.py b/litellm/router.py index 80d78549138..faabac87fbd 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -1194,11 +1194,11 @@ class Router: ) return valid[0] if valid else None - def _build_model_group_selector(self, strategy: str, args: Mapping[str, object]) -> object | None: + def _build_model_group_selector(self, strategy: str, args: Mapping[str, object]) -> object | TypeError | ValueError: try: return self._build_strategy_selector(strategy=strategy, routing_strategy_args=args) - except (TypeError, ValueError): - return None + except (TypeError, ValueError) as exc: + return exc def _live_model_group_selector_keys(self) -> frozenset[str]: """ @@ -1246,15 +1246,14 @@ class Router: if cached is not None: verbose_router_logger.debug("routing_group=model-info model=%s strategy=%s", model, strategy) return strategy, cached - self._evict_stale_model_group_selectors() built: Final = self._build_model_group_selector(strategy, args) - if built is None: + if built is None or isinstance(built, Exception): _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.", + f"'{strategy}' ({built}); falling back to the routing-group / top-level strategy.", ) return None with self._override_selectors_lock: diff --git a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py index 454849d6430..bb5ad1ca88d 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py @@ -771,6 +771,41 @@ class TestDeleteModelClearsRouterRegistry: assert mock_router.complexity_routers.get("shared-name") is config_router +class TestUpdateDbModelMergesModelInfo: + def test_patch_omitting_routing_strategy_preserves_stored_value(self): + """ + The UI's edit form and any partial PATCH rely on update_db_model merging + model_info non-destructively: keys absent from the patch keep their + stored value instead of being wiped. + """ + import json as json_lib + + from litellm.proxy.management_endpoints.model_management_endpoints import ( + update_db_model, + ) + from litellm.types.router import Deployment, ModelInfo, updateDeployment + + db_model = Deployment( + model_name="quality", + litellm_params={"model": "openai/gpt-4o-mini"}, + model_info={ + "id": "mid-1", + "routing_strategy": "cost-based-routing", + "routing_strategy_args": {"ttl": 60}, + }, + ) + + result = update_db_model( + db_model=db_model, + updated_patch=updateDeployment(model_info=ModelInfo(id="mid-1", mode="chat")), + ) + + stored = json_lib.loads(result["model_info"]) + assert stored["routing_strategy"] == "cost-based-routing" + assert stored["routing_strategy_args"] == {"ttl": 60} + assert stored["mode"] == "chat" + + class TestUpdateModel: """ Tests for the update_model (POST /model/update) handler. 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 223544bca95..c6392b2eccc 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 @@ -19,6 +19,7 @@ from litellm import Router from litellm.router import _warn_model_group_strategy_once from litellm.router_strategy.lowest_cost import LowestCostLoggingHandler from litellm.router_strategy.lowest_latency import LowestLatencyLoggingHandler +from litellm.types.router import Deployment @pytest.fixture(autouse=True) @@ -184,7 +185,17 @@ def test_conflict_winner_is_stable_across_model_list_order(): assert reversed_router._get_routing_context("quality")[0] == "cost-based-routing" -def test_invalid_args_evict_previously_cached_selector(): +def _upsert(router, model_name, model, deployment_id, model_info): + router.upsert_deployment( + Deployment( + model_name=model_name, + litellm_params={"model": model, "api_key": "sk-test", "api_base": "https://example.invalid"}, + model_info={"id": deployment_id, **model_info}, + ) + ) + + +def test_invalid_args_evict_previously_cached_selector(caplog): router = _build_router( [ _deployment( @@ -198,13 +209,20 @@ def test_invalid_args_evict_previously_cached_selector(): _, 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"} + _upsert( + router, + "quality", + "openai/gpt-4o", + "d1", + {"routing_strategy": "latency-based-routing", "routing_strategy_args": {"ttl": "bogus"}}, + ) - strategy, _ = router._get_routing_context("quality") + with caplog.at_level(logging.WARNING, logger="LiteLLM Router"): + 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) + assert any("ttl" in r.getMessage() for r in caplog.records if "cannot initialize strategy" in r.getMessage()) def test_deleting_deployment_evicts_its_selector(): @@ -309,8 +327,13 @@ def test_stale_selector_evicted_when_args_change(): old_keys = {k for k in router._override_selectors if "|" in k} assert len(old_keys) == 2 - for idx in router.model_name_to_deployment_indices["quality"]: - router.model_list[idx]["model_info"]["routing_strategy_args"] = {"ttl": 240} + _upsert( + router, + "quality", + "openai/gpt-4o", + "d1", + {"routing_strategy": "latency-based-routing", "routing_strategy_args": {"ttl": 240}}, + ) _, new_selector = router._get_routing_context("quality") assert new_selector is not old_selector diff --git a/ui/litellm-dashboard/src/components/model_info_view.tsx b/ui/litellm-dashboard/src/components/model_info_view.tsx index d324d26964c..61698355ccf 100644 --- a/ui/litellm-dashboard/src/components/model_info_view.tsx +++ b/ui/litellm-dashboard/src/components/model_info_view.tsx @@ -433,25 +433,13 @@ export default function ModelInfoView({ health_check_model: values.health_check_model, }; } - const formRoutingStrategy = values.routing_strategy ?? ""; - if (formRoutingStrategy !== (localModelData.model_info?.routing_strategy ?? "")) { + if (values.routing_strategy !== undefined || values.routing_strategy_args !== undefined) { updatedModelInfo = { ...updatedModelInfo, - routing_strategy: formRoutingStrategy, + routing_strategy: values.routing_strategy ?? "", + routing_strategy_args: values.routing_strategy_args ? JSON.parse(values.routing_strategy_args) : {}, }; } - if (values.routing_strategy_args !== undefined) { - const parsedArgs = values.routing_strategy_args ? JSON.parse(values.routing_strategy_args) : {}; - if ( - hasRoutingStrategyArgs(parsedArgs) || - hasRoutingStrategyArgs(localModelData.model_info?.routing_strategy_args) - ) { - updatedModelInfo = { - ...updatedModelInfo, - routing_strategy_args: parsedArgs, - }; - } - } } catch (e) { NotificationsManager.fromBackend("Invalid JSON in Model Info"); return;