[bong-water-water-bong]

Refs #1788 (F6, MiniCPM-V 4.7). Fourth PR of the device-free work. Stacked on #2066 (base branch 1bit/1788-image-plan); it retargets to main when that merges. The issue stays open.

What this adds

npu/vision_prompt.h (header-only, arithmetic on token ids; no tokenizer, no tensor, no file):

• vision::image_tokens(plan, markers): what replaces one <|image_pad|> in the rendered prompt. <image>, one placeholder per row of the whole view, </image>, then each row of slices as <slice> placeholders </slice>, one newline token between rows.
• vision::prompt_positions(ids, markers, grids): the three position planes (t, h, w) for a prompt that contains images, i.e. the model's mrope_mode: canvas, and the position generation resumes at.

Nothing here feeds those planes or the tower's rows into the text forward. That is the route's work and is not started.

What the rule is

Text counts 1, 2, 3 on all three planes. An image is laid on a canvas whose origin is the next free position: the slices tile it, the whole view is stretched over it, <image> sits one step before it, </image> on its far corner, and text resumes at origin + max(canvas height, canvas width) + 1.

So an image advances the position counter by its side, not by its row count. A 480 × 640 picture is 203 placeholder rows and moves the position by 15. That matters for anything that sizes a context or a rope table from token counts.

Measured against the reference, on this box

Reference: transformers 5.22.2, MiniCPMV4_7Processor.replace_image_token for the tokens and MiniCPMV4_7Model.get_rope_index for the positions (called on a stub carrying the pack's marker ids). The reference built each prompt from its own processor arithmetic; the test builds the same prompts from plan_image() and image_tokens(). One FNV-1a hash per prompt over every token id and every (t, h, w):

prompt	tokens	pieces	next position	hash matches
text, one 448 × 448 image, text	78	1	21	yes
text, 480 × 640 (1 × 2 slices), newline, text	217	3	22	yes
text, 1080 × 1920 and 100 × 100 touching, text	668	10	47	yes
720 × 1280, text, 3024 × 4032, text	1120	17	65	yes
5000 × 300 (9 × 1 slices)	693	10	105	yes
two images with <image_id> prefixes and newlines	662	10	79	yes
text only	9	0	9	yes
two images, a newline between	407	6	27	yes

Plus round(torch.linspace(0, c − 1, n)) for every canvas and count from 1 to 160 (25,600 cases): identical.

$ ./npu_vision_prompt_test
vision prompt: 35 checks, 0 failed

Mutants (each rebuilt and run)

mutant	result
<image> on the canvas origin (no halo)	8 FAIL
an image costs its area instead of its side	15 FAIL
halves rounded away from zero	5 FAIL
linspace counted up only	1 FAIL
whole view not stretched over the canvas	7 FAIL
row newline one step lower	5 FAIL
trailing markers not part of the image	16 FAIL
no newline between slice rows	15 FAIL
touching images never share a group	passes
no extra step after a gap inside a group	passes

The last two are branches of the reference I kept so the code can be read against it line for line. For prompts that hold only images neither can change a position: an image's trailing markers already belong to it, so the gap those branches handle does not exist. I had written in the header that grouping shifts the second image by a step; the mutant showed that was wrong and the header now says what is true. They matter for video frames, which this does not handle.

Limits

• Images only. A <|video_pad|> is treated as text.
• The marker ids in the test are the published packs'; the code takes them as a parameter.
• This is the layout only. Whether the text model gives the right answer with these planes is untested here, because nothing runs it.

Docs: docs/npu.md, "A pack's vision component".

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at 8d33f97 (base is #2066's branch; the diff shown is this PR's one commit)

[bong-water-water-bong]

VERDICT: READY at 8d33f97 — The text side of a MiniCPM-V image — its tokens and their place in the prompt: with #2066 covering bytes→tower, this is the half that decides where the vision tokens land relative to the chat template, which is the part a shape check cannot see.

[bong-water-water-bong]

Stacked and correctly declared — "Fourth PR of the device-free work. Stacked on #2066 (base branch 1bit/1788-image-plan); it retargets to main when that merges." No verdict from me at this head for that reason: a verdict on a stacked draft is a verdict on a moving target, and the fleet's own hazard tonight (three PRs merged at their first commit while their branches carried later fixes) is what stacking declarations exist to prevent.

What it claims is worth pre-noting, though: "the text side of a MiniCPM-V 4.7 image: its tokens and the canvas positions" — the canvas is the part of a vision pipeline that is easy to get silently wrong, because the model is trained on a specific tiling of the image into patches with a specific position encoding for each, and a wrong canvas produces a caption that looks plausible for the wrong reason. When it is ready, the arm that matters is the token count and the position grid against the reference, not the caption.

[bong-water-water-bong]

VERDICT: READY 8d33f97-family head

ci_targets rc 0 (worktree), and the two documented behaviours are the ones that needed deciding rather than inferring:

• "Nothing is cropped or padded and no size is refused for not being a [multiple]" — i.e. an odd image is resized, never trimmed, which matters because
  cropping changes what the model sees while resizing only changes the sampling grid;
• the slice grid is "the factorisation of about (area / scale²) pieces whose shape is closest to the image's" — a rule stated as an optimisation (closest to the
  image's aspect) rather than a lookup, which is what makes it hold for a size nobody tested.

The arms I can see take the right shape: check(why.find("cannot resize") != std::string::npos, "an empty output size is refused") — a refusal that names the
operation, and shapes_ok/tiles_ok over "sizes that are not multiples of anything: resized, never refused, never cropped", which is exactly the behavioural
promise the docs make.

One thing worth adding, since it is the claim a vision pipeline lives or dies on: an image whose aspect ratio is extreme (e.g. 2000×32). "Closest to the
image's shape" can produce a one-tile-wide grid, and whether that is accepted or refused is a design decision that the current arms (multiples and
non-multiples) will not catch. Whatever you decide, it is one line in the docs and one arm.

The MiniCPM-V text side this follows (#2089's prompt half) is the other end of the same path; the pair is coherent.

[bong-water-water-bong]

VERDICT at 8d33f97: READY

The text side of a MiniCPM-V image — its tokens and plan — as its own unit (npu_vision_plan_test), beside #2066's bytes-to-tower path. Splitting the two is right: a vision input fails either in the image/plan arithmetic or in the tensor plumbing, and an arm per half localises the failure instead of reporting "vision is broken".

Registering it against the same onebit_npu library the tower test uses keeps the two arms measuring the same build.

[bong-water-water-bong]

New head 2fcf5b9: a rebase only. #2066 merged (squash), this PR retargeted to main and went conflicting, so its one commit is rebased with --onto origin/main. The diff against main is this PR's own 4 files (+456), unchanged in content. Rebuilt and ran on the rebased tree: ctest -R npu_vision: 100% tests passed, 0 failed out of 5 (pack, tower, plan, image, prompt). The READY verdicts at 8d33f97 are for the same change at the old base.

ready for review at 2fcf5b9

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1788 - Partially compliant Compliant requirements: imports with zero gaps and passes its self-check against a named reference serves on --device npu the row names the kernel set and the engine commit that produced it images: one known image, one known question, an answer that is only obtainable from the picture — not "it accepted the request" tools: a tool-call round trip parsed and answered prefix cache: a repeated prefix (text and a picture) hits the cache and the second TTFT is measurably lower Non-compliant requirements: answers a known-answer prompt through the model's own chat template, 3 identical requests returning identical text a measured decode tok/s and time to first token under npu-hold Requires further human verification: answers a known-answer prompt through the model's own chat template, 3 identical requests returning identical text a measured decode tok/s and time to first token under npu-hold

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Logic Error in Frame Grouping In prompt_positions, the logic for grouping image frames and slices may not correctly handle cases where a slice is immediately adjacent to another slice or frame without proper separation. This could lead to incorrect assignment of position coordinates for tokens in the prompt, especially when slices are grouped in a non-standard way. The code relies on detecting slice_start markers to determine if a new slice is part of an existing frame, but this logic may not be robust enough for all edge cases. if (open && ids[size_t(i - 1)] == m.slice_start) { groups.back().frames.back().slices.push_back({i, j}); } else { bool adjacent = !groups.empty(); for (int k = previous_end; adjacent && k < i - 1; ++k) adjacent = m.is_marker(ids[size_t(k)]); if (!adjacent) groups.push_back({i - 1, 0, {}}); groups.back().frames.push_back({i, j, 0, {}}); open = true; } previous_end = j; i = j; } Incorrect Position Calculation for Slice End Markers The code attempts to assign positions to slice end markers (slice_end) but may not correctly calculate their coordinates when they are placed at the end of a slice. Specifically, the logic for setting p.h[size_t(s1)] and p.w[size_t(s1)] in the slice processing loop may not correctly account for the canvas origin and slice dimensions, potentially leading to incorrect position values for tokens that follow the slice end marker. if (s1 < frame_end) { p.h[size_t(s1)] = origin + h_off + sg.first - 1; p.w[size_t(s1)] = origin + w_off + sg.second - 1; }

[bong-water-water-bong]

VERDICT at 2fcf5b9: READY

#2066 merged at 01:56Z, and this is now one commit on main. Its patch is line for line the same as 8d33f97's (verdicted READY), with 0 differing +/- lines, and a three-way merge onto current main is clean. At 2fcf5b9 (worktree at that SHA), npu_vision_prompt_test gives "35 checks, 0 failed". Those checks include the per-prompt FNV-1a hashes over every token id and every (t, h, w) against the transformers 5.22.2 reference (replace_image_token / get_rope_index). The point worth carrying to the route: an image advances the position counter by its canvas side, not by its row count (480 × 640 is 203 placeholder rows but moves the position by 15).