feat(proxy): type Customer Management response_model for OpenAPI coverage - #31043
Conversation
…rage Add response_model to the five remaining untyped /customer operations (block, unblock, new, update, delete) so the generated OpenAPI schema documents a concrete response body. new/update reuse the canonical LiteLLM_EndUserTable (matching info/list); block, unblock, and delete get small dedicated models in litellm/types/proxy/management_endpoints/customer_endpoints.py. Together with the already-typed info/list/daily-activity routes this brings the Customer Management group to full response_model coverage. Regression tests assert each public /customer/* route declares the expected response_model and that /customer/new surfaces a typed schema in app.openapi(), so dropping a response_model fails CI.
Greptile Summary
Confidence Score: 4/5The change is narrowly scoped, but the customer response typing can reject valid partial nested budget payloads and surface as endpoint failures. The modified response model path is well covered conceptually, and focused runtime evidence confirms the remaining issue is concrete and localized. litellm/types/proxy/management_endpoints/customer_endpoints.py
What T-Rex did
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Address review feedback on the Customer Management response_model typing. Greptile flagged that response_model=LiteLLM_EndUserTable on /customer/new and /customer/update silently drops fields the raw Prisma model_dump() echoed. Checking the schema, budget_id is the only such scalar column that was missing from the Pydantic model (created_at/updated_at/tpm_limit do not exist on litellm_endusertable), so add budget_id to LiteLLM_EndUserTable. This restores budget_id on new/update and also fixes the pre-existing gap where /customer/info and /customer/list (already typed) dropped it, which the UI Customer type expects. A regression test pins budget_id surviving the response_model filter on /customer/update. Also document UnblockUsersResponse.blocked_users via a Field description: it holds the users that remain blocked after the call. The key name predates this PR and is kept to avoid a backwards-incompatible rename on a beta route.
|
@greptileai re review |
response_model=LiteLLM_EndUserTable nests the budget as the narrow write allowlist LiteLLM_BudgetTable, which silently drops the server-managed fields the customer endpoints used to return (budget_reset_at, created_at). Introduce CustomerResponse, a thin response model that nests LiteLLM_BudgetTableFull (the repo's budget response model), and apply it on /customer/new, /customer/update, /customer/info and /customer/list. list also builds CustomerResponse so its budget isn't narrowed at construction time. created_by/updated_at/updated_by remain omitted, matching how budgets are returned elsewhere. The shared LiteLLM_EndUserTable is left untouched: it's constructed in many places that pass narrow budget instances, and pydantic v2 won't coerce a budget instance into a wider nested model. Typing only at the response boundary (where the handler hands FastAPI a dict) sidesteps that. A regression test pins budget_reset_at + created_at through the filter and asserts the internal audit fields stay out.
…sponses Lock the exact JSON body each customer-object endpoint (info/list/new/update) and delete return today, so the upcoming type-safety refactor of the handlers is only allowed to land if it reproduces these byte for byte. Pins null-field inclusion, the nested budget shape (server fields kept, audit fields dropped), and object_permission reverse-relation stripping. Green against current code.
Replace the untyped dict + bolt-on response_model pattern on the customer object endpoints with explicit typed construction. A single mapper, _to_customer_response, validates a DB row into CustomerResponse at one Any -> typed seam; new/update/info/list now return it (or a list of it) and carry real -> CustomerResponse / -> List[CustomerResponse] return annotations, and delete returns DeleteCustomersResponse. basedpyright now verifies the handlers' return shapes instead of a runtime filter doing it silently. This also deletes the four copy-pasted object_permission reverse-relation cleanup loops: pydantic's extra=ignore drops those undeclared fields during validation, so the loops were dead code (proven by the golden-master tests, which stay byte-for-byte green). basedpyright errors on the file drop from 140 to 116, all from removed dict plumbing. CustomerResponse stays a thin subclass of LiteLLM_EndUserTable so it inherits the existing validators/config unchanged (behavior preservation); only the nested budget type is widened.
|
also, you might need to reformat + pull latest internal staging to fix linting issues |
Address review nit: the mapper's untyped `record` added an ANN001 violation. The incoming rows are pydantic v2 models, so type the param as BaseModel rather than object (object has no model_dump, which would just move the problem to basedpyright). This clears the ANN001 and also drops three basedpyright unknown-type violations the untyped param was adding.
…itellm_customer_response_models
…itellm_customer_response_models # Conflicts: # litellm/proxy/management_endpoints/customer_endpoints.py
The type-safe response refactor validates the update result via _to_customer_response (CustomerResponse.model_validate(record.model_dump())). These budget tests mocked the end-user update to return a bare MagicMock, so model_dump() yielded a MagicMock that fails validation. Give each update mock a minimal valid dict; the tests assert on the prisma calls, not the body.
f935c67 to
99f17cb
Compare
…itellm_customer_response_models
Python 3.13 strips a docstring's common leading indentation at compile time while 3.12 keeps it, so app.openapi() emits differently-indented description strings depending on the interpreter. The dashboard type generator ran locally on 3.13 and in CI on 3.12, so schema.d.ts drifted and the "Verify schema.d.ts matches the proxy OpenAPI spec" check failed Normalize every description through inspect.cleandoc in the spec dump so the output is identical regardless of interpreter, then regenerate
|
@greptileai re review |
| the narrow write-allowlist shape LiteLLM_EndUserTable carries for internal use. | ||
| """ | ||
|
|
||
| litellm_budget_table: Optional[LiteLLM_BudgetTableFull] = None # pyright: ignore |
There was a problem hiding this comment.
Nested budget validation breaks
CustomerResponse now validates every customer row's nested budget as LiteLLM_BudgetTableFull, but that model requires created_at. When _to_customer_response() receives a customer row whose joined budget payload lacks created_at, the customer endpoints can fail response validation and return a 500. This can happen with partial joined budget rows or existing callers/tests that previously used the narrower LiteLLM_BudgetTable shape, which did not require created_at.
Artifacts
Repro: focused mapper and endpoint harness source
- Contains supporting evidence from the run (text/x-python; charset=utf-8).
Repro: direct validation error and HTTP 500 Internal Server Error output
- Keeps the command output available without making the summary code-heavy.
There was a problem hiding this comment.
Greptile's repro fabricates a budget object with no created_at and feeds it straight to the internal mapper, but no real /customer/* request can reach that state since the budget is always None or a DB row whose created_at is NOT NULL DEFAULT now()
There was a problem hiding this comment.
Fair point — litellm_budgettable.created_at is NOT NULL DEFAULT now(), so any real DB row returned from a /customer/* handler will always carry it. The concern only applies to hand-crafted test stubs that omit created_at, which isn't a production failure path. I'll withdraw this comment.
3dce3da
into
litellm_internal_staging
…rage (BerriAI#31043) * feat(proxy): type Customer Management response_model for OpenAPI coverage Add response_model to the five remaining untyped /customer operations (block, unblock, new, update, delete) so the generated OpenAPI schema documents a concrete response body. new/update reuse the canonical LiteLLM_EndUserTable (matching info/list); block, unblock, and delete get small dedicated models in litellm/types/proxy/management_endpoints/customer_endpoints.py. Together with the already-typed info/list/daily-activity routes this brings the Customer Management group to full response_model coverage. Regression tests assert each public /customer/* route declares the expected response_model and that /customer/new surfaces a typed schema in app.openapi(), so dropping a response_model fails CI. * fix(proxy): keep budget_id in typed customer responses Address review feedback on the Customer Management response_model typing. Greptile flagged that response_model=LiteLLM_EndUserTable on /customer/new and /customer/update silently drops fields the raw Prisma model_dump() echoed. Checking the schema, budget_id is the only such scalar column that was missing from the Pydantic model (created_at/updated_at/tpm_limit do not exist on litellm_endusertable), so add budget_id to LiteLLM_EndUserTable. This restores budget_id on new/update and also fixes the pre-existing gap where /customer/info and /customer/list (already typed) dropped it, which the UI Customer type expects. A regression test pins budget_id surviving the response_model filter on /customer/update. Also document UnblockUsersResponse.blocked_users via a Field description: it holds the users that remain blocked after the call. The key name predates this PR and is kept to avoid a backwards-incompatible rename on a beta route. * fix(proxy): keep nested budget fields in customer responses response_model=LiteLLM_EndUserTable nests the budget as the narrow write allowlist LiteLLM_BudgetTable, which silently drops the server-managed fields the customer endpoints used to return (budget_reset_at, created_at). Introduce CustomerResponse, a thin response model that nests LiteLLM_BudgetTableFull (the repo's budget response model), and apply it on /customer/new, /customer/update, /customer/info and /customer/list. list also builds CustomerResponse so its budget isn't narrowed at construction time. created_by/updated_at/updated_by remain omitted, matching how budgets are returned elsewhere. The shared LiteLLM_EndUserTable is left untouched: it's constructed in many places that pass narrow budget instances, and pydantic v2 won't coerce a budget instance into a wider nested model. Typing only at the response boundary (where the handler hands FastAPI a dict) sidesteps that. A regression test pins budget_reset_at + created_at through the filter and asserts the internal audit fields stay out. * test(proxy): add golden-master characterization tests for customer responses Lock the exact JSON body each customer-object endpoint (info/list/new/update) and delete return today, so the upcoming type-safety refactor of the handlers is only allowed to land if it reproduces these byte for byte. Pins null-field inclusion, the nested budget shape (server fields kept, audit fields dropped), and object_permission reverse-relation stripping. Green against current code. * refactor(proxy): make the customer response flow type-safe Replace the untyped dict + bolt-on response_model pattern on the customer object endpoints with explicit typed construction. A single mapper, _to_customer_response, validates a DB row into CustomerResponse at one Any -> typed seam; new/update/info/list now return it (or a list of it) and carry real -> CustomerResponse / -> List[CustomerResponse] return annotations, and delete returns DeleteCustomersResponse. basedpyright now verifies the handlers' return shapes instead of a runtime filter doing it silently. This also deletes the four copy-pasted object_permission reverse-relation cleanup loops: pydantic's extra=ignore drops those undeclared fields during validation, so the loops were dead code (proven by the golden-master tests, which stay byte-for-byte green). basedpyright errors on the file drop from 140 to 116, all from removed dict plumbing. CustomerResponse stays a thin subclass of LiteLLM_EndUserTable so it inherits the existing validators/config unchanged (behavior preservation); only the nested budget type is widened. * refactor(proxy): annotate customer response mapper param as BaseModel Address review nit: the mapper's untyped `record` added an ANN001 violation. The incoming rows are pydantic v2 models, so type the param as BaseModel rather than object (object has no model_dump, which would just move the problem to basedpyright). This clears the ANN001 and also drops three basedpyright unknown-type violations the untyped param was adding. * style(test): ruff format customer endpoint tests * test(proxy): give customer budget test update mocks a valid model_dump The type-safe response refactor validates the update result via _to_customer_response (CustomerResponse.model_validate(record.model_dump())). These budget tests mocked the end-user update to return a bare MagicMock, so model_dump() yielded a MagicMock that fails validation. Give each update mock a minimal valid dict; the tests assert on the prisma calls, not the body. * chore(ui): regenerate API types from proxy OpenAPI spec * fix(ui): make generated API types stable across Python versions Python 3.13 strips a docstring's common leading indentation at compile time while 3.12 keeps it, so app.openapi() emits differently-indented description strings depending on the interpreter. The dashboard type generator ran locally on 3.13 and in CI on 3.12, so schema.d.ts drifted and the "Verify schema.d.ts matches the proxy OpenAPI spec" check failed Normalize every description through inspect.cleandoc in the spec dump so the output is identical regardless of interpreter, then regenerate
Relevant issues
Linear ticket
Pre-Submission checklist
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
With a proxy running against this branch, the OpenAPI schema documents a concrete response body for every Customer Management operation:
/customer/new,/customer/update,/customer/info,/customer/listreferenceCustomerResponse;block/unblock/deletereference their dedicated models. Previously these returned the default untyped 200 response.Type
🆕 New Feature
🧹 Refactoring
✅ Test
Changes
Two things, in order.
First, the Customer Management endpoint group is fully typed for OpenAPI coverage. The five previously-untyped operations (
block,unblock,new,update,delete) get response models;block/unblock/deleteget three small dedicated models inlitellm/types/proxy/management_endpoints/customer_endpoints.py, and the customer-object endpoints (new/update/info/list) shareCustomerResponse. Response fidelity was checked against the table schema:budget_idwas missing fromLiteLLM_EndUserTableand is added back, and the nested budget now usesLiteLLM_BudgetTableFullso server-managed fields (budget_reset_at,created_at) survive rather than being narrowed to the write-allowlist shape. No field the endpoints used to return is dropped. This also resolves the earlier review feedback on this PR (the silentbudget_id/ nested-budget narrowing, and the misleadingUnblockUsersResponse.blocked_usersname, now documented via aFielddescription).Second, the customer response flow is made type-safe rather than relying on a runtime filter. The handlers no longer return an untyped
dictfrom.model_dump()withresponse_modeldoing the shaping at the exit. Instead a single mapper,_to_customer_response, validates a DB row intoCustomerResponseat one typed boundary, and the handlers construct and return typed models with real return annotations (-> CustomerResponse,-> List[CustomerResponse],-> DeleteCustomersResponse). basedpyright now verifies the return shapes. This also let four copy-pastedobject_permissionreverse-relation cleanup loops be deleted: pydantic'sextra=ignoredrops those undeclared fields during validation, so the loops were dead code. basedpyright errors on the endpoint file drop from 140 to 116.To guarantee the refactor changes no behavior, it was done under a characterization (golden-master) safety net: tests that lock the exact JSON body each customer-object endpoint emits today, committed green against the old code, then kept green byte-for-byte through the refactor. Those live in
test_customer_endpoints.pyalongside the per-endpoint response-model assertions, thebudget_idand nested-budget fidelity regressions, and the existing error-schema tests.response_model=stays on the decorators for OpenAPI generation and defense in depth; it now matches the handler return annotations instead of compensating for an untyped return.