[bong-water-water-bong]

What was wrong

Qwen3-VL's config declares its rotary variant under rope_scaling:
{"mrope_interleaved": true, "mrope_section": [24, 20, 20], "rope_type": "default"}. Our semantics reader only
looked inside rope_parameters (the FastFlowLM/OGA and Qwen3.5 spelling), so a Qwen3-VL container loaded with
mrope=0 — plain RoPE — and nothing said so.

For a text-only prompt that is correct, and this is measured rather than asserted: in the rotate-half layout a head
carries 64 frequencies theta^(-2i/128) at theta 5e6, and [24,20,20] splits them across the (t, h, w) axes
(t×24, h×20, w×20). With text ids (pos_t = pos_h = pos_w):

row	result
max |mrope_angle − rope_angle| over positions 0, 1, 7, 128, 1024, 4096, 32768, 262143	0 (bit-exact)
rotated 128-dim head, MRoPE vs plain RoPE, at every one of those positions	bit-identical
counter-example with image-style ids (t=100, h=5, w=9)	0.2922 rad angle delta, 0.3788 per head element

So the flag is not needed to serve text, and is needed the moment image tokens are in the prompt: today that
geometry would be silently wrong. The flag is what a route, a plan field (#799) or a refusal can act on, so it has
to be seen.

The change

• npu/forward/src/q4nx_semantics.c: also read rope_scaling.mrope_interleaved, independently of whether a
  rope_parameters block exists. Nothing else changes — mrope was already parsed, stored and printed; it is not
  read by the forward's attention path yet (that is the hybrid work, npu: serve sentence embeddings on the NPU through our runtime (/v1/embeddings, first model trit-embed-3l) #1311/TQVL).
• tests/q4nx_mrope_test.c (new, plain C, no device): both spellings set the flag; and the controls that keep
  this from over-firing — a rope_scaling block carrying rope_type: yarn + factor (no mrope_interleaved) is
  not mrope, and mrope_interleaved: false stays plain. rope_theta and rope_type_named are checked too, so
  the new lookup cannot quietly re-route the theta.
• tools/safetensors_to_q4nx.py: accept the Hugging Face VL layout (model.language_model.* beside
  model.visual.*) that Qwen3-VL ships, skipping the vision half — this is what let the text half be packed at all
  for the TQVL work.

Verified

• ctest q4nx_mrope npu_rmsnorm_offset q4nx_layer_kinds → 3/3.
• On the real container (Qwen/Qwen3-VL-2B-Instruct text half, packed to q4nx): the load line flips from
  mrope=0 to mrope=1, with rope_theta=5e+06 unchanged and the derived geometry still
  H=2048 NL=28 NH=16 NKV=8 HD=128 IM=6144 NV=151936.
• tools/copyright.py --check rc 0.

refs #1311 (TQVL: Qwen3-VL-2B, text decoder on our NPU). End-to-end numbers for the model itself stay in the TQVL
state file, not here.

[bong-water-water-bong]

#1415 ready for review at a6be23ab — two spellings, one flag, plus the controls that keep it honest (yarn/linear rope_scaling is not mrope, mrope_interleaved:false stays plain). ctest 3/3; on the real Qwen3-VL-2B container the load line flips mrope=0 → mrope=1 with theta 5e6 unchanged. Context: with text-only ids the two are bit-identical (proved over 8 positions incl. 262143), with image ids they differ by 0.29 rad — so the flag matters exactly where the TQVL hybrid puts image tokens.

[bong-water-water-bong]

VERDICT: READY at a6be23ab10b85f7654b15432141ac7c55d092f2a.

A silent-wrong-behaviour fix, so I built and ran its test here (it needs -std=gnu11 for fdopen; CMake's default GNU dialect is what builds it in the tree):

$ gcc -std=gnu11 -I… tests/q4nx_mrope_test.c npu/forward/src/q4nx_semantics.c -o /tmp/mrope && /tmp/mrope
ok   rope_theta still read from the top level (5e6, not lost)
ok   rope_type "default" is not flagged as a named variant
ok   rope_parameters.mrope_interleaved -> mrope=1 (unchanged)
ok   rope_parameters.rope_theta still read
ok   a rope_scaling block WITHOUT mrope_interleaved is NOT mrope      <- the row that matters
ok   theta untouched by the control case
ok   mrope_interleaved:false stays plain
q4nx_mrope: ok

The two negative controls are what make this a fix rather than a relaxation: a rope_scaling block that merely exists does not set mrope, and mrope_interleaved: false stays plain — so the change cannot start rotating text-only prompts (which the body shows would be a regression: in the rotate-half layout the numbers are equivalent for text-only, but not for the multi-section case). And the tolerance is right: it reads HF's rope_scaling spelling in addition to rope_parameters, leaving the FastFlowLM/OGA path asserted unchanged rather than replaced.

The bug itself is the kind this fleet keeps finding: a container that silently loads with mrope=0, where "nothing said so" is the actual defect — a log line or a refusal naming the unrecognised key would have caught it, and the test now pins the recognition instead.

[bong-water-water-bong]

VERDICT: READY at a6be23ab10b85f7654b15432141ac7c55d092f2a

The failure this closes is the worst shape a config bug can take — silently exact, then wrong only once an image is in the prompt — and the fix is a second spelling read with the controls to keep it from over-firing.

The test compiles and passes here (g++-16 -x c tests/q4nx_mrope_test.c npu/forward/src/q4nx_semantics.c → 0 errors, q4nx_mrope: ok), and its arms are the right five:

ok   rope_parameters.mrope_interleaved -> mrope=1 (unchanged)      the spelling that already worked
ok   rope_parameters.rope_theta still read                          the old block still parses
ok   a rope_scaling block without mrope_interleaved is NOT mrope     negative control
ok   theta untouched by the control case                            the control is not collateral
ok   mrope_interleaved:false stays plain                            the "explicitly off" case

The reasoning is the part worth keeping in the comment: a Hugging Face qwen3_vl config declares the same thing under rope_scaling, and a container that missed it loaded as mrope=0 — plain RoPE, silently: exact for a text-only prompt (with pos_t = pos_h = pos_w the two are bit-identical, measured), wrong by ~0.29 rad on the rotary block once image tokens are in it. A config read that is right for every test anyone would write and wrong for the only prompt that matters is exactly the class that needs a control arm, and there are two (a yarn/linear rope_scaling block must not be read as mrope; mrope_interleaved:false must stay plain).

The converter half is the same insight from the naming side: a FastFlowLM/OGA pack nests the language model under language_model.*, while a Qwen3VLForConditionalGeneration nests the same tensors under model.language_model.* beside model.visual.* — so both spellings map to the names Q4nxNpuForward loads, and the vision half stays unread (which is what the NPU text decoder wants). Mapping both rather than guessing is the difference between "this pack imports" and "this pack imports the wrong tensors".

Not run: a real Qwen3-VL pack end to end (needs the 8B VL container on the NPU). What I can take — that the parser accepts both spellings, that three controls refuse to over-read, and that the converter documents both nestings — is green.

[bong-water-water-bong]

VERDICT: READY at b699616e6ce6f622d890aa07878676b63bdcb47a

Main merge only over the head I reviewed: the PR's own added lines hash to 5bbd02acaa6c at a6be23ab and at this head, so the interleaved-MRoPE reading, the three controls and the converter's two nestings are unchanged. My build-and-run at that head (q4nx_mrope: ok, five arms green, 0 compile errors) still describes this tree — the test is device-free and drives the parser directly.

[bong-water-water-bong]

VERDICT: READY at b699616e6ce6f622d890aa07878676b63bdcb47a (re-verified after the merge).

Three things checked at the new head, in the order my own mistakes have taught me:

$ git diff a6be23ab b699616e -- npu/forward/src/q4nx_semantics.c tests/q4nx_mrope_test.c tools/safetensors_to_q4nx.py
   (empty)                       # the merge of main touched none of this PR's own files
$ python3 tools/copyright.py --check
   (clean)                       # this PR adds a file, so the notice gate applies: it carries one
$ gcc -std=gnu11 -I… tests/q4nx_mrope_test.c npu/forward/src/q4nx_semantics.c -o /tmp/mrope2 && /tmp/mrope2
ok   a rope_scaling block without mrope_interleaved is NOT mrope
ok   theta untouched by the control case
ok   mrope_interleaved:false stays plain
q4nx_mrope: ok

So the earlier READY stands on the same evidence — 7 rows, both negative controls green — and the merge changed nothing about it.

(The copyright line is in there because I missed exactly that gate on engine#1406 an hour ago: a PR that adds a file gets copyright.py --check before the verdict now, not after CI goes red.)

[bong-water-water-bong]

VERDICT: READY at b699616e.

Its own three files are byte-identical to the head that carried the READY (q4nx_semantics.c, tests/q4nx_mrope_test.c,
tools/safetensors_to_q4nx.py), and the ~1400-line move since then is main arriving (npu_plan_op_reference.cpp from #1409, the pins,
registry/architectures.json). So the previous verdict describes these bytes; I rebuilt and ran the test rather than only saying so:

gcc -std=gnu11 -Inpu/forward/include tests/q4nx_mrope_test.c npu/forward/src/q4nx_semantics.c -lm   -> rc 0
/tmp/mrope                                                                                          -> rc 0, 8 "ok", "q4nx_mrope: ok"

(matching CMakeLists.txt:1106-1108, which registers q4nx_mrope_test and links m.)

The change is the right shape: interleaved MRoPE is now read from both spellings — OGA/FLM's nested rope_parameters block and
Hugging Face's rope_scaling (Qwen3-VL) — and the test pins the negative control, which is the part that makes it worth having: a
rope_scaling block carrying yarn/linear must not be read as mrope, and mrope_interleaved: false must stay plain. Without those a
"detect mrope" rule would quietly turn every rope_scaling pack into interleaved.

The failure mode the comment names is the one that costs a day later: reading only the first spelling left a qwen3_vl container as
mrope=0 — plain RoPE, silently, because with pos_t = pos_h = pos_w the two are bit-identical for a text-only prompt and only diverge
(~0.29 rad on the rotary block) once image tokens are in the sequence. A silent correctness cliff that only multimodal input crosses is exactly
what a host test is for, and this one asserts both sides of it.

Two flag traps I walked into, recorded because they would have produced a false report against this PR: compiling with -std=c11 fails on
fdopen (needs POSIX feature macros; the project uses gnu11), and without -lm the link fails on sqrt — CMake links m for this target,
so both are my invocation, not their code. My first attempt also printed a passing run from a binary whose compile had failed, which is the
stale-artifact trap in a new costume: I only trust a run now when the compile's own rc is in the same output.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 799 - Partially compliant Compliant requirements: Plan schema and loader implemented in npu/plan/ tools/npu_plan.py generates plan.json Q4nxNpuForward now consumes plan.json for design derivation t2 chunk kind support added in q4nx.h and converters NpuTarget interface introduced with XDNA target implementation Targets refuse unsupported configurations with named reasons Non-compliant requirements: Hexagon target implementation not yet included in this PR Drafter protocol shared not yet implemented int8 KV and prefill chunks not yet implemented Requires further human verification: Verification that the NPU targets properly refuse unsupported configurations Testing of the Hexagon target implementation once available Verification of the drafter protocol shared functionality 1311 - Partially compliant Compliant requirements: Implementation of MRoPE handling for Qwen3-VL models Support for embedding models with --device npu Correctness verification for MRoPE handling Non-compliant requirements: Full embedding model serving implementation not yet included Performance measurement and reporting not yet implemented Requires further human verification: Full embedding model serving functionality testing Performance measurement and reporting verification

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review MRoPE Declaration Handling The code now correctly reads mrope_interleaved from both rope_parameters and rope_scaling blocks. This is important for correctly handling Qwen3-VL models which declare MRoPE in the rope_scaling block. The change ensures that the engine will not silently load such models as plain RoPE, which would be incorrect for prompts containing image tokens. /* Interleaved MRoPE is declarable in two spellings and only one was read here. FastFlowLM/OGA packs and * Qwen3.5 nest the rotary block under `rope_parameters` (the block below); a Hugging Face `qwen3_vl` config * (Qwen3-VL, engine #1311/TQVL) declares the same thing under `rope_scaling`. Reading only the first meant * such a container loaded as mrope=0 -- plain RoPE, silently: exact for a text-only prompt (with * pos_t = pos_h = pos_w the two are bit-identical, measured), wrong by ~0.29 rad on the rotary block once * image tokens are in it. tests/q4nx_mrope_test.c pins both spellings and the controls (a yarn/linear * rope_scaling block must NOT be read as mrope). */ { const char* rs = value_for_top(json, "rope_scaling"); double mr = 0; if (rs && *rs == '{' && get_num(rs, "mrope_interleaved", &mr) && mr != 0) s->mrope = 1; } Qwen3-VL Tensor Name Handling The qwen35_rename function in tools/safetensors_to_q4nx.py has been updated to correctly handle Qwen3-VL tensor names. It now properly distinguishes between visual and language model tensors, ensuring that only the relevant tensors are processed for the text decoder. This change is crucial for correctly packing Qwen3-VL models. """A VL pack's HF tensor name -> the name Q4nxNpuForward loads (engine #796, and Qwen3-VL's layout). Two spellings reach us and both mean the same thing: a FastFlowLM/OGA-style pack nests the language model under `language_model.*`, and a Hugging Face `Qwen3VLForConditionalGeneration` nests the same tensors under `model.language_model.*` beside `model.visual.*` (the vision half, which the NPU text decoder never reads). """ if name.startswith("model.visual.") or name.startswith("visual."): return None # the text path: the forward never reads these prefix = "model.language_model." if name.startswith("model.language_model.") else "language_model." if not name.startswith(prefix): raise SystemExit(f"{name}: not a qwen3_5 or qwen3_vl text tensor; expected " f"language_model.*, model.language_model.* or (model.)visual.*") rest = name[len(prefix):]