Skip to content

fix(tokenizer): implement metaspace prefix handling for issue #473 - #481

Merged
michalharakal merged 4 commits into
SKaiNET-developers:developfrom
Sehul07:fix/issue-473-metaspace-prefix
Oct 12, 2026
Merged

michalharakal merged 4 commits into
SKaiNET-developers:developfrom
Sehul07:fix/issue-473-metaspace-prefix

Conversation

@Sehul07

@Sehul07 Sehul07 commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Closes #473

@Sehul07 Sehul07 closed this Oct 10, 2026
@Sehul07 Sehul07 reopened this Oct 10, 2026

@michalharakal michalharakal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Sehul07. Two things, one about the branch and one about the fix itself.

The branch carries both lanes. This PR and #482 point at the same three commits, so the two diffs are identical right now. The last commit (af0872f7, the BPEStrategy change) belongs to #474. Could you reset this branch to 04ede43e and force-push, so that #481 only contains the #473 work and #482 only the #474 work? One PR per lane keeps each review small and lets them merge independently.

The #473 fix is not in the diff yet. After the split, what remains here is a comment change and re-indentation in detectFromVocab; addSpacePrefix is still hard-coded to false at the GGUFTokenizer(...) call at the end of fromTokenizerJson. The actual change is small and the value is already plumbed:

  1. In fromTokenizerJson, locate the "pre_tokenizer" block and read "prepend_scheme" (for older files also a normalizer of type Prepend with "prepend": "▁", possibly inside a Sequence).
  2. Map it: always → true, first → true, never → false, no block at all → false (today's behaviour).
  3. Pass that boolean as addSpacePrefix instead of the constant.

The engine does the same sniffing in SentencePieceTokenizer.kt (lines 382–391 at the commit linked in the issue) — copying that shape is fine.

Tests (step 4 of #473): a synthetic ▁-vocab file with prepend_scheme: always encodes "Hi" to the ▁Hi id; the same file with never encodes to the Hi id. If you have Gemma 4 locally, Gemma4TokenizerParityTest must stay green ("Hi" → 10979); if not, just say so and I will run it.

Please also revert the indentation-only change in detectFromVocab so the diff is only the fix, add a CHANGELOG.md line and Closes #473 in the description.

CI runs are approved on your branches now, so you get compiler and test feedback on every push.

@michalharakal

Copy link
Copy Markdown
Contributor

CI finished on this branch and fails in :llm-core:compileCommonMainKotlinMetadata; the build treats Kotlin deprecation warnings as errors:

e: llm-core/src/commonMain/kotlin/sk/ainet/apps/llm/tokenizer/BPEStrategy.kt:43:21 'expect fun String(chars: CharArray): String' is deprecated. Use CharArray.concatToString() instead.

That line goes away entirely with the table-based preprocess suggested in the #482 review, so no need to patch it separately.

…SKaiNET-developers#473)

fromTokenizerJson no longer assumes addSpacePrefix=false for SentencePiece
style vocabularies. detectPrependScheme reads pre_tokenizer.prepend_scheme
(always and first -> true, never -> false) and, for older files, a Prepend
normalizer with "prepend": "▁", also inside a Sequence. A file that declares
neither keeps the previous behaviour, so Gemma 4 still encodes "Hi" as [Hi].

Reverts the indentation-only change in detectFromVocab, keeps the comment.

GGUFTokenizerPrependSchemeTest covers always, first, never, no block and the
legacy normalizer on a five-token vocabulary. CHANGELOG line under Unreleased.
@michalharakal
michalharakal force-pushed the fix/issue-473-metaspace-prefix branch from af0872f to 5d90e42 Compare October 12, 2026 19:53
@michalharakal

Copy link
Copy Markdown
Contributor

Thanks @Sehul07. I took the branch split off your plate and finished the lane here:

With #480 merged alongside, the mmBERT file now encodes the #473 acceptance sentence exactly as the reference: "Which department handles this request?" → [12236, 9888, 26446, 736, 3853, 235336].

Squash-merge with you as author once CI is green. Same note as on #480 about the commit email, it applies here too.

@michalharakal
michalharakal merged commit 8c6bed4 into SKaiNET-developers:develop Oct 12, 2026
michalharakal added a commit that referenced this pull request Oct 12, 2026
…ributor

The Unreleased section lists the three fixes from #480, #481 and #482 (issues
#472, #473, #474); this names the contributor who implemented them.

Co-authored-by: Sehul Vagadiya <179387438+Sehul07@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Lane 2 · kotlin-core] tokenizer.json parity — GGUFTokenizer: honour Metaspace prepend_scheme instead of a hard-coded addSpacePrefix=false

2 participants