* fix(proxy): make /team/member_delete's four cleanups atomic
The team roster update, the user.teams update, the team membership
delete, and the team-scoped verification token delete ran as four
sequential writes with no transaction around them, so a failure
between any two left the removal half applied. Thread a single
prisma transaction through all four writes, following the same
tx.<table> pattern /team/member_add and /team/member_update already
use, so either all four land or none do.
* fix(team): serialize member_add, member_delete, and delete under the team's advisory lock
/team/member_add validated a team exists and then wrote the user's teams array and
a membership row without holding anything across that gap, so a /team/delete could
commit its reference sweeps in between and leave a member pointing at a team id that
no longer exists. The write path already re-read members_with_roles under a row lock
before this change, but SELECT ... FOR UPDATE can deadlock with the access-group
endpoints, which lock an access group and then a team.
member_add now takes pg_advisory_xact_lock(hashtext(team_id)) before re-reading the
team and only writes if it is still there, so a delete that already committed is
visible before any write happens. delete_team takes the same lock around its own
row delete and reference sweep, so the two requests can never interleave: whichever
acquires the lock first runs to completion before the other's read can proceed.
Dropping the row lock from member_add's read also dropped the incidental protection
it gave against a concurrent member_delete, which still wrote from the snapshot it
validated against, unlocked, and could silently overwrite whatever member_add had
just committed. member_delete now takes the same advisory lock and re-reads the
roster under it before computing its own write, so it can never resurrect a member
by overwriting from stale data.
Resolves LIT-5544
* fix(team): run member writes on the advisory lock's transaction
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(team): keep member writes on the lock holder's connection after merge
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(team): keep the transactional member create an upsert on user_id
The transaction path was creating the email-identified user row outright, where the
regular client path upserts on user_id. Share one upsert helper between both member
paths so the create stays idempotent on the lock holder's connection.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(team): read member_delete's user and key rows on the lock-holding transaction
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>
Every repository handed its `.table` back untyped, so a dozen modules had
each grown a private `_PrismaTableActions` Protocol to paper over it. They
had drifted: some declared `update` as returning the row, others the row or
None, and none agreed on whether `find_many` was covariant
Replace all of them with a single `TableActions[RowT_co]` in
`litellm/repositories/prisma_protocols.py`, keyed to the prisma row each
repository is bound to. Query inputs stay `Mapping[str, object]` so callers
keep passing plain dicts, and `find_many` returns `Sequence` so the row type
stays covariant
Typing the nullable returns honestly surfaced paths that were already
crashing. A team admin could never edit or delete a memory entry owned by
their team: the write-auth check fed a raw prisma row to a helper that
expects the domain model, so `members_with_roles` arrived as plain dicts and
the request died as a 500 instead of applying the edit. Non-admin members hit
the same 500 in place of the 403 they were owed, so refusal and breakage were
indistinguishable. `/v2/model/info?user_models_only=true` dereferenced a
missing user row rather than returning the 400 the route already had, three
team routes dereferenced a team deleted between the read and the write, and
the agent registry dereferenced a missing agent instead of naming it
basedpyright drops 2,132 errors, 1,454 of them reportAny and 73
reportExplicitAny. The dashboard's generated types pick up `string[]` where
they had `unknown[]` for a team's members, admins and models
When get_team_object fails, the centralized auth gate rebuilds the team
from the token's own fields. A token whose team row was missing when the
key was read carries team_models=[] and team_blocked=False, and the
model-access check reads an empty model list as every model, so the
rebuilt team grants more than the real team ever did.
get_team_object reported a deleted team and a database that would not
answer as the same 404, so the fallback could not tell a definitive
answer from a degraded read. Raise a TeamNotFoundError subclass, still a
404 with the same detail so every other caller is unaffected, only when
the database answers and the row is absent.
A team that is provably gone now refuses, and no setting overrides that.
Otherwise the grant is merely unknown: a token carrying one may vouch,
since replaying a recorded grant cannot widen it, and a token carrying
none may not. allow_requests_on_db_unavailable still opts back out there,
and is only consulted once the failure is known to be a degraded read.
The Admin UI mints every session key against the UI_TEAM_ID sentinel,
which by design never has a team row, so every UI request hit the new
refusal with no override. Exempt UI_TEAM_ID explicitly so it keeps
reconstructing from the token unconditionally, matching how the MCP
handler and agent_permission_handler already special-case it.
Resolves LIT-5522
* test: drop the cwd-relative sys.path.insert calls from the test suite
TQ003 stands at 1,077 across 1,058 files, and 1,015 of them are the same shape:
sys.path.insert(0, os.path.abspath("../..")) and its deeper siblings. The
argument resolves against the working directory rather than the file, so from
the repo root, where every job runs pytest, it inserts the directory two levels
above the checkout. It has never pointed at litellm. The package is installed
into the environment anyway, which is what actually makes the import work, and
what the rule's message has said all along.
Removing them leaves 1,634 imports of sys and os with no remaining reference,
and those go too, except where another test module imports the name back out of
the file. The rest of TQ003 is 62 call sites that resolve against __file__ or a
variable, which are a different question and are left alone.
Collection is identical either way: 45,871 tests and the same 51 pre-existing
collection errors before and after, and ruff reports no new undefined name.
* test: drop the duplicate imports the sys.path sweep exposed to F811
* test(pre-call-utils): restore the os import the new bedrock tests need
Four more ruff rules for code the test suite runs but never checks. F601 is the
one that paid: the duplicate key it flagged in a get_form_data fixture was the
mock reproducing the production bug fixed in the previous commit.
B025 removed two unreachable handlers, one of them a pytest.skip shadowed by an
earlier `pass`, so an upstream Vertex flake reported green having asserted
nothing. F632 turned an `is ""` identity check, which passes only on CPython
interning, into the `== ""` it meant. B023 fixed three closures over loop
variables, all latent today but one iteration-order change away from checking the
last case N times.
A test that asserts on the error inside its own except block passes when the
call stops raising, because nothing runs the handler. That is the exact case
the test exists to catch, so the regression lands green.
Rewrites all 111 such blocks into pytest.raises, which fails when the call
succeeds, and selects PT017 in ruff-tests.toml so no new one lands.
A name bound twice keeps only the second binding. In `tests/` that is nearly
always a repeated import, harmless but misleading, and the same rule is what
catches the cases that are not harmless: a local that shadows an import the
module still calls, and a second `def test_x` that quietly replaces the first.
311 of the 344 sites were repeated imports and came out with ruff's own fix.
The remaining 33 needed a decision. Four modules imported a name they never
used because a local definition below already shadowed it. Two comprehensions
bound `call` over `unittest.mock.call`, which those modules import and use.
One test rebound the two module handles its nested reload closure had captured.
One class attribute shadowed an unused `status` import.
The load-test fixtures move to a conftest, which is how pytest is meant to share
them, so the test module no longer imports three fixture names it never calls.
The nine `prisma_client` parameters keep a narrow `noqa`: pytest resolves that
fixture by name before the body runs, so the parameter never shadows anything.
The Prisma query engine is a separate process whose resident memory grows with
what it is asked to hold and glibc never returns it, so a pod's memory floor
ratchets up to its worst statement and stays there for the life of the worker.
#34956 bounded a spend-log flush by payload bytes, which caps that floor when
prompts are stored and does nothing when they are not: rows carrying only
attribution metadata run about 1.2 KB, so a 1000-row statement is roughly
1.2 MB, the 2 MB byte budget never binds, and every statement stays at 1000
rows forever.
The engine charges per row as well as per byte. Measured on a container running
the same engine build (5.4.2) against real Postgres, with rows shaped like a
store_prompts_in_spend_logs=false deployment, writing the same 200,000 rows:
rows/statement engine RSS still resident after the flush
1000 179 MB
500 91 MB
250 41 MB
100 19 MB
None of those statements came near the byte budget, so the whole difference is
row count. The floor is a plateau rather than a leak: 1,000,000 rows written at
1000 per statement settles around 229 MB and stops climbing.
Adds SPEND_LOG_WRITE_BATCH_MAX_ROWS, default 100, applied alongside the
existing byte budget so whichever binds first splits the statement. Both are
needed, since bytes are what track a prompt-carrying row and rows are what
track the engine's per-row bookkeeping.
One consequence worth naming: a flush now issues more statements, and a
statement that fails under a poison flood costs one insert before any
isolation runs, so the irreducible floor rises by the statement count. The
isolation budget still caps the amplification on top of that, and the tests
assert the bound derived from the configured row cap rather than a constant.
Per-model budgets were three separate things pretending to be one. The
enforcement check, the post-call increment and the info endpoints each derived
their own cache key, so a budget could refuse traffic at 429 while /key/info
reported zero usage, and a Bedrock model id never matched a budget keyed on the
bare family name. /user/new echoed a model_max_budget back and stored an empty
dict, and nothing enforced a user-scoped per-model budget at all.
One owner now builds the counter key from the configured budget model, and
enforcement, the increment and the info endpoints all read it. Bedrock ids
resolve through the model-cost map. Auth carries the user's budget onto the
token on every branch that reaches the spend hook, including JWT and
auto-registration. Native passthrough attaches the three budget metadata keys
its StandardLoggingUserAPIKeyMetadata does not carry, so /anthropic/... and
/bedrock/... traffic is counted and capped like /v1/chat/completions.
The dashboard gains the per-model budget editor it never had, on the key create,
key edit and internal-user edit forms. It is read-only without an enterprise
license, matching the write gate the proxy already enforces, and an untouched
budget is left out of an update so an unrelated edit cannot trip that gate.
The editor hydrates from either BudgetConfig spelling, since model_max_budget is
a plain dict that the proxy stores exactly as the client sent it, and it carries
through the fields it does not model. Without both, editing one model would drop
another model row entirely and silently discard its tpm_limit and rpm_limit.
/user/info refreshes its local copy of the user field by field after a save, so
model_max_budget joins that list. Left out, a saved cap read back as the old one
when the form was reopened, and clearing the row to recover would then wipe the
value that had actually persisted.
A zero-dollar cap is the strictest limit expressible, not the absence of one,
so it is enforced rather than skipped on falsiness, spend exactly at the cap is
refused the way every sibling budget check already refuses it, and a counter
that was never written reads as zero spend rather than as unknown. The usage
endpoints read every counter in one batched lookup, so a large model_max_budget
cannot fan out into one concurrent cache call per configured model.
Every auth path honours the same zero-cost skip flag, so none of them can refuse
a free request that another serves. The custom-auth helper gains the flag it
never had, which also changes its pre-existing key and end-user checks.
The compaction summary gate checks the user scope alongside the key and end-user
ones. This file propagates all three budgets into the summary subrequest, so
enforcing only two let compaction increment a counter it could not be refused by.
Custom auth attaches the user's budget to the token unconditionally, since the
post-call spend hook reads it there: gating the attach on the same condition as
enforcement left the counter uncharged whenever the request was not itself
enforceable. An entry that will not validate is skipped rather than raised on,
so one malformed scope cannot abort every other scope's increment or turn a
config typo into a 500.
The edit forms re-seed the budget editor when a different key or user is loaded.
Its rows are seeded once and cannot re-read their own value prop, so without
this a save wrote the previously loaded record's budgets onto the current one.
Only the built-in provider pass-through routes carry the budget metadata.
get_model_from_request deliberately resolves no model for a user-defined
pass-through, since its body is forwarded verbatim and names an upstream model,
so attaching there would charge a counter nothing on that route can refuse.
`pytest.raises(Exception)` with no `match=` passes on any error that broad. A
TypeError from a refactor, a botched fixture, an import that moved: all of them
read as the rejection the test claims to police, so the test goes green for the
wrong reason and stays green after the behaviour it guards is gone.
PT011 closes that gap for the 317 sites B017 could not reach, because B017 only
fires on a single-statement body with no `as e` binding. Each pattern here is the
message the code actually raised, recorded by running the sites under a plugin
that logged the concrete type and text per call site, so the assertions describe
observed behaviour rather than a guess. Where a site raises more than one message
across its parametrize cases, the pattern is an alternation of what was seen;
where the exception carries an empty `str()` and puts the text on `.message`, the
site keeps a narrow `noqa` with the reason.
PT014 removes four parametrize cases that were listed twice. The duplicate re-runs
an assertion that already passed, and it usually marks a case someone meant to
vary and forgot to edit.
* test: enforce PT012 so a pytest.raises block cannot hide dead assertions
`with pytest.raises(...)` stops at the first statement that raises. Anything
sequenced after it inside the block never runs, so an assertion written there is
never checked and the test still reports green.
Two sites were doing exactly that, and both assertions turned out to be wrong
once they started running. tests/llm_translation/test_prompt_factory.py asserted
the bedrock rejection names "requires at least one non-system message", which
holds. tests/proxy_unit_tests/test_proxy_server.py asserted the prisma startup
failure mentions "httpx.ConnectError", which never appears: the failure is an
httpx.ConnectError whose message is "All connection attempts failed", so that
test now asserts the type. Its DATABASE_URL override moves to monkeypatch, since
the old restore sat below the assertion and leaked the invalid URL into every
later DB test the moment the assertion started being able to fail.
The remaining 72 sites are rewritten without changing what they exercise: setup
that cannot raise moves above the block, a nested `patch` moves outside it, and
bodies with real control flow (a stream drain, an if/else on sync_mode, a
retry loop) move into a local closure the block calls.
Fixing PT012 unmasked two B017s, since ruff only reports a blind
pytest.raises(Exception) once the block holds a single statement.
tests/proxy_unit_tests/test_auth_checks.py narrows to the ProxyException
can_key_call_model actually raises. tests/local_testing/test_completion_cost.py
was asserting vertex_ai/medlm-medium has no cost entry, which stopped being true
at some point; that dead first half is gone and the rest of the test, which
checks medlm pricing resolves above zero, now runs instead of being skipped.
* chore(ci): ratchet TQ004 to 768 after the prisma test moved to monkeypatch
* test(lint): ban blind pytest.raises(Exception) with ruff B017
A bare pytest.raises(Exception) accepts whatever the body throws. The TypeError
a refactor introduces satisfies it exactly as well as the rejection the test was
written for, so the crash reads as a pass and the test never goes red.
All 111 existing sites are narrowed here. A runtime probe recorded the concrete
exception each one actually catches, and each site now names that type. Where
the code under test genuinely raises a bare Exception, the site pins a stable
slice of the message with match= instead.
Two sites tell on themselves. The shared responses-API cancel test raises
"custom_llm_provider is required but passed as None" rather than talking to a
provider at all, because cancel_responses takes a provider, not a model. And
test_bedrock_guardrails_with_streaming was the only test in its file still
passing without AWS credentials, because the NoCredentialsError boto3 raised
long before the guardrail ran satisfied the blind raises.
* fix(test): widen the openai batch-dispatch assertion to OpenAIError
The narrowed NotFoundError only holds where OPENAI_API_KEY is set. Without one
the SDK raises OpenAIError while building the client, long before any 404, so CI
went red. OpenAIError covers both and still rejects a TypeError from a refactor.
Every pod and uvicorn worker schedules its own CheckBatchCost poller against the
shared managed-object table, so two of them can select the same completed batch in
one polling window and both write an aretrieve_batch spend log for it, counting
that batch's cost twice.
Claim the row with a compare-and-swap on batch_processed, and skip the batch when
another pod already holds it. The claim sits immediately before the spend log is
written rather than before the results fetch, because batch_processed is also what
blocks deletion of the files the fetch reads and what keeps an unbilled row
selectable by later poll cycles, so claiming up front would strand the spend of any
worker that died mid-fetch. A failed spend log write hands the row back.
Co-authored-by: Yassin Kortam <yassin@berri.ai>
ruff.toml excludes tests/* from `ruff check`, so nothing has ever checked the
test tree for names that do not exist. That matters more in tests than in
product code: a NameError inside a test whose body is wrapped in
`except Exception: pass` is swallowed, and the test reports green forever.
Adds ruff-tests.toml selecting F821 alone, wired into the lint workflow and
`make lint-ruff`, and clears every existing violation:
- 4 tests interpolated an unbound `e` into a `pytest.fail` message reached only
on the failure path, so the NameError, not the assertion, is what ran.
test_llm_guard_error_raising is the worst: it passes today with content
safety disabled entirely. It now asserts the 400 and its detail body.
- 5 sites construct BaseExceptionGroup, a 3.11 builtin, in a tree that still
supports 3.10. Guarded behind the exceptiongroup backport that anyio already
pulls in below 3.11.
- 9 missing imports (json, openai, Any, Final, HTTPException), including one in
a helper that catches HTTPException by a name it never imported, so the
challenge path it exists to detect raises NameError instead.
- 5 annotations naming types imported inside the function body, hoisted to
module scope or TYPE_CHECKING.
- 2 blocks of dead code: everything after a pytest.fail in
test_claude_agent_sdk, and an unused helper in test_end_users calling a
function defined in a different module.
- 1 error-path f-string in the router-settings doc test that masked the real
FileNotFoundError behind a NameError.
Only F821 for now. Widening the select list means ratcheting thousands of
pre-existing findings, so rules go in one at a time with their violations
already fixed.
Python binds a name once per scope, so when a module or class defines the same
test twice only the last one exists. The earlier definitions are unreachable:
pytest never collects them, and nothing that references them can fail.
A sweep in August cleared nine of these. Five have appeared since, which is the
argument for a rule rather than another sweep.
Each survivor is the better version, so nothing is lost. The two SQS logger
twins additionally stub `asyncio.create_task`, which the shadowed copies did
not. The cost-calculator duplicate is a two-line stub that also takes a
`model_item` parameter no fixture supplies, so it could not have run even
unshadowed. The two `test_prompt_caching` bodies are both `pass`.
Collecting the four files reports 416 tests before and after.
`tests/proxy_unit_tests/conftest copy.py` goes with them. pytest only loads a
file named exactly `conftest.py`, nothing imports this one, and the space in the
name says what it was.
Probe the column before the scheduler registers CheckBatchCost, closing the window where a retrieve that decided the poller was inactive billed a batch the first poll cycle then billed again. Also drop narration docstrings and section banners from the new tests.
* fix(proxy): requeue spend logs when the DB write fails with a transport error
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(proxy): hardcode the spend log queue cap and drop the stale re-export
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* refactor(proxy): keep the spend log requeue within the type discipline budget
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(proxy): apply the spend log queue cap to producer appends too
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(proxy): lower the spend log queue cap to 1k and make it env configurable
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(proxy): bound the spend log queue by bytes instead of row count
A row cap cannot bound memory: a row carries the whole prompt under store_prompts_in_spend_logs, so a cap that rides out an outage of counter-only rows is an OOM once prompts are stored. Every enqueue and dequeue now goes through one pair that tracks what the queue costs and drops the oldest rows past a 64 MB budget.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(proxy): make the spend log queue byte budget env configurable
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(proxy): use a string default for the spend log queue byte budget env read
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(proxy): make the spend log queue byte total a public attribute
The queue it accounts for is already public, and a private name only bought reportPrivateUsage errors at every call site.
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>
Co-authored-by: shivam <shivam@berri.ai>
This reverts commit ab2333b6c4.
Every Admin UI login mints its session key against the sentinel team_id
`litellm-dashboard`, and no LiteLLM_TeamTable row is ever created for it.
That lookup is therefore a provably-absent row on every UI request, which
#36837 turned into a hard refusal with no override, so the whole dashboard
404s.
Reverting restores the token-derived fallback. The model-access widening
#36837 closed is reopened and needs a re-land that exempts the UI sentinel
team.
The handoff asked whether the poller was running, when what matters is whether it
will actually account for the batch. Those differ on a schema without the
batch_processed column: the poller cannot filter on it, so it falls back to a
query that excludes complete and completed rows, and it cannot set it either. A
caller retrieving a provider-completed batch before the poller saw it therefore
suppressed inline accounting, then marked the row complete, and the fallback query
could never find it again. Nobody accounted for that batch, so its cost escaped
the caller's budget entirely.
The poller now publishes batch_processed_support_confirmed, set only once a
filtered query has actually succeeded, and the handoff requires it. Defaulting to
unconfirmed keeps accounting on the retrieve path in exactly the cases the poller
would drop the batch, including the window before the poller's first cycle. All
four combinations account exactly once: unconfirmed leaves the retrieve
accounting and setting the marker, whether or not the column exists, and
confirmed is only reachable when the column is present, where the poller accounts
and sets it.
A scheduler that hands back something other than a bound method leaves no poller
to interrogate, which reads as unconfirmed rather than as working.
get_configured_s3_bucket_name accepts the output bucket only from the immutable
_litellm_internal_model_credentials snapshot or AWS_S3_BUCKET_NAME. That refusal to read
litellm_params is deliberate: the bucket is what validate_managed_cloud_file_id checks a
file id against, so trusting a request-supplied value would let a caller redirect reads
to a bucket of their choosing
Two live entry points reach the Bedrock file-content transformation without ever building
that snapshot. The managed-files pre-call hook sets data["model"] for any id carrying
llm_output_file_id, which is every batch output, so get_file_content always takes the
model-routed branch; that branch called llm_router.afile_content directly, and
managed_files_obj.afile_content, the only caller that built the snapshot, is therefore
unreachable for batch output. CheckBatchCost spread the deployment credentials as plain
kwargs, and get_litellm_params does not carry s3_bucket_name across (gcs_bucket_name is
listed for exactly this reason, its S3 counterpart is not), so the poller lost the bucket
the same way
The result was that every completed Bedrock managed batch failed files.content with
"S3 bucket_name is required" and never had its cost tracked, leaving the row to be
re-polled every cycle. Both paths now resolve the deployment credentials and pass the
same MappingProxyType snapshot the managed-files hook already builds
A managed batch whose request lines all failed can reach a terminal provider
status (completed) with output_file_id=None and only an error_file_id. Such a
row matched neither the completed-with-output billing branch nor the
failed/expired/cancelled branch, so batch_processed stayed False and the poller
re-selected it on every cycle for the lifetime of the deployment; output/error
file deletion is also gated on batch_processed, so those files could never be
deleted.
Broaden the terminal handling so a completed/complete/expired batch with an
output file is billed, and any terminal batch with nothing to bill
(failed/cancelled, or completed/expired with no output) is marked terminal
exactly once. Non-terminal statuses (validating/in_progress) are still left for
the next poll, and an expired batch that did produce output is now billed.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
When get_team_object fails, the centralized auth gate rebuilds the team
from the token's own fields. A token whose team row was missing when the
key was read carries team_models=[] and team_blocked=False, and the
model-access check reads an empty model list as every model, so the
rebuilt team grants more than the real team ever did.
get_team_object reported a deleted team and a database that would not
answer as the same 404, so the fallback could not tell a definitive
answer from a degraded read. Raise a TeamNotFoundError subclass, still a
404 with the same detail so every other caller is unaffected, only when
the database answers and the row is absent.
A team that is provably gone now refuses, and no setting overrides that.
Otherwise the grant is merely unknown: a token carrying one may vouch,
since replaying a recorded grant cannot widen it, and a token carrying
none may not. allow_requests_on_db_unavailable still opts back out there,
and is only consulted once the failure is known to be a degraded read.
Three groups, all verified by running the suite rather than by inspection.
18 files whose every test function carries an unconditional @pytest.mark.skip,
39 test functions in total. They are collected on every CI run and always skip,
so they advertise coverage the suite does not have. Reasons on the marks include
"AWS Suspended Account", "lakera deprecated their v1 endpoint" and "moved to
using 'otel' for logging"; 26 of the marks predate 2025.
30 test functions with a byte-identical body and identical decorators to a
sibling in the same file and class, differing only in name. Deleting one of each
pair removes no coverage. Four further candidates were excluded because they
override an inherited test, where deleting the override un-shadows the base
class implementation instead of removing a duplicate.
9 test functions that a later definition of the same name shadows, so Python
never binds them and pytest cannot collect them.
One file that is a demo script rather than a test; its own docstring says to run
it with python.
Verification: collecting the 26 edited files gives 2,492 node IDs before and
2,462 after. The 30 duplicate deletions account for exactly 30 removals, the 9
shadowed deletions account for 0 (confirming at runtime that they were never
collectable), nothing unexplained disappeared, and nothing new appeared. No
other test or module imports any deleted symbol.
The daily spend flush emitted one INSERT ... ON CONFLICT per aggregated key,
so every replica put hundreds of statements on the database each interval,
all contending for the same handful of hot rows and each holding its row
locks for the rest of the enclosing batch transaction. LiteLLM_DailyTagSpend
felt it worst because a request writes one row per tag, and litellm adds two
user-agent tags of its own by default.
A batch now goes out as a single multi-row statement. Rows are folded by the
conflict tuple first, and every nullable member of that tuple is normalized
to '': a NULL can never match itself in a unique index, so such a row was
re-inserted on every flush rather than aggregating, and a NULL model made
prisma reject the whole batch.
* feat(router): add per-deployment allowed_fails_policy and cooldown_time override support
Three bugs fixed in the router cooldown system: (1) deployment-level allowed_fails and
allowed_fails_policy in model_info now take precedence over router-level settings in
_should_cooldown_deployment; (2) failed fallback deployments now get evaluated for
cooldown via _trigger_cooldown_for_failed_deployment, bypassing the Logging dedup gate;
(3) DualCache promotes Redis cooldown entries using default 600s TTL instead of true
remaining cooldown time -- _corrected_active_cooldown now evicts expired entries and
corrects stale in-memory TTLs on backfill. Adds ServiceUnavailableError, BadGatewayError,
and NotFoundError fields to AllowedFailsPolicy and cooldown_time to LiteLLMParamsTypedDict.
* fix(router): gate fallback cooldown trigger on has_logged_async_failure; use only litellm_metadata for deployment ID
* fix(router): use X | Y union syntax to fix UP007 strict lint gate
* test(router_utils): add coverage for _trigger_cooldown_for_failed_deployment and has_logged_async_failure gate
* test(router_utils): cover deployment cooldown override and exception swallow paths
* fix(router): add InternalServerError/ServiceUnavailableError/BadGatewayError/NotFoundError to router-level get_allowed_fails_from_policy
* fix(router): format router.py and add router-level policy tests
* test(router): add CI-visible coverage for per-deployment cooldown policy
Tests for `_get_deployment_cooldown_policy`, `_resolve_allowed_fails_from_policy`,
and `_should_cooldown_based_on_deployment_policy` (cooldown_handlers.py), the
`_corrected_active_cooldown` branches in CooldownCache, and the four new
exception-type branches in `Router.get_allowed_fails_from_policy` (router.py) --
all in `tests/test_litellm/` which the enterprise-routing CI job runs.
* fix(router): use is not None guard for cooldown_time_override in should_cooldown_based_on_allowed_fails_policy
A cooldown_time_override of 0 was previously treated as falsy and silently
fell through to the router-level cooldown_time value. Switched to an explicit
is not None check so that zero is honored as a valid override.
Added a regression test covering the zero case.
* fix(router): honor has_logged_async_failure and metadata for fallback cooldown; support both model_info and litellm_params locations
Manual verification against a live proxy surfaced that the fallback-cooldown-gap
trigger never actually fired: the has_logged_async_failure check read a plain
attribute that Logging never sets (the real flag lives in model_call_details),
and the deployment_id lookup only trusted litellm_metadata, which regular chat
completions never populate (only batch/thread/file endpoints do). Router
overwrites model_info on whichever key is present before every attempt, so
metadata is equally authoritative there, not caller-controlled as previously
assumed. Also let allowed_fails/allowed_fails_policy/cooldown_time be set under
either model_info or litellm_params, each preferring its own canonical location.
* fix(router): fix ContentPolicyViolationError policy shadowing and partial-policy zero-threshold
Two bugs from Greptile review on PR #34416:
- ContentPolicyViolationError subclasses BadRequestError, so listing
BadRequestError first in _EXCEPTION_POLICY_FIELDS made the isinstance
check always match BadRequestError for content-policy errors, using the
wrong allowed_fails threshold. Reordered so the subclass is checked first.
- A deployment with a partial allowed_fails_policy and no deployment-wide
allowed_fails forced allowed_fails_override=0 for any exception type its
policy didn't cover, cooling the deployment down on the first unrelated
failure. Now defers to router-level behavior for uncovered exception
types instead of forcing an immediate cooldown.
* fix(router): only trust a metadata/litellm_metadata bucket the router itself wrote deployment info into
veria-ai flagged that preferring litellm_metadata whenever present could pick up a
caller-supplied litellm_metadata.model_info.id (preserved via allow_client_pricing_override)
instead of the metadata bucket the router actually populated for a regular completion's
fallback attempt, naming an arbitrary "victim" deployment for cooldown.
Router._update_kwargs_with_deployment() always writes model_info and
deployment_model_name into the same bucket together. Only trust a bucket that
carries deployment_model_name alongside model_info, since that marker is only
ever set by the router itself, not by request-body metadata.
* test(router): add regression coverage for ContentPolicyViolationError policy shadowing
The subclass-ordering fix in commit 38fe4e4490 had no regression test.
Verified the new test fails on the pre-fix ordering (asserts 2, got 10)
before restoring the fix, and confirmed the same behavior through the full
_should_cooldown_deployment call path against a real Router instance.
* fix(router): let explicit allowed_fails_policy entries override the generic 4XX cooldown exclusion
_is_cooldown_required skips cooldown evaluation for any 4XX status outside
{429, 401, 408, 404} by default, since a generic client error is usually not
the deployment's fault. BadRequestError and ContentPolicyViolationError both
carry status 400, so their AllowedFailsPolicy fields (BadRequestErrorAllowedFails,
ContentPolicyViolationErrorAllowedFails, both router-level pre-existing and the
new deployment-level ones) were silently unreachable: an operator could set
them to any value with no effect, since _is_cooldown_required blocked cooldown
evaluation before that policy was ever consulted.
_should_run_cooldown_logic now also checks whether an explicit allowed_fails_policy
entry (deployment-level or router-level) covers the exception's type, and if so,
proceeds with cooldown evaluation regardless of the generic status-code exclusion.
The exclusion remains the default for exception types with no explicit policy.
Verified live against a mock-triggered ContentPolicyViolationError (config-level
mock_response, azure/gpt-4.1-mini deployment) with BadRequestErrorAllowedFails=100
and ContentPolicyViolationErrorAllowedFails=0 on the same deployment: it now cools
down after exactly one ContentPolicyViolationError instead of never cooling down.
* fix(router): use the router-stamped failed_deployment_id for fallback cooldown targeting
Greptile flagged a real gap in the metadata-bucket-based deployment lookup:
for a generic-API-call fallback, the router writes the current attempt into
litellm_metadata, but a stale "metadata" bucket carrying the same
deployment_model_name marker (from an earlier point) would be picked first,
cooling the wrong deployment.
Router already has a more robust, pre-existing mechanism for this exact
problem: _set_failed_deployment_id_on_exception stamps the failing
deployment's id directly onto the exception at the point of failure,
immune to metadata-bucket ambiguity since a caller can't influence it and
it doesn't depend on which bucket the current call type happens to use.
It just wasn't called from _ageneric_api_call_with_fallbacks_helper's
except block, unlike _completion/_acompletion.
Added the missing call there (matching the existing pattern exactly), and
changed _trigger_cooldown_for_failed_deployment to prefer
exception.failed_deployment_id when present, falling back to metadata-bucket
inspection only for call paths that don't stamp it yet.
Verified live: the standard fallback-cooldown-gap scenario (two bad-key
deployments in a fallback chain) still correctly cools down both the
originally-called and fallback deployment.
* fix(router): address human review on per-deployment cooldown overrides
Scope allowed_fails_policy override to deployment-level only (a router-level
policy predates this feature and must keep its existing behavior), exempt
advisor-orchestration failures from the fallback cooldown trigger, keep the
single-deployment model group protection intact against a generic
deployment-level allowed_fails, make cooldown_time precedence consistent
across resolution paths, fix a falsy-zero swallowing bug in the router-level
allowed_fails fallback, and make allowed_fails_policy resolution fall through
to the next matching exception type instead of stopping at the first unset
field.
Also restrict allowed_fails/allowed_fails_policy/cooldown_time to model_info:
litellm_params gets copied into the actual provider request, so a router-only
setting placed there would leak into that request.
* test(router): update test_cooldown_handlers.py for the deployment-policy signature change
Surfaced by the rebase: this mirrored test file (tests/test_litellm/ mirrors
litellm/) predates the router_unit_tests/ coverage added earlier in this PR and
was still calling _should_cooldown_based_on_deployment_policy with its old
4-argument signature and asserting the now-removed litellm_params cooldown_time
location.
* test(router): update test_fallback_event_handlers.py for model_info-only cooldown_time
Another mirrored test file surfaced by the rebase that still asserted the
now-removed litellm_params.cooldown_time location.
* fix(router): match cooldown-duration precedence in the fallback path to the primary path
_trigger_cooldown_for_failed_deployment only checked deployment config before
falling back to the router default, skipping the response Retry-After header
step that Router.deployment_callback_on_failure applies on the primary path.
* fix(router): restore litellm_params.cooldown_time as a pre-existing fallback
cooldown_time already had litellm_params support on Router.deployment_callback_on_failure
before this PR; the earlier model_info-only restriction (aimed at the leak concern
for the genuinely new allowed_fails/allowed_fails_policy fields) incorrectly dropped
that pre-existing capability too. model_info still takes priority when both are set.
* fix(router): keep the fallback-cooldown trigger in sync with #35104's review fixes
Applies the same two fixes landed on the split-out PR #35104 (which #34416
still duplicates until it's rebased onto the merged base): increment the
deployment's per-minute failure counter before evaluating cooldown, and
require the server-stamped failed_deployment_id instead of trusting a
metadata bucket, since neither "metadata" nor "litellm_metadata" can be told
apart from a caller-supplied one without knowing the call's function_name.
* fix(router): freeze the model_info fallback mapping to satisfy the type-discipline gate
* fix(router): defer f-string interpolation in fallback-cooldown debug logs
* fix(router): annotate cooldown-path locals with Final to satisfy the LIT010 budget
* fix(router): suppress reportPrivateUsage for cross-module cooldown helpers
* fix(router): don't cool down deployments for request-scoped 404s on generic API fallbacks
* fix(router): stamp the dynamic client-side-credential deployment id, not the shared static one
* fix(router): keep up with upstream typing modernization and Final-annotation ratchet
* fix(router): don't cool down deployments for a caller-supplied x-litellm-timeout
* fix(router): stamp dynamic client-side-credential id in completion fallback paths too
The generic-API-call helper already stamped the effective (dynamic-if-client-side-credential)
deployment id on exceptions, but the regular _completion/_acompletion exception handlers still
stamped the static shared deployment's id. A tenant using invalid forwarded credentials could
generate repeated failures attributed to, and eventually cooling down, the shared deployment
other tenants rely on. Extracted the stamping logic into one shared helper used by all three
call sites (generic API, sync completion, async completion) so the fix and future changes to it
stay in one place.
* fix(proxy): recognize body-supplied timeout/request_timeout/stream_timeout as caller-controlled
client_side_timeout was only set when the caller used the x-litellm-timeout header, but
Router._get_timeout also resolves the effective timeout from kwargs["timeout"],
kwargs["request_timeout"], and kwargs["stream_timeout"], all settable directly in the
request body (and x-litellm-stream-timeout wasn't marked either). A caller could set any
of those to a near-zero value, force a 408 on every deployment in a fallback chain, and
cool down deployments other tenants rely on without the guard in
_trigger_cooldown_for_failed_deployment recognizing it as caller-controlled. Also strip
any client-forged client_side_timeout from the request body so the marker is always
server-computed.
---------
Co-authored-by: Deepanshu <deepanshu.lulla@alpha-sense.com>
A poll of a Vertex passthrough batch wrote nothing to the managed-object row,
so status and file_object stayed frozen at the create-time snapshot and
GET /v1/batches served a stale status and an empty output file id for the life
of the batch. Only the create may claim a batch, but every observation of one
may refresh its state.
store_unified_object_id takes create_if_missing, which the poll clears: it
refreshes status and file_object through update_many, and leaves a row that is
absent absent rather than creating one owned by the observer, since created_by
and team_id are written by whoever reaches the create branch. The update payload
is now shared with the upsert so it cannot drift into writing api_key,
request_tags, created_by or team_id.
The passthrough identity re-assertion that was previously part of this PR ships
separately in #36121, so this PR keeps only the batch attribution work.
The creating key owns user_api_key_alias only when it actually has one. Guarding
the overwrite on the presence of a key rather than on a resolved alias nulled the
field out for every key generated without key_alias, and for any key rotated or
deleted before its batch finished, losing the creating user's alias that the spend
row previously carried. The guard now matches the team-alias line below it.
When store_model_in_db is true, general_settings are persisted to the
LiteLLM_Config DB table. On subsequent startups and periodic reloads,
_add_general_settings_from_db_config() unconditionally overwrites the
in-memory general_settings with DB-cached values, including
store_prompts_in_spend_logs.
This means a YAML config change (e.g. store_prompts_in_spend_logs: false)
deployed via CI/CD has no effect because the stale DB value (true) always
wins. The admin must manually update via /config/update API after every
deploy, defeating config-as-code.
Fix: track which general_settings keys were explicitly set in YAML at
startup (_yaml_general_settings_keys). During DB config merge, prefer the
YAML value for tracked keys. The DB value is only used as fallback when
YAML does not set the key, preserving the admin UI's ability to change
settings at runtime.
Steps to reproduce:
1. Start proxy with store_model_in_db: true, store_prompts_in_spend_logs: true
2. Change YAML to store_prompts_in_spend_logs: false, restart
3. Send a request, query LiteLLM_SpendLogs - prompts still stored
4. Check LiteLLM_Config table - DB still has true, overriding YAML
Slack thread: https://dataset-jsonhackathon.slack.com/archives/C0ACUS7LM29/p1785835131860139
