Skip to content

feat(spend): fold auto-router benchmarks into a per-session rollup - #35474

Closed
tin-berri wants to merge 82 commits into
litellm_lit5046_autorouter_savingsfrom
litellm_lit4712_autorouter_session_rollup
Closed

feat(spend): fold auto-router benchmarks into a per-session rollup#35474
tin-berri wants to merge 82 commits into
litellm_lit5046_autorouter_savingsfrom
litellm_lit4712_autorouter_session_rollup

Conversation

@tin-berri

@tin-berri tin-berri commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • The auto-router benchmarks dashboard answered every question by scanning LiteLLM_SpendLogs at read time: four aggregate queries per auto-router, two of them window functions over the response JSONB, re-deriving on every page load which model the previous turn used, how long a tier had been idle, and how big the prefix was last time
  • Those are sequential facts, and the request that produces them already knows all of them, so the work was being redone on the widest table in the schema (492 MB for 8k rows on the rig used here)
  • The turn buckets were not exhaustive: a session's opening turn landed in the headline turn count and in none of the three buckets, so the bucket totals silently disagreed with the headline

How it solves it:

  • A LiteLLM_AutoRouterSession row per (session, auto-router) carries both the counters and the state that classifies the next turn, so each turn is classified once, when it happens
  • The read path is a single aggregate over pre-folded rows covering every auto-router at once, and touches no per-request table at all
  • Nothing reads LiteLLM_SpendLogs anywhere in the feature, with no exception to qualify

Relevant issues

Linear ticket

Resolves LIT-4712

Pre-Submission checklist

  • 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

Screenshots / Proof of Fix

Proxy booted on the rollup branch against a database holding 30 days of real auto-routed traffic.

Three real turns in one session against a real provider (the sandbox gateway stood in for api.anthropic.com, whose key is out of credit here; a direct re-run is owed), with no backfill involved anywhere:

$ for i in 1 2 3; do curl -s http://localhost:4716/v1/chat/completions \
    -H "Authorization: Bearer $LITELLM_MASTER_KEY" -H "Content-Type: application/json" \
    -d "{\"model\":\"live-auto\",\"messages\":[{\"role\":\"user\",\"content\":\"Reply with exactly: turn$i\"}],\"litellm_session_id\":\"$SID\"}"; done
turn1 -> live-auto | turn1
turn2 -> live-auto | turn2
turn3 -> live-auto | turn3

$ psql litellm_autorouter -c "SELECT ... FROM \"LiteLLM_AutoRouterSession\" WHERE session_id='$SID'"
 model_group | router_kind | turns | same_model_turns | first_visit_turns | return_turns | last_model                             | spend
-------------+-------------+-------+------------------+-------------------+--------------+----------------------------------------+----------
 live-auto   | complexity  |     3 |                2 |                 1 |            0 | anthropic/us.anthropic.claude-opus-4-8 | 0.000677

The rollup advanced from the request path alone, the three buckets sum to the turn count, and the persisted model_state is what lets the next turn classify correctly after a restart or a move between pods.

Read back through the endpoint:

$ curl -s "http://localhost:4713/auto_router/benchmarks?start_date=2026-07-02&end_date=2026-08-01" \
    -H "Authorization: Bearer $LITELLM_MASTER_KEY"
auto              baseline=anthropic/claude-opus-4-8   routed=$34.65   vs baseline=$43.87    saved=$9.22 (21%)
claude-auto       baseline=anthropic/claude-opus-4-8   routed=$325.21  vs baseline=$365.27   saved=$40.06 (11%)
claude-router-2   baseline=anthropic/claude-opus-4-8   routed=$4.73    vs baseline=$5.49     saved=$0.76 (14%)

Type

🆕 New Feature

Changes

fold_turn is pure: it takes the session's prior state and one turn's facts and returns the increments plus the next state, with no I/O and no clock, so every rate and dollar formula is testable without a database. AutoRouterSessionQueue follows AdaptiveRouterUpdateQueue: the logging path only folds into memory, and a background flusher writes atomic increment upserts, so two pods writing one session compose rather than overwrite. A pod that has never seen a session loads its row once and classifies from memory afterwards, which costs one read per session rather than one per turn.

The hook sits in update_database rather than beside the daily transactions, because that is the one place a request passes through exactly once; the daily path runs per entity type and would have counted every turn six times over. It is independent of disable_spend_logs, since the rollup is what the dashboard reads now, and it can never raise: a dashboard aggregate is not worth failing spend tracking over.

Retention rides the existing spend-log cutoff via SpendLogCleanup, keyed on last activity rather than session start so a long-running conversation is not pruned out from under itself.

Two behaviour fixes came with the move. The three turn buckets are now mutually exclusive and exhaustive, because a session's opening turn is a first visit to whatever tier served it. And a turn with no ephemeral cache-creation evidence now reads as the five minute tier rather than the one hour tier, which had been the default only because zero is not less than zero.

QA runbook

  1. Boot a proxy with at least one auto-router configured
  2. Send two turns in one session through an auto-router alias with the same litellm_session_id; after the next spend flush the row should read turns=2 with one first visit and one same-model turn
  3. GET /auto_router/benchmarks?start_date=<30d ago>&end_date=<today> and confirm, per group, that same_model_turns + first_visit_turns + return_turns equals cache.turns
  4. Restart the proxy and send a third turn in that same session; it should read as a same-model continuation rather than a new first visit, which is the state round-trip working
  5. Confirm the table is pruned by the existing spend-log retention cutoff rather than growing without bound

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

milan-berri and others added 3 commits July 24, 2026 19:04
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…file ids

CheckBatchCost built unified output file ids with the provider model name, so key model-access checks resolved the file to e.g. gpt-5.5 and every GET /v1/files/{output_file_id}/content failed. Resolve the model group from the batch's managed input file, falling back to the deployment's model_name.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@greptile-apps

greptile-apps Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces read-time auto-router benchmark scans with cumulative per-session rollups and adds live folding, historical backfill, retention, and benchmark endpoints.

  • Adds a three-schema Prisma model and migration for LiteLLM_AutoRouterSession.
  • Folds auto-router turns through an in-memory queue and periodically upserts counters and sequential state.
  • Adds aggregate benchmark and administrative backfill endpoints.
  • Updates cache-bucket and auto-router savings calculations and their tests.

Confidence Score: 4/5

The persistence and concurrency defects must be fixed before merging because they can permanently lose rollup increments or overwrite valid session history.

Failed flushes discard pending increments, partial backfills replace cumulative rows, and independent pod-local folds cannot preserve the global sequential state required by the benchmark classifications.

Files Needing Attention: litellm/proxy/spend_tracking/auto_router_session_queue.py, litellm/proxy/spend_tracking/auto_router_backfill.py

Important Files Changed

Filename Overview
litellm/proxy/spend_tracking/auto_router_session_queue.py Introduces live session folding and atomic counter upserts, but failed writes are discarded and sequential state does not compose across pods.
litellm/proxy/spend_tracking/auto_router_backfill.py Replays historical turns through the shared fold, but absolute updates can erase cumulative state outside the selected window.
litellm/proxy/spend_tracking/auto_router_sessions.py Implements the pure classification and cache-economics fold with exhaustive turn buckets and serialized next-state.
litellm/proxy/spend_tracking/auto_router_benchmarks.py Aggregates pre-folded session rows into benchmark responses with bounded date windows.
litellm/proxy/db/db_spend_update_writer.py Hooks rollup recording into the spend path and flushes it independently of the Redis-elected writer.
litellm/proxy/proxy_server.py Adds authenticated benchmark and admin-only backfill routes with database and router availability checks.
litellm-proxy-extras/litellm_proxy_extras/migrations/20260801000000_add_auto_router_session_rollup/migration.sql Creates the cumulative session table and activity indexes consistently with the Prisma models.

Reviews (1): Last reviewed commit: "feat(spend): fold auto-router benchmarks..." | Re-trigger Greptile

Comment on lines +169 to +177
async def flush(self, prisma_client: "PrismaClient") -> int:
"""Drain the aggregate into the session rollup. Returns rows written."""
async with self._lock:
batch = self._pending
self._pending = {} # mutable-ok: fresh aggregate for the next interval

for key in sorted(batch.keys()):
await self._write(key, batch[key], prisma_client)
return len(batch)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Failed flushes discard increments

When a session upsert encounters a transient database error, flush has already removed the entry from _pending and _write catches the exception without requeuing it, permanently omitting that interval's turns, tokens, spend, and cache counters while leaving the in-memory state ahead of the durable row.

Knowledge Base Used:

Comment on lines +302 to +307
"session_id": session_id,
"model_group": model_group,
**record,
},
"update": record,
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Partial backfills replace session history

When an administrator backfills a narrower or overlapping window for a session with existing turns outside that window, the absolute update record replaces the cumulative per-session row with only the selected rows, erasing counters, timestamps, and model state and causing dashboard undercounts and incorrect classification of the next turn.

Knowledge Base Used:

Comment on lines +207 to +213
"update": { # mutable-ok: prisma's write API takes dict payloads
**{ # mutable-ok: a JSON object is a dict by definition
field: {"increment": value} # mutable-ok: a JSON object is a dict by definition
for field, value in counters.items() # mutable-ok: spread into the prisma payload immediately below
}, # mutable-ok: spread into the prisma payload immediately below
**shared,
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Pod-local state loses turn ordering

If requests for the same session overlap across pods, each pod folds against its own stale prior state; the counters increment atomically but last_model and model_state use last-writer-wins assignments, causing incorrect turn buckets and leaving subsequent turns classified from whichever pod flushed last.

Knowledge Base Used: Database Schema and Proxy DB Access Layer

@tin-berri
tin-berri force-pushed the litellm_lit4712_autorouter_session_rollup branch from 32c85a6 to 05b285f Compare August 1, 2026 20:49
yassin-berriai and others added 4 commits August 1, 2026 14:12
#35485)

The migrations image ran `prisma migrate deploy` against a bake anchored in
$HOME with no node in the runtime stage, so prisma-client-py fell through to
nodeenv and tried to download a Node runtime on first start. In an
egress-restricted cluster that fails outright, and under an arbitrary uid the
uid-specific cache path is unreadable, so the job never applies a migration.

Move the bake to /opt/prisma with world-readable modes, install node in the
runtime stage, and pin PRISMA_BINARY_CACHE_DIR / PRISMA_CLI_PATH /
PRISMA_OFFLINE_MODE so the migration entrypoint runs the cached CLI directly.
This is the same treatment the root, non_root and database images already
carry.

Resolves LIT-4727
…_id_encoding_lit4964

fix(batches): encode public model group on background-created output file ids
…nd images (#35490)

The componentized images exec uvicorn directly, so ddtrace-run never wraps the
interpreter. USE_DDTRACE is not inert there; the proxy lifespan still runs
patch_all and litellm's own manual spans still emit. What never gets installed
is ddtrace's ASGI TraceMiddleware: starlette builds its middleware stack lazily
on the first __call__, which is the lifespan scope, so patching from inside the
lifespan body is already too late and no root request span is ever created.

Route both entrypoints through a shared docker/component_entrypoint.sh that
mirrors the monolith's prod_entrypoint.sh contract, including the
DD_TRACE_OPENAI_ENABLED=False export that keeps ddtrace's openai integration
from double-reporting calls litellm instruments itself.

Co-authored-by: Yassin Kortam <yassin.kortam@gmail.com>
…#35497)

The gateway and backend probes omitted timeoutSeconds, so kubelet applied its
1s default. Both containers run a single uvicorn worker (the gateway defaults
NUM_WORKERS to 1; the backend passes no --workers at all), so each pod is one
asyncio event loop and its per-request latency under closed-loop saturation
rises by queueing (~57-62ms serial vs ~6s at 100 concurrent users against one
replica). Both /health/readiness and /health/liveliness then time out on the
stage cluster while the pod is serving traffic correctly, which exposes the
deployment to losing a healthy pod from its load balancer during a burst and
to restarting a merely busy one.

Readiness now gets timeoutSeconds 10, equal to periodSeconds and above the
measured saturated latency, and keeps failureThreshold 3. kubelet drives each
probe from a time.Ticker of periodSeconds rather than sleeping between
attempts, and coalesces ticks that arrive mid-probe, so the interval between
probe starts is max(periodSeconds, probeDuration) and not their sum. Keeping
timeoutSeconds <= periodSeconds is what holds that interval at 10s, so three
consecutive failures still evict a genuinely wedged pod in ~30s.

Liveness gets the same timeout plus failureThreshold 6: /health/liveliness is
an in-memory flag check, so a timeout there only ever means event-loop
starvation, which a restart makes worse, and it now needs ~90s of sustained
unresponsiveness to fire.

The ui container keeps the default. It is nginx serving a Next.js static
export, so / is a file off disk with no application runtime that could queue
behind saturated work, and nothing measured suggests it needs more than 1s.
@tin-berri
tin-berri force-pushed the litellm_lit4712_autorouter_session_rollup branch from 05b285f to 8f7b24a Compare August 1, 2026 21:31
@tin-berri
tin-berri requested a review from a team August 1, 2026 21:31
… of a delete count

_delete_deployment stopped returning a count of evictions in #35400 and now returns
the frozenset of ids the db and config still want, so a caller judging its own reload
can tell a deliberate eviction from a deployment that went missing. These two tests in
tests/local_testing were left comparing that frozenset against an int and have been
failing since; the directory is only referenced by .circleci/config.yml, which no
longer reports checks on PRs, so nothing caught them.

The eviction behavior itself is unchanged, so the fix is on the assertions: compare
against the expected id set, and pin the router's surviving ids so a mutation that
evicts the wrong deployment is caught rather than passing a bare length check.
usage_object=usage_obj,
)
await self.auto_router_session_queue.record_turn(
key=(session_id, model_group),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low: Unbounded session identifiers exhaust worker memory

An authenticated caller can submit distinct, oversized litellm_session_id values on auto-router requests. Each value becomes a queue key retained in _state for up to 10,000 sessions, while _pending is unbounded between flushes and flushing does not clear _state; sequential requests can therefore retain enough attacker-controlled data to terminate the worker. Normalize or hash identifiers to a fixed size before queueing them, and bound pending/cache storage by bytes as well as entry count.

@veria-ai

veria-ai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

PR overview

This PR updates spend processing to aggregate auto-router benchmark data into per-session rollups. It introduces session-based queueing and state management in the database spend update writer.

One availability issue remains open: an authenticated caller can supply many distinct, oversized session identifiers that are retained in memory, potentially exhausting memory and terminating a worker. No reported issues have been addressed yet, so bounding or normalizing session-key storage remains necessary.

Open issues (1)

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

yuneng-berri and others added 3 commits August 1, 2026 14:39
…oning gate

Adding a team member by a user_id with no user row is now proxy-admin-only,
so the /team/member_add authz matrix, which targeted a never-seeded user_id,
started 403ing every non-proxy-admin caller. Seed the member as a real user
row so the matrix reads _validate_team_member_add_permissions alone; leaving
it unseeded and relaxing the expectations to 403 would have left all 18 rows
green with that gate deleted outright.

Cover the new gate at the HTTP boundary, where only the helper was pinned
before: a team admin and an org admin both clear the permission check on the
same team and are still refused an unprovisioned user_id, with no user row
left behind. Pin the escape hatch that refusal names too, so closing the
email-invite path for non-proxy-admins cannot pass silently.

Promote the user seeder the member-info pins had kept private to conftest,
and reclaim invited users by their scratch-prefixed email, since an invite
allocates the user_id server-side.
…ends

Gating the mock testing request params behind
general_settings.dangerously_allow_mock_testing_request_params (#35423) turned
every fallback, retry and timeout drill in tests/test_fallbacks.py into a 400:
the build_and_test job mounts proxy_server_config.yaml, which never opted in.

Opt that config in. It is the config the CI proxy runs with, and the suite it
serves exists to drive synthetic failures.

Add a unit test that ties the two together: it scans the top-level tests/test_*.py
files build_and_test globs for gated param names and fails if the config they run
against has not opted in, so the next change to either side is caught in a fast
lint-tier job rather than a Docker E2E.
@codspeed-hq

codspeed-hq Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_lit4712_autorouter_session_rollup (8f7b24a) with litellm_lit5046_autorouter_savings (d539fa6)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_lit5046_autorouter_savings (10a2a29) during the generation of this report, so 22ea702 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

yuneng-berri and others added 6 commits August 1, 2026 15:25
`import litellm` reaches litellm/integrations/otel/model/config.py via
litellm_core_utils/litellm_logging.py, so pydantic-settings is needed at import
time. It was declared only in the `proxy` extra, which left a plain
`pip install litellm` unimportable on every platform.

Adds tests/base_sdk_tests/check_base_sdk_install.py and a base_sdk_install
CircleCI job that builds the wheel, installs it into a clean venv with no extras,
and smoke-checks the import, a mock completion, a mock embedding, the bundled
pricing metadata and the token counter. The check is stdlib-only on purpose;
installing pytest into that venv would add packaging, pluggy and iniconfig and
could mask the class of undeclared dependency it exists to catch.

Previously the Windows job was the only one installing without extras, so this
class of break was caught by accident rather than by design.
…-tests-e7dc24

fix(ci): let the E2E proxy accept the mock testing params its suite sends
…-tests-3948e1

test(proxy): separate the member_add permission gate from the provisioning gate
…ata-4685a9

test(logging): pin routing_decision and internal_call_origin in the gcs pubsub spend log fixture
…-Router screens (#35500)

PR #35471 added classifier_context_include_assistant_turns to ComplexityRouterConfig.
It worked through config.yaml and the model API but had no control on the Add Model or
Edit Auto-Router screens, so an operator working from the dashboard could not reach it.
Wires it into the create and edit forms, shown only when the LLM classifier is
selected, matching what #35315 did for the two context-window fields

The create and edit stacks share the rendered control but keep their own serializer,
their own hydration, and their own managed-key set, so the field is added in five
places rather than one. A field wired into only one stack fails in a way neither
serializer unit test can see, since those are handed a form value assembled by hand,
so the edit-modal test drives the real component through open, edit and save

The switch is emitted even when false, because there the operator turning it off is a
choice that has to overwrite a stored true rather than an absent value a truthiness
gate would drop
…745045

test(proxy): assert _delete_deployment's still-desired id set instead of a delete count
ryan-crabbe-berri and others added 22 commits August 3, 2026 16:19
…locked dynamic params (#35115) (#35687)

Team-scoped DD credentials (dd_api_key, dd_site) set via POST /team/{id}/callback were silently dropped because _request_blocked_callback_params blocks them from standard_callback_dynamic_params. The security block is correct for request-level injection, but team callback_vars are admin-configured and trusted.

Store the raw init kwargs on the Logging instance and read dd_* params from there in _process_dynamic_callback_list instead of from standard_callback_dynamic_params.

Adds an integration test that exercises the full Logging.__init__ flow with team callback_vars to prevent regression.

Co-authored-by: Aanchal Khandelwal <aan2210khandelwal@gmail.com>
refactor(ui): extract the MCP create form's logic and field groups
Adds 61 unit tests on the modules #35694 extracted: 46 on the payload
builder, 15 on the OAuth redirect snapshot. They run in 9ms against 240s
for the 77 full-render tests they partly replace. Nine of nine mutants
were killed when the extracted logic was deliberately broken, so the
speed does not come at the cost of signal.

Deletes six cases across four blocks that rendered the whole modal to
assert one payload key belonging to a field they never touched. Every
test that proves a form field reaches the right payload key stays; those
cover field to form value to payload, which a unit test cannot reach.

Replaces "should not render when user is not an admin", which asserted
the admin title was absent and so passed for the wrong reason: the modal
does render for a non-admin, retitled. registerMCPServer was mocked but
never asserted anywhere, leaving the whole non-admin submission path
uncovered. It now drives a real submit and asserts the call lands there
and never on createMCPServer.

Renames the slow file to CreateMCPServer.integration.test.tsx and
documents the three tiers in the dashboard CLAUDE.md. No production code
changes.
The Pretty view only parsed the Chat Completions shape (messages /
choices[0].message), so any spend log storing the Responses API shape
(input / output) rendered an empty Input card and the literal text
"No response data available" even though the row held the full request
and response. This also hit plain /v1/chat/completions callers, because
litellm may route those over the Responses bridge and then store the
upstream Responses-shaped body.

Parsing now branches on a tagged union covering both shapes, which also
replaces the any-typed key sniffing and the role guessing it relied on.
…groups, and cache settings

Replace Any seams in three proxy modules with real types so the values keep
their shape through the call graph:

- reset_budget_job: Protocols for the Prisma spend-linked tables, the reset
  batcher, and each cascade row shape, with the per-table counter/cache key
  lambdas promoted to typed module functions so the row type is inferred
- access_group_endpoints: Protocols for the access group record, the team and
  key tables, and the transaction handle; record to response conversion now
  goes through model_validate on the record dict
- cache_settings_endpoints: the opaque cache settings blobs are Mapping[str,
  object] / dict[str, object] instead of Any, keeping Any only on the two
  returns that feed the dynamic litellm.Cache kwargs bag

Whole-tree basedpyright: reportAny 19435 -> 19306, reportExplicitAny 6518 ->
6487, total errors 148372 -> 148117, with no rule above its baseline and no
untouched file changed. No behavior changes.
test(ui): tier the MCP create tests into unit and integration
…5678)

* fix(proxy): redact credential headers from request logging copies

clean_headers preserves an Anthropic subscription OAuth token, and other
client-supplied provider credentials, so they can be forwarded upstream. The
same dict was also stored as proxy_server_request["headers"] and
metadata["headers"], so those credentials reached every logging callback and
the SpendLogs proxy_server_request column that the Admin UI logs page renders.

Build the observability facing copies through redact_credential_headers, and
drop the transport-only keys (provider_specific_header, headers, api_key) from
the request body snapshot since they have to keep the real values.

* fix(proxy): use the redacted header copy in the request debug log

The stdout secret filter matches Bearer and sk- shaped values, so an MCP auth
token printed by the request-header debug line survived it in cleartext.

* fix(proxy): resolve the configured MCP auth header name through the secret manager

get_secret_str also consults a configured secret manager, so a deployment that
stores the header name there now gets that header masked too. Drops the added
comments in favour of a named constant.

* perf(proxy): resolve the MCP auth header name once per process

get_secret_str issues a blocking secret-manager SDK call when one is configured,
and configured_credential_header_names runs on every proxied request.

* fix(proxy): read the MCP auth header name live, cache only the secret manager

The config reloader rewrites os.environ on an interval and after /config/update,
and MCPRequestHandler resolves the same setting per request, so caching the env
lookup left a renamed header logged in the clear until the process restarted.
Only the blocking secret-manager call stays cached.

* refactor(proxy): narrow header redaction to the reported credential set

Drops the MCP header-name resolution, its per-request config and secret-manager
lookups, and the x-mcp- prefix rule. Those cover a separate credential family
than the one this ticket reports and carried their own config-reload staleness
surface; they belong in their own change.

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…treaming buffer, failure logging (#35722)

* feat(guardrails/rubrik): prompt moderation, response-text blocking, streaming buffer, failure logging (#34019)

* feat(guardrails/rubrik): add prompt moderation, response-text blocking, streaming buffer, failure logging

- Add `pre_call` prompt moderation via `/v1/before_prompt/openai/v1` webhook:
  structured messages are flattened and sent before the LLM is called; blocked
  prompts surface a `ModifyResponseException` with the refusal text.
- Extend `post_call` response moderation to cover assistant text in addition to
  tool calls; text blocks (wholesale replacement) are distinguished from
  tool-block explanations (appended) via `startswith` diffing.
- Add `streaming_end_of_stream_only = True` and `streaming_buffer_until_moderated = True`
  so streamed responses are withheld until end-of-stream moderation passes
  (requires litellm >= #31389; older versions fall back to
  detect-only).
- Add `_MalformedToolBlockingResponseError` for structurally invalid service
  responses; `_guarded` logs at CRITICAL so operators notice misconfiguration.
- Add `max_queue_size = 10_000`, `_enforce_max_queue_size`, and drop-oldest
  backpressure so a webhook outage cannot grow the retry queue unboundedly.
- Add `flush_queue` override that snapshots once for both send and drain,
  preventing duplicate delivery on concurrent flush calls.
- Make `_log_batch_to_rubrik` re-raise on error so `flush_queue` preserves
  undelivered events for the next retry.
- Add `async_post_call_failure_hook` to log blocked requests
  (`ModifyResponseException`) with a best-effort fallback payload for prompt
  blocks (where no `standard_logging_object` exists yet).
- Add `_correlation_id` / `_apply_correlation_id` / `_prepend_system_prompt`
  helpers; `_prepare_log_payload` now applies them for all providers (not just
  Anthropic) so every log correlates by `litellm_call_id`.
- Add `get_supported_event_hooks` classmethod advertising `[pre_call, post_call]`.
- Use dedicated `httpx.AsyncClient` (`moderation_client`) for webhook calls
  with explicit pool limits, separate from the shared logging client.
- Drop module-level `rubrik_handler` singleton (inappropriate for a library).
- Update `initialize_guardrail` docstring to explain `pre_call` vs `post_call` mode.
- Update tests: rename `tool_blocking_client` → `moderation_client`,
  `tool_blocking_endpoint` → `response_moderation_endpoint`, `_flush_task` →
  `_periodic_flush_task`; migrate `TestExtractBlockedTools` to
  `TestExtractResponseBlock` for the new combined text+tool block API; add
  tests for prompt moderation, text blocking, streaming flags, and failure
  payload construction.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* test(guardrails/rubrik): add tests to reach 100% coverage

50 new tests across 18 classes covering previously-untested paths:

- Prompt moderation: passthrough, block, no-messages skip, message
  flattening (content-list → string), payload construction with
  tools/user/correlation_key/litellm_call_id fallback, refusal extraction
- async_post_call_failure_hook: non-matching exception no-op, missing
  stash warning, valid stash → enqueue, AttributeError in payload build,
  flush exception handling
- Block payload building: standard_logging_object present vs fallback
  path, missing start_time
- async_log_success_event: _rubrik_blocked=True skip path
- aclose: task cancel + moderation_client.aclose()
- Edge cases: sampling rate clamp warning, unknown input_type passthrough,
  empty-inputs early return, model_call_details warning, _stash_block_context,
  duck-typed tool-call normalization, request_data["tools"] preference over
  optional_params, system-prompt exception handler, flush-at-batch-size,
  enqueue exception swallowing, queue empty/lock-None guards, non-dict JSON
  response TypeError

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): use get_async_httpx_client, ruff format

- Replace bare httpx.AsyncClient with get_async_httpx_client (required
  by ensure_async_clients_test; avoids per-request client creation)
- aclose() calls close() (AsyncHTTPHandler interface, not aclose())
- ruff format on rubrik.py and guardrail_hooks/rubrik/__init__.py
- Update 3 tests for AsyncHTTPHandler type (isinstance check, close())

osv-scan and documentation CI failures are pre-existing on the base
branch and unrelated to this PR.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): fix UP006 strict ruff violation

get_supported_event_hooks return type used List[...] (UP006) instead of
list[...]. Replace with the built-in generic and remove the now-unused
List import from typing.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): fix 3 reportArgumentType basedpyright violations

Use `# pyright: ignore[reportArgumentType]` (not `# type: ignore`) to
suppress the three errors basedpyright reports in --outputjson mode:
- convert_content_list_to_str call (dict vs AllMessageValues)
- _apply_correlation_id call (StandardLoggingPayload vs dict[str, Any])
- _prepend_system_prompt call (same)

Also tighten _apply_correlation_id and _prepend_system_prompt signatures
from bare `dict` to `dict[str, Any]`.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): don't close shared HTTP client in aclose()

moderation_client and async_httpx_client both come from LiteLLM's global
HTTP-client cache (get_async_httpx_client keys on llm_provider + params).
Two RubrikLogger instances with the same parameters share the same
underlying AsyncHTTPHandler object. Calling close() in aclose() closed
the shared connection pool for all instances, breaking any subsequent
moderation request on other loggers.

aclose() now only cancels the periodic flush task and lets LiteLLM
manage the shared client lifecycle. Tests updated to assert close() is
NOT called.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): use Counter for duplicate tool-call ID detection

Set-based comparison lost ID multiplicity: two original tool calls with
the same ID both appeared "allowed" even when the service returned only
one (e.g. one allowed + one prohibited sharing an ID). Replace with
Counter so returned_id_counts[id] >= required_id_counts[id] must hold
for every ID. Matches the approach in the original _extract_blocked_tools.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): respect default_on=true when omitted from config

LitellmParams.__init__ converts an omitted default_on to False before
initialize_guardrail receives it, so litellm_params.default_on is always
bool and never None. The is-None guard in RubrikLogger.__init__ therefore
never fired on the proxy path, leaving prompt/response moderation inactive
for any config that omitted default_on.

Fix: read the raw guardrail dict (before LitellmParams coercion) to
distinguish an explicit `default_on: false` from the absent-means-True
default. When the key is absent from the raw config, default_on=True is
used; when it is explicitly set (either True or False), that value wins.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* style: ruff format rubrik.py after Counter import addition

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): detect ID-less tool call removal; fix UP045

ID-less tool calls (tc.id is falsy) were excluded from required_id_counts,
so the Counter comparison never caught their removal. Add a cardinality
check (len(returned) < len(original)) that fires on any removal regardless
of ID presence, combined with the Counter check for duplicate-ID attacks.

Also fix 5 UP045 violations (Optional[X] → X | None) introduced by our
new code against the daily-branch baseline.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): filter optional_params through ModelParamHelper in fallback payload

_build_fallback_payload forwarded the raw optional_params dict as
model_parameters. optional_params can contain extra_headers, api_key,
and other upstream provider credentials that must not reach the Rubrik
webhook. The normal standard_logging_object path already filters through
ModelParamHelper.get_standard_logging_model_parameters(), which
allowlists only safe LLM API parameters. Apply the same filter here.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): scope failure hook by guardrail_name; moderate text-completions

Guard async_post_call_failure_hook by guardrail_name so multiple Rubrik
instances don't cross-log: the failure hook is called for every registered
callback; without the check the first instance pops the stash and the
originating instance finds None and silently skips logging. Now each
instance only handles blocks raised by itself.

Also moderate /v1/completions prompts: _moderate_prompt returned early
when structured_messages was absent. For text-completion requests litellm
supplies inputs["texts"] with no structured_messages. Added a fallback
that synthesises a user-message from texts so the before_prompt webhook
can evaluate text-completion prompts.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(lint): add reason comments to pyright: ignore suppressions

type-discipline budget requires each # pyright: ignore[...] to carry an
explanatory comment. Add reasons to the three bare suppressions on lines
483, 651, 652.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): include tool-call arguments in prompt moderation

_flatten_messages_for_moderation only sent the content field, silently
dropping tool_calls[].function.arguments and function_call.arguments.
An attacker could embed prohibited text in tool-call arguments inside
assistant history turns and bypass prompt moderation entirely.

Now collects all attacker-controlled text per message: text content via
convert_content_list_to_str, plus all tool_calls[].function.arguments
and the deprecated function_call.arguments, joined with newlines before
being sent to the before_prompt webhook.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): tighten append detection to prevent prefix bypass

startswith(sent_content) allowed any replacement whose text shares the
original as a prefix (e.g. "Hello" → "Hello, blocked.") to be classified
as a tool-block append rather than a text block, bypassing detection.

Use startswith(f"{sent_content}\n\n") to require the exact two-newline
separator the webhook uses between original text and appended tool-block
explanations. Also add `returned_content != sent_content` to text_blocked
so an unchanged passthrough is never classified as a block.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(guardrails/rubrik): default_on=False when omitted (follow existing pattern)

Remove the custom raw-dict lookup that was defaulting default_on to True
when omitted from the guardrail config. Follow the standard litellm
convention: omitted resolves to False (users must explicitly opt in with
default_on: true).

- initialize_guardrail: pass litellm_params.default_on directly
- RubrikLogger.__init__: is-None guard defaults to False not True
- Test updated to assert the correct False default

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* chore(rubrik): keep the ported guardrail within staging lint budgets

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* chore: credit the original author of the rubrik guardrail work

Co-authored-by: Joseph Barker <156112794+seph-barker@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* chore: keep this mirror PR's diff limited to the rubrik files

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: Joseph Barker <156112794+seph-barker@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: yucheng <yucheng@berri.ai>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
fix(ui): render Responses API request and response in the logs drawer
* fix(ui): hide guardrail review buttons from non-admin users

The team guardrail submissions list rendered Approve/Reject buttons for
non-admin users even though the backend correctly rejected the calls.
Thread userRole from the page through GuardrailsPanel into
TeamGuardrailsTab and gate the row-card and detail-panel review buttons
on isAdmin so the UI matches the backend authorization.

Defense in depth only — the backend remains the source of truth and is
double-gated at both the route admin check and the explicit endpoint
role check.

Refs LIT-2494

* refactor(ui): read userRole from useAuthorized hook instead of prop drilling

Drop the userRole prop chain through GuardrailsPage → GuardrailsPanel →
TeamGuardrailsTab. Each component reads userRole directly from the
useAuthorized hook, matching the pattern used elsewhere in the dashboard.

Tests now mock useAuthorized per case (the same pattern as
top_key_view.test.tsx) instead of passing userRole as a prop.

Refs LIT-2494

* fix(ui): drop userRole prop on GuardrailsPanel call site in src/app/page.tsx

Missed in the earlier refactor — GuardrailsPanel no longer accepts
userRole as a prop (reads from useAuthorized hook), so callers must
not pass it. The build was failing in production type-check.

Refs LIT-2494

* fix(ui): gate guardrail forward-key toggle and header editors on proxy admin

* refactor(ui): remove dead app_admin case from user role formatting
#33353)

* feat(team): custom metadata validation hook for team create and update

Operators can point general_settings.custom_team_metadata_validate at an
async Python function that validates team metadata before /team/new,
POST /team/update, and PATCH /team/{team_id} commit their writes. The
hook receives the metadata that will actually be written (the merged
result on PATCH) plus the stored metadata and requester context, and
fails closed: a rejected value returns the function's own message as a
400 while any exception or timeout blocks the write with a configurable
generic message as a 503. Premium-gated like enforced_params.

* fix(team): validate metadata before model alias writes and strip system keys from validator input

Review follow-ups on the team metadata validation hook: run the validator
before the model_aliases table insert so a rejected create leaves no
orphaned model rows, strip system-managed keys from existing_metadata so
the validator sees symmetric input on both fields, and accept class
instances exposing an async __call__ as validators. Adds a three-way
validator implementation matrix (allowlist function, HTTP-service-backed
function, immutability-enforcing class instance) driven through the real
create, update, and patch endpoints, including an HTTP stub service and
outage coverage.

* test(team): run the metadata validation matrix against the DB-backed proxy in CI

Adds the validator matrix to the proxy_store_model_in_db_tests CircleCI
job so every scenario runs full e2e against a Postgres-backed proxy. The
proxy config registers a dispatching validator that routes each request
to one of the three implementations via a metadata key and accepts
anything that does not opt in, keeping the rest of the suite unaffected.
CI starts a stand-in cost center service on the host for the HTTP-backed
implementation, reached from the container via host.docker.internal, and
the outage path targets a closed port to prove the fail-closed 503
without stopping services.

* feat(ui): edit team metadata as key-value pairs in team create and edit forms

The team create and edit forms asked for metadata as a raw JSON blob in a
textarea buried under Additional Settings. Both forms now render a key-value
pair editor directly under the TPM/RPM limit fields, backed by a shared
MetadataKeyValueFields component. Values round-trip losslessly: non-string
values display as JSON and parse back to their typed form on save, and
JSON-ambiguous strings are quoted so their type survives the trip. The edit
form hides UI-managed keys (logging, guardrails, model rate limits, etc.)
that dedicated controls already own and re-add on save.

* fix(ui): explain typed JSON parsing in the team metadata help text

* feat(team): schema-driven metadata fields from team_metadata_schema config

* refactor(team): render schema metadata fields as locked key-value rows, drop allowed_values

* refactor(team): schema fields reduce to key and label, tag-rendered keys, clean rejection toasts

* refactor(ui): prepopulate declared metadata keys as ordinary key-value rows

* fix(team): let non-admin dashboard users read the team metadata schema

* test(proxy): pin timeout wiring, boundary, and error-message contracts for team metadata validation

* fix(proxy): use pooled async httpx client in the e2e team metadata validator example

* refactor(team): satisfy staging lint ratchets inherited by the merge
* ci(circleci): install a pinned Rust toolchain on the Linux jobs

The cimg/python images have no Rust toolchain, so every Linux job that
runs `uv sync` or `uv build` builds litellm-rust through maturin with no
cargo on PATH. maturin's puccinialin helper then fetches rustup-init from
the unversioned /rustup/dist/ path with no checksum and provisions a
floating `stable` toolchain, so the compiler a job builds with drifts
with whatever upstream published that day. uv hides build-backend output
on a successful sync, so none of this shows up in the job log.

Add an install_rust command that mirrors the Windows job: download a
pinned rustup 1.28.2, verify its SHA-256 against rust-lang's published
sidecar, install toolchain 1.97.1 with the minimal profile, and export
~/.cargo/bin through BASH_ENV. Run it after install_uv in every job that
builds the workspace; upload-coverage only runs `uv tool run coverage`
and is left alone.

Net download cost is unchanged, since puccinialin was already pulling a
rustup and a toolchain in each of these jobs.

* test(ci): guard that no CircleCI job builds the workspace without a pinned Rust

A green CI run does not notice the gap this closes: uv suppresses
build-backend output on a successful sync, so a job that syncs with no
cargo on PATH silently gets maturin's own unpinned rustup and a floating
toolchain, and the log looks identical either way.

Pin the invariant statically instead. Every job and reusable command is
walked in step order, and reaching a `uv sync` / `uv build` without a
Rust toolchain provisioned first is a failure. install_rust and the
Windows job's inline pinned install both satisfy it, so a new job that
forgets one is named in the assertion message at PR time. Separate cases
cover install_rust's own pins: a versioned /rustup/archive/ URL, a
SHA-256 verified before the installer is executed, and an exact
toolchain version rather than a channel name.

* ci(circleci): provision Rust for base_sdk_install

base_sdk_install landed on staging while this branch was open. It runs
`uv build --wheel` on cimg/python:3.12 behind install_uv alone, so it
built the bridge with maturin's own unpinned rustup. The guardrail added
here caught it on the merge result, which is the case it exists for.
* fix(bedrock): stop forwarding no-op toolSpec.strict to Converse

`strict: false` is the Chat Completions default, so sending it to Bedrock
Converse communicates nothing the provider does not already assume, while
Bedrock rejects the key by presence rather than by value: any Claude model
routed through its Anthropic-compatible validator 400s with
`tools.0.custom.strict: Extra inputs are not permitted`.

The existing `bedrock_converse_supports_strict_tools` gate only protects
models whose `model_prices_and_context_window.json` entry carries the flag,
which makes every newly released Claude model broken by default until someone
adds it. That is a losing race for a field that carries no information when
false, and it is unrecoverable from the client side on `/v1/responses`, where
the Responses to Chat Completions bridge stamps `strict: false` onto every
function tool even when the caller never sent one. `drop_params` cannot help
there because the caller never supplied the param.

Drop the key when falsy instead. `strict: true` still honors the per-model
gate, so models that accept strict schemas keep the behavior they have today
and the flag keeps doing its job for the values that actually mean something.

* fix(bedrock): flag Claude Sonnet 5 as rejecting toolSpec.strict

Bedrock routes Sonnet 5 through the Anthropic-compatible validator that
rejects `toolSpec.strict`, but its six pricing-map entries never got
`bedrock_converse_supports_strict_tools: false`, so the gate fell back to
forwarding for Anthropic models and every tool call carrying `strict: true`
400'd. Verified live in us-east-1: before this, `strict: true` against
`us.anthropic.claude-sonnet-5` returns
`tools.0.custom.strict: Extra inputs are not permitted`; after, it returns a
real tool call.

Measured the rest of the family the same way rather than trusting the map:
Sonnet 4.5, Sonnet 4.6 and Haiku 4.5 all accept `strict: true`, and Opus 4.8
already carries the flag. Sonnet 5 was the only entry where the map disagreed
with the provider, so it is the only one changed here.

Same shape as the Opus 4.7/4.8 and Sonnet 4 fixes before it.
…pping it (#35705)

"Add keyword rule" seeds a row with no keywords, and the only check that a
rule carried one lived inside getSemanticConfigError, which returns early
when semantic keyword matching is off. Off is the default, so an unfilled
row fell through to serializeKeywordTierRules and was discarded on the way
to the payload; the create reported success and the rule was gone.

The row now reports the gap itself and the submit is withheld while one is
outstanding, on the create form and the edit modal alike, both reading
emptyKeywordTierRuleIndexes so the row named and the row marked cannot
differ. Enter commits a typed keyword: the dropdown is kept closed, which
left antd nothing for Enter to select, and submitting was what used to
supply the blur that saved the word.

The backend already refused such a rule, but only when the router built the
deployment, so a caller that sent one anyway got the row written, dropped on
reload, and a 500. The management write paths now parse the incoming
complexity_router_config with the router's own ComplexityRouterConfig, judged
on the config alone so a patch that writes one without naming a model is
covered too, and reject it with a 400 having persisted nothing.
… made them (#35734)

The block event Rubrik receives sourced caller identity from
model_call_details[metadata], where the enriched litellm metadata never
lives; it sits under litellm_params. Every block therefore reported
user_api_key_hash as an empty string, so a security block could not be
traced to a key, user, or team.

Read identity off the authenticated UserAPIKeyAuth the failure hook is
already handed, via the same mapper the success path and the proxy spend
logger use, so a block log and a success log describe their caller with an
identical key set.
…_responses_api

fix(responses): forward client headers to the provider on /v1/responses
…hboard (#35521)

* feat(spend): add net auto-router savings to the cost-optimization dashboard

The dashboard credited compression and prompt caching but said nothing about the
optimization that picks the model, so the driver with the largest lever on a bill
was the one an operator could not see.

Savings are the counterfactual: without a router a deployment runs one model, and
it has to be one that can carry the hardest request, so the baseline is the
priciest model in the router's hardest configured tier. A cheap tier is a choice
the router made, not a ceiling it was bounded by. `auto_router_savings_baseline_model`
overrides it for operators who would genuinely have run something else. Both are
provider-qualified before pricing, because a bare name can resolve to a different
vendor's rates or to nothing at all, and a deployment is priced by its `base_model`
where it has one, which is how Azure deployments are priced everywhere else.

Both arms price the request's real usage through `generic_cost_per_token` rather
than re-deriving per-token arithmetic, so tiered rates, ephemeral cache-write tiers
and regional uplifts stay consistent with what was actually billed. `prompt_tokens`
already includes the cache buckets, so charging them again at the input rate would
price the same tokens twice.

Cache state is what makes this hard. The baseline serves every turn, so whether it
had the prompt cached is whether the conversation was already underway. On a
continuing conversation it wrote the prompt earlier and would only read it now, so
this request's write is what switching cost and counts against the saving. On a
first turn nothing was cached for any model, the baseline would have written the
same prompt, and both arms carry the write at their own rates. Charging the write
to both cases understates a first turn to a few percent of its value, and because
the write premium is fixed by prompt size while the saving grows with completion
length, it can render a profitable route as a loss.

That shape is read off the conversation rather than remembered: a second human ask
means an earlier turn was served. No cache, no session id, and no dependence on a
caller sending a session header. It cannot see a switch on a turn the router did
not classify, and it reads a few-shot prompt's synthetic turns as prior
conversation; both err toward charging the write, which under-claims.

The baseline and the shape ride on the existing `routing_decision` record, which is
already carried from the router to the spend log, already classified for redaction,
and already written-or-cleared per attempt. A fallback that re-enters the hook
therefore cannot leave either fact behind to be attributed to a deployment that
never routed, and no new metadata key crosses the trust boundary.

The result is signed. Whether a switch pays off is a race between the rate gap and
the cache-write cost, and a narrow gap loses; flooring at zero would hide exactly
the routing behaviour an operator needs to see. The donut plots only drivers that
saved, while the card and range total keep the sign.

Savings accrue into a new `autorouter_savings_spend` column on the six daily rollup
tables, declared `NotRequired` because rows queued by a pod on the previous release
carry no such key. It is summed by the rollup merge the cross-pod Redis drain also
runs, and carried through the aggregation query, the per-row accumulation and the
response model, so the dashboard reads a value the API actually sends. Tests
enumerate the drivers from the response model itself and assert each is summed,
accumulated, carried and totalled, so one added later cannot be half-wired.

* fix(spend): let the baseline pay for a continuing turn's own growth

`_baseline_usage` moved every cache-creation token into the baseline's read bucket
whenever the conversation was underway. That is right for a switch, where the
baseline never left the model it was on and really would only read, but wrong for a
turn that stayed put: the prompt grew, and the tokens written are that growth. They
are new to every model, so the baseline would have paid to write them too. Forgiving
it that write made the counterfactual cheaper than it was and shrank the reported
saving on ordinary steady-state traffic, by about 2% per turn.

The selected arm was never involved; it has always been priced on the real usage.
The error sat entirely on the baseline.

The condition is that the request read more than it wrote, not that it read anything.
A switch onto a model already holding a small prefix of this prompt still writes most
of it, and that write is the switch's own cost; keying off a nonzero read would have
handed such a request the full rate gap, turning +$0.0056 into +$0.1177. Comparing
the two buckets separates a warm continuation, which reads far more than it writes,
from a cold arrival, which does the reverse, and it leaves the existing invariant
intact: a request reading 0 and one reading 1 both still land in the same place.

* fix(spend): price each arm under the key litellm billed it, and see agent turns

Two ways the savings number read the wrong thing, both from identifying a model by
its name when the name is not what it costs.

The counterfactual was ranked and priced on the public rate for the model a
deployment names. A deployment may not be charged that rate: the router registers
its configured prices under the deployment's own id and deliberately keeps them off
the shared model-name key so deployments sharing a backend model do not pollute each
other. So a hardest-tier deployment configured above its public rate lost the
ranking to a cheaper candidate, and once chosen was priced at a rate nobody pays.
Which key prices a deployment is now `_select_model_name_for_cost_calc`'s decision,
the resolver the real request is billed through, rather than a second rule here that
would have to re-learn that per-second and tiered overrides count, that a partial
override still counts, and that a deployment configured at zero is priced at zero
rather than treated as unpriced.

The arm being subtracted had the same fault and a sharper edge. It priced the spend
log's `model`, which on Azure is the deployment name, absent from the cost map, so
the whole driver silently read zero for that traffic. It no longer re-derives
anything: `model_map_information.model_map_key` is what litellm actually billed the
request under, recorded at request time by that same resolver with `base_model` and
custom pricing already applied.

Separately, the conversation-shape discriminator counted human asks, and an agent
loop can run twenty turns on one of them. Its tool traffic rides `tool_result`
blocks on user turns that flatten to empty text, and `tool` roles that are never
read, so a long agentic conversation looked like its own first turn and was handed
the arithmetic that leaves the cache write on both arms. That is the one direction
this must never fail in, because it inflates. An assistant turn is the direct
evidence that something answered earlier, and it is blind to how the tool plumbing
is spelled on either surface.

* fix(spend): give the cost-key resolver both inputs the selected arm needs

The served model was resolved through one input at a time, and each choice broke the
half the other fixed.

`model_map_key` is the served model already resolved through `base_model`, which is
the only way an Azure deployment name reaches the cost map at all; without it the
selected arm priced a name absent from the map, returned nothing, and the whole
driver silently read zero for that traffic. But it is built without
`router_model_id`, so it never carries a deployment's own price overrides, and a
custom-priced deployment was compared at its public rate while the baseline used the
real override. On a deployment configured well above its public rate that inverted
the answer outright: a route that lost $21.88 reported saving $0.10.

`_select_model_name_for_cost_calc` takes both, so it gets both. Which key prices a
deployment stays its decision rather than a rule restated here.

* fix(spend): same model is only the same cost when it is the same deployment

The short-circuit compared resolved model identity, so two deployments of one model
collapsed to "no switch" and reported zero. They are not the same cost: a deployment
can carry a negotiated rate, and routing from the dear one to the list-price one is a
real saving the dashboard reported as $0.00 against a true $21.93.

Both arms now carry the key litellm prices them under, so the comparison is between
deployments rather than between names.

* refactor(spend): price from resolved rates, not from a name we keep re-resolving

Four review rounds landed on one mechanism: which identifier prices a deployment.
base_model, then the deployment id, then cache-only overrides. Each round added a
clause to a resolution rule that should not exist, and a wrong primitive fails once
per input shape, so each shape arrived as its own finding.

`Router.get_deployment_model_info` already owns this. It merges a deployment's
configured prices over the built-in map, folds in `base_model` defaults for
deployments whose name is not a model, and falls back to the model name when nothing
is overridden. Every shape hand-rolled here (cache-only, partial, per-second, Azure)
was that function re-implemented badly.

`generic_cost_per_token` now accepts already-resolved rates instead of demanding a
name it looks up itself, which is what forced the name-bending in the first place.
Both arms resolve through the owner and pass what they got: the counterfactual by the
deployment the router would have used, the served request by the deployment that
served it. The invented cost-key resolver is gone, and `Baseline` carries a
deployment id rather than a key we chose on litellm's behalf.

Net 64 insertions against 79 deletions.

* test(spend): follow _most_expensive onto the router that prices its candidates

Ranking moved through `Router.get_deployment_model_info`, since what a deployment
costs is the router's answer to give; these four cases were still calling the old
free-function signature.

* fix(spend): rank baseline candidates by what a request costs, not by two rates

"Most expensive" was decided by comparing output rate then input rate. That is a
property of a rate, not of a request: a deployment dearer per output token can be
cheaper per cached token, so the comparison ordered cache-heavy traffic backwards and
recorded the wrong counterfactual.

Candidates are now costed on one reference request through the same engine the
savings themselves use, which leaves cache read and write rates, tiered tables and
every other billing dimension to that engine rather than to another rule restated
here. The reference request is cache-heavy because auto-routed traffic is.

* fix(spend): pick the baseline against the request that ran, not a stand-in for one

Ranking happened in the pre-routing hook, where the request has not executed yet, so
candidates were costed against a hard-coded reference workload: 20k prompt, 19k of it
cached, 1k out. Which candidate is dearest depends on that mix, so a pooled hardest
tier holding a deployment with non-proportional configured rates could be ranked for
a request nothing like the one served.

The mix is known on the spend path, so the ranking belongs there. The routing
decision now carries the tier's candidates rather than a winner already chosen, and
the baseline is resolved against the usage that actually happened. The reference
workload is gone; nothing here assumes a traffic shape any more.

The router is passed in rather than imported from `proxy_server` inside the
computation, so the savings stay a pure function of their arguments and the caller
owns where the router comes from. That also makes the spend path testable without a
running proxy, which the previous shape was not.

* refactor(spend): measure savings against one configured model, not a derived one

The counterfactual was derived per request: enumerate the hardest tier's
deployments, resolve each one's effective pricing, price them all, take the dearest.
That machinery produced a review finding per input shape it had not anticipated,
and every answer it gave was one an operator could have stated in a line of config.

So they state it. `litellm_settings.autorouter_savings_baseline_model` names the
model the traffic would have run on without a router, for every auto-router on the
proxy, and unset means the driver is off rather than a model nobody named being
guessed at. `savings_baseline.py` and its tests are deleted outright, along with the
tier enumeration, the candidate list on the routing decision, and the per-deployment
override that shadowed it.

Cache-state handling is untouched: the baseline is still priced on this request's own
read and write split, so a switch still pays for re-warming the cache and a first
turn still charges the write to both arms.

45 insertions against 482 deletions.

* refactor(router): compute the conversation shape once and pass it down

`_classify_and_route` re-derived it from the messages the hook had already resolved,
so an ordinary routed request walked the turn list twice for one boolean. The hook
computes it and hands it over, which is also where the affinity-hit path already got
it from.

Also moves `_get_llm_router` below the imports it sat among.

* fix(router): drop the dead conversation_continuing parameter off the hook

It was added to `async_pre_routing_hook` by mistake and immediately overwritten by
the value the hook computes, so it never did anything. It also widened a signature
every pre-routing strategy shares with the protocol in `types/router.py`, leaving
this one router diverged from `AutoRouter` and the interface for no reason.

Also records why an unreadable request counts as continuing: no messages is no
evidence a turn was served, so it pays the cache write and under-claims rather than
being handed a first turn's larger saving on nothing.

* fix(spend): charge a baseline its input rate for cache buckets it cannot price

A model with no cache_creation_input_token_cost, which is every OpenAI, Azure and Gemini entry, resolved that rate to 0.0 and carried the whole written prompt for free, so a first turn routed onto a cheaper model reported a loss. Same hole on cache reads. Those tokens are plain input on such a model, so they move into the text bucket.

* refactor(spend): build the daily upsert payloads in one shot

`common_data` and `update_data` were constructed and then appended to: `request_id`
conditionally for tag rows, `endpoint` unconditionally a few lines later. A dict that
grows after its literal cannot be reasoned about by reading the literal, which is the
whole point of building it at once.

The conditional key resolves to a spreadable value before either payload, so both are
single expressions and the tag branch appears once instead of twice.

Not wrapped in MappingProxyType, though it was suggested: these go straight to
prisma, whose query builder branches on `isinstance(value, dict)` to tell a nested
node from a scalar. A mappingproxy is a Mapping but not a dict, so it falls through
to the serializer and raises `TypeError: Type <class 'mappingproxy'> not
serializable` inside the batch upsert, where the surrounding except would log it and
leave the rollups silently unwritten.

* fix(spend): keep the one-shot upsert payloads under the type-discipline budget

Building both payloads as single literals traded a mutation for two dict literals,
and LIT002 counts construction rather than mutation, so the change the review asked
for is the one the gate charges for.

The empty branch is the avoidable half: it is the same value every time, so it moves
to a module constant built once instead of a literal per transaction, and it is a
read-only mapping so none of the call sites that spread it can fill it in later.
…3_2026

chore(typing): clear basedpyright Any errors in budget reset, access groups, and cache settings
…ng it again (#35736)

The auto-router savings driver recomputes what the served request cost, but that
request is not a counterfactual: it ran, and the cost calculator already billed it and
wrote the number down. Recomputing means restating every pricing dimension the biller
applied, and the two this missed were enough to halve it. A request billed at a
priority tier is recomputed at standard rates, and a regional host's uplift is dropped
entirely, so the driver writes a savings figure into the same rollup row as the `spend`
it disagrees with. On `gpt-5.4-mini` at priority the row is billed 0.024 and the driver
prices the same usage at 0.012.

Neither omission cancels between the two arms, because both are per-model. The uplift
is a multiplier read off each model's own entry, so 1.1*A - 1.1*B is 1.1*(A-B) and a
model without one does not move at all. Tier coverage is sparser and asymmetric:
`gpt-5.6` has priority rates and `gpt-5.4-nano` has none.

`cost_breakdown` already carries the answer and already reaches the call site. The cost
calculator records it, it rides the standard logging payload into the spend log's
metadata, and OTEL, the log drawer and the response headers all read it rather than
re-deriving; this driver was the only downstream consumer in the tree still pricing a
completed request from its tokens. `input_cost` and `output_cost` sum to exactly what
the pricer returns, so the served arm reads them. Tool spend, discount and margin stay
out, since the counterfactual cannot be priced with them and charging them to one arm
alone would read as the router losing money on every tool call.

The baseline never ran, so it is still priced through the cost engine, now on the basis
the biller used. `CostBreakdown` carries that basis because it cannot be recovered
afterwards: the tier the biller used comes from `optional_params`, which no log record
keeps, and the served tier that does survive on the usage object is a different fact
with the opposite precedence. Rows written before this shipped carry no basis and price
at standard rates, exactly as they do today; there is no backfill.

Two smaller things in the same path. The router is passed as a provider rather than a
router, so a spend write that was never auto-routed no longer fetches and discards one,
and the complexity router resolves its messages once per hook instead of once per
consumer.
…#35522)

Adds the auto-router as a third optimization driver beside compression and prompt
caching: a summary card, a donut segment, and a series in the savings graph across
both the cumulative and per-day views.

The number is signed, because a switch that thrashes the prompt cache can cost more
than the cheaper rates save and an operator needs to see that. The donut plots only
drivers that saved, since a negative slice has no meaning, while the card and the
range total keep the sign. `usd()` sizes and signs off the magnitude so a small loss
renders as -$0.01 rather than "$-0.00".

The card's popover states the counterfactual and its two consequences: that a switch
pays to re-warm the cache, and that a first turn the router could not identify is
charged that write and therefore under-reported.
@tin-berri
tin-berri force-pushed the litellm_lit4712_autorouter_session_rollup branch from 8f7b24a to 7d73c60 Compare August 4, 2026 04:16
The benchmarks dashboard answered every question by scanning LiteLLM_SpendLogs at
read time: four aggregate queries per auto-router, two of them window functions
over the response JSONB, re-deriving on every page load which model the previous
turn used, how long a tier had been idle, and how big the prefix was last time.
Those are sequential facts and the request that produces them already knows all
of them, so they are now computed once, when the turn happens.

A new LiteLLM_AutoRouterSession row per (session, auto-router) carries both the
counters and the state that classifies the next turn. fold_turn is pure, so every
rate and dollar formula is unit-testable without a database, and the in-memory
queue plus background flusher follow AdaptiveRouterUpdateQueue: atomic increment
upserts, so two pods writing one session compose instead of overwriting. A pod
that has never seen a session loads its row once and classifies from memory
after, which is what keeps a session correct across a restart or a pod move.

The counters are declared once, on TurnDelta. COUNTER_FIELDS derives from that
declaration and the merge, the flush payload and the read query all build off it,
so a metric added there reaches the database and the dashboard without a second
edit. A test asserts the read query aggregates every declared counter; it caught
two that were being written on every request and read by nothing.

The read path is a single aggregate over pre-folded rows covering every
auto-router at once, and touches no per-request table at all. Rollup rows expire
on the existing spend-log retention cutoff, keyed on last activity so a live
conversation is not pruned out from under itself.

Two behaviour fixes came with the move. The turn buckets are now exhaustive: a
session's opening turn used to land in the headline turn count and in none of the
three buckets, so the bucket totals silently disagreed with the headline. And a
turn with no ephemeral cache-creation evidence now reads as the five minute tier
rather than the one hour tier, which had been the default purely because zero is
not less than zero.

Savings come from compute_savings_spend, the same primitive the usage tab uses,
so the two surfaces cannot report different numbers for the same traffic. The
baseline recorded on each row is the one that priced its turns, so the tab names
what the numbers were computed against rather than whatever the config says by
the time someone opens it.
@tin-berri
tin-berri force-pushed the litellm_lit4712_autorouter_session_rollup branch from 7d73c60 to d0c24f2 Compare August 4, 2026 04:30
@tin-berri

Copy link
Copy Markdown
Contributor Author

Superseded by a replacement PR. #35402 was closed in favour of #35522, which landed a different baseline mechanism, so this needed rebasing directly onto litellm_internal_staging; GitHub would not allow the base change on a stacked PR. The two P1s raised here are fixed in the replacement.

@CLAassistant

CLAassistant commented Aug 4, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
7 out of 9 committers have signed the CLA.

✅ milan-berri
✅ mateo-berri
✅ yuneng-berri
✅ tin-berri
✅ yucheng-berri
✅ Classic298
✅ ryan-crabbe-berri
❌ devin-ai-integration[bot]
❌ yassin-berriai
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants