Fix kimi linear tokenizer regex - #35
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Kimi Linear’s pretokenizer regex compatibility by extending the Oniguruma→JavaScript regex translation to support balanced character-class intersections (&&) (including nested, chained, negated, and POSIX operands), and updating tests/fixtures/docs accordingly.
Changes:
- Implement structural parsing/rewriting of Oniguruma character classes, including
&&intersections and common POSIX bracket expressions, in the regex translator. - Expand edge-case coverage and fixture corpus to validate new intersection behavior (quantifier scoping, POSIX operands, nested classes, malformed constructs).
- Update README to document the newly supported constructs and the intentionally rejected/unsupported ones.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/fixtures/splitRegexPatterns.json | Removes “known divergence” flags for now-supported patterns and adds new fixture cases covering intersections/POSIX/ranges. |
| tests/edgeCases.test.ts | Adds targeted tests for Kimi-shaped intersections and a broad set of intersection parsing/composition scenarios. |
| src/utils/core.ts | Reworks regex translation to structurally parse character classes and compile intersections/unions safely in JS u mode; improves Unicode property fallback handling. |
| README.md | Updates compatibility documentation to reflect intersection/POSIX support and the specific unsupported/rejected constructs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The overlapping union must be a membership assertion followed by one consumer.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tests/fixtures/splitRegexPatterns.json:638
- New split-regex fixture cases were added/expanded, but the committed oracle file (tests/fixtures/splitRegexOracle.json.gz) is not updated in this PR. splitRegexOracle.test.ts asserts that oracle.tokens has an entry for every case name, so adding these patterns will make the oracle completeness test fail unless you regenerate and commit the updated gzip oracle.
{
"id": "edge-class-intersection-kimi",
"pattern": "[\\p{L}\\p{M}&&[^\\p{Han}]]+",
"inputs": ["Aé\u0301汉B字 שלום", "A𐐀𠀀B"]
},
nico-martin
left a comment
There was a problem hiding this comment.
I'm not super good with regexes. So I ask GPT 5.6 Sol to do the review and after some back and forth there are two findings that should be adressed.
|
thanks @nico-martin 🙏 taking a look now. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
tests/fixtures/splitRegexPatterns.json:590
- New split-regex patterns were added/updated here, but
tests/splitRegexOracle.test.tsrequirestests/fixtures/splitRegexOracle.json.gzto contain expected tokens for every case. Without regenerating/committing the oracle, the suite will fail at "oracle has expected tokens for every case".
{
"id": "edge-posix-punct",
"pattern": "[[:punct:]]+",
"inputs": ["a!+$|§©™😀b"]
},
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Provides a fix for Kimi Linear's pretokenizer regex:
This wasn't caught by the checking script from #24 because it's under a PR (https://huggingface.co/moonshotai/Kimi-Linear-48B-A3B-Instruct/discussions/20) and not officially merged.