Skip to content

[Bugfix] Stop XGrammar at terminal tokens - #2

Closed
chubby-cutie wants to merge 1 commit into
wangyxbh:fixed-bug-xgrammar_structured_output_crashfrom
chubby-cutie:agent/xgrammar-stop-at-termination
Closed

chubby-cutie wants to merge 1 commit into
wangyxbh:fixed-bug-xgrammar_structured_output_crashfrom
chubby-cutie:agent/xgrammar-stop-at-termination

Conversation

@chubby-cutie

Copy link
Copy Markdown

Summary

  • stop XGrammar token acceptance as soon as the matcher terminates
  • stop speculative validation at a terminal token and roll back only accepted tokens
  • reset the cached termination state together with the matcher
  • add regression coverage for accepted and validation batches

This is a follow-up to vllm-project#37506 and is based on commit 9228d56.

Why

A speculative batch may contain EOS followed by trailing draft tokens. Once XGrammar accepts EOS, its matcher is terminal and must not receive another token. Continuing past EOS emits matcher warnings and can leave vLLM's cached state inconsistent.

Validation

  • tests/v1/spec_decode/test_mtp_structured_output.py: 18 passed
  • Ruff check and format passed
  • git diff --check passed
  • DeepSeek-V4-Flash with DSpark and tool calling: 9,344/9,344 successful requests during a 10-minute concurrent test
  • no target XGrammar/FSM warnings or errors

Avoid advancing XGrammar matchers past a terminal token in accepted and speculative validation batches. Reset the cached termination state alongside the matcher.

Co-authored-by: OpenAI Codex <noreply@openai.com>
Signed-off-by: chubby-cutie <chubby-cutie@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors.

You ask your reviewers to trigger select CI tests on top of fastcheck CI.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

🚀

@chubby-cutie

Copy link
Copy Markdown
Author

@wangyxbh Could you please review this follow-up fix and merge it into fixed-bug-xgrammar_structured_output_crash if it looks good?

The focused tests, Ruff checks, and a 10-minute DSpark + tool-calling stress test have passed. The remaining workflow requires maintainer approval.

@gaby

gaby commented Aug 19, 2026

Copy link
Copy Markdown

@chubby-cutie I think this is fixed by vllm-project#52805

@chubby-cutie

Copy link
Copy Markdown
Author

Thanks for pointing this out. I confirmed that vllm-project#52805 covers the same termination-state synchronization and speculative token-batch handling. I'll close this PR as superseded.

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.

2 participants