test(e2e): probe the full spend read surface including schema-hidden routes - #32267
Conversation
…eway.create_model The split-transport routing table listed only /model/info as a control-plane prefix, so /model/new and /model/delete were sent to the data-plane gateway, which does not serve management routes and 404s them. Every suite that registers deployments at runtime (llm_translation, batches, access_control) failed on the split stage deployment because of this. Widen the prefix to /model/ so all model-management routes reach the control plane while /models stays on the data plane. Separately, batch_client.py and several llm_translation tests call gateway.create_model, but Gateway never had that method, so all 17 batch tests errored at fixture setup with AttributeError. Add create_model/delete_model to Gateway (with the optional mode that batches needs) and make EndpointsClient delegate to it instead of carrying its own copy. Regression tests cover both: the routing predicate for management vs LLM paths and the Gateway model-management surface via a typed fake Transport. Both fail on the previous code
The recording fake always answered with {"model_id": ...} even when the
caller asked for NoBody, which only validated because pydantic ignores extra
fields by default. Return an empty payload for response types that carry no
fields so a future extra="forbid" on NoBody cannot turn the delete test into
a ValidationError inside the fake
…routes The curated spend-route list missed twelve read endpoints, most of them include_in_schema=False and therefore invisible to the schema-discovery test: /spend/logs/v2, /spend/logs/session/ui, /global/all_end_users, /global/activity/exceptions/deployment, and the per-entity daily activity family (user, user aggregated, team, organization, customer, end_user, tag). Add them all, verified responsive against the live split stage deployment. /end_user was missing from CONTROL_PLANE_PREFIXES, so /end_user/daily/activity would have been routed to the data plane and 404ed like /model/new used to; add the prefix and pin it plus the daily-activity routes in the transport routing test. /provider/budgets stays excluded with a documented reason: it returns 500 whenever router_settings.provider_budget_config is absent, so probing it on a proxy without provider budget routing configured can never be green
Greptile SummaryThis PR expands the e2e spend-route probe suite with twelve previously-uncovered read endpoints and fixes a silent routing bug where all
Confidence Score: 4/5All changes are confined to the tests/e2e directory and have no production code impact; the routing fix and route additions are well-verified against a live deployment. The routing fix and new route coverage are correct and well-tested. The only gap is that _SPEND_PREFIXES does not cover the new route families, so the schema auto-discovery safety net would silently miss them if any route's include_in_schema=False flag were ever removed. No production logic is touched. tests/e2e/spend_tracking/test_spend_routes.py — the _SPEND_PREFIXES tuple may need expanding if any of the new routes ever becomes schema-visible.
|
| Filename | Overview |
|---|---|
| tests/e2e/transport.py | Changed /model/info prefix to /model/ in CONTROL_PLANE_PREFIXES, correctly routing all model-management calls (/model/new, /model/delete, /model/update) to the control plane; also adds /end_user prefix. The trailing-slash guard correctly excludes /models. |
| tests/e2e/spend_tracking/test_spend_routes.py | Adds 12 previously-uncovered spend read routes to SPEND_ROUTES with clear exclusion rationale in comments. The _SPEND_PREFIXES used for schema auto-discovery does not cover several of the new route families, but since they are all include_in_schema=False this is currently benign. |
| tests/e2e/e2e_gateway.py | Consolidates create_model/delete_model from EndpointsClient into Gateway, adding warnings.warn on delete failure. Mode parameter properly threaded through for batch mode support. |
| tests/e2e/test_e2e_gateway.py | New unit tests for Gateway.create_model and delete_model using a typed fake Transport. Correctly verifies path, body shape, mode field, and model_id return value. |
| tests/e2e/test_transport.py | New unit tests for is_control_plane_path, covering the /end_user and /model/* additions. Correctly asserts /models stays on the data plane while /model/new goes to the control plane. |
| tests/e2e/llm_translation/endpoints_client.py | Delegates create_model/delete_model to Gateway, removing duplicated implementation. Clean refactor with no behavioral change. |
| tests/e2e/models.py | Extracts ModelMode as a named type alias from the inline Literal in ModelInfoBody.mode, enabling reuse in Gateway.create_model signature. |
Comments Outside Diff (1)
-
tests/e2e/spend_tracking/test_spend_routes.py, line 73 (link)_SPEND_PREFIXESdoesn't cover the new route familiesThe auto-discovery test (
test_schema_listed_spend_routes_are_responsive) filters schema paths with_SPEND_PREFIXES = ("/spend", "/global/spend", "/global/activity"). Seven of the twelve newly-added routes —/global/all_end_users,/user/daily/activity,/team/daily/activity,/organization/daily/activity,/customer/daily/activity,/end_user/daily/activity, and/tag/daily/activity— do not start with any of those prefixes. If any of them later loses theinclude_in_schema=Falseflag, the auto-discovery safety net will silently miss it.
Reviews (1): Last reviewed commit: "test(e2e): probe the full spend read sur..." | Re-trigger Greptile
Greptile SummaryThis PR expands E2E spend-route coverage by adding 11 previously untested routes (daily-activity family,
Confidence Score: 5/5All changes are confined to the tests/e2e directory and carry no production-code impact; the routing fix corrects a pre-existing mismatch between model-management route calls and the control-plane prefix list. The No files require special attention.
|
| Filename | Overview |
|---|---|
| tests/e2e/transport.py | Adds /end_user prefix and widens /model/info to /model/ in CONTROL_PLANE_PREFIXES; the /models data-plane route is unaffected since it doesn't start with /model/ |
| tests/e2e/spend_tracking/test_spend_routes.py | Adds 11 previously untested spend/activity routes to SPEND_ROUTES; exclusion rationale is now documented inline; no existing assertions weakened |
| tests/e2e/test_transport.py | New unit tests for is_control_plane_path covering all management prefixes and data-plane LLM routes; good boundary coverage |
| tests/e2e/test_e2e_gateway.py | New unit tests for Gateway.create_model and delete_model using a typed fake Transport; pins the model-management surface against signature drift |
| tests/e2e/e2e_gateway.py | Adds create_model/delete_model to Gateway with a mode parameter; NoBody and is_ok are properly imported; warnings.warn used for non-fatal delete failures |
| tests/e2e/llm_translation/endpoints_client.py | Delegates create_model/delete_model to gateway; removes now-unused imports; no behavior change |
| tests/e2e/models.py | Extracts ModelMode Literal type alias from inline ModelInfoBody field; purely cosmetic, no logic change |
Reviews (2): Last reviewed commit: "test(e2e): probe the full spend read sur..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…racy plus /spend/logs/v2 pagination The spend suite assumed its three driver deployments (gemini-2.5-flash, claude-haiku-4-5, openai-text-embedding-3-small) were baked into the proxy config, so on any proxy without them every test failed with invalid model name. A session fixture now registers whichever are missing via /model/new, carrying the provider key from the runner's env when set (a local proxy container without the key still works) or an os.environ reference otherwise, and deletes only what it created so config-baked deployments on stage are never touched. Two accuracy behaviors ported from the CircleCI integration suites, both verified against the live stage deployment: a six-way concurrent burst on one key must land six distinct costed rows whose sum equals the key aggregate (the lost-increment contract sequential tests cannot regress), and /spend/logs/v2 must cap pages by page_size, keep total stable on an out-of-range page, and report zero for a no-match filter. The v2 filter matches the hashed token as stored on rows, not the raw sk- key, so the test reads the filter value off the polled rows
24082bc
into
litellm_internal_staging
2 participants
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewStacked on #32261; the first two commits here are that PR and this one should merge after it
Screenshots / Proof of Fix
Every added route probed against the live split stage deployment (kubectl port-forward of litellm-backend:4001 as 14001 for the control plane and litellm-gateway:4000 as 14000 for the data plane), with the master key and a one-day date range:
The 422s are the two routes whose required params a bare probe intentionally omits (session_id and deployment grouping); per the suite's healthy definition that still proves the route is wired and the handler ran. The data-plane 404 column shows why the /end_user prefix addition matters: without it the probe of /end_user/daily/activity would have gone to the gateway and failed the way /model/new did before #32261
The full suite run against the same live deployment:
Follow-up commits on this branch, both live-verified against stage: the suite no longer assumes its driver deployments are baked into the proxy config; a session fixture registers any missing driver via /model/new (provider key from the runner's env when set, os.environ reference otherwise) and deletes only what it created. Two accuracy behaviors were ported from the CircleCI integration suites as true e2e tests: a six-way concurrent burst on one key must produce six distinct costed rows summing exactly to the key aggregate (the lost-increment contract), and /spend/logs/v2 pagination must cap rows by page_size, keep total stable for out-of-range pages, and report zero for a no-match filter. One live-contract note: the v2 api_key filter matches the hashed token as stored on rows, not the raw sk- key, so the test reads the filter value off the polled rows
Type
Test
Changes
The spend read surface is mostly served with
include_in_schema=False, so the schema-discovery test misses roughly 70% of it and the curatedSPEND_ROUTESlist is the real coverage. That list was missing twelve read routes:/spend/logs/v2,/spend/logs/session/ui,/global/all_end_users,/global/activity/exceptions/deployment, and the seven per-entity daily activity routes (/user/daily/activity,/user/daily/activity/aggregated,/team/daily/activity,/organization/daily/activity,/customer/daily/activity,/end_user/daily/activity,/tag/daily/activity). All are now probed/end_useris added toCONTROL_PLANE_PREFIXESbecause/end_user/daily/activityis a management route that the data-plane gateway 404s on the split deployment, and the transport routing test gains rows for it and the daily-activity familyDeliberately excluded, with the reasons documented next to the list: path-param routes, the POST readers (
/spend/calculatealready has a dedicated test), the mutating/global/spend/resetand/global/spend/refresh, and/provider/budgets, which raises a 500 wheneverrouter_settings.provider_budget_configis not set, so it cannot be probed green on a proxy without provider budget routing. That 500 on an unconfigured feature arguably deserves a product fix to return an empty object instead; left out of scope here