Skip to content

Two independent GGUF tokenizer implementations silently disagree (GGUFTokenizer vs TokenizerFactory) #330

Description

@michalharakal

Summary

sk.ainet.apps.llm.tokenizer.GGUFTokenizer.fromSource(...) and sk.ainet.apps.llm.tokenizer.TokenizerFactory.fromGgufSource(...) are two independently-implemented BPE/tokenization code paths for the same GGUF format, and they produce different tokenizations for identical input on the same model. This is a silent correctness bug, not a crash — it's much more dangerous for exactly that reason.

Repro

Loading SmolLM2-135M-Instruct-Q8_0.gguf and encoding the same ChatML-wrapped prompt string with each:

  • GGUFTokenizer.fromSource(...) → 25 tokens
  • TokenizerFactory.fromGgufSource(...) → 16 tokens

Confirmed with greedy (temperature=0) generation on identical weights/quant-policy/reader-path: the TokenizerFactory path produces the correct, on-topic answer ("The capital of France is Paris...") while the GGUFTokenizer path produces fluent but completely off-topic text. Root cause: the divergent tokenization corrupts the ChatML special-token structure (<|im_start|>/<|im_end|>) the model relies on to distinguish "you are being asked a question" from plain text continuation — the model's underlying language ability is intact, it just loses the instruction-following signal.

Full write-up and a standalone reproduction harness (Node + Kotlin/Wasm) in SKaiNET-examples PR #35, which hit this as the actual cause of "hallucinating" output in a browser (wasmJs) build — DesktopModelProvider/AndroidModelProvider already use TokenizerFactory.fromGgufSource in production, but a wasm fallback path elsewhere had been written against GGUFTokenizer.fromSource instead. Nothing signaled that these two were not interchangeable.

Why this is easy to trip over

From reading the code, TokenizerFactory looks like a deliberate, more careful successor to GGUFTokenizer — it delegates the actual BPE/SentencePiece mechanics to an "upstream" implementation and layers SentencePieceSpecialTokens on top specifically to get chat-template special tokens right (the code comments call out fixes for Gemma 4 specifically). But GGUFTokenizer.fromSource/fromGGUF/fromRandomAccessSource are all still public, still exported, and look equally legitimate to a new caller — there's no deprecation marker, doc pointer, or compile-time signal steering consumers toward the correct one.

Suggested direction

Rather than just picking a winner between the two entry points (which only fixes it until the next caller picks the wrong one again), it might be worth reconsidering the architecture: tokenizer construction arguably shouldn't be a separately-selectable utility at all — it's an integral, inseparable part of a model definition (the tokenizer is part of what makes a GGUF checkpoint usable, not an independent concern). Coupling tokenizer resolution into wherever model loading/definition already lives (the NN DSL / model loader layer, whichever is the actual single source of truth for "given this GGUF, build the runnable model") would make it structurally impossible to get a model and an incompatible tokenizer out of the same load call. Deferring to the maintainers on where that boundary should actually sit in the current module layout — just flagging that "two selectable tokenizer factories that can silently diverge" is the shape of bug that will recur otherwise.

At minimum, GGUFTokenizer.fromSource should probably be deprecated (or made to delegate to TokenizerFactory internally) so there's exactly one correct way to get a tokenizer for a GGUF file.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions