feat(router): make routing groups callable as virtual models and list them in /v1/models

[tin-berri]

TLDR

Problem this solves:

• Routing group names were never callable as models
• The Create Group modal has promised exactly that since day one
• Groups never appeared in /v1/models, so Claude Code and Codex discovery cannot surface them
• A group over single-deployment model names did nothing at all

How it solves it:

• model=<group_name> now routes across the union of member deployments
• The group's own strategy picks among them
• Group names appear in /v1/models and are grantable on keys and teams
• Rides the model_group_alias machinery instead of a parallel mechanism

User Flow

Before, creating a routing group gave you a name you could not use.

1. Open Router Settings > Routing Groups, create group claude-quality over claude-fast and claude-smart with cost-based routing
2. GET /v1/models with your key: claude-quality is not in the list, so Claude Code's model picker (with gateway discovery on) never shows it
3. POST /v1/chat/completions with "model": "claude-quality": 400 "Invalid model name passed in"
4. The modal said "Use this name as the model in API calls", so step 3 reads like a gateway bug

After, the group behaves like the modal always claimed.

1. Create the same group in Router Settings > Routing Groups
2. GET /v1/models: claude-quality is listed, and Claude Code or Codex discovery shows it in the picker
3. POST /v1/chat/completions (or /v1/messages, /v1/responses) with "model": "claude-quality": 200, and the response's model field says claude-quality
4. Repeat the call: the gateway keeps picking the cheapest member deployment, and the spend page attributes rows to claude-quality with the actual backend model recorded per row

Relevant issues

• fix(router): resolve model_group_alias before pre-routing hook lookups #34311 and fix(router): resolve model_group_alias in pre-routing hook #27540 fix model_group_alias resolution ordering on the async path; group resolution here lives inside _common_checks_available_deployment, which both paths share, so it does not inherit that bug
• feat(router): per-group allowed_fails, cooldown_time, and health-check routing via routing_groups #33965 extends routing groups with per-group reliability settings and touches the same init path; flagging for merge coordination

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

• I have added meaningful tests
• My PR passes all CI/CD checks (e.g., lint, format, unit tests)
• My PR's scope is as isolated as possible; it only solves 1 specific problem
• I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

Live proxy on port 4020 (proxy_cli.py --config qa_callgroups_config.yaml --detailed_debug --use_v2_migration_resolver, DB-backed), real Bedrock calls. Before captured at b0fac57 (unmodified base), after at e7905f8, identical commands. Config: two single-deployment model names (claude-fast on haiku, claude-smart on opus, per-deployment cost overrides since the lowest-cost selector prices bedrock/-prefixed names at its 10.0 default) and one group:

router_settings:
  routing_strategy: simple-shuffle
  routing_groups:
    - group_name: claude-quality
      models: [claude-fast, claude-smart]
      routing_strategy: cost-based-routing

Before (b0fac57):

$ curl -s localhost:4020/v1/models -H "Authorization: Bearer sk-qa..." | jq -r '[.data[].id]|sort'
["claude-fast", "claude-smart"]
$ curl -s -o /dev/null -w "%{http_code}" localhost:4020/v1/chat/completions ... -d '{"model":"claude-quality",...}'
400            # same 400 on /v1/messages and /v1/responses

After (e7905f8):

$ curl -s localhost:4020/v1/models -H "Authorization: Bearer sk-qa..." | jq -r '[.data[].id]|sort'
["claude-fast", "claude-quality", "claude-smart"]
$ for i in 1 2 3 4; do curl -s localhost:4020/v1/chat/completions ... -d '{"model":"claude-quality","messages":[{"role":"user","content":"say ok"}],"max_tokens":5}'; done
model=claude-quality, 200 x4
$ curl -s localhost:4020/v1/messages ... -d '{"model":"claude-quality",...}'     -> type=message
$ curl -s localhost:4020/v1/responses ... -d '{"model":"claude-quality",...}'    -> status=completed
$ grep "get_available_deployment for model: claude-quality" qa_callgroups.log | grep -o "'model': 'bedrock/[^']*'" | sort | uniq -c
   6 'model': 'bedrock/us.anthropic.claude-haiku-4-5-20251001-v1:0'   # cost-based always picked the cheap member across BOTH model names
$ grep "routing_group=" qa_callgroups.log | tail -1
routing_group=claude-quality model=claude-quality strategy=cost-based-routing

Authz is symmetric for a scoped key (/key/generate with {"models": ["claude-quality"]}):

$ curl -s localhost:4020/v1/models -H "Authorization: Bearer <scoped>" | jq -r '[.data[].id]'
["claude-quality"]                       # only the group is listed
$ ... -d '{"model":"claude-quality",...}'   -> 200
$ ... -d '{"model":"claude-fast",...}'      -> 403   # member not granted, member-direct denied

Spend rows attribute the group with the real backend recorded per row:

$ psql ... -c 'SELECT model, model_group, LEFT(model_id,12) FROM "LiteLLM_SpendLogs" WHERE model_group=$$claude-quality$$ LIMIT 3;'
bedrock/us.anthropic.claude-haiku-4-5-20251001-v1:0 | claude-quality | 95ac109effb5   (x3)

The Models + Endpoints admin table stays deployment-only (/v2/model/info returns claude-fast, claude-smart; no group row), and member-direct calls behave exactly as before.

UI proof, captured on the live rig above. Router Settings > Routing Groups showing the claude-quality group:

The Create Group modal, whose "Use this name as the model in API calls" copy this PR makes accurate with no UI change:

Models + Endpoints > All Models listing deployments only, with no claude-quality row:

For picker discovery, point any OpenAI-compatible client at the gateway (Claude Code with CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY=1, or Codex): claude-quality appears via /v1/models as shown in the scoped-key listing above

Type

🆕 New Feature

Caveats (if any)

• Claude Desktop's client-side validator only accepts Anthropic-shaped names, so name groups like claude-quality for Desktop pickability
• least-busy, lowest-cost, lowest-latency, and usage-v1 selectors key state by the requested name, so group calls and member-direct calls keep separate stats (same as model_group_alias today)
• Fallbacks and model_group_retry_policy are keyed by name; a group needs its own entries
• Access groups do not contain routing groups; grant the group name itself on keys or teams
• A group name colliding with a model_name or alias warns and stays shadowed instead of failing config load

Changes

Router.get_routing_group resolves a callable group (a real deployment model_name added later shadows a same-named group, and config-time collisions with model names or aliases are rejected at init). _common_checks_available_deployment consults it before the early-resolve step so wildcard or pattern routes cannot hijack a group call, and serves the union of member deployments while model stays the group name; every selector already keys its state by that string. _get_routing_context resolves the group's own strategy for group-name requests. Discovery reuses the model_group_alias materialization: get_model_list_from_routing_groups emits member deployments under the group name via _get_all_deployments' existing model_alias rewrite, which is what surfaces groups in get_model_names, /v1/models, get_model_group_usage, /model_group/info, and the blocked and unhealthy hiding. The proxy gates (route_request and the adapter pass-through endpoint) now share one Router-owned is_recognized_model predicate covering model names, deployment ids, aliases, and routing groups, so the next virtual-model kind cannot be forgotten at one gate; both gates shrank in the process. Cooldown handling keys on the failing request's model group (read through the dual metadata-bucket owner and alias-resolved), so a 429 on a group call moves traffic to the group's alternatives while direct calls to a single-deployment member keep their cooldown exemption. Deleting a deployment now also drops its name from model_names at the removal-repair owner, so a runtime shadow's deletion restores the group, and group rebuilds invalidate the model-group info cache

Authz needs no code: a group is granted by putting its name on a key or team, and listing and enforcement both read the same literal allowlists, so they agree by construction. Member expansion in either direction was rejected on purpose since any-member semantics leak access

Tests extend the mapped files: 12 engine cases in test_router_routing_groups.py (union dispatch, discovery, group strategy selection, collisions, runtime shadowing, alias-to-group composition, empty group, cooldown alternatives, member-direct invariance; 10 of 12 fail on the base commit), a request-gate case in test_route_llm_request.py, and two cooldown cases in test_cooldown_handlers.py

Final Attestation

• The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 4/5

Here's the breakdown:

---

What's done well

• Correct reuse of existing machinery. Routing group discovery is wired through get_model_list_from_routing_groups → _get_all_deployments(model_alias=group.group_name), the exact same path model_group_alias uses. Nothing in /v1/models, get_model_names, or get_model_group_usage needed touching separately.
• Precedence is right. Checking _get_routing_group_deployments before _try_early_resolve_deployments_for_model_not_in_names ensures wildcard/pattern routes cannot hijack a group call. Alias-to-group composition is tested and works because _model_from_alias is already resolved before the group check.
• Zero DB reads in the critical path. get_routing_group and _get_routing_group_deployments are pure in-memory dict lookups.
• Cooldown fix is correct. A single-deployment member of a multi-deployment group should not get the single-deployment exemption; deployment_has_routing_group_alternatives fixes this precisely.
• Init-time collision detection. Rejecting group_name == model_name and group_name == alias_name at startup prevents silent shadowing.
• Tests. 12 engine cases, 2 cooldown cases, 1 gate case, with 10/12 failing on the base commit — the tests verify the actual new behavior, not just the happy path.

---

What holds it back from 5/5

1. _init_routing_groups(None) at line 607 is a hidden contract. The None call is purely to ensure self._routing_groups exists before later uses; a one-liner self._routing_groups: dict[str, RoutingGroup] = {} directly in __init__ would be clearer and safer. The current pattern relies on readers knowing that _init_routing_groups(None) is a no-op loop.

2. DB-backed group loading path is not tested. When routing_groups is loaded from the database and _init_routing_groups is called after a hot-reload, the collision check runs against self.model_group_alias and known_model_names at that moment in time. There's no test that a DB reload with conflicting names raises rather than silently overwriting.

3. get_model_list_from_routing_groups return type annotation (Sequence) doesn't match the actual usage pattern. .extend() works on any iterable so this is not a bug, but the annotation is imprecise — callers extend a mutable list with the result, yet the declared return type is the immutable-view Sequence.

4. No proxy-layer integration test for /v1/models actually listing the group. All tests are at the router engine layer. The gate test confirms the routing group name passes route_request, but there's no test that GET /v1/models with a scoped key returns only the group name. Given the PR's central claim for Claude Code / Codex discovery, a proxy-layer test here would close the loop.

[greptile-apps]

Greptile Summary

Callable routing groups are added to request routing and model discovery, with group-specific strategy selection and revised cooldown handling.

• Resolves group names to the union of member deployments.
• Materializes group names through the existing model-alias discovery machinery.
• Allows the proxy request gate to dispatch group-name requests.
• Adjusts cooldown behavior when a member has alternatives elsewhere in its routing group.
• Adds routing, discovery, collision, request-gate, and cooldown tests.

Confidence Score: 3/5

The PR should not merge until direct-member cooldown behavior and restoration after deleting a runtime group shadow are corrected.

Group membership now causes direct single-deployment requests to lose their cooldown exemption, and model_names-based shadowing can keep a callable group disabled after the shadowing deployment is deleted.

Files Needing Attention: litellm/router.py, litellm/router_utils/cooldown_handlers.py

Important Files Changed

Filename	Overview
litellm/router.py	Adds callable-group resolution, discovery materialization, strategy selection, and shadowing, but deleted runtime shadows can leave groups disabled.
litellm/router_utils/cooldown_handlers.py	Disables the single-deployment cooldown exemption based on group-wide alternatives, which also changes direct-member request behavior.
litellm/proxy/route_llm_request.py	Extends the existing model gate to accept configured routing-group names.
tests/test_litellm/router_strategy/test_router_routing_groups.py	Adds broad engine coverage but does not cover deleting a runtime deployment that shadows a callable group.
tests/test_litellm/router_utils/test_cooldown_handlers.py	Tests the new cooldown decision but not the resulting failure of subsequent direct-member requests.
tests/test_litellm/proxy/test_route_llm_request.py	Verifies routing-group names pass the proxy request gate and dispatch through the router.

_{Reviews (1): Last reviewed commit: "feat(router): make routing groups callab..." | Re-trigger Greptile}

[greptile-apps]

Direct-member cooldown behavior breaks

When a direct request to a single-deployment member receives a 429 or crosses the failure threshold, routing-group alternatives disable its single-deployment exemption even though those alternatives are unavailable to the direct request, causing subsequent calls to that member to fail with no healthy deployment until cooldown expires.

Knowledge Base Used: Router: deployment selection, retries, fallbacks, and cooldowns

[tin-berri]

Fixed in 8cbadcc: cooldown now keys on the failing request's model group, so direct member calls keep the exemption

[tin-berri]

Fixed in 8cbadcc: cooldown now keys on the failing request's model group, so direct member calls keep the exemption

[greptile-apps]

Deleted shadow remains active

When a runtime deployment whose model name matches a routing group is added and later deleted, its name remains in self.model_names, so get_routing_group continues suppressing the group and requests and discovery cannot use it until the model list is rebuilt.

Knowledge Base Used: Router: deployment selection, retries, fallbacks, and cooldowns

[tin-berri]

Fixed in 8cbadcc: model_names is now maintained on deletion at the removal-repair owner, healing the other stale consumers too

[tin-berri]

Fixed in 8cbadcc: model_names is now maintained on deletion at the removal-repair owner, healing the other stale consumers too

[veria-ai]

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

[codecov]

Codecov Report

❌ Patch coverage is 98.30508% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...y/pass_through_endpoints/pass_through_endpoints.py	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 4/5

---

What's strong:

1. Correct use of existing machinery — get_model_list_from_routing_groups reuses _get_all_deployments's model_alias rewrite identically to how get_model_list_from_model_alias works, so groups surface everywhere that aliases do: get_model_names, /v1/models, get_model_group_usage, blocked/unhealthy hiding. No parallel mechanism.

2. Precedence is sound — routing group resolution runs inside _common_checks_available_deployment before _try_early_resolve_deployments_for_model_not_in_names, so a wildcard default_deployment or pattern route cannot hijack a group call. The documented order (specific deployment → model id → alias → routing group → model_name → team/pattern/default) is correct.

3. Collision protection — _init_routing_groups rejects group names that collide with existing model_name values or model_group_alias entries at init time, and get_routing_group applies the runtime-shadowing rule (a deployment added later wins) correctly.

4. Cooldown logic is right — routing_group_has_alternatives properly distinguishes the two cases: a 429 on a group call (where other members exist as alternatives → cool the member down) vs. a 429 on a direct member call (single-deployment exemption preserved). Passing requested_model_group from the failure callback through _set_cooldown_deployments → _should_cooldown_deployment is the right hook.

5. Tests are thorough — 15 new tests, 10/12 failing on the base commit. Covers union dispatch, discovery, group strategy selection, both collision types, runtime shadowing and restore, alias-to-group composition, empty group, cooldown with/without alternatives, and member-direct invariance.

6. Live proof — the screenshots + psql output in the description confirm the end-to-end behavior including spend attribution.

---

What's worth a second look:

1. _invalidate_model_group_info_cache() in the early init path — _init_routing_groups(None) is now called at the very top of __init__ (before model_group_alias and many other attributes exist). _init_routing_groups now calls _invalidate_model_group_info_cache() unconditionally before the if not groups_input: return guard. If that method touches an attribute that hasn't been set yet, it'll raise AttributeError on every Router construction. Worth a quick check that the method is safe when called with a partially-initialized self (e.g., it just does self._model_group_info_cache = None).

2. Cooldown path: indirect requested_model_group propagation — the failure callback reads (litellm_params.get("metadata") or {}).get("model_group") to recover the requested group name. This is correct if metadata is consistently populated for group calls. The unit tests for cooldown call _should_cooldown_deployment directly with the group name rather than exercising the full failure callback → _set_cooldown_deployments chain, so there's no test confirming the metadata key is always present for group calls.

3. model_names.discard in _update_deployment_indices_after_removal — this is a real existing-bug fix (without it the shadow/restore test fails), but it's a side-change that touches deployment removal for all router users, not just routing-group users. Low risk, but worth noting.

None of these are blockers — (1) is likely a non-issue (the method probably just assigns None), (2) is a test coverage gap rather than a correctness gap, and (3) is a net improvement. The design is clean and the feature fills a genuine user-facing gap.

[codspeed]

Merging this PR will not alter performance

✅ 31 untouched benchmarks

---

_{Comparing litellm_callable_routing_groups (b81f318) with litellm_internal_staging (bea3187)}

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 3.5 / 5

Here's the breakdown:

---

What's well done

