fix(spend): give a batch's cost row a primary key of its own - #36876
Conversation
Greptile SummaryThis PR changes spend-log request ID selection so batch cost rows use stable, namespaced batch identities instead of response hashes
Confidence Score: 5/5The PR appears safe to merge No blocking failure remains
|
| Filename | Overview |
|---|---|
| litellm/proxy/spend_tracking/spend_tracking_utils.py | Replaces response hashing with ordered ID resolution and namespaces batch cost rows to prevent primary-key collisions |
| tests/test_litellm/proxy/spend_tracking/test_spend_tracking_utils.py | Adds focused regression tests for redacted responses, stable batch identity, distinct batches, and creation-row separation |
| basedpyright-code-budget.json | Lowers resolved type-check diagnostic ceilings |
| ruff-strict-budget.json | Lowers strict lint ceilings after removing obsolete hashing code |
| type-discipline-budget.json | Ratchets down type-discipline limits associated with the cleanup |
Reviews (4): Last reviewed commit: "test(spend): annotate the batch cost row..." | Re-trigger Greptile
|
Added Final to both constants. Worth noting no other test file under tests/test_litellm uses it, so the convention currently lives in source only. |
request_id is the primary key of LiteLLM_SpendLogs and the flush inserts with
skip_duplicates, so a spend log whose id already exists is dropped with no error
raised and a "processed 1 spend log" line still logged. Batch cost accounting
produced exactly such an id twice over, and on a proxy with message redaction
enabled no batch cost row could be written at all.
get_spend_logs_id derived the id by md5-hashing the response for two call types,
aretrieve_batch and acreate_file. Redaction makes that hash a constant:
perform_redaction returns the fixed {"text": "redacted-by-litellm"} placeholder
for any shape it cannot redact, which is what a batch object and a file body both
become, so every such row hashed to md5('{"text": "redacted-by-litellm"}') =
00fcbef15a3b0097e14b0ca016ed30a0 regardless of provider, user, or amount. The
first row to claim that id owned it and every later row was discarded. Verified
against a live proxy: four payloads spanning two providers and three distinct
spend values all computed that id, and the table held one acreate_file row dating
to 2025-05-25, the row that had claimed it.
Keying off the batch's own identity instead is necessary but not sufficient,
because creating a batch already writes an acreate_batch row under exactly that
id, so the cost row becomes a duplicate of the batch's own creation row. Also
verified live: after the hash was removed the poller computed and flushed a
batch's cost, and the only row carrying that id was the acreate_batch row from
when the batch was submitted.
The id now comes from the response's own id, then the standard logging payload's
id, then litellm_call_id, and a batch cost row is namespaced with a _batch_cost
suffix so it cannot collide with the creation row. The middle term is what keeps
this correct under redaction: that payload is built from the unredacted response,
so it still carries the batch id after redaction has flattened the body. Keying
the cost row to the batch rather than to the call also keeps accounting the same
batch twice collapsing to one row instead of billing it twice. Every other call
type still derives its key exactly as before.
Cost and usage themselves are unaffected by redaction: the token columns fall back
to the standard logging payload and spend comes from its response_cost, neither of
which redaction touches. generate_hash_from_response had no other caller and is
removed with it.
3fb82ab to
363e3f3
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 363e3f3. Configure here.
mateo-berri
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution!
3ac2fbe
into
BerriAI:litellm_internal_staging

TLDR
Problem this solves:
How it solves it:
User Flow
Before: a team running batch jobs through the gateway with message redaction on never sees a completed batch's cost in their spend logs
litellm_settings: turn_off_message_logging: true) and restarts the proxypurpose: batch,target_model_names: <model>) and gets back a gateway file idinput_file_idand get back a batch id withstatus: validatingstatus: completed00fcbef15a3b0097e14b0ca016ed30a0) for every upload and every batch on the proxy, so the batch's tokens and cost never reach the key's or team's spendAfter: the same batch lands in the spend logs as its own entry with its real cost
litellm_settings: turn_off_message_logging: true) and restarts the proxypurpose: batch,target_model_names: <model>) and gets back a gateway file idinput_file_idand get back a batch id withstatus: validatingstatus: completedcall_type: aretrieve_batch, request id{batch_id}_batch_cost, and the batch's real token counts and cost, next to the batch creation row keyed by the batch id and the file upload row keyed by the provider's own file id instead of one fixed id shared with every other uploadRelevant issues
Sibling finding from the same investigation as #36638, which fixed message redaction aborting success logging on a batch output body. Independent of it, since this bug predates that PR and neither fix depends on the other.
#36877 is the companion change that stops a batch's cost being accounted twice, or not at all when a caller's retrieve races the cost poller. That one decides whether the cost gets computed; this one decides whether the resulting row can be stored.
Linear ticket
Resolves LIT-5621
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Same end-user flow run twice against live proxies, each on a fresh Postgres database, with message redaction on and a real OpenAI managed batch (
openai/gpt-5.4-nano, two chat requests, real $). Config used by both:Before, at 6704a10 (merge base), proxy on port 29010
No
aretrieve_batchrow anywhere. The proxy did compute and try to write the batch's cost, under the same fixed id the file upload already occupied, so the flush silently dropped it:After, at 363e3f3 (this PR's tip), proxy on port 51449
The batch cost row lands under
{batch_id}_batch_costwith the batch's real usage (23 prompt, 8 completion tokens, the same $7.3e-06 the before proxy computed and lost), the batch creation row is keyed by the batch id, and the file upload row is keyed by the provider's file id. The seven in-progress polls wrote no spend row on either proxy, only the completed poll did:Type
🐛 Bug Fix
Caveats (if any)
Changes
get_spend_logs_idno longer hashes the response. Every call type now derives its id the same way, preferring the response's own id, then the standard logging payload's id, thenlitellm_call_id.The middle term is what keeps this correct under redaction. That payload is built from the unredacted response, so it still carries the batch id after redaction has flattened the body. Without it a redacted batch falls through to the per-call id, which writes a fresh row per retrieve, each carrying the same batch's full cost, and overstates spend by however many times the caller polled.
A batch cost row is namespaced with a
_batch_costsuffix so it cannot collide with theacreate_batchrow written when the batch was submitted. This follows the existing precedent for cache hits, which already suffix the id to avoid duplicating a request id.Cost and usage themselves are unaffected by redaction and were verified to stay correct in both modes: the token columns fall back to the standard logging payload and spend comes from its
response_cost, neither of which redaction touches.generate_hash_from_responsehad no other caller and is removed with it.Final Attestation
Note
Cursor Bugbot is generating a summary for commit 363e3f3. Configure here.