Skip to content

[Bugfix] Reject mixed structured output backends during validation - #49625

Merged
arpera merged 7 commits into
vllm-project:mainfrom
QwertyJack:codex/fix-structured-output-mixed-backend
Oct 20, 2026
Merged

arpera merged 7 commits into
vllm-project:mainfrom
QwertyJack:codex/fix-structured-output-mixed-backend

Conversation

@QwertyJack

@QwertyJack QwertyJack commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Fixes #43920.

The V1 structured-output manager initializes one engine-level backend from the first structured-output request. However, backend=auto still resolves every request independently. A valid xgrammar-compatible schema followed by a valid schema using multipleOf resolves to xgrammar and guidance respectively, while EngineCore continues compiling both with the first backend. After #47312, this no longer kills EngineCore, but the second request still reaches grammar compilation through the wrong backend and returns HTTP 500.

This change adds two consistency checks:

  • The frontend records the first resolved backend on its engine config and raises VLLMValidationError for a later mismatch, preserving an admission-time HTTP 400 for the common single-frontend path.
  • StructuredOutputManager records the backend name after successful backend creation and rejects any later mismatch before backend construction or grammar compilation. This is the authoritative guard for multi-API-server and data-parallel topologies where frontend config state is process-local.

Requests without structured outputs do not execute either check.

This is not a duplicate of existing work:

Test Plan

PYTHONPATH=$PWD .venv/bin/python -m pytest tests/v1/structured_output/ -q
.venv/bin/pre-commit run --files vllm/sampling_params.py vllm/v1/structured_output/__init__.py tests/v1/structured_output/test_validation.py

The frontend regression test uses real xgrammar and guidance validation. The first JSON schema resolves to xgrammar; a second valid schema containing multipleOf resolves to guidance and must be rejected because the engine already selected xgrammar.

The manager regression test models two independent frontends sending requests resolved to xgrammar and guidance into one EngineCore. It verifies that the second backend is not constructed and the guidance grammar is not compiled through the cached xgrammar backend.

Test Result

Pre-fix, order-dependent repro on unmodified main (one engine config with backend=auto, real xgrammar 0.2.2):

1) frontend per-request auto resolution on one engine config:
  valid JSON schema resolves to: xgrammar
  duplicate-root EBNF resolves to: guidance
2) engine-side compilation with the latched backend:
  JSON: compiled by latched xgrammar
  GRAMMAR: FAILED under latched xgrammar -> RuntimeError

The same request succeeds on a fresh engine (guidance accepts the EBNF) and fails only after an xgrammar request latched the backend, which is why this shows up as intermittent 500s rather than as a deterministic failure of a single request.

Regression tests:

  • tests/v1/structured_output/test_validation.py: 20 passed (2 new tests; all pre-existing validation tests still pass).
  • tests/v1/structured_output/: 74 passed.
  • pre-commit run --files ... for the three touched files: all hooks passed, including ruff, mypy, typos, config validation, and SPDX checks.
  • Model evaluation is not applicable because generation logic and model outputs are unchanged; this only rejects unsupported mixed-backend routing before it reaches an incompatible compiler.

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • Documentation update is not required for this request-validation bug fix.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@sfeng33 sfeng33 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR! The fix is correct for a single frontend but silently incomplete for the default multi-API-server / data-parallel topology.

I think the single-backend check belongs in StructuredOutputManager.grammar_init() inside the shared EngineCore, immediately before backend creation/grammar compilation, conceptually:

if self.backend is None:
    self.backend_name = request_backend
    self.backend = create_backend(request_backend)
elif request_backend != self.backend_name:
    reject_request(...)

Wdyt?

@QwertyJack
QwertyJack force-pushed the codex/fix-structured-output-mixed-backend branch from 5d22add to c99312f Compare July 27, 2026 15:46
@QwertyJack
QwertyJack requested a review from benchislett as a code owner July 27, 2026 15:46
@QwertyJack

Copy link
Copy Markdown
Contributor Author

Good catch, agreed. The frontend config state is process-local, so it cannot be the only guard.

I updated the PR to record the selected backend in StructuredOutputManager after successful backend creation and reject a different request backend before backend construction or grammar compilation. I kept the frontend check as an early HTTP 400 path when requests share one frontend; the manager check is now authoritative for multi-API-server / DP topologies.

I also added a regression test that models two independent frontends resolving requests to xgrammar and guidance before sending them to one manager. It verifies that the guidance backend is not constructed and the second grammar is not compiled through the cached xgrammar backend. The focused suite now passes 6 tests. Thanks for pointing this out.

@mergify

mergify Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @QwertyJack.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Aug 14, 2026
@arpera arpera moved this to Unsorted queue in Structured Output (arpera) Aug 26, 2026
@arpera arpera moved this from Unsorted queue to Backlog in Structured Output (arpera) Aug 27, 2026
@QwertyJack
QwertyJack force-pushed the codex/fix-structured-output-mixed-backend branch from c99312f to 9d0fade Compare September 15, 2026 04:41
@QwertyJack
QwertyJack requested a review from arpera as a code owner September 15, 2026 04:41
@mergify mergify Bot removed the needs-rebase label Sep 15, 2026
@arpera

arpera commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Hi @QwertyJack, thank you for your PR! I see you implemented approach with rejection of requests that needs to use different grammar backends.
What do you think of an approach that would process these queries and not reject them? For example, to store multiple grammar backends per StructuredOutputManager. I believe this would be a better approach not to reject such requests. What do you think about it, could this be implemented? Or there are some obstacles that stops us from this idea in general? Would be glad to hear your thoughts on this first.

@mergify

mergify Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @QwertyJack.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Oct 6, 2026
@QwertyJack

QwertyJack commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

@arpera Thanks for the suggestion! I agree that supporting multiple backends would be preferable for backend=auto.

This PR preserves the existing single-backend constraint and prevents requests from reaching an incompatible compiler. Removing that constraint would also require validating mixed-backend batches, bitmask compatibility, and async/speculative decoding paths. I expect that work to extend beyond this PR’s scope and be better addressed separately.

@arpera

arpera commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #96098 for commit 1e5bd327060b.

@arpera

arpera commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

/ci retry

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #96121 for commit a5fba8b7cccf, running 5 failed step(s) from Buildkite CI #96098.

@arpera

arpera commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

@QwertyJack, please, look at this failed ci job first
https://buildkite.com/vllm/ci/builds/96121/list?jid=01a158b5-7a2c-49a3-aa2b-50d40be0f607&tab=output
It looks like this PR broke some of the tests

Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
@arpera

arpera commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #96225 for commit ec442299940d.

@arpera
arpera merged commit 88077b7 into vllm-project:main Oct 20, 2026
235 checks passed
@QwertyJack
QwertyJack deleted the codex/fix-structured-output-mixed-backend branch October 21, 2026 00:42
linfeng-yuan pushed a commit to vllm-project/vllm-ascend that referenced this pull request Oct 27, 2026
### What this PR does / why we need it?

Port `vllm-ascend` to vLLM main `544f182d` (bumped from `c0b6f74`) while
keeping the
`v0.31.0` release lane green. The upstream window drops
`replace_parameter(..., prefer_copy=...)`,
fuses the Kimi-K3 output gate into the QKV-A projection, adds the ROCm
Kimi-K3
specialized-decode hooks, widens the EPLB expert-weight and
rejection-sample contracts, and
adds the DP-drafter `num_reqs_override`, the MRv2 `skip_sample` drafting
skip, the HiSparse
`topk_buffer_width`, the SimpleCPUOffload CPU-block RPC and the abstract
`AsyncModelRunnerOutput.get_kv_connector_output`. Every gate below is
either a
`vllm_version_is("0.31.0")` split or an attribute/signature probe.

| Files | Upstream vLLM change (why it breaks vllm-ascend) | vllm-ascend
adaptation |

|-------|--------------------------------------------------|------------------------|
| `.github/vllm-main-verified.commit` | — (main-lane pin only) |
Verified main pin bumped `c0b6f74` -> `544f182d` |
| `tests/ut/attention/test_mla_prolog_v3_route.py` |
[vllm#60805](vllm-project/vllm#60805) makes
`MLAAttention.get_kv_cache_spec` read `topk_buffer_width` (set in
`__init__`);
[vllm#59634](vllm-project/vllm#59634) makes
`_mla_preprocess` return a 3-tuple | Set `owner.topk_buffer_width =
None` on the stub; mock `_mla_preprocess` as `(None, None, None)` |
| `tests/ut/attention/test_mla_v1.py` |
[vllm#59634](vllm-project/vllm#59634) fuses the
Kimi-K3 output gate as a third QKV-A shard | Add tests for
`qkv_a_gate_fused`/`use_output_gate` and `_split_fused_qkv_lora` gate
peeling; unpack the 3-tuple |
| `tests/ut/models/minimax_m3/test_minimax_m3_vit.py` |
[vllm#60238](vllm-project/vllm#60238) makes
`_mark_tower_model` read `use_v2_model_runner` | Add
`use_v2_model_runner=False` to the constructed runner stub |
| `tests/ut/models/test_kimi_k3_adapter.py` |
[vllm#60683](vllm-project/vllm#60683) adds the
ROCm specialized-decode hooks (`init_specialized_decode`,
`get_model_state_cls`, the `step` forward argument, and `layer.kv_cache`
reassignment via `clear_layer_kv_caches`) | Add main-only tests (`skipif
vllm_version_is("0.31.0")`) covering the inherited hooks |
| `tests/ut/ops/test_routed_experts.py` |
[vllm#61786](vllm-project/vllm#61786) registers
expert weights via `w.view(torch.uint8) if w.element_size() == 1 else w`
| New UT asserting `EplbExpertTensorList.element_size()/view()` keep the
container and its aliasing semantics |
| `tests/ut/patch/platform/test_patch_structured_output.py` |
[vllm#49625](vllm-project/vllm#49625) makes
`StructuredOutputManager.grammar_init` read `backend_name` | Set
`manager.backend_name = None` on the stub |
| `tests/ut/runtime_guard/test_unified_wiring.py` |
[vllm#56430](vllm-project/vllm#56430) adds
`AsyncModelRunnerOutput.get_kv_connector_output` | Add a stub
`get_kv_connector_output` to the fake async output |
| `tests/ut/simple_kv_offload/test_simple_kv_offload.py` |
[vllm#52921](vllm-project/vllm#52921) adds the
`get_simple_cpu_offload_num_cpu_blocks` worker RPC | New main-only UTs
for the aligned / no-group / no-connector paths |
| `tests/ut/spec_decode/test_dflash2_v2.py` |
[vllm#58984](vllm-project/vllm#58984) adds
`num_reqs_override` to `prepare_dflash_inputs_triton` | Update the
expected kwarg; new test for positional forwarding |
| `tests/ut/worker/test_model_runner_v1.py` |
[vllm#60805](vllm-project/vllm#60805)
`topk_buffer_width` | Set `layer.topk_buffer_width = None` on the layer
stub |
| `tests/ut/worker/test_model_runner_v2.py` | Same
[vllm#60805](vllm-project/vllm#60805) | Same
stub attribute |
| `tests/ut/worker/v2/test_speculator.py` |
[vllm#62784](vllm-project/vllm#62784) adds
`skip_sample` to the drafter `_prefill` | Expect/forward `skip_sample`
on main (7 -> 8 positional args) |
| `tests/ut/worker/v2/test_v2_block_table.py` |
[vllm#60767](vllm-project/vllm#60767) makes
`init_block_table_layout_tensors` read `_sliding_window_reach` | Set
`tables._sliding_window_reach = [0, 0]` |
| `vllm_ascend/attention/mla_v1.py` |
[vllm#59634](vllm-project/vllm#59634) fuses the
Kimi-K3 output gate into the QKV-A projection (three shards, no
standalone `g_proj`);
[vllm#62326](vllm-project/vllm#62326) drops
`replace_parameter(..., prefer_copy=)` | Detect the gate-fused layout
from `fused_qkv_a_proj.output_sizes`, disable MLAPO/PROLOG_V3 for it,
bound the KV-A slice with `_fused_qkv_a_kv_end`, peel the gate in
`_split_fused_qkv_lora`; version-split `replace_parameter` |
| `vllm_ascend/attention/sfa_v1.py` |
[vllm#62326](vllm-project/vllm#62326) drops
`replace_parameter(..., prefer_copy=)` | Version-split
`replace_parameter` (keep `prefer_copy=True` on v0.31.0) |
| `vllm_ascend/models/deepseek_v4/indexer.py` |
[vllm#52670](vllm-project/vllm#52670) keeps
`STR_DTYPE_TO_TORCH_DTYPE["fp8"]` as the uint8 storage dtype, so
`kv_cache_dtype_str_to_dtype` cannot validate the indexer's fp8 compute
dtype | Validate `indexer_kv_dtype` against the platform default string
and return the compute dtype directly |
| `vllm_ascend/models/kimi_k3.py` |
[vllm#60683](vllm-project/vllm#60683) adds the
ROCm specialized-decode hooks and reassigns `layer.kv_cache` on teardown
| Add a `kv_cache` setter, call `init_specialized_decode` + override
`get_model_state_cls` to the Ascend hybrid state on main, forward `step`
|
| `vllm_ascend/observability/runtime_guard/hooks.py` |
[vllm#56430](vllm-project/vllm#56430) makes
`AsyncModelRunnerOutput.get_kv_connector_output` required | Implement it
on `AscendAsyncOutput` (main only) |
| `vllm_ascend/ops/fused_moe/routed_experts.py` |
[vllm#61786](vllm-project/vllm#61786) moves
1-byte expert tensors as uint8 views | Add `element_size()`/`view()` to
`EplbExpertTensorList`; widen the `get_expert_weights` return type |
| `vllm_ascend/ops/gdn.py` |
[vllm#62326](vllm-project/vllm#62326) drops
`replace_parameter(..., prefer_copy=)` | Version-split
`replace_parameter` for the packed conv weight |
| `vllm_ascend/ops/kimi_kda.py` | Same
[vllm#62326](vllm-project/vllm#62326) | Same
version-split for the packed conv weight |
| `vllm_ascend/ops/linear.py` | Same
[vllm#62326](vllm-project/vllm#62326) | Same
version-split for `weight_fp32` |
| `vllm_ascend/ops/triton/v2/spec_decode/prepare_dflash_inputs.py` |
[vllm#58984](vllm-project/vllm#58984) forwards
the DP-synced request count during dummy runs | Accept
`num_reqs_override`, cap `num_reqs` and slice `num_scheduled_tokens` |
| `vllm_ascend/worker/v2/spec_decode/autoregressive/speculator.py` |
[vllm#62784](vllm-project/vllm#62784) adds
`skip_sample` to `_prefill` | Version-split `_prefill`, forward
`skip_sample` on main |
| `vllm_ascend/worker/v2/spec_decode/dflash/speculator.py` |
[vllm#58984](vllm-project/vllm#58984) adds
`num_reqs_override` | Forward `num_reqs_override` through both
`prepare_dflash_inputs` wrappers |
| `vllm_ascend/worker/v2/spec_decode/gemma4/speculator.py` |
[vllm#62784](vllm-project/vllm#62784) adds
`skip_sample` | Accept and forward `skip_sample` |
| `vllm_ascend/worker/v2/spec_decode/rejection_sampler_utils.py` |
[vllm#53987](vllm-project/vllm#53987) adds the
FLy entropy-gated verification mode; upstream also threads the
watermarking kwargs | Accept both kwarg groups, but raise
`NotImplementedError` for FLy and watermarking on NPU |
| `vllm_ascend/worker/worker.py` |
[vllm#52921](vllm-project/vllm#52921) adds the
`get_simple_cpu_offload_num_cpu_blocks` RPC to the worker | Implement it
(main only) via
`_find_simple_cpu_offload_connector().get_aligned_num_cpu_blocks()` |

### Does this PR introduce _any_ user-facing change?

### How was this patch tested?

---

### Checklist

#### Testing

- [ ] **Boundary / edge-case coverage**: I reviewed whether this change
involves
boundary conditions (e.g. empty input, single/zero element, min/max
values,
off-by-one, sequence-length limits, integer overflow). Where applicable,
I
added unit tests covering these edge cases in `tests/ut/` or
`tests/e2e/`.
- [ ] **Regression test**: If this PR fixes a bug, I added a regression
test
  that would fail without the fix.
- [ ] **Error / failure path coverage**: I covered abnormal paths
(exceptions,
timeouts, out-of-range, division by zero) in addition to the happy path.
- [ ] **Meaningful & reliable tests**: New tests assert real behavior
and are
  not written merely to satisfy CI.
- [ ] **Performance impact**: If this PR touches a performance-sensitive
path,
  I attached benchmark results and added performance regression tests.


- vLLM main:
vllm-project/vllm@c0b6f74

---------

Signed-off-by: zhangxinyuehfad <starmoon_zhang@163.com>
Signed-off-by: hfadzxy <starmoon_zhang@163.com>
bantianyue pushed a commit to bantianyue/vllm that referenced this pull request Oct 28, 2026
…llm-project#49625)

Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
Signed-off-by: Artem Perevedentsev <aperevedents@nvidia.com>
Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
Co-authored-by: Artem Perevedentsev <aperevedents@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed scheduler structured-output v1

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: V1 structured outputs: a malformed grammar request after a valid one crashes EngineCore

3 participants