Gate the Kimi pre-tokenizer, which nothing was watching - #818
Open
ZacharyZcR wants to merge 1 commit into
Open
Conversation
o200k has had a gate since it was written: tests/test_tok_o200k.c, 40/40
encode + decode against a 4 KB in-repo fixture, no model download. The Kimi
family has had none. tests/test_tok_kimi.c needs a real tokenizer.json plus a
cases.bin generated by tiktoken, so it has no build rule and has never run --
c/Makefile lists it among the files that "deliberately have no rule".
The two families are not independent code. tok.h's o2_letters_masked() and
pretok_chunk_o2fam() serve both from the same lines, separated by a flag, so a
change made for o200k reshapes Kimi silently and only the o200k half had
anything watching. (That sharing is recent -- and when it landed, the Kimi
side had to be checked by hand-written cross-checks precisely because no gate
existed.)
This adds the fixture o200k already had, for Kimi: tests/tok_kimi_tiny.json,
the same tiny vocabulary with the Kimi pre-tokenizer regex, which is what
selects the family (tok.h keys on \p{Han} being in the pattern).
What it asserts is the pre-tokenizer BOUNDARY, not ids from the real Kimi
vocabulary -- reproducing those needs the actual tokenizer, which is exactly
what made the existing test unrunnable. The boundary is a property of the
rules and is what the shared code decides:
- a Han run is its own chunk 中文 -> 1 id (a vocab entry)
- Han does not absorb adjacent Latin 中文abc -> 3 ids
- a Han codepoint at the range edge 龥a -> 2 ids
still splits from Latin
The fixture makes those observable: "中文" is one vocabulary entry, so a
correct split encodes it as ONE id and any other split falls back to bytes.
Two of the assertions are differential -- the same input must NOT encode the
same through the o200k fixture. Without them the file would pass vacuously the
moment the Kimi flag stopped reaching the splitter, which is the failure most
worth catching.
Verified in both directions. All checks pass; routing pretok_chunk_kimi
through pretok_chunk_o200k on purpose fails three of them, including the
differential one. make test-c picks the new binary up automatically -- gates
are derived from build rules, so no shared list to edit.
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.
The gap
o200khas had a gate since it was written —tests/test_tok_o200k.c, 40/40 encode + decode against a 4 KB in-repo fixture, no model download.The Kimi family has had none.
tests/test_tok_kimi.cneeds a realtokenizer.jsonplus acases.bingenerated by tiktoken, so it has no build rule and has never run.c/Makefilelists it among the files that "deliberately have no rule".That matters because the two families are not independent code.
tok.h'so2_letters_masked()andpretok_chunk_o2fam()serve both from the same lines, separated by a flag — a change made for o200k reshapes Kimi silently, and only the o200k half had anything watching.What this adds
The fixture o200k already had, for Kimi:
tests/tok_kimi_tiny.json— the same tiny vocabulary carrying the Kimi pre-tokenizer regex, which is what selects the family (tok.hkeys on\p{Han}appearing in the pattern).It asserts the pre-tokenizer boundary, not ids from the real Kimi vocabulary. Reproducing those needs the actual tokenizer, which is precisely what made the existing test unrunnable. The boundary is a property of the rules, and it is what the shared code decides:
中文中文abc龥aThe fixture makes those observable:
中文is one vocabulary entry, so a correct Han-run split encodes it as one id and any other split falls back to per-byte ids.Two assertions are differential — the same input must not encode identically through the o200k fixture. Without them the file would pass vacuously the moment the Kimi flag stopped reaching the splitter, which is the failure most worth catching.
Verified both directions
Routing
pretok_chunk_kimithroughpretok_chunk_o200kon purpose fails three of them, including a differential one — so the gate is not vacuous.make test-cpicks the new binary up automatically: gates are derived from build rules, so there is no shared list to edit and no line for two PRs to conflict on.Note on scope
This does not replace
tests/test_tok_kimi.c— that one still cross-checks the full BPE against tiktoken on a real vocabulary, which is a stronger claim than this makes and still worth running by hand when a real model is available. This covers the part that can run on every commit, for free.