fix(router): tag-aware pre-routing strategy selection for shared model_name - #33691
Conversation
…l_name Complexity/auto/adaptive/quality router registries were keyed by model_name alone, so a second deployment sharing a model_name but carrying different tags was rejected and every request used the first config. This made tag-based routing to distinct provider configs behind one alias impossible, surfacing as 401 'Not allowed to access model due to tags configuration' for the second tag. Each registry now holds a list of tag-scoped strategies and async_pre_routing_hook selects the entry whose tags match the request before classification, falling back to a default-tagged then first-registered entry. A repeat of the same (model_name, tags) pair is still rejected. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 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:
|
|
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Greptile SummaryThis PR fixes a bug where multiple deployments sharing a
Confidence Score: 4/5Safe to merge with minor concerns about the unmatched-tag fallback path and a missing test for it. The registry refactoring and selection logic are mechanically sound — registration, deduplication, and tag-matching all behave correctly. The one behavioral edge worth watching is that a request whose tags match no registered strategy silently falls through to the first-registered entry instead of returning
|
| Filename | Overview |
|---|---|
| litellm/router.py | Core change: adds _select_pre_routing_strategy, _register_pre_routing_strategy, _has_registered_strategy, and _deployment_tags helpers; all four strategy registries now hold list[TaggedPreRoutingStrategy[...]] instead of a scalar value. Selection logic is generally correct but has a subtle case where multiple candidates exist (both a complexity router and its promoted adaptive router share the same tags) — the complexity router always wins due to list ordering, which is the intended behavior. |
| litellm/types/router.py | Adds TaggedPreRoutingStrategy (frozen dataclass, covariant TypeVar, slots=True) and PreRoutingStrategy Protocol (runtime_checkable). Both types are well-constructed; the covariant TypeVar is appropriate for the read-only frozen dataclass. |
| litellm/proxy/proxy_server.py | Three sites updated to iterate through the new tagged list shape: startup state load, adaptive flusher loop, and /adaptive_router/state snapshot endpoint. All three transformations are mechanically correct and preserve prior behavior. |
| tests/test_litellm/router_strategy/test_complexity_router.py | New TestComplexityRouterTagBasedRouting class tests the two-strategy registration, the duplicate-tag rejection, and the happy-path tag-to-strategy dispatch. Uses local heuristic classification (no network calls). Missing a test case for the no-match fallback (when request tags don't match any strategy's tags). |
| tests/router_unit_tests/test_router_helper_utils.py | Two existing tests updated to reflect new list-valued registry: success assertion uses [0].strategy, duplicate test pre-seeds with TaggedPreRoutingStrategy and updates error message regex. Coverage preserved. |
| tests/test_litellm/router_strategy/adaptive_router/test_router_dispatch.py | Accessor pattern updated via new _adaptive(r, name) helper that extracts [0].strategy. All existing test assertions are preserved and the helper correctly reflects the new single-tag-entry shape. |
| tests/test_litellm/router_strategy/adaptive_router/test_state_endpoint.py | Added _entry() helper to wrap _make_router() in TaggedPreRoutingStrategy; all three test scenarios that mock fake_router.adaptive_routers are correctly updated. |
| tests/test_litellm/proxy/proxy_server/test_background_health.py | Mock updated to wrap fake_ar in a single-element TaggedPreRoutingStrategy list. Flush-and-load logic is still exercised correctly. |
| tests/test_litellm/proxy/proxy_server/test_routes_misc.py | Mock updated to wrap the bandit in a TaggedPreRoutingStrategy list. Snapshot endpoint behavior is unchanged. |
Comments Outside Diff (1)
-
tests/test_litellm/router_strategy/test_complexity_router.py, line 706-721 (link)No test for the no-match fallback path
TestComplexityRouterTagBasedRoutingcovers both happy paths ("cn" → "gpt-cn", "us" → "gpt-us") and the duplicate-tag rejection, but doesn't test what happens when the request carries a tag that matches neither registered strategy (e.g.tags=["eu"]). Per_select_pre_routing_strategy, the fallback iscandidates[0].strategy(first registered). Adding a test for that case would lock in the documented fallback behavior and prevent accidental regressions if the selection logic changes.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (1): Last reviewed commit: "test(router): cover tag-scoped pre-routi..." | Re-trigger Greptile
| for tagged in candidates: | ||
| if "default" in tagged.tags: | ||
| return tagged.strategy | ||
| return candidates[0].strategy |
There was a problem hiding this comment.
Unmatched tags fall through to first-registered strategy
When request_tags is non-empty but doesn't match any candidate's tags, the loop exits silently and the code falls through to the "default" check and then candidates[0].strategy. A request tagged ["eu"] against only ["cn"]/["us"] strategies will pre-route through the cn config, then be rejected at the post-routing tag filter — correct overall, but the pre-routing step is doing unnecessary work with a mismatched config. If enable_tag_filtering is off this also means the eu request is silently forwarded through the wrong regional tier without any error. Adding an explicit return None (or an explicit log) when request tags are present but match nothing would make the fallback intentional rather than implicit.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
637fc1f
into
litellm_internal_staging
Relevant issues
Fixes #33655
Linear ticket
Pre-Submission checklist
Type
🐛 Bug Fix
Changes
The pre-routing strategy registries (
auto_routers,complexity_routers,adaptive_routers,quality_routers) were keyed bymodel_namealone with a scalar value:So two deployments sharing a public
model_namebut carrying differenttagscollapsed to one config: registration either kept the first router or raised"... already exists ..."at startup. Because complexity/auto classification runs inasync_pre_routing_hookbefore ordinary deployment tag filtering, a request carrying the second tag was classified/routed through the first deployment's config and then rejected with a 401 (not allowed to access model due to tags configuration) once tag filtering ran.This mirrors the same handling the issue asks for across all four pre-routing families, not just complexity routers.
Each registry now holds a list of tag-scoped strategies:
TaggedPreRoutingStrategy(new frozen dataclass intypes/router.py) pairs a strategy with thetagsit was registered under;PreRoutingStrategy(new protocol) is the shared structural type of the four routers._register_pre_routing_strategycentralizes registration for all four families: a repeated(model_name, tags)pair is still rejected, but distinct tags under onemodel_namenow coexist._select_pre_routing_strategyruns inasync_pre_routing_hookbefore classification and picks the entry whose tags match the request tags (via the existingis_valid_deployment_tag/tag_filtering_match_anysemantics), then adefault-tagged entry, then the first registered — consistent with normal tag-based routing, which only filters later.proxy/proxy_server.py(startup state load,/adaptive_router/statesnapshots, background flusher) iterate the tagged lists.Selection sketch:
Tests:
TestComplexityRouterTagBasedRoutingregisters twosmartcomplexity deployments taggedcn/usand asserts both register, that each request tag resolves to its own tier model throughasync_pre_routing_hook, and that same-(model_name, tags)is still rejected. Existing adaptive/proxy tests updated for the list-valued registry shape. These fail against the old scalar registry (which raised at startup or routed both tags through the first config).Note: this is the broader-scope alternative to #33660 (which fixes complexity routers only); consolidating per maintainer direction.
Final Attestation
Link to Devin session: https://app.devin.ai/sessions/967b3f08af4d47cab5c37f4320a5eac8
Requested by: @krrish-berri-2