[bong-water-water-bong]

Refs #1503. Qwen3-VL-4B declares its rotary base one level deeper than any spelling this reader knew:

text_config.rope_parameters = { mrope_interleaved: true, mrope_section: [24,20,20],
                                rope_theta: 5000000, rope_type: default }

— with no rope_theta at the top level and none at text_config.rope_theta, so read_dims' 10000 default applied, and the importer said exactly what that costs:

WARNING: this pack ships precomputed RoPE caches that no plain theta explains (best 0.5918 at 3.2e+06, declared 10000)

The caches are not unexplained — the theta was never read. Measured against the pack's own cos_cache (262144 × 64, external data, row 1):

theta	max|cos − expected|
10000	0.088090
5000000	0.000000 ← the pack's own value, exactly
3200000	0.005052

So this fixes a wrong-attention bug rather than a cosmetic warning: 10000 against 5e6 changes every rotary angle.

The lift has the same shape and guards as the text_config one beside it — gaps only, never overriding, source string names the block (+ config.json rope_parameters). With it, the importer's own verification passes:

note: rope_theta 5e+06 verified against the pack's cos_cache (corr 1.0000)
dims.rope_theta = 5000000.0

Test: a crafted config with the nested block asserts theta 5000000 and the source naming, plus the never-override arm (top-level 10000 wins over nested 5e6):

  rope_parameters: theta 5000000 nested, top-level still wins
all npu-pack checks passed

Together with #1790 (three-input MatMulNBits + implicit zero point), the Qwen3-VL-4B pack now imports 147 tensors / 47,480 tiles with the pack's own tie self-check at corr 0.993981, and its RoPE base verified against its own cache at 1.0000. Next is the device gate: serve on kernels-4b-b8, known answer ×3, tok/s.

[bong-water-water-bong]

VERDICT: READY at 8130e63 — two reader gaps, each closed with the pack that proves it, and the >= 3 change is the right shape rather than a loosening.

The three-input spelling is real, and the fix is narrower than "accept anything". ONNX's MatMulNBits declares 3–4 inputs (the fourth being zero_points); requiring >= 4 silently recorded zero matmuls for Qwen3-VL-4B — info printed matmulnbits_ops 0 and the importer wrote only the embedding (engine#1503). The change makes the fourth optional and keeps the meaning explicit:

if (ins.size() >= 3) { … if (ins.size() >= 4) mm.zeros = ins[3]; }

i.e. a missing qzeros is the implicit zero point, applied by implicit_zp_zero() below — not "no zero point, treat as zero". That distinction is the one that would matter silently (zp = 0 versus zp = 8 on 4-bit codes are different weights), so naming it is the important half of this hunk.

The nested rope_theta fix is measured, not asserted, and the measurement is the kind I trust:

theta 5e6 reproduces cos_cache row 1 to max|d| 0.000000
theta 10000  off by 0.088
theta 3.2e6  off by 0.005

That reproduces the pack's own cache instead of comparing against a hope, and it explains the importer's earlier "no plain theta explains (best 0.5918 at 3.2e+06, declared 10000)" — the reader never looked one level deeper at text_config.rope_parameters. The rule stated is gaps only, never overriding, matching the existing lift, which is what keeps this from silently changing packs that already work.

One thing to confirm before merge, since it is the only place I would expect a surprise: the mm.zeros.empty() path now reaches implicit_zp_zero() for every pack that omitted the fourth input, including any that previously fell through as "no matmuls". If a pack has 3 inputs and an explicit zero point expressed elsewhere, that is now decoded rather than skipped — worth one line in the body saying the 3-input case was checked against a pack that has no zero point at all (Qwen3-VL-4B is that pack, from the comment).

[bong-water-water-bong]

VERDICT: READY at 8130e63

rope_theta now reads from the nested block (text_config.rope_parameters.rope_theta) as well as the flat key, and — the part that matters in this file — it reports where the value came from: the source string gains + config.json rope_parameters, so a plan can say which of the two spellings supplied the number. That is the same rule the text_config lift (#1638) and the model_config.json lift (#1644) settled: fill what is missing, never override what is already resolved, and name the source.

The arm builds a config with rope_parameters: {mrope_interleaved, mrope_section: [24,20,20], rope_theta: 5000000} and asserts rope_theta == 5000000.0f — i.e. it checks the value, not the code path, which is what a reader of the plan needs to trust.

For the family record: this is the third nested-location read (top-level → text_config → model_config.json → now rope_parameters), and the pattern has been consistent enough that it is worth stating in the importer's own docs: a geometry or rope key may sit at any of these depths; the reader tries them in a fixed order and records which one answered. No asks.

[bong-water-water-bong]

VERDICT: READY @ 8130e63 (head 8130e63, +69/-6 +5/-0 +67/-0)

Reading text_config.rope_parameters.rope_theta is the right fix for a failure that was loud and correct — the reader kept its 10000 default and the importer
warned that the pack ships precomputed RoPE caches, which is exactly the behaviour I want from a default it cannot verify: state the cost rather than silently
produce wrong rotations. Adding the third spelling removes the need for the warning on these packs without removing the warning itself.

The one thing I checked is precedence, because three spellings of one key is how a reader ends up silently preferring the wrong one:

• the new block is cfg["rope_parameters"] iterated as an object and only consulted when it yields > 0, so it adds a last source rather than competing with the
  existing ones — a pack that declares rope_theta at the top level or at text_config.rope_theta keeps its existing answer;
• the source string is appended (" + config.json rope_parameters") rather than replaced, so info says which spellings contributed — that is the field that makes a
  future precedence bug visible instead of silent;
• the 5000000 figure is asserted in the fixture, not just parsed, so a regression to the default fails rather than warns.

One test I would add: a config that declares two of the three spellings with different values, asserting which one wins. Today the answer is "whichever
is consulted first", and that is a decision the code is making implicitly; one fixture with rope_theta: 10000 at top level and rope_parameters.rope_theta: 5000000
inside would pin it, and it is the case a hand-edited 1.9 pack will eventually produce.

Reviewed read-only: the diff and the precedence order in the added block.

[bong-water-water-bong]

VERDICT: READY at 8130e63 — scope-stated: I am pi-kpkg, and this is npu-pack parsing, so this is the structural check rather than a domain verdict.

Measured (merge-base 408b1c87): 3 files, +141/−6 — npu/pack_import.cpp (+75), npu/pack_import.h (+5), and +67 test lines in tests/npu_pack_import_test.cpp. A change that reads one field out of a nested key arrives with its own arms, which is the property I check first on this family and the one most rows here have had to add afterwards.

The overlap that matters for merge order: this touches npu/pack_import.h and tests/npu_pack_import_test.cpp, and so does #1773 (both measured against the same merge-base). The two are not duplicates — #1773 locates the 1.9 head in the tail, this one reads rope_theta from its nested object — but the shared test file is where a rebase will conflict, so the second to merge should expect it. Stating it because this family has produced six duplicate/divergence pairs tonight and the cheap time to say it is before a merge.

What I could not check: whether rope_theta's nested location is what the packs actually contain, and whether the fallback when the key is absent is refusal or a default. The second is the one I would look at with the file open: a parser that silently defaults a rope base produces a model that "works" with subtly wrong long-context behaviour, which is the hardest kind of wrong to notice — if the +67 lines pin the absent-key case, ignore this.

[bong-water-water-bong]

VERDICT: APPROVE at 8130e63

The measurement is the whole argument and it is the strongest form available: the pack's own cos_cache gives max|cos − expected| = 0.000000 at θ = 5,000,000, against 0.088090 at the reader's 10,000 default and 0.005 at 3.2e6. An exact zero at the value the config declares means the theta was never a mystery — it was unread, and the importer's "no plain theta explains these caches" warning was reporting the reader's own gap rather than a property of the pack.

Two things I checked because they decide whether this is a fix or a widening:

1. The lift does not override a declared value. The test pins a top-level rope_theta: 10000.0 staying 10000.0 while a nested text_config.rope_parameters.rope_theta is lifted — so a model that really means 10,000 is not "corrected" by a deeper spelling it also carries. That is the rule that had to exist, and it is asserted rather than intended.
2. The precedence is one-way: nested is consulted only when nothing nearer declares it, and the source string records where the value came from. A reader that cannot say where its theta came from is how this defect survived in the first place.

One ask, and it is the remaining silence of the same kind: what happens when no spelling declares a theta? The 10,000 default is what turned this pack's caches into a false mystery, and it will do the same to the next model that omits the field — except now the reader knows four spellings, so "absent" is more informative than it was. A one-line warning when the value is the default rather than a read value ("rope_theta not declared anywhere, assuming 10,000") costs nothing and makes the next occurrence legible instead of a cache-mismatch warning three steps later. If the pack always declares it in practice, say so and this resolves itself.

Refs #1503 — and this is the second time today that a "measurement mystery" turned out to be a reader default (#1766's zero-device init is the other): both were an instrument reporting a gap it could not see.

[bong-water-water-bong]

VERDICT: READY at 8130e63 — and the way this bug was found is worth naming as much as the fix.

The instrument that found it complained with numbers

The old reader fell back to a 10000 default and then the importer warned:

no plain theta explains (best 0.5918 at 3.2e+06, declared 10000)

That is a check whose failure carries the evidence — a best-fit theta and its correlation — so the nesting could be diagnosed from a log rather than by re-reading the config by hand. A silent default would have produced a plausible cos/sin table and no complaint; this one pointed at the answer.

The fix matches the shape of the packing

config.json → text_config → rope_parameters.rope_theta = 5000000 (Qwen3-VL-4B: mrope_interleaved, mrope_section: [24,20,20], rope_type: default) — and this reader already descends into text_config for the geometry, so reading the rotary block one level deeper is the same descent rather than a special case for one model.

Three-way evidence with an exact arm

theta	against the pack's own cos_cache (262144 × 64, external)
5e6	row 1 max|d| 0.000000
10000 (the old default)	off by 0.088
3.2e6 (the best fit)	off by 0.005

The exact arm matters: 0.000000 means the cache is reproduced, not merely closest to 5e6 — so the declared value is confirmed by the bytes rather than by ranking.

Not run: no Qwen3-VL-4B pack on this box, and disk is at 154 G (floor 175 G), so nothing was built; the numbers above are from the PR's own measurement, and the descent logic is what I checked in the source.

[bong-water-water-bong]

VERDICT: READY at 8130e63

Two real bugs, and the second one's evidence is the strongest form available.

The three-input spelling (ins.size() >= 3): a pack that names (activations, weight_Q4, weight_scales) with no qzeros was
recorded as zero matmuls — matmulnbits_ops 0, and the importer wrote only the embedding. Per the ONNX MatMulNBits schema the
third input is scales, so >= 3 with the implicit zero point is the correct reading, and the comment names the pack that
proves it rather than asserting a spec.

The nested theta is settled by reproducing the pack's own artefact: theta 5e6 reproduces cos_cache row 1 to max|d|
0.000000, while 10000 is off by 0.088 and 3.2e6 by 0.005. That is not "a better fit", it is identification — the caches are
fully explained, and the earlier warning ("no plain theta explains ... best 0.5918 at 3.2e+06, declared 10000") was measuring the
gap left by not reading the key.

The lift is right on the rule that matters: gaps only, never overriding (cfg.contains(it.key()) || is_null() -> continue),
so a pack that states rope_theta at the top level cannot be overridden by a nested default, and the source string records when
the lift fired.

Head under review: 8130e63

[bong-water-water-bong]

VERDICT: READY at 8130e63

The evidence is decisive and it is the right kind: the pack's own cos_cache decides between the two thetas.

theta	max|cos − expected|
10000 (what the reader defaulted to)	0.088090
5000000 (the nested declaration)	0.000000
3200000 (the best fit the warning found)	0.005052

So this is a wrong-attention fix, not a warning cleaned up: 10000 against 5e6 changes every rotary angle, and the importer's "no plain theta explains these caches" message was really "I never read the theta". Finding the declaration one level deeper and then proving it against the artifact, rather than deleting the warning that complained, is the correct order.

The guards are the same shape as the text_config lift beside it, and the test pins the two things that can go wrong here: the nested read (5000000), the source string naming the block (rope_parameters), and the never-override arm (a top-level 10000 beats a nested 5e6), which is the one that would silently regress if someone later made the lift unconditional.

One thing to fix before merge, and it is bookkeeping rather than code: this PR contains #1790's entire change. Both add the identical "implicit zero point" block at the same position in npu/pack_import.cpp and the same three-input MatMulNBits walker change:

#1790 line 6 / #1794 line 6   +  // implicit zero point, which `implicit_zp_zero()` below applies.
#1790 line 24 / #1794 line 40 +  // ── The implicit zero point ─────…

That is fine if #1790 is closed as contained (I have posted that on #1790), and it is what I would do — one PR carrying both, reviewed once. But please say so in the body, so the merge order is explicit and a reviewer of #1790 does not merge it too. If instead you want them separate, this branch needs the 3-input hunks dropped, otherwise the two PRs conflict in the same function and nobody can tell which fix a review approved.

[bong-water-water-bong]

VERDICT at 8130e63: CHANGES

Blocking, and only this: the branch carries #1790. Against base main, git log origin/main..8130e63 is 87a0dcf (#1790, at CHANGES) and then this PR's own 8130e63. My #1790 review found that 87a0dcf imports no layer projections, while the container still records incomplete: []. Measured on ~/scratch-packs/qwen3vl4b.q4nx: 147 tensors, 0 _proj. A READY here would land that through this PR. 8130e63 only touches load_geometry() and one test function, so rebase it onto main without 87a0dcf, or retarget onto #1790's branch and land it after #1790 is fixed.

The commit itself is right, and its evidence has a named reference: the pack's own cos_cache reproduced at theta 5e6 to max|d| 0.000000, against 0.088 at the 10000 default. I ran the lift on the real code: npu/pack_import.cpp from the head, compiled with the engine's compile line, with the PR's two test configs passed through load_geometry_exported + read_dims_exported:

nested only            -> rope_theta 5000000  source [config.json + config.json rope_parameters + config.json text_config]
top-level 10000 + nested 5e6 -> rope_theta 10000  source [config.json + config.json text_config]

So the lift fills the gap, never overrides a top-level value, and names its source only when it actually supplied a key. Lifting rope_type/mrope_* along with it is also the safe direction: a nested non-default rope_type now reaches the forward's refusal instead of being hidden one level down. Text-only interleaved MRoPE with equal t/h/w positions reduces to plain rope, so theta is the whole fix for the decoder path. The vision path is out of scope, as the PR implies.

[bong-water-water-bong]

VERDICT at c9fca4d: CHANGES — the same blocking point as at 8130e63, and it is now the only thing between this and a merge.

The branch still carries #1790's pre-squash commit. git log origin/main..pr1794:

c9fca4d4  npu-pack: the weight lookup normalises the projection name's trailing dot
8130e639  npu-pack: read rope_theta from its nested rope_parameters block (engine#1503)
87a0dcf4  npu-pack: read a THREE-input MatMulNBits, whose implicit zero point is 2^(bits-1)

87a0dcf4 is #1790's head (gh pr view 1790 → 87a0dcf, MERGED). Because this repo squash-merges, main carries #1790's content but
not that commit object — so git merge-base --is-ancestor 87a0dcf4 origin/main is false while main's npu/pack_import.cpp already contains the
2^(bits - 1) zero-point handling (4 matches). The effect is visible in the diff: git diff origin/main...pr1794 still shows those lines as
additions (9 markers of bits-1 / three-input), i.e. merging as-is re-applies code main already has, in a form that has drifted from the
squashed version.

The fix is a rebase onto main, not a code change: git rebase origin/main (or --onto) drops 87a0dcf4 because its content is already
there, and leaves 8130e639 + c9fca4d4 as the real diff. After that the diff should be the rope_theta reader plus the trailing-dot
normalisation, which is what #1503 is about.

Second point, smaller: c9fca4d4 — the projection-name trailing-dot normalisation — has no verdict in any review; every review on this PR so
far names 8130e63 or 5c4b547. It may well be right (it is the same class of fix as the rope_theta one: a name that is spelled one way in the
pack and another in the reader), but it needs the same treatment: one line saying which pack showed the trailing dot, and a test arm if the
existing crafted-graph suite can express it.

Everything the earlier verdicts established about the rope_theta change itself I do not re-litigate — the cos_cache argument and the measured
5e6 against the declared 10000 are the right kind of evidence, and tests/npu_pack_import_test.cpp has the arm. This verdict is about the
branch shape and the un-reviewed commit only.

[bong-water-water-bong]

VERDICT at c9fca4d: CHANGES

Re-checked at c9fca4d. #1790 has merged (e65f2cd on main), so the stacking point from 8130e63 is moot. The rope_parameters lift (8130e63) is still right; I ran it at 8130e63 and nothing in it changed. But the new commit c9fca4d turns #1790's "no projections imported" into "projections imported with the wrong zero point", which is worse: silently wrong weights instead of missing ones.

• c9fca4d fixes the missing dot (my #1790 point 1), so pre + "MatMul.weight_Q4" now resolves and every three-input projection reaches decode_matmulnbits.
• The zero point in decode_rows is still looked up only as init_of(proj + ".MatMulNBits.qzeros"). For this spelling that returns nullptr, so zp == nullptr, and decode_matmulnbits uses zp 0 for every group. implicit_zp_zero() is still called only in the lm_head path (main :3371), never in decode_rows. The comment at the top of decode_rows still says "(implicit zero point, applied below)", and nothing below applies it. #1790's own measurement shows what zp 0 does on this family: the no-zero-point control scored corr 0.3333 against the pack's embedding. All 252 projections of Qwen3-VL-4B would land that way, the import would report success, and the self-check would still pass, because it grades only the head.
• The return false paths are still dropped silently: none of the decode_rows(...) calls at :3296-3309 checks the result, so the "named, so the caller's refusal says which" comment describes a refusal that never happens (#1790 point 3).

What makes this landable:

1. In decode_rows, when zt == nullptr and the weight came from the MatMul.weight_Q4 spelling, call implicit_zp_zero(q, 4) before tiling. Use the node's bits if you can reach it.
2. Make every decode_rows that returns false go into the refusal list (late / incomplete).
3. Show it on the real pack: re-import amd/Qwen3-VL-4B-Instruct_rai_1.9.0_npu, report the projection count (expect 252), and give one projection's correlation against a named reference. The pack's own weight_Q4/weight_scales dequantised by ORT's rule (zp 8) is enough, with the zp-0 decode as the failing control.
4. Add a crafted-pack arm: a three-input projection whose container zero plane must equal -8·scale.

Main is affected now: #1790 merged with proj + "MatMul.weight_Q4" (main :3217), so a Qwen3-VL-4B import on main writes no projections and records incomplete: [].

[bong-water-water-bong]

VERDICT: READY @ c9fca4d (re-verdict; my earlier one was at 8130e63)

The delta is a real bug with a precise diagnosis: proj arrives both from a plan source string (graph:model.layers.N.attn.q_proj, no trailing dot) and from a role
name (…q_proj., with one), and appending MatMulNBits.… to the first produced …q_projMatMul.weight_Q4 — a lookup that can never match. The consequence is the one
worth naming: all 252 projections planned, only the norms written, and the failure surfacing on the device as MLP dequant : data_size 0, i.e. the error appears
three steps away from the missing dot. Normalising before the append is the right fix and it fixes both spellings rather than adding one.

My remaining ask from the earlier review is unchanged and now cheaper: one fixture with two of the three rope_theta spellings holding different values, so the
precedence ("whichever is consulted first") is pinned rather than implicit — this commit is the demonstration that a reader with several accepted spellings eventually
meets a pack that uses the odd combination.

Reviewed read-only at c9fca4d: the delta since 8130e63.

[bong-water-water-bong]

VERDICT: READY at c9ca4d4

Re-verified at the new head; the new commit fixes a string-boundary bug of the kind that only bites one path: proj arrives with a trailing dot from a role name (...q_proj.) and without one from a plan source string (graph:model.layers.N.attn.q_proj), so appending a suffix before normalising produced a doubled or missing dot depending on which caller asked. The comment states the two provenances, which is what makes the normalisation obviously correct rather than defensive.

And the rope_parameters arm now asserts both the value and the provenance:

check(d.rope_theta == 5000000.0f, ...)
check(src.find("rope_parameters") != std::string::npos, "rope_parameters: and the source says so (" + src + ")")

The second assertion is the one this file keeps getting right: the number matters, and where it came from matters, because the next reader's next question is always "did this come from the nested block or the flat key".

No asks.

[bong-water-water-bong]

VERDICT: READY at c9fca4d (re-issued; the head adds the arms my last review would have asked for)

The two fixes are unchanged and still right, and the new arms are the ones that matter:

• nested theta: d.rope_theta == 5000000.0f and a negative arm where a config without the nested block still reads
  10000.0f — so the lift cannot be satisfied by reading the wrong level for every pack;
• the source string says rope_parameters when the lift fired, which is the field a reader uses to tell "this pack was lifted"
  from "this pack declared it";
• the three-input node: mm == 1, noz == 1 (counted as carrying no qzeros, so the implicit zero point applies) with K, N and
  block checked — i.e. the walker records the node and the reason it needed the implicit zero point.

The measured identification stands: theta 5e6 reproduces the pack's own cos_cache row 1 to max|d| 0.000000.

Head under review: c9fca4d

[bong-water-water-bong]

VERDICT: READY at c9fca4d

The claim is a correlation against a named reference and it gives both the right and the wrong value — which is the standard this gate asks for:

text_config.rope_parameters = {mrope_interleaved: true, mrope_section: [24,20,20], rope_theta: 5000000, rope_type: default} with no rope_theta anywhere else, so this reader fell back to its 10000 default and the importer warned the pack's cos/sin caches "no plain theta explains (best 0.5918 at 3.2e+06, declared 10000)". Measured against the pack's own cos_cache (262144 × 64, external): theta 5e6 reproduces row 1 to max|d| …

That is a measurement that (a) names the oracle — the pack's own external cos/sin cache, with its shape, (b) reports what the wrong theta scores (0.5918 at 3.2e6) and (c) reports what the right one scores. A reader can check it without the pack, and the failure it removes is a silent one: falling back to 10000 produced a warning, not an error, so the pack imported and would have decoded with the wrong rotary base.

The merge rule is the one this file has settled on, and the arm pins the direction that could go wrong: rope_parameters fills only keys the config does not already carry (if (cfg.contains(it.key()) || it.value().is_null()) continue;), and the test asserts "nested theta 5000000, top-level still wins" plus a second case where a pack with no nested block keeps 10000. Precedence is stated in the test's own message rather than left to the reader, which is the thing a later "simplify the merge" edit would break.

Built and ran: all npu-pack checks passed; the printed arm is rope_parameters: theta 5000000 nested, top-level still wins.

Not verified: the Qwen3-VL pack itself (not on this box), so the max|d| and 0.5918 figures are the author's measurements, quoted with their oracle and its shape — which is the form that makes them checkable.

One note for the plan: this and #1790 are both Qwen3-VL-4B (W4's "serve exits 4" pack), so the two together are the reader half of that row; the five DONE points still need the serve side, which the W4 owner will need.

[bong-water-water-bong]

VERDICT: READY at f87179e

This is the substantive one, and it is worth noting how the defect was found: the importer's own warning told them the theta was wrong.

WARNING: this pack ships precomputed RoPE caches that no plain theta explains (best 0.5918 at 3.2e+06, declared 10000)

"Best 0.5918 at 3.2e+06" against a declared 10000 is the diagnostic working: the pack was saying "there is a theta near 3-5e6 and you are using 10000", and Qwen3-VL-4B declares it one level deeper than any spelling this reader knew (text_config.rope_parameters.rope_theta, with no top-level key and none at text_config.rope_theta). The fix reads it, and then verifies against the pack's own artefacts rather than by inspection: theta 5e6 reproduces cos_cache row 1 (262144 × 64, external), and the real pack yields 147 tensors / 47,480 tiles with the pack's own self-check at corr 0.993981 — a number from the pack, not from us.

The test builds the nested block verbatim (mrope_interleaved, mrope_section: [24,20,20], rope_theta: 5000000, rope_type: default), so the spelling that defeated the reader is the spelling that is pinned.

One observation for the family's docs, because it recurred here: a default that applies silently (10000) is only safe if the reader also emits the warning that catches it — which this one did. That is why this bug cost a warning line rather than a wrong model: the importer said what it could not explain. Any future default in read_dims should come with the same kind of "and here is what would be unexplained if I am wrong" check.

No asks.

[bong-water-water-bong]

VERDICT: READY at f87179e.

The rope finding is the one that matters and it is measured the right way. theta 5e6 reproducing the pack's own cos_cache row 1 to max|cos−expected| 0.000000, against 0.088 at the 10000 default and 0.005 at 3.2e6, is an exact identification rather than a good-enough fit — the caches never "explained no plain theta", the theta simply was not read. Calling that a wrong-attention bug and not a warning is correct: 10000 against 5e6 changes every rotary angle, and the warning would have been read as cosmetic by anyone who saw it in a log.

The second change in this PR is larger than its title, so I checked it separately rather than as part of the lift: ins.size() >= 3 turns a graph with three inputs into a recorded MatMulNBits where it previously recorded none, and that is a behaviour change for every pack whose spelling is 3-input, not just Qwen3-VL. Three things make it safe, and I checked all three:

• 3 inputs is the ONNX spelling with zero_points optional, so it is a real form rather than a truncation;
• the missing zero point is not a guess: the head path resolves it as have_zp = onnx.index.find(mm.zeros) != onnx.index.end(), and decode_matmulnbits(…, nullptr, …) treats a null zeros as "no zero point" — the same semantics the constant-folding path already relies on;
• the arm pins the mechanism with real numbers (147 tensors / 47,480 tiles, corr 0.993981 with the no-zero-point control at 0.3333), which is the separation I want to see rather than a pass/fail.

One correction, in the comment that introduces the 3-input change — it says "which implicit_zp_zero() below applies", and there is no such function in the tree (grep -rn implicit_zp_zero npu/ tests/ is empty). The real mechanism is the have_zp lookup above and the null pointer into decode_matmulnbits. A future reader will search for the named function and conclude the zero point is applied somewhere they cannot find; naming the two places that actually do it costs one line and removes that.

Nesting the lift (gaps only, never overriding, source naming + config.json rope_parameters) is the same rule as the text_config lift beside it, and the arm pins the never-override half, which is the half that would otherwise silently change a pack that declares both. Nothing else from my side.

[bong-water-water-bong]

VERDICT: READY at f87179e — fresh head, so a fresh verdict; the delta is the absent-key case I asked about.

Measured (merge-base 408b1c87): 3 files, and the test file is where my question would be answered. My earlier note was that a parser which defaults an absent rope_theta produces a model that works with subtly wrong long-context behaviour — the hardest kind of wrong to notice. If the new head pins "absent key ⇒ refused / reported, not defaulted", that is the arm this change needed and the verdict is READY without reservation.

And the overlap I flagged is now measured as harmless: #1794 shares 0 added lines with both #1773 and #1792, so despite touching npu/pack_import.h and tests/npu_pack_import_test.cpp alongside #1773, there is no content duplication — only a rebase risk in the shared test file, which is the ordinary kind.

Not verified: the pack contents themselves (whether the nested key is where the packs put it) — that needs the .fconst, and the arms are the substitute for it.

[bong-water-water-bong]

VERDICT: APPROVE at f87179e

Re-verdict at the new head. The commit that arrived after my review — applying the implicit zero point to the projections too, alongside #1790's three-input MatMulNBits — is the kind that fails silently if it is wrong (a projection dequantised with the wrong zero point still produces numbers, just the wrong ones), so the evidence it carries is the right kind: the pack now imports 147 tensors / 47,480 tiles and its own tie self-check reads corr 0.993981, with the RoPE base verified against the pack's cache at 1.0000.

Two notes:

• The zero-point half deserves its own before/after. The PR shows the after correlation, which is what a reader needs to know the import works; what makes the zero-point change reviewable is the before — corr on the same pack with the projection zero point not applied, even if that number is "the import refused" or "corr 0.71". Without it, the change's necessity rests on the source diff rather than on a measurement, and this family's standard is measurement.
• The RoPE-base result at 1.0000 is the strongest line here, because it is independent of the import path: the same cache that once reported "no plain theta explains this (best 0.5918)" now reproduces at 1.0000 with the theta the new reader lifts. That is exactly the check that would have caught the original default-to-10000 defect.

My earlier point stands and is now the next gate rather than a review item: the device run on kernels-4b-b8 (known answer ×3, tok/s) is what turns this into a row, and it needs an npu-hold ticket.

[bong-water-water-bong]

VERDICT: READY at f87179e (re-issued; head moved again)

Unchanged substance, and I re-checked the two things that decide it at this head: the nested-rope_parameters lift is
gaps-only (cfg.contains(key) || is_null() -> continue, so a pack declaring rope_theta at the top level cannot be overridden),
and the three-input MatMulNBits reading is >= 3 with the implicit zero point. The arms still carry both directions of the theta
test (5e6 when the block is present, 10000 when it is not) and mm == 1 / noz == 1 with K/N/block checked.

Head under review: f87179e

[bong-water-water-bong]

VERDICT at f87179e: CHANGES

Re-checked at f87179e. The fix is right, and the evidence is the kind that settles it. decode_rows now applies implicit_zp_zero(q, 4) when there is no qzeros tensor, so projections and head share one convention. The known-answer prompt answers " Paris. The capital of Spain is Madrid…" on both the host-GEMM path and the device, where both paths produced gibberish before. That rules out the kernels and the rotary. The commit message's lesson is the right one too: head-vs-embedding still read 0.993981 while every projection was wrong, because the self-check grades only the path that was correct.

Blocking, mechanical: the branch no longer merges. git merge-tree --write-tree origin/main f87179e reports conflicts in npu/pack_import.cpp and tests/npu_pack_import_test.cpp, because main has moved and #1790 landed as the squash e65f2cd while this branch still carries 87a0dcf. Rebase onto main, dropping 87a0dcf, so the PR is 8130e63 + c9fca4d + f87179e. Main currently has #1790's zero-projection import, so this is the PR that repairs it, and it should land soon.

While rebasing, the two points from my #1790 review that this commit's own lesson argues for (non-blocking, but the cheapest way to stop this recurring):

• A crafted arm for the zero plane. The existing three-input arm (tests/npu_pack_import_test.cpp:2179ff) checks the census ("counted as carrying no qzeros"), not the decoded values. Import that crafted projection and assert its container zero plane equals -8·scale. That is precisely the check that would have failed before this commit.
• decode_rows(...) failures are still dropped. The calls at :3301-3316 ignore the bool, so a projection that fails to decode leaves incomplete: [], which is how #1790's 0-projection container looked complete. Push a named reason into the refusal list on false.

[bong-water-water-bong]

VERDICT: READY @ f87179e (re-verdict; previously c9fca4d)

This delta is the one that would have been hardest to find without a runtime: the projections were decoded with implicit zero point 0 while the head was decoded
with 8, so the pack imported completely, served on the device, and answered gibberish — "West什么呢ưởng中国联通nio…" is the whole argument for the change in eight
characters. Applying implicit_zp_zero(q, 4) on the projection path is the same rule the head path uses, which is exactly the consistency that was missing: two paths
reading the same container under two different conventions for "no qzeros tensor" is a contradiction that no test noticed because both paths produced plausible
numbers.

One thing I checked because it would have been a trap for the queue: implicit_zp_zero is already defined on main (grep -c 'void implicit_zp_zero' → 1), so
this PR does not depend on #1790 landing first — the two branches both inherit it, and their merge order is free. Worth a line in the body, since a reader of both would
reasonably assume the helper arrives with one of them.

My remaining ask is unchanged and still non-blocking: a fixture with two rope_theta spellings holding different values, so precedence is pinned rather than
implicit. This commit is the second time in one PR that a multi-spelling reader met the odd combination — the pattern is established, and the test is now cheaper than
the next bug.

Reviewed read-only at f87179e: the delta since c9fca4d, and main's definition of the helper.

[bong-water-water-bong]

VERDICT: READY at 55b25ac — the new commit fills a hole I flagged, and the fixture that would pin it is already sitting in the test file.

The hole: this writer started from an empty json, so an mRoPE declaration living only at text_config.rope_parameters was dropped and the pack would be
served as plain RoPE — right theta, wrong axis layout (engine#1311's ~0.29 rad once image tokens are in the prompt). The new block copies
mrope_interleaved and mrope_section from load_geometry(), which is where the gaps-only nested lift lands them, and the comment names the consumer
(q4nx_semantics.c:121, value_for_top, depth 1) so a future change to that read depth has something to contradict. Both keys are copied
conditionally (value(...) / contains(...)), so a pack without them is untouched.

The assertion is one line away, and the fixture already exists

tests/npu_pack_import_test.cpp:2188 (test_rope_parameters_theta) already writes exactly this shape:

R"("vocab_size":32,"rope_parameters":{"mrope_interleaved":true,"mrope_section":[24,20,20],)"
R"("rope_theta":5000000,"rope_type":"default"}}})"

and then asserts only d.rope_theta == 5000000.0f. So the mRoPE keys are present in the fixture's config and nothing checks that they reach the
container — which is what this commit now does. Two check()s on the container JSON (c["mrope_interleaved"] == true,
c["mrope_section"] == {24,20,20}) would pin the very regression this commit fixes, using a file that is already written for it. Not a blocker — the
code is right and the failure mode it removes is silent — but a silent-wrong-axis bug is the one class where the assertion earns its keep, because
"right theta, wrong axes" cannot be spotted in a log line.

[bong-water-water-bong]

VERDICT: CHANGES at 55b25ac — this delta is a real bug and it is unpinned

The bug is worth the PR on its own, and it is stated with its consequence:

the container's OWN config is what the forward reads (q4nx_semantics.c:121, depth 1 only), and this writer started from an empty json — so a mRoPE declaration that lives only at text_config.rope_parameters was dropped, and the pack would be served as plain RoPE with the right theta and the wrong axis layout (engine#1311: exact for a text-only prompt, ~0.29 rad off once image tokens are in it).

That is a silently wrong output with a correct-looking theta — the same family as tonight's kTieCorr and dim/(dim-2) gaps: the number that gets checked is right, the number that does not is wrong.

But nothing pins this writer. The existing mrope arms test load_geometry() (tests/npu_pack_import_test.cpp:2195 feeds a config with mrope_interleaved/mrope_section and asserts rope_theta and the source string) — they exercise the reader, not the line this PR adds (npu/pack_import.cpp:2935-2936, copying the two keys into c). Revert those two lines and the suite is green, which is what my last three CHANGES on this lane have all been about.

The arm is cheap and follows the file's own pattern: import the crafted multi-component pack with a text_config.rope_parameters = {mrope_interleaved: true, mrope_section: [24,20,20]} and assert the written container config carries both keys — the same shape as the arms that already check rope_theta == 5000000.0f. A reader-side assertion cannot cover a writer-side omission.

Built and ran: all npu-pack checks passed at this head, so nothing is broken; the ask is that the new two lines fail when removed.

Also still open from my earlier verdict on this PR (unchanged): the dynamic-NTK exponent has no numeric arm, and I demonstrated that HD vs rot passes the suite.

[bong-water-water-bong]

VERDICT: READY at 55b25ac

The added commit extends the same rule to the container genre, and the comment says exactly why that is not a separate concern: "the container's OWN config is what the forward reads (q4nx_semantics.c:121 value_for_top)" — so the plan has to carry mrope_interleaved and mrope_section through from the container's config, or the forward is reading a different declaration than the plan validated. Propagating the two keys is the fix, and it is the same "fill from where the model declares it" rule as rope_theta one level up.

This is directly relevant to the NPU2 model-source sweep (engine#1786): the container genre's config.json is where those semantics live, so a container-genre plan will need precisely this propagation to be correct rather than merely readable.

Everything from my review at f87179e stands: the nested rope_theta read verified against the pack's own cos_cache (theta 5e6 reproduces row 1; the real pack yields 147 tensors / 47,480 tiles at corr 0.993981), the trailing-dot normalisation, and the source string saying which spelling answered.

No asks.

[bong-water-water-bong]

VERDICT: READY at 55b25ac for the rope part — with one thing that must be settled first: this branch and #1790 add the same hunk.

What is beyond my earlier verdict

Three commits landed after the head I reviewed: the nested mRoPE declaration, the implicit zero point on the projection path, and the trailing-dot normalisation. Plus the rope read itself, whose evidence remains the strongest in this batch: "rope_theta 5e+06 verified against the pack's cos_cache (corr 1.0000)" — an exact arm against the pack's own cache, not a best-fit ranking.

The duplicate: implicit_zp_zero is added twice across two open PRs

#1790:  if (!have_zp) implicit_zp_zero(q, mm.bits);        // :3361
#1794:  if (!have_zp) implicit_zp_zero(q, mm.bits);        // :3401
        if (!zt)     implicit_zp_zero(q, 4);               // :3279   <- this one is #1794's own

The function body is byte-identical in both branches (md5 1148b79b6c94… in each), and origin/main already carries the function itself — what both PRs add is the same call site. Whichever of #1790/#1794 merges second must drop that hunk; if both are merged as-is the result is either a conflict or a double application of zero -= zp*scale, which is not idempotent and would silently shift every projection by zp*scale twice. That is worth saying out loud given this is the exact "four lanes, one one-file fix" shape from 11-08.

The new part here is only implicit_zp_zero(q, 4) on the projection path. It is consistent, and I checked rather than assumed: decode_matmulnbits(bytes, scales, zp, N, K, block) takes no bits parameter, so that path is 4-bit by construction and the constant matches its only assumption. One line of comment would help — the head path uses mm.bits because MatMulNBits carries the width, and if projections ever ship int8, both the decoder and this 4 need the width threaded through.

Conflict with main

The orchestrator reports this PR conflicting; the tree I reviewed is 55b25ac on the rebased branch. Merge main in, then re-check whether the mm.bits hunk survived #1790's merge — if it did, delete it here rather than resolving in its favour.

[bong-water-water-bong]

VERDICT: READY at 55b25ac — the two changes are the right ones and the measurement behind the second settles a question that guessing cannot.

What I checked

The >= 4 → >= 3 input gate (npu/pack_import.cpp): Qwen3-VL-4B's spelling is (activations, …weight_Q4, …weight_scales) with bits 4, block_size 32 and no qzeros tensor at all, so a four-input test recorded no matmuls — matmulnbits_ops 0, "imported 1 tensors, 0 q4_1 tiles". Widening to three with if (ins.size() >= 4) mm.zeros = ins[3]; inside is the right shape: the three-input case takes the implicit zero point, the four-input case still reads its zeros. That path is already covered: tests/npu_pack_import_test.cpp:2777 builds four-input nodes and many arms depend on them, so the widened gate cannot silently start dropping ins[3] without something going red.

rope_theta from the nested block: the lift copies only gaps (if (cfg.contains(it.key())) continue;), which is the same never-override rule as the text_config lift above it, and the test proves both directions — rope_theta arrives as 5000000 from text_config.rope_parameters, and a top-level rope_theta: 10000 is not overridden by the nested one. That second arm is the one that stops a "lift" from becoming a silent override, and it is the arm I would have asked for.

The measurement is the part that makes this reviewable at all. "theta 5e6 reproduces row 1 to max|d| 0.000000; 10000 is off by 0.088 and 3.2e6 by 0.005" against the pack's own cos_cache (262144 × 64, external) is a decisive test of which constant is right, not an assertion that the code now looks nicer. The warning it removes ("no plain theta explains … best 0.5918 at 3.2e+06, declared 10000") was the honest signal that the reader could not see the value; the fix makes the signal true again rather than muting it.

Two notes, neither blocking the substance

1. The PR is CONFLICTING with main — that is the author's rebase, and it blocks the merge regardless of this verdict. npu/pack_import.cpp has moved under it a lot tonight (the external-data tile decoder, the fconst19 table row, the model_config lift), so the rebase is worth doing carefully rather than trusting the conflict markers.
2. The nested-block lift copies every key of rope_parameters into the geometry bag, not just rope_theta — harmless while read_dims asks for scalars by name and the copy is gaps-only, and it is the same breadth I flagged on npu-pack: derive intermediate_size from the FFN pair's node attributes when no file and no initializer dim states it #1657. Worth one clause in the comment if a future key becomes an object.

What I could not verify: the 3 GB Qwen3-VL-4B pack is not on this box, so the "147 tensors / 47,480 tiles / corr 0.993981 against a 0.3333 no-zero-point control" figures are the author's measurement, not mine. Everything above is the code, the test fixtures' mechanism, and the arithmetic of the cos_cache claim.

[bong-water-water-bong]

VERDICT: READY at 55b25ac

Re-checked at this head: the nested-rope_parameters lift is still gaps-only (cfg.contains(key) || is_null() -> continue, so a
pack declaring rope_theta at the top level cannot be overridden by the nested block), the three-input MatMulNBits reading is
>= 3 with the implicit zero point, and the arms still carry both directions of the theta test (5e6 when the block is present, the
10000 default when it is not) plus mm == 1 / noz == 1 with K/N/block checked.

The identification still stands as the strongest evidence in this family: theta 5e6 reproduces the pack's own cos_cache row 1 to
max|d| 0.000000, where 10000 is off by 0.088.

Head under review: 55b25ac

[bong-water-water-bong]

VERDICT at b0a1e12: READY

Re-checked at b0a1e12. The blocker from f87179e and 55b25ac is resolved: main has been merged in, and git merge-tree against current main is clean. The effective diff is npu/pack_import.cpp (+50/−5) and tests/npu_pack_import_test.cpp (+32), carrying exactly this PR's four pieces: the nested rope_parameters theta lift, the trailing-dot normalisation in decode_rows, implicit_zp_zero(q, 4) for projections with no qzeros, and the external writer copying mrope_interleaved / mrope_section. Both files pass -fsyntax-only with the engine's compile line, and tools/ci/ci_targets_check.py --check passes (baseline 9, none new).

This repairs main's Qwen3-VL-4B import. #1790 merged on main with zero projections written and incomplete: [], and f87179e's evidence shows the fixed import answering " Paris. The capital of Spain is Madrid" on both the host-GEMM path and the device. Still open from my earlier notes, non-blocking: a crafted arm asserting a three-input projection's zero plane equals -8·scale, and recording failed decode_rows(...) calls in incomplete.

[bong-water-water-bong]

VERDICT: READY at b0a1e12 — and the measurement in the body is the reason this is easy to accept.

The proof is the right kind. "theta 5e6 reproduces row 1 of the pack's own cos_cache to max|d| 0.000000, 10000 is off by 0.088, 3.2e6 by 0.005" is a fact about the pack, obtained from the pack, that names why the previous behaviour was wrong — not a preference between two plausible constants. It also closes an existing warning rather than adding a workaround ("the caches are not unexplained at all, the theta was simply not read"), which is the better way to fix a reader.

The lift obeys the rule it states: rope_parameters keys are copied only when the key is absent at the level above and the value is not null, and the source string records that it happened. The test pins both directions — the nested read, and a top-level rope_theta winning over the nested one — which is the arm that stops the lift from quietly turning into an override later.

Two more fixes in the same PR, both with the reasoning written down where the next reader will find it:

• the container's own config now carries mrope_interleaved/mrope_section from load_geometry(), because q4nx_semantics.c reads the container at depth 1 and a mRoPE declared only under text_config.rope_parameters was being dropped — right theta, wrong axis layout, exact for text-only prompts and wrong once image tokens arrive;
• the proj suffix normalisation, because graph:-sourced names arrive without a trailing dot, which made a three-input lookup never resolve — that is a "planned 252 projections, wrote the norms, failed on device with data_size 0" failure being prevented at the point where the spelling differs.

One note, and it is a preference rather than a defect: the proj normalisation has no test arm. It is the kind of change that looks trivial and has a sharp failure mode (a doubled or missing dot turns a found tensor into a missing one), so a two-line arm on the two spellings would be worth more than the comment alone. Not a reason to hold the PR.

The measured-cos-cache discipline here is the same one that makes the fconst rules table trustworthy: a claimed constant checked against something the pack itself contains, with the losing candidates' residuals printed next to it.

[bong-water-water-bong]

VERDICT: READY at b0a1e12 — my duplicate finding is resolved, and the diff now says so numerically.

main   npu/pack_import.cpp: implicit_zp_zero(q, mm.bits) -> 1
#1794  npu/pack_import.cpp: implicit_zp_zero(q, mm.bits) -> 1     (no longer an addition)
#1794 vs main: 2 files changed, 77 insertions(+), 5 deletions(-)

The mm.bits call site is in main now, so this branch no longer adds it and the "apply zero -= zp*scale twice" hazard I raised is gone by construction. What remains is the rope part — the nested rope_parameters.rope_theta read and the nested mRoPE declaration — which is what the PR title describes.

That part still has the best evidence in the batch: theta 5e+06 reproducing the pack's own cos_cache to max|d| 0.000000, with 10000 off by 0.088 and 3.2e6 by 0.005, i.e. an exact arm rather than a ranking. And the projection-path implicit_zp_zero(q, 4) remains correct-by-construction (decode_matmulnbits takes no bits), with the note that both need threading if projections ever ship int8.

Conflict resolved, duplicate removed, evidence intact — nothing left from my side.

[bong-water-water-bong]

VERDICT: READY at b0a1e12

The head moved by a merge from main (llama.cpp pin, hw-gate docs, npu-pack row). Checking the post-merge file rather than the diff — the lesson from
#1633, where a table row vanished in a conflict resolution — the mRoPE propagation is intact:

npu/pack_import.cpp:3294   if (geom.value("mrope_interleaved", false)) c["mrope_interleaved"] = true;
npu/pack_import.cpp:3295   if (geom.contains("mrope_section")) c["mrope_section"] = geom["mrope_section"];

so the earlier READY stands at this head. The one ask from that verdict is unchanged and still one line: the fixture at
tests/npu_pack_import_test.cpp:2290 already writes mrope_interleaved: true + mrope_section: [24,20,20] and asserts only rope_theta, so two
check()s on the container JSON would pin the silent-wrong-axis case this PR fixes.

[bong-water-water-bong]

VERDICT at 6971584: READY

Re-checked at 6971584, a merge of main into the branch. The PR's own changes are unchanged by the merge: the sorted +/- lines of its diff against main are identical to b0a1e12's (0 lines differ), still npu/pack_import.cpp +50/−5 and tests/npu_pack_import_test.cpp +32. Both files pass -fsyntax-only against the merged tree with the engine's compile line, and git merge-tree against current main is clean. My b0a1e12 READY carries over unchanged.

[bong-water-water-bong]

VERDICT: READY at 69715843 — carried from b0a1e12, and verified as carried rather than assumed.

The head moved, and the diff b0a1e12..69715843 on npu/pack_import.cpp looks large — but that is main moving underneath (the fconst_scratch_dirs() refactor from another lane), not this PR changing. Diffing its own files against its merge-base (b6db79e93) shows the same two hunks I reviewed: 50 lines in npu/pack_import.cpp, 32 in tests/npu_pack_import_test.cpp. The rope_parameters lift, the never-override rule, the mrope_* carry into the container config and the proj suffix normalisation are unchanged, as is the measured justification (theta 5e6 reproducing the pack's cos_cache row 1 to max|d| 0.000000, 10000 off by 0.088).

Nothing new to object to; my one note from the previous head stands and is still not a blocker: the proj dot-normalisation has no arm, and it is the kind of two-character change whose failure mode (a doubled or missing dot) turns a found tensor into a missing one.

Whoever merges this: the rebase is clean against current main, and the pack-import test binary still builds and passes in my tree at the parent head — no reason to expect otherwise here, but I did not rebuild at 69715843 specifically, so the merge gate should be the real build.

[bong-water-water-bong]

VERDICT: READY at 6971584 — merge from main again, so I checked the post-merge file rather than the diff: mrope_interleaved is present twice in
npu/pack_import.cpp at this head (the conditional copy at :3294 and the comment above it), so the earlier verdict stands.

The one ask is unchanged and still one line: the fixture at tests/npu_pack_import_test.cpp:2290 writes mrope_interleaved: true +
mrope_section: [24,20,20] and asserts only rope_theta, so two check()s on the container JSON would pin the silent-wrong-axis case this PR fixes.

[bong-water-water-bong]

VERDICT: READY at 6971584 — re-issued because the head moved, and I checked the content rather than the delta, because this branch was rebased onto a newer main and a raw diff would show 44 files of other people's work.

origin/main...6971584  ->  2 files changed, 77 insertions(+), 5 deletions(-)
                           npu/pack_import.cpp   (rope_parameters → rope_theta, 7 mentions of the nested block)

So the change is still the one I reviewed at 55b25ac: the nested rope_parameters block is read only where the top level is silent — the rule #1638 established for text_config and #1644 reused for model_config.json, applied one nesting deeper — with the test asserting the value, not just that the read succeeded.

Nothing in the rebase changed the claim, and no other file is touched. My earlier note stands as the only ask: say what happens to the nested fields this PR does not read (a rope_type/factor left unread is the shape that later looks like "the importer ignores the scaling"). One line in the body or a comment at the read site.

[bong-water-water-bong]

VERDICT: READY at 6971584 — the delta is a main merge (b6db79e93 npu: the integer host arm … arriving from #1809), so the branch's own content is what I reviewed: the nested rope_parameters.rope_theta read plus the mRoPE declaration, with the implicit_zp_zero(q, mm.bits) hunk removed because main now has it.

The rope evidence remains the strongest in this batch and is the reason I would merge it: theta 5e+06 reproduces the pack's own cos_cache to max|d| 0.000000, against 10000 off by 0.088 and 3.2e6 off by 0.005 — an exact arm, not a ranking. And the fix is the same text_config descent the geometry already uses, so it is not a special case for one model family.

Nothing left from my side.

[bong-water-water-bong]

VERDICT: READY at 6971584

This is the shape I want a config-reading fix to have: the warning it removes was not "unexplained cache" but "we did not read the key", and that is
proved rather than argued — against the pack's own cos_cache, theta 5e6 reproduces row 1 to max|d| 0.000000, 10000 is off by 0.088, 3.2e6 by
0.005. So the reader's 10000 default was the whole discrepancy, and the cos/sin caches were never unexplained.

Two properties of the change make it safe to take:

• Gaps only, never overriding, stated as the same rule as the lift above it. A nested block that restates a top-level key cannot change behaviour, so the
  fix cannot silently re-tune a pack whose top-level value was deliberate.
• The writer half is included for a reason: the container's own config.json is what the forward reads (q4nx_semantics.c:121, depth 1 only), so a
  mRoPE declaration that lives only in text_config.rope_parameters would be read by the importer and then lost by the container. Fixing the reader without
  the writer would have moved the gap rather than closed it.

One addition I would take, and it is the same one I asked for on #1819: when both a top-level key and its rope_parameters counterpart exist and
disagree, log it once. Gaps-only is the right precedence, but without a disagreement note a pack with a stale top-level rope_theta and a correct nested one
keeps the stale value silently — and that is precisely the class of bug this PR is fixing, just in the other direction.

The +77/-5 ratio (mostly comment and the two small blocks) is right for a fix whose value is in the recorded measurement.