Skip to content

fix(sglang): copy caller extra_body before merging structured outputs#1937

Merged
RobinPicard merged 1 commit into
dottxt-ai:mainfrom
Solaris-star:fix/1931-sglang-extra-body-copy
Jul 21, 2026
Merged

fix(sglang): copy caller extra_body before merging structured outputs#1937
RobinPicard merged 1 commit into
dottxt-ai:mainfrom
Solaris-star:fix/1931-sglang-extra-body-copy

Conversation

@Solaris-star

Copy link
Copy Markdown
Contributor

Summary

#1931 reported that VLLM/AsyncVLLM._build_client_args mutated the caller's extra_body. That path was fixed in #1933. The same pop + in-place update pattern remains in SGLang / AsyncSGLang._build_client_args, so a reused extra_body dict still leaks structured-output keys (regex / ebnf) into later unconstrained calls.

Change

  • Copy the caller extra_body with dict(...) before merging (sync + async)
  • Regression test: shared dict is unchanged after a constrained call; unconstrained follow-up has no regex

Test plan

  • Pure logic repro of old vs new pop/update behavior
  • Added test_sglang_build_client_args_does_not_mutate_caller_extra_body
  • Note: package requires Python <3.14; CI on supported versions should run the new test

Fixes #1931

@github-actions

Copy link
Copy Markdown

📚 Documentation preview: https://dottxt-ai.github.io/outlines/pr-preview/pr-1937/

Preview updates automatically with each commit.

@RobinPicard RobinPicard left a comment

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.

Thanks for opening a PR! The change looks good, but the tests are not working because you're using a fixture that does not exist.

_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
@Solaris-star
Solaris-star force-pushed the fix/1931-sglang-extra-body-copy branch from 54703c3 to 074c0e8 Compare July 20, 2026 14:53
@Solaris-star

Copy link
Copy Markdown
Contributor Author

Thanks for the review — fixed the test to use the module-level mock openai_client / sglang_model_name globals (same pattern as test_sglang_build_client_args_merges_user_extra_body) instead of non-existent fixtures. Local: 4 passed on the two _build_client_args unit tests.

@Solaris-star

Copy link
Copy Markdown
Contributor Author

Re-requesting review: the fixture issue is fixed on the latest commit (module-level openai_client / sglang_model_name globals, same as the existing merge test). CI unit tests for _build_client_args pass locally (4 passed).

@RobinPicard RobinPicard left a comment

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.

Looks good!

@RobinPicard
RobinPicard merged commit 129cec0 into dottxt-ai:main Jul 21, 2026
6 checks passed
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.

VLLM/AsyncVLLM._build_client_args mutates and leaks caller's extra_body dict across calls

2 participants