fix(rag): track LLM completion usage and spend for /v1/rag/query - #32438
Conversation
|
|
Greptile SummaryThis PR fixes zero-spend tracking for
Confidence Score: 5/5Safe to merge — the changes are well-scoped to the RAG pipeline, backed by end-to-end proof with real billing data, and do not touch the core authentication or routing paths. The core billing fix follows an established pattern already used by the file-search emulation handler, and the non-streaming/streaming cost-folding paths are logically disjoint (hidden_params vs. model_call_details) with no double-counting possible. All new tests use try/finally teardown for global state and mock at the transport layer only. The single non-streaming edge case where sub_call_cost can be lost (completion cost absent from hidden_params) is a cost-accounting gap in a failure path, not a correctness regression on the happy path. litellm/rag/main.py — the non-streaming sub_call_cost folding silently drops search/rerank cost when the completion's response_cost is absent from hidden_params (failure-path gap).
|
| Filename | Overview |
|---|---|
| litellm/rag/main.py | Core fix: wraps vector store search, rerank, and completion sub-calls in _suppressed_sub_call_billing so only the parent aquery event fires; folds sub-call costs via hidden_params on non-streaming path and additional_response_cost on streaming path. Minor gap: sub_call_cost is silently dropped on the non-streaming path when the completion's response_cost is absent from hidden_params. |
| litellm/litellm_core_utils/litellm_logging.py | Adds additional_response_cost accumulation in _response_cost_calculator — reads the key from model_call_details, guards with isinstance and > 0, and adds it on top of the computed completion cost. Correctly scoped to the streaming path since non-streaming cost is folded via hidden_params["response_cost"] instead. |
| litellm/proxy/rag_endpoints/endpoints.py | Adds x-litellm-response-cost and related headers to the RAG query response, and routes streaming CustomStreamWrapper results through select_data_generator wrapped in StreamingResponse — fixing the HTTP 500 that previously occurred when FastAPI tried to serialize the raw stream wrapper. |
| litellm/types/utils.py | Registers ingest, aingest, query, and aquery in the CallTypes enum — previously missing entries for these @client-decorated entry points, which caused deployment hooks and call-type-driven logic to silently no-op for RAG calls. |
| tests/test_litellm/rag/test_main.py | New unit tests covering: single billing event with completion usage, hidden_params cost, priced vector store cost folding, rerank cost folding, and streaming additional_response_cost. All tests use try/finally to restore litellm.callbacks, addressing the previous teardown concern. The polling pattern (50 × 0.1s + 0.5s sleep) remains but is a pre-existing pattern. |
| tests/test_litellm/proxy/rag_endpoints/test_rag_endpoints.py | Adds regression tests for the cost header on non-streaming responses and for stream=true returning an SSE response (content-type, chunk structure, [DONE] sentinel). Both tests mock litellm.aquery at the endpoint layer — no real network calls. |
| ui/litellm-dashboard/src/lib/http/schema.d.ts | Generated TypeScript schema update reflecting the four new CallTypes enum values (ingest, aingest, query, aquery). |
Reviews (5): Last reviewed commit: "fix(rag): track LLM completion usage and..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Addressed the test cleanup in f87dcd5: the aquery billing test now restores litellm.callbacks in a finally block. On the P1 about the rerank step: leaving arerank outside the is_internal_call guard is deliberate. The search and completion sub-calls forward **kwargs, so they share the parent litellm_logging_obj and would consume its single billing slot; that is what the guard prevents. The arerank call does not forward kwargs (litellm/rag/main.py, step 3), so it creates its own logging object and bills itself independently, exactly as it did before this PR. Wrapping it in is_internal_call would not consolidate its cost into the aquery record; it would suppress the rerank's only billing event while the parent aquery event still prices only the completion response, so the rerank spend would be dropped entirely. Consolidating rerank cost into the parent record would require summing costs across logging objects, which is a behavior change beyond this fix. The emulated file-search handler has the same structure: only the nested calls that share the parent logging object are marked internal, while its vector store search bills itself. Rerank spend attribution (the standalone rerank event carries no proxy key metadata) is a pre-existing gap that deserves its own ticket @greptileai please review the current head f87dcd5 |
PR overviewThis pull request updates the RAG query path for The PR has made progress, with two issues already addressed, but one billing gap remains open. An authenticated streaming caller can disconnect before end-of-stream finalization and avoid spend tracking for completed vector search and rerank sub-calls. The remaining impact is limited to usage and cost accounting rather than direct data access or code execution. Open issues (1)
Fixed/addressed: 2 · PR risk: 5/10 |
|
@greptileai please review the current head ce96b0a (two test-only commits since your last review: router-branch coverage for the aquery billing test, and pinning the vector store globals so the cost header test is order-independent) |
|
@greptileai please review the current head 72ffe4b. Since your last review: the vector store search cost is now folded into the aquery billing event (review feedback from the security pass), and the rerank concern from your review is resolved for real; the rerank sub-call now runs under the internal-call context and its cost is folded into the same single aquery event, so spend is no longer split across two rows |
72ffe4b to
eb51805
Compare
3c63d0d to
10e9af4
Compare
|
@greptileai please review the current head 10e9af4 |
10e9af4 to
b71998b
Compare
| custom_llm_provider=retrieval_config.get("custom_llm_provider", "openai"), | ||
| **kwargs, | ||
| ) | ||
| with _suppressed_sub_call_billing(): |
There was a problem hiding this comment.
Medium: Client disconnects bypass sub-call billing
The vector search, and the rerank below, finish before the streaming response is returned, but their own billing events are suppressed. An authenticated caller can request stream=true and disconnect before end-of-stream processing; CustomStreamWrapper.aclose() closes the provider stream without emitting the parent success event that consumes additional_response_cost, so the completed search and rerank calls never enter spend tracking. Keep these sub-calls on separate billing events, or explicitly finalize the accumulated sub-call cost during stream cancellation.
There was a problem hiding this comment.
This is the proxy-wide streaming disconnect behavior rather than a gap this PR introduces. When a streaming client disconnects mid-stream, neither the success nor the failure logging callback fires for the request (documented on _release_max_parallel_requests_on_disconnect in litellm/proxy/utils.py, which exists precisely because of that), so the completion tokens already streamed are not billed either; that holds for /chat/completions and every streamed route today. The sub-call cost rides the same single billing event as the completion, so it bills exactly when the platform bills. Verified live on this branch: a fully drained stream=true query writes the aquery SpendLogs row with completion plus sub-call cost, and a mid-stream disconnect writes no row at all, completion tokens included, which is the platform contract this PR inherits rather than creates
Splitting the sub-calls back onto standalone events would reintroduce the two defects fixed earlier in this review: a direct vector_stores.asearch billing event prices through the web-search call_type branch at 0 (the vector store branch is keyed on avector_store_search, which only the router factory sets), and a standalone rerank event carries no key metadata, so both would under-bill or orphan spend on every streamed request instead of only on early disconnects. Finalizing only the sub-call cost at cancellation would bill a request's search while dropping its typically larger completion tokens, which is inconsistent partial accounting. Disconnect-time finalization of streamed spend is platform infrastructure that should cover all streamed routes in one change; happy to see it tracked separately, but it is out of scope for this fix
There was a problem hiding this comment.
Thanks for the detail. I can't automatically confirm this is safe to dismiss, so I'm leaving the thread open for a maintainer to make the call.
There was a problem hiding this comment.
Follow-up: the platform-wide disconnect billing gap this finding pointed at is now fixed in #33736, which finalizes partial streamed spend (completion tokens plus the folded sub-call cost) at disconnect time in the shared streaming cleanup, covering every streamed route rather than only this pipeline
215ce9f
into
litellm_internal_staging
Relevant issues
Linear ticket
Resolves LIT-4187
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
All runs below are against a live proxy (
python litellm/proxy/proxy_cli.py --config config.yaml --port 4187) backed by a real Postgres, with a real OpenAI vector store and realgpt-4o-minicalls costing real money. Setup used for both runs:Before (commit d6cbf6e, branch base)
The query succeeds and the response body carries real usage (
"total_tokens": 49), but there is nox-litellm-response-costheader, the SpendLogs row for the request is zeroed, and key spend only reflects the/v1/chat/completionscontrol request made on the same key:After (commit 2124e20)
Same curl. The cost header is returned and the request is tracked with its real tokens and cost, attributed to the key:
Exactly one SpendLogs row is written per RAG query (the vector store search and the underlying completion do not double-bill), and the
/v1/chat/completionscontrol on the same proxy still tracks as beforeBudget enforcement (commit 2124e20)
A key with
"max_budget": 0.00001makes one successful RAG query (cost 1.365e-05), then the second query is rejected. Before this fix, RAG spend never accrued so this limit was unenforceable:Streaming (commit b71998b)
On the previous head, the same streamed query against a live proxy returned HTTP 500 (
PydanticSerializationErrorfrom FastAPI trying to serialize the raw stream wrapper) and wrote no SpendLogs row. After this round, against a live proxy on port 4244 backed by a real Postgres, with the same real OpenAI vector store flow and a realgpt-5.5completion:Once the stream drains, the request bills exactly one
aqueryrow with the completion's real usage, attributed to the key:OpenAI prices vector store search at 0 per query, so the folded sub-call cost is 0 here; the fold arithmetic for priced providers (e.g. Vertex AI search API) and for rerank is pinned by the unit tests
Independent e2e verification
Reproduced independently on this branch (commit 2124e20) against a live proxy on port 4000 backed by a real Postgres, with a real OpenAI vector store and real
gpt-4o-minicalls. A virtual key withmax_budget1.0 ingested a small text doc, then issued one/v1/rag/query; the response carried bothx-litellm-response-costandx-litellm-key-spend(3.87e-05), theaquerySpendLogs row recordedspend 3.87e-05with122/34/156prompt/completion/total tokens as the only billing row, and the key spend incremented to match. A second key withmax_budget0.00001succeeded once then returned HTTP 429budget_exceededon an identical second query after the spend flushed, confirming budget enforcement now applies to RAG trafficFull terminal walkthrough:
Response headers carrying the cost:
SpendLogs aquery row and matching key spend:
Budget enforcement returning HTTP 429:
Type
🐛 Bug Fix
Changes
The RAG query pipeline (
litellm/rag/main.py) forwards its kwargs, including the parentlitellm_logging_objinjected by the@clientwrapper, into its sub-calls (vector_stores.asearch, thenrouter.acompletion/litellm.acompletion). All three calls therefore share one logging object, andshould_run_logging()allows only oneasync_successevent per object. The vector store search finishes first and consumes the slot, so the completion's usage and cost never reach_PROXY_track_cost_callbackand SpendLogs records the request with zero tokens and zero spend, bypassingmax_budgetThe fix wraps the search and completion sub-calls in the
is_internal_callcontext, the same pattern the emulated file-search handler uses for its nested calls, so the parentaqueryevent bills exactly once with the final completion response. Additionally,query/aquery/ingest/aingestare registered in theCallTypesenum (they were the only@cliententry points missing from it), and the/v1/rag/queryroute now sets the standard response headers sox-litellm-response-costis returned like other endpoints.schema.d.tsis regenerated for the enum changeReview follow-ups: the pipeline now also folds sub-call costs into that single billing event. Vector store search cost (per-query pricing, e.g. Vertex AI search API; 0 for providers without search pricing) is computed via the existing
vector_store_search_costhelper, and the optional rerank sub-call runs under the same internal-call context with its already-computed response cost added in, instead of firing a standalone billing event that carried no proxy key metadata. The aquery SpendLogs row therefore carries completion plus search plus rerank cost, all attributed to the calling keyStreaming is handled by carrying the accumulated sub-call cost into the stream's single final billing event, as suggested in review. On the non-streaming path the fold mutates the response's hidden
response_costas before; on the streaming path there is no response object to fold into (the cost is computed from the assembled chunks after the pipeline returns), so the pipeline stores the search plus rerank cost on the parent logging object asadditional_response_costandLogging._response_cost_calculatoradds it when it prices the assembled stream. All sub-calls are therefore suppressed on both paths and their cost lands on the oneaqueryevent, which also restores proxy key attribution for rerank spend on streamed queries (its previous standalone event carried no key metadata)While verifying this on a live proxy it turned out
stream=trueagainst/v1/rag/queryfailed outright with a 500: the route returned the raw stream wrapper, which FastAPI cannot serialize, so the stream never drained and its billing event never fired at all. The route now returns a propertext/event-streamStreamingResponseviaselect_data_generator, the same shape the other LLM routes use, which is also what lets the stream drain and billTests:
tests/test_litellm/rag/test_main.pydriveslitellm.aquerythrough the real@clientwrappers (mock transport only) and asserts the single billing event carries the completion's usage, cost, andaquerycall type; it fails on the unfixed code because the event then carries the vector store search response. A streaming test asserts the one streamedaquerybilling event includes the priced search and rerank cost, and fails without theadditional_response_costcarry.tests/test_litellm/proxy/rag_endpoints/test_rag_endpoints.pygains regression tests for the cost header and forstream=truereturning an SSE response instead of a 500Link to Devin session: https://app.devin.ai/sessions/7481bf2805db40948c206bf32d349ffc