[Fix] Spend Management Tests - #21088
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile OverviewGreptile SummaryThis PR makes two key changes: (1) Backend performance optimization — the spend log list endpoints (
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| litellm/proxy/spend_tracking/spend_management_endpoints.py | Switches list endpoint and session endpoint from Prisma find_many to raw SQL to exclude heavy columns (messages, response, proxy_server_request). Adds DB fallback in the detail endpoint. However, the raw SQL hardcodes ORDER BY "startTime" DESC, ignoring the validated sort_by/sort_order parameters — a functional regression. |
| tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py | Refactors mock infrastructure from per-test MockDB/MockPrismaClient classes to a shared make_ui_spend_logs_mock_prisma factory, updated to mock query_raw instead of find_many. Reduces duplication significantly. Minor formatting issue with missing blank line before imports. |
| ui/litellm-dashboard/src/app/(dashboard)/hooks/logDetails/useLogDetails.ts | New React Query hook for lazy-loading log details (messages/response) on-demand when the drawer opens, replacing the previous prefetch-all approach. Well-structured with appropriate staleTime/gcTime settings. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailContent.tsx | Adds isLoadingDetails prop to show a spinner while lazy-loaded log details are being fetched, and defers the "missing data" warning until loading completes. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailsDrawer.tsx | Integrates the useLogDetails hook, builds an enriched log entry by merging lazy-loaded details with the list data, and passes loading state to LogDetailContent. |
| ui/litellm-dashboard/src/components/view_logs/index.tsx | Removes prefetch-all logic (prefetchLogDetails call and PrefetchedLog type), passes startTime to LogDetailsDrawer for on-demand detail loading. |
Sequence Diagram
sequenceDiagram
participant UI as Dashboard UI
participant List as /spend/logs/ui
participant Detail as /spend/logs/ui/{id}
participant DB as PostgreSQL
participant Logger as Custom Logger (S3/GCS)
UI->>List: GET (filters, pagination)
List->>DB: COUNT(*) via Prisma ORM
List->>DB: Raw SQL (lightweight columns only)
DB-->>List: Rows without messages/response
List-->>UI: Paginated log list
Note over UI: User clicks a log row
UI->>Detail: GET /spend/logs/ui/{request_id}
Detail->>Logger: get_request_response_payload()
alt Custom logger has data
Logger-->>Detail: messages + response
else No custom logger / no data
Detail->>DB: Raw SQL (messages, response, proxy_server_request)
DB-->>Detail: Heavy columns
end
Detail-->>UI: Log details (messages/response)
Note over UI: Drawer shows enriched log entry
Last reviewed commit: ae44022
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return MockPrismaClient() | ||
| from litellm.proxy._types import ( |
There was a problem hiding this comment.
Missing blank line before import
There should be a blank line between the end of the make_ui_spend_logs_mock_prisma function and the module-level import statement. This is a minor formatting issue — the from litellm.proxy._types import ... runs directly into the function's return statement.
| return MockPrismaClient() | |
| from litellm.proxy._types import ( | |
| from litellm.proxy._types import ( |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Greptile OverviewGreptile SummaryThis PR optimizes the spend logs list endpoints (
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| litellm/proxy/spend_tracking/spend_management_endpoints.py | Replaces Prisma find_many with raw SQL query_raw to exclude heavy columns (messages, response, proxy_server_request) for performance; adds DB fallback in the detail endpoint for when no custom logger is configured. Sort parameters are now correctly applied to the raw SQL query. |
| tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py | Refactored all spend log tests to use a shared make_ui_spend_logs_mock_prisma helper that mocks both count and query_raw. Reduces code duplication but sort test mock has a testing weakness where it pre-sorts data based on test parameters rather than verifying the SQL query. |
| ui/litellm-dashboard/src/app/(dashboard)/hooks/logDetails/useLogDetails.ts | New React Query hook for lazy-loading log details (messages/response) on demand when the drawer is opened, replacing the previous prefetch-all approach. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailContent.tsx | Added isLoadingDetails prop to show a spinner while log details are being lazy-loaded and suppress the "missing data" warning during loading. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailsDrawer.tsx | Integrates useLogDetails hook to lazy-load and merge log detail data (messages/response) into the displayed log entry when the drawer is open. |
| ui/litellm-dashboard/src/components/view_logs/index.tsx | Removed prefetch logic for log details, passing startTime to the drawer so it can lazy-load details on demand instead. |
Sequence Diagram
sequenceDiagram
participant UI as UI (SpendLogsTable)
participant Drawer as LogDetailsDrawer
participant ListAPI as GET /spend/logs/ui
participant DetailAPI as GET /spend/logs/ui/{request_id}
participant DB as PostgreSQL (LiteLLM_SpendLogs)
participant Logger as Custom Logger (S3/GCS)
UI->>ListAPI: Fetch logs (page, filters, sort)
ListAPI->>DB: COUNT(*) via Prisma count()
ListAPI->>DB: SELECT (lightweight columns) via query_raw
DB-->>ListAPI: Paginated rows (no messages/response)
ListAPI-->>UI: { data, total, page, total_pages }
UI->>Drawer: User clicks row → open drawer
Drawer->>DetailAPI: Lazy-load details (request_id, startTime)
DetailAPI->>Logger: Check custom loggers for payload
alt Custom logger has data
Logger-->>DetailAPI: { messages, response, proxy_server_request }
else No custom logger / no data
DetailAPI->>DB: SELECT messages, response, proxy_server_request WHERE request_id=$1
DB-->>DetailAPI: Heavy columns for single row
end
DetailAPI-->>Drawer: { messages, response, proxy_server_request }
Drawer->>Drawer: Merge details into enrichedLog
Drawer-->>UI: Display full log details
Last reviewed commit: b29c0bf
| async def mock_query_raw(sql_query, *params): | ||
| # Endpoint uses raw SQL with ORDER BY startTime DESC; mock returns sorted data | ||
| order = {"startTime": "desc"} if sort_by is None else {sort_by: sort_order or "desc"} | ||
| sorted_logs = _sort_logs(base_logs, order) | ||
| page_size = params[-2] if len(params) >= 2 else 50 | ||
| skip = params[-1] if len(params) >= 1 else 0 | ||
| return sorted_logs[skip : skip + page_size] |
There was a problem hiding this comment.
Sort test doesn't verify SQL ordering
The mock_query_raw function reads sort_by and sort_order from the outer test scope and pre-sorts the mock data accordingly. This means the test always passes regardless of whether the endpoint actually incorporates the sort parameters into the SQL query. If the endpoint had a bug where it hardcoded ORDER BY "startTime" DESC, this test would still pass because the mock returns the expected order independently.
Consider parsing the sql_query string inside mock_query_raw to extract the actual ORDER BY clause and sort based on that, so the test validates the endpoint's SQL construction.
| filter_date = ( | ||
| datetime.datetime.fromisoformat(fd.replace("Z", "+00:00")) | ||
| if "T" in fd | ||
| else datetime.datetime.strptime(fd, "%Y-%m-%d %H:%M:%S") | ||
| ) |
There was a problem hiding this comment.
Naive vs aware datetime comparison
When the date filter string does not contain "T", datetime.strptime returns a naive datetime (no timezone info), while log_date (line 38-40) is always timezone-aware (from fromisoformat with +00:00). Comparing aware and naive datetimes in Python raises a TypeError.
In the current test suite, filter dates passed through where_conditions["startTime"] are ISO format strings from isoformat() (which always contain T), so this branch is not triggered today. However, the helper function's docstring and signature suggest general reuse, making this a latent bug.
| filter_date = ( | |
| datetime.datetime.fromisoformat(fd.replace("Z", "+00:00")) | |
| if "T" in fd | |
| else datetime.datetime.strptime(fd, "%Y-%m-%d %H:%M:%S") | |
| ) | |
| filter_date = ( | |
| datetime.datetime.fromisoformat(fd.replace("Z", "+00:00")) | |
| if "T" in fd | |
| else datetime.datetime.strptime(fd, "%Y-%m-%d %H:%M:%S").replace(tzinfo=timezone.utc) | |
| ) |
| filter_date = ( | ||
| datetime.datetime.fromisoformat(fd.replace("Z", "+00:00")) | ||
| if "T" in fd | ||
| else datetime.datetime.strptime(fd, "%Y-%m-%d %H:%M:%S") | ||
| ) |
There was a problem hiding this comment.
Same naive vs aware datetime issue
Same issue as the gte branch above — datetime.strptime without .replace(tzinfo=timezone.utc) produces a naive datetime.
| filter_date = ( | |
| datetime.datetime.fromisoformat(fd.replace("Z", "+00:00")) | |
| if "T" in fd | |
| else datetime.datetime.strptime(fd, "%Y-%m-%d %H:%M:%S") | |
| ) | |
| filter_date = ( | |
| datetime.datetime.fromisoformat(fd.replace("Z", "+00:00")) | |
| if "T" in fd | |
| else datetime.datetime.strptime(fd, "%Y-%m-%d %H:%M:%S").replace(tzinfo=timezone.utc) | |
| ) |
[Fix] Spend Management Tests
1 participant
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
✅ Test
Changes
This PR merges a contributor's PR #20720 and adjusts the tests around spend management. The previous tests only mocked the
find_manybut the new flow usesraw_query, so all the ui spend tests needed to be upgraded. This PR refactors the spend tests to reduce code reuse as well as ensure they all work with the newraw_queryapproach.All tests passing:
