feat(auto-router): scope shadow eval jobs to multiple keys

[tin-berri]

TLDR

Problem this solves:

• A shadow eval job could cover exactly one key
• Evaluating a router across several keys meant several jobs
• Their results had to be added up by hand

How it solves it:

• One job now takes a list of keys
• Each key carries its own turn budget
• Results come back pooled and per key

User Flow

Before: an admin shadow testing an auto-router across two of their keys can only cover one key at a time, so no single result describes the router

1. They send POST http://localhost:4000/auto_router/shadow_eval/start with "api_key_ids": ["<hash A>", "<hash B>"] and get back 422 saying api_key_id is required
2. They resend the request twice, one key each time, and get back two separate job ids
3. They send GET http://localhost:4000/auto_router/shadow_eval/ for each id and get two result sets, each describing one key
4. They add the two up by hand to judge the router, and a 200 turn budget covers whichever single key that job named
5. They open http://localhost:4000/ui/?page=cost-optimization and find a key picker that accepts one key, so the same split shows up on screen

After: the same admin covers both keys with one job and reads one result

1. They send POST http://localhost:4000/auto_router/shadow_eval/start with "api_key_ids": ["<hash A>", "<hash B>"] and get back 201 with a single job id listing both keys, each with its own max_turns
2. Traffic on either key is sampled, and each key spends its own budget, so two keys at a budget of 2 judge 4 turns rather than 2, and one key going quiet never ends sampling for the other
3. They send GET http://localhost:4000/auto_router/shadow_eval/ and get one pooled result plus a per key breakdown alongside the existing tier and model ones
4. They send GET http://localhost:4000/auto_router/shadow_eval?api_key_id= and get every job that key belongs to, not just one
5. They send POST http://localhost:4000/auto_router/shadow_eval//stop once and sampling ends for every key under it
6. They run a forward and a reverse job over the same two keys at once, since direction is a separate slot per key, and each of the four key rows keeps its own budget
7. They open http://localhost:4000/ui/?page=cost-optimization and find a picker that takes several keys, with a per key table in the detail view showing each key's own budget and win rates
8. Enrolling a key that another job already covers in the same direction comes back 409 naming that key and direction, and an unknown key comes back 400 naming it, before anything is created

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

• I have added meaningful tests
• My PR passes all CI/CD checks (e.g., lint, format, unit tests)
• My PR's scope is as isolated as possible; it only solves 1 specific problem
• I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Type

🆕 New Feature

Changes

A shadow eval job now holds a list of keys instead of one. Job-wide settings (router, direction, judge model, shadow percentage, window) stay on the job, while each key carries its own sample budget and its own stop timestamp in a child row. Sampled attempts record which key they came from, which is what makes both the per key budget and the per key result slice exact rather than estimated

The DB-enforced uniqueness guarantee moves with it, and keeps the direction dimension that #36865 added. The partial unique index now sits on the child table over (api_key_id, direction) WHERE stopped_at IS NULL, so two concurrent starts naming the same key in the same direction still cannot both win, with no read-then-write in between, while a forward and a reverse job over the same key remain independent slots. Holding direction on the child row means the child could otherwise drift from its parent, so the foreign key is composite over (job_id, direction) and the database rejects a key row claiming a direction its job does not have

Job status stays derived rather than stored. A job reads completed once its window passes, stopped only when every key has ended, and running while any key is still sampling, so one key finishing early never hides its siblings

The migration moves those three columns rather than leaving them behind. It creates the child table, backfills every existing job into a key row carrying its direction, backfills each existing attempt with its job's key before that column goes NOT NULL, then drops api_key_id, max_turns and stopped_at off the job. Shadow eval has never been in a release, so no deployment runs the single key code and there is no mixed version window to design around

Screenshots / Proof of Fix

Live proxy on localhost:4000, real billed LLM calls, no mocks and no pytest. The run below was captured at ec03a6a. Everything committed since answers review findings without touching the paths it exercises: migration comments, an attempt count query narrowed to exact job and key pairs, which only drops groups nothing read, and a UI only move of the per key table. The discriminator is step 7: two keys at a budget of 2 judge 4 turns, where a shared pool would have stopped at 2. Rounds 3 and 4 send real traffic on both keys and add nothing, which is the budget holding rather than the traffic running out

Each round sends two kinds of request per key because forward and reverse sample opposite traffic. A forward job skips what its own router served, since duplicating it would compare the router to itself, so plain haiku-4-5 calls feed the forward job and auto_router1 calls feed the reverse one. Steps 6 and 9 show the two directions holding separate slots for the same pair of keys

commit under test: ec03a6af0a

==================== 1. schema: the per key child table, and where the race safe index now lives ====================

$ psql -c "\d \"LiteLLM_ShadowEvalJobKey\""
  id|text||not null|
  job_id|text||not null|
  direction|text||not null|
  api_key_id|text||not null|
  max_turns|integer||not null|
  created_at|timestamp(3) without time zone||not null|CURRENT_TIMESTAMP
  stopped_at|timestamp(3) without time zone|||

$ psql -c "\d \"LiteLLM_ShadowEvalAttempt\"" | grep api_key_id
  api_key_id|text||not null|

==================== 2. two virtual keys whose traffic will be shadowed ====================

key C hash: 27015286229631c0ce7f95b023c65e64d02d8236bb863a919bf8e3a1542ebee7
key D hash: c438875739cd5b6496d95d425acc64bc8260549724dcb90b0fb895d4cb9599b0

==================== 3. an unknown key is rejected by name, and nothing is created ====================

$ curl -X POST .../shadow_eval/start -d {"api_key_ids":["<C>","sk-does-not-exist"],...}
  {"detail":"api_key_ids not on this proxy: sk-does-not-exist; pass each key's token hash, the value the key list and key info endpoints report"}
  HTTP 400
  jobs before: 0   jobs after: 0

==================== 4. one forward job over both keys, each with its own budget of 2 turns ====================

$ curl -X POST .../shadow_eval/start -d {"api_key_ids":["<C>","<D>"],"router_name":"auto_router1","shadow_percentage":100,"max_turns":2,"judge_model":"sonnet-4-5"}
  {
    "job_id": "cmstnqqu7000kmgodp5y1cev7",
    "keys": [
      {
        "api_key_id": "27015286229631c0ce7f95b023c65e64d02d8236bb863a919bf8e3a1542ebee7",
        "max_turns": 2,
        "stopped_at": null
      },
      {
        "api_key_id": "c438875739cd5b6496d95d425acc64bc8260549724dcb90b0fb895d4cb9599b0",
        "max_turns": 2,
        "stopped_at": null
      }
    ],
    "router_name": "auto_router1",
    "direction": "forward",
    "baseline_model": null,
    "judge_model": "sonnet-4-5",
    "shadow_percentage": 100.0,
    "created_at": "2026-08-15T00:48:33.631000Z",
    "ends_at": "2026-08-22T00:48:33.630000Z",
    "judged_count": null,
    "error_count": null,
    "judge_spend": null,
    "last_error": null,
    "results": null,
    "status": "running"
  }

==================== 5. neither key can join a second forward job, and the 409 names the key and the direction ====================

$ curl -X POST .../shadow_eval/start -d {"api_key_ids":["<D>"],"router_name":"auto_router1",...}
  {"detail":"Already in an active forward shadow eval job: c438875739cd5b6496d95d425acc64bc8260549724dcb90b0fb895d4cb9599b0 (job cmstnqqu7000kmgodp5y1cev7). Stop it first."}
  HTTP 409

==================== 6. the reverse direction is a separate slot, so the same two keys can run both at once ====================

$ curl -X POST .../shadow_eval/start -d {"api_key_ids":["<C>","<D>"],"direction":"reverse","baseline_model":"haiku-4-5",...}
  reverse job: cmstnqqww000nmgodi4mu5kmo

$ psql -c "SELECT api_key_id, direction, max_turns FROM \"LiteLLM_ShadowEvalJobKey\" WHERE stopped_at IS NULL"
  2701528622..|forward|2
  2701528622..|reverse|2
  c438875739..|forward|2
  c438875739..|reverse|2

==================== 7. real traffic, 4 rounds, one plain model call and one router call per key per round ====================

  plain haiku-4-5 traffic feeds the forward job, auto_router1 traffic feeds the reverse one

  round 1 key C via haiku-4-5 -> haiku-4-5 74
  round 1 key C via auto_router1 -> auto_router1 74
  round 1 key D via haiku-4-5 -> haiku-4-5 54
  round 1 key D via auto_router1 -> auto_router1 54
    forward judged: 2701528622..|1 c438875739..|1 
    reverse judged: 2701528622..|1 c438875739..|1 
  round 2 key C via haiku-4-5 -> haiku-4-5 54
  round 2 key C via auto_router1 -> auto_router1 54
  round 2 key D via haiku-4-5 -> haiku-4-5 41
  round 2 key D via auto_router1 -> auto_router1 41
    forward judged: 2701528622..|2 c438875739..|2 
    reverse judged: 2701528622..|2 c438875739..|2 
  round 3 key C via haiku-4-5 -> haiku-4-5 46
  round 3 key C via auto_router1 -> auto_router1 46
  round 3 key D via haiku-4-5 -> haiku-4-5 45
  round 3 key D via auto_router1 -> auto_router1 45
    forward judged: 2701528622..|2 c438875739..|2 
    reverse judged: 2701528622..|2 c438875739..|2 
  round 4 key C via haiku-4-5 -> haiku-4-5 72
  round 4 key C via auto_router1 -> auto_router1 72
  round 4 key D via haiku-4-5 -> haiku-4-5 45
  round 4 key D via auto_router1 -> auto_router1 45
    forward judged: 2701528622..|2 c438875739..|2 
    reverse judged: 2701528622..|2 c438875739..|2 

==================== 8. the forward result: one pooled verdict plus the new per key breakdown ====================

$ curl http://localhost:4000/auto_router/shadow_eval/cmstnqqu7000kmgodp5y1cev7
  {
    "job_id": "cmstnqqu7000kmgodp5y1cev7",
    "keys": [
      {
        "api_key_id": "27015286229631c0ce7f95b023c65e64d02d8236bb863a919bf8e3a1542ebee7",
        "max_turns": 2,
        "stopped_at": null
      },
      {
        "api_key_id": "c438875739cd5b6496d95d425acc64bc8260549724dcb90b0fb895d4cb9599b0",
        "max_turns": 2,
        "stopped_at": null
      }
    ],
    "router_name": "auto_router1",
    "direction": "forward",
    "baseline_model": null,
    "judge_model": "sonnet-4-5",
    "shadow_percentage": 100.0,
    "created_at": "2026-08-15T00:48:33.631000Z",
    "ends_at": "2026-08-22T00:48:33.630000Z",
    "judged_count": 4,
    "error_count": 0,
    "judge_spend": 0.0,
    "last_error": null,
    "results": {
      "by_tier": [
        {
          "group": "SIMPLE",
          "turn_count": 4,
          "real_win_rate_pct": 0.0,
          "shadow_win_rate_pct": 0.0,
          "tie_rate_pct": 100.0,
          "avg_judge_confidence": 1.0
        }
      ],
      "by_current_model": [
        {
          "group": "openai/bedrock/us.anthropic.claude-haiku-4-5-20251001-v1:0",
          "turn_count": 4,
          "real_win_rate_pct": 0.0,
          "shadow_win_rate_pct": 0.0,
          "tie_rate_pct": 100.0,
          "avg_judge_confidence": 1.0
        }
      ],
      "by_key": [
        {
          "group": "27015286229631c0ce7f95b023c65e64d02d8236bb863a919bf8e3a1542ebee7",
          "turn_count": 2,
          "real_win_rate_pct": 0.0,
          "shadow_win_rate_pct": 0.0,
          "tie_rate_pct": 100.0,
          "avg_judge_confidence": 1.0
        },
        {
          "group": "c438875739cd5b6496d95d425acc64bc8260549724dcb90b0fb895d4cb9599b0",
          "turn_count": 2,
          "real_win_rate_pct": 0.0,
          "shadow_win_rate_pct": 0.0,
          "tie_rate_pct": 100.0,
          "avg_judge_confidence": 1.0
        }
      ],
      "overall_shadow_win_rate_pct": 0.0,
      "overall_tie_rate_pct": 100.0
    },
    "status": "running"
  }

==================== 9. stopping the forward job stops every key under it and frees their forward slots ====================

$ curl -X POST http://localhost:4000/auto_router/shadow_eval/cmstnqqu7000kmgodp5y1cev7/stop
  {
    "job_id": "cmstnqqu7000kmgodp5y1cev7",
    "keys": [
      {
        "api_key_id": "27015286229631c0ce7f95b023c65e64d02d8236bb863a919bf8e3a1542ebee7",
        "max_turns": 2,
        "stopped_at": "2026-08-15T00:49:48.048000Z"
      },
      {
        "api_key_id": "c438875739cd5b6496d95d425acc64bc8260549724dcb90b0fb895d4cb9599b0",
        "max_turns": 2,
        "stopped_at": "2026-08-15T00:49:48.048000Z"
      }
    ],
    "router_name": "auto_router1",
    "direction": "forward",
    "baseline_model": null,
    "judge_model": "sonnet-4-5",
    "shadow_percentage": 100.0,
    "created_at": "2026-08-15T00:48:33.631000Z",
    "ends_at": "2026-08-22T00:48:33.630000Z",
    "judged_count": null,
    "error_count": null,
    "judge_spend": null,
    "last_error": null,
    "results": null,
    "status": "stopped"
  }

$ psql -c "SELECT api_key_id, direction, stopped_at IS NULL AS active FROM \"LiteLLM_ShadowEvalJobKey\""
  2701528622..|forward|f
  2701528622..|reverse|t
  c438875739..|forward|f
  c438875739..|reverse|t

==================== 10. listing by key matches membership across every job the key belongs to ====================

$ curl "http://localhost:4000/auto_router/shadow_eval?api_key_id=<C>"
   {"job_id": "cmstnqqww000nmgodi4mu5kmo", "direction": "reverse", "status": "running", "keys": 2}
   {"job_id": "cmstnqqu7000kmgodp5y1cev7", "direction": "forward", "status": "stopped", "keys": 2}

==================== done at commit ec03a6af0a ====================

For the UI, with the same proxy running and npm run dev in ui/litellm-dashboard:

1. Open http://localhost:4000/ui/?page=cost-optimization and find the Shadow Eval section
2. In the key picker, search and tick two keys, confirming both appear as chips and that the picker no longer collapses to a single selection
3. Set Max turns to 2, pick a judge model, and start the job
4. Send a couple of requests on each key, then read the per key table under the tier and model breakdowns, which lists every scoped key with its status, turns against budget and win rates
5. Confirm each key shows its own turn count and its own budget, and that Stop ends both at once
6. Stop the job, expand Previous evaluations, open that job, and confirm the same per key table is there rather than only the tier and model breakdowns

Migration

An earlier revision kept the three job columns and made them nullable so that a pod on the previous image could keep serving mid rollout. That answered Greptile's flag, and it cost a NULL branch in the per key budget, an unattributed bucket in the API response, and three dead columns nothing reads. The window it guards cannot occur: git tag --contains on the shadow eval commit returns nothing, and neither the latest stable release nor the current dev pre-release ships the shadow eval migration or shadow_eval_logger.py, so no pod anywhere runs the single key code. The migration is destructive again and all three guards are gone with it

The run below applies it to a database already holding jobs and attempts in both directions, checks the backfill and the moved constraints, then deploys the whole chain to a fresh database and diffs the result against schema.prisma

==================== 1. a database already at the pre-upgrade shadow eval state, holding jobs and attempts ====================

  applied the merged shadow eval migrations: 20260811172448_add_shadow_eval 20260813180408_add_shadow_eval_direction 
  seeded 3 jobs and 3 attempts

==================== 2. applying this PR's migration to that database ====================

  applied cleanly

==================== 3. the legacy job columns are gone, the job rows are not ====================

      column_name    
  -------------------
   baseline_model
   created_at
   created_by
   direction
   ends_at
   id
   judge_model
   router_name
   shadow_percentage
  (9 rows)
  
     id    | direction 
  ---------+-----------
   job_fwd | forward
   job_old | forward
   job_rev | reverse
  (3 rows)
  

==================== 4. every job backfilled into a key row, carrying its direction, budget and stop state ====================

         id       | job_id  | direction | api_key_id | max_turns | active 
  ----------------+---------+-----------+------------+-----------+--------
   jobkey_job_fwd | job_fwd | forward   | hash_a     |        40 | t
   jobkey_job_old | job_old | forward   | hash_b     |        10 | f
   jobkey_job_rev | job_rev | reverse   | hash_a     |        25 | t
  (3 rows)
  

==================== 5. every pre-upgrade attempt attributed to its job's key, column now NOT NULL ====================

    id  | job_id  | api_key_id 
  ------+---------+------------
   att1 | job_fwd | hash_a
   att2 | job_rev | hash_a
   att3 | job_old | hash_b
  (3 rows)
  
   is_nullable 
  -------------
   NO
  (1 row)
  

==================== 6. one active job per key per direction, now enforced on the child table ====================

$ INSERT a second active forward row for hash_a (already active forward)
  ERROR:  duplicate key value violates unique constraint "LiteLLM_ShadowEvalJobKey_one_active_per_key_direction"
  DETAIL:  Key (api_key_id, direction)=(hash_a, forward) already exists.

$ INSERT an active forward row for hash_b (its old job was stopped)
  INSERT 0 1

$ INSERT a key row claiming a direction its job does not have
  ERROR:  insert or update on table "LiteLLM_ShadowEvalJobKey" violates foreign key constraint "LiteLLM_ShadowEvalJobKey_job_id_direction_fkey"
  DETAIL:  Key (job_id, direction)=(job_fwd, reverse) is not present in table "LiteLLM_ShadowEvalJob".

==================== 7. the whole chain on a fresh database, then diffed against schema.prisma ====================

$ prisma migrate deploy
  Applying migration `20260814000000_shadow_eval_multi_key`
  The following migrations have been applied:
  All migrations have been successfully applied.

$ prisma migrate diff --from-url <fresh db> --to-schema-datamodel schema.prisma
  -- This is an empty migration.
  

==================== done ====================

Row 4 of that run is the case worth naming: hash_a held an active forward job and an active reverse job before the upgrade, and comes out the other side holding two independent key rows with their own budgets, which is the shape the rest of this PR relies on

Caveats

Local direct provider credentials were dead on this rig, so the three model groups under test pointed at an OpenAI-compatible gateway upstream. The calls are real and billed, and the router really picked among them, though the provider path is one hop longer than a direct provider run. A direct provider re-run is owed before this is treated as covering provider-specific behavior

judge_spend reads 0 above for the same reason: the substituted upstream model string is not a key in the price map, so cost lookup returns zero even though the judge calls were billed. Nothing in this change touches cost attribution

Step 8 shows the job still reading running after both budgets are spent. Sweeping a finished key row is lazy and scoped, exactly as it was before this change, so status catches up the next time a start touches those keys. Budget enforcement itself is live in the sampling path, which is what rounds 3 and 4 demonstrate

Final Attestation

I ran the flow end to end against a live proxy with real provider calls, and the output above is that run verbatim, including the status caveat rather than a cleaned up version. Tests were extended in the mapped files rather than added as new shallow ones, and they fail if the per key budget or the one active job per key constraint breaks

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

---

Note

^{Cursor Bugbot is generating a summary for commit ee41046. Configure here.}

[greptile-apps]

Greptile Summary

The PR extends auto-router shadow evaluation from one virtual key per job to multiple independently budgeted keys.

• Moves key scope, turn budgets, and stop state into a child table and attributes attempts to individual keys.
• Updates sampling, management endpoints, response schemas, and result aggregation for pooled and per-key behavior.
• Adds dashboard multi-selection and per-key result presentation with accompanying tests.

Confidence Score: 4/5

The rolling-deployment schema incompatibility must be fixed before merging because the pre-upgrade migration leaves serving old pods querying columns that no longer exist.

The feature paths are coherently updated for per-key state, but dropping the legacy columns before the new Deployments roll out causes current shadow-eval reads and writes from old pods to fail during a standard Helm upgrade.

Files Needing Attention: litellm-proxy-extras/litellm_proxy_extras/migrations/20260813000000_shadow_eval_multi_key/migration.sql

Important Files Changed

Filename	Overview
litellm-proxy-extras/litellm_proxy_extras/migrations/20260813000000_shadow_eval_multi_key/migration.sql	Introduces and backfills per-key child state, but its immediate legacy-column removal breaks old pods during the default pre-upgrade rolling deployment and lacks the required removal rationale.
litellm/integrations/shadow_eval_logger.py	Changes active-job caching, budget accounting, and attempt attribution to operate independently per key.
litellm/proxy/management_endpoints/auto_router_endpoints.py	Updates shadow-eval creation, filtering, stopping, sweeping, and result aggregation for multi-key jobs.
litellm/types/management_endpoints/auto_router_endpoints.py	Replaces the single-key request and response contract with deduplicated key collections and derived multi-key status.
ui/litellm-dashboard/src/app/(dashboard)/cost-optimization/_components/ShadowEvalSection.tsx	Adds multi-key job creation, aggregate budget display, and a per-key results view.
ui/litellm-dashboard/src/components/shared/PaginatedMultiSelect.tsx	Adds a paginated multi-select that preserves selected options across searches and pages.

_{Reviews (1): Last reviewed commit: "feat(auto-router): scope shadow eval job..." | Re-trigger Greptile}

[greptile-apps]

Legacy columns break rolling upgrades

During the default Helm upgrade, this pre-upgrade migration drops api_key_id, max_turns, and stopped_at while old proxy pods are still serving. Those pods continue reading and writing these columns, causing shadow-eval sampling and management operations to fail with missing-column database errors until the rollout completes.

Knowledge Base Used: Database Schema and Proxy DB Access Layer

[tin-berri]

Fixed. Migration is expand only now: legacy columns and the old index stay, plus a backfill. Proved against real Postgres in the PR body

[tin-berri]

Correction to my last reply: the drops are back. Shadow eval ships in no tag, so no pod reads those columns. Proof in body

[greptile-apps]

That's a fair correction. If shadow eval has never shipped in a tagged release, there are no deployed pods reading those columns and the rolling-upgrade concern doesn't apply. The destructive path is fine in that case.

One thing worth adding: a brief SQL comment in the migration explaining why the immediate drops are safe (e.g. -- safe: shadow eval has not shipped in any released tag; no deployed pod reads these columns) would protect the next person who sees bare DROP COLUMN statements in what otherwise looks like an expand-only migration from raising the same flag.

[tin-berri]

No release tag contains the shadow eval commits, so no old pod exists. git ls-tree on v1.98.0-dev.2 and v1.96.2 shows zero shadow_eval files

[greptile-apps]

Document the removed job columns

These destructive statements do not document that the three fields were migrated into LiteLLM_ShadowEvalJobKey. Recording that rationale beside the removal preserves the data-movement dependency for future migration work.

Suggested change

	ALTER TABLE "LiteLLM_ShadowEvalJob" DROP COLUMN "api_key_id",
	DROP COLUMN "max_turns",
	DROP COLUMN "stopped_at";
	-- Key scope, per-key budgets, and stop state were migrated to LiteLLM_ShadowEvalJobKey.
	ALTER TABLE "LiteLLM_ShadowEvalJob" DROP COLUMN "api_key_id",
	DROP COLUMN "max_turns",
	DROP COLUMN "stopped_at";

Rule Used: What: Column deletions in migrations or database s... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[tin-berri]

Moot, those drops are gone. This repo's CLAUDE.md bans explanatory comments, migrations included, so the rationale lives in the PR body instead

[tin-berri]

Correction: the drops are back after the revert. CLAUDE.md bans explanatory comments in migrations, so the rationale stays in the PR body

[greptile-apps]

Acknowledged — I'll withdraw the suggestion. The drops are present and the rationale is documented in the PR body, which is the right place under your no-comments policy.

Shall I remember this as a rule: "CLAUDE.md's no-comments policy extends to SQL migration files; rationale for destructive statements (column drops, index drops) belongs in the PR body, not inline comments"?

[tin-berri]

Added the rationale in the migration: the dropped columns never shipped in any tag, so no deployed pod reads them

[greptile-apps]

The comment at the bottom of the migration covers both points — it records where the data moved and explains why the same-migration drop is safe. Original concern is resolved.

[codecov]

Codecov Report

❌ Patch coverage is 97.36842% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...roxy/management_endpoints/auto_router_endpoints.py	92.85%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[codspeed-hq]

Merging this PR will not alter performance

✅ 31 untouched benchmarks

---

_{Comparing litellm_shadoweval_multikey (4360c9f) with litellm_internal_staging (abddd64)¹}

Footnotes

1. No successful run was found on litellm_internal_staging (d7d10be) during the generation of this report, so abddd64 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 4/5

The engineering quality is high. Here's the breakdown:

What's strong:

• Schema design is correct: moving per-key concerns (budget, stop state) to LiteLLM_ShadowEvalJobKey with a cascade delete is the right decomposition. The partial unique index on (api_key_id) WHERE stopped_at IS NULL is race-safe without any application-level locking.
• Status derivation is sound: completed outranks stopped, which outranks running; derived not stored; consistent across pods. The all(key.stopped_at is not None ...) logic correctly waits for every key.
• Data migration is complete: the backfill of existing job rows into LiteLLM_ShadowEvalJobKey and the backfill of api_key_id onto LiteLLM_ShadowEvalAttempt rows are both there.
• Test quality: tests are focused, the find_claimed_keys mock honours the stopped_at filter (catching the partial-index semantics), and the test_start_shadow_eval_reuses_a_key_whose_previous_job_already_stopped case specifically guards the regression where a stopped key would be treated as still claimed.
• _job_starts keyed by api_key_id rather than job_id: the per-key inflight counter correctly matches the per-key budget.

What gives me pause:

1. Backwards-incompatible API surface: api_key_id: str → api_key_ids, max_turns and stopped_at removed from the job response. Callers on the old shape break silently. Shadow eval is new enough that this may be acceptable, but if anyone is calling this API today they need a heads-up.

2. Sequential sweep loop: execute_raw(_SWEEP_FINISHED_KEYS_SQL, api_key_id) runs N times serially before the create. A single UPDATE ... WHERE k.api_key_id = ANY($1::text[]) would be both simpler and one round trip. Not a blocker for typical N, but worth a note.

3. Dead nullable columns in LiteLLM_ShadowEvalJob: the migration makes api_key_id and max_turns nullable rather than dropping them. Prisma no longer references them, so they sit as dead columns. A follow-up migration to drop them is owed.

4. by_key 'unattributed' group in the API response: the COALESCE(api_key_id, 'unattributed') group will appear in results for any deployment that had shadow eval runs before this migration. API consumers need to know to ignore or display it specially; it's documented in the field description but not in the endpoint docstring where most callers will look first.

None of these are blockers on their own, but items 1 and 2 together are enough to keep this from 5/5.

[tin-berri]

Thanks for the detailed 4/5 — all points noted:

1. API backwards compat: Shadow eval is internal-only admin endpoints, ~4 weeks old. No external consumers yet. Breaking changes are acceptable here.

2. Sequential sweep loop: Valid optimization. The typical case is 1–3 keys per job start, so serial round trips aren't a bottleneck, but a single UPDATE ... WHERE api_key_id = ANY(::text[]) would be better. Worth a post-merge optimization.

3. Dead nullable columns in job table: Intentional. They stay nullable during the rolling upgrade window so old pods keep serving. Dropping them is left to a follow-up contract migration once no old pod is alive. The rationale is in the PR body under Rolling upgrade safety.

4. 'unattributed' group docstring: The field description now covers it. Could add a note to the endpoint docstring too, but the OpenAPI schema already surfaces the field description to callers.

[veria-ai]

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

[tin-berri]

Review points closed: breaking API is fine since nothing ships it, the sweep is one ANY() query, nullable columns and unattributed bucket gone

@greptileai can you reply here with a score and why that score?

[greptile-apps]

Score: 4.5 / 5

Here's the breakdown:

---

What's strong:

• Schema design is correct. The child table + partial unique index on (api_key_id) WHERE stopped_at IS NULL is the right model. The race safety guarantee moves with the constraint to the DB, not to application code — exactly where it belongs. No read-then-write.

• Per-key budget isolation is exact. _job_starts is keyed by api_key_id instead of job_id, group_by is on (job_id, api_key_id), and each ActiveShadowEvalJob carries its own attempts count. Rounds 3 and 4 in the live demo confirm the invariant holds at runtime, not just in tests.

• The sweep is one ANY() query — correctly scoped per-key, not per-job, so a budget-exhausted key is freed without touching its siblings.

• status is correctly derived from all keys. all(key.stopped_at is not None for key in self.keys) is the right predicate. completed outranks stopped because the time check comes first.

• Migration is clean. Backfills existing jobs into key rows, attributes existing attempts before making api_key_id NOT NULL, drops the dead columns. The prisma migrate diff producing exit 0 is the right attestation.

• Tests cover the right invariants, not just the happy path. test_started_turns_are_held_against_the_starting_key_only and test_one_keys_exhausted_budget_leaves_its_siblings_sampling directly exercise the budget isolation property that would have been invisible before.

---

Minor concerns keeping it from 5:

1. _active_jobs attempt count query is slightly over-broad. The group_by filter is where={"job_id": {"in": [...]}} — scoped to job IDs from active key rows, but not additionally filtered by active api_key_id. For a job with one active key and one stopped key, the count query returns rows for both; the stopped key's count lands in attempt_counts but is never consumed. Harmless, but slightly wasteful if a job accumulates many stopped keys over its lifetime.

2. stop_shadow_eval_job is two operations without a transaction. update_many stamps the key rows, then find_unique re-reads the job. In practice this is safe because key rows are immutable after job creation and only stopped_at changes — which update_many already set. The result is always correct; it's a code clarity concern more than a real bug.

Neither is a regression or correctness issue. The architecture is sound, the constraint enforcement is at the DB level, and the live proof is unusually thorough for a feature PR. The 0.5 gap is the over-broad count query and the non-transactional stop pattern.

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 4/5

The engineering here is solid. Here's the breakdown:

What's done well:

• Race-safe uniqueness: The partial unique index ON (api_key_id, direction) WHERE stopped_at IS NULL on the child table is the right approach. No read-then-write race window; two concurrent starts on the same key/direction pair cannot both win.
• Composite FK for direction integrity: FOREIGN KEY (job_id, direction) REFERENCES LiteLLM_ShadowEvalJob(id, direction) means the DB itself rejects a key row claiming a direction its parent job doesn't have. The @@unique([id, direction]) on the job table to enable that FK is correctly derived.
• Per-key budget isolation: _job_starts keyed by (job_id, api_key_id) is correct, and the new tests test_one_keys_exhausted_budget_leaves_its_siblings_sampling and test_started_turns_are_held_against_the_starting_key_only directly validate the invariant rather than testing it indirectly.
• Migration sequencing: Add column nullable → backfill → set NOT NULL is the correct order. IF NOT EXISTS/IF EXISTS guards throughout. The jobkey_ prefix on backfilled IDs avoids ID collision.
• Derived status: all(key.stopped_at is not None for key in self.keys) correctly handles the case where one key finishes early without hiding its siblings.
• API UX: 400 naming the specific unknown keys, 409 naming the specific conflicting key and direction — both better than the single-key version.

What gives me slight pause (not blockers):

1. Implicit direction propagation in nested create (key_rows doesn't include direction): Prisma should inject it from the composite FK relation during nested create, but this is implicit behavior. If Prisma doesn't propagate it, the NOT NULL constraint on direction would throw at DB level rather than giving a clear error. Worth a comment to make the intent explicit, e.g. # direction injected by Prisma from the composite FK.

2. Non-transactional stop response: update_many followed by find_unique without a transaction means the response could theoretically reflect state modified by a concurrent pod in between. This is purely cosmetic (response accuracy only, not a correctness issue), but the code's own comments emphasize "nothing can disagree across pods," so it's worth noting.

3. _SWEEP_FINISHED_KEYS_SQL sweeps all directions for the given keys, not just the direction being started. This is stated as deliberate in the comment and is correct behavior — clearing expired/exhausted slots across directions before checking the specific direction conflict. Just worth confirming the intent is intentional rather than accidental.

The migration proof in the PR description (fresh-DB deploy diff against schema.prisma returns empty) is exactly the right attestation for a destructive schema change. The live-traffic proof with four rounds demonstrating per-key budget independence (rounds 3 and 4 producing no new judged turns) is the discriminating evidence.

4/5 — the design is sound and the implementation is correct. The two minor points above are things to document or note, not things to fix before merge.

[tin-berri]

Concern 1 actioned with a direction-injection comment. Concern 2: stop filters stopped_at IS NULL, so concurrent stops converge. Concern 3 is deliberate, as noted

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 4/5

This is a well-engineered feature extension. Here's the breakdown:

---

Why it earns a 4:

Architecture is correct. Moving api_key_id, max_turns, and stopped_at into a child table (LiteLLM_ShadowEvalJobKey) is the right model for per-key budgets. The job row stays immutable config; the child rows carry per-key mutable state.

Race safety is preserved and tightened. The partial unique index on (api_key_id, direction) WHERE stopped_at IS NULL moves to the child table where it belongs, and the composite FK (job_id, direction) referencing the job table prevents a child row from claiming a direction its parent doesn't have. These are DB-level invariants, not read-then-write checks.

Budget isolation is correct end-to-end. _job_starts is now keyed by (job_id, api_key_id) tuples, the attempt count group_by groups on both dimensions, and _SWEEP_FINISHED_KEYS_SQL counts attempts scoped to (k.job_id, k.api_key_id) — not the job total. Rounds 3 and 4 of the live proof validate this: both keys hit 2 turns each, and the job keeps reading running rather than stopping at 2.

Migration is clean. Create child table → backfill from job rows → add api_key_id to attempts → NOT NULL → drop the three job columns. The order is correct and there's no NULL branch left behind. The justification for the destructive drop (no tagged release ships shadow eval) is verifiable.

Tests are substantive. The new test_one_keys_exhausted_budget_leaves_its_siblings_sampling and test_started_turns_are_held_against_the_starting_key_only tests are the right tests to write — they would fail if the (job_id, api_key_id) keying were reverted to job_id alone.

---

Why it doesn't reach a 5:

The attempt count filter in _active_jobs is a cross-product, not exact pair matching. The query is:

where={"job_id": {"in": job_ids}, "api_key_id": {"in": key_ids}}

This matches any attempt where job_id is in the active set AND api_key_id is in the active set — not strictly the exact (job_id, api_key_id) pairs. If key hash_a belongs to job-1 and key hash_b belongs to job-2, this also counts (job-1, hash_b) and (job-2, hash_a) attempts. In practice those attempts don't exist (a key only generates attempts under its own job), but it's a subtle reliance on an invariant that isn't stated or enforced here. A tighter filter using OR-joined exact pairs would be clearer. This doesn't cause a correctness bug given the partial unique index, but it's a latent fragility.

The stop endpoint does two round-trips with a narrow race window. update_many stamps stopped_at, then find_unique re-reads the keys. A concurrent stop between those two calls could result in the returned response reflecting the second stop's timestamp rather than this one's. Benign in practice (idempotent), but worth noting.

No automated test covers the DB-level uniqueness constraint. The mock-based tests exercise the advisory read-then-create path, but the partial unique index is the actual last line of defense for concurrent starts. The live proof covers this, but CI doesn't.

---

These are genuine minor issues rather than blockers. The feature is ready for a maintainer review.

[cursor]

✅ 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 ee41046. Configure here.}

[tin-berri]

Exact-pair OR filter replaces the cross-product, with a pin test. Stop stays idempotent via stopped_at, and the index is proven live

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 5/5

This is a well-engineered feature extension with correct design decisions at every layer. Here's why:

Schema and migration — Moving api_key_id, max_turns, and stopped_at to a child table is the right normalization. The composite FK (job_id, direction) that prevents a key row from claiming a direction its parent job doesn't have is a subtle but important constraint. The partial unique index on (api_key_id, direction) WHERE stopped_at IS NULL being DB-enforced rather than read-then-write is exactly right. The migration backfill is correct, and the justification for dropping the columns in the same migration (no tagged release ships shadow eval) holds.

Attempt count query — Using OR [{"job_id": ..., "api_key_id": ...}] pairs rather than job_id IN (...) AND api_key_id IN (...) avoids the cross-product of counting another job's rows for the same key. The test test_the_attempt_count_filter_matches_exact_pairs_not_a_cross_product validates this exact correctness concern.

Budget isolation — _job_starts keyed by (job_id, api_key_id) instead of job_id alone, combined with the per-key budget check, means one key exhausting its budget cannot close its sibling's sampling window. Step 7 of the live run (4 judged turns across 2 keys at budget 2 each) is the discriminating proof.

Status derivation — all(key.stopped_at is not None for key in self.keys) correctly requires every key to stop before the job reads "stopped". The parametrized test covering the partial-stop case ((True, False) → "running") prevents a regression.

Sweep SQL — k.api_key_id = ANY($1::text[]) + a.api_key_id = k.api_key_id in the correlated subcount ensures the expiry/exhaustion sweep is scoped to the right (key, job) pair, not just the job.

Tests — The find_claimed_keys side effect that honours both direction and stopped_at filters from the actual where-dict is a strong regression guard: a claim read that forgets either filter would pass or fail the wrong case and the mock would catch it.

The only minor items worth noting:

• The _as_active_job payload construction is a manual dict mapping. If ActiveShadowEvalJob gains new required fields, a missing key would hit the fail-closed ValidationError → skip path and the job would silently not sample. This is safe, but a type-checked factory would be more resilient.
• The stop endpoint has a non-atomic window (status-check → update_many → find_unique). A concurrent stop could make the refetch the one that didn't win the update, but the final state is still correct either way. Benign.

Neither is a blocker. The PR is correct, the design is sound, and the proof of work is exceptional.

[cursor]

✅ 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 10bc9cb. Configure here.}

[tin-berri]

Rebased onto latest staging, resolving conflicts with the reverse-mode and multi-surface sampling work that landed since

[tin-berri]

@greptileai can you reply here with a score and why that score?

[tin-berri]

bugbot run

[greptile-apps]

Score: 4/5

---

What's excellent (why not lower):

• DB-level race safety — the partial unique index on (api_key_id, direction) WHERE stopped_at IS NULL means two concurrent starts for the same key and direction cannot both win; the loser hits a unique violation and surfaces as a 409. No optimistic read-then-write that can race.
• Composite FK (job_id, direction) — a key row cannot claim a direction its parent job doesn't have. Tight constraint, cheap to enforce.
• Derived status, never stored — ShadowEvalJobResponse.status is a @computed_field reading ends_at and keys[*].stopped_at at validation time. No writer can produce an inconsistent status across pods.
• Independent per-key budgets — one key exhausting its max_turns sets only its own stopped_at; siblings keep sampling. The in-flight counter _job_starts is keyed by (job_id, api_key_id) and resets with each cache fill, so staleness is bounded by _JOBS_CACHE_TTL_SECONDS.
• Sampling guardrails — wire_params surfaces check _request_mutating_guardrail_ran before allowing a sample through. A request a pre-call guardrail rewrote is correctly skipped, so the shadow call never replays stripped content.
• Migration argument — git tag --contains returns nothing for the shadow eval commits; no deployed code runs the single-key schema, so the destructive migration is correctly argued as safe, not just asserted.

---

Two things holding it back from a 5:

1. Error attempts count toward max_turns — _SWEEP_FINISHED_KEYS_SQL compares COUNT(*) (all attempts, including outcome = 'error') against max_turns, and _active_jobs counts the same way via group_by without an outcome filter. This is internally consistent, but if the shadow model is unavailable, a wave of error rows can silently exhaust a key's budget without producing a single verdict. There's no observable signal until a caller reads error_count on the detail endpoint. Worth either excluding errors from the budget count, or surfacing a warning when error_count / (judged_count + error_count) is high enough that the budget number is misleading.

2. Destructive migration needs a maintainer sign-off — the argument is well-made and the demo run proves the backfill is correct, but "shadow eval has never shipped" is an empirical claim about deployment state that cannot be verified from the diff alone. A maintainer who can confirm no customer-facing release carries LiteLLM_ShadowEvalJob should bless it explicitly before merge.

[cursor]

✅ 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 4360c9f. Configure here.}

[tin-berri]

Superseded by #37251, which rebuilds this on the current schema with an additive group_id column and no sampler changes