fix(batches): mark terminal batch with no output file as processed in CheckBatchCost - #35360
Conversation
|
|
Greptile SummaryThe PR finalizes terminal batches that have no billable output and preserves billing for expired batches with output.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the only previous concern was acknowledged as a deliberate legacy-schema constraint rather than a defect introduced by this change.
|
| Filename | Overview |
|---|---|
| enterprise/litellm_enterprise/proxy/common_utils/check_batch_cost.py | Expands terminal finalization and preserves the provider's expired status after successful cost tracking; no eligible follow-up defect remains. |
| tests/proxy_unit_tests/test_check_batch_cost.py | Updates terminal-status coverage and adds focused regressions for completed batches without output, non-terminal batches, and expired batches with output. |
Reviews (3): Last reviewed commit: "fix(batches): persist real terminal stat..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… CheckBatchCost A managed batch whose request lines all failed can reach a terminal provider status (completed) with output_file_id=None and only an error_file_id. Such a row matched neither the completed-with-output billing branch nor the failed/expired/cancelled branch, so batch_processed stayed False and the poller re-selected it on every cycle for the lifetime of the deployment; output/error file deletion is also gated on batch_processed, so those files could never be deleted. Broaden the terminal handling so a completed/complete/expired batch with an output file is billed, and any terminal batch with nothing to bill (failed/cancelled, or completed/expired with no output) is marked terminal exactly once. Non-terminal statuses (validating/in_progress) are still left for the next poll, and an expired batch that did produce output is now billed. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
95b9cc0 to
1918469
Compare
|
bugbot run |
| ## RETRIEVE THE BATCH JOB OUTPUT FILE | ||
| if ( | ||
| response.status == "completed" | ||
| response.status in ("completed", "complete", "expired") |
There was a problem hiding this comment.
Expired rows still excluded from polling
Medium Severity
The new billing path runs for provider status expired with an output file, but the primary find_many still excludes DB status expired. After GET /v1/batches persists that status (without setting batch_processed), the poller never selects the row, so partial expired spend is still never recorded.
Reviewed by Cursor Bugbot for commit 1918469. Configure here.
There was a problem hiding this comment.
Pre-existing exclusion, unchanged here: expired rows were never billed before either. Removing it would retro-bill every historical expired row; better as a follow-up
|
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 eacea13. Configure here.


TLDR
Problem this solves:
completedbatch with no output file re-polled foreverbatch_processedstuckfalse, so output/error files never deletableHow it solves it:
completed/expiredbatches that did produce an output fileUser Flow
Before: a batch user who never re-checks their batch can never delete its input file once every request line fails; the gateway refuses cleanup forever
all_failed.jsonl(every line invalid at execution time, e.g.temperature: 3) via POST https://litellm-domain/v1/files withpurpose="batch"andtarget_model_names, getting back a unifiedfile-...idstatus: "validating"and their pipeline moves on without polling itcompletedwith anerror_file_idandoutput_file_id: null; nothing was served, and correctly no spend appears at https://litellm-domain/ui/?page=logsexpiredwith a realoutput_file_id) also never shows any spend for its served lines at https://litellm-domain/ui/?page=logsAfter: the same batch is finalized shortly after it ends, cleanup succeeds, and partially-served expired batches are billed
all_failed.jsonl(every line invalid at execution time, e.g.temperature: 3) via POST https://litellm-domain/v1/files withpurpose="batch"andtarget_model_names, getting back a unifiedfile-...idstatus: "validating"and their pipeline moves on without polling itcompletedwith anerror_file_idandoutput_file_id: null; nothing was served, and correctly no spend appears at https://litellm-domain/ui/?page=logs"deleted": trueexpiredbatch with a realoutput_file_idnow shows its served lines' spend at https://litellm-domain/ui/?page=logsRelevant issues
Fixes #35354
Linear ticket
Resolves LIT-5193
Pre-Submission checklist
Screenshots / Proof of Fix
Live two-leg QA against real OpenAI batches. Each leg boots a DB-backed proxy from a fresh worktree with managed files enabled and
PROXY_BATCH_POLLING_INTERVAL=15, then drives the User Flow above with the OpenAI Python SDK pointed at the proxy. Re-checking a batch through the gateway finalizes it on both commits, so after creation neither leg ever touches the batch through the proxy again: batch state is confirmed against the provider directly, the same check an admin makes on their own OpenAI accountBefore, proxy at merge base
9d069f21dc: the batch endscompletedwith only an error file and the input file can never be deleted (times PDT, 2026-08-14)The provider finishes the batch at 14:57 with nothing served (checked directly against OpenAI, not through the proxy):
Every input-file DELETE through the proxy is still refused hours later, identically at 17:19:47, 17:20:47, 17:21:47, and 17:25:44, roughly 2.5 hours and ~88 polling cycles after the batch went terminal:
After, proxy at the PR tip
eacea13a25: identical flow and the very first DELETE attempt succeedsThe batch was finalized 5 seconds after the provider marked it terminal, well inside one 15s polling cycle, and the DELETE fired ~3.5 minutes later succeeded on the first attempt with no proxy-side batch check ever made
QA observations:
Type
🐛 Bug Fix
Caveats (if any)
expired-with-output moved from the persists test to the billing testChanges
CheckBatchCost.check_batch_costhad two arms:completedwith an output file (bill, then mark processed) andfailed/expired/cancelled(mark processed without billing). Acompleted/completebatch withoutput_file_id=Nonematched neither, sobatch_processedstayedfalseand the poller re-selected the same row on every cycle for the life of the deployment; output/error file deletion is gated onbatch_processed, so those files could never be deleted through the managed API either.The billing arm now triggers for
completed/complete/expiredwhen an output file is present, so anexpiredbatch that really produced output is still billed (those request lines were served). It writes back the batch's real terminal status (normalizingcompletedtocompletelike the retrieve endpoint does) instead of hardcodingcomplete, so a billedexpiredbatch staysexpiredin the DB, matching the stored batch JSON. The second arm now covers the full terminal set (completed/complete/failed/expired/cancelled) so any terminal batch with nothing to bill (failed/cancelled, or completed/expired with no output) is marked processed exactly once. Non-terminal statuses (validating/in_progress) still fall through both arms and are left for the next pollThe two report-flagged behaviours are covered as tests: a still-in-progress batch is not written back, and an expired batch with an output file is billed. The
#35131-style case where the provider fetch itself raises is unchanged and out of scope here (that row is already left for retry by the existingexceptaroundaretrieve_batch)On the rebase itself: staging's terminal arm now also mints unified managed file IDs before the write-back (
ensure_batch_response_managed_file_ids), which this branch keeps; and sinceexpired-with-output now takes the billing arm,test_terminal_status_persists_managed_output_file_idsparametrizes onlyfailed/cancelled, with the expired case covered bytest_expired_with_output_file_is_billedFinal Attestation
Note
Cursor Bugbot is generating a summary for commit eacea13. Configure here.