fix(proxy): restore spend logging for /cursor/chat/completions (#30126) - #30277
fix(proxy): restore spend logging for /cursor/chat/completions (#30126)#30277discobot wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryRestores spend logging for
Confidence Score: 4/5The production change is a single-entry addition to a routing tuple with no behavioral impact outside the cursor endpoint; safe to merge. The core fix is minimal and correct. The integration test covers the regression path end-to-end but accesses call_args.kwargs without first asserting the mock was actually invoked, which would produce a confusing AttributeError rather than a clear failure if the endpoint ever returned 200 without reaching aresponses. tests/test_litellm/proxy/response_api_endpoints/test_endpoints.py — the new integration test should assert mock_router.aresponses.assert_called_once() before accessing call_args.
|
| Filename | Overview |
|---|---|
| litellm/proxy/litellm_pre_call_utils.py | Adds /cursor/chat/completions to LITELLM_METADATA_ROUTES so proxy auth fields are written to litellm_metadata instead of the user-facing metadata param; minimal and correct one-line change. |
| tests/test_litellm/proxy/response_api_endpoints/test_endpoints.py | Adds TestCursorChatCompletionsSpendTracking integration test; correctly mocks llm_router.aresponses and verifies metadata routing, but accesses call_args.kwargs without first asserting the mock was called. |
| tests/test_litellm/proxy/test_litellm_pre_call_utils.py | Adds a clean unit test for _get_metadata_variable_name with the new /cursor/chat/completions route; no issues. |
Reviews (1): Last reviewed commit: "fix(proxy): restore spend logging for /c..." | Re-trigger Greptile
| assert response.status_code == 200 | ||
| request_kwargs = mock_router.aresponses.call_args.kwargs | ||
| litellm_metadata = request_kwargs.get("litellm_metadata") or {} | ||
| assert litellm_metadata.get("user_api_key") == "hashed-test-key" | ||
| assert litellm_metadata.get("user_api_key_user_id") == "test-user" | ||
| # proxy internals must not leak into the user-facing metadata param | ||
| assert "user_api_key" not in (request_kwargs.get("metadata") or {}) |
There was a problem hiding this comment.
Missing guard before
call_args access
call_args is None when the mock was never invoked, so call_args.kwargs would raise AttributeError rather than producing a useful failure message. If the handler returns 200 through an alternative code path without ever reaching llm_router.aresponses (e.g., a middleware short-circuit or an exception swallowed upstream), the test will blow up with an opaque AttributeError instead of a clear assertion failure. Adding mock_router.aresponses.assert_called_once() immediately after the status check pins the failure message to the actual absence of the call.
…AI#30126) The /cursor/chat/completions route processes requests through the Responses API pipeline, but _get_metadata_variable_name did not treat it as a litellm_metadata route. Proxy auth metadata therefore landed in the user-facing metadata param, so the spend tracking callback saw user_api_key=None and silently skipped the SpendLogs write. Add the route to LITELLM_METADATA_ROUTES and cover it with regression tests.
2a6664e to
efcb0f8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efcb0f8b02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "/v1/messages", | ||
| "responses", | ||
| "files", | ||
| "/cursor/chat/completions", |
There was a problem hiding this comment.
Preserve proxy metadata for callbacks with client metadata
When a /cursor/chat/completions request includes the Responses API metadata field, this route now puts proxy auth fields only in litellm_metadata while leaving the client metadata in metadata. litellm.utils.function_setup only copies litellm_metadata into litellm_params["metadata"] when metadata is absent, and callbacks such as the Lago integration read user_api_key_user_id / user_api_key_team_id only from litellm_params["metadata"], so Cursor calls with user metadata lose callback billing/attribution even though the spend log path is fixed. Please also mirror the proxy identity metadata for callback consumers or update those consumers to read litellm_metadata.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The responses pipeline overrides this in update_from_kwargs, which sets the logging metadata from kwargs["litellm_metadata"] whenever it is present (litellm/responses/main.py), so a client supplied metadata field lands in optional_params for the provider request body while litellm_params["metadata"] keeps the proxy auth fields. I verified with a spy CustomLogger passing both metadata and litellm_metadata through aresponses; the callback saw user_api_key, user_api_key_user_id and user_api_key_team_id in both litellm_params["metadata"] and get_litellm_metadata_from_kwargs, matching the existing /v1/responses route which has been in LITELLM_METADATA_ROUTES all along
|
@discobot — could you add a screenshot or short video showing that this change works as expected? It really helps reviewers verify the fix quickly. Thanks! |
|
Added a real-provider run to the PR description under Proof of Fix. Short version: on a live proxy with a Postgres SpendLogs table, a |
Relevant issues
Fixes #30126
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Re-ran the A/B against a live proxy with a real provider and a Postgres-backed SpendLogs table, so the row actually lands instead of hitting a mock. The OpenAI key I had was rate-limited, so the model here is
claude-haiku-4-5; the fix is proxy-side metadata routing so it is provider-agnostic, and the cursor response-format conversion is OpenAI-specific and separate from spend tracking, so the check is the SpendLogs write itself (which is what #30126 reports missing)Config is one model
claude-haiku-4-5 -> anthropic/claude-haiku-4-5-20251001,master_key: sk-1234,DATABASE_URLset. Generate a key, hit the cursor route, and read SpendLogs (truncating it before each run so the diff is clean):Without the fix (current
litellm_internal_staging) the cursor call's cost is never recorded; only the proxy-level row lands, at spend 0:With the fix the same call writes the LLM spend row with the real cost:
So proxy auth metadata now reaches the spend callback and the SpendLogs write for
/cursor/chat/completionsfires the same way it does for/v1/chat/completionsType
🐛 Bug Fix
Changes
/cursor/chat/completionsis served through the Responses API pipeline, which keeps proxy-internal metadata underlitellm_metadatabecausemetadatais the user-facing Responses API param. The route was missing fromLITELLM_METADATA_ROUTES, soadd_litellm_data_to_requestwroteuser_api_keyand friends into the user-facingmetadataparam, the spend tracking callback sawuser_api_key=None, and the SpendLogs write was silently skipped; hence 200 + provider billed + nothing in the Logs UIThe fix adds the route to
LITELLM_METADATA_ROUTES, matching/v1/responseswhich this endpoint bridges to. Two regression tests cover it (the_get_metadata_variable_namemapping and an endpoint test asserting the kwargs reachingllm_router.aresponsescarrylitellm_metadata.user_api_key); both fail without the one line change