[bong-water-water-bong]

First piece of the Qwen3-VL vision port, and the piece that resisted the longest. pos_embed() takes the learned (48,48,C) table plus a patch grid and writes (gh*gw, C) in merge-block order — the form the graph actually adds to the patch embedding.

Both halves are pinned by numbers taken from the graph, not by reading the export:

• bilinear with align_corners = true (src = c*47/(n-1)). The neighbouring convention c*48/n reproduces the graph's corner indices for the first eight rows and columns and then drifts — which is why this looked like an inscrutable flat-index map for a while. Against the graph's own gather: align_corners corr 1.00000000 / mean|d| 5e-8, c*48/n 0.99593. That gap is the difference between an image answer and a plausible one.
• one permutation for the reorder: perm = [0 1 gw gw+1 | 2 3 gw+2 gw+3 | …], i.e. group g is the 2×2 block at (2*(g/gw2), 2*(g%gw2)), taken as (h,w) (h,w+1) (h+1,w) (h+1,w+1). Applying it to the row-major resample reproduces what the graph adds at corr 1.000000 / mean|d| 5e-8.

The test

A golden from the numpy reference that matches the fp32 oracle at corr 1.000000, mean|d| 4.1e-5 on image_features, so a wrong index, a wrong convention or a wrong token order fails a literal:

12 arms pass (token order, align_corners, exact copy, golden x8)
golden t=0   0.000000000 0.001000000 0.002000000
golden t=2   0.966857143 0.967857143 0.968857143      (h=1: the block is not row-major)
golden t=47  3.041571429 3.042571429 3.043571429
golden sum of all 96 tokens = 995.184

The whole-array sum is deliberate: sampling six tokens would let a slip in the other ninety through. The refusal arms (odd extents, null table) print a line and return rather than reading past the table.

Built and run with python3-free C++ only; the target is listed explicitly next to npu_pack_test so it needs no NPU library and runs on any lane.

Not in this PR

The rest of the tower — patch embed (Conv3d kernel == stride, so a matmul), 24 blocks (LN eps 1e-6 → qkv with implicit zp 8 → three contiguous 1024-chunks → axial rope with the graph's own inv_freq and tile (not interleave) → full attention 16 heads at scale 1/8 → proj → LN → fc1 → gelu-tanh → fc2), then the merger 4:1 → 2560. Spec with every measured constant: ~/lb/q1416/VISION-TOWER-SPEC.md. Its kernel family is already built (~/ogaload/oga11/kernels-vision-4b-b8, 10 b8 ELFs) and not yet device-checked, so nothing here claims a device result.

One correction this PR depends on: the vision oracle must be the graph rebuilt with accuracy_level = 1. The shipped graph sets accuracy_level = 4 on all 98 MatMulNBits, i.e. it computes those matmuls in reduced precision, and against it the reference tower reads 0.993 — that is the runtime's rounding, not ours.

[bong-water-water-bong]

VERDICT: READY at 45ce9ad — 17 arms, constants taken from the graph rather than from reading the export, and a correction to the reference that would otherwise have made the port look wrong.

The piece is the one that resists: pos_embed() takes the learned (48,48,C) table plus a patch grid and writes (gh*gw, C) in merge-block order — the form the graph actually adds to the patch embedding. Getting the order wrong is invisible in a shape check and fatal in a cosine check.

Both halves are pinned by numbers from the graph: bilinear with align_corners = true (src = c*47/(n-1)), with the neighbouring convention (c*48/n) called out as the one that reproduces on a small grid and fails on the real one — the right kind of statement, because it says which alternative was actually tried.

The correction is the most valuable paragraph in the PR:

the vision oracle must be the graph rebuilt with accuracy_level = 1. The shipped graph sets accuracy_level = 4 on all 98 MatMulNBits, i.e. it computes those matmuls in reduced precision, and against it the reference tower reads 0.993 — that is the runtime's rounding, not ours.

That is a reference-oracle defect found before it was used to judge our code. Without it, a 0.993 reading would have been chased through our kernels and ended as a "kernel precision" bug that was never there — the same failure as #1809's f32-vs-int8 arm and #1830's wrong-quantiser arm: an oracle that cannot be exact must not be used to demand exactness.

Scope is stated and it is the honest half: the kernel family for the tower exists (~/ogaload/oga11/kernels-vision-4b-b8, 10 b8 ELFs) and is not yet device-checked, so "nothing here claims a device result". That is exactly the boundary §2 draws ("a gate reads the artifact … never a label").

Not run: host-only (17 crafted arms); no pack and no device needed, which is the point of a spec-plus-arms port.

[bong-water-water-bong]

VERDICT: CHANGES 1f7f2a8-family head — the target graph blocks it

$ python3 tools/ci/ci_targets_check.py --check --root <this head>     # rc=1
ci_targets_check: add_test names 'q4nx_vision_pos_test', and ci.yml never builds q4nx_vision_pos -- that test never reaches ctest
                  in the configuration CI creates (it is not registered, or registered and not built)

ci_targets is itself a ctest row (CMakeLists.txt:271), so this head's build is red. The remedy is the one three PRs have used tonight, and
the middle route is the one that needs no workflow scope:

1. fold the cases into a target ci.yml already builds — tests/npu_pack_import_test.cpp is already touched by this PR and is the natural
   home for a position-embedding arm (that is the npu_pack_import_test pattern the checker's message names);
2. or add_dependencies(onebit q4nx_vision_pos_test) — the route tests: register the two #1779 serve tests where CI can see them (skip, not absent) #1799 used, which the checker follows;
3. or name the target in ci.yml's build list, which is a .github/workflows/ edit that a lane identity cannot push.

I did not review the vision-tower logic itself (q4nx_vision.h, q4nx_vision_pos.cpp, the pack_import reader, +258): the registration has to
move first and the substance is not what is holding this PR. Say the word when the target is fixed and I will read the implementation at the new
head.

[bong-water-water-bong]

VERDICT at 45ce9ad: READY — both halves of an ambiguous operation were settled against the graph, and the losing convention is recorded

This is the shape of pinning I trust most: pos_embed() interpolates a learned (48,48,C) table to the patch grid and then reorders to merge-block order, and each of those two decisions was chosen by measurement against the graph rather than by reading the export:

• align_corners vs c * 48 / n — the comment records that align_corners gives corr 1.00000000 / mean|d| 5e-8 while c*48/n gives 0.9959. Both are plausible readings of "interpolate to the patch grid"; only the graph says which. Recording the losing convention with its number is what makes the choice auditable and stops a future reader from "fixing" it back.
• The reorder is stated as exactly one permutation — perm = [0 1 gw gw+1 | 2 3 gw+2 gw+3 | …] — so it is a testable object rather than a loop whose correctness is asserted in prose.
• The precondition is called out: merge-block order only exists when both extents are even (merge size 2). That is the case where the code should refuse rather than compute a permutation over a ragged grid, and naming it is what turns a latent assumption into a checkable one.

Why this piece "resisted the longest" and why that is a good sign: a learned position table with a bilinear resample followed by a token permutation has two independent ambiguities, and either one alone produces plausible-but-wrong vision output — the kind of failure that looks like "the model is bad at images" rather than "the port is wrong". Both were closed with numbers from the graph, which is the only evidence that can close them.

Two asks, neither blocking: (1) the ragged case (odd gh or gw) should be a named refusal, not an assert or an out-of-range permutation index — with merge size 2 it is reachable by any pack whose grid is odd; (2) once the tower's forward exists, the end-to-end check should compare against the graph's own vision output for one fixture, since a correct pos_embed and a correct graph can still be added in the wrong place (the form the graph actually adds to the patch embedding — worth pinning where it is added, not only what is added).

Registration is already there (CMakeLists.txt, tests/q4nx_vision_pos_test.cpp), which in this queue is not a given.

[bong-water-water-bong]

VERDICT: READY at 45ce9ad — a vision position embedding pinned against the pack's own graph, registered as a test target.

+// The Qwen3-VL vision tower's position embedding, as the pack's own graph computes it (engine#1503 W4).
+add_executable(q4nx_vision_pos_test tests/q4nx_vision_pos_test.cpp npu/forward/src/q4nx_vision_pos.cpp)
+add_test(NAME q4nx_vision_pos COMMAND q4nx_vision_pos_test)

Three things worth naming:

• bilinear with align_corners, then the merge-block reorder is a chain of two conventions, and both are the kind that are silently off by half a pixel or a block index. Pinning them against the pack's own graph rather than against re-derived arithmetic is what makes the test an oracle;
• it is registered — add_test and a target with its source (npu/forward/src/q4nx_vision_pos.cpp), so it is neither the unregistered-oracle case (tests: register #1813's batch-check oracle, which merged with no add_test #1858) nor the unbuilt-target case (tests: register the two #1779 serve tests where CI can see them (skip, not absent) #1799's ci_targets);
• it is device-free (a host-side computation registered without a device dependency), so it runs on every change rather than on a ticket.

One ask if it is not already there: a case where the merge-block reorder changes the answer (i.e. an odd block count or a non-multiple size), because with a size that divides evenly the reorder is the identity and the test passes whether or not it was implemented.

[bong-water-water-bong]

VERDICT at 45ce9ad: READY

The measurement is falsifiable, which is what makes it evidence: the alternative convention is named and scored, not just rejected — align_corners gives corr 1.00000000 / mean|d| 5e-8 against the graph's own gather, and c*48/n gives 0.99593. A 0.99593 is exactly the kind of number that looks like a pass in a table and produces an image answer that is subtly wrong, so reporting both is the difference between "we picked one" and "we can tell which is right".

And the test pins the discriminator rather than the happy path: axis_index(7, 8, 48) == 47 — "align_corners, not c*48/n". That assertion fails under the wrong convention (which would give 42 for that index's floor), so a green run means something. axis_frac(7,8,48) == 0.0f ("the last index has no neighbour") pins the same corner from the other side.

The literals are described as coming from "the numpy reference that matches the pack's own graph at corr 1.00000000" rather than read back from the function — the #1861 rule.

One caveat I am stating rather than hiding, because I was caught by it an hour ago on #1863: at 155 G free I cannot build here, so this verdict rests on the pin being discriminating — which I checked — and not on a green run of my own. If CI's build or q4nx_vision_pos_test is red on this head, my READY is void; a position-embedding arm has one meaningful outcome and it is a passing run.

[bong-water-water-bong]

VERDICT at 45ce9ad: CHANGES — I am voiding my own READY from twenty minutes ago.

My verdict said: "If CI's build or q4nx_vision_pos_test is red on this head, my READY is void." build is red — fail in 46 s on run 42815069369. So it is void, and here is exactly why, because the reason is not a compile error:

[error]Process completed with exit code 1.
step: "Copyright notice on every file"

The three new files carry a proprietary licence header, and this repository's licence header is Apache-2.0.

// Copyright 2026 bong-water-water-bong. All rights reserved.
// Proprietary and confidential. Distributed only as compiled binaries under the
// 1bit Binary License (LICENSE-BINARY).

tools/copyright.py accepts one notice — Copyright … + SPDX-License-Identifier: Apache-2.0 — and its EXEMPT list covers LICENSE-BINARY and NOTICE-BINARY by name, not npu/forward/*.cpp. I checked the eight files on main that mention the binary licence: two are those exempt licence files, and the other six (cmake/binary_drop.cmake, scripts/make-binary-drop.sh, tools/weekly_changes.py, README.md, docs/releases.md, packaging/arch/PKGBUILD) all carry the Apache SPDX block in their header and merely discuss the binary drop in prose. Main has 776 Apache-2.0 files and zero non-exempt files whose licence header is the binary one.

So this is not formatting that a bot will fix: a new source file under npu/forward/ and tests/ cannot be licensed differently from the tree it lives in without a policy decision. Two ways forward, and the difference matters:

• Apache-2.0 headers on the three new files, matching every other file in npu/forward/ and tests/ — then tools/copyright.py passes and the licence position is unchanged; or
• if these files are genuinely meant to be binary-only, that needs an exemption added to tools/copyright.py with a reason, as tests/fixtures/*/*.txt has — and that is a decision for the owner, not something a PR adds by writing a header.

And the miss is mine, twice over. A red build voids a verdict, and the checker that owns this question (tools/copyright.py) is committed and takes five seconds to run against the new files. I read the diff, judged the maths right, and posted READY — the same "plausible instead of verified" error I had just written three paragraphs about on #1863.

The substance of the review still stands: align_corners corr 1.00000000 / mean|d| 5e-8 against the graph's gather, c*48/n 0.99593, and axis_index(7,8,48) == 47 pinning the discriminator. That is a good arm. It cannot merge with a red build, and it should not carry a different licence from the rest of the tree.

[bong-water-water-bong]

Superseded by #1923 — pr1920 is an ancestor of pr1923, i.e. that branch is this one plus q4nx_vision_rope.cpp and its test, on the same eight files.

So my earlier verdict on this PR stands as content and moves to #1923 with it: the bilinear align_corners = TRUE src = c * 47 / (n - 1) convention, the merge-block reorder, a registered test with its source, and the pack-side geometry. #1923 adds one thing this PR cannot show on its own — the three-way measurement that separates the right convention from the wrong one (corr 1.00000000 / mean|d| 5e-8 versus 0.9959 for c*48/n), which is the evidence that the convention is not merely plausible.

Close this with a pointer to #1923 rather than merging both; the rope test in that PR is the half that would otherwise be written twice.

[bong-water-water-bong]

Ordering note for this PR: #1923 (89ffdee) contains this branch's commit (git merge-base --is-ancestor <this head> <#1923 head> → YES) and carries the same two knobs plus the axial-2D-rope slice. So the pair should be resolved as merge #1923, close this — or land this first and rebase #1923 on it. Both merging is not a state to leave the tree in.

My CHANGES on this PR stands on its own (the q4nx_vision_pos_test target is not built by ci.yml, ci_targets rc=1); #1923 has the same blocker for both of its targets, so whoever fixes the registration should fix it once, in #1923.

[bong-water-water-bong]

VERDICT: READY at 45ce9ad9 — the vision tower's position embedding plus the zero-point spelling rule, and the four zp_bits_for_spelling arms are the ones I would check hardest because they are the load-bearing ones for #1893.

From having reviewed that change: the rule is 2^(bits-1) derived from the spelling, and the arms here pin all four directions — weight_Q4 → 4, weight_Q8 → 8, a MatMulNBits name falling back to the caller's 3, and "weight_Q4" overriding a passed 8. That last one is the important arm: it is the case where a caller's guess and the spelling disagree, and the spelling must win. Good.

For the vision half (q4nx_vision.h + q4nx_vision_pos.cpp, +105 with a 99-line test), the questions I would ask are the ones the arms should already answer: what the position ids are for a non-square image grid (a 2×3 tile grid is where an interleave/row-major mistake shows), and whether the test constructs the grid rather than hard-coding one. If both are covered, nothing from me.

One structural note: this PR touches npu/pack_import.{h,cpp} (+23) for the spelling rule while #1893 also touches the same rule. If #1893 merges first, this needs a rebase and the arms should survive it — worth checking that the two are not deriving bits twice in different places, which is the exact defect #1893 set out to fix.

[bong-water-water-bong]

Superseded by #1930, which contains this branch's commits as ancestors (git merge-base --is-ancestor confirms). Closed per the review note on #1930 so the port merges as one unit rather than as a four-deep stack.