From dcbff0d7b3553ee783d5d007927e6475ef2ef2e1 Mon Sep 17 00:00:00 2001 From: Mihidum Hettiyahandi <55163074+mihidumh@users.noreply.github.com> Date: Wed, 15 Jul 2026 12:13:54 +1000 Subject: [PATCH] fix(router): scope registry cleanup to the removed deployment's own strategy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (cross-strategy eviction): model_names are only unique per registry, so a complexity router and a quality router may legally share a name — blanket pops across all four registries let deleting one evict the other. Route the pop by the litellm_params.model prefix. Also make the adaptive hook-count assertions baseline-relative so hooks leaked by unrelated tests can't flake them. --- litellm/router.py | 21 +++++++++-- tests/test_litellm/test_router.py | 63 +++++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/litellm/router.py b/litellm/router.py index fc3f6deaa67..3b68769fa29 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -7734,11 +7734,24 @@ class Router: upsert/delete cycle leaks one hook (and a deleted router's hook keeps firing forever). """ - if not (litellm_model or "").startswith("auto_router/"): + litellm_model = litellm_model or "" + if not litellm_model.startswith("auto_router/"): + return + + # scope the pop to the removed deployment's OWN strategy registry — + # model_names are only unique per registry, so a complexity router and + # a quality router may legally share a name; deleting one must not + # evict the other. + if litellm_model.startswith("auto_router/complexity_router"): + self.complexity_routers.pop(model_name, None) + return + if litellm_model.startswith("auto_router/quality_router"): + self.quality_routers.pop(model_name, None) + return + if not litellm_model.startswith("auto_router/adaptive_router"): + # plain auto_router/ (semantic router) + self.auto_routers.pop(model_name, None) return - self.auto_routers.pop(model_name, None) - self.complexity_routers.pop(model_name, None) - self.quality_routers.pop(model_name, None) adaptive_router = self.adaptive_routers.pop(model_name, None) if adaptive_router is not None: diff --git a/tests/test_litellm/test_router.py b/tests/test_litellm/test_router.py index 2380a8f5b20..71bd27df75e 100644 --- a/tests/test_litellm/test_router.py +++ b/tests/test_litellm/test_router.py @@ -5518,22 +5518,77 @@ class TestStrategyRegistryCleanupGuards: ] ) + def test_deleting_one_strategy_does_not_evict_same_named_other_strategy(self): + """model_names are only unique per registry — a complexity router and a + quality router may share a name; deleting one must not evict the other.""" + router = litellm.Router( + model_list=[ + { + "model_name": "backing-model", + "litellm_params": {"model": "gpt-4o-mini", "api_key": "fake-key"}, + } + ] + ) + from litellm.types.router import Deployment, LiteLLM_Params + + tiers = { + "SIMPLE": "backing-model", + "MEDIUM": "backing-model", + "COMPLEX": "backing-model", + "REASONING": "backing-model", + } + router.add_deployment( + Deployment( + model_name="dual-name", + litellm_params=LiteLLM_Params( + model="auto_router/complexity_router", + complexity_router_config={"tiers": tiers}, + complexity_router_default_model="backing-model", + ), + model_info={"id": "dual-complexity-row"}, + ) + ) + router.add_deployment( + Deployment( + model_name="dual-name", + litellm_params=LiteLLM_Params( + model="auto_router/quality_router", + quality_router_config={ + "quality_tiers": {"default": "backing-model"}, + "default_model": "backing-model", + }, + ), + model_info={"id": "dual-quality-row"}, + ) + ) + assert "dual-name" in router.complexity_routers + assert "dual-name" in router.quality_routers + + router.delete_deployment(id="dual-complexity-row") + + assert "dual-name" not in router.complexity_routers + assert "dual-name" in router.quality_routers, ( + "deleting the complexity router must not evict the same-named quality router" + ) + def test_adaptive_router_delete_retires_post_call_hook(self): + baseline = self._count_adaptive_hooks() # tolerate hooks leaked by other tests router = self._adaptive_router() assert "adaptive-test-router" in router.adaptive_routers - assert self._count_adaptive_hooks() == 1 + assert self._count_adaptive_hooks() == baseline + 1 router.delete_deployment(id="adaptive-row") assert "adaptive-test-router" not in router.adaptive_routers - assert self._count_adaptive_hooks() == 0, "deleted adaptive router's hook must be retired" + assert self._count_adaptive_hooks() == baseline, "deleted adaptive router's hook must be retired" def test_adaptive_router_upsert_applies_new_config_without_leaking_hooks(self): from litellm.types.router import Deployment, LiteLLM_Params + baseline = self._count_adaptive_hooks() # tolerate hooks leaked by other tests router = self._adaptive_router() original = router.adaptive_routers["adaptive-test-router"] - assert self._count_adaptive_hooks() == 1 + assert self._count_adaptive_hooks() == baseline + 1 router.upsert_deployment( Deployment( @@ -5553,6 +5608,6 @@ class TestStrategyRegistryCleanupGuards: assert router.adaptive_routers["adaptive-test-router"] is not original, ( "upsert must rebuild the adaptive router with the new config" ) - assert self._count_adaptive_hooks() == 1, ( + assert self._count_adaptive_hooks() == baseline + 1, ( "exactly one hook must remain after upsert (old retired, new registered)" )