[tokinasin]

Overview

fixes: #30043

The PLaMo-3 tokenizer inserts hard boundaries before running the Unigram DP, around <|plamo:...|>-looking text, and around runs of at least 4 identical characters or 2 spaces. Without them llama.cpp tokenizes these cases (e.g. code indentation or repeated punctuation) differently from the reference.

Reproduce the two re.sub() passes in llm_tokenizer_plamo2 by encoding each segment independently.

Input	Tokenizer	Tokens
helloooo	reference	hell oooo
helloooo	llama-tokenize (before)	hello ooo
helloooo	llama-tokenize (after)	hell oooo
hello world	reference	hello world
hello world	llama-tokenize (before)	hello world
hello world	llama-tokenize (after)	hello world

• PLaMo-3 GGUFs converted before this PR will keep loading without pre-segmentation. You need to update tokenizer.ggml.model to "plamo3" or re-convert GGUF.

Additional information

• Reference: https://huggingface.co/pfnet/plamo-3-610m-fin-instruct/blob/main/tokenization_plamo.py
• Related: convert : Fix token configuration for PLaMo-3 #29843

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: YES
  • I used Codex for research, code generation and reviews, etc.

[CISC]

/bot review

[ggml-gh-bot]

Automated code review

Review of PR #30045 (vocab : implement PLaMo-3 tokenizer pre-segmentation)

Checklists run: scope gate, security, general, plus vocab/conversion areas. The approach itself is sound: a pre_segment_ flag on the existing llm_tokenizer_plamo2 plus a dedicated LLAMA_VOCAB_TYPE_PLAMO3 is a minimal, convention-fitting change, and gating on the tokenizer model string instead of LLM_ARCH_PLAMO3 (as the intermediate commit did) is the right final design. No security issues: all loops are bounded by the input length, cut is sized n + 1, and every index (cut[j + 2], cut[i + run], cut[seg_end]) stays in range.

Correctness points to verify against the reference (these are the only places I can see the port possibly diverging from tokenization_plamo.py):

(point 1) BOM handling is silently dropped for plamo3. src/llama-vocab.cpp:1542 moves the leading-U+FEFF strip into the !pre_segment_ branch only, so a plamo3 prompt starting with a BOM now gets it tokenized as a normal character. If the reference Plamo3Tokenizer still strips the BOM (e.g. pre-segmentation wraps a shared entry point that does the strip), this is a mismatch. Either restore the strip before segmentation, or add a short comment stating the reference plamo3 does not strip it.

(point 2) <|plamo:|> with empty content matches. The content scan in pass 1 starts at j = i + prefix_len and immediately accepts |>, so a zero-character body gets hard boundaries. If the reference regex is {1,64} rather than {0,64}, this is wrong. Please double-check the quantifier, and while at it confirm the transcribed details: the 64-char cap, the 0x1C-0x1F exclusion, and "whitespace" meaning the full Unicode set (llama.cpp's unicode_set_whitespace matches Python \s minus 0x1C-0x1F, which the explicit range check compensates for - that part looks right).

(point 3) Literal U+EE00 in user input is silently dropped (src/llama-vocab.cpp:1602). This reproduces a reference that inserts chr(0xEE00) markers and then split()s on them - but only if the reference really does it that way rather than tracking positions. Please confirm, and regardless: the magic number has zero explanation. Add 1-2 lines stating that U+EE00 is the reference implementation's private-use boundary marker, and that literal occurrences in the input split segments the same way the reference's split does. Without that, the next reader cannot tell marker from payload.

Will slow the review:

(point 4) No test coverage or verification evidence. Exact-match with the reference is the entire point of this PR, but nothing exercises it. At minimum, document in the PR description the adversarial cases checked against the reference (code indentation / 4+ repeats, 2-space runs, newline runs, <|plamo:...|> variants: empty body, 64 vs 65 content chars, whitespace inside the body, content run like aaaa inside a tag, tag adjacent to same-char runs) and their reference outputs. A small set of checked-in vectors would be even better.

(point 5) Worth noting for users: plamo-3 GGUFs converted before this PR carry tokenizer_model "plamo2" and will keep loading without pre-segmentation (silently different tokenization). A "reconvert your GGUF" line in the PR description would help.

Nits:

(point 6) conversion/base.py:2511: the ValueError message prints None when tokenizer_class is missing; including the model dir (and that the key is absent) would make it actionable. The fail-fast itself is an improvement over silently writing plamo2.

(point 7) static const uint32_t prefix[] could be static constexpr; trivial.

Style is otherwise clean: ASCII only, concise comments (the run-skip invariant comment at line ~1586 is exactly the kind that earns its place), and the column alignment matches the repo clang-format config.

This review was generated automatically by pi coding agent using zai-org/GLM-5.3. It may contain mistakes. Maintainers make the final call.

[CISC]

Address bot points and we're good, if you want you can test the vocab (using test-tokenizer-0) locally, but I will generate test-files for our CI.

[tokinasin]

done. thank you for reviewing!