refactor(proxy): plan the team members list from its declared query params

The list framework gains a typed way in next to the text one: an endpoint
that declares its query params passes them as a ListQuery to plan_list_query,
so values are not turned back into text to be parsed again. build_query_plan
and handle_list behave as before, so budgets and the model hub are unchanged.

The team members plan is now built from TeamMembersQuery. The request is only
read to reject a param that is sent twice.
This commit is contained in:
ryan-crabbe-berri 2026-10-06 13:13:41 -07:00
parent 2a2cb98f52
commit 45f1e29e8f
6 changed files with 179 additions and 62 deletions

View file

@ -4,7 +4,9 @@ A resource declares a `ListSpec`; `build_query_plan` turns query parameters into
`QueryPlan` or an RFC 9457 problem without touching a database, and `handle_list`
runs that plan through an injected `ListExecutor`. Keeping the planning pure is what
lets a caller assert the plan as a value instead of asserting against a live Prisma
client, and it keeps this module free of any database dependency.
client, and it keeps this module free of any database dependency. An endpoint that
declares its query parameters hands them over typed, as a `ListQuery`, and
`plan_list_query` plans them the same way.
A plan's `where` is a tuple of frozen `Predicate`s rather than a backend-shaped
mapping, so the framework never has to know which query builder executes it and a
@ -368,8 +370,7 @@ def _parse_page_size(spec: ListSpec[TRow, TOut], params: Mapping[str, str]) -> i
return min(value, spec.max_page_size)
def _parse_sort(spec: ListSpec[TRow, TOut], params: Mapping[str, str]) -> tuple[SortKey, ...] | ProblemDetail:
raw: Final = params.get(SORT_PARAM)
def _sort_keys(spec: ListSpec[TRow, TOut], raw: str | None) -> tuple[SortKey, ...] | ProblemDetail:
if raw is None:
return spec.default_sort
segments: Final = tuple(segment.strip() for segment in raw.split(","))
@ -464,8 +465,7 @@ def _parse_filters(spec: ListSpec[TRow, TOut], params: Mapping[str, str]) -> tup
return tuple(item for item in parsed if not isinstance(item, ProblemDetail))
def _search_predicate(spec: ListSpec[TRow, TOut], params: Mapping[str, str]) -> Predicate | None:
raw: Final = params.get(SEARCH_PARAM)
def _search_predicate(spec: ListSpec[TRow, TOut], raw: str | None) -> Predicate | None:
if not raw:
return None
return AnyOf(clauses=tuple(Compare(field=field, op="contains", value=raw) for field in sorted(spec.searchable)))
@ -505,7 +505,7 @@ def build_query_plan(
if isinstance(page_size, ProblemDetail):
return page_size
sort: Final = _parse_sort(spec, params)
sort: Final = _sort_keys(spec, params.get(SORT_PARAM))
if isinstance(sort, ProblemDetail):
return sort
@ -513,10 +513,22 @@ def build_query_plan(
if isinstance(filters, ProblemDetail):
return filters
search: Final = _search_predicate(spec, params)
return _assemble_plan(spec, scope_predicates, filters, params.get(SEARCH_PARAM), sort, page, page_size)
def _assemble_plan(
spec: ListSpec[TRow, TOut],
scope_predicates: tuple[Predicate, ...],
filters: tuple[Predicate, ...],
search: str | None,
sort: tuple[SortKey, ...],
page: int,
page_size: int,
) -> QueryPlan:
search_predicate: Final = _search_predicate(spec, search)
return QueryPlan(
# Scope first: conjuncts a caller filter sits behind and cannot replace.
where=scope_predicates + filters + ((search,) if search is not None else ()),
where=scope_predicates + filters + ((search_predicate,) if search_predicate is not None else ()),
# Ordering by an all-null column without a unique final key lets Postgres return
# the same row on two different pages.
order=sort + (SortKey(field=spec.tiebreaker, descending=False),),
@ -525,6 +537,35 @@ def build_query_plan(
)
@dataclass(frozen=True, slots=True)
class ListQuery:
"""List parameters an endpoint declared in its signature, so they arrive typed and already validated."""
page: int = 1
page_size: int | None = None
sort: str | None = None
search: str | None = None
filters: tuple[Predicate, ...] = ()
def plan_list_query(
spec: ListSpec[TRow, TOut],
query: ListQuery,
caller: UserAPIKeyAuth,
) -> QueryPlan | ProblemDetail:
"""`build_query_plan` for an endpoint that declares its query parameters instead of reading them as text."""
scope_predicates: Final = _scope_predicates(spec.scope(caller))
if isinstance(scope_predicates, ProblemDetail):
return scope_predicates
sort: Final = _sort_keys(spec, query.sort)
if isinstance(sort, ProblemDetail):
return sort
page_size: Final = spec.default_page_size if query.page_size is None else min(query.page_size, spec.max_page_size)
return _assemble_plan(spec, scope_predicates, query.filters, query.search, sort, query.page, page_size)
def _facet_allowed_params(spec: ListSpec[TRow, TOut]) -> tuple[str, ...]:
"""A facet's values are always ascending, so `sort` is not one of its parameters."""
return tuple(name for name in _allowed_params(spec) if name != SORT_PARAM)
@ -541,7 +582,7 @@ def _facet_where(
filters: Final = _parse_filters(spec, params)
if isinstance(filters, ProblemDetail):
return filters
search: Final = _search_predicate(spec, params)
search: Final = _search_predicate(spec, params.get(SEARCH_PARAM))
return scope_predicates + filters + ((search,) if search is not None else ())
@ -602,15 +643,9 @@ def _duplicate_params(request: Request) -> tuple[str, ...]:
return tuple(sorted(frozenset(name for name in names if names.count(name) > 1)))
def plan_list_request(spec: ListSpec[TRow, TOut], request: Request, caller: UserAPIKeyAuth) -> QueryPlan:
"""The request's query parameters as a plan. Parameters that cannot be planned raise their RFC 9457 problem."""
plan: Final = build_query_plan(spec=spec, params=request.query_params, caller=caller)
if isinstance(plan, ProblemDetail):
raise ManagementProblem(plan)
# Checked here rather than in build_query_plan because a Mapping[str, str] cannot
# represent a repeat: query_params.get() silently keeps the last one, so ?page=1&page=999
# would page from 999 without the caller ever being told which value won.
def reject_repeated_params(request: Request) -> None:
"""Neither a `Mapping[str, str]` nor a declared query model can represent a repeat: both silently keep
the last one, so ?page=1&page=999 would page from 999 without the caller ever being told which value won."""
duplicates: Final = _duplicate_params(request)
if duplicates:
raise ManagementProblem(
@ -622,6 +657,14 @@ def plan_list_request(spec: ListSpec[TRow, TOut], request: Request, caller: User
f"use a comma-separated list for multiple sort keys or filter values.",
)
)
def plan_list_request(spec: ListSpec[TRow, TOut], request: Request, caller: UserAPIKeyAuth) -> QueryPlan:
"""The request's query parameters as a plan. Parameters that cannot be planned raise their RFC 9457 problem."""
plan: Final = build_query_plan(spec=spec, params=request.query_params, caller=caller)
if isinstance(plan, ProblemDetail):
raise ManagementProblem(plan)
reject_repeated_params(request)
return plan

View file

@ -7,11 +7,11 @@ from fastapi import Depends, Query, Request
from litellm.proxy._types import CommonProxyErrors, LiteLLM_TeamTable, UserAPIKeyAuth
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
from litellm.proxy.list_api.common import PROBLEM_TYPE_BASE, ManagementProblem
from litellm.proxy.list_api.list_framework import QueryPlan, plan_list_request
from litellm.proxy.list_api.list_framework import QueryPlan, plan_list_query, reject_repeated_params
from litellm.proxy.management.teams.authz import TeamAccess
from litellm.proxy.management.teams.exceptions import members_not_readable, team_not_found
from litellm.proxy.management.teams.schemas import TeamMembersQuery
from litellm.proxy.management.teams.service import TEAM_MEMBERS_LIST_SPEC
from litellm.proxy.management.teams.service import TEAM_MEMBERS_LIST_SPEC, team_members_list_query
from litellm.proxy.management.users.service import PrismaOrgRoles
from litellm.repositories.team_repository import TeamRepository
from litellm.types.proxy.management_endpoints.management_v1 import ProblemDetail
@ -73,5 +73,8 @@ def get_team_members_plan(
request: Request,
caller: Annotated[UserAPIKeyAuth, Depends(user_api_key_auth)],
) -> QueryPlan:
"""`query` is declared so FastAPI documents and validates the parameters; the plan is read from the same request."""
return plan_list_request(TEAM_MEMBERS_LIST_SPEC, request, caller)
plan: Final = plan_list_query(TEAM_MEMBERS_LIST_SPEC, team_members_list_query(query), caller)
if isinstance(plan, ProblemDetail):
raise ManagementProblem(plan)
reject_repeated_params(request)
return plan

View file

@ -5,9 +5,9 @@ from types import MappingProxyType
from typing import Final
from litellm.proxy._types import TeamMemberBudgetSource, UserAPIKeyAuth
from litellm.proxy.list_api.list_framework import FilterSpec, ListSpec, QueryPlan, Scope, ScopeAll
from litellm.proxy.list_api.list_framework import Compare, ListQuery, ListSpec, QueryPlan, Scope, ScopeAll, Within
from litellm.proxy.management.teams.repository import RawQuery, TeamMemberRow, TeamMemberRows
from litellm.proxy.management.teams.schemas import TeamMemberListItem
from litellm.proxy.management.teams.schemas import TeamMemberListItem, TeamMembersQuery
def member_budget_source(budget_id: str | None, team_default_budget_id: str | None) -> TeamMemberBudgetSource:
@ -46,7 +46,7 @@ TEAM_MEMBERS_LIST_SPEC: Final[ListSpec[TeamMemberRow, TeamMemberListItem]] = Lis
("user_alias", "user_email", "user_id", "role", "spend", "total_spend", "max_budget_in_team", "budget_reset_at")
),
searchable=frozenset(("user_id", "user_email")),
filters=MappingProxyType({"role": FilterSpec(type=str, ops=frozenset(("eq", "in")))}),
filters=MappingProxyType({}),
default_sort=(),
default_page_size=50,
max_page_size=100,
@ -56,6 +56,23 @@ TEAM_MEMBERS_LIST_SPEC: Final[ListSpec[TeamMemberRow, TeamMemberListItem]] = Lis
)
def team_members_list_query(query: TeamMembersQuery) -> ListQuery:
return ListQuery(
page=query.page,
page_size=query.page_size,
sort=query.sort,
search=query.q,
filters=(
*(() if query.role is None else (Compare(field="role", op="eq", value=query.role),)),
*(
()
if query.role_in is None
else (Within(field="role", values=tuple(role.strip() for role in query.role_in.split(","))),)
),
),
)
@dataclass(frozen=True, slots=True)
class TeamMembersPage:
members: tuple[TeamMemberListItem, ...]

View file

@ -17,6 +17,7 @@ from litellm.proxy.list_api.list_framework import (
Compare,
FilterSpec,
IsNull,
ListQuery,
ListSpec,
QueryPlan,
ScopeAll,
@ -28,6 +29,7 @@ from litellm.proxy.list_api.list_framework import (
handle_facet,
handle_list,
order_by_sql,
plan_list_query,
where_sql,
)
from litellm.proxy.management_endpoints.management_v1.common import MANAGEMENT_V1_PREFIX
@ -140,7 +142,10 @@ def _conjuncts(plan: QueryPlan) -> tuple[object, ...]:
def test_appends_the_tiebreaker_to_the_default_sort():
"""Without a unique final key, ordering by an all-null column lets Postgres hand the
same row back on two different pages."""
assert _plan({}).order == (SortKey(field="created_at", descending=True), SortKey(field="budget_id", descending=False))
assert _plan({}).order == (
SortKey(field="created_at", descending=True),
SortKey(field="budget_id", descending=False),
)
def test_appends_the_tiebreaker_to_an_explicit_multi_key_sort():
@ -240,13 +245,7 @@ def test_a_search_renders_as_a_parenthesised_or():
)
)
assert sql == (
'"created_by" = $1 AND ('
"\"budget_id\" ILIKE $2 ESCAPE '\\'"
" OR "
"\"created_by\" ILIKE $3 ESCAPE '\\'"
")"
)
assert sql == ('"created_by" = $1 AND ("budget_id" ILIKE $2 ESCAPE \'\\\' OR "created_by" ILIKE $3 ESCAPE \'\\\')')
assert params == ("alice", "%prod%", "%prod%")
@ -274,11 +273,7 @@ def test_a_planned_filter_renders_end_to_end():
sql, params = where_sql(_plan({"filter[max_budget][is_null]": "true", "q": "prod"}).where)
assert sql == (
'"max_budget" IS NULL AND ('
"\"budget_id\" ILIKE $1 ESCAPE '\\'"
" OR "
"\"created_by\" ILIKE $2 ESCAPE '\\'"
")"
'"max_budget" IS NULL AND ("budget_id" ILIKE $1 ESCAPE \'\\\' OR "created_by" ILIKE $2 ESCAPE \'\\\')'
)
assert params == ("%prod%", "%prod%")
@ -627,15 +622,11 @@ def test_in_splits_on_commas_and_coerces_every_member():
def test_is_null_true_matches_rows_with_no_budget():
"""Budgets renders a null max_budget as "Unlimited"; without is_null there is no way
to ask for those rows."""
assert _conjuncts(_plan({"filter[max_budget][is_null]": "true"})) == (
IsNull(field="max_budget", negated=False),
)
assert _conjuncts(_plan({"filter[max_budget][is_null]": "true"})) == (IsNull(field="max_budget", negated=False),)
def test_is_null_false_matches_rows_that_have_one():
assert _conjuncts(_plan({"filter[max_budget][is_null]": "false"})) == (
IsNull(field="max_budget", negated=True),
)
assert _conjuncts(_plan({"filter[max_budget][is_null]": "false"})) == (IsNull(field="max_budget", negated=True),)
def test_is_null_rejects_a_non_boolean():
@ -688,9 +679,7 @@ async def test_returns_the_page_mode_envelope():
total_count=42,
)
response = await handle_list(
spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER
)
response = await handle_list(spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER)
body = response.model_dump(by_alias=True)
assert body["meta"] == {"total_count": 42, "page": 2, "page_size": 5, "total_pages": 9}
@ -714,9 +703,7 @@ async def test_serializes_rows_flat_without_a_json_api_resource_wrapper():
async def test_links_let_a_client_page_without_building_urls():
executor = RecordingExecutor(rows=(), total_count=42)
response = await handle_list(
spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER
)
response = await handle_list(spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER)
links = response.model_dump(by_alias=True)["links"]
assert links["self"] == f"{BUDGETS_PATH}?page_size=5&page=2"
@ -730,9 +717,7 @@ async def test_links_let_a_client_page_without_building_urls():
async def test_the_last_page_has_no_next_link():
executor = RecordingExecutor(rows=(), total_count=10)
response = await handle_list(
spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER
)
response = await handle_list(spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER)
links = response.model_dump(by_alias=True)["links"]
assert links["next"] is None
@ -997,3 +982,48 @@ async def test_a_facet_rejects_a_page_size_that_is_not_a_positive_integer():
assert problem.status == 400
assert "'page_size'" in problem.detail
# ------------------------------------------------------ declared (typed) parameters
@pytest.mark.parametrize(
("typed", "as_text"),
[
pytest.param(ListQuery(), {}, id="defaults"),
pytest.param(ListQuery(page=3, page_size=10), {"page": "3", "page_size": "10"}, id="paging"),
pytest.param(ListQuery(page_size=1000), {"page_size": "1000"}, id="page-size-over-the-cap"),
pytest.param(ListQuery(sort="-max_budget,budget_id"), {"sort": "-max_budget,budget_id"}, id="sort"),
pytest.param(ListQuery(search="acme"), {"q": "acme"}, id="search"),
pytest.param(
ListQuery(
filters=(
Within(field="created_by", values=("u1", "u2")),
Compare(field="max_budget", op="gte", value=5.0),
)
),
{"filter[created_by][in]": "u1,u2", "filter[max_budget][gte]": "5"},
id="filters",
),
],
)
def test_declared_params_plan_exactly_as_the_same_params_sent_as_text(typed: ListQuery, as_text: Mapping[str, str]):
scoped = _spec(scope=lambda caller: ScopeWhere(where=(Compare(field="created_by", op="eq", value="caller-1"),)))
assert plan_list_query(scoped, typed, CALLER) == _plan(as_text, scoped)
@pytest.mark.parametrize(
("typed", "spec", "status"),
[
pytest.param(ListQuery(sort="created_by"), _spec(), 400, id="unsortable-field"),
pytest.param(ListQuery(), _spec(scope=lambda caller: ScopeDenied(reason="no")), 403, id="scope-denied"),
],
)
def test_declared_params_that_cannot_be_planned_are_a_problem(
typed: ListQuery, spec: ListSpec[BudgetRow, BudgetOut], status: int
):
problem = plan_list_query(spec, typed, CALLER)
assert isinstance(problem, ProblemDetail)
assert problem.status == status

