feat: pre-adoption shadow eval for the auto-router (backend) - #36571
feat: pre-adoption shadow eval for the auto-router (backend)#36571tin-berri wants to merge 1 commit into
Conversation
|
@greptileai can you reply here with a score and why that score? |
|
bugbot run |
Greptile SummaryThis PR adds backend shadow evaluations for auto-router adoption, including durable jobs and verdicts, sampling and judging pipelines, management endpoints, and internal-call accounting exclusions
Confidence Score: 1/5This PR is not safe to merge until budget admission, post-stop verdict consistency, and referenced-key validation are corrected Concurrent shadow calls can exceed key or team budgets, completed jobs can acquire uncounted verdicts, and start can create evaluations that no legitimate traffic can ever reach Files Needing Attention: litellm/integrations/shadow_eval_logger.py, litellm/proxy/management_endpoints/auto_router_endpoints.py
|
| Filename | Overview |
|---|---|
| litellm/integrations/shadow_eval_logger.py | Implements the shadow pipeline and lifecycle, but permits post-completion verdict writes and non-atomically admits chargeable calls near budget limits |
| litellm/proxy/management_endpoints/auto_router_endpoints.py | Adds start, list, get, and stop APIs plus aggregation and estimates, but start accepts nonexistent or noncanonical key identifiers |
| litellm/litellm_core_utils/internal_call_metadata.py | Centralizes identity forwarding and reservation stripping for internal calls; its behavior exposes the shadow pipeline's missing independent reservation |
| litellm/litellm_core_utils/llm_judge.py | Consolidates verdict parsing, model resolution, and Router-or-SDK judge dispatch without an identified regression |
| litellm/proxy/db/db_spend_update_writer.py | Preserves internal-call spend and token accounting while excluding those calls from user request and auto-router savings metrics |
| litellm/proxy/db/autorouter_session_rollup.py | Excludes internal routed sub-calls from adoption session rollups |
| litellm/proxy/proxy_server.py | Registers one shadow logger and starts its lifecycle loop with duplicate-registration protection |
| schema.prisma | Adds shadow-evaluation job and verdict models consistently with the two schema copies, though verdict writes are not relationally constrained by job lifecycle |
| litellm-proxy-extras/litellm_proxy_extras/migrations/20260811125249_add_shadow_eval/migration.sql | Creates the new tables and enforces one pending or running job per key through a partial unique index |
| litellm/proxy/guardrails/guardrail_hooks/llm_as_a_judge/init.py | Delegates existing judge parsing and dispatch behavior to the new shared utilities |
| litellm/router_strategy/complexity_router/complexity_router.py | Replaces private internal-call metadata sanitization with the shared implementation |
| tests/test_litellm/integrations/test_shadow_eval_logger.py | Covers sampling, pipeline outcomes, budgets, metadata, and lifecycle basics but not the completion-versus-in-flight-verdict race |
| tests/test_litellm/proxy/management_endpoints/test_auto_router_endpoints.py | Covers endpoint authorization and common validation but not invalid, raw, deleted, or nonexistent key identifiers |
Reviews (1): Last reviewed commit: "feat: pre-adoption shadow eval for the a..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| await prisma.db.litellm_shadowevalverdict.create( | ||
| data={ # mutable-ok: Prisma payload | ||
| "job_id": job.id, | ||
| "request_id": request_id, | ||
| "tier_classification": shadow.tier, | ||
| "real_model": real_model, | ||
| "shadow_model": shadow.model, | ||
| "judge_preference": verdict.preference, | ||
| "judge_confidence": verdict.confidence, | ||
| } | ||
| ) |
There was a problem hiding this comment.
When completion overlaps an in-flight pipeline, the unconditional verdict insert succeeds while counters remain frozen, so completed-job results change and disagree with counts
Knowledge Base Used: Database Schema and Proxy DB Access Layer
There was a problem hiding this comment.
Fixed in 8d3376e: the status guarded counter update now decides whether the verdict lands, pinned by test_job_stopped_mid_flight_drops_the_verdict
| if await _key_or_team_is_over_budget(budget_metadata): | ||
| return | ||
|
|
||
| shadow: Final = await self._call_router_shadow(job.router_name, messages, model_parameters, parent_metadata) |
There was a problem hiding this comment.
If shadow pipelines overlap near a budget limit, each passes the same read and collectively overspends the key or team
How this was verified: Internal calls omit reservations and update spend only after completion
Knowledge Base Used: Cost Tracking and Budget Enforcement
There was a problem hiding this comment.
As designed and documented: overshoot is bounded by the 16 per pod cap, spend lands on the same counters, and the job spend cap bounds the eval
There was a problem hiding this comment.
Fair point — the overshoot is explicitly bounded and documented, which addresses the concern. The 16-per-pod cap sets a hard ceiling on how many pipelines can race past the budget read simultaneously, the job spend cap provides a secondary bound on the overall eval, and the PR description calls this out explicitly as an intentional trade-off against double-finalization risk. The soft check is appropriate here.
| existing: Final = await prisma_client.db.litellm_shadowevaljob.find_first( | ||
| where={ # mutable-ok: Prisma filter | ||
| "api_key_id": data.api_key_id, | ||
| "status": {"in": ["pending", "running"]}, # mutable-ok: Prisma filter | ||
| }, | ||
| ) | ||
| if existing is not None: | ||
| raise HTTPException( | ||
| status_code=409, | ||
| detail=f"Key already has an active shadow eval job ({existing.id}). Stop it first.", | ||
| ) | ||
|
|
||
| recent_requests: Final = await _recent_request_volume(prisma_client, data.api_key_id) |
There was a problem hiding this comment.
When an admin submits a noncanonical or nonexistent key identifier, start persists it successfully, creating an evaluation that legitimate traffic never matches
Knowledge Base Used:
There was a problem hiding this comment.
Fixed in 8d3376e: start now 400s when api_key_id is not a LiteLLM_VerificationToken row, with a pinning test
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| real_text: Final = self._extract_response_text(response_obj) | ||
| if not real_text or not messages: | ||
| return | ||
| if await _key_or_team_is_over_budget(budget_metadata): |
There was a problem hiding this comment.
Medium: Shadow calls bypass budget reservations
Once an eval is active, a key holder can submit concurrent requests whose shadow and judge tasks all pass this read-only check before their spend is recorded, exceeding key or team quotas; user, organization, project, end-user, and model budgets are not checked here at all. Reserve the estimated cost of both internal calls atomically against the same budget counters before dispatch and reconcile it with their actual combined cost afterward; the job's cost_actual and spend cap should also include the shadow-router call rather than only verdict.cost.
There was a problem hiding this comment.
Declined as designed: the soft check plus in flight cap and per job spend cap bound overshoot; reservations for detached sub calls risk double finalization
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
PR overviewThis PR adds backend support for pre-adoption shadow evaluation of the auto-router, including shadow-router and judge calls and tracking their evaluation costs. One budget-enforcement issue remains open after another issue was addressed. An authorized key holder can submit concurrent evaluation requests whose internal shadow and judge calls bypass atomic budget reservations and several quota checks, potentially causing spend to exceed configured key, team, user, organization, project, end-user, or model limits. Open issues (1)
Fixed/addressed: 1 · PR risk: 6/10 |
d1420d5 to
8d3376e
Compare
|
Addressed the review in 8d3376e: post-stop verdicts now impossible (guarded counter decides the insert), start validates the key exists, shadow and judge calls never retry or fall back. Budget stays a soft check by design; bounds are in the PR body. @greptileai can you reply here with a score and why that score? |
|
bugbot run |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Score: 4/5 The three items you addressed are verified in the code:
Two minor points that hold back a 5: 1. SDK judge path does not pass In # llm_judge.py – SDK branch
return await litellm.acompletion(model=judge_model, messages=messages, num_retries=0, **params)
# ^ fallbacks=[] missing2. The read-then-update in The live-provider staging pass the PR notes is the remaining gate before merge. Everything else looks production-ready. |
|
Bugbot round two addressed in 43a67bb: the recording section is now one transaction, so a pipeline lands in exactly one of completed, failed, or dropped and counts always match stored verdicts. @greptileai can you reply here with a score and why that score? |
8d3376e to
43a67bb
Compare
|
bugbot run |
|
Score: 3.5 / 5 The transaction fix is correct and closes the specific atomicity gap. Here is the full breakdown. What the fix gets right The three-way invariant now holds in production: async with prisma.tx() as transaction:
counted: Final = await transaction.litellm_shadowevaljob.update_many(
where={"id": job.id, "status": {"in": ["pending", "running"]}},
...
)
if counted:
await transaction.litellm_shadowevalverdict.create(...)
The What is still holding back a 4/5
Neither of these is a regression against existing functionality, but together they mean the feature hasn't been exercised against a real provider and there is a latent miss in the call_type gate. Fix point 2, land the real-provider run, and this is a clean 4/5. |
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 43a67bb. Configure here.
43a67bb to
df0eb9d
Compare
|
Fixed the call_type gate in df0eb9d: unknown or missing types now fail closed, test updated. The real provider staging run stays owed before merge. @greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
Score: 3.5 / 5 Here's the breakdown: What's now solid (the call_type fix is correct) The gate at if payload.get("call_type") not in ("completion", "acompletion", "chat_completion"):
returndoes fail closed for Why 3.5 and not higher
What earns the 3.5 and not lower The architecture is genuinely well-designed: one dict lookup in the hot path, all lifecycle off-path, status-guarded transaction for verdict+counter atomicity, spend cap with floor, To get to 4/5: complete the live-provider staging run and post a brief summary of the spend attribution rows from a real Postgres instance (same format as the PR description, but with real provider responses, not stubs). The |
df0eb9d to
4a96d07
Compare
|
Both minor points fixed in 4a96d07: chat_completion dropped from the allowlist and the slot release is a bound method. Staging run remains the open item. @greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
4/5 — the code is in very good shape. Here's the reasoning: What earns the score:
What holds it at 4 instead of 5: The staging run with live providers is the one genuine open item. This is a feature that bills real provider spend to keys through a background path that bypasses the normal auth+budget reservation lifecycle. The live proof used stubbed upstreams (all local credentials dead), so real-provider behavior — cost attribution landing in Everything else is addressed. Once a staging pass with a live key and real providers confirms the spend attribution and metric exclusion paths behave as the Postgres output shows for the stub run, this is ready. |
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 4a96d07. Configure here.
|
bugbot run |
|
4/5 — the reuse pass delivered real improvement and the architecture is solid; one concrete bug remains before merge. What the reuse pass got right
The one remaining bug (breaks a stated invariant) In counted: Final = await transaction.litellm_shadowevaljob.update_many(...)
if counted:
await transaction.litellm_shadowevalverdict.create(...)
Fix: Fix that and this is a clean 5/5. |
|
Declined: the Python prisma client returns int from update_many, not BatchPayload; prisma_protocols.py:26 types it, and a live tx probe returned falsy 0 on zero matches |
|
Probe for the record: inside db.tx(), update_many on a no-match filter returned 0 as int with bool False; counted.count would AttributeError on every pipeline |
|
@greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
Score: 3/5 The architecture is genuinely well-thought-out — one dict lookup on idle paths, deterministic hash sampling, Bug: The most significant issue is in counted: Final = await transaction.litellm_shadowevaljob.update_many(
where={"id": job.id, "status": {"in": ["pending", "running"]}},
data={...},
)
if counted: # ← always True — BatchPayload is an object, not an int
await transaction.litellm_shadowevalverdict.create(...)
Other concerns:
|
9b4fd37 to
781de79
Compare
|
@greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
Score: 3.5 / 5 This is a genuinely well-thought-out feature with solid architectural decisions, but several issues prevent a higher score. Why it is not higher1. Real-provider validation deferred (PR's own admission) 2. try:
from litellm.proxy.proxy_server import prisma_client, proxy_logging_obj, user_api_key_cache
except ImportError:
return FalseOn a pod where the proxy has not finished initializing, the financial safety gate is silently skipped — not just once at startup, but on every call until the import succeeds. "Fail-open" is documented, but the scope of that failure is wider than the docstring implies. 3. Multi-pod spend-cap overshoot is understated 4. No test for the stopped-job verdict-drop invariant 5. Why it is not lower
Bottom line: The architecture is sound and the tradeoffs are well-reasoned, but the PR needs (a) a real-provider staging run, (b) a test for the stopped-job verdict-drop invariant, and (c) an honest statement of multi-pod overshoot bounds before it can be merged with confidence. |
|
On BatchPayload: the generated Python client types update_many as int, and a live db.tx() probe returned falsy 0 on zero matches |
|
Tests exist in this diff: tests/test_litellm/integrations/test_shadow_eval_logger.py, 36 tests; your earlier review cited test_job_stopped_mid_flight_drops_the_verdict by name |
|
Lifecycle start is wired in cost_tracking via _register_shadow_eval_logger, pinned by test_registration_starts_the_lifecycle_loop_on_the_registered_instance |
|
fallbacks=[] on the SDK path would be a no-op: main.py resolves fallbacks or litellm.model_fallbacks, and num_retries=0 already prevents retry spend |
|
@greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
The stopped-job invariant test exists: test_job_stopped_mid_flight_drops_the_verdict, cited by your own earlier review; body now states multi-pod overshoot bounds |
|
Score: 4/5 The implementation is architecturally sound and shows careful thinking on the hard problems. Here's the breakdown: What earns the points (4):
What keeps it from 5:
Summary: The design decisions (spend isolation, sampling, lifecycle, race handling) are all correct and well-executed. The score gap is almost entirely the stubbed-upstream gap — a staging pass with real providers should close it before merge. |
…dge, per-tier win rates)
781de79 to
52ad253
Compare
|
@greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
Score: 3.5 / 5 Here's my breakdown: What's strong (pushes toward 4–5)
What holds it back (pushes toward 3)
Summary The implementation is serious and iterates correctly on findings — major issues (transaction atomicity, post-stop verdicts, retry/fallback on shadow, key validation, redaction) were all addressed. The one remaining open thread (SDK fallback suppression) is a concrete gap on a code path that's billed to customer keys, and the absence of real-provider staging validation is flagged by the author. Fix those two before requesting maintainer merge. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 52ad253. Configure here.
| ) | ||
| task.add_done_callback(self._release_shadow_slot) | ||
| except Exception as e: # noqa: BLE001 # logging hooks must never fail the request | ||
| verbose_logger.debug("shadow_eval: failed to schedule task: %s", e) |
There was a problem hiding this comment.
Inflight slot leaked on schedule failure
Medium Severity
_inflight_shadow_tasks is incremented before asyncio.create_task and add_done_callback, but the surrounding except never decrements it. If scheduling fails after the increment (argument evaluation, create_task during shutdown, etc.), that slot is never released. After 16 such failures on a pod, the inflight cap stays saturated and shadow sampling stops until process restart.
Reviewed by Cursor Bugbot for commit 52ad253. Configure here.
| data={ # mutable-ok: Prisma payload | ||
| "failed_count": {"increment": 1}, # mutable-ok: Prisma operator | ||
| "last_error": error[:_MAX_LAST_ERROR_CHARS], | ||
| }, |
There was a problem hiding this comment.
Failed jobs stay pending while spending
Medium Severity
status is flipped to running only inside the successful verdict transaction. _bump_failed increments failed_count and last_error but leaves status at pending. A job that is actively sampling and billing shadow/judge calls while every pipeline fails therefore keeps reporting pending, which reads as not started even though spend and failures are accumulating.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 52ad253. Configure here.
|
Superseded by #36587 (rebuilt shadow-eval logger). |
1 participant
TLDR
Problem this solves:
How it solves it:
internal_call_origin, which excludes them fromapi_requests,autorouter_savings_spend, and theLiteLLM_AutoRouterSessionbenchmarks rollup, so a running eval cannot inflate adoption metrics or its own next cost estimateUser Flow
POST /auto_router/shadow_eval/startwith a key, a configured auto-router, a sampling percentage, a judge model, and a duration; the response carries an upfront judge-cost estimate priced from the key's trailing 7 day request volumeGET /auto_router/shadow_eval/{job_id}returns counters and win rates by tier and by incumbent model;POST .../stophalts sampling and freezes counters; the job also stops itself at its scheduled end or when judge spend reaches 1.5x the estimate ($1 floor)Relevant issues
ci_cd/run_migration.pyrather than amended in placeChanges
LiteLLM_ShadowEvalJobandLiteLLM_ShadowEvalVerdicttables; one active job per key is enforced by a partial unique index, so a concurrent start on another pod surfaces as the same 409 as the advisory readShadowEvalLogger(litellm/integrations/shadow_eval_logger.py): the success hook is one dict lookup against a lifecycle-loop-refreshed job snapshot; deterministic hash sampling; at most 16 in-flight shadow pipelines per pod, shedding rather than queueing; skips internal sub-calls, self-shadowing, over-budget keyslitellm/litellm_core_utils/llm_judge.pyowning the fence-tolerant verdict parser, the router-or-SDK judge dispatch, and the router-resolvable predicate; the llm_as_a_judge guardrail now delegates to it instead of carrying private copies, and start-time judge validation calls the same predicate the dispatch useslitellm/litellm_core_utils/internal_call_metadata.pyowning identity forwarding and budget-reservation stripping for internal sub-calls; the complexity router's private copy is deleted in favor of itauto_router_endpoints.py(start, list, get, stop); start and stop require PROXY_ADMIN, reads also allow PROXY_ADMIN_VIEW_ONLY; start rejects a router that is not configured, a judge the dispatch cannot resolve, and an api_key_id that is not a key on this proxyWhat does not change
Requests on keys with no active job pay one dict lookup in the success hook and nothing else. No existing endpoint, table, or logger changes behavior; the guardrail and classifier refactors are pure delegation with their existing tests untouched and passing
Things a reviewer will ask about
LiteLLM_DailyUserSpend(a handful of indexed rows per key/day), neverLiteLLM_SpendLogs; verdict aggregation is two GROUP BYs over one job's verdicts through thejob_idindex, so read cost at a million requests is bounded by the job's own verdict count, which the spend cap bounds in turnPre-Submission checklist
Screenshots / Proof of Fix
Live proxy on this branch (port 4214, fresh Postgres with the new migration deployed, stub upstream minting unique response ids; every provider credential in the local .env is out of credit, so upstream calls are stubbed and a real-provider re-run is owed before merge, precedent #30277 / #34029)
Before, at the merge-base (e37ae03), same rig:
After, on this branch. Start a job on a fresh key, with the rejection matrix:
12 chat requests on the shadowed key (mix of simple, complex, reasoning prompts), then:
Spend attribution and metric exclusion, straight from the rig's Postgres:
Stop freezes the job even as traffic continues:
Type
🆕 New Feature
Caveats (if any)
/v1/chat/completions) is sampled;/v1/messagesand/v1/responsessurfaces are skipped by the call_type gate by designQA runbook
auto_router/complexity_router) plus its tier models and start the proxy with a databasePOST /auto_router/shadow_eval/startwith{"api_key_id": "<key token hash>", "router_name": "<router>", "shadow_percentage": 100, "judge_model": "<any configured model or provider/model>", "duration_days": 1}as admin; expect 201 with an estimate, 409 on repeat, 400 for an unknown router or unresolvable judge/v1/chat/completionsrequests with the shadowed key, wait ~15s,GET /auto_router/shadow_eval/{job_id}; expect request_count, completed_count and per-tier results to growLiteLLM_SpendLogs.metadata->>'internal_call_origin'groups spend into shadow_eval_router / shadow_eval_judge rows billed to the shadowed key, andLiteLLM_DailyUserSpend.api_requestscounts only the user requestsPOST .../stop; expect status completed, counters frozen, verdicts keptFinal Attestation
Note
High Risk
Bills real shadow/judge spend to customer keys and modifies spend aggregation plus budget-adjacent paths on the request success hook. Large new surface with DB schema, background LLM calls, and metric-exclusion logic.
Overview
Enables admins to pre-evaluate an auto-router against one key's live traffic before adoption. A sampled slice of successful chat requests is duplicated through the router in a detached task; an LLM judge compares real vs shadow responses blind, and win rates are stratified by router tier and incumbent model.
Adds
POST/GET /auto_router/shadow_eval*admin endpoints (start/list/get/stop), plusLiteLLM_ShadowEvalJob/LiteLLM_ShadowEvalVerdicttables with a partial unique index enforcing one active job per key. Start returns an upfront judge-cost estimate from trailing daily spend volume; jobs auto-stop atends_ator a 1.5x spend cap.New
ShadowEvalLoggerruns off the success hook (one dict lookup when idle): deterministic sampling, concurrency shedding, budget/redaction/self-shadow skips, and a 10s lifecycle loop for counter flush and finalization. Shadow/judge calls bill to the shadowed key but are stampedinternal_call_origin, so spend writers and autorouter session rollups keep spend while zeroingapi_requestsand autorouter savings.Also extracts shared
llm_judgeandinternal_call_metadatahelpers; the llm-as-a-judge guardrail and complexity router now delegate to them instead of private copies.Reviewed by Cursor Bugbot for commit 52ad253. Bugbot is set up for automated code reviews on this repo. Configure here.