From b829586071ec1b6beff240f63a7d33cdca331d10 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:19:50 +0000 Subject: [PATCH] fix(routing_groups): own strategy validation in router_utils and cover group swap Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- basedpyright-code-budget.json | 2 +- litellm/proxy/proxy_server.py | 10 +---- litellm/router.py | 31 ++++---------- litellm/router_utils/routing_groups.py | 40 ++++++++++++++----- .../test_router_routing_groups.py | 40 ++++++++++++++++++- type-discipline-budget.json | 2 +- .../routing_groups/RoutingGroupModal.test.tsx | 13 ++++++ .../routing_groups/RoutingGroupModal.tsx | 2 +- 8 files changed, 94 insertions(+), 46 deletions(-) diff --git a/basedpyright-code-budget.json b/basedpyright-code-budget.json index 225a2c04339..a3a849b07af 100644 --- a/basedpyright-code-budget.json +++ b/basedpyright-code-budget.json @@ -105,7 +105,7 @@ "limit": 109 }, "reportUnknownMemberType": { - "limit": 38808 + "limit": 38806 }, "reportUnknownParameterType": { "limit": 19829 diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 622db36e0ab..2675c6a185e 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -6291,10 +6291,7 @@ class ProxyConfig: if raw_groups is None: return router_settings try: - parse_routing_groups( - TypeAdapter(list[RoutingGroup]).validate_python(raw_groups), - validate_strategy=Router._validate_routing_strategy, - ) + parse_routing_groups(TypeAdapter(list[RoutingGroup]).validate_python(raw_groups)) except ValueError as validation_error: verbose_proxy_logger.error( "Ignoring invalid router_settings.routing_groups from config/DB, all other router settings still " @@ -15790,10 +15787,7 @@ async def update_config( if config_info.router_settings is not None: try: - parse_routing_groups( - config_info.router_settings.routing_groups, - validate_strategy=Router._validate_routing_strategy, - ) + parse_routing_groups(config_info.router_settings.routing_groups) except ValueError as validation_error: raise HTTPException(status_code=400, detail={"error": str(validation_error)}) diff --git a/litellm/router.py b/litellm/router.py index 020f0d65eb2..18ce316a821 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -179,7 +179,7 @@ from litellm.router_utils.router_callbacks.track_deployment_metrics import ( increment_deployment_failures_for_current_minute, increment_deployment_successes_for_current_minute, ) -from litellm.router_utils.routing_groups import parse_routing_groups +from litellm.router_utils.routing_groups import parse_routing_groups, validate_routing_strategy from litellm.scheduler import FlowItem, Scheduler from litellm.types.llms.openai import ( AllMessageValues, @@ -1081,19 +1081,7 @@ class Router: @staticmethod def _validate_routing_strategy(routing_strategy: RoutingStrategy | str | None) -> None: - # See: https://github.com/BerriAI/litellm/issues/11330 - valid_strategy_strings: Final = ["simple-shuffle", "lar1"] + [s.value for s in RoutingStrategy] - if routing_strategy is None: - return - is_valid_string: Final = isinstance(routing_strategy, str) and routing_strategy in valid_strategy_strings - is_valid_enum: Final = isinstance(routing_strategy, RoutingStrategy) - if not is_valid_string and not is_valid_enum: - raise ValueError( - f"Invalid routing_strategy: '{routing_strategy}'. " - f"Valid options: {valid_strategy_strings}. " - f"Check 'router_settings.routing_strategy' in your config.yaml " - f"or the 'routing_strategy' parameter if using the Router SDK directly." - ) + validate_routing_strategy(routing_strategy) def _build_strategy_selector( self, @@ -1207,17 +1195,14 @@ class Router: self._replace_routing_groups(()) return - known_model_names: Final = frozenset(m["model_name"] for m in (self.model_list or []) if m.get("model_name")) - groups: Final = parse_routing_groups( - groups_input, - validate_strategy=self._validate_routing_strategy, - known_model_names=known_model_names, - ) + known_model_names: Final = frozenset(m["model_name"] for m in (self.model_list or ()) if m.get("model_name")) + groups: Final = parse_routing_groups(groups_input, known_model_names=known_model_names) + alias_names: Final = frozenset(self.model_group_alias or ()) shadowed_names: Final = tuple( group.group_name for group in groups - if group.group_name in known_model_names or group.group_name in (self.model_group_alias or {}) + if group.group_name in known_model_names or group.group_name in alias_names ) for shadowed_name in shadowed_names: verbose_router_logger.warning( @@ -1230,7 +1215,7 @@ class Router: failures: Final = tuple(outcome for _, outcome in built if isinstance(outcome, ValidationError)) if failures: self._unregister_router_selectors( - [outcome for _, outcome in built if not isinstance(outcome, ValidationError)] + tuple(outcome for _, outcome in built if not isinstance(outcome, ValidationError)) ) raise failures[0] @@ -1246,7 +1231,7 @@ class Router: self, "_group_selectors", {} ) self._unregister_router_selectors( - [sel for selectors in previous_selectors.values() for sel in selectors.values()] + tuple(sel for selectors in previous_selectors.values() for sel in selectors.values()) ) self._routing_groups: dict[str, RoutingGroup] = {group.group_name: group for group, _ in built} diff --git a/litellm/router_utils/routing_groups.py b/litellm/router_utils/routing_groups.py index 4b3f20c5b72..c9b3b205bf1 100644 --- a/litellm/router_utils/routing_groups.py +++ b/litellm/router_utils/routing_groups.py @@ -4,18 +4,36 @@ proxy's config-update endpoint so a config the UI saves cannot be one the runtime refuses to load. """ -from collections.abc import Callable, Sequence +from collections.abc import Sequence from typing import Final from litellm._logging import verbose_router_logger from litellm.types.router import RoutingGroup, RoutingStrategy -ValidateStrategy = Callable[[RoutingStrategy | str | None], None] + +def validate_routing_strategy(routing_strategy: RoutingStrategy | str | None) -> None: + """ + Raises `ValueError` unless `routing_strategy` is a known strategy or None. + + See: https://github.com/BerriAI/litellm/issues/11330 + """ + if routing_strategy is None: + return + + valid_strategy_strings: Final = ("simple-shuffle", "lar1", *(s.value for s in RoutingStrategy)) + is_valid_string: Final = isinstance(routing_strategy, str) and routing_strategy in valid_strategy_strings + is_valid_enum: Final = isinstance(routing_strategy, RoutingStrategy) + if not is_valid_string and not is_valid_enum: + raise ValueError( + f"Invalid routing_strategy: '{routing_strategy}'. " + f"Valid options: {list(valid_strategy_strings)}. " + f"Check 'router_settings.routing_strategy' in your config.yaml " + f"or the 'routing_strategy' parameter if using the Router SDK directly." + ) def parse_routing_groups( groups_input: Sequence[RoutingGroup | dict] | None, - validate_strategy: ValidateStrategy, known_model_names: frozenset[str] = frozenset(), ) -> tuple[RoutingGroup, ...]: """ @@ -35,21 +53,21 @@ def parse_routing_groups( if any(group.group_name == "default" for group in groups): raise ValueError("routing_groups: 'default' is reserved for the implicit fallback group.") - names: Final = [group.group_name for group in groups] - duplicate_names: Final = sorted({name for name in names if names.count(name) > 1}) + names: Final = tuple(group.group_name for group in groups) + duplicate_names: Final = frozenset(name for name in names if names.count(name) > 1) if duplicate_names: - raise ValueError(f"routing_groups: group names must be unique, duplicate group_name '{duplicate_names[0]}'.") + raise ValueError(f"routing_groups: group names must be unique, duplicate group_name '{min(duplicate_names)}'.") for group in groups: - validate_strategy(group.routing_strategy) + validate_routing_strategy(group.routing_strategy) - owners_by_model: Final = { - model_name: tuple(group.group_name for group in groups if model_name in group.models) + owners_by_model: Final = tuple( + (model_name, tuple(group.group_name for group in groups if model_name in group.models)) for model_name in dict.fromkeys(model_name for group in groups for model_name in group.models) - } + ) conflicts: Final = tuple( f"model_name '{model_name}' appears in {' and '.join(repr(owner) for owner in owners)}" - for model_name, owners in owners_by_model.items() + for model_name, owners in owners_by_model if len(owners) > 1 ) if conflicts: diff --git a/tests/test_litellm/router_strategy/test_router_routing_groups.py b/tests/test_litellm/router_strategy/test_router_routing_groups.py index 88d587e6b90..9ff60dc19b7 100644 --- a/tests/test_litellm/router_strategy/test_router_routing_groups.py +++ b/tests/test_litellm/router_strategy/test_router_routing_groups.py @@ -10,7 +10,6 @@ from unittest.mock import patch import pytest from pydantic import ValidationError - import litellm from litellm import Router from litellm.types.router import RoutingGroup, RoutingStrategy @@ -486,6 +485,45 @@ def test_build_strategy_selector_constructs_for_known_strategies(monkeypatch): assert selector is not None +def test_replace_routing_groups_swaps_state_and_drops_old_selectors(monkeypatch): + monkeypatch.setattr(litellm, "callbacks", []) + monkeypatch.setattr(litellm, "input_callback", []) + router = _build_router( + routing_groups=[ + { + "group_name": "fast", + "models": ["filtered-model"], + "routing_strategy": "least-busy", + } + ] + ) + old_selector = router._group_selectors["fast"]["least-busy"] + + replacement = RoutingGroup( + group_name="quality", + models=["other-model"], + routing_strategy="latency-based-routing", + ) + new_selector = router._build_strategy_selector( + strategy=replacement.routing_strategy, + routing_strategy_args={}, + register_callbacks=True, + ) + router._replace_routing_groups(((replacement, new_selector),)) + + assert set(router._routing_groups) == {"quality"} + assert router._model_to_group == {"other-model": "quality"} + assert router._group_selectors == {"quality": {"latency-based-routing": new_selector}} + assert all(c is not old_selector for c in litellm.callbacks) + assert new_selector in litellm.callbacks + + router._replace_routing_groups(()) + assert router._routing_groups == {} + assert router._model_to_group == {} + assert router._group_selectors == {} + assert all(c is not new_selector for c in litellm.callbacks) + + def test_unregister_router_selectors_removes_by_identity(monkeypatch): monkeypatch.setattr(litellm, "callbacks", []) monkeypatch.setattr(litellm, "input_callback", []) diff --git a/type-discipline-budget.json b/type-discipline-budget.json index 4465580657b..399ce06b3a7 100644 --- a/type-discipline-budget.json +++ b/type-discipline-budget.json @@ -3,7 +3,7 @@ "limit": 22733 }, "LIT002": { - "limit": 26864 + "limit": 26860 }, "LIT003": { "limit": 269 diff --git a/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.test.tsx b/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.test.tsx index 3699a57a657..890a61924f2 100644 --- a/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.test.tsx +++ b/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.test.tsx @@ -52,6 +52,7 @@ const renderModal = (overrides: Partial { expect(onSubmit.mock.calls[0][0]).toStrictEqual(expected); }); + it("blocks a model another group already claims", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ groupNameByModel: { "gpt-4o": "cheap" } }); + + await typeName(user, "security"); + await pickModels(user, "gpt-4o"); + await save(user, "Create Group"); + + expect(await screen.findByText(/Already claimed: gpt-4o/)).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + it("describes the selected strategy", async () => { renderModal(); diff --git a/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.tsx b/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.tsx index 9529197576c..88e9100cbd7 100644 --- a/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.tsx +++ b/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.tsx @@ -127,7 +127,7 @@ const RoutingGroupModal: React.FC = ({ control={form.control} name="group_name" label="Group Name" - description="Use this name as the model in API calls — LiteLLM routes the request to one of the group's models." + description="Names the shared routing strategy for these models. Requests still use the model names, not this name." > {({ ref, ...field }) => }