View file

@ -10,11 +10,9 @@ from fastapi.testclient import TestClient
from litellm.proxy._types import LiteLLM_TeamTable, LitellmUserRoles, Member, UserAPIKeyAuth
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
from litellm.proxy.list_api.common import ManagementProblem
from litellm.proxy.list_api.list_framework import QueryPlan, build_query_plan
from litellm.proxy.list_api.list_framework import QueryPlan
from litellm.proxy.management.teams.authz import TeamAccess
from litellm.proxy.management.teams.dependencies import get_readable_team, get_team_members_plan
from litellm.proxy.management.teams.service import TEAM_MEMBERS_LIST_SPEC
from litellm.types.proxy.management_endpoints.management_v1 import ProblemDetail
TEAM: Final = LiteLLM_TeamTable(
team_id="team-1", organization_id=None, members_with_roles=[Member(user_id="member", role="user")]
@ -98,9 +96,8 @@ def test_get_team_members_plan_is_not_reached_by_params_the_query_model_rejects(
assert plan_client(caller("member")).get("/plan", params=params).is_client_error
def test_documented_query_params_are_exactly_the_ones_the_roster_can_be_planned_by() -> None:
refusal: Final = build_query_plan(spec=TEAM_MEMBERS_LIST_SPEC, params={"undeclared": "1"}, caller=caller("member"))
documented: Final = plan_client(caller("member")).app.openapi()["paths"]["/plan"]["get"]["parameters"]
def test_get_team_members_plan_refuses_a_param_sent_twice() -> None:
with pytest.raises(ManagementProblem) as refused:
plan_client(caller("member")).get("/plan?page=1&page=2")
assert isinstance(refusal, ProblemDetail)
assert sorted(param["name"] for param in documented) == refusal.allowed
assert refused.value.problem.status == 400

View file

@ -8,13 +8,15 @@ from typing import Final
import pytest
from litellm.proxy._types import TeamMemberBudgetSource
from litellm.proxy.list_api.list_framework import QueryPlan, SortKey
from litellm.proxy.list_api.list_framework import Compare, ListQuery, QueryPlan, SortKey, Within
from litellm.proxy.management.teams.repository import TeamMemberRow
from litellm.proxy.management.teams.schemas import TeamMembersQuery
from litellm.proxy.management.teams.service import (
TEAM_MEMBERS_LIST_SPEC,
TeamMembersPage,
get_team_members_list,
member_budget_source,
team_members_list_query,
)
RESET_AT: Final = datetime(2026, 11, 1, tzinfo=timezone.utc)
@ -91,3 +93,28 @@ def test_list_item_carries_the_member_row_and_derives_its_budget_source() -> Non
"rpm_limit": 10,
"allowed_models": ("gpt-a", "gpt-b"),
}
def test_team_members_list_query_carries_every_declared_param_with_roles_as_roster_filters() -> None:
declared: Final = TeamMembersQuery.model_validate(
{
"q": "ada",
"filter[role]": "admin",
"filter[role][in]": "admin, user",
"sort": "-spend",
"page": 2,
"page_size": 10,
}
)
assert team_members_list_query(declared) == ListQuery(
page=2,
page_size=10,
sort="-spend",
search="ada",
filters=(Compare(field="role", op="eq", value="admin"), Within(field="role", values=("admin", "user"))),
)
def test_team_members_list_query_with_no_params_filters_nothing() -> None:
assert team_members_list_query(TeamMembersQuery()) == ListQuery()