From d5f5b72078907eab822368cc9dc15932af1e81c1 Mon Sep 17 00:00:00 2001 From: James Burton Date: Fri, 14 Aug 2026 22:54:32 +0100 Subject: [PATCH 1/2] fix(tokenizers): add qwen2 + qwen35 pre-tokenizer pipelines (#397, #373, #387) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `qwen2` and `qwen35` were absent from `TiktokenPreTokenizer`, so since the loud gate landed (8abf2281, #373) every Qwen2/Qwen3 GGUF threw at load, and before that they silently ran with NO pre-tokenization at all — BPE merges crossing digit boundaries the model was trained to respect (#237 measured tokenizer drift alone at +3.46% PPL). Both expressions are the ORIGINAL tokenizer.json patterns that llama.cpp quotes verbatim above its own copies in `src/llama-vocab.cpp` (`LLAMA_VOCAB_PRE_TYPE_QWEN2` / `_QWEN35`). llama.cpp spells the contractions out as `'[sS]|'[tT]|…` only because std::regex has no `(?i:…)` group; .NET supports the original form directly. - qwen2 = the llama3 expression with a bare `\p{N}` (one digit per segment) instead of `\p{N}{1,3}`. Aliases routed here by llama.cpp: `deepseek-r1-qwen`, `kormo`, `f2llmv2`, `megrez`, plus `stablelm2`/`hunyuan`/`solar-open`, which share the same regex block by case fall-through. - qwen35 = qwen2 with combining marks folded into the letter run (`[\p{L}\p{M}]+`, and `\p{M}` excluded from the punctuation class). - Also adds `minerva-7b`, llama.cpp's actual spelling of the value the table already listed as `minerva`. Tests: - Discriminating unit tests: both Qwen pipelines split every digit where gpt2 and llama3 group runs; and a decomposed "e" + U+0301 stays one segment under qwen35 but splits under qwen2/llama3/gpt2 — the only assertion that separates qwen35 from qwen2. - Real-GGUF token-id parity against llama.cpp b10434 for both `pre` values (Qwen2.5-0.5B-Instruct Q8_0 and Ternary-Bonsai-27B Q2_0), fixtures resolved through KnownTestFixtures; no weights enter the repo. - Negative control for #373: the pre-fix "no pre-tokenization" path is reconstructed explicitly and shown to load quietly and mis-tokenize. docs/TOKENIZERS.md now tabulates every supported `pre` value, including `tekken` (#387), which the table had not picked up. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DzekWWxE4d52Hpa31WBYfX --- docs/TOKENIZERS.md | 24 +++- .../Bpe/TiktokenPreTokenizer.cs | 38 +++++- .../Fixtures/KnownTestFixtures.cs | 26 ++++ .../Tokenizers/QwenPreTokenizerParityTests.cs | 83 +++++++++++++ .../Tokenizers/PreTokenizerPolicyTests.cs | 115 ++++++++++++++++-- 5 files changed, 267 insertions(+), 19 deletions(-) create mode 100644 tests/DotLLM.Tests.Integration/Tokenizers/QwenPreTokenizerParityTests.cs diff --git a/docs/TOKENIZERS.md b/docs/TOKENIZERS.md index f7bcb580..a54c97e9 100644 --- a/docs/TOKENIZERS.md +++ b/docs/TOKENIZERS.md @@ -18,11 +18,25 @@ pipeline is selected in `TiktokenPreTokenizer.GetRegexes`. - **Unknown value** → `InvalidDataException` at load (llama.cpp throws here too). Escape hatch: `DOTLLM_ALLOW_UNKNOWN_PRETOKENIZER=1` proceeds with the GPT-2 default pipeline — never with "no pre-tokenization", which silently mis-tokenizes. -- Supported `pre` values: `default`/`gpt2`; `llama3`/`llama-v3`/`llama-bpe`/`falcon3`/`falcon-h1`/ - `pixtral`/`midm-2.0`/`lfm2`/`jina-v5-nano` (one shared Llama-3 pipeline, as in llama.cpp); - `starcoder`/`refact`/`command-r`/`smollm`/`codeshell`/`exaone`/`minerva`/`mellum2`; - `deepseek-llm`; `deepseek-coder`; `gpt-4o`/`llama4`. New values are sourced from llama.cpp - `llama-vocab.cpp` (authoritative), never invented. +- Supported `pre` values (one row per pipeline; aliases share a pipeline exactly as they share + a `case` block in llama.cpp): + + | Pipeline | `tokenizer.ggml.pre` values | + |---|---| + | GPT-2 default | `default`, `gpt2` (also the absent/empty and opt-out fallback) | + | Llama 3 | `llama3`, `llama-v3`, `llama-bpe`, `falcon3`, `falcon-h1`, `pixtral`, `midm-2.0`, `lfm2`, `jina-v5-nano` | + | StarCoder/SmolLM | `starcoder`, `refact`, `command-r`, `smollm`, `codeshell`, `exaone`, `minerva`, `minerva-7b`, `mellum2` | + | DeepSeek LLM | `deepseek-llm` | + | DeepSeek Coder | `deepseek-coder` | + | Qwen 2 | `qwen2`, `deepseek-r1-qwen`, `kormo`, `f2llmv2`, `megrez`, `stablelm2`, `hunyuan`, `solar-open` | + | Qwen 3.5 | `qwen35` | + | GPT-4o | `gpt-4o`, `llama4` | + | Tekken | `tekken` | + + New values are sourced from llama.cpp `llama-vocab.cpp` (authoritative), never invented. + The Qwen pipelines differ from Llama 3 only in the digit alternative — a bare `\p{N}` + splits every digit — and Qwen 3.5 additionally folds combining marks into the letter run + (`[\p{L}\p{M}]+`). Any `pre` value not in this table throws; see the policy above. ### SentencePiece BPE (Llama 2) diff --git a/src/DotLLM.Tokenizers/Bpe/TiktokenPreTokenizer.cs b/src/DotLLM.Tokenizers/Bpe/TiktokenPreTokenizer.cs index 03c32f07..127f0402 100644 --- a/src/DotLLM.Tokenizers/Bpe/TiktokenPreTokenizer.cs +++ b/src/DotLLM.Tokenizers/Bpe/TiktokenPreTokenizer.cs @@ -71,6 +71,34 @@ internal static class TiktokenPreTokenizer RegexOptions.Compiled), ]; + // ── Qwen 2 / Qwen 3 (llama.cpp LLAMA_VOCAB_PRE_TYPE_QWEN2) ────── + // Identical to the Llama-3 expression EXCEPT the digit alternative is a bare + // `\p{N}` (one digit per segment) instead of `\p{N}{1,3}`, so BPE never merges + // across digits. This is the ORIGINAL tokenizer.json pattern that llama.cpp + // quotes verbatim in the comment above its own copy (llama-vocab.cpp, the + // QWEN2 case); llama.cpp spells the contractions out as `'[sS]|'[tT]|…` only + // because std::regex has no `(?i:…)` group — the two are equivalent and .NET + // supports the original form directly. + private static readonly Regex[] Qwen2Pipeline = + [ + new(@"(?i:'s|'t|'re|'ve|'m|'ll|'d)|[^\r\n\p{L}\p{N}]?\p{L}+|\p{N}| ?[^\s\p{L}\p{N}]+[\r\n]*|\s*[\r\n]+|\s+(?!\S)|\s+", + RegexOptions.Compiled), + ]; + + // ── Qwen 3.5 (llama.cpp LLAMA_VOCAB_PRE_TYPE_QWEN35) ──────────── + // Qwen2's expression with combining marks folded into the letter run: + // `[\p{L}\p{M}]+` instead of `\p{L}+`, and `\p{M}` excluded from the + // punctuation class (`[^\s\p{L}\p{M}\p{N}]+`). A decomposed "e" + U+0301 + // therefore stays ONE segment here but splits into letter + mark under + // qwen2/llama3/gpt2 — the property the discriminating test exercises. + // Original tokenizer.json pattern, quoted verbatim by llama.cpp above its + // QWEN35 case. + private static readonly Regex[] Qwen35Pipeline = + [ + new(@"(?i:'s|'t|'re|'ve|'m|'ll|'d)|[^\r\n\p{L}\p{N}]?[\p{L}\p{M}]+|\p{N}| ?[^\s\p{L}\p{M}\p{N}]+[\r\n]*|\s*[\r\n]+|\s+(?!\S)|\s+", + RegexOptions.Compiled), + ]; + // ── Tekken (Mistral NeMo / Pixtral-12B tokenizer family; also NVIDIA // Nemotron-Nano-9B-v2). llama.cpp LLAMA_VOCAB_PRE_TYPE_TEKKEN. This is the // ORIGINAL tokenizer.json pattern (quoted verbatim in llama-vocab.cpp:408); @@ -111,10 +139,18 @@ internal static class TiktokenPreTokenizer // (llama-vocab.cpp, the "llama3" case block). "llama3" or "llama-v3" or "llama-bpe" or "falcon3" or "falcon-h1" or "pixtral" or "midm-2.0" or "lfm2" or "jina-v5-nano" => Llama3Pipeline, + // "minerva-7b" is llama.cpp's actual spelling; "minerva" is kept because + // earlier dotLLM releases accepted it and no GGUF is known to carry it. "starcoder" or "refact" or "command-r" or "smollm" - or "codeshell" or "exaone" or "minerva" or "mellum2" => StarCoderPipeline, + or "codeshell" or "exaone" or "minerva" or "minerva-7b" + or "mellum2" => StarCoderPipeline, "deepseek-llm" => DeepSeekLlmPipeline, "deepseek-coder" => DeepSeekCoderPipeline, + // llama.cpp routes all of these to LLAMA_VOCAB_PRE_TYPE_QWEN2, and gives + // STABLELM2 / HUNYUAN / SOLAR_OPEN the same regex block by case fall-through. + "qwen2" or "deepseek-r1-qwen" or "kormo" or "f2llmv2" or "megrez" + or "stablelm2" or "hunyuan" or "solar-open" => Qwen2Pipeline, + "qwen35" => Qwen35Pipeline, "gpt-4o" or "llama4" => Gpt4oPipeline, "tekken" => TekkenPipeline, _ => Environment.GetEnvironmentVariable("DOTLLM_ALLOW_UNKNOWN_PRETOKENIZER") == "1" diff --git a/tests/DotLLM.Tests.Integration/Fixtures/KnownTestFixtures.cs b/tests/DotLLM.Tests.Integration/Fixtures/KnownTestFixtures.cs index 09f40031..f65e114e 100644 --- a/tests/DotLLM.Tests.Integration/Fixtures/KnownTestFixtures.cs +++ b/tests/DotLLM.Tests.Integration/Fixtures/KnownTestFixtures.cs @@ -39,6 +39,32 @@ internal static class KnownTestFixtures /// Human-readable name for skip messages. public const string Gemma4_26BDescription = "Gemma-4-26B-A4B-it Q4_K_M GGUF (~15.7 GB)"; + /// + /// Qwen2.5-0.5B-Instruct, Q8_0 (~0.5 GB) — Qwen/Qwen2.5-0.5B-Instruct-GGUF. + /// Carries tokenizer.ggml.pre = qwen2, the value on every Qwen2/Qwen3 GGUF (#397). + /// + public static FixtureLocation Qwen2_5_0_5B_Q8_0 => TestFixtureResolver.ResolveFile( + "DOTLLM_QWEN25_0_5B_Q8_0_GGUF", + "Qwen", + "Qwen2.5-0.5B-Instruct-GGUF", + "qwen2.5-0.5b-instruct-q8_0.gguf"); + + /// Human-readable name for skip messages. + public const string Qwen2_5_0_5BDescription = "Qwen2.5-0.5B-Instruct Q8_0 GGUF (tokenizer.ggml.pre = qwen2)"; + + /// + /// Ternary-Bonsai-27B, Q2_0 (~7.2 GB) — prism-ml/Ternary-Bonsai-27B-gguf. + /// Carries tokenizer.ggml.pre = qwen35 (#397). + /// + public static FixtureLocation TernaryBonsai27B_Q2_0 => TestFixtureResolver.ResolveFile( + "DOTLLM_BONSAI_PQ2_0_GGUF", + "prism-ml", + "Ternary-Bonsai-27B-gguf", + "Ternary-Bonsai-27B-Q2_0.gguf"); + + /// Human-readable name for skip messages. + public const string TernaryBonsai27BDescription = "Ternary-Bonsai-27B Q2_0 GGUF (tokenizer.ggml.pre = qwen35)"; + /// /// DeepSeek-V2-Lite HF safetensors snapshot directory — /// deepseek-ai/DeepSeek-V2-Lite. diff --git a/tests/DotLLM.Tests.Integration/Tokenizers/QwenPreTokenizerParityTests.cs b/tests/DotLLM.Tests.Integration/Tokenizers/QwenPreTokenizerParityTests.cs new file mode 100644 index 00000000..6f06ee83 --- /dev/null +++ b/tests/DotLLM.Tests.Integration/Tokenizers/QwenPreTokenizerParityTests.cs @@ -0,0 +1,83 @@ +using DotLLM.Models.Gguf; +using DotLLM.Tests.Integration.Fixtures; +using Xunit; + +namespace DotLLM.Tests.Integration.Tokenizers; + +/// +/// Issue #397 — token-id parity against llama.cpp for the two Qwen +/// tokenizer.ggml.pre pipelines, on real GGUFs. +/// +/// +/// Why real weights. The unit-level tests in +/// DotLLM.Tests.Unit.Tokenizers.PreTokenizerPolicyTests prove the routing is +/// distinguishable from gpt2/llama3 on a synthetic vocabulary. Only a real vocab + +/// merge table can show the segmentation is byte-for-byte the one llama.cpp produces, which +/// is what the perplexity numbers in #237/#397 actually depend on. +/// Provenance of the expected ids. Produced with the official +/// ggml-org/llama.cpp release b10434 Windows CPU build: +/// llama-tokenize.exe -m <gguf> -f <fixture> --ids --no-bos, with the +/// fixture file written as raw UTF-8 with no trailing newline. Upstream llama.cpp cannot +/// open the Ternary-Bonsai Q2_0 tensors (a non-upstream quantization type), so its ids were +/// taken from a tensor-less copy of that file's header + KV section — the same vocabulary, +/// merges and tokenizer.ggml.pre, no tensor data. That scratch file is not checked in; +/// regenerating it is a header/KV byte copy with tensor_count patched to 0. +/// Both fixtures resolve through (env override, then the +/// dotLLM test cache, then the HF hub cache — #308). A missing fixture skips loudly, +/// naming every probed path. +/// +public sealed class QwenPreTokenizerParityTests +{ + /// + /// Exercises every alternative the two pipelines disagree on: contractions, a digit run + /// (bare \p{N} splits it per digit), a decomposed combining acute and a decomposed + /// combining diaeresis (qwen35 keeps them inside the letter run, qwen2 does not), an + /// em dash, a slash, and a newline + tab run. + /// + /// Built from code points so no source-file re-encoding or NFC normalization can + /// silently change what is being tokenized. + private static string Fixture => + "It's 2026: cafe" + (char)0x0301 + + " costs 1234.56 " + (char)0x2014 + + " nai" + (char)0x0308 + "ve/OK\n\tdone"; + + /// llama.cpp b10434, qwen2.5-0.5b-instruct-q8_0.gguf (pre = qwen2). + private static readonly int[] Qwen2ExpectedIds = + [ + 2132, 594, 220, 17, 15, 17, 21, 25, 40930, 53839, 7049, 220, 16, 17, 18, 19, + 13, 20, 21, 1959, 308, 2143, 136, 230, 586, 14, 3925, 198, 40495, + ]; + + /// llama.cpp b10434, Ternary-Bonsai-27B-Q2_0.gguf vocab (pre = qwen35). + private static readonly int[] Qwen35ExpectedIds = + [ + 2064, 579, 220, 17, 15, 17, 21, 25, 39579, 52033, 6829, 220, 16, 17, 18, 19, + 13, 20, 21, 1892, 238883, 136, 230, 571, 14, 3793, 198, 39157, + ]; + + [SkippableFact] + public void Qwen2Gguf_TokenIds_MatchLlamaCpp() + { + FixtureLocation loc = KnownTestFixtures.Qwen2_5_0_5B_Q8_0; + Skip.If(!loc.Found, loc.SkipMessage(KnownTestFixtures.Qwen2_5_0_5BDescription)); + + using var gguf = GgufFile.Open(loc.Path!); + Assert.Equal("qwen2", gguf.Metadata.GetStringOrDefault("tokenizer.ggml.pre")); + + var tokenizer = GgufBpeTokenizerFactory.Load(gguf.Metadata); + Assert.Equal(Qwen2ExpectedIds, tokenizer.Encode(Fixture)); + } + + [SkippableFact] + public void Qwen35Gguf_TokenIds_MatchLlamaCpp() + { + FixtureLocation loc = KnownTestFixtures.TernaryBonsai27B_Q2_0; + Skip.If(!loc.Found, loc.SkipMessage(KnownTestFixtures.TernaryBonsai27BDescription)); + + using var gguf = GgufFile.Open(loc.Path!); + Assert.Equal("qwen35", gguf.Metadata.GetStringOrDefault("tokenizer.ggml.pre")); + + var tokenizer = GgufBpeTokenizerFactory.Load(gguf.Metadata); + Assert.Equal(Qwen35ExpectedIds, tokenizer.Encode(Fixture)); + } +} diff --git a/tests/DotLLM.Tests.Unit/Tokenizers/PreTokenizerPolicyTests.cs b/tests/DotLLM.Tests.Unit/Tokenizers/PreTokenizerPolicyTests.cs index b5325ece..e19fda89 100644 --- a/tests/DotLLM.Tests.Unit/Tokenizers/PreTokenizerPolicyTests.cs +++ b/tests/DotLLM.Tests.Unit/Tokenizers/PreTokenizerPolicyTests.cs @@ -24,22 +24,43 @@ public class PreTokenizerPolicyTests /// with NO pre-tokenization (the old unknown-pre behavior) "1234" is one /// segment and 3+4 merges. Token count 4 vs 3 discriminates the two. /// - private static BpeTokenizer Build(string? preType) - { - char[] byteToUnicode = new char[256]; - for (int b = 33; b <= 126; b++) byteToUnicode[b] = (char)b; - for (int b = 161; b <= 172; b++) byteToUnicode[b] = (char)b; - for (int b = 174; b <= 255; b++) byteToUnicode[b] = (char)b; - int n = 0; - for (int b = 0; b < 256; b++) - if (byteToUnicode[b] == 0) byteToUnicode[b] = (char)(0x100 + n++); + private static BpeTokenizer Build(string? preType) => Build(preType, "34", "3 4"); + /// + /// Builds the same 256-byte GPT-2 vocabulary with one extra merged token, so a + /// single merge either fires or is blocked purely by where the pre-tokenizer put + /// its segment boundary. + /// + /// GGUF tokenizer.ggml.pre value under test. + /// The extra vocabulary entry, in byte-level (GPT-2 + /// bytes_to_unicode) spelling. + /// The single merge rule producing . + private static BpeTokenizer Build(string? preType, string mergedToken, string merge) + => BpeTokenizer.CreateTiktoken(Vocab(mergedToken), merges: [merge], tokenTypes: null, + bosId: 0, eosId: 0, preTokenizerType: preType); + + /// The 256 byte tokens plus one merged entry. + private static string[] Vocab(string mergedToken) + { string[] tokens = new string[257]; - for (int i = 0; i < 256; i++) tokens[i] = byteToUnicode[i].ToString(); - tokens[256] = "34"; + for (int i = 0; i < 256; i++) tokens[i] = ByteToUnicode[i].ToString(); + tokens[256] = mergedToken; + return tokens; + } - return BpeTokenizer.CreateTiktoken(tokens, merges: ["3 4"], tokenTypes: null, - bosId: 0, eosId: 0, preTokenizerType: preType); + /// GPT-2 bytes_to_unicode table — printable bytes map to themselves. + private static readonly char[] ByteToUnicode = BuildByteToUnicode(); + + private static char[] BuildByteToUnicode() + { + char[] map = new char[256]; + for (int b = 33; b <= 126; b++) map[b] = (char)b; + for (int b = 161; b <= 172; b++) map[b] = (char)b; + for (int b = 174; b <= 255; b++) map[b] = (char)b; + int n = 0; + for (int b = 0; b < 256; b++) + if (map[b] == 0) map[b] = (char)(0x100 + n++); + return map; } [Theory] @@ -75,6 +96,51 @@ public void Tekken_SplitsEveryDigit_UnlikeGpt2AndLlama3() Assert.Single(Build("llama3").Encode("34")); // {1,3} group → "34" merges } + /// + /// Issue #397 — qwen2 is the value on every Qwen2/Qwen3 GGUF, and it + /// differs from llama3 in exactly one place: the digit alternative is a bare + /// \p{N}, not \p{N}{1,3}. With a "3 4" merge in the vocab, "34" + /// therefore stays two tokens under qwen2 and collapses to one under both gpt2 + /// ( ?\p{N}+) and llama3 — so this fails if qwen2 is routed to either. + /// + [Theory] + [InlineData("qwen2")] + [InlineData("qwen35")] + [InlineData("deepseek-r1-qwen")] // llama.cpp: -> LLAMA_VOCAB_PRE_TYPE_QWEN2 + [InlineData("kormo")] + [InlineData("f2llmv2")] + [InlineData("megrez")] + public void QwenPipelines_SplitEveryDigit_UnlikeGpt2AndLlama3(string preType) + { + Assert.Equal(2, Build(preType).Encode("34").Length); + Assert.Single(Build("gpt2").Encode("34")); + Assert.Single(Build("llama3").Encode("34")); + } + + /// + /// The discriminator between qwen35 and qwen2 (they share the bare + /// \p{N}, so the digit test above cannot tell them apart): qwen35's letter + /// run is [\p{L}\p{M}]+, so a decomposed "e" + U+0301 COMBINING ACUTE stays + /// one segment, while qwen2/llama3/gpt2 all use \p{L}+ and split the mark + /// off into the punctuation alternative. The vocab carries the byte-level merge + /// "e" + 0xCC (the first UTF-8 byte of U+0301), which can only fire inside a single + /// segment — 2 tokens under qwen35, 3 under everything else. + /// + [Fact] + public void Qwen35_KeepsCombiningMarksInTheLetterRun_UnlikeQwen2Gpt2AndLlama3() + { + // Built from code points rather than source literals so the test cannot be + // defeated by an editor silently NFC-composing the file. + string input = "e" + (char)0x0301; // e + COMBINING ACUTE ACCENT (UTF-8: 65 CC 81) + string merged = "e" + (char)0x00CC; // bytes_to_unicode spelling of the bytes 65 CC + string merge = "e " + (char)0x00CC; + + Assert.Equal(2, Build("qwen35", merged, merge).Encode(input).Length); + Assert.Equal(3, Build("qwen2", merged, merge).Encode(input).Length); + Assert.Equal(3, Build("llama3", merged, merge).Encode(input).Length); + Assert.Equal(3, Build("gpt2", merged, merge).Encode(input).Length); + } + [Fact] public void UnknownPreType_Throws_NamingTheValue() { @@ -83,6 +149,29 @@ public void UnknownPreType_Throws_NamingTheValue() Assert.Contains(OptOutVar, ex.Message, StringComparison.Ordinal); } + /// + /// Negative control for , required by + /// #373. The pre-fix behaviour — unknown pre resolved to a null regex, i.e. + /// NO pre-tokenization — is reconstructed explicitly here via + /// and shown to (a) load without + /// complaint and (b) produce a materially different token stream. That is exactly what the + /// old silent path did on an unknown value: it "passed", and mis-tokenized. + /// + [Fact] + public void NegativeControl_TheOldSilentNoPreTokenizationPath_LoadsQuietlyAndMisTokenizes() + { + var silent = BpeTokenizer.CreateTiktokenWithRegex( + Vocab("34"), merges: ["3 4"], tokenTypes: null, bosId: 0, eosId: 0, preRegex: null); + + // (a) No throw, no diagnostic — the failure mode #373 removed. + // (b) One segment for the whole input, so the "3 4" merge fires where every + // real pipeline forbids it: 1 token vs 2 under qwen2, 3 vs 4 under llama3. + Assert.Single(silent.Encode("34")); + Assert.Equal(2, Build("qwen2").Encode("34").Length); + Assert.Equal(3, silent.Encode("1234").Length); + Assert.Equal(4, Build("llama3").Encode("1234").Length); + } + [Fact] public void UnknownPreType_WithOptOut_FallsBackToGpt2Default_NotToNone() { From e361c0a6866097166e788ddd49d2842ae411b0fe Mon Sep 17 00:00:00 2001 From: James Burton Date: Sat, 15 Aug 2026 00:17:03 +0100 Subject: [PATCH 2/2] test(tokenizers): make the Qwen parity fixture actually discriminate (#397) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the integration parity fixture could not detect a wrong pipeline. Measured on the two real vocabularies, the original fixture was byte-identical under qwen2, qwen35, llama3 AND tekken — only a fall-back to gpt2 was caught, and only via the trailing "\n\tdone". So Qwen35Gguf_TokenIds_MatchLlamaCpp passed unchanged if qwen35 were routed to qwen2, which is precisely the bug the test exists to catch. Two vocabulary properties caused it, and the amended fixture defeats both: - Neither vocab contains any ASCII multi-digit token, so `\p{N}` vs `\p{N}{1,3}` is unobservable in ids however many ASCII digits are used. Fullwidth U+FF11 U+FF10 are the only multi-digit tokens in either vocab and now separate qwen2/qwen35 from llama3. - Latin decomposed marks never merge with letter bytes here (café, naïve, Zürich all identical under qwen2/qwen35). A Thai cluster (U+0E17 U+0E35 U+0E48) does merge, and now separates qwen35's `[\p{L}\p{M}]+` from qwen2's `\p{L}+`. Parity and discrimination are asserted as two separate properties: - Expected ids REGENERATED from llama.cpp b10434 for the amended fixture — still llama.cpp's output, never dotLLM's, so this stays a parity test and does not become a self-consistency test. - New AssertDiscriminates re-tokenizes the same real vocabulary under a forced `pre` and requires every wrong pipeline (gpt2, llama3, starcoder, gpt-4o and the other Qwen) to produce a different id stream, with a failure message explaining how the discriminators were chosen. Also adds the qwen35-vs-tekken discrimination at unit level (Qwen35_KeepsCaseBoundariesInsideTheLetterRun_UnlikeTekken): the two agree on both real vocabularies because neither contains a case-crossing merge, so the assertion is only meaningful on a vocabulary built to have one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DzekWWxE4d52Hpa31WBYfX --- .../Tokenizers/QwenPreTokenizerParityTests.cs | 138 +++++++++++++----- .../Tokenizers/PreTokenizerPolicyTests.cs | 20 +++ 2 files changed, 125 insertions(+), 33 deletions(-) diff --git a/tests/DotLLM.Tests.Integration/Tokenizers/QwenPreTokenizerParityTests.cs b/tests/DotLLM.Tests.Integration/Tokenizers/QwenPreTokenizerParityTests.cs index 6f06ee83..99405f98 100644 --- a/tests/DotLLM.Tests.Integration/Tokenizers/QwenPreTokenizerParityTests.cs +++ b/tests/DotLLM.Tests.Integration/Tokenizers/QwenPreTokenizerParityTests.cs @@ -1,62 +1,92 @@ using DotLLM.Models.Gguf; using DotLLM.Tests.Integration.Fixtures; +using DotLLM.Tokenizers.Bpe; using Xunit; namespace DotLLM.Tests.Integration.Tokenizers; /// -/// Issue #397 — token-id parity against llama.cpp for the two Qwen -/// tokenizer.ggml.pre pipelines, on real GGUFs. +/// Issue #397 — real-GGUF gate for the two Qwen tokenizer.ggml.pre pipelines, asserting +/// two independent properties that are easy to conflate: /// /// -/// Why real weights. The unit-level tests in -/// DotLLM.Tests.Unit.Tokenizers.PreTokenizerPolicyTests prove the routing is -/// distinguishable from gpt2/llama3 on a synthetic vocabulary. Only a real vocab + -/// merge table can show the segmentation is byte-for-byte the one llama.cpp produces, which -/// is what the perplexity numbers in #237/#397 actually depend on. -/// Provenance of the expected ids. Produced with the official -/// ggml-org/llama.cpp release b10434 Windows CPU build: -/// llama-tokenize.exe -m <gguf> -f <fixture> --ids --no-bos, with the -/// fixture file written as raw UTF-8 with no trailing newline. Upstream llama.cpp cannot -/// open the Ternary-Bonsai Q2_0 tensors (a non-upstream quantization type), so its ids were -/// taken from a tensor-less copy of that file's header + KV section — the same vocabulary, -/// merges and tokenizer.ggml.pre, no tensor data. That scratch file is not checked in; -/// regenerating it is a header/KV byte copy with tensor_count patched to 0. -/// Both fixtures resolve through (env override, then the -/// dotLLM test cache, then the HF hub cache — #308). A missing fixture skips loudly, -/// naming every probed path. +/// 1. Parity. dotLLM's token ids equal llama.cpp's on a fixture string. The expected +/// ids come from llama.cpp and nothing else — deriving them from dotLLM would make this a +/// self-consistency test wearing a parity test's name. +/// 2. Discrimination. Re-tokenizing the same real vocabulary under every other +/// pipeline must produce a different id stream. Without this a parity test passes unchanged +/// when the value is routed to the wrong pipeline, which is exactly what the first version of this +/// file did: measured on these two vocabularies, the original fixture was byte-identical under +/// qwen2, qwen35, llama3 and tekken, so it could only ever have caught a fall-back to gpt2. +/// Two properties of these vocabularies caused that, and the fixture below is built to defeat both: +/// +/// Neither vocabulary contains a single ASCII multi-digit token, so +/// \p{N} vs \p{N}{1,3} is unobservable in ids no matter how many ASCII digits the +/// fixture has. The fullwidth digits U+FF11 U+FF10 are the only multi-digit tokens in +/// either vocabulary, so they are what separates qwen2/qwen35 from llama3. +/// Latin decomposed marks never merge with letter bytes here, so +/// café/naïve/Zürich tokenize identically under qwen2 and qwen35. A +/// Thai cluster (U+0E17 U+0E35 U+0E48 — base plus vowel sign plus tone mark) does merge, +/// so it is what separates qwen35's [\p{L}\p{M}]+ from qwen2's \p{L}+. +/// +/// Provenance of the expected ids. Official ggml-org/llama.cpp release +/// b10434 Windows CPU build: +/// llama-tokenize.exe -m <gguf> -f <fixture> --ids --no-bos, the fixture written as +/// raw UTF-8 with no trailing newline. Upstream llama.cpp cannot open the Ternary-Bonsai Q2_0 +/// tensors (a non-upstream quantization type), so its ids were taken from a tensor-less copy of +/// that file's header + KV section — same vocabulary, merges and tokenizer.ggml.pre, no +/// tensor data. That scratch file is not checked in; regenerating it is a header/KV byte copy with +/// tensor_count patched to 0. The dotLLM side of both tests is a normal end-to-end +/// on the real GGUF. +/// Fixtures resolve through (env override, then the dotLLM +/// test cache, then the HF hub cache — #308); a missing one skips loudly, naming every +/// probed path. No weights enter the repository. +/// Not covered here: qwen35 vs tekken. Those two agree exactly on both +/// vocabularies even with this fixture, because neither contains a merge that crosses an +/// upper/lower-case boundary — the only place their expressions differ. That discrimination is +/// therefore proved at unit level instead, on a vocabulary built to contain such a merge +/// (PreTokenizerPolicyTests.Qwen35_KeepsCaseBoundariesInsideTheLetterRun_UnlikeTekken). /// public sealed class QwenPreTokenizerParityTests { /// - /// Exercises every alternative the two pipelines disagree on: contractions, a digit run - /// (bare \p{N} splits it per digit), a decomposed combining acute and a decomposed - /// combining diaeresis (qwen35 keeps them inside the letter run, qwen2 does not), an - /// em dash, a slash, and a newline + tab run. + /// Exercises the alternatives these pipelines actually disagree on in these vocabularies + /// (verified, not assumed — see the class remarks): a contraction, fullwidth digits, a Thai + /// base+mark cluster, an ASCII digit run, a decomposed combining acute and diaeresis, an em + /// dash, a slash, and a newline + tab run. /// - /// Built from code points so no source-file re-encoding or NFC normalization can - /// silently change what is being tokenized. + /// Built from code points so no source re-encoding or NFC normalization can silently + /// change what is being tokenized. private static string Fixture => - "It's 2026: cafe" + (char)0x0301 + - " costs 1234.56 " + (char)0x2014 + - " nai" + (char)0x0308 + "ve/OK\n\tdone"; + "It's " + (char)0xFF11 + (char)0xFF10 + // fullwidth 1 0 + " " + (char)0x0E17 + (char)0x0E35 + (char)0x0E48 + // Thai base + vowel sign + tone + " 2026: cafe" + (char)0x0301 + // + COMBINING ACUTE + " 1234.56 " + (char)0x2014 + // + EM DASH + " nai" + (char)0x0308 + "ve/OK\n\tdone"; // + COMBINING DIAERESIS /// llama.cpp b10434, qwen2.5-0.5b-instruct-q8_0.gguf (pre = qwen2). private static readonly int[] Qwen2ExpectedIds = [ - 2132, 594, 220, 17, 15, 17, 21, 25, 40930, 53839, 7049, 220, 16, 17, 18, 19, - 13, 20, 21, 1959, 308, 2143, 136, 230, 586, 14, 3925, 198, 40495, + 2132, 594, 220, 20109, 26022, 220, 35884, 47171, 220, 17, 15, 17, 21, 25, 40930, + 53839, 220, 16, 17, 18, 19, 13, 20, 21, 1959, 308, 2143, 136, 230, 586, 14, 3925, + 198, 40495, ]; /// llama.cpp b10434, Ternary-Bonsai-27B-Q2_0.gguf vocab (pre = qwen35). private static readonly int[] Qwen35ExpectedIds = [ - 2064, 579, 220, 17, 15, 17, 21, 25, 39579, 52033, 6829, 220, 16, 17, 18, 19, - 13, 20, 21, 1892, 238883, 136, 230, 571, 14, 3793, 198, 39157, + 2064, 579, 220, 19496, 25191, 149496, 220, 17, 15, 17, 21, 25, 39579, 52033, 220, + 16, 17, 18, 19, 13, 20, 21, 1892, 238883, 136, 230, 571, 14, 3793, 198, 39157, ]; + /// + /// Pipelines a mis-routing could plausibly land on. tekken is deliberately absent — + /// see the class remarks. + /// + private static readonly string[] WrongPipelines = ["gpt2", "llama3", "starcoder", "gpt-4o"]; + [SkippableFact] - public void Qwen2Gguf_TokenIds_MatchLlamaCpp() + public void Qwen2Gguf_TokenIds_MatchLlamaCpp_AndDifferUnderEveryOtherPipeline() { FixtureLocation loc = KnownTestFixtures.Qwen2_5_0_5B_Q8_0; Skip.If(!loc.Found, loc.SkipMessage(KnownTestFixtures.Qwen2_5_0_5BDescription)); @@ -64,12 +94,16 @@ public void Qwen2Gguf_TokenIds_MatchLlamaCpp() using var gguf = GgufFile.Open(loc.Path!); Assert.Equal("qwen2", gguf.Metadata.GetStringOrDefault("tokenizer.ggml.pre")); + // Property 1 — parity with llama.cpp, through the production factory path. var tokenizer = GgufBpeTokenizerFactory.Load(gguf.Metadata); Assert.Equal(Qwen2ExpectedIds, tokenizer.Encode(Fixture)); + + // Property 2 — a wrong pipeline on this same vocabulary is actually observable. + AssertDiscriminates(gguf.Metadata, "qwen2", [.. WrongPipelines, "qwen35"]); } [SkippableFact] - public void Qwen35Gguf_TokenIds_MatchLlamaCpp() + public void Qwen35Gguf_TokenIds_MatchLlamaCpp_AndDifferUnderEveryOtherPipeline() { FixtureLocation loc = KnownTestFixtures.TernaryBonsai27B_Q2_0; Skip.If(!loc.Found, loc.SkipMessage(KnownTestFixtures.TernaryBonsai27BDescription)); @@ -79,5 +113,43 @@ public void Qwen35Gguf_TokenIds_MatchLlamaCpp() var tokenizer = GgufBpeTokenizerFactory.Load(gguf.Metadata); Assert.Equal(Qwen35ExpectedIds, tokenizer.Encode(Fixture)); + + AssertDiscriminates(gguf.Metadata, "qwen35", [.. WrongPipelines, "qwen2"]); + } + + /// + /// Re-tokenizes with the model's real vocabulary and merge table but a + /// forced tokenizer.ggml.pre, and asserts every wrong pipeline yields a different id + /// stream than the correct one. This is what makes the parity assertion above load-bearing: + /// it proves the fixture can tell the pipelines apart at all. + /// + /// Metadata of the real GGUF under test. + /// The pre value the file actually declares. + /// Pipelines that must all disagree with . + private static void AssertDiscriminates( + GgufMetadata metadata, string correctPre, string[] wrongPres) + { + string[] tokens = metadata.GetStringArray("tokenizer.ggml.tokens"); + string[] merges = metadata.ContainsKey("tokenizer.ggml.merges") + ? metadata.GetStringArray("tokenizer.ggml.merges") + : []; + int[]? tokenTypes = metadata.ContainsKey("tokenizer.ggml.token_type") + ? metadata.GetInt32Array("tokenizer.ggml.token_type") + : null; + + int[] Encode(string pre) => BpeTokenizer + .CreateTiktoken(tokens, merges, tokenTypes, bosId: 0, eosId: 0, preTokenizerType: pre) + .Encode(Fixture); + + int[] correct = Encode(correctPre); + foreach (string wrong in wrongPres) + { + Assert.False( + correct.AsSpan().SequenceEqual(Encode(wrong)), + $"Fixture cannot discriminate '{correctPre}' from '{wrong}' on this vocabulary — " + + "the parity assertion would pass even if the pre value were routed to the wrong " + + "pipeline. Extend the fixture (see this class's remarks for how the current " + + "discriminators were chosen)."); + } } } diff --git a/tests/DotLLM.Tests.Unit/Tokenizers/PreTokenizerPolicyTests.cs b/tests/DotLLM.Tests.Unit/Tokenizers/PreTokenizerPolicyTests.cs index e19fda89..40559d6b 100644 --- a/tests/DotLLM.Tests.Unit/Tokenizers/PreTokenizerPolicyTests.cs +++ b/tests/DotLLM.Tests.Unit/Tokenizers/PreTokenizerPolicyTests.cs @@ -149,6 +149,26 @@ public void UnknownPreType_Throws_NamingTheValue() Assert.Contains(OptOutVar, ex.Message, StringComparison.Ordinal); } + /// + /// Separates qwen35 from tekken — the one pairing the real-GGUF parity suite + /// cannot cover. Both split every digit and both fold combining marks into the letter run, + /// so they differ only at a case boundary: qwen35's [\p{L}\p{M}]+ runs straight through + /// it, while tekken's [\p{Lu}…]*[\p{Ll}…]+ / [\p{Lu}…]+[\p{Ll}…]* pair must end + /// the segment before an upper-case letter that follows a lower-case one. With an "a B" merge + /// in the vocab, "aB" is therefore 1 token under qwen35 and 2 under tekken. + /// + /// + /// Deliberately unit-level: neither Qwen vocabulary contains a merge that crosses a case + /// boundary, so at integration level both routings emit identical ids and the assertion would + /// be vacuous. + /// + [Fact] + public void Qwen35_KeepsCaseBoundariesInsideTheLetterRun_UnlikeTekken() + { + Assert.Single(Build("qwen35", "aB", "a B").Encode("aB")); + Assert.Equal(2, Build("tekken", "aB", "a B").Encode("aB").Length); + } + /// /// Negative control for , required by /// #373. The pre-fix behaviour — unknown pre resolved to a null regex, i.e.