• is_recognized_model() is the right abstraction — centralizing all virtual-model kinds into one predicate prevents future gate-miss bugs, and the pass-through / route-request refactors are correct and semantically equivalent to the original logic.
• Reusing _get_all_deployments(model_alias=group.group_name) to materialize group rows mirrors how model_group_alias works, so discovery, /v1/models, get_model_group_usage, and blocked/unhealthy hiding all get groups for free.
• Config-time collision detection is correct, and the runtime-shadowing precedence rule (real deployment wins, group restores after deletion) is well-specified and tested.
• The cooldown fix rationale (routing_group_has_alternatives) is logically sound: a 429 on a member with siblings should trigger cooldown so the group selector can move on.
• Test breadth is good — 15 new cases covering union dispatch, strategy selection, collisions, shadowing, alias composition, empty group, cooldown alternatives, member-direct invariance.

---

Concerns

1. Cooldown metadata gap (medium risk)
The fix only works if litellm_metadata["model_group"] is already set to the group name when the failure callback fires. test_failure_callback_reads_model_group_from_litellm_metadata_bucket proves the callback reads it correctly, but it sets litellm_metadata: {"model_group": "quality"} by hand. There's no test or diff showing that the real routing path (a live acompletion call via a group name) actually writes the group name into litellm_metadata["model_group"] — or that it was already there from the pre-existing model_group tracking infrastructure. If it isn't, routing_group_has_alternatives is never invoked and the cooldown exemption for single-member model-groups silently suppresses cooldowns even when the call came through a multi-member group.

2. Hot-path overhead
_get_routing_group_deployments is called on every _common_checks_available_deployment invocation, including all non-group requests. For a router with no routing groups this is two no-op dict lookups, but for a busy proxy the early-exit could be tightened to a not self._routing_groups guard.

3. _init_routing_groups(None) calls _invalidate_model_group_info_cache() very early in __init__
This is the first thing set on self after _zero_cost_cache. If _invalidate_model_group_info_cache touches any attribute not yet initialized at that point in __init__, this will AttributeError on every Router construction. The existing test suite passing suggests it's safe in practice, but the ordering is fragile and worth a comment or guard.

4. Missing integration test for alias → group cooldown path
test_routing_group_has_alternatives_resolves_aliases covers the alias-resolution branch of routing_group_has_alternatives in isolation, but there's no end-to-end test that a 429 on a group-aliased call actually cools down the deployment.

---

The design is clean and the test coverage is above average for a feature of this complexity, but the cooldown metadata gap is a real risk: the fix could silently be a no-op in production if model_group isn't written into litellm_metadata by the existing routing infrastructure for group calls.

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 4/5

This is a well-engineered, security-conscious feature with strong test coverage. Here's the breakdown:

What's done well:

• is_recognized_model predicate shared across both proxy gates (route_request, pass_through_endpoints) — prevents future virtual-model kinds from being forgotten at one gate, and the unification is correct (covers model_names, model_id, alias, routing group)
• _as_routing_group_row correctly strips access_groups from member deployments when emitted under the group name — the access leakage case is explicitly tested
• Collision detection at init time (_init_routing_groups rejects group names that shadow existing model_names or aliases)
• Runtime shadowing + deletion restore is covered by test (test_real_model_added_later_shadows_group) and the model_names.discard fix in _update_deployment_indices_after_removal closes the loop
• Cache invalidation (_invalidate_model_group_info_cache, _invalidate_access_groups_cache) on every group rebuild
• Cooldown path passes requested_model_group through the metadata bucket so a 429 on a group call cools the member while member-direct calls keep the single-deployment exemption — and the E2E tests cover all three cases

What keeps it from a 5:

1. requested_model_group metadata dependency: deployment_callback_on_failure reads requested_model_group from litellm_metadata. If that key is absent (e.g., some non-standard call path that doesn't populate litellm_metadata correctly), requested_model_group is None and the single-deployment exemption silently applies even for group calls. The test mocks the metadata directly rather than exercising the full call stack, so this code path isn't end-to-end verified for correctness under missing metadata.

2. _get_routing_group_deployments on the critical path: It's called for every _common_checks_available_deployment invocation, including non-group requests. The fast path (not self._routing_groups) is O(1), but when groups exist, every non-group request still pays two dict lookups (_routing_groups.get + model_name_to_deployment_indices check). Minor, but worth noting for high-throughput deployments with groups configured.

3. get_model_list_from_routing_groups materializes member deployments for each group on every get_model_list call: The per-name filtering is efficient, but the all-groups path iterates and expands all member deployments. This is acceptable at current scale but could compound with large group counts.

Overall this is a solid, production-ready feature — the security and authz considerations are correct by construction, the test coverage is unusually thorough, and the proxy gate unification is a genuine improvement over the prior state.

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit b98a7c2. Configure here.}

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 4 / 5

---

Why not 5:

1. Backwards-incompatible init-time error for existing configs
_init_routing_groups now raises at startup if a group_name matches an existing model_name. Any production config that already had a routing group with the same name as a deployment model (even though that group was previously un-callable) will now refuse to start. The rule against backwards-incompatible breaks without a flag applies; a warning with graceful degradation (or at least a documented migration step) would be safer for existing users.

2. Critical-path complexity in _common_checks_available_deployment
The routing group lookup now runs on every request before the early-resolve step. get_routing_group does two dict lookups and one model_name_to_deployment_indices lookup per call — cheap in isolation, but this function is on the hot path. There's no fast-exit when _routing_groups is empty (the if not self._routing_groups: return None in get_routing_group helps, but _get_routing_group_deployments wraps it without a top-level guard). Minor, but worth noting.

3. requested_model_group threading relies on metadata being set correctly
The cooldown fix reads get_litellm_metadata_from_kwargs(kwargs).get("model_group") inside deployment_callback_on_failure. If anything in the call chain doesn't set model_group in the metadata bucket (e.g., a non-standard call path or a future refactor), the cooldown silently falls back to the old behavior. There's no test that verifies the metadata is actually populated end-to-end for a group call before the cooldown handler reads it.

---

What's solid:

• The is_recognized_model predicate unifying all proxy gates is a clean and correct simplification — future virtual-model kinds can't be forgotten at one gate
• Access group stripping in _as_routing_group_row is correct and tested
• Cache invalidation (_routing_group_rows = None in _invalidate_model_group_info_cache) is wired to all the right call sites
• model_names.discard on deployment removal restoring the group after a shadow is deleted is a nice correctness detail
• Test coverage is strong: 12 engine cases covering dispatch, discovery, collisions, shadowing, alias composition, empty groups, cooldown alternatives, and member-direct invariance

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 9ecc0e8. Configure here.}

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 4/5

---

What this gets right:

• Routing precedence is correct. _common_checks_available_deployment consults _get_routing_group_deployments before _try_early_resolve_deployments_for_model_not_in_names, so wildcard/pattern routes cannot hijack a group call. The chain (deployment → model id → alias → routing group → model_name → fallbacks) is well-ordered.

• is_recognized_model unification is a real improvement. The previous gate in route_llm_request.py had a subtle operator-precedence bug (or ... and ... without parens) that this PR incidentally fixes while collapsing three separate checks into one predicate. Same cleanup in pass_through_endpoints.py and response_api_endpoints/endpoints.py. A new virtual-model kind now only needs one place to register.

• Security is correctly handled. _as_routing_group_row strips access_groups from the member's model_info so holding a member's access group cannot unlock the whole group. The test test_group_rows_do_not_inherit_member_access_groups covers this.

• Cooldown logic is sound. routing_group_has_alternatives correctly gates the single-deployment exemption: a 429 on a group call (which has alternatives) cools the member, while a direct call to a single-deployment member keeps its exemption. The alias-to-group path in routing_group_has_alternatives (_get_model_from_alias then get_routing_group) is also correct.

• Cache invalidation is complete. _routing_group_rows is cleared in _invalidate_model_group_info_cache (called by set_model_list, add_deployment, _init_routing_groups) and in _init_routing_groups itself. The model_names.discard(model_name) fix in _update_deployment_indices_after_removal lets a shadow deployment's deletion restore the group.

• Test coverage is substantive. 12 engine cases, including union dispatch, strategy selection, collision/shadowing, alias-to-group composition, empty group, cooldown alternatives, and member-direct invariance. The end-to-end cooldown tests use mock_response="litellm.RateLimitError" so they exercise the real failure path.

---

Where the point comes off:

Cooldown metadata dependency is fragile. In deployment_callback_on_failure:

requested_model_group=(get_litellm_metadata_from_kwargs(kwargs) or {}).get("model_group"),

_should_cooldown_deployment only honors routing_group_has_alternatives when requested_model_group is the group name. If model_group in metadata is absent or set to something else by the time the callback fires (e.g., an intermediate retry that rewrites the metadata bucket, or a fallback chain), the group call silently falls back to the single-deployment exemption and the member is not cooled down. The unit tests construct the metadata correctly, but there is no integration-level assertion that model_group is reliably the group name in the metadata bucket at callback time across retry/fallback paths. A mis-propagation here would be invisible until production traffic.

---

Minor notes (not scoring):

• _init_routing_groups(None) is called in __init__ before set_model_list, which is fine — _routing_groups, _model_to_group, and _group_selectors are always reset at the top of the function and the early-return on not groups_input makes the pre-set_model_list call safe.
• The warning-not-error on collision is a deliberate design call (the group still applies strategy to members, the name just isn't callable). Acceptable.
• get_routing_group is O(1) and guarded by if not self._routing_groups: return None, so the added call in the hot path is negligible for users without routing groups.

Overall: solid, well-tested feature with a clean architecture. The one deduction is the silent-failure risk in the cooldown metadata propagation path.

[tin-berri]

On the cooldown metadata concern: the same callback stamp already drives least-busy and spend accounting for ordinary model groups, and absence degrades to the pre-existing exemption

[tin-berri]

Fallbacks re-stamp the failing group before each attempt; 64ab436 adds a retry-path cooldown test alongside the existing group and alias e2e tests

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 4/5

---

What's good:

• Correct resolution precedence — specific deployment > model id > alias > routing group > model_name. _common_checks_available_deployment inserts the group check before early-resolve so wildcard/default-deployment routes can't hijack a group call. This is the right call.
• is_recognized_model consolidation — three proxy gates (route_llm_request, pass_through_endpoints, response_api_endpoints) now share one predicate instead of duplicating alias+model_names+model_id checks. This is a real maintainability win.
• Security posture on _as_routing_group_row — stripping access_groups from materialized group rows is correct; inheriting them would let a key holding a member access group call the whole group.
• model_names.discard after deployment removal — fixes a latent bug where deleting a runtime-shadow deployment left the name in model_names, permanently blocking group restoration.
• Test coverage — 12 engine cases + gate cases + cooldown cases. The end-to-end cooldown tests (_call_and_get_cooldowns with pinned choice) are particularly well-designed.

---

Concerns (not blocking, but worth noting):

1. requested_model_group extraction is an indirect dependency. In deployment_callback_on_failure, requested_model_group is pulled from get_litellm_metadata_from_kwargs(kwargs).get("model_group"). Whether model_group in the metadata bucket holds the routing group name (vs. the underlying member's group) for group calls depends on the metadata-bucket ownership chain — which is not touched by this PR. The end-to-end tests confirm it works today, but it's a somewhat fragile extraction point if metadata tracking changes.

2. _routing_group_rows cache has a soft race on first population. The all-groups path checks cached = self._routing_group_rows then assigns if None. Under concurrent asyncio, two coroutines can both see None and both materialize, with the second overwriting the first. This is idempotent (same result) but doubles work on cold start under load. A None-check + assign pattern is fine here given asyncio's cooperative scheduling, but it's worth noting.

3. Per-name queries bypass the all-groups cache. get_model_list_from_routing_groups(model_name="quality") always re-materializes, never reads from or writes to _routing_group_rows. This is intentional (model_name-scoped queries return a subset) but it means /model_group/info or any per-name caller pays re-materialization cost every time.

4. _init_routing_groups(None) at __init__ time clears caches that were just initialized. It's harmless (the caches are empty), but calling _invalidate_model_group_info_cache and _invalidate_access_groups_cache before any data is loaded is a no-op that adds confusion about ordering invariants.

---

Summary:

The feature closes a real UX gap that the UI has promised since day one, the implementation is additive and well-contained, access control semantics are correct, and the test suite is thorough. The concerns above are minor and none affect correctness in the happy path. Ready for maintainer review.

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 64ab436. Configure here.}

[tin-berri]

UI proof screenshots: Routing Groups tab with claude-quality, the Create Group modal copy this PR makes accurate, and the All Models table showing deployments only