Two review findings, both in the handoff between the create's success
callback and the background poll task.
The callback deferred its budget reservation release for any interactions
response with no usage, but the scheduler only starts a poll task when the
status is in_progress and an id is present. A create that came back
terminal without usage therefore matched the callback's test, got no poll
task, and left its reservation open forever: the pre-call estimate stayed
added to the key, user, team and org spend counters, and the key began
refusing traffic against budget it had never spent. The two conditions now
come from one shared gate so they cannot drift apart again.
Settling a background interaction re-runs the success handlers for a second
result on the same request, and OTEL dedupes span emission on a marker held
in that request's metadata. The in-progress create claimed the marker, so
the completion, the only event carrying usage and cost, was dropped as a
duplicate by OTEL and by every integration deriving from it. Clearing the
success-scoped markers alongside the existing dedup flag lets the cost span
through, leaving failure and guardrail markers untouched.
The standalone migration entrypoint re-runs `prisma generate` after the
migration completes. That refresh writes into the installed prisma package in
site-packages, which an arbitrary non-root uid cannot do, and which no uid can
do under a read-only root filesystem. Both are supported configurations of the
migrations Job: helm/litellm-helm/tests/migrations-job_tests.yaml asserts
runAsNonRoot, runAsUser and readOnlyRootFilesystem all render.
The write has always failed there, but the failure used to be swallowed. Making
migration failures fatal turned it into a hard exit 1, so a Job that applied
every migration correctly now reports Failed and blocks the rollout it was
supposed to gate.
The refresh is redundant in the shipped images: every Dockerfile generates the
client at build time from the same baked schema, copies it into the runtime
stage, and asserts it resolves there. It stays load-bearing only for a source
checkout, where CircleCI runs the entrypoint under `set +e` and ignores the exit
code anyway. So the call stays and only its exit code stops propagating;
migration failures are still fatal.
image-scan never ran on the change that introduced this, because its path filter
did not list the entrypoint it exercises. Add prisma_migration.py and
entrypoint.sh so the non-root offline migration test gates them from now on.
Write the fallback reasoning item id back to the cache so the
reasoning-done path and the completed snapshot cannot drift apart, and
cover the shared delta id and the snapshot alignment with tests.
Guardrails, token counting and rate limiting share the input transform with
the provider path, so moving reasoning onto reasoning_content hid it from
them. Provider-bound callers opt in with replay_reasoning.
The ceiling used to go through `int(... or 3)`, so anything `int()` accepted
worked. Tightening the new shared validator to `isinstance(int)` turned a
config that boots today into a proxy that refuses to start, because
`max_agentic_loops: os.environ/MAX_AGENTIC_LOOPS` is resolved to a string
before it reaches either check, and a YAML-quoted "5" is a string too.
Accept ints, integral floats, and strings that parse to a whole number. Keep
refusing bools, fractional floats, words, and anything below 1.
The experimental /v1/messages adapters lost prior-turn reasoning three
different ways once the request left for an OpenAI-shaped backend.
On the Responses path, thinking blocks were flattened into output_text
inside the assistant message, so the model read its own private reasoning
back as visible prose and no reasoning item was ever sent. They now become
Responses reasoning input items, grouped by signature so summary parts that
arrived as one item go back as one item. The response direction stops
hardcoding signature=None and carries the reasoning item id, which is what
lets the next turn regroup them; the streaming wrapper emits the matching
signature_delta.
On the chat completions path the adapter attached thinking_blocks but never
set reasoning_content, so Moonshot and DeepSeek substituted a single-space
placeholder and other providers sent nothing. It is now derived from the
thinking blocks.
With use_chat_completions_url_for_anthropic_messages and a model that itself
bridges to /v1/responses, the assistant message was dropped whole: reasoning,
text, and all. That branch now emits the reasoning items and the message
content alongside the tool calls.
Fixes#24985
Align the response.completed item IDs by copying each output item rather than
writing to it in place, and move the regression cases into the existing
completion-response and image-generation test modules.
Streaming /v1/responses over the completion bridge minted a fresh resp_{uuid4}
for every response, while spend tracking stored the inner chat completion id as
request_id. The session lookup queries on request_id, so a follow-up sent with
that response id matched no rows and the prior conversation was silently
dropped. The iterator now pulls the first upstream chunk before emitting
response.created, so created, in_progress and completed all carry the same
encoded chat completion id.
Two more ways the same history went missing:
- The session lookup only read spend logs already written to the DB, so a
follow-up sent inside the batch writer's window found nothing. It now also
reads the rows still queued in memory.
- Input was only accepted as a string or a single dict, so the list shape the
Responses API actually sends dropped every user turn from the reconstructed
history.
* fix(otel): emit LLM Call spans for speech, image, moderation, ocr and transcription
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(otel): log the image request before caller headers are merged in
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(otel): map non-chat routes to standard genai operations
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(otel): stop caller image headers aliasing the logged request body
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(otel): keep resolved api_base in async moderation pre_call
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(otel): log resolved client endpoint for speech pre_call
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* chore(otel): justify mutable request payloads in speech and image pre_call
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(otel): keep caller headers out of the logged speech request body
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>
The ceiling was only checked at the feature level, on
litellm_settings.websearch_interception_params. The per-deployment
litellm_params.max_agentic_loops, which wins over it, went straight into
int(kwargs.get("max_agentic_loops", 3) or 3), so a 0 was swallowed by the
falsy fallback and read as the default 3. Asking for the tightest ceiling
handed you the loosest one. A non-integer booted the proxy and then failed
every request to that model with "invalid literal for int() with base 10".
Both settings now share one validator, which names the field it rejected,
and the per-deployment value is checked while the model list is read at
startup so a bad value stops the proxy rather than surfacing per request.
The check sits in load_config rather than on LiteLLM_Params because the
proxy builds its router with ignore_invalid_deployments=True, where a
validation error drops the deployment silently instead of refusing to
start. This is the same placement the complexity_router_config plugin
check already uses.
Chat completions read the same key through a separate path that turned 0
into 1 and true into a ceiling of 1, so it now shares the validator too
and the key means one thing on both surfaces.
encrypted_content on a reasoning input item is written by LiteLLM's own
_encode_thinking_blocks as a JSON array of Anthropic/Bedrock thinking
blocks, so decode it back and replay the signed blocks on the assistant
message instead of dropping them. Providers without a native
ResponsesAPIConfig now keep the verifiable chain-of-thought across turns,
and prior-turn reasoning stops reaching the provider as visible
assistant text.
The Chat Completions -> Responses bridge stamped the upstream chatcmpl-*
ID onto message output items, so replaying bridged history into native
OpenAI Responses failed with "Expected an ID that begins with 'msg'".
Image generation calls were minted as chatcmpl-*_img_N instead of ig_*,
and reasoning items used a salted hash() that is not stable across
processes.
Streaming minted msg_* for its incremental events but rebuilt the
response.completed snapshot through the same broken transform, so the
snapshot contradicted the events it had just sent and streaming clients
hit the same 400. The snapshot now reuses the IDs already streamed.
Fixes#27333
Reading every operator let a comparison beside an assignment look like one.
`ok := n = 1 AND stmt = '<dml>'` registered `stmt` as written, which collided
with the `EXECUTE stmt` further down and flagged a block that rewrites nothing.
A statement holds one assignment at most, so the search now stops at the first
operator that reads as one: everything after it is the expression being
assigned, where an `=` only ever compares. Nine shapes were flagged this way,
a cast, a `coalesce`, a `format`, a named-argument arrow and the rest, and all
of them are valid PL/pgSQL that leaves the table untouched.
`INTO` and `USING` no longer count as names an `EXECUTE` runs. Masking blanks a
literal in place, so `EXECUTE '<sql>' INTO n` left `INTO` looking like the name
being run, and an ordinary query reaching the same word collided with it. The
docstring claiming that collision was impossible was wrong, and both words are
now dropped instead.
A loop is a fourth way a literal reaches a variable. `FOR stmt IN SELECT
'<dml>' LOOP EXECUTE stmt` empties the table and the gate passed it, so the
target of a `FOR` or a `FOREACH` is read as assigned too.
Reading each statement once rather than once per operator also drops the cost
of a statement with thousands of them from seconds to milliseconds.
A capped turn on a streaming request is rebuilt into SSE by
FakeAnthropicMessagesStreamIterator. It emitted content_block_stop for
every block but content_block_start only for text, thinking,
redacted_thinking and tool_use, so a web search turn's server_tool_use
and web_search_tool_result blocks produced stops with no matching start.
Anthropic's SDK accumulator appends on content_block_start and then
indexes content[event.index] on content_block_delta, so the orphan stops
shifted every later index and client.messages.stream() raised IndexError
on the text block. Unknown block types now pass through with a start of
their own, which keeps position equal to index.
Also corrects two claims that said no current caller reaches the loop
with stream=True. AgenticStreamingIterator does, and it keeps raising,
because its events are already on the wire.
Eleven DummyManagedFiles stubs still declared afile_list(self, purpose,
litellm_parent_otel_span). The real hook grew user_api_key_dict, limit and
after, so the doubles no longer stand in for the interface they replace.
Their tests pass today only because every one of them takes a provider
branch that never reaches the hook, which means a stub going stale is
invisible until some later test does reach it and reads a TypeError as a
behavior change.
Signatures only; no test changes behavior.
OpenAIFilesPurpose was missing evals, which OpenAI documents. The upload
route validates against that set, so POST /v1/files with purpose=evals was
already being rejected, and the new listing validator extended the same
rejection to GET /v1/files?purpose=evals, turning a purpose OpenAI accepts
into a hard 400. Nothing branches exhaustively on the type, so widening it
changes no routing.
The managed-file listing test fake only understood a created_by filter. The
OR filter a key carrying both a user_id and a team_id produces, the team_id
filter a service-account key produces, and the empty filter a proxy admin
produces all fell through it and returned every row, so the shapes most real
keys send went uncovered. The fake now applies the filter it is handed, and
the listing is tested against all three, including paging an OR filter
across a cursor.
Two docstrings claimed the continuation chunk bounds what a filtered page
costs. It bounds queries per row scanned; the walk is still linear in the
rows the caller owns.
A bare = was found with partition, so a comparison earlier on the line took
the one slot and the assignment after it went unread. INTO targets and the
name an EXECUTE runs are also allowed to sit on the next line now.
* 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
Thirteen tests flipped the flag directly, and an autouse fixture reset it to
True around each of them by hand. monkeypatch.setattr does both jobs, so the
fixture keeps only the part that says what the default is, and each test states
its own override at the point it needs one.
Fifteen tests assigned litellm.callbacks directly and left the conftest global
snapshot to clean up after them. monkeypatch.setattr restores it as part of the
test, so the file no longer depends on that safety net to stay isolated.
Scanning a statement's own literals for SQL only makes sense when the name
before INTO is a variable the body later executes. INSERT INTO names a table
there, so an insert into a table sharing a variable's name was flagged for
whatever its column values happened to spell. An INSERT that really does
assign reaches INTO through RETURNING, which the preceding word separates.
Also names the scope boundary in the module docstring: the ban is on
row-rewriting DML, not on everything whose cost scales with table size.
Twenty-three tests across eleven files opened with litellm.set_verbose = True
and never put it back, so the flag stayed on for everything that ran after them
in the same process. None of those files read the output it produces: no
caplog, no capsys, no assertion on a log line, so the flag was left over from
debugging. Deleting it beats restoring it, since restoring keeps the noise.
Ten of the eleven stop leaving the flag on. test_volcengine_embedding.py still
ends with it set, from something it exercises rather than from the test itself,
which is worth its own look.
Fifteen tests opened with litellm.set_verbose = True and never put it back, so
the flag stayed on for everything that ran after them in the same process.
Nothing in the file reads the output it produces: there is no caplog, no capsys
and no assertion on a log line, so the flag was left over from debugging.
Deleting it beats restoring it, since restoring keeps the noise.
Seven tests captured litellm.use_legacy_interactions_schema, wrapped their body
in a try, and put it back in a finally. monkeypatch.setattr does that, so the
capture, the try and the finally go and the bodies lose an indentation level.
The remaining hand-rolled restores stay. They hold the flag only across the
iterator's constructor and put it back before the test iterates, so handing
them to monkeypatch would widen that window to the whole test and change what
the streaming assertions run against.
Nine tests in test_http_handler.py captured litellm.disable_aiohttp_transport,
force_ipv4, ssl_ecdh_curve or the request_timeout pair, wrapped their whole body
in a try, and put the value back in a finally. monkeypatch.setattr does all of
that, so the captures, the try and the finally go away and the bodies lose a
level of indentation. The class-scoped restore_request_timeout fixture existed
only for that same bookkeeping and goes with them.
litellm.in_memory_llm_clients_cache is left alone on purpose: the eviction tests
assert a handler is garbage collected, and monkeypatch holds the replaced value
alive until teardown, which keeps the weakref they check from clearing.
Ten tests set litellm.s3_callback_params by hand. Four of them reset it to None
on the last line of the test body, which only runs when the test passes; the
other six wrap the body in try/finally to put the old value back. Raising inside
test_s3_verify_false_handling on the current file leaves the whole callback
config, bucket, endpoint and keys, set in the process for whatever runs next.
monkeypatch.setattr covers both shapes and restores on failure, so the 28 TQ005
violations and the try/finally scaffolding come out together.
51 tests pass, and the wider tests/test_litellm/integrations tree is unchanged.
The five TQ002 mock-echo tests in this file are left alone; those need a
judgement about what S3 logging should assert, not a mechanical sweep.
test_zai_provider.py set LITELLM_LOCAL_MODEL_COST_MAP and litellm.model_cost
directly and never put them back, so every test that ran after it in the same
process saw a local cost map instead of the real one. The two respx tests did
the same to litellm.disable_aiohttp_transport with no restore at all.
Both now go through monkeypatch, which restores on teardown including when the
test fails. The cost-map setup moves into a fixture requested by exactly the
five tests that read the cost map.
Onyx, prompt security, hiddenlayer, repelloai and deepkeep all write straight to
os.environ and unset again at the bottom of each test. None of the five has a
try/finally, so the moment a test fails it returns to the runner with the keys
still set and whatever runs next in that worker inherits them.
Raising inside test_onyx_guard_with_custom_timeout_from_kwargs on the current
files leaves ONYX_API_BASE and ONYX_API_KEY behind; doing the same in
test_hiddenlayer_config_saas leaves HIDDENLAYER_API_BASE. Both come back clean
after this.
89 raw writes and the hand-rolled deletes become monkeypatch calls. The
class-level setup_method and teardown_method pair in the onyx file, sweeping the
same three keys twice, becomes one autouse fixture. The sys.path.insert lines
and their now-unused imports go too, and litellm.set_verbose = True, which only
turned global debug logging on for whatever ran next, is dropped rather than
restored.
test_onyx_guard_config and test_prompt_security_guard_config asserted nothing at
all, so they could only fail by raising. Each now pins what init_guardrails_v2
produces: exactly one guardrail of the right class on litellm.callbacks,
carrying the configured name, default_on and hook. The zero-assert tests in the
other three are left alone; those are a judgement about each guardrail rather
than a mechanical sweep.
tests/test_litellm/proxy/guardrails passes at 2873.
* test(policy-engine): unwind the callback global the pipeline tests scaffold around
Every one of the 16 tests in this file set litellm.callbacks by hand, each
wrapping its body in a try/finally to put the old value back, and each capturing
that old value with a .copy() first. That is 32 TQ005 violations and about 70
lines of scaffolding to say what monkeypatch.setattr says in one.
The write also sat outside the try, so the block that restores it did not cover
the statement that changed it.
16 tests pass either way, and litellm.callbacks reads restored on both sides,
because the conftest snapshot already lists it. The point is that these tests
stop depending on that snapshot to clean up after them.
* test(realtime): unwind the same callback global in the realtime streaming tests
Same global, same shape as the previous commit. 25 writes to litellm.callbacks,
2 of them wrapped in a try/finally that resets to [] rather than to the old
value, and 12 tests that write it with no protection at all.
monkeypatch.setattr replaces all of them, and the sys.path.insert with its
now-unused os and sys imports goes too.
Both sides read restored here as well, for the same reason as the previous
commit: litellm.callbacks is in the conftest snapshot. What changes is that
these tests no longer lean on it.
101 tests pass in this file, 16 in the policy engine one.
* style(realtime): wrap the one signature the monkeypatch param pushed past 120
Seventeen tests in this file save a litellm module global, open a try, write
it, and restore it in a finally. Four more sit behind autouse fixtures that
reset the flag to a hard-coded False rather than to whatever it was.
monkeypatch.setattr does all of that, so the capture, the try and the finally
come out and the test body loses a level of indentation. The alias-format
fixtures stop guessing the value they are restoring to.
Also drops the sys.path.insert, whose argument resolves four levels above the
repo, so it was never what made the imports work.
TQ003 1077 -> 1076 and TQ005 2836 -> 2796, and the budget ceilings come down
with them. 443 tests pass either way; the conftest snapshot was already
catching these globals, so this is about not needing it.
* test(cost-calc): stop 182 global writes leaking out of the cost-calc suites
Across test_cost_calculator.py and llm_cost_calc/test_llm_cost_calc_utils.py,
58 tests opened by setting LITELLM_LOCAL_MODEL_COST_MAP in os.environ and
replacing litellm.model_cost, and none of them put the env var back. The
second file already had a _local_model_cost_map fixture doing it by hand with
a try/finally, so both idioms sat in the same file.
Keep that fixture, give it monkeypatch, and have every one of those tests ask
for it. The margin and discount tests drop their hand-rolled
copy-then-restore in favour of monkeypatch.setattr, which also puts the
global back when an assertion fails part way through.
Both files also drop a sys.path.insert whose argument resolves outside the
repo, so it was never what made the imports work.
TQ003 1077 -> 1075, TQ004 768 -> 693, TQ005 2836 -> 2731, and the budget
ceilings come down with them.
* fix(test): make the streamed-cost tests load the map they assert against
The local_cost_map fixture set LITELLM_LOCAL_MODEL_COST_MAP but never reloaded
litellm.model_cost, and reading the variable is not what loads the map. So the
three streaming-cost tests billed against whatever map the process happened to
be holding, and their hardcoded prices only held when something else had
already swapped in the checked-in one. This branch stops the cost-calc tests
leaking that map, which left test_main billing at the ambient prices instead.
The fixture now loads the map it names, so the prices these tests assert hold
on their own.
Both datadog test files hand-roll what monkeypatch.setenv already does: read the
old value, write the test value, put the old one back on the way out. The cost
management fixture checks the old value for truthiness rather than for None, so
an operator running the suite with DD_API_KEY set to the empty string gets it
deleted rather than restored. Starting from DD_API_KEY="" and running test_init
leaves it None on the current file, and "" after this.
13 raw os.environ writes become monkeypatch.setenv, the two fixtures stop being
yield fixtures because there is nothing left to do on the way out, and the now
unused os import goes with them.
27 tests pass across the two files, 88 across tests/test_litellm/integrations/datadog.
The managed hook returned the plain dict build_list_page builds, while
every other GET /v1/files path returns an SDK page object. A post-call
success hook or a logging callback that reads response.data off the
listing raised AttributeError as soon as a request took the managed path
FileListPage is a pydantic model over the same five fields, so hooks read
.data again and the response body does not move: jsonable_encoder gives
the same keys in the same order for the model and for the dict. It sits
in litellm.types.llms.openai because base_llm/files/transformation.py
already imports from there and cannot import proxy modules. It is
deliberately not subscriptable, since the provider-backed path returns a
page object that is not either, and dict access would be a third contract
to keep alive
Also reject a purpose the Files API never accepts. An unknown purpose
matches no row, so the listing answered an empty page for what is really
a bad request, while the upload route in this same file already refuses
those values against get_args(OpenAIFilesPurpose). The check runs before
the first query, and only in the managed hook, so providers that define
their own purposes keep them
Also put back the route's original except tail. Sending every error
through handle_exception_on_proxy changed error.type on a bad
target_model_names from "None" to the exception class name, which a
caller matching on the body would read as a break. create_file in this
file already pairs base's tail with a ProxyException passthrough, so
list_files does the same and the handle_exception_on_proxy import is gone
* test: use monkeypatch.setenv for env writes in tests/test_litellm
`os.environ["X"] = v` inside a test leaks the value into every test that runs
after it in the same worker, so ordering decides the result. 262 of those
writes across 40 files now go through pytest's `monkeypatch` fixture, which
restores the previous value at teardown.
The rewrite skips any test that a mock.patch-family decorator wraps, any test
with defaulted positional parameters, any test whose own name is called
directly elsewhere, and rebinds nothing inside nested defs, because in each of
those cases appending a fixture parameter changes what pytest or mock binds.
Ratchets the TQ004 ceiling from 768 to 506.
* fix(test): delete the key through monkeypatch instead of popping it first
Five tests popped a key straight out of `os.environ`, ran, then restored it with
`monkeypatch.setenv`. By the time monkeypatch saw the name it was already gone,
so it recorded "absent" as the value to go back to and deleted the key at
teardown. On a worker that inherited a real `RESEND_API_KEY`, `SENDGRID_API_KEY`,
`UI_PASSWORD`, `LITELLM_SALT_KEY` or `OPENAI_API_KEY`, every test after the first
one ran without it.
`monkeypatch.delenv(..., raising=False)` removes the key and restores whatever
was there, so the try/finally the manual restore needed goes with it.
* chore(test): leave the two cost-calc files to the PR that rewrites them fully
Both files are also in #37815, which converts the module-global writes as well
as the env writes and folds them into one fixture. Two PRs rewriting the same
lines differently is a conflict nobody benefits from resolving, so this one
drops back to staging on those two and keeps the other 39.
TQ004 clears 200 here instead of 275; the rest moves with #37815.
