Skip to content

[Bugfix][Frontend] Forward parser grammar reasoning hint through Responses render - #62185

Open
dskwe wants to merge 2 commits into
vllm-project:mainfrom
dskwe:fix-responses-render-parser-grammar
Open

dskwe wants to merge 2 commits into
vllm-project:mainfrom
dskwe:fix-responses-render-parser-grammar

Conversation

@dskwe

@dskwe dskwe commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Preserve parser-derived Responses grammar gating across render → generate, matching coupled Responses serving introduced in #62052 and the existing transport from #60062.

Claims

  • When a parser grammar covers reasoning from the message start, Responses render emits reasoning_ended=true, so the existing generate consumer can apply it immediately.
  • Other requests retain reasoning_ended=null, preserving engine prompt-based reasoning resolution.

Validation

Main refresh (9c40166d92)

Merged upstream main 90c4a1a73eecea0428ee4303ad96c9fdeaac3f80, preserving the original feature commit. Resolved the overlap in Responses GenerateRequest construction by retaining both the parser grammar reasoning_ended hint and upstream's new effective-template-kwargs call. The aggregate diff against this main remains the original one-line production fix and regression tests.

Fresh isolated Linux CPU validation using the complete resolved source tree:

.venv/bin/python -m pytest tests/entrypoints/scale_out/render/test_render.py \
  -m skip_global_cleanup -q --disable-warnings --tb=short
# 14 passed, 27 deselected
.venv/bin/python -m pytest -p completion_offset_ci_assets \
  tests/parser/test_beam_parser.py \
  tests/entrypoints/scale_out/token_in_token_out/test_protocol.py \
  -q --disable-warnings --tb=short
# 119 passed

The second command's local plugin only bypasses GPU/distributed cleanup; it does not replace parser/protocol behavior. Removing only the Responses hint on this resolved tree makes the producer-driven regression fail (1 failed, 1 passed, None is True); restoring it passes. Patch-scoped pinned pre-commit, including mypy and CI test tethering, and git diff upstream/main --check pass. Full model-serving integration remains unverified: an attempted broader selection encountered 27 server-fixture setup errors because model snapshots are unavailable offline. Earlier HTTP/consumer evidence below was not rerun on this refresh.

Original submission validation (historical)

The submitter performed the human line-by-line review and ran the relevant tests. Native generation/model-output validation is unavailable; no generated-model correctness claim.

Base: 21e09ea95926fcfe4df1c57bf2eb95123cab70c3 (current main at submission). Commands use existing CPU Docker with explicit cwd/PYTHONPATH=/validation/responses-grammar-20261017 and .venv/bin/python; full harness, provenance hashes, and limitations in the private evidence record.

Re-validated on this base after duplicate-search refresh (target files unchanged between the original test base and this base):

  • Producer-driven regression test_render_responses_carries_parser_grammar_hint: real BeamParser (existing test-beam fixtures, no manual flag assignment) adjusts the request; real render_responses_request must emit reasoning_ended=True at message-start and None inside open reasoning. RED on baseline serving: 1 failed 1 passed 37 deselected (None is True on the message-start case); GREEN on fixed: 2 passed 37 deselected.
  • Fixed render-unit selection -m pytest tests/entrypoints/scale_out/render/test_render.py -m skip_global_cleanup -q --disable-warnings: 12 passed 27 deselected (the skipped marker excludes live-server fixtures requiring model downloads). Existing tests/parser/test_beam_parser.py: 84 passed. All applicable patch-scoped pinned pre-commit hooks including mypy passed.
  • Real BeamParser/fixture-tokenizer probe with stub rendering: baseline missing flag; fixed true at message start, null inside reasoning. Grammar-language checks confirm differing intended application boundaries; not inference/HTTP integration.
  • Normal render CLI/Uvicorn HTTP A/B (responses-grammar-http-v1.py): real render.entry startup with diagnostic OPT tokenizer carrying the Beam response_template and ASCII anchors; /v1/responses/render POST 200 for message-start and inside-reasoning cases. Baseline (serving sha 8d6e0796d9...): producer sets _grammar_from_parser=true but wire reasoning_ended stays null in both cases. Fixed (sha 96925f7dff...): wire reasoning_ended=true at message start, null inside reasoning; GenerateRequest JSON roundtrip preserves the value; xgrammar accepts full reasoning framing and rejects answer-only at message start on both variants (the wire hint, not the grammar, is the delta). All responses 200; servers exit 0.
  • Independent saved-artifact audit (audit-responses-grammar-http-v1.py): 38/38 checks pass — harness hashes, HTTP status, per-variant serving-module child import provenance (baseline/fixed SHAs exact, all other modules identical), one-line source delta recomputed from git, producer order/flags, wire null-vs-true behavior, roundtrip, grammar semantics, cross-variant prompt equality.
  • Consumer-capture run (responses-grammar-consumer-v1.py): the exact wire GenerateRequest JSON from the HTTP A/B artifacts was fed through real ServingTokens.serve_tokens (repo's own test fixtures, MagicMock(spec=AsyncLLM) engine — no inference). engine_client.generate received reasoning_ended=[null, null] on the baseline wire and [true, null] on the fixed wire, with reasoning_parser_kwargs threaded intact in all 4 cases — the serving→generate hop is executed, not just traced. Independent saved-artifact audit (audit-responses-grammar-consumer-v1.py): 18/18 checks pass.
  • Two additional independent reviews of the HTTP A/B and consumer-capture evidence found no blockers; reviewer re-ran both audits (exit 0), verified the exact one-line diff, hash chains, and that scope claims do not overstate. The checked-in test stays deliberately mocked at the engine boundary (real coverage lives in the harnesses described above).

Details

BeamParser stores a private _grammar_from_parser flag when its constraint includes reasoning framing. Coupled Responses converts it to the existing engine hint, but Responses render previously discarded it when building GenerateRequest. Forward the same expression at that serialization boundary; no new field or parser logic.

Related: #62108 (request-body log guard), #62114 (completion derender token IDs), #62163 (completion derender stream offsets) — independent frontend fixes validated with the same methodology.

AI assistance was used. Duplicate searches refreshed at submission (reasoning_ended render, grammar_from_parser, scale_out render, plus full-title scan of open PRs): #62095 applies stop strings after reasoning in chat completion serving (different endpoint, no render/grammar transport); #44084 binds grammar on the client-facing Responses API (entrypoints/openai/responses/, protocol+kwargs plumbing) — different layer, no file overlap; #54078 lets the reasoning parser decide reasoning_ended in chat completion serving — same symptom family, different pipeline; no open PR touches scale_out/render/serving.py grammar transport.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

…onses render

Signed-off-by: Yuan Li <dskwelmcy@163.com>

@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.

@mergify mergify Bot added frontend bug Something isn't working labels Oct 17, 2026
@mergify

mergify Bot commented Oct 21, 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, @dskwe.

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 21, 2026
Preserve the parser grammar reasoning hint alongside upstream's effective chat-template kwargs handling.

Co-authored-by: Hermes Agent
Signed-off-by: Yuan Li <dskwelmcy@163.com>
@dskwe

dskwe commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the main-branch conflict in 9c40166 by merging current main, preserving the original feature commit. The resolution keeps both the grammar-derived reasoning_ended hint and upstream's updated effective-template-kwargs handling.

Fresh validation: 14 render unit tests and 119 parser/protocol tests pass; the producer-driven regression still fails without the hint and passes with it. Patch-scoped pre-commit, including mypy, passes. Full model-serving integration was not verified locally; the PR description records that limitation and separates earlier HTTP evidence from these fresh runs.

@mergify mergify Bot removed the needs-rebase label Oct 21, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working frontend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant