Revert "[Fix] Team Usage Spend Truncated Due to Pagination"#23028
Revert "[Fix] Team Usage Spend Truncated Due to Pagination"#23028yuneng-jiang merged 1 commit intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR reverts #22938 ("Fix Team Usage Spend Truncated Due to Pagination"), re-introducing the original bug where
The test that validated the aggregated-with-entity-breakdown behavior ( Confidence Score: 1/5
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/team_endpoints.py | Reverts to paginated get_daily_activity, reintroducing the spend truncation bug and silently dropping timezone parameter support. |
| litellm/proxy/management_endpoints/common_daily_activity.py | Reverts _build_aggregated_sql_query and get_daily_activity_aggregated to always skip entity breakdown; list-based api_key filtering in the SQL builder also removed. |
| tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py | Updates mocks to match reverted function names; removes the test that validated aggregated-with-entity-breakdown behavior, reducing coverage. |
Sequence Diagram
sequenceDiagram
participant Client
participant Endpoint as GET /team/daily/activity
participant DB as litellm_dailyteamspend
Note over Endpoint,DB: BEFORE revert (PR #22938 — correct)
Client->>Endpoint: GET /team/daily/activity?team_ids=X&timezone=480
Endpoint->>DB: SQL GROUP BY (all rows, no pagination)
DB-->>Endpoint: ~2-3k aggregated rows
Endpoint-->>Client: SpendAnalyticsPaginatedResponse (accurate totals, per-team breakdown)
Note over Endpoint,DB: AFTER revert (this PR — buggy)
Client->>Endpoint: GET /team/daily/activity?team_ids=X
Endpoint->>DB: find_many(skip=0, take=10)
DB-->>Endpoint: 10 rows (page 1 only)
Endpoint-->>Client: SpendAnalyticsPaginatedResponse (truncated totals — rest of data ignored)
Comments Outside Diff (1)
-
litellm/proxy/management_endpoints/team_endpoints.py, line 3884-3893 (link)Backward-incompatible removal of
timezoneparameterThe
timezonequery parameter was removed from the endpoint signature. Any callers (UI or API consumers) that were already passing?timezone=480(or any offset) to/team/daily/activitywill silently get UTC-based results instead of timezone-adjusted ones, with no error or warning. This is a silent data correctness regression — the date boundaries used for filtering will be wrong for users in non-UTC timezones.Per the repository's convention, removing a previously-supported parameter without a feature flag or deprecation period breaks existing behaviour for users relying on it.
Last reviewed commit: 034e83e
| return await get_daily_activity( | ||
| prisma_client=prisma_client, | ||
| table_name="litellm_dailyteamspend", | ||
| entity_id_field="team_id", | ||
| entity_id=team_ids_list, | ||
| entity_metadata_field=team_alias_metadata, | ||
| exclude_entity_ids=exclude_team_ids_list, | ||
| start_date=start_date, | ||
| end_date=end_date, | ||
| model=model, | ||
| api_key=final_api_key_filter, | ||
| exclude_entity_ids=exclude_team_ids_list, | ||
| timezone_offset_minutes=timezone, | ||
| include_entity_breakdown=True, | ||
| page=page, | ||
| page_size=page_size, | ||
| ) |
There was a problem hiding this comment.
Pagination truncation bug reintroduced
This revert switches get_team_daily_activity back to the paginated get_daily_activity, which only fetches page_size (default 10) rows from the database per request. The _aggregate_spend_records call inside get_daily_activity then computes totals (total spend, tokens, API requests, etc.) from only those paginated rows, not from the full dataset.
For teams with hundreds or thousands of rows in litellm_dailyteamspend (e.g. many models × many dates × many api keys), the metadata.total_spend and all other aggregate metrics in the response will be systematically understated — reflecting only the current page, not the true total. This is the exact bug PR #22938 was introduced to fix.
The correct approach is to use get_daily_activity_aggregated (which performs a SQL GROUP BY and returns all aggregated rows in one query), or to compute totals with a separate SUM() query over the full filtered result set before applying pagination.
# Before revert (correct):
return await get_daily_activity_aggregated(
...
include_entity_breakdown=True,
)
# After revert (buggy — totals only reflect current page):
return await get_daily_activity(
...
page=page,
page_size=page_size,
)| aggregated = await _aggregate_spend_records( | ||
| prisma_client=prisma_client, | ||
| records=records, | ||
| entity_id_field=entity_id_field if include_entity_breakdown else None, | ||
| entity_metadata_field=entity_metadata_field if include_entity_breakdown else None, | ||
| entity_id_field=None, | ||
| entity_metadata_field=None, | ||
| ) |
There was a problem hiding this comment.
get_daily_activity_aggregated loses all entity breakdown capability
After this revert, _aggregate_spend_records is always called with entity_id_field=None and entity_metadata_field=None, unconditionally. This means get_daily_activity_aggregated can never produce per-entity (per-team, per-user, per-org) spend breakdowns — even if a caller explicitly wants them.
The include_entity_breakdown parameter that controlled this was removed, and the function is now equivalent to the version before PR #22938 except that the SQL still correctly aggregates without the entity dimension in GROUP BY. Any future callers expecting entity-level breakdown from this function will silently get a flat aggregation instead.
If this revert is intentional as a short-term rollback, the entity breakdown capability should be restored before the next release.
Reverts #22938