Repository navigation
fix: preserve Unicode character class ranges in FSMs - #904
Closed
Seven-Streams wants to merge 1 commit into
Closed
Seven-Streams wants to merge 1 commit into
Seven-Streams wants to merge 1 commit into
Conversation
Extract the Unicode range fixes from PR mlc-ai#901 by izhuhaoran. Complement negated classes in codepoint space, split UTF-8 widths, and fix the three-byte upper-tail bound. Update FSM snapshots and tokenizer masks for the newly rejected invalid leading bytes.
Seven-Streams
requested review from
DarkSharpness and
Ubospica
as code owners
September 18, 2026 15:59
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The regex FSM path still admits overlong UTF-8 sequences because it bypasses the new codepoint-width splitting.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR corrects Unicode character-class FSM construction by preserving codepoint ranges, enforcing canonical UTF-8 boundaries, and fixing three-byte upper-tail handling.
Changes:
- Reworked positive and negated character-class range generation.
- Corrected UTF-8 range boundary construction.
- Updated regression tests and serialized FSM/token-mask snapshots.
File summaries
| File | Description |
|---|---|
cpp/grammar_functor.cc |
Codepoint-based character-class handling |
cpp/fsm_builder.cc |
UTF-8 boundary correction |
tests/cpp/test_fsm_builder.cc |
FSM snapshot updates |
tests/python/test_grammar_fsm_builder.py |
Unicode regression coverage |
tests/python/test_grammar_matcher_json.py |
Token-mask snapshot updates |
tests/python/test_grammar_matcher_json_schema.py |
Cache test snapshot updates |
tests/python/test_serialization.py |
Serialization snapshot updates |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| int tmp_state_max = fsm.AddState(); | ||
| fsm.AddEdge(from, tmp_state_max, byte_max[2], byte_max[2]); | ||
| AddSameLengthCharacterRange(fsm, tmp_state_max, to, 0x0080, (max & 0x00FFFF)); | ||
| AddSameLengthCharacterRange(fsm, tmp_state_max, to, 0x008080, (max & 0x00FFFF)); |
Seven-Streams
marked this pull request as draft
September 18, 2026 16:03
Seven-Streams
added a commit
that referenced
this pull request
Sep 20, 2026
Extracted from #901 by @izhuhaoran, second piece after #906. The branch keeps the original commit and its revert at the bottom for attribution; the net diff against `main` is the `fix(fsm): preserve Unicode character class ranges` commit. Same change as the closed #904, rebased. Character-class FSMs mishandled Unicode in three places: - Negated classes truncated codepoint endpoints to bytes, complemented over an ASCII bitset, and then accepted every multi-byte sequence. `[^\u0000-\U0010FFFF]` accepted all non-ASCII characters and non-ASCII endpoints were dropped with a "clamping to 127" warning (#848). - Positive ranges crossing UTF-8 widths were encoded in one packed-byte call, which admits overlong encodings such as `C0 80`. - The three-byte upper-tail path used lower bound `0x80` instead of `0x8080`, so `[一-龥]` rejected every character with lead byte `E9` (`道`, `部`, `金`, ...). Fix: complement negated classes in codepoint space, split every range at the UTF-8 width boundaries so only canonical encodings are accepted, and fix the tail bound. FSM snapshots grow because canonical UTF-8 needs more states. The `hf_token_required` token-mask goldens change in two ways, verified by diffing per-position masks against `main`: JSON string positions reject exactly 5 more single-byte tokens (`C0 C1 F5 F6 F7`), and the `[a-zа-я一-龥]+` test accepts 78 more CJK tokens that the tail bug had rejected. Nothing else changes. Overlaps #849, which fixes only the negated-class complement and still admits overlong encodings. Closes #848. Validation: 11 new regression tests that all fail on `main`; `xgrammar_test` 77/77; full `tests/python` with `HF_TOKEN` 4764 passed. Performance was measured in #904 on this same diff: a few percent more compile time and mask latency on JSON, +12.6% mask latency on a 1024-char bounded Kimi XML string, from the extra states. This is a correctness trade-off. --------- Signed-off-by: zhuhaoran <zhuhaoran.zhr@alibaba-inc.com> Signed-off-by: yuchuan <yuchuan.7streams@gmail.com> Co-authored-by: zhuhaoran <zhuhaoran.zhr@alibaba-inc.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Character-class FSMs truncated negated Unicode endpoints to bytes and used noncanonical UTF-8 boundaries when a range crossed encoding widths. For example,
[^\u0000-\U0010FFFF]incorrectly accepted non-ASCII characters. Compute complements in codepoint space, split ranges by UTF-8 width, and correct the two-continuation-byte lower bound in the three-byte upper-tail path.This extracts the Unicode fixes from #901 by @izhuhaoran onto main at 1de4247. Production changes are limited to cpp/grammar_functor.cc and cpp/fsm_builder.cc; the other five changed files are regression tests and affected snapshots.
Validation
Performance
Same GCC 13.3 Release builds with -O3 and LTO on a Ryzen Threadripper 3970X; Llama-3.2-1B-Instruct tokenizer, 8 compiler threads, compiler cache disabled, CPU affinity 0–7. Three paired rounds in base/fixed, fixed/base, base/fixed order. Each process runs one warmup and three measured iterations per case; numbers below are medians of nine samples. Tokenization and compiler construction are outside compilation timing. Mask latency measures fill_next_token_bitmask only.
The bounded cases each use one required string with maxLength 1024 and mixed ASCII/Chinese/emoji content. The Kimi XML case uses the upstream behavior without string exclusions. Each mask sample replays the output 30 times (3 for bounded Kimi XML); every replay is validated. Memory is CompiledGrammar.memory_size_bytes, not process RSS.
The stricter UTF-8 automata require additional states. The measured bounded-string compilation, memory, and Kimi mask overheads are retained as a correctness tradeoff and are disclosed above; this PR does not claim a performance improvement. These measurements cover the listed workloads on one host.