Repository navigation
Conversation
OmarFaig
requested review from
DarkSharpness,
Seven-Streams and
Ubospica
as code owners
August 17, 2026 22:32
Collaborator
|
cc @Seven-Streams Looks good to support this |
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 branch has not been deployed
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.
BuildNegativeCharacterClass computed the complement of a negated character class ([^...]) over a 128-entry ASCII bitset, then unconditionally added an edge accepting every codepoint from kMin2BytesUnicode to kMax4BytesUnicode regardless of what the class excluded. So [^] correctly rejected the intended ASCII exclusions but admitted every non-ASCII codepoint outright, including ones the pattern explicitly excluded — the resulting token mask permitted tokens the regex said could never appear. The "byte greater than 127, clamping to 127" warning was a downstream symptom: the class endpoints are Unicode codepoints, but this function truncated them to uint8_t before the range check, mangling any non-ASCII endpoint before the clamp even fired.
The fix computes the complement over the full codepoint domain [0, 0x10FFFF] instead: collect the class's ranges, sort and merge them, then walk the gaps and emit an FSM edge for each, reusing the existing AddCharacterRange/CodepointToPackedUTF8 helpers the positive-class path already uses for UTF-8 packing. This mirrors the sort-merge-complement shape RepetitionNormalizerImpl::AppendPositiveRanges already uses elsewhere in this file. The clamp and its warning are gone.
New cases in test_grammar_matcher_regex.py cover range rejection, non-ASCII acceptance outside the excluded range, a positive-class regression guard, the ASCII/non-ASCII boundary, the top of the codepoint domain, and a token-mask-level test reproducing the original symptom directly. Two golden FSM/serialization snapshots embedding the old, over-permissive UTF-8 lead-byte range are updated to match.
Fixes #848.