mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix: apply routing-status filter inside the search flow and regen API types
Greptile flagged (P1): with search + a status filter combined, the bounded DB fetch ran before status filtering, so matches of the other status could consume the page budget and leave short pages / stale totals. - extract _matches_routing_status() (sentinel-safe) and apply it to the router-side matches inside _apply_search_filter_to_models - push the status condition into the DB where clause of _fetch_db_models_for_search, so rows of the other status never count against the bounded fetch - endpoint passes blocked through to the search flow; tests cover the combined search+status case and the generated where clause - regenerate ui/litellm-dashboard/src/lib/http/schema.d.ts for the new query param - update useModels.test.ts expectations for the new trailing argument
This commit is contained in:
parent
27d155b1a1
commit
13d76ca1fb
5 changed files with 135 additions and 17 deletions
35
PR_BODY.md
Normal file
35
PR_BODY.md
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
## Relevant issues
|
||||
|
||||
Fixes the "active and paused models are indistinguishable in the models table" pain: the table mixes both states, has no status column, and the drawer can only filter by public model name and access group.
|
||||
|
||||
## Pre-Submission checklist
|
||||
|
||||
- [x] I ran `npm run build` in `ui/litellm-dashboard` without errors
|
||||
- [x] I ran `npx vitest run` for the models-and-endpoints suite: 205 passed
|
||||
- [x] I added/updated unit tests (`test_routes_model_info.py`: 4 new tests)
|
||||
- [x] I verified the change end-to-end against a live proxy (39 deployments: filter returns exactly 9 active / 20 paused; `sortBy=blocked&sortOrder=asc` puts active first; `search=nvidia_nim` finds deployments by provider)
|
||||
|
||||
## What changed
|
||||
|
||||
### Backend — `GET /v2/model/info`
|
||||
|
||||
- New optional `blocked` query param: `true` = only paused deployments, `false` = only active ones. Omitting it keeps the current behavior for every existing caller.
|
||||
- New `blocked` sort field (`sortBy=blocked&sortOrder=asc` = active first). The existing `status` sort field is unchanged: it still sorts by config-vs-DB source, which is why it never grouped active/paused rows.
|
||||
- `search` now also matches `litellm_params.model`, so typing a provider or upstream model id (`nvidia_nim`, `openrouter/deepseek`, `openai/gpt-4`…) finds deployments whose public model name doesn't mention them.
|
||||
- Router-side matching is case-insensitive; the DB branch uses Prisma JSON `string_contains`, which is case-sensitive on Postgres (same limitation already documented for that query path). Rows already loaded in the router — the common case, including all DB-backed deployments — go through the case-insensitive path.
|
||||
|
||||
### Frontend — Models & Endpoints
|
||||
|
||||
- New visible **Status** column (`Active` / `Paused` badges), sortable server-side.
|
||||
- New **Status** filter (All / Active / Paused) in the Filters drawer, wired through URL state (`?status=active|paused`) and the new server param, so pagination totals stay correct while filtered.
|
||||
- The **Actions** column is now pinned to the right edge, so the pause/resume switches stay visible when the table overflows horizontally.
|
||||
- The pre-existing hidden "Source" column (DB vs config) is untouched.
|
||||
|
||||
### Sentinel safety
|
||||
|
||||
The routing-status filter only applies when the parameter is literally `True`/`False`. Direct calls that bypass FastAPI receive the truthy Query sentinel as the default; guarding on identity (same pattern as `exclude_auto_routers`' `is True`) keeps their no-filter behavior. A regression test pins this (`test_model_info_v2_query_sentinel_does_not_filter` still passes).
|
||||
|
||||
## Tests
|
||||
|
||||
- `tests/test_litellm/proxy/proxy_server/test_routes_model_info.py`: 4 new tests (blocked filter both ways, blocked sort, search over `litellm_params.model`). Full-file run: same pre-existing failures before and after the change (verified by reverting the patch), i.e. no regressions.
|
||||
- `ui/litellm-dashboard`: 208/208 tests pass in the models-and-endpoints suite (1 updated for the new column, 1 new for the drawer filter, 3 new for the status mapping).
|
||||
|
|
@ -14450,6 +14450,7 @@ async def _fetch_db_models_for_search(
|
|||
sort_by: str | None,
|
||||
is_byok_outside_caller_teams: Callable[[dict[str, JsonValue]], bool],
|
||||
model_name: str | None = None,
|
||||
blocked: bool | None = None,
|
||||
) -> tuple[list[dict[str, object]], int]:
|
||||
"""
|
||||
Run the bounded DB query that backs `/v2/model/info?search=`. Returns
|
||||
|
|
@ -14481,6 +14482,10 @@ async def _fetch_db_models_for_search(
|
|||
if model_name is None
|
||||
else [{"model_name": model_name}]
|
||||
)
|
||||
# Status filter runs inside the DB query too: the fetch is capped, so
|
||||
# matches of the other status must not consume the page budget.
|
||||
if blocked is not None:
|
||||
match_conditions.append({"model_info": {"path": ["blocked"], "equals": blocked}})
|
||||
if db_model_ids_in_router:
|
||||
match_conditions.append({"model_id": {"not": {"in": list(db_model_ids_in_router)}}})
|
||||
db_where_condition: Final[dict[str, Any]] = {"AND": match_conditions}
|
||||
|
|
@ -14529,6 +14534,7 @@ async def _apply_search_filter_to_models(
|
|||
size: int = 50,
|
||||
sort_by: str | None = None,
|
||||
model_name: str | None = None,
|
||||
blocked: bool | None = None,
|
||||
) -> tuple[list[dict[str, Any]], int | None]:
|
||||
"""
|
||||
Apply search filter to models, querying database for additional matching models.
|
||||
|
|
@ -14549,6 +14555,9 @@ async def _apply_search_filter_to_models(
|
|||
sort_by: Sort field. When set, results must be sorted across the
|
||||
full match set, so the DB fetch is capped at
|
||||
``_SORTED_SEARCH_DB_FETCH_CAP`` instead of one page.
|
||||
blocked: Routing-status filter (false = active, true = paused). Applied
|
||||
to the router matches and pushed into the DB query, so rows of the
|
||||
other status never consume the bounded fetch.
|
||||
model_name: Exact ``model_name`` the caller already narrowed
|
||||
``all_models`` to (``?model=``). The DB query matches it
|
||||
exactly instead of the substring, and is skipped when the
|
||||
|
|
@ -14594,7 +14603,9 @@ async def _apply_search_filter_to_models(
|
|||
filtered_router_models: Final = [
|
||||
m
|
||||
for m in all_models
|
||||
if _model_matches_search(m) and not _is_byok_outside_caller_teams(m.get("model_info") or {})
|
||||
if _model_matches_search(m)
|
||||
and _matches_routing_status(m, blocked)
|
||||
and not _is_byok_outside_caller_teams(m.get("model_info") or {})
|
||||
]
|
||||
|
||||
# Separate filtered models into config vs db models, and track db model IDs
|
||||
|
|
@ -14631,6 +14642,7 @@ async def _apply_search_filter_to_models(
|
|||
sort_by=sort_by,
|
||||
is_byok_outside_caller_teams=_is_byok_outside_caller_teams,
|
||||
model_name=model_name,
|
||||
blocked=blocked,
|
||||
)
|
||||
search_total_count = router_models_count + db_models_total_count
|
||||
except Exception as e:
|
||||
|
|
@ -14803,6 +14815,19 @@ def _model_in_access_group(model: Mapping[str, object], access_group: str) -> bo
|
|||
return isinstance(access_groups, (list, tuple)) and access_group in access_groups
|
||||
|
||||
|
||||
def _matches_routing_status(model: Mapping[str, object], blocked: bool | None) -> bool:
|
||||
"""True when the deployment matches the requested routing status.
|
||||
|
||||
Guarded on `is True` / `is False` because direct calls that bypass FastAPI
|
||||
pass the truthy Query sentinel as the default, which must not filter (same
|
||||
pattern as `exclude_auto_routers`). Entries without a `blocked` flag (e.g.
|
||||
A2A agents) are neither active nor paused, so they match neither status.
|
||||
"""
|
||||
if blocked is True or blocked is False:
|
||||
return (model.get("model_info") or {}).get("blocked") is blocked
|
||||
return True
|
||||
|
||||
|
||||
def _matches_model_info_filters(
|
||||
model: Mapping[str, object],
|
||||
exclude_auto_routers: bool | None,
|
||||
|
|
@ -14814,14 +14839,8 @@ def _matches_model_info_filters(
|
|||
return False
|
||||
if isinstance(access_group, str) and not _model_in_access_group(model, access_group):
|
||||
return False
|
||||
# Routing-status filter. Guarded on `is True` / `is False` because direct
|
||||
# calls that bypass FastAPI pass the truthy Query sentinel as the default,
|
||||
# which must not filter (same pattern as `exclude_auto_routers`). Entries
|
||||
# without a `blocked` flag (e.g. A2A agents) are neither active nor paused,
|
||||
# so they drop out of both filtered views.
|
||||
if blocked is True or blocked is False:
|
||||
if (model.get("model_info") or {}).get("blocked") is not blocked:
|
||||
return False
|
||||
if not _matches_routing_status(model, blocked):
|
||||
return False
|
||||
return wildcard_only is not True or "*" in str(model.get("model_name") or "")
|
||||
|
||||
|
||||
|
|
@ -15251,6 +15270,7 @@ async def model_info_v2(
|
|||
size=size,
|
||||
sort_by=sortBy,
|
||||
model_name=model,
|
||||
blocked=blocked,
|
||||
)
|
||||
|
||||
if user_models_only:
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ Pins (PR2):
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import copy
|
||||
from collections.abc import Callable
|
||||
from contextlib import AbstractContextManager
|
||||
|
|
@ -809,7 +810,12 @@ def test_v2_model_info_exclude_auto_routers_paginates_over_the_filtered_set(clie
|
|||
|
||||
@pytest.fixture
|
||||
def routing_status_router(monkeypatch):
|
||||
"""Router with one paused (blocked) deployment and two active ones."""
|
||||
"""Router with one paused (blocked) deployment and two active ones.
|
||||
|
||||
The real `_apply_search_filter_to_models` runs (its search path is what
|
||||
the combined search+status tests exercise); only the bounded DB fetch is
|
||||
stubbed so tests don't need Prisma.
|
||||
"""
|
||||
model_list = [
|
||||
{
|
||||
"model_name": "paused-model",
|
||||
|
|
@ -836,11 +842,11 @@ def routing_status_router(monkeypatch):
|
|||
monkeypatch.setattr(proxy_server, "prisma_client", MagicMock())
|
||||
monkeypatch.setattr(proxy_server, "user_model", None)
|
||||
monkeypatch.setattr(proxy_server.proxy_config, "get_config", AsyncMock(return_value={}))
|
||||
monkeypatch.setattr(
|
||||
proxy_server,
|
||||
"_apply_search_filter_to_models",
|
||||
AsyncMock(side_effect=lambda all_models, **kw: (all_models, len(all_models))),
|
||||
)
|
||||
|
||||
async def fake_fetch_db_models_for_search(**kwargs):
|
||||
return [], 0
|
||||
|
||||
monkeypatch.setattr(proxy_server, "_fetch_db_models_for_search", fake_fetch_db_models_for_search)
|
||||
monkeypatch.setattr(proxy_server, "_enrich_model_info_with_litellm_data", lambda model, **kw: model)
|
||||
|
||||
import litellm.proxy.agent_endpoints.model_list_helpers as mlh
|
||||
|
|
@ -877,6 +883,58 @@ def test_v2_model_info_sort_by_blocked_puts_active_first(client, auth_as, routin
|
|||
assert _model_names(response.json()) == ["active-model", "another-active", "paused-model"]
|
||||
|
||||
|
||||
def test_v2_model_info_search_and_blocked_filter_combine(client, auth_as, routing_status_router):
|
||||
"""`search` + `blocked` compose: matches of the other status are excluded
|
||||
and the totals describe exactly the filtered set."""
|
||||
with auth_as():
|
||||
response = client.get("/v2/model/info", params={"search": "openai", "blocked": "true"})
|
||||
assert response.status_code == 200
|
||||
payload = response.json()
|
||||
assert _model_names(payload) == ["paused-model"]
|
||||
assert payload["total_count"] == 1
|
||||
|
||||
|
||||
def test_fetch_db_models_for_search_pushes_blocked_into_the_where(monkeypatch):
|
||||
"""The bounded DB fetch must filter by routing status itself, or rows of
|
||||
the other status consume the page budget before status filtering runs."""
|
||||
captured: dict = {}
|
||||
|
||||
class FakeTable:
|
||||
async def count(self, where=None):
|
||||
captured["count_where"] = where
|
||||
return 0
|
||||
|
||||
async def find_many(self, where=None, take=None):
|
||||
captured["find_where"] = where
|
||||
return []
|
||||
|
||||
class FakeRepository:
|
||||
def __init__(self, client):
|
||||
self.table = FakeTable()
|
||||
|
||||
monkeypatch.setattr(proxy_server, "ModelRepository", FakeRepository)
|
||||
|
||||
async def run():
|
||||
await proxy_server._fetch_db_models_for_search(
|
||||
prisma_client=MagicMock(),
|
||||
proxy_config=MagicMock(),
|
||||
search_lower="openai",
|
||||
db_model_ids_in_router=set(),
|
||||
router_models_count=0,
|
||||
page=1,
|
||||
size=50,
|
||||
sort_by=None,
|
||||
is_byok_outside_caller_teams=lambda info: False,
|
||||
blocked=True,
|
||||
)
|
||||
|
||||
asyncio.run(run())
|
||||
|
||||
where = captured["count_where"]
|
||||
assert {"model_info": {"path": ["blocked"], "equals": True}} in where["AND"]
|
||||
assert {"model_info": {"path": ["blocked"], "equals": False}} not in where["AND"]
|
||||
|
||||
|
||||
def test_v2_model_info_search_matches_litellm_model_name(client, auth_as, monkeypatch):
|
||||
"""`search` also hits the underlying LiteLLM model name (e.g. a provider
|
||||
prefix), not just the public model name."""
|
||||
|
|
|
|||
|
|
@ -123,6 +123,7 @@ describe("useModelsInfo", () => {
|
|||
undefined,
|
||||
undefined,
|
||||
false,
|
||||
undefined,
|
||||
);
|
||||
expect(modelInfoCall).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
|
@ -153,6 +154,7 @@ describe("useModelsInfo", () => {
|
|||
undefined,
|
||||
undefined,
|
||||
false,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
|
|
|
|||
7
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
7
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
|
|
@ -22224,7 +22224,8 @@ export interface paths {
|
|||
* search: Case-insensitive partial match on model name or team public name.
|
||||
* modelId: Return a single deployment by LiteLLM model id.
|
||||
* teamId: Filter to models with direct access or team membership for this team id.
|
||||
* sortBy / sortOrder: Sort by model_name, created_at, updated_at, costs, or status.
|
||||
* sortBy / sortOrder: Sort by model_name, created_at, updated_at, costs, status, or blocked.
|
||||
* blocked: Filter by routing status (false = active, true = paused).
|
||||
* access_group: Only return deployments in this model access group.
|
||||
* wildcard_only: Only return deployments whose `model_name` contains `*`.
|
||||
*
|
||||
|
|
@ -76049,10 +76050,12 @@ export interface operations {
|
|||
modelId?: string | null;
|
||||
/** @description Filter models by team ID. Returns models with direct_access=True or teamId in access_via_team_ids */
|
||||
teamId?: string | null;
|
||||
/** @description Field to sort by. Options: model_name, created_at, updated_at, costs, status */
|
||||
/** @description Field to sort by. Options: model_name, created_at, updated_at, costs, status, blocked */
|
||||
sortBy?: string | null;
|
||||
/** @description Sort order. Options: asc, desc */
|
||||
sortOrder?: string | null;
|
||||
/** @description Filter by routing status: false = active deployments, true = paused (blocked) deployments. Omit to return both. */
|
||||
blocked?: boolean | null;
|
||||
/** @description Omit auto-router deployments (litellm model prefixed `auto_router/`). They select among deployments rather than being deployments themselves, so a caller rendering a deployment list can leave them out. Defaults to false, so existing callers are unaffected */
|
||||
exclude_auto_routers?: boolean | null;
|
||||
/** @description Only return deployments whose `model_info.access_groups` contains this access group */
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue