[StructuredOutput] Update XGrammar #2817
Conversation
apaniukov
requested review from
Wovchena,
as-suvorov,
Copilot,
pavel-esir and
yatarkan
github-actions
bot
added
category: sampling
There was a problem hiding this comment.
Pull Request Overview
This PR updates XGrammar to version 0.1.25, deprecating compound grammar and old structural tags APIs in favor of new structural tags that work similarly to compound grammar.
- Updates XGrammar from v0.1.18 to v0.1.25 for improved grammar compile time and new features
- Introduces new structural tags API with extended building blocks (ConstString, AnyText, QwenXMLParametersFormat, Tag, TriggeredTags, TagsWithSeparator)
- Migrates samples and tests from deprecated compound grammar to new structural tags API with deprecation warnings
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/python_tests/test_structured_output.py | Adds comprehensive tests for new structural tags API and renames old test function |
| src/python/py_utils.cpp | Updates Python bindings to handle new structural tag types and variant conversions |
| src/python/py_generation_config.cpp | Implements new structural tag classes with proper initialization and serialization |
| src/python/openvino_genai/py_openvino_genai.pyi | Updates type stubs for new structural tag classes and API changes |
| src/cpp/src/sampling/structured_output/xgrammar_backend.* | Refactors grammar parsing for new structural tag format and adds deprecation warnings |
| src/cpp/src/generation_config.cpp | Updates StructuredOutputConfig with new structural tag support and enhanced operators |
| src/cpp/include/openvino/genai/generation_config.hpp | Adds comprehensive structural tag types and JSON serialization methods |
| src/cpp/CMakeLists.txt | Updates XGrammar version dependency |
| samples/python/text_generation/*.py | Migrates samples to use new structural tags API instead of deprecated compound grammar |
Comments suppressed due to low confidence (1)
src/python/py_generation_config.cpp:1
- Corrected spacing in 'SOC.Concat(A, B, C).' - removed extra period at the end.
// Copyright (C) 2023-2025 Intel Corporation
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| std::visit([&error_message](const auto& err) { | ||
| if constexpr (std::is_member_function_pointer<decltype(&std::decay_t<decltype(err)>::what)>::value) { | ||
| error_message = err.what(); | ||
| } else { | ||
| error_message = "Unknown error type"; | ||
| } | ||
| }, error); |
There was a problem hiding this comment.
The error handling logic is complex and duplicated. Consider extracting this into a separate helper function to improve maintainability and reduce code duplication.
Copilot uses AI. Check for mistakes.
| } | ||
| }; | ||
|
|
||
| struct AnyText { |
There was a problem hiding this comment.
Is it possible to include AnyText into a sample? It seems like it's equivalent to not applying any schema, but why AnyText is needed in that case
There was a problem hiding this comment.
AnyText is useful as part of the bigger schema. For example, we can enforce thinking like this:
Tag(begin="<think>", content=AnyText(), end="</think>") + AnyText()
There was a problem hiding this comment.
Will you extend one of the samples? Seems to be harmless
There was a problem hiding this comment.
In the future, yes. Not in this PR.
| } | ||
| }; | ||
|
|
||
| struct QwenXMLParametersFormat { |
There was a problem hiding this comment.
Seem to be a niche thing. Can you add a description a a link to learn more to the docstrings
There was a problem hiding this comment.
After I add the documentation.
| }; | ||
| }; | ||
|
|
||
| struct TagsWithSeparator { |
There was a problem hiding this comment.
Would be nice to have a text describing big picture and relations between the structs. Sure it's possible to infer that info, but it requires more effort
There was a problem hiding this comment.
I will address it in the documentation.
The PR is backward compatible, so no need for an additional review. |
Pull Request is not mergeable
7bb6566
3 participants
Description
Update XGrammar to 0.1.25 version.
OVMS StructuralTags compilation benchmark results:
0.1.18: 2613.6 ms
0.1.19: 10242.3 ms
0.1.20: 15852.5 ms
0.1.21: 17536.1 ms
0.1.22: 205489.0 ms
0.1.23: 703.5 ms
0.1.24: 692.8 ms
0.1.25 86.4 ms
Ticket: CVS-173972
Checklist: