fix(router): support tag-based routing for complexity routers sharing a model_name - #33660
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Greptile SummaryThis PR fixes a bug where two
Confidence Score: 3/5The core routing logic and tag-selection helper are correct, but the eviction helper has an over-eviction bug when model_id is None, and the adaptive-router init loop silently ignores the adaptive config of any tagged sibling beyond the first. The null model_id eviction issue is a real defect on a code path exercised by the new feature; deleting any deployment whose model_info has no id would drop all complexity routers with model_id=None under that model_name, not just the intended one. litellm/proxy/management_endpoints/model_management_endpoints.py (_evict_complexity_router's None-id branch) and litellm/router.py (adaptive-router init loop around line 7703)
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/model_management_endpoints.py | Adds _evict_complexity_router helper and wires it into the delete endpoint; has a latent over-eviction bug when model_id is None |
| litellm/router.py | Changes complexity_routers registry to list-per-key, adds _select_complexity_router and _resolve_alias_index; adaptive-init loop silently drops siblings' adaptive config |
| litellm/router_strategy/tag_based_routing.py | Adds select_index_by_tags helper that mirrors existing tag-routing semantics for the single-pick case; logic is correct |
| litellm/router_strategy/complexity_router/complexity_router.py | Adds tags and model_id fields to ComplexityRouter constructor; straightforward and correct |
| tests/test_litellm/router_strategy/test_complexity_router.py | Adds TestComplexityRouterTagBasedRouting covering registration, tag selection, unmatched-tag error, and disabled-filtering fallback; all mock-only, no network calls |
| tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py | Updates existing delete-model test for new list-based registry and adds test_evict_complexity_router_keeps_sibling_tagged_routers; coverage is thorough |
Reviews (1): Last reviewed commit: "fix(router): support tag-based routing f..." | Re-trigger Greptile
| """ | ||
| existing = complexity_routers.get(model_name) | ||
| if not existing: | ||
| return | ||
| remaining = [router for router in existing if router.model_id != model_id] | ||
| if remaining: | ||
| complexity_routers[model_name] = remaining | ||
| else: | ||
| complexity_routers.pop(model_name, None) |
There was a problem hiding this comment.
Null
model_id evicts all unidentified routers
When deployment.model_info is None in init_complexity_router_deployment, model_id is stored as None. If a deployment is later deleted with the same None id, the filter router.model_id != model_id evaluates as router.model_id != None, which is False for every router whose model_id is also None. All of those routers are dropped from remaining and evicted together — not just the one backing the deleted deployment.
In practice model_info.id is auto-generated, but any edge case where it is absent causes a broader-than-intended eviction.
| for model_name, complexity_routers in self.complexity_routers.items(): | ||
| for complexity_router in complexity_routers: | ||
| if not complexity_router.config.adaptive or model_name in self.adaptive_routers: | ||
| continue | ||
| adaptive_router = complexity_router._ensure_adaptive_router() | ||
| if adaptive_router is not None: | ||
| self.adaptive_routers[model_name] = adaptive_router |
There was a problem hiding this comment.
Only first tagged complexity router gets adaptive mode
When two complexity routers share a model_name (the new tag-based scenario) and both have config.adaptive = True, the inner-loop check model_name in self.adaptive_routers will be True after the first router creates its adaptive router. Every subsequent tagged sibling is skipped silently. Since adaptive_routers is a Dict[str, AdaptiveRouter] keyed by model_name (not by tag), only one adaptive router can exist per model_name — so adaptive mode will use the first registered tag's configuration regardless of which tag's complexity router handled the request.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Relevant issues
Fixes #33655
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Type
🐛 Bug Fix
Changes
Two
auto_router/complexity_routerdeployments that share amodel_namebut carry differenttagsused to collapse to a single config.Router.complexity_routerswas aDict[str, ComplexityRouter], soinit_complexity_router_deploymenteither kept only the first-registered router or raised"... already exists ..."at startup. Every request then classified through the first deployment's tier config regardless of the key's tag, so a key carrying the second tag ended up on the wrong provider and got a 401 after model selectionThe registry is now
dict[str, list[ComplexityRouter]]and eachComplexityRouterremembers its owntagsandmodel_id. Selection happens in a newRouter._select_complexity_router, called fromasync_pre_routing_hookbefore classification:select_index_by_tags(new helper intag_based_routing.py) reuses the existing exact-tag semantics:!tagexcludes a candidate, a positive request tag picks the first candidate whose tags match undermatch_any/match-all, an untagged request prefers adefault-tagged candidate, and no match with request tags present surfaces the standard tag-routing error instead of silently using the wrong configBecause the hook swaps
modelfrom the alias to the chosen tier model, aliaslitellm_paramsare now merged from the alias deployment that actually matches the selected router (_resolve_alias_indexmatches onmodel_info.id), not always the first alias, so the correct tags/params flow onto the requestRouter-registry cleanup on deployment delete now evicts only the router backed by the deleted
model_id(_evict_complexity_router) and keeps sibling routers registered under the samemodel_name, dropping the map entry only once the last sibling is gone. Single-router and non-tagged behaviour is unchangedTests:
TestComplexityRouterTagBasedRoutingregisters twosmart-routerdeployments taggedcnandrowand asserts each request tag resolves to its own tier model, that an unmatched tag raises the tag-routing error, and that disabling tag filtering falls back to the first router;test_evict_complexity_router_keeps_sibling_tagged_routerscovers per-model_ideviction. These fail against the old single-router registry (which raised at startup or routed both tags through the first config)Final Attestation
Link to Devin session: https://app.devin.ai/sessions/23a95f953c3140c4a94b75a3313c563d