mtmd: add mtmd_tokenize_from_parts()

[ngxson]

Overview

Target support jinja's input marking: #28249

Add mtmd_tokenize_from_parts() for fine-control per-segment tokenization (for example, one segment can be marked as parse_special, while other doesn't)

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: most code written by AI

[ngxson]

tests are all passed

[vision] OK:   ggml-org/SmolVLM-500M-Instruct-GGUF:Q8_0
[vision] OK:   ggml-org/SmolVLM2-2.2B-Instruct-GGUF:Q4_K_M
[vision] OK:   ggml-org/SmolVLM2-500M-Video-Instruct-GGUF:Q8_0
[vision] OK:   ggml-org/gemma-3-4b-it-GGUF:Q4_K_M
[vision] OK:   THUDM/glm-edge-v-5b-gguf:Q4_K_M
[vision] OK:   second-state/Llava-v1.5-7B-GGUF:Q2_K
[vision] OK:   cjpais/llava-1.6-mistral-7b-gguf:Q3_K_M
[vision] OK:   ibm-research/granite-vision-3.2-2b-GGUF:Q4_K_M
[vision] OK:   second-state/MiniCPM-Llama3-V-2_5-GGUF:Q2_K
[vision] OK:   openbmb/MiniCPM-V-2_6-gguf:Q2_K
[vision] OK:   openbmb/MiniCPM-o-2_6-gguf:Q4_0
[vision] OK:   bartowski/Qwen2-VL-2B-Instruct-GGUF:Q4_K_M
[vision] OK:   ggml-org/Qwen2.5-VL-3B-Instruct-GGUF:Q4_K_M
[vision] OK:   ggml-org/InternVL2_5-1B-GGUF:Q8_0
[vision] OK:   ggml-org/InternVL3-1B-Instruct-GGUF:Q8_0
[vision] OK:   ggml-org/Qwen2.5-Omni-3B-GGUF:Q4_K_M
[vision] OK:   ggml-org/LFM2-VL-450M-GGUF:Q8_0
[vision] OK:   ggml-org/granite-docling-258M-GGUF:Q8_0
[vision] OK:   ggml-org/LightOnOCR-1B-1025-GGUF:Q8_0
[vision] OK:   ggml-org/DeepSeek-OCR-GGUF:Q8_0
[vision] OK:   ggml-org/dots.ocr-GGUF:Q8_0
[vision] OK:   ggml-org/HunyuanOCR-GGUF:Q8_0
[vision] OK:   ggml-org/gemma-4-E2B-it-GGUF:Q8_0
[audio]  OK:   ggml-org/ultravox-v0_5-llama-3_2-1b-GGUF:Q8_0
[audio]  OK:   ggml-org/Qwen2.5-Omni-3B-GGUF:Q4_K_M
[audio]  OK:   ggml-org/Voxtral-Mini-3B-2507-GGUF:Q4_K_M
[audio]  OK:   ggml-org/LFM2-Audio-1.5B-GGUF:Q8_0
[audio]  OK:   ggml-org/gemma-4-E2B-it-GGUF:Q8_0
[audio]  OK:   ggml-org/Qwen3-ASR-0.6B-GGUF:Q8_0

[ngxson]

/bot review

[ggml-gh-bot]

Automated code review

Static review of mtmd_tokenize_from_parts() (tools/mtmd + mtmd-cli + tests).

Blocking

(point 1) Per-part add_special does not mean "add special tokens for this segment", it means llama_tokenize(..., add_special = true), which adds both BOS and EOS around that segment (src/llama-vocab.cpp lines 3395 and 3432). Two consequences:

• In mtmd-cli.cpp (~line 293), add_bos on the first segment produces BOS ... EOS mid-prompt for any vocab with add_eos set, and the comment at line 290 ("only the first text part may add BOS") is factually wrong for those vocab. The old mtmd_tokenize() path instead put BOS at position 0 and EOS at the very end of the whole sequence (tokenize() in mtmd.cpp ~line 1265), so this is a behavior regression in mtmd-cli itself for such models.
• The from-parts path can never produce a trailing EOS: the tokenizer-level add_special is hard-coded false (mtmd.cpp line 1176), and nothing else appends it. Yet mtmd.h documents the new call as "same as mtmd_tokenize()" - it is not.

Either split the flag per part (add_bos/add_eos) or document precisely that per-part add_special wraps that one segment with BOS+EOS and that end-of-sequence EOS must be requested some other way. If the intent is really BOS-only for the first segment, mtmd-cli is currently wrong, not just the comment.

Will slow the review

(point 2) No test exercises the new public API. tests/test-mtmd-c-api.c already exists and covers the C surface; add at least one case for mtmd_tokenize_from_parts() (success path plus the both-set/neither-set validation error). mtmd-cli exercises it, but that is a manual tool, not CI.

(point 3) Input validation in mtmd_tokenize_from_parts() (mtmd.cpp line 1730) checks the text/bitmap XOR but not the text payload itself: std::string(p->text->text, p->text->text_len) at line 1183 dereferences p->text->text without a null check. A caller passing text == nullptr with text_len > 0 (or even text_len == 0, which is UB for std::string(nullptr, 0)) crashes inside the library. Add a cheap guard in the existing validation loop. (The legacy mtmd_tokenize has the same weakness, but this is a new public entry point - cheap to get right now.)

Nits

(point 4) Near-identical names for different things: public mtmd_input_part (C struct, mtmd.h) vs internal mtmd_internal_part (C++ struct, mtmd-internal.h). Consider a more distinct internal name (e.g. mtmd_part_internal) to avoid confusion in code that includes both headers.

(point 5) mtmd.h line 313: the continuation lines of the mtmd_tokenize_from_parts declaration are off by one column relative to the opening paren (they align with mtmd_context, the paren is one char further right). Compare the neighboring mtmd_tokenize declaration for alignment.

(point 6) mtmd-cli.cpp line 269-270: the comment "because we want to demonstrate how to use mtmd_tokenize_from_parts()" references the PR's purpose rather than a technical reason. A one-line "split on the marker so we can control add_special per segment" says the same thing without the demo framing.

(point 7) tools/mtmd/README-dev.md line 22 still describes only mtmd_tokenize(); a one-liner mentioning the from-parts variant would keep the dev doc accurate.

Scope checks that pass: linked issue exists (#28249), the change is self-contained, reuses the existing mtmd_tokenizer machinery (new constructor rather than a parallel path), and mtmd-cli serves as the required working example for the new API. No server changes, no security-sensitive parsing added.

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