fix(vllm): _build_client_args mutates caller's extra_body dict across calls#1933
Merged
RobinPicard merged 1 commit intoJul 20, 2026
Merged
Conversation
`VLLM._build_client_args` and `AsyncVLLM._build_client_args` previously
popped the caller-provided `extra_body` dict and called `.update(...)` on
it directly. `dict.pop(key, {})` only returns a fresh dict when the key
is *absent* — when the caller passes `extra_body=my_dict`, the same
object comes back, so the subsequent `.update(output_type_args)` mutates
the caller's dict in place. Reusing one `extra_body` dict across calls
then leaks the previous call's `structured_outputs` constraint into
later unconstrained calls, silently constraining generation to a stale
regex/JSON schema.
Wrap the pop in `dict(...)` so we own the dict before merging into it.
Shallow copy is sufficient because `output_type_args` only adds
top-level keys (`structured_outputs`, `response_format`).
Adds a parametrized regression test over `sync_model`/`async_model`
covering both the in-place mutation and the cross-call leak.
Fixes dottxt-ai#1931.
Contributor
Author
|
Gentle ping for review — @rlouf @brandonwillard. This PR fixes a mutation bug where Test results: All existing vLLM tests pass. The fix is minimal (3 lines) and surgical. Happy to address any feedback. |
|
📚 Documentation preview: https://dottxt-ai.github.io/outlines/pr-preview/pr-1933/ Preview updates automatically with each commit. |
2 tasks
Solaris-star
added a commit
to Solaris-star/outlines
that referenced
this pull request
Jul 20, 2026
_pop("extra_body", {}) returns the caller's dict when present, so
.update() leaked structured_outputs/regex into subsequent unconstrained
calls. Mirror the VLLM fix (dottxt-ai#1933) with dict(...).
Fixes dottxt-ai#1931
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
VLLM._build_client_args(and the duplicatedAsyncVLLM._build_client_args) builds the client kwargs with:dict.pop(key, {})only returns a fresh dict when the key is absent. When the caller passesextra_body=my_dict,popreturns that same object, so the subsequent.update(output_type_args)mutates the caller's dict in place — addingstructured_outputs(orresponse_format) to it permanently.Concrete failure mode: a caller reusing one
extra_bodydict across calls — a common pattern when keeping a module-level default config and tweaking per call — silently leaks the previous call'sstructured_outputsconstraint into later unconstrained calls. An "unconstrained" generation is then actually constrained to a stale regex/JSON schema from a prior call. Silent data corruption, hard to debug.Reporter: #1931. The sibling
sglang.py::_build_client_argshas a different bug (single-call self-clobber viainference_kwargs.update(output_type_args)) and is being fixed separately in #1925 — that file is intentionally left untouched here.Fix
In both
VLLM._build_client_argsandAsyncVLLM._build_client_args, wrap thepopindict(...)so we own the dict before merging into it:Shallow copy is sufficient:
output_type_argsonly adds top-level keys (structured_outputs,response_format); nested values are not mutated.dict({})is{}so the no-arg default path is unchanged.Testing
test_vllm_build_client_args_does_not_mutate_caller_extra_body(parametrized oversync_model/async_model) covering both failure modes:Regex(r"[0-9]{3}")output type,extra_body=shared): caller's dict stays{"top_k": 5}(nostructured_outputsleaked in), and the returnedclient_args["extra_body"]is a separate object that does carry the constraint.Noneoutput type, sameextra_body=shared): the returnedclient_args["extra_body"]is{"top_k": 5}(no stalestructured_outputsfrom call 1).src/outlines/models/vllm.py(keeping the new test) reproduces both failures on both parametrizations; reapplying the fix passes.tests/models/test_vllm.pysuite: 21 passed (19 existing + 2 new parametrizations).ruff check src/outlines/models/vllm.py tests/models/test_vllm.pypasses.ruff formatreports pre-existing unrelated drift in this file from prior commits; not addressed here to keep the diff surgical.)Fixes #1931.
AI-Generated disclosure
Found via an AI-assisted code review pass (Claude Code) over
src/outlines/models/after issue #1931 was filed. I personally reproduced the cross-call mutation with a minimal repro (caller-providedextra_bodydict reused across two calls, observingstructured_outputsleak into the second, unconstrained call), verified the surgicaldict(...)wrapper against both thestructured_outputspath (regex/CFG/JSON) and theresponse_formatpath (JsonSchema) to confirm no regression, confirmed red→green by reverting onlyvllm.py, and ran the fulltest_vllm.pysuite before submitting.