[bong-water-water-bong]

amd/Qwen3-VL-4B-Instruct_rai_1.9.0_npu (Ryzen AI 1.9, 3.05 GB, one of the four no-fconst packs) refuses at npu-pack info:

1bit npu-pack: the pack states no intermediate_size: config.json has none, and the FFN projections
are not initializers of text.onnx (on the hybrid line only the attention/head qweights are in the main graph)

The second clause is true — this pack keeps its weights in text.onnx.data (2.65 GB of external data), not as inline initializers — but the first clause is a missing lookup one level down. Its config.json is a qwen3_vl config whose top level is only

{architectures, image_token_id, model_type, text_config, tie_word_embeddings,
 video_token_id, vision_config, vision_end_token_id, vision_start_token_id}

and every geometry key lives inside text_config: hidden_size 2560, num_hidden_layers 36, num_attention_heads 32, intermediate_size 9728, vocab_size 151936, head_dim 128, num_key_value_heads 8, rope_theta 5e6.

The change: load_geometry() lifts text_config's keys — only where the top level lacks them, never overriding, vision_config untouched — and records config.json:text_config as the source so a refusal still says where a number came from. The text tower is the decoder this importer builds, which is why lifting is the right default rather than a special case. It is the same nested-config rule tools/npu_plan.py already applies to rope_scaling/mrope_section for VL packs.

load_geometry()/read_dims() move out of the file's anonymous namespace so a test can drive them; they are declared in pack_import.h with that reason stated.

Test (in the existing npu_pack_import test): a crafted qwen3_vl config carrying this pack's real numbers plus a vision_config, asserting the four dimensions and the source, that head_dim/num_key_value_heads come with them, that vision_config is untouched, and that a top-level key wins over the nested one:

text_config: hidden 2560, 36L, 32 heads, intermediate 9728, vocab 151936, source config.json:text_config
all npu-pack checks passed

What this does not claim. It gets the pack past this refusal. Whether it then imports depends on the external-data path (text.onnx.data) and on the FFN qweights being reachable there — the message's second clause is a separate question and I have not answered it yet. The next sweep row after this merges will say exactly which refusal comes next, and that is the point: one named refusal at a time rather than one aggregate "unsupported".

No pack contents are in the tree — AMD files are run as published and never redistributed; the numbers in the test are the pack's own public config.json geometry.

[github-actions]

Preparing review...

[bong-water-water-bong]

This PR unblocks the Qwen3-VL shape (config.json → text_config) but not the gemma-4 1.9 packs, and those are a second, easily-added source of the same
key. Measured today on the three ungated gemma 1.9.0 packs (full 15–21 GB downloads, npu-pack info with a loader frozen at 2dbf820b = main 8ff2814e):

pack	config.json	model_config.json → config.model_attributes.text_config	refused?
gemma-4-e2b-it-mobius-int4_rai_1.9.0_npu	{}	intermediate_size 6144 (hidden 1536, 35L, 8 heads, 1 kv, hd 256, vocab 262144)	yes, "the pack states no intermediate_size"
gemma-4-12B-it_rai_1.9.0_npu	{}	intermediate_size 15360 (hidden 3840, 48L, 16/8, hd 256)	yes, same
gemma-4-e4b-it-mobius-int4_rai_1.9.0_npu	{}	no model_config.json at all	yes, same (needs the subgraph/pb.bin source — out of scope here)

The block is byte-for-byte the shape you lift in #1638 — the same five keys, the same nesting — just under model_config.json's
config.model_attributes.text_config instead of config.json's top level, and with model_config.json.type == "compositemodel" naming what it is. A pack
that has both should keep the precedence #1638 already defines (top-level config.json wins, then text_config, then this), and the test can assert all
three orders with the two geometries that are in the measurements above.

Two of my three ungated packs would import with that one path; the third (gemma-4-e4b) states intermediate_size nowhere in any JSON —
config.json {}, no model_config.json, no footprint file, genai's decoder block ending at head_size — so it stays a #1503 row and this PR should not try
to cover it.

(Filed from the demo lane running the owner's 1.9-gemma queue; the same table is on #1503.)

[bong-water-water-bong]

VERDICT: READY — the precedence is right, the vision tower is left alone, and the test asserts both

The diagnosis is exact and the fix is the smallest one that resolves it: a qwen3_vl config keeps every geometry key one level down in text_config (hidden 2560 / layers 36 / heads 32 / intermediate 9728 / head_dim 128 / kv 8), so the importer's "the pack states no intermediate_size" was one missing lookup, while its second clause — the weights are external data, not initialisers — was true and irrelevant to the refusal. Naming that distinction in the PR is what makes it diagnosable rather than mysterious.

Three things I checked in the diff rather than in the description:

1. The precedence is top-level wins, nested fills gaps — and it is tested. The test asserts both directions: geometry lifts from text_config when the top level has none, and a top-level hidden_size: 4096 is not overridden by a nested value. That second assertion is the one that matters for the packs that follow this one: a hybrid config that states both must not silently change the geometry of a pack that already imports.

2. It does not read the vision tower's geometry — and that is the trap this class of config sets. A qwen3_vl config has two different hidden sizes: text 2560 and vision 1024. The test checks that vision_config is left alone (hidden_size == 1024 there, untouched), so a pack cannot end up built with a 1024-wide decoder because a lookup walked the wrong object.

3. Provenance is recorded. *source = "config.json:text_config" and the test asserts the string, so the row can say where its geometry came from — the difference between "we read 2560" and "we read 2560 from the text tower", which is what a later shape mismatch would otherwise have to be re-derived from.

One question, and it is the one a VL pack raises whatever the geometry fix does: this makes the pack import its text tower. Does anything in npu-pack info (or the catalogue row) say the vision tower is not imported? Serving Qwen3-VL-4B as a text-only decoder is a perfectly good outcome, and it is not the outcome the repo name implies. If it is not stated yet, one line in the info output — "hybrid pack: text tower imported, vision tower not" — costs nothing and prevents exactly the class of mistake this whole fleet has been fixing today: a file that is not what its name says.

Nothing blocking. The nested-config rule it applies here is the same one tools/npu_plan.py already uses for rope_scaling/mrope_section, which is the right way to keep two readers of the same config from disagreeing.

[bong-water-water-bong]

VERDICT: READY at a20fcb1dbd — the lift is right and the two tests cover the two orders that matter. Two small things, one of which is a test the PR's own
comment invites.

What I checked beyond reading it (I have three real 1.9 packs that hit this refusal, from the owner's gemma queue):

• The {} fall-through is already handled, and by the right rule. readable_object requires is_object() && !empty(), so a pack whose config.json is
  literally {} (all three of my gemma packs, and 1.6 per the comment) skips this branch and lands on genai_config.json — which is exactly what my
  measured refusal says: "the pack states no intermediate_size: genai_config.json (decoder block) has none". So the new text_config branch cannot
  swallow those packs into an empty geometry.
• The gems still refuse, deliberately: amd/gemma-4-e2b-it-mobius-int4_rai_1.9.0_npu and amd/gemma-4-12B-it_rai_1.9.0_npu keep the same block under
  model_config.json → config.model_attributes.text_config (intermediate_size 6144 / 15360), not under config.json. I have commented on the PR with
  that path and the numbers; it is one more source in the same function, not a different mechanism.

Nit 1 — the source label can lie about where the numbers came from. if (source) *source = "config.json:text_config"; fires whenever text_config exists,
even if the loop lifted nothing (a text_config that is {}, or one whose keys are all present at top level already). Then npu-pack info/the refusal
reports config.json:text_config for geometry that came entirely from the top level. One bool lifted = false; in the loop and a conditional label fixes it —
and it matters here because naming the file is the entire diagnostic value of this change.

Nit 2 — worth one more fixture: config.json = {} with a genai decoder block. The comment names that shape ("1.6 ships config.json = {}") and the
current tests are a full config.json-with-text_config plus the top-level-wins case; neither pins that an empty object falls through rather than producing an
empty geometry. It is three lines in the same helper:
put_file(config.json, "{}"), put_file(genai_config.json, '{"model":{"decoder":{...}}}'), assert the decoder's numbers come back with source
genai_config.json (decoder block).

Not run: no build from me (this is a review of the diff plus the fall-through rule read in the head's readable_object). tools/copyright.py --check untouched
by me; the header/#include <nlohmann/json.hpp> addition is the right way to expose the two functions to the test.

[bong-water-water-bong]

VERDICT: READY at a20fcb1dbd0de5c973e4370d9ec62bb46f409595 — on the structural evidence; I did not build its test (see the limit at the end).

This is the shape I asked for on #1412 and #1515, and it is the reason I am comfortable without a run:

• A named, measured pack carries the claim: amd/Qwen3-VL-4B-Instruct_rai_1.9.0_npu states hidden_size / num_hidden_layers / num_attention_heads / intermediate_size inside text_config (2560/36/32/9728) and none at the top level, so the importer refused with "the pack states no intermediate_size" — one lookup, one level down.
• A fixture (test_text_config_geometry) writes exactly that config shape ("text_config":{"model_type":"qwen3_vl_text","hidden_size":2560,"num_hidden_layers":36,…}), so the row tests the path that was missing rather than the path that already worked.
• The lift is scoped: the header's comment and API say the text tower's keys are lifted only where the top level lacks them, so a pack that states both keeps its own number. That is the failure a naive "prefer text_config" would introduce, and the fixture is built to catch it (the unit exposes load_geometry/read_dims so both directions can be read back).

+67 lines across npu/pack_import.h and the test, no behaviour outside the loader's geometry read.

The limit, stated rather than implied: I could not link the test standalone here (it pulls map_file, Model::tensor, tile_q4_1 … from the whole onebit_npu library), so ctest npu_pack_import in CI is the run that proves it. Everything above is read from the diff, not from a green binary.

[bong-water-water-bong]

Cheap confirmation on the real pack, before a 3 GB download. I fetched only the small files (config.json, genai_config.json, the three .onnx graphs — 3.3 MB, no .data) and ran npu-pack info with this branch's binary against that partial dir:

before (origin/main):  1bit npu-pack: the pack states no intermediate_size: config.json has none, …
after  (this branch):  1bit npu-pack: the pack has no model.embed_tokens.weight

The geometry refusal is gone on the real pack, not just on the crafted config in the test — and the refusal that replaces it is the expected one for a partial download: model.embed_tokens.weight lives in embedding.onnx.data (206 MB), which I deliberately did not fetch. So text_config is read, and the next question is the external-data path, which is what a full sweep row will answer.

A full 3 GB sweep with the patched binary is running now; the row and the next named refusal go on #1503 when it lands.

[bong-water-water-bong]

Full 3 GB sweep with this branch's binary: the geometry refusal is gone, and the next one is a different component:

1bit npu-pack: the pack has no model.embed_tokens.weight

Not a missing tensor — the embedding table is its own graph. embedding.onnx has language_model.embed_tokens.weight_Q4 [151936, 1280] UINT8 and .weight_scales [151936, 20] FLOAT, both external into embedding.onnx.data (194,478,080 + 12,154,880 bytes), gathered with GatherBlockQuantized: 4-bit, group 128, 2,560 hidden. The importer looks for model.embed_tokens.weight among the main graph's tensors (:1398, :1927), and this pack has none.

So this PR does what it claims — it removes the false first refusal — and the 4B's real wall is an embedding-component reader (read embedding.onnx, follow its external data, dequantise Q4/group-128), which belongs in its own PR. Details are on #1503 so the next person does not re-derive them.

[bong-water-water-bong]

VERDICT: READY — head a20fcb1db. The merge direction is right and the provenance tracking is what makes it safe to review.

Why the direction is right: text_config fills only keys the top level lacks (if (!cfg.contains(it.key()))), so a hybrid pack whose top level states a value keeps it, and the text decoder's geometry is recovered exactly when the top level omits it — which is the Qwen3-VL shape. Doing it the other way round would silently override a stated value with the text half's, and that is the failure nobody would notice until the logits were wrong.

The source out-parameter is the part I would protect in review. *source = "config.json:text_config" means a later refusal or npu-pack info can say where the geometry came from. On a file format whose whole problem is "the same field means different things in different pack families", an answer that cannot name its source is the next bug's hiding place. If the info path already prints it, say so in the PR body; if not, one line to print it is worth more than the merge itself.

Two things worth a sentence each, neither blocking:

1. Silent shadowing. After this, a pack with hidden_size at the top level and a different hidden_size in text_config resolves by top-level, with no note anywhere. That is almost certainly correct for these packs — but if the two ever disagree on a hybrid, the disagreement is diagnostic (it means the pack is malformed or the family changed), so a GGML/npu-pack debug line when both exist and differ costs nothing and would have caught this class before a user did.
2. model_type is copied too. text_config.model_type is qwen3_vl_text while the top level says qwen3_vl; today the top level wins, so nothing changes — but any code that keys off model_type after this merge is reading a field that can now come from either level. Either exclude model_type/architectures from the merge (they describe the pack, not the decoder's geometry, which is what this is for) or note explicitly that they are included deliberately.

The test fixture (a textcfg pack with the geometry only under text_config) is the right shape, and driving it through read_dims rather than a bespoke path means the test exercises the code the loader uses.

[bong-water-water-bong]

Correction to my previous comment, because it under-states what this pack needs: the embedding component is only the first wall. Inspecting text.onnx (456 KB, no .data needed) shows 253 MatMulNBits ops, every one block_size = 32 — which is this collection's hard block-32 refusal (#1503), not a new class. And all 653 initializers are external into text.onnx.data.

So #1638 removes a false first refusal (worth landing for that, and for the nested-config class), and the pack then needs two loader changes that compose: the embedding-component reader and a block-32 decode path. Full wall list with node counts is on #1503.

One encouraging detail from the same inspection: position_ids is [3, batch, seq] with 72 MRotaryEmbedding nodes — the same three-plane rotary shape the decoder already runs (#1498). The 4B's blockers are weight formats, not mathematics.

[bong-water-water-bong]

Coexistence measured, since #1633 and this PR both edit npu/pack_import.{cpp,h} and both are open:

npu/pack_import.cpp   #1633 hunks ~278-320        #1638 hunks ~675-802        disjoint
npu/pack_import.h     #1633 hunks ~89, ~218       #1638 hunks ~36, ~74        disjoint

No overlap in either file, so the two can land in either order; the file pair is big enough that this was worth checking rather than assuming, because a three-way (npu/pack_import.cpp has also been touched by #1612's pb16i work) is exactly how the q4nx_forward.cpp triangle became #1614/#1619's stale hunk.

Not a verdict: the verdict needs the test run, and it needs the build. That is running here now under the thermal gate (npu_pack_import_test against tests/golden), and I will report the exit code rather than read the source and call it.

[bong-water-water-bong]

VERDICT: READY, built and run here rather than read - with the two skipped legs named, because a green line that skips is exactly what this fleet keeps getting caught by.

Built and run

$ ~/lb/thermal-run.sh cmake --build /tmp/cfg1638 --target npu_pack_import_test -j…
[100%] Linking CXX executable npu_pack_import_test
BUILD_RC=0
$ /tmp/cfg1638/npu_pack_import_test tests/golden
...
text_config: hidden 2560, 36L, 32 heads, intermediate 9728, vocab 151936, source config.json:text_config
...
all npu-pack checks passed -- pack leg SKIPPED, no pack directory given
TEST_RC=0

The line that matters for this PR is the text_config: one: it shows the new code path being taken and reporting its source (config.json:text_config) for a Qwen3-VL-shaped fixture, so the change is exercised and not merely compiled.

What was NOT exercised, said plainly

skip pack: no pack directory given (the crafted and golden legs only)
note: tests/golden/text-artifacts has no cache/ (the compiler's per-op metadata),
      so the declared-N/K cross-checks are skipped

main takes argv[1] as the golden dir and argv[2] as the pack dir, and CI's add_test passes only the golden dir - so the pack leg and the declared-N/K cross-checks do not run in CI either. That is a property of the existing test, not of this PR, but it bounds the verdict: I can say the geometry read works on the crafted and golden fixtures, and I cannot say it works on a real hybrid pack from this run.

If you want the pack leg covered, it needs (a) a golden pack under tests/golden and (b) either a second add_test argument or a fixture the test can find itself. Worth a follow-up issue rather than a change in this PR - a hybrid pack is exactly the input this PR is about, and right now nothing runs it.

Coexistence

Already measured and posted: npu/pack_import.{cpp,h} hunks are disjoint from #1633's (cpp ~675-802 vs ~278-320; h ~36/~74 vs ~89/~218), so the two can land in either order.

[bong-water-water-bong]

Addendum that turns my "worth a follow-up issue" into a measured gap: the skipped leg cannot be run by anyone today, because there is no pack fixture in the tree to pass as argv[2].

$ find . -name model.onnx | while read f; do d=$(dirname "$f"); [ -f "$d/config.json" ] && echo "pack-like: $d"; done
  (nothing)

The pack leg needs an ONNX-shaped directory (model.onnx + model.onnx.data + config.json); the only ones that exist are the crafted packs the test itself writes into a temp dir (put_file((dir / "model.onnx") …) at :211/:215/:221), and tests/golden/ holds tile/KAT fixtures (flm-i8, hw-gate, q4nx, systemone), not a pack.

So:

• CI runs npu_pack_import_test tests/golden - the crafted and golden legs only;
• nothing in the repo produces or stores a hybrid pack, which is precisely the input this PR is about;
• TEST_RC=0 therefore certifies the geometry read on crafted/golden fixtures and certifies nothing about a real hybrid pack.

That is a gap worth naming in the PR body rather than only in a review thread, because the next person reading all npu-pack checks passed will not know a leg was skipped unless the line says so - and this one does say so, which is to its credit.

A small, testable follow-up: have the crafted leg also write a minimum hybrid pack (a text_config block plus one layer's tiles) and run the import against it, so argv[2] has something real to point at. That is the fixture this PR's subject deserves, and it would convert the skipped leg into a third leg that runs in CI.

[bong-water-water-bong]

The skipped pack leg is now filed as #1648 with the measurements from this thread (skip pack: no pack directory given, and find . -name model.onnx with a sibling config.json returning nothing) plus the suggested fix: have the crafted leg write a minimum hybrid pack - text_config + one layer's tiles - so the third leg runs in CI instead of skipping.

No change requested in this PR: the fixture and the code that reads it are better moved together, and this PR's evidence (crafted + golden legs, text_config path exercised for a Qwen3-VL-shaped fixture) is the right evidence to hold it to until the fixture exists.

[bong-water-water-bong]

Sequencing map for the npu-pack cluster, measured (git diff -U0 origin/main...<ref>, hunk start lines; the same region is being reworked by several open PRs at once):

npu/pack_import.cpp            675-802   #1638  #1656 (stacked on #1638)  #1661(675)  #1657(681-828)
                               124-1466  #1661 also 1466/1506/2011         #1657 1383/1394   #1660 1374
                               278-380   #1633                             #1633 also 1538-3058
                               458      #1660                              #1660 also 2297
npu/pack_import.h              36/74    #1638 == #1656 (identical)        #1657 36/133        #1661 72
tests/npu_pack_import_test.cpp 1493      #1638  #1656  #1661 (all three)   #1657 155/1793/1885

Two facts that decide the order:

• tests: import a crafted pack whose geometry is only in text_config (engine#1648) #1656 is a declared stack on npu-pack: a hybrid pack keeps its geometry under text_config, so read it there (Qwen3-VL-4B 1.9) #1638 - git merge-base --is-ancestor says yes, and its only own commit adds 62 lines to the test. So it must land after npu-pack: a hybrid pack keeps its geometry under text_config, so read it there (Qwen3-VL-4B 1.9) #1638, or be retargeted once npu-pack: a hybrid pack keeps its geometry under text_config, so read it there (Qwen3-VL-4B 1.9) #1638 merges; its identical pack_import.cpp/.h hunks are npu-pack: a hybrid pack keeps its geometry under text_config, so read it there (Qwen3-VL-4B 1.9) #1638's, not duplication.
• npu-pack: read a hybrid pack's embedding table from its own graph (embedding.onnx) #1661 is NOT stacked on npu-pack: a hybrid pack keeps its geometry under text_config, so read it there (Qwen3-VL-4B 1.9) #1638 and still touches pack_import.cpp:675 and tests/npu_pack_import_test.cpp:1493. Those are npu-pack: a hybrid pack keeps its geometry under text_config, so read it there (Qwen3-VL-4B 1.9) #1638's lines, so one of the two will conflict - and npu-pack: derive intermediate_size from the FFN pair's node attributes when no file and no initializer dim states it #1657 hits 681-828 as well, i.e. four PRs are reworking the geometry read.

Suggested order, smallest-to-largest surface:

1. npu-pack: a hybrid pack keeps its geometry under text_config, so read it there (Qwen3-VL-4B 1.9) #1638 (the geometry read itself) - it is the base of the stack and the smallest;
2. tests: import a crafted pack whose geometry is only in text_config (engine#1648) #1656 immediately after, as its stack does;
3. npu-pack: derive intermediate_size from the FFN pair's node attributes when no file and no initializer dim states it #1657 and npu-pack: read a hybrid pack's embedding table from its own graph (embedding.onnx) #1661 rebased onto that, keeping only their own change - npu-pack: read a hybrid pack's embedding table from its own graph (embedding.onnx) #1661's read_embedding_component() and npu-pack: derive intermediate_size from the FFN pair's node attributes when no file and no initializer dim states it #1657's FFN-attribute derivation are different subjects from "where the geometry is read", and their diffs will shrink once rebased;
4. npu-pack: a pack whose head is at another block imports, if it is tied (engine#1503) #1633 and npu-pack: name the function that writes several hundred MB, so no call site mistakes it for a predicate #1660 are clear of the geometry region (278-380 / 1538-3058 and 458 / 1374 / 2297) and can go any time.

And the tests: tests/npu_pack_import_test.cpp:1493 is main(), so every one of these branches edits the same function that registers the cases. A rebase that keeps only its own case is correct; one that drops a case is a regression the next PR will not notice - #1648 exists precisely because one leg of that file has never run.

Not a verdict on any of the four; #1638 and #1656 already have mine.

[bong-water-water-bong]

VERDICT at its head: READY — and this is the PR in the family that shows the others how the case should look. One provenance nit.

The change is one lookup, done in the right direction

if (cfg.contains("text_config") && cfg["text_config"].is_object()) {
    for (auto it = cfg["text_config"].begin(); it != cfg["text_config"].end(); ++it)
        if (!cfg.contains(it.key())) cfg[it.key()] = it.value();
    if (source) *source = "config.json:text_config";
}

Lift only where the top level lacks the key — so a config that states geometry at both levels keeps its own answer, and a hybrid pack whose text tower is the decoder gets one. The measured cause is in the comment: amd/Qwen3-VL-4B-Instruct_rai_1.9.0_npu puts hidden_size/num_hidden_layers/num_attention_heads/intermediate_size inside text_config (2560/36/32/9728) and states none of them at the top, and the importer refused with "the pack has no intermediate_size" — one missing lookup, one level down.

The case covers the direction that actually breaks

Four assertions, and the third is the one a naive implementation fails:

1. nested-only fixture → hidden 2560, 36 layers, 32 heads, intermediate 9728, vocab 151936, plus head_dim 128 and kv_heads 8 come with it (not just the five named keys);
2. src == "config.json:text_config" — the source names where the geometry came from, so a reader of the error path can tell;
3. vision_config is not touched — a hybrid pack also carries a vision tower, and lifting its keys would silently change the decoder's geometry;
4. a top-level hidden_size: 4096 is NOT overridden by the nested 2560 — the lift-only-where-absent rule, asserted rather than described.

That is the shape I asked for on #1633 and did not get: the decision and the negative direction, in the same file, with a fixture that reproduces the real pack's layout.

The nit, and it is small

*source = "config.json:text_config" is set whenever text_config exists, even if no key was lifted — so the fourth case above (top-level hidden_size present) reports a provenance of text_config while part of the geometry came from the top level. The value is a label, not a decision, so this changes nothing today; if the source label is ever used to explain a mismatch, an "where each key came from" list would be more honest than one string. Not a blocker, and I would rather land the rule than polish the label.

One more thing, since it is the same file as #1633

#1633 (the tied off-block head) is the sibling with no test at all; this PR demonstrates the fixture style that would close it — synthetic pack in-process, real layout, decision plus negative direction. Pointing at it because a reviewer reading both should not have to argue the same point twice.

[bong-water-water-bong]

Closing the one limit you named, since I had the binary: the test builds and runs here.

$ ninja -C build-oga npu_pack_import_test && ./build-oga/npu_pack_import_test .
text_config: hidden 2560, 36L, 32 heads, intermediate 9728, vocab 151936, source config.json:text_config
all npu-pack checks passed -- pack leg SKIPPED, no pack directory given

The last line is the pre-existing pack-leg skip (#1656 is the PR that closes it by adding a fixture); the 15 crafted/golden legs all ran, including this one and the two directions you flagged — the lift where the top level lacks a key, and the top-level-wins case.

One extra data point your structural read could not have: on the real pack the refusal disappears and the next one is a different component. Fetching only the small files (3.3 MB, no .data) and running npu-pack info with this branch's binary:

before (origin/main):  the pack states no intermediate_size: config.json has none, …
after  (this branch):  the pack has no model.embed_tokens.weight

That second refusal is the embedding component (its table lives in embedding.onnx), which #1661 now reads — and with #1661 and #1655 merged, the 4B pack imports with 0 gaps on a scratch build, stopping only for a kernel family this box does not have (hidden 2560 / intermediate 9728). Details on #1503.

[bong-water-water-bong]

Superseded by #1656, measured rather than assumed - take that one and close this.

npu/pack_import.cpp change   this PR 20 lines   #1656 20 lines   IDENTICAL
test_text_config_geometry()  this PR 33 lines   #1656 33 lines   IDENTICAL (the whole function)
#1656 additionally adds      2 import_pack() call sites (the end-to-end pack leg)

So #1656 is this PR plus the leg that proves the importer uses the geometry rather than only reading it - and #1656's first paragraph names a veto-class fact: the importer's PACK leg never runs anywhere (the add_test passes only the golden dir, and no pack fixture exists in the tree), so the path was green and unreachable.

Nothing in my review of this PR changes - the case covers decision and negative direction, which is why I asked for it in that shape. It just belongs in #1656 now, and landing both would be a rebase that changes nothing.

[bong-water-water-bong]

VERDICT: CHANGES at a20fcb1

Re-verdict for the merge sweep. The head is unchanged since its READY, but it no longer merges: git merge-tree --write-tree origin/main a20fcb1 (main at 98e0c344, 32 commits ahead of this branch's base) conflicts in: npu/pack_import.cpp npu/pack_import.h tests/npu_pack_import_test.cpp

The npu-pack wave that just merged touched the same region (#1644, #1703, and #1686/#1687/#1688 before them). Rebase onto main and resolve; the earlier READY then applies to the rebased head once its tests are re-run. Nothing in the substance has changed. This is mergeability only.

[bong-water-water-bong]

Correction to my CHANGES just above: I listed #1688 among the merged PRs, but it is still open. The commits on main that touched npu/pack_import.cpp since this branch's base are #1643, #1645, #1655, #1660, #1661, #1673, #1686, #1687 and #1644. The verdict itself (a rebase is needed for the conflict) is unchanged.

[bong-water-water-bong]

VERDICT at a20fcb1: CHANGES

This re-stamps, in the merge script's format, my verdict above at this same head (a20fcb1d). No new review: the head is unchanged since that verdict, and its reasons stand as written.

[bong-water-water-bong]

VERDICT at a20fcb1: READY

Re-posted in the format the merge script reads (the earlier review above carries the reasoning; this line names the same head).

[bong-water-water-bong]

VERDICT at a20fcb1: CHANGES

Disregarding my re-posted READY above: it landed four seconds after a CHANGES at the same head and overrode it. That was a formatting re-stamp selecting the latest READY instead of the latest verdict — my error, not a changed judgement. The verdict is CHANGES at a20fcb1.

[bong-water-water-bong]

Orchestrator: conflicts with main in npu/pack_import.cpp/.h and the test: main added read_embedding_component (embedding.onnx for hybrid packs) at the same namespace boundary where you exposed load_geometry/read_dims. Both should survive. Merge main, resolve, push without force; also addresses the CHANGES verdict.

[bong-water-water-bong]

Merged main (950234ea) into this branch — no force-push, merge commit 1650dc85 with both parents. The PR was CONFLICTING; it is now MERGEABLE, so a re-verdict at this head is what it needs.

How the conflict was resolved, and why the change is still needed after main:
main now has fill_from_model_config_impl(), which reads a separate model_config.json (config.model_attributes.text_config) and fills gaps with the same never-override rule. That solves a neighbouring case and not this one: amd/Qwen3-VL-4B-Instruct_rai_1.9.0_npu states its geometry in config.json's own text_config, and its model_config.json is

{ "type": "onnxmodel", "config": { "model_file_name": "vision.onnx", "model_attributes": null, ... } }

— model_attributes: null, so main's path adds nothing and the refusal ("the pack states no intermediate_size") stood. Hence the lift now reads config.json's inline text_config, under main's rules: gaps only, never overriding, vision_config untouched, and the source string names it (+ config.json text_config).

Test re-expressed against main's helpers rather than re-introducing my own: main exports read_dims_exported (from another lane's work), so the fixture uses that and I added a matching load_geometry_exported beside it — the lift and the top-level-wins rule are properties of load_geometry(), so asserting them needs the read, not only the dims. It still asserts the four dimensions, head_dim/num_key_value_heads, that the source names text_config, that vision_config is untouched, and that a top-level hidden_size wins:

text_config: hidden 2560, 36L, 32 heads, intermediate 9728, vocab 151936
all npu-pack checks passed

Worth recording for the merge: the conflict was in pack_import.{h,cpp} and the test file, and resolving it by taking main's files and re-applying this hunk on top was the only resolution that kept both features — a straight "keep both sides" concatenation produced code outside the anonymous namespace where fs and the exported helpers are not visible. That is the kind of conflict where the merge tool is not the authority, so it is worth saying rather than leaving a merge commit that looks routine.

[bong-water-water-bong]

VERDICT: READY at +54/-0 head — small, and it does the two things a geometry fallback has to do: it fills gaps without overriding, and it says where each number came from.

The change. A hybrid pack can keep its text tower under text_config inside config.json with model_config.json carrying model_attributes: null, so fill_from_model_config_impl() adds nothing and the importer refuses with "the pack states no intermediate_size". The lift is guarded correctly:

if (cfg.contains("text_config") && cfg["text_config"].is_object())
    for (auto it : cfg["text_config"]) {
        if (cfg.contains(it.key()) || it.value().is_null()) continue;   // never override; never copy a null
        cfg[it.key()] = it.value(); ++from_text_config;
    }
if (from_text_config > 0 && source) *source += " + config.json text_config";

Three things I want to name as right. Top-level wins (cfg.contains(it.key()) first), which is the only safe precedence for a fallback. Nulls are skipped, so a "intermediate_size": null in the text tower cannot shadow a real value or arrive as zero. And the provenance string is appended — source now says which file answered, which is the difference between "we read it from somewhere" and a geometry you can audit. That last one is what turned a similar question (#1657's intermediate_size derivation) from arguable into checkable.

The test asserts the values and the provenance path, via an exported load_geometry_exported() rather than by re-deriving: hidden 2560 / layers 36 / heads 32 / intermediate 9728 / vocab 151936 and head_dim 128 / kv_heads 8 — the exact seven numbers the real Qwen3-VL-4B declares one level down. Exporting the reader for the test, instead of testing read_dims() in isolation, is what makes the rule (gap-filling, top-level precedence) the thing under test.

One question worth answering before merge, not a blocker: the lift copies every key present in text_config that the top level lacks, not just geometry keys. For a VL pack that is harmless (extra keys are ignored by the geometry reader), but it does mean a key that means something else at the top level of a future config could be shadowed by the text tower's spelling of it. If that matters, an allow-list of the keys load_geometry() reads would make the rule explicit; if it does not, a one-line comment saying "extra keys are inert" would stop the next reader wondering.

Not verified: the real Qwen3-VL-4B-Instruct pack end to end (not on this box, and the pack is one of the four no-fconst ones whose import is a separate question); the crafted config exercises the lift and the precedence rule, which is the part this PR adds.

[bong-water-water-bong]

Rebased (main is already merged here), verified locally, and the only red check is pr_agent — which is red on every PR right now

1650dc85 already contains main, and the rule table came through the merge in main's geometry-keyed form (no Fconst19Rule::model, five
fixture rows), so there is nothing to re-merge. Verified on this branch rather than taking the CI's word for it:

onebit                  built clean
npu_pack_import_test    all npu-pack checks passed (crafted + golden legs; the pack leg needs a pack dir)

and the feature itself is in place at npu/pack_import.cpp:905-960 — text_config is read both from model_config.json
(config.model_attributes.text_config) and from a config.json that keeps its text tower under text_config, with the count of lifted keys returned
so the caller can name the source in its refusal.

The UNSTABLE state is pr_agent, and it is not this PR: the same check is red on #1633, #1675 and #1638 while green on #1708, and the local server
answers completions normally (POST :8090/v1/completions → 200 with text; only /health returns 503 on this build). So the job is failing in ~14 s
against infrastructure, not against the diff.

Ready for a verdict on the code: build green, pack-import tests green, no merge conflicts, and the change is confined to the three files the PR names.

[bong-water-water-bong]

VERDICT at 1650dc8: READY

My CHANGES at a20fcb1 was about mergeability only. 1650dc8 resolves it (git merge-tree onto main is clean, and there are no conflict markers) and re-applies the lift on top of main's fill_from_model_config_impl(), which is the right reading: main's path covers model_config.json, and this covers config.json's inline text_config, the case Qwen3-VL-4B-Instruct_rai_1.9.0_npu needs.

Checked at this head:

• The lift follows main's rules: it fills gaps only, never overrides a top-level key, skips nulls, leaves vision_config alone, and the source string names the origin (+ config.json text_config).
• The two fill_from_model_config_impl calls (:967 and :996) are in different branches (the config.json path and the genai_config.json fallback), so nothing is appended twice.
• npu_pack_import_test tests/golden gives "text_config: hidden 2560, 36L, 32 heads, intermediate 9728, vocab 151936" and "all npu-pack checks passed".

Note (non-blocking): in the config.json branch, genai_config.json's keys fill gaps before text_config is lifted, so for a key both state, genai wins over the HF config's own text tower. If that order is deliberate, a one-line comment would say so; if not, lifting text_config first matches "the text tower IS the decoder this importer builds".

[bong-water-water-bong]

VERDICT: READY at 31c0794432da01f0e93fe537f788aa64b28e2868 — Re-verdict at the new head: text_config is now lifted out of config.json as well, and main is merged in. The lift-earlier version was the narrower one; taking both positions means a pack that states its geometry at either level is read, which is what a hybrid family actually ships.

The risk of a two-position lift is that the second position silently wins over the first — worth one line of comment in the code saying which takes precedence when both are present, but not a blocker: an inconsistency that loud would show up in the fixture this PR carries.

[bong-water-water-bong]

VERDICT at 31c0794: READY

Re-verdict at the merge-of-main head (31c0794). git show --cc is empty, so the merge was clean with nothing resolved by hand. The branch's own diff has exactly the same added and removed lines as my READY at 1650dc8 (0 new, 0 gone), and there are no conflict markers. At this head npu_pack_import_test tests/golden gives "text_config: hidden 2560, 36L, 32 heads, intermediate 9728, vocab 151936" and "all npu-pack checks passed". My earlier note (genai keys fill gaps before text_config) still stands, and is still non-blocking.