fix(shadow_eval): split slot release from stops so stopped_at means exactly one thing - #37382
fix(shadow_eval): split slot release from stops so stopped_at means exactly one thing#37382tin-berri wants to merge 1 commit into
Conversation
Greptile SummaryThe PR separates shadow-evaluation slot release bookkeeping from explicit stop records.
Confidence Score: 4/5The PR is not yet safe to merge because the data migration can erase legacy operator-stop intent and report those evaluations as completed The new runtime split is internally consistent, but the migration clears every actor-less stopped_at value even though expired legacy operator stops were not guaranteed to receive stopped_by Files Needing Attention: litellm-proxy-extras/litellm_proxy_extras/migrations/20260820150000_shadow_eval_released_at/migration.sql
|
| Filename | Overview |
|---|---|
| litellm-proxy-extras/litellm_proxy_extras/migrations/20260820150000_shadow_eval_released_at/migration.sql | Adds and backfills released_at but unconditionally clears actor-less stop timestamps, including legacy explicit stops missed by the prior backfill |
| litellm/proxy/management_endpoints/auto_router_endpoints.py | Moves slot sweeps, active claims, and explicit stop release writes consistently to released_at |
| litellm/integrations/shadow_eval_logger.py | Restricts sampling to rows that are neither explicitly stopped nor internally released |
| litellm/types/management_endpoints/auto_router_endpoints.py | Gives stop stamps precedence over spend arithmetic while retaining window completion handling |
| tests/test_litellm/proxy/management_endpoints/test_auto_router_endpoints.py | Updates mocks and adds regression coverage for released sweeps and stamped-over-budget status, but does not exercise the legacy migration gap |
| schema.prisma | Adds the nullable internal released_at lifecycle field consistently with the two schema copies |
Reviews (7): Last reviewed commit: "fix(shadow_eval): split slot release fro..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
|
@greptileai review pls |
|
bugbot run |
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 a1df700. Configure here.
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e43e3bd. Configure here.
| DROP INDEX IF EXISTS "LiteLLM_ShadowEvalJob_one_active_per_key_direction"; | ||
|
|
||
| CREATE UNIQUE INDEX IF NOT EXISTS "LiteLLM_ShadowEvalJob_one_active_per_key_direction" | ||
| ON "LiteLLM_ShadowEvalJob"("api_key_id", "direction") WHERE "released_at" IS NULL; |
There was a problem hiding this comment.
Stop no longer frees the eval slot
Medium Severity
The active-job unique index and start lookup now key only on released_at, while sweeps only fill that column when the window or turn budget is already spent. Any stop that still writes stopped_at without released_at (rolling-deploy pods, or a stop that races the new SQL) leaves the key+direction slot occupied. Sampling already halted, but starting a replacement eval keeps returning 409 until ends_at, which can be days later.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit e43e3bd. Configure here.
e43e3bd to
71ff912
Compare
| UPDATE "LiteLLM_ShadowEvalJob" SET stopped_at = NULL | ||
| WHERE stopped_at IS NOT NULL AND stopped_by IS NULL; |
There was a problem hiding this comment.
If an expired legacy operator stop lacks stopped_by, this update clears its only stop marker, causing list and detail responses to report completion
Knowledge Base Used:


TLDR
Problem this solves:
How it solves it:
User Flow
Before: whether a stamped eval reads "stopped" or "completed" depends on racy arithmetic
After: a stamp is a stop, and no spend timing can reinterpret it
Relevant issues
Linear ticket
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
Setup: proxy from this repo against Postgres, config with
auto_router1(complexity router over gpt-5 family deployments) and a deterministic OpenAI-compatible upstream at :4299. Evals started with per-keymax_budget(dollars); spend rows are topped up via SQL where noted because the stub's per-call cost is fractions of a centBefore (1d7f675)
A stamped job's label depends on spend arithmetic
"completed"; the stamp's meaning is decided by arithmeticThe sweep writes the stop column
stopped_at: slot bookkeeping and stops share one column, which is what forces every reader to guessAfter (71ff912)
Spend exhaustion reads completed with nothing stamped
max_budget: 0.01, send 2 chats, top spend to $0.0202"completed",spend: 0.0202of0.01,stopped_at: null,stopped_by: nullThe sweep frees the slot without touching the stop record
"completed",stopped_atstill nullA stop outranks racing spend
"stopped",stopped_byrecorded"stopped"; spend is never consulted for a stamped keyA bare stamp from a pre-released_at pod reads stopped
stopped_atset, nostopped_by, spend over budget"stopped"; a stamp means a stop, unconditionallyType
🐛 Bug Fix
Caveats (if any)
Final Attestation