feat(proxy): add generic list handler for /management/v1 - #35308
Merged
yuneng-berri merged 8 commits intoJul 31, 2026
Conversation
Adds the ListSpec/QueryPlan machinery the control-plane list endpoints are meant to share, so a resource declares what it exposes instead of hand-rolling its own paging, sorting and filter parsing. build_query_plan is pure: it turns query parameters into a QueryPlan or an RFC 9457 problem without any I/O, which is what lets the plan be asserted as a value. The database half is a ListExecutor protocol injected by the caller, so this module has no Prisma dependency at all. Four things the framework guarantees rather than leaving to each resource: the spec's unique tiebreaker is always the final sort key, so pages cannot repeat rows when the leading column is all nulls; ordering is NULLS LAST in both directions, since Postgres otherwise floats empty values to the top the moment the sort direction flips; the scope predicate is a separate conjunct ahead of every caller filter, so a filter on a scoped column cannot widen it; and a denied scope is a 403 problem rather than a 200 with an empty list. No route and no consumer yet; budgets registers against it next. The facet endpoint's has_more shapes are untouched, and a test pins them so page mode cannot quietly absorb them.
…itellm_/hopeful-nash-1230fe
Contributor
Greptile SummaryAdds an unregistered generic list framework for future
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; specification invariants are checked during construction, and repeated parameters are rejected before executor database operations.
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/management_v1/list_framework.py | Introduces query planning and execution abstractions; both previously reported issues are addressed without leaving a concrete reachable failure. |
| litellm/proxy/management_endpoints/management_v1/common.py | Extracts shared unknown-parameter problem construction and adds page-mode list links without altering existing facet pagination behavior. |
| litellm/types/proxy/management_endpoints/management_v1.py | Adds generic list response, metadata, and link models while preserving the existing facet response types. |
| tests/test_litellm/proxy/management_endpoints/management_v1/test_list_framework.py | Adds comprehensive isolated tests for planning, validation, scoping, duplicate rejection, filtering, ordering, and response envelopes. |
Reviews (2): Last reviewed commit: "fix(proxy): validate list specs at const..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Section 5 of the design doc spells equality without an operator bracket (`?filter[status]=active`, and `/management/v1/keys?filter[team_id]=` in the sub-resource paragraph); only the other operators carry a second bracket. The parser only understood `filter[field][op]`, so the canonical spelling came back as an unknown query parameter. `filter[field]` now resolves to the field's `eq` operator, which means it still goes through the declared operator set rather than around it: a field that does not offer `eq` rejects the shorthand. The allowed-parameter list advertises the bare spelling for `eq` and the bracketed one for everything else. Drops two guards from the key parser that could not fire. Operator validation already rejects every malformed operator, and `field in spec.filters` already rejects every field nobody declared, so a well-formedness check on top of them was unreachable; the tests cover the malformed keys directly instead.
…arams Two gaps a review flagged on the list framework. The page-size cap was only enforced against a supplied page_size, so a spec whose default_page_size exceeded its max_page_size served more rows than the resource allows on exactly the request that omits the parameter. A default of zero was worse: it reached the total_pages division and made the resource 500 on every request. ListSpec now validates 1 <= default_page_size <= max_page_size when it is built, so a misconfigured resource fails as it is registered rather than per request. default_sort is checked against sortable for the same reason; caller-supplied sort was already validated, but the default never passed through that path and a typo there reached the ORDER BY clause untouched. Raising is right here despite the usual model-failures-as-values rule: there is no request in flight and no caller to answer. Repeated query parameters silently collapsed to their last value, so ?page=1&page=999 paged from 999 and a repeated sort key quietly won, which is the same silently-altered-semantics failure the surface already rejects unknown parameters to avoid. They are now a 400. The check lives in handle_list rather than build_query_plan because a Mapping[str, str] cannot represent a repeat at all; the boundary that can see one is the boundary that rejects it. A denied scope still outranks it, matching every other rejection here. Also corrects the order_by_sql docstring, which claimed every field reaching it had been validated against sortable. That held for caller-supplied sort only.
Contributor
Author
The LIT002 budget rejected the framework: building a where-fragment meant a dict literal per operator, and a dict keyed by a column name chosen at runtime cannot be frozen into a TypedDict or a dataclass field, so there was no spelling of the old shape the rule would accept. Replacing the fragments with a tagged union removes the construction entirely. A plan's where is now a tuple of frozen Compare / Within / IsNull / AnyOf, matched exhaustively, and the field name is a value rather than a key. That also retires the Mapping[str, object] the plan used to carry, which said nothing about what was inside it and left the fragment shape as a convention two sides had to keep agreeing on. Scope predicates take the same type, so a resource declares its row filter in the same vocabulary rather than hand-rolling a backend dict. where_sql renders a plan for a raw-SQL executor, binding every caller-supplied value to a numbered placeholder and writing only spec-declared column names into the statement. It is the counterpart to order_by_sql, which already existed for the same reason: nulls ordering forces the executor onto raw SQL, so the escaping and placeholder arithmetic belong in one reviewed place rather than in each consumer. Also folds the two remaining mutable builds out of the module (set comprehensions and Counter to frozenset/tuple, the serialized page to a tuple pydantic coerces), and lifts the LIKE escaper into common.py so the facet endpoint and the framework share one copy instead of two that can drift. No behavioural change to the facet endpoint; its tests, including the one pinning the escaping, pass untouched.
…itellm_/hopeful-nash-1230fe
…to litellm_/hopeful-nash-1230fe
ryan-crabbe-berri
approved these changes
Jul 31, 2026
yuneng-berri
merged commit Jul 31, 2026
416e398
into
litellm_internal_staging
80 of 81 checks passed
5 tasks
yuneng-berri
added a commit
that referenced
this pull request
Jul 31, 2026
PR #35308 landed a different shape than this branch was written against: `where` is a tuple of frozen predicates rather than a Prisma-shaped mapping, `ListSpec` carries both the row and the wire type, and `where_sql` / `order_by_sql` render for a raw-SQL executor. The budgets executor now queries through `query_raw` the way the spend logs facet does, selecting only the columns it serves. Also casts datetime binds in `where_sql`. They cross into the query engine as JSON, so an uncast placeholder arrives as text and Postgres refuses `timestamp >= text` outright; every `filter[created_at][gte|lte]` was answering 500. The cast reads the bind as an instant and drops it to naive UTC to match Prisma's TIMESTAMP(3) column, the same one /spend/logs/ui applies.
22 tasks
5 tasks
yuneng-berri
added a commit
that referenced
this pull request
Jul 31, 2026
* feat(proxy): add a generic list contract for management/v1 entity lists Paging, sorting, filtering and search for an entity collection, declared once as a ListSpec and served by handle_list. The route injects a ListExecutor that owns its table, so this module never imports Prisma. The caller's scope is derived from the caller alone and ANDed with whatever they filtered on, so a query parameter can only narrow what they may read. This is the shared half of the budgets list; it lands here so the endpoint has something to register against, and drops out when the framework arrives on its own branch. * feat(proxy): add GET /management/v1/budgets The Budgets page reads /budget/list, which returns the whole table as a bare array with no way to page, sort or filter it. A customer with enough budgets to fill the page has no way to find one. Registers LiteLLM_BudgetTable against the management/v1 list contract: sortable on budget_id, max_budget, tpm_limit, rpm_limit and created_at, default order newest-first with budget_id breaking ties, search on budget_id, and filters for budget_duration, max_budget and created_at. budget_duration is deliberately not sortable; the column holds "7d"/"30d" strings, so a lexicographic ORDER BY puts "30d" ahead of "7d". tpm_limit and rpm_limit are BigInt? in Prisma, so rows validate through a pydantic model on the way out and serialize as JSON numbers. A caller without admin view is refused 403 as a problem document rather than served an empty page. /budget/list is untouched. * fix(proxy): rework the budgets list onto the merged list contract PR #35308 landed a different shape than this branch was written against: `where` is a tuple of frozen predicates rather than a Prisma-shaped mapping, `ListSpec` carries both the row and the wire type, and `where_sql` / `order_by_sql` render for a raw-SQL executor. The budgets executor now queries through `query_raw` the way the spend logs facet does, selecting only the columns it serves. Also casts datetime binds in `where_sql`. They cross into the query engine as JSON, so an uncast placeholder arrives as text and Postgres refuses `timestamp >= text` outright; every `filter[created_at][gte|lte]` was answering 500. The cast reads the bind as an instant and drops it to naive UTC to match Prisma's TIMESTAMP(3) column, the same one /spend/logs/ui applies. * refactor(proxy): fold the predicate renderer instead of recursing recursive_detector flags `_render_all`, and the flag is fair: it recursed once per predicate, so the stack grew with the number of filters on the request for no reason. Walking a predicate list is a running bind index, which is a fold. `_render` still re-enters for `AnyOf`, but its clauses are plain comparisons built by `?q=`, so that nesting is one level deep and no caller can drive it deeper.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
Problem this solves:
How it solves it:
ListSpec; the framework does the restbuild_query_planis pure, so the plan is assertable as a valueRelevant issues
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)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
There is nothing to curl here, and I am not going to dress test output up as proof. This PR adds a module and registers no route, so no request reaches it and no user can see anything change. That is deliberate: it is the first of three, and the runtime proof belongs to the second one, which registers
GET /management/v1/budgetsagainst this framework and can be exercised against a live proxy end to endThe only behaviour that exists today and could regress is the
/management/v1/spend_logs/end_usersfacet endpoint, which sharescommon.py. Its response shapes are unchanged, and there is a test pinning them so page mode cannot quietly absorb the facet shapesType
🆕 New Feature
Changes
build_query_plan(spec, params, caller)turns query parameters into aQueryPlanor an RFC 9457ProblemDetail. It performs no I/O and returns failures as values rather than raising, so the whole parsing surface can be tested by comparing plans instead of standing up a database. The execution half is aListExecutorprotocol supplied by the caller, which is why this module imports nothing from Prisma and why the tests substitute an in-memory executor rather than monkeypatching a client.handle_listcomposes the two and builds the{data, meta, links}envelopeFour invariants are the reason this is a framework rather than a helper, and each one is a bug that has to be re-fixed in every endpoint otherwise. The spec's unique
tiebreakeris always appended as the final sort key, because ordering by a column that is entirely null lets Postgres hand the same row back on two different pages. Ordering isNULLS LASTin both directions, since Postgres defaults to nulls-last ascending and nulls-first descending, so flipping the sort direction on a nullable column drags every empty row to the top of the table. The scope predicate is emitted as its own conjunct ahead of every caller-supplied filter, so a caller filtering on the scoped column narrows the result rather than replacing the scope. And aScopeDeniedbecomes a 403 problem instead of a 200 with an empty list, which would otherwise tell the caller the resource is empty rather than that they cannot read itQuery parameters follow JSON:API:
?sort=-created_at,name,?filter[status]=active,?filter[created_at][gte]=2026-01-01,?q=,?page=,?page_size=. Equality is the bare form with no operator bracket, which is the design doc's canonical spelling; it resolves to the field'seqoperator, so a field that does not offer equality rejects the shorthand rather than being filtered around. An unknown parameter, an unsortable sort field, an operator a field does not declare, and a search against a spec with nothing searchable are all 400 problems carrying the allowed set, rather than filters that get silently dropped.is_nullis an addition to the doc's operator set; without it there is no way to express "max_budget IS NULL", and a table that renders nulls as "Unlimited" has to be able to filter on themScope is a tagged union matched exhaustively with
assert_never, so adding a scope kind later is a type error rather than a fall-throughListSpecvalidates itself when it is built. A resource whosedefault_page_sizeexceeded itsmax_page_sizeserved more rows than its own cap on the request that omitted the parameter, since the cap was only applied to a supplied value; a default of zero reached thetotal_pagesdivision and made the resource fail on every request.default_sortis checked againstsortablefor the same reason, because only caller-supplied sort passed through that validation and a typo in the default reached the ORDER BY clause untouched. These raise rather than returning a problem: a malformed spec is a programming error at import time, with no request in flight and no caller to answerRepeated query parameters are a 400. Starlette keeps the last value, so
?page=1&page=999paged from 999 and a repeated sort key quietly won, which is the same silently-altered-semantics failure that unknown-param rejection exists to prevent. The check sits inhandle_listrather thanbuild_query_planbecause aMapping[str, str]cannot represent a repeat at all, so the boundary that can see one is the boundary that rejects it; the pinned pure-function signature is unchangedcommon.pygainsbuild_list_linksas a sibling tobuild_page_links; the existing facet-mode function is untouched. The unknown-parameter problem body is extracted so the FastAPI dependency and the framework cannot drift apartThe LIKE escaper moved into
common.py, so the facet endpoint and the framework share one copy rather than two that can drift apartDeliberately omitted, to be added by whichever PR first needs them:
includes,cursor,parents,serialize_legacy, and ETag /If-None-Match. Adding a field to a frozen dataclass later is additiveTwo deviations from the doc worth flagging for review, both forced by the same constraint.
QueryPlan.orderis a tuple ofSortKeyrather than of Prismaorder_byfragments, because prisma-client-py 0.11.0 typesSortOrderasLiteral['asc', 'desc']and has no way to express nulls ordering, so a Prisma fragment cannot carry the NULLS LAST invariant at all.QueryPlan.whereandScopeWhere.whereare likewise tuples of frozenCompare/Within/IsNull/AnyOfpredicates rather than Prisma where-fragments; a dict keyed by a column name chosen at runtime cannot be frozen, which the repo's mutable-collection budget rejects, and the oldMapping[str, object]said nothing about what was inside it either way.order_by_sqlandwhere_sqlrender a plan for a raw-SQL executor, with every caller-supplied value bound to a numbered placeholder and only spec-declared column names written into the statementQA runbook
Final Attestation