Repository navigation
Conversation
|
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
4f435d7 to
731213e
Compare
|
@greptileai Please review the current commit, including creator billing isolation, concurrent budget admission, and the fixes to your previous findings |
|
bugbot run Please review commit 731213e for billing isolation, creator budget enforcement, cancellation handling, and regressions after rebasing |
731213e to
f974ac8
Compare
|
@greptileai Please review f974ac8: budget ownership, configured pricing, cancellation, and expiring model holds were rebuilt with focused regressions |
|
bugbot run Please review f974ac8 for nested-call ownership, cancellation cleanup, configured pricing, and per-request budget hold settlement |
f974ac8 to
a5922fa
Compare
|
@greptileai Please review this revision for atomic creator budgets, empty fallback handling, cancellation safety, and configured deployment pricing |
|
bugbot run Please review the latest revision, especially atomic budget settlement, budget-window expiry, empty fallback lists, and nested SDK calls |
a5922fa to
0e460ab
Compare
|
@greptileai Please review the latest commit, including selected-deployment pricing through auto-routing, atomic creator budgets, and nested SDK ownership |
|
bugbot run Please review the latest commit, especially selected-deployment pricing through auto-routing, atomic settlement, and empty fallback handling |
|
bugbot run Please recheck failure settlement against the Redis evidence in the description. Shared entries retain applied_adjustment, so callbacks apply only deltas |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 0e460ab. Configure here.
TLDR
Problem this solves:
How it solves it:
Intentional product change: The evaluation form identifies the initiating admin as payer
User Flow
Before: the sampled user pays for an admin's evaluation, and the admin's limits do not stop it
POST https://litellm-domain/auto_router/shadow_eval/startselecting a user and 100% samplingPOST https://litellm-domain/v1/chat/completionsAfter: the admin pays for evaluation work within their configured budgets
Changes
Evaluation work captures the creator separately from the sampled request's routing and access context. Database receipts, collector events and billing exports charge the creator, clearing sampled user, key, team, organization, project, agent and tag attribution
Each SDK attempt owns its reservation. Nested adapters share that ownership, while fallback attempts and cache hits get separate receipt state. Delayed callbacks retain the reservation and cost from their original attempt
Admission prices the selected deployment through the router and enforces its logical model group with the existing total-user budget engine. Per-model reservations use unique expiring holds, separate from recorded spend. Admission reads spend and acquires a hold atomically. Settlement records actual cost and removes that hold in the same operation; cancellation finishes and refunds any interrupted acquisition. Ordinary user-model admission includes outstanding evaluation holds
Missing or unreadable named creators withhold evaluation work. Invalid optional end-user attribution cannot bypass authenticated model budgets. There are no workflow, dependency, migration or lint-budget changes
Linear ticket
Resolves LIT-9199
Pre-Submission checklist
Validation
make checkpasses on the current commit. The combined SDK, budget, auth and cache run passes 949 tests. One unchanged auth subprocess stalls during Python shutdown; its exact probe and original assertions pass separately. The model-limiter suite passes 65 tests, and four integration cases exercise real RedisTargeted mutations killed 13 budget regressions and seven SDK regressions. These cover routing prices, ownership, cache receipt isolation, fallback admission, concurrent settlement, lease expiry and cancellation
Real Redis verification used four separate processes and 50 competing admissions under a budget of 2.5 estimates. Two calls were admitted and 48 rejected: $0.056 remained in temporary holds, recorded spend stayed zero during execution, and settlement left $0.014 actual spend
Delayed Redis responses exercised repeated cancellation after total and model writes had committed. Both scopes returned to zero. Expired or released holds could neither remove another request's reservation nor be renewed. A Redis error reading recorded spend rejected admission and refunded its hold
Admission and settlement passed a forced concurrent-settlement race, 150 reads during 100 settlements, 56 namespace/hash-slot combinations and persistent-window expiry checks. Rejected oversized requests leave no hold
Four SDK transport cases verify paid classifier and embedding calls enforce both creator budgets. Origin metadata does not enable the separate flag used for costs intentionally aggregated into a parent request
Bugbot's repeated-failure settlement finding was reproduced against real Redis and found incorrect. Shallow reservation copies share entries with
applied_adjustment, so reconciliation applies only the remaining difference. Wrapper release, two failure callbacks and eight concurrent duplicates preserved earlier and unrelated spend. A later higher cost added only its increase, and a later zero cost changed nothingScreenshots / Proof of Fix
The same six cases ran against isolated PostgreSQL databases and a local proxy, with real paid Together DeepSeek-V4.1-Flash source/shadow calls and Moonshot Kimi-K3 judge calls. Keys below are placeholders.
<source-agent>in the receipt output represents the generated agent UUIDShared proxy configuration:
Start the proxy on port 55183 with this configuration, provider credentials, and a fresh QA database for each commit. Create a separate evaluation owner and an agent-bound source key, then enable 100% shadow sampling:
The setup returned HTTP 200 for users/agent/key creation and HTTP 201 for evaluation creation. Run cases in the listed order. New evaluation jobs propagate through the existing active-job cache within about ten seconds. For replay, wait eleven seconds after creating a job before sending its first sample. After each source response, wait for the evaluation receipts, then inspect the new rows with:
Before (ad8baba)
Chat billing
Run:
Observed response and new receipts:
Creator total budget
Run:
Observed response and new receipts:
Creator per-model budget
Run:
Observed response and new receipts:
Responses billing
Run:
Observed response and new receipts:
The matching active job classified this bridged Responses sample as unjudgeable. This case verifies the paid source request only; evaluation billing is unverified for this bridge
Messages billing
Run:
Observed response and new receipts:
Configured deployment pricing
Run:
Observed response and new receipts:
The shadow deployment uses the real Together endpoint through its OpenAI-compatible API. Its provider-qualified model has no built-in price entry; the proxy config supplies input/output prices of $1/$2 per million tokens. The evaluation succeeds under the creator’s $1 total limit. The configured $0.01 hourly
qa-pricedcap is not exercised because this routed request uses theqa-priced-routerbudget identity; focused tests separately cover configured per-model pricingAfter (0e460ab)
Chat billing
Run:
Observed response and new receipts:
Creator total budget
Run:
Observed response and new receipts:
Creator per-model budget
Run:
Observed response and new receipts:
Responses billing
Run:
Observed response and new receipts:
The matching active job classified this bridged Responses sample as unjudgeable. This case verifies the paid source request only; evaluation billing is unverified for this bridge
Messages billing
Run:
Observed response and new receipts:
Configured deployment pricing
Run:
Observed response and new receipts:
The shadow deployment uses the real Together endpoint through its OpenAI-compatible API. Its provider-qualified model has no built-in price entry; the proxy config supplies input/output prices of $1/$2 per million tokens. The evaluation succeeds under the creator’s $1 total limit. The configured $0.01 hourly
qa-pricedcap is not exercised because this routed request uses theqa-priced-routerbudget identity; focused tests separately cover configured per-model pricingPaid nested evaluation-owned Messages/compaction remains unverified: native compaction requires Anthropic, whose account rejected the earlier probe for exhausted credits. The Messages case above tests a public Messages source request; focused provider-transport regressions separately exercise evaluation-owned nested calls
Type
Bug Fix
Caveats
Medium
Low
Final Attestation
Verification status
Current commit:
0e460ab47af3c2a0c6599277b0ab8e66138ac92b. All ten required CI checks pass. Greptile reports 5/5 and Veria passed. Bugbot's fresh recheck passed with no issues after reviewing the repeated-settlement evidence aboveCodecov's final commit report and an independent merge of CI artifacts agree on 95.30% changed-line coverage: 426 of 447 executable lines. SDK wrappers, internal billing metadata and cache changes are fully covered; the model limiter covers 51/52 changed lines and evaluation budgets cover 214/226. The GitHub coverage check still shows an earlier partial-upload result
Broader CI is still completing. The push-triggered Rust job failed an unchanged ClickHouse row-limit test, while the parallel PR Rust job passed. A rerun request was rejected while its workflow was still active
The CodeQL aggregate flags existing API-key hashing in alert 9134. The merge-base callback already passed the same API-key getter to the same spend writer and SHA-256 token helper. This PR removes the token for evaluation receipts and preserves the existing path for ordinary requests; it adds no password-hashing path