Skip to content

Conversation

@ExtReMLapin
Copy link
Contributor

@ExtReMLapin ExtReMLapin commented Oct 21, 2025

Purpose

Fixing string formats when using xgrammar, xgrammar is required when we want to prevent infinite generation (reasoning) with --structured-outputs-config "{\"disable_any_whitespace\": true, \"backend\": \"xgrammar\"}"

Source of the supported formats :
https://github.com/mlc-ai/xgrammar/blob/a32ac892676d2eedc0327416105b9b06edfb94b2/cpp/json_schema_converter.cc#L2992

Test Plan

updated tests to not deny 'email' format (it's supported) and to check if a non existing string is still denied.

Test Result

/opt/vllm$ CUDA_VISIBLE_DEVICES=-1 pytest vllm_last/tests/v1/structured_output/test_utils.py 
================================================================================= test session starts =================================================================================
platform linux -- Python 3.10.12, pytest-8.4.2, pluggy-1.6.0
rootdir: /opt/vllm/vllm_last
configfile: pyproject.toml
plugins: anyio-4.11.0
collected 6 items                                                                                                                                                                     

vllm_last/tests/v1/structured_output/test_utils.py ......                                                                                                                       [100%]

================================================================================== warnings summary ===================================================================================
<frozen importlib._bootstrap>:241
  <frozen importlib._bootstrap>:241: DeprecationWarning: builtin type SwigPyPacked has no __module__ attribute

<frozen importlib._bootstrap>:241
  <frozen importlib._bootstrap>:241: DeprecationWarning: builtin type SwigPyObject has no __module__ attribute

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================================================ 6 passed, 2 warnings in 1.27s ============================================================================
sys:1: DeprecationWarning: builtin type swigvarlink has no __module__ attribute

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
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

CNE Pierre FICHEPOIL added 2 commits October 21, 2025 07:08
Signed-off-by: CNE Pierre FICHEPOIL <[email protected]>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the xgrammar backend to correctly handle supported string formats, which were previously being denied. The changes involve adding a list of supported formats and modifying the validation logic to check against this list. The tests have also been updated to reflect this change. My review focuses on improving the maintainability of the new code by adhering to Python's PEP 8 style guide for constants and by using a permanent link for the source reference. These changes will make the code more robust and easier to maintain in the future.

ExtReMLapin and others added 2 commits October 21, 2025 09:48
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: ExtReMLapin <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: ExtReMLapin <[email protected]>
@ExtReMLapin
Copy link
Contributor Author

cc main contributors of the backend :

@russellb @benchislett @shen-shanshan

@shen-shanshan
Copy link
Contributor

LGTM if the test passed.

Copy link
Member

@russellb russellb left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@russellb russellb enabled auto-merge (squash) October 21, 2025 15:20
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 21, 2025
@russellb russellb merged commit 4a8a567 into vllm-project:main Oct 21, 2025
45 checks passed
baonudesifeizhai pushed a commit to baonudesifeizhai/vllm that referenced this pull request Oct 21, 2025
…roject#27253)

Signed-off-by: CNE Pierre FICHEPOIL <[email protected]>
Signed-off-by: ExtReMLapin <[email protected]>
Co-authored-by: CNE Pierre FICHEPOIL <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
sstamenk pushed a commit to sstamenk/vllm that referenced this pull request Oct 23, 2025
…roject#27253)

Signed-off-by: CNE Pierre FICHEPOIL <[email protected]>
Signed-off-by: ExtReMLapin <[email protected]>
Co-authored-by: CNE Pierre FICHEPOIL <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: sstamenk <[email protected]>
usberkeley pushed a commit to usberkeley/vllm that referenced this pull request Oct 23, 2025
…roject#27253)

Signed-off-by: CNE Pierre FICHEPOIL <[email protected]>
Signed-off-by: ExtReMLapin <[email protected]>
Co-authored-by: CNE Pierre FICHEPOIL <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
albertoperdomo2 pushed a commit to albertoperdomo2/vllm that referenced this pull request Oct 23, 2025
…roject#27253)

Signed-off-by: CNE Pierre FICHEPOIL <[email protected]>
Signed-off-by: ExtReMLapin <[email protected]>
Co-authored-by: CNE Pierre FICHEPOIL <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Alberto Perdomo <[email protected]>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
…roject#27253)

Signed-off-by: CNE Pierre FICHEPOIL <[email protected]>
Signed-off-by: ExtReMLapin <[email protected]>
Co-authored-by: CNE Pierre FICHEPOIL <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: 0xrushi <[email protected]>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
…roject#27253)

Signed-off-by: CNE Pierre FICHEPOIL <[email protected]>
Signed-off-by: ExtReMLapin <[email protected]>
Co-authored-by: CNE Pierre FICHEPOIL <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: 0xrushi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed structured-output v1

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants