fix(tokenizer): port the real GPT-4o pre-tokenizer, and stop silently selecting Llama-3 - #363
Merged
Conversation
…pre) Muse Glimmer's GGUF k-quant declared `tokenizer.ggml.pre = "llama4"` and `Tokenizer::FromGguf` refused it by name, so the ~17 GB arm -- the only artifact our engine and llama.cpp can both hold -- had no e2e path at all (#347, blocking #333). Adds `SplitPattern::kGpt4o` and accepts the four pre names llama.cpp maps to `LLAMA_VOCAB_PRE_TYPE_GPT4O`: `gpt-4o`, `llama4`, `kanana2`, `talkie` (llama.cpp src/llama-vocab.cpp:2294-2299 @ 153d324bcf). NOT AN ALIAS. The regex is transcribed verbatim from the checkpoint's own tokenizer.json, which is byte-equal to the string llama.cpp records as "original regex from tokenizer.json" (llama-vocab.cpp:432). It differs from kLlama3 in three ways that all change ids: one letter alternative becomes two over the MINOR letter categories (so CamelCase splits), the contraction is a word SUFFIX rather than a leading alternative, and the punctuation run absorbs a trailing `/`. llama.cpp's own transcription (llama-vocab.cpp:433) is an approximation -- its engine cannot spell \p{Lu}/\p{Ll}, so it drops \p{M} and puts every non-ASCII cased letter in both classes. We port the original, and that choice is measured: on the 57-entry corpus through the 16.76 GB k-quant, ours has 0 mismatches against HF `tokenizers` and llama.cpp 153d324bcf has 4 (Devanagari, Thai, Arabic). A SECOND, SILENT INSTANCE of the same bug is fixed here too. `DetectPattern` returned kLlama3 for ANY regex containing `\p{N}{1,3}`; the GPT-4o regex contains it, so `FromHfJson` on Muse Glimmer's own tokenizer.json was already selecting kLlama3 with no error -- the bf16 safetensors arm has been mistokenizing since the model landed. Now matched exactly, before the heuristic. `ignore_merges` (HF sets it, GGUF cannot express it) is MEASURED, not assumed: 29704 distinct vocab-derived strings plus a 320-string corpus encode byte-identically with the flag on and off. Gates. RED first, on the real artifact and on both new gates: * `vllm-bench` on the k-quant: `unsupported tokenizer.ggml.pre "llama4"`. * A: kGpt4o aliased to kLlama3 -> test_pretokenizer 5 cases RED * B: exact GPT-4o regex match removed -> parity_gpt4o 2 cases / 15 asserts * C: `/` dropped from the punct tail -> pretokenizer 2, parity 1 * D: class B widened to class A -> pretokenizer 4 * E: contraction suffix dropped -> pretokenizer 2, parity 1 * F: kGpt4o aliased to kQwen2 -> pretokenizer 6, parity 1 * G: the `A*` backtrack disabled -> pretokenizer 2 Green after, restored byte-for-byte from an in-memory snapshot. Existing patterns are inert: all 108 pre-existing golden rows keep byte-identical qwen and llama columns, and the regenerated unicode tables are byte-identical except for the added letter-subcategory table. Live differential, `examples/tokenize` vs `tools/parity/verify_tokenizer_gguf.py` over the committed corpus, all 0 diffs: the 16.76 GB GGUF, the checkpoint's full 28 MB tokenizer.json, and the committed fixture. The fixture is the checkpoint's tokenizer.json with its 439802-entry merge list cut to the 3791 merges this corpus can reach (vocabulary kept whole so added tokens keep their ids); the generator proves the cut faithful by re-encoding the whole corpus with it before writing. FOUND, NOT FIXED: with the tokenizer working the k-quant now runs a forward and emits degenerate text (" is is is ...") where llama.cpp on the SAME file emits "Paris. It is the most populous city in France and". Loader gates are 12/12 (1064 assertions fully materialised) and our CPU backend generates coherently for opt-125m, so the defect is in the GGUF forward. Filed as #359 rather than folded in here; #333 stays blocked for that new reason. No speed claim is made on any axis. Issue: #347 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
Brings in ENG-RELEASE-CONTAINERS, the PromRegistry reference fix (#330) and the in-flow incidental-bug policy (#358). One conflict, in the docs/STATUS.md character ratchet in scripts/check-public-doc-tables.py: both sides appended a ratchet entry. Taken main's entry wholesale and re-measured mine on top of the merged page (243243, still strictly DOWN from main's 243245), per the keyed-record rule. Re-gated after the merge on a clean rebuild: test_pretokenizer 25/25, test_tokenizer_parity_gpt4o 5/5, test_unicode_data 10/10, test_bpe 18/18, test_tokenizer_parity 4/4, test_tokenizer_parity_deepseek 6/6, test_tokenizer_parity_mistral 6/6, test_detokenizer 12/12, test_tokenizer_metaspace_split 7/7, test_gguf 33/33; preflight --staged all gates green. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
This change lowered the STATUS char ratchet again (net -2, re-measured after merging main rather than carried over from before it), which is a checker semantic change and the size gate refused it without evidence. The direction test already on main says a re-pin may only go DOWN. This says it must land exactly ON the page: pinned == live, zero slack. A ratchet re-pinned to a guessed round number leaves headroom, and headroom is where a page regrows without the gate noticing. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
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.
Closes #347.
Muse Glimmer's GGUF arm died at
unsupported tokenizer.ggml.pre "llama4". Fixing that turned up a second, worse bug that was already live.Two bugs
1.
llama4was unsupported. llama.cpp maps it toLLAMA_VOCAB_PRE_TYPE_GPT4O. We now implementSplitPattern::kGpt4oand accept the four pre names that share it (gpt-4o,llama4,kanana2,talkie).2.
DetectPatternwas silently selectingkLlama3for this model. It returnedkLlama3for any regex containing\p{N}{1,3}— and the GPT-4o regex contains it. SoTokenizer::FromHfJsonon Muse Glimmer's owntokenizer.jsonpicked the wrong pre-tokenizer with no error: the bf16 safetensors arm has been mistokenizing since the model landed. Now matched exactly, before the heuristic.We port the original regex, not llama.cpp's approximation — and it measurably wins
llama.cpp records the true pattern in a comment (
llama-vocab.cpp:432) then implements an approximation at:433, because its engine cannot spell\p{Lu}/\p{Ll}: it uses((?=[\p{L}])([^a-z]))/([^A-Z]), which drops\p{M}and puts every non-ASCII cased letter in both classes. The:432string is byte-equal to this checkpoint'stokenizer.json, so that is what we implement.Same GGUF, same 57-entry corpus, against HF
tokenizers0.22.2:153d324bcfStructurally it is not "Llama-3 with different digits": one letter alternative becomes two over the minor categories (
A* B+|A+ B*, splitting CamelCase), the contraction is a word suffix, and the punct run absorbs a trailing/. Needed a new generatedLetterSubtable (Ll / Lu+Lt / Lm+Lo).Evidence
RED captured first on the real artifact (
vllm-bench→unsupported ... "llama4"). Seven mutations, all RED, tree restored byte-for-byte from an in-memory snapshot each time:aliasing
kGpt4o→kLlama3(the named trap) · removing the exact match fromDetectPattern· dropping/from the punct tail · widening class B to class A · dropping the contraction suffix · aliasing→kQwen2· disabling theA*backtrack.14 of 57 corpus entries change ids if
kGpt4ois swapped forkLlama3orkQwen2. Also measured:ignore_mergesis a no-op for this vocab (29,704 vocab-derived strings + 320-string corpus, byte-identical both ways).Gates:
test_pretokenizer147,221 assertions,test_tokenizer_parity_gpt4o1,000,test_bpe905,test_tokenizer_parity1,175, plus deepseek/mistral/detokenizer/metaspace/gguf — all green, all 108 pre-existing golden rows byte-identical.Fixture
tests/parity/goldens/tokenizer_muse_glimmer/keeps the vocabulary whole and cuts only the merge list (trimming the vocab silently renumbers every<|...|>token — measured:<|begin_of_text|>moved 200000 → 2104). No GGUF and no network in CI.What this does NOT fix
The GGUF now tokenizes correctly and runs a forward, but emits degenerate output (
" is is is…") where llama.cpp on the same file produces "Paris. It is the most populous city in France and". That is a separate defect in our GGUF forward, filed as #359, and it is why the k-quant still cannot generate usable text.No speed claim on any axis.
🤖 Generated with Claude Code