Repository navigation
Conversation
GitHub Actions
Inactive
GitHub Actions
Inactive
|
👋 Hi bjck! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
github-actions
Bot
added
external-contribution
GitHub Actions
Inactive
…l guidance
Under schema mode auto, structural tags enforce the argument schema of a
tool that omits strict. A pattern XGrammar cannot compile (lookahead,
lookbehind, backreferences, group modifiers, \b, \p{..}, empty classes,
malformed or oversized repetitions, possessive quantifiers) made the
backend reject the whole request.
Leave such patterns out of the grammar for omitted-strict tools and keep
the rest of the schema enforced. An unsupported patternProperties key
falls back to strict: false, since dropping it would change which names
are allowed. Explicit strict and schema mode strict are unchanged.
Applied in the Rust preprocessor and in the Python vLLM and SGLang
frontend paths.
Part of ai-dynamo#15687
Signed-off-by: Bjarne Kock <bjarnekock@hotmail.com>
Keep only why-comments in code; measurements and issue history live in the PR description. Signed-off-by: Bjarne Kock <bjarnekock@hotmail.com>
967dc59 to
0cac921
Compare
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Detect unsupported regex and schema constructs components/src/dynamo/frontend/structural_tag_policy.py, lib/llm/src/preprocessor/structural_tag.rs, components/src/dynamo/frontend/tests/test_structural_tag_policy.py |
Python and Rust code scan regex patterns and nested schema keywords for unsupported constructs. Tests cover supported and unsupported syntax and schema traversal. |
Adapt schemas according to strictness components/src/dynamo/frontend/structural_tag_policy.py, lib/llm/src/preprocessor/structural_tag.rs, components/src/dynamo/frontend/tests/test_structural_tag_policy.py |
Python and Rust code remove unsupported pattern constraints for eligible tools. If a patternProperties key is unsupported, they disable enforcement instead. Tests cover adaptation rules and input preservation. |
Apply policy in structural-tag paths components/src/dynamo/frontend/prepost.py, components/src/dynamo/frontend/sglang_prepost.py, lib/llm/src/preprocessor/structural_tag.rs, components/src/dynamo/frontend/tests/test_sglang_processor_unit.py, lib/llm/src/preprocessor/structural_tag.rs, docs/fern/pages/use-cases/tool-calling-and-reasoning/structural-tag.md |
The Python frontend and Rust tool-choice paths pass adapted schemas into structural-tag processing. Tests and documentation cover the behavior, including Kimi choice cases. |
Estimated code review effort: 4 (Complex) | ~45 minutes
Merge Risk: 
ae10c
-
No identified issue blocks merging after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title names the main change: uncompilable regex patterns are left out of automatic tool guidance. It follows the fix(frontend) convention and is specific. It is 79 characters, which is within a no… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description follows the repository template closely. It has Overview, Key Changes, Validation, and Issues and Ongoing Work sections, plus the checklist item marked as done. The Overview states the… Full details: Title check
-
Explanation
The title names the main change: uncompilable regex patterns are left out of automatic tool guidance. It follows the fix(frontend) convention and is specific. It is 79 characters, which is within a normal title length. The title does not mention the Rust and Python paths, but that detail is not needed.
Full details: Description check
-
Explanation
The description follows the repository template closely. It has Overview, Key Changes, Validation, and Issues and Ongoing Work sections, plus the checklist item marked as done. The Overview states the problem and the fix: an XGrammar-incompatible tool schema
patternmakes the backend reject the whole request, and the PR leaves that pattern out while keeping the rest of the schema enforced. Key Changes lists the Rust and Python files and what each changes. Validation gives a root cause, a before/after table covering the affected cases, the tests added, and the local runs (Python 96 passed, cargo 59 passed). It states that end-to-end runs on vLLM, SGLang, and TensorRT-LLM were not done, and that the SGLang call-site test awaits its first CI run. Issues and Ongoing Work links the work to#15687and explains the scope limits. The template's 'Closes#XXXX' line is absent. The PR references#15687as 'Part of', which is not a closing link. The template comment says the PR Issue Link check is advisory today. Because the check is advisory and the PR links an issue, this gap is minor.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 21 functions across 6 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
The Python frontend runs next to vLLM or SGLang, which ship XGrammar, so compile each pattern with Grammar.from_regex instead of mirroring its regex syntax. The verdict now follows the installed version. The Rust preprocessor has no XGrammar and keeps its list. The shared test table now checks that list against the compiler. Part of ai-dynamo#15687 Signed-off-by: Bjarne Kock <bjarnekock@hotmail.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
Overview
A tool schema with a
patternXGrammar cannot compile, such as the lookahead in #15687, makes the backend reject the whole request once structural tags enforce it. With this PR that pattern is left out of the grammar, and the rest of the schema and the tool call stay enforced.Key Changes
About 390 lines of core logic: 259 Rust, 129 Python. The Rust and Python frontends build tags on separate paths. The Python frontend asks the installed XGrammar whether a pattern compiles. The Rust preprocessor has no XGrammar, so it checks a list of constructs, and the Python test checks that list against the compiler.
structural_tag.rs:unsupported_regex_constructandadapt_unsupported_auto_schemas. Only omitted-strict tools under schema modeautochange. Explicitstrictand schema modestrictstill reach the backend.structural_tag_policy.py: the same rule inguidance_tool_schema, used by the vLLM and SGLang preprocessors.unsupported_regex_constructcompiles each pattern withxgrammar.Grammar.from_regex, so the verdict follows the version the backend ships with.structural-tag.md: lists the checked constructs and replaces the limitation documented in feat: Enable structural tags by default and improve Kimi K3 guidance #13654.Validation
Cause:
to_v2_toolmaps omittedstricttotrue, so the tag carries the client's regex into XGrammar. Leaving the pattern out removes the only part XGrammar rejects. An unsupportedpatternPropertieskey cannot be left out without changing which names are allowed, so that tool falls back tostrict: false.Dynamo's tags from dynamo-parsers-v2 0.7.35, compiled with XGrammar 0.2.7, reproducer schema:
strictomittedLookahead is not supported yetstrictomittedstrict: trueor schema modestrict^[a-z]+$patternPropertieskeyThe Rust detector flags what any 0.2.7 tag format rejects: lookahead, lookbehind, backreferences, group modifiers,
\b,\p{..},[], malformed{..}, counts above 10^15 and possessive quantifiers. Fuzzed against the compiler on the qwen_xml path: 31.9k patterns, zero false flags, zero misses.Key tests:
unsupported_regex_constructs_match_the_grammar_compiler(Rust, Python): one shared table. Rust checks its detector against it, Python checks it against the installed XGrammar, so the Rust list drifting from the compiler fails a test.automatic_argument_enforcement_leaves_out_patterns_the_backend_cannot_compile: relaxing too much or too little across strict omitted/true/false and schema mode strict. Fails without the fix.kimi_k3_named_tag_leaves_out_patterns_the_backend_cannot_compile_when_global_mode_is_off: the K3 named tag the opt-out keeps. Fails without the fix.client_sized_repetition_counts_are_rejected_without_parsing(Rust): a crash on huge{n}counts.TestConvertTools::test_guidance_copy_leaves_out_patterns_the_backend_cannot_compile: the SGLang call site. It needssglangand the compiled bindings, so CI is its first run.Run locally: Python policy tests with XGrammar 0.2.7, 96 passed.
cargo test -p dynamo-llm --no-default-features --lib structural_tag, 59 passed. Not run end to end on vLLM, SGLang or TensorRT-LLM.Cost: one scan per omitted-strict tool schema, only when a tag is built under schema mode
auto. The Rust scan does not allocate. In Python each pattern costs oneGrammar.from_regexcall, under 0.1 ms locally. Tools are copied only when a pattern is left out.Issues and Ongoing Work
strict: falsefor the whole tool. Review showed that dropstype,requiredandenum, so this leaves only the pattern out.patterncase upstream. Its JSON and Qwen XML converters ignore a pattern they cannot parse, the same policy as this PR. Once it ships and Dynamo's XGrammar is bumped, backends stop rejecting these schemas on their own, and this check only matters for older pinned versions. It does not coverpatternPropertieskeys.I had AI help with the code, the probes and the fuzzing. I have read and validated the change.
Pull Request Checklist
/pr-checklistskill.Summary by CodeRabbit
automode. For tools without an explicit strictness setting, unsupportedpatternconstraints are omitted; schemas with unsupportedpatternPropertiesuse non-strict enforcement.