Repository navigation
os: prepare-stick stages an NPU model directory, so the release check can ask the NPU - #818
Conversation
… can ask the NPU 1bit-release-check's chat-npu row SKIPped on the c20 hardware boot (2026-11-02) with 'no onebit NPU entry is listed by Lemonade'. Not a defect of that run: the NPU route needs a model, the stick carried only the 0.6B GGUF, and Lemonade lists only what it can load - so there was nothing to ask. init.sh already registers every /data/models/*/model.q4nx as '<directory>-1bit' with onebit_backend npu; prepare-stick simply never put one there. --npu-model DIR copies the directory (dereferencing symlinks: on a build box it is symlinks into the FLM install, and a dangling link fails at load time on the console) into data/models/<name>/, where init.sh registers it. The directory name is the entry name, so the catalogue ids (Qwen3-0.6B-NPU-1bit, ...) are what to name it; a name already ending in -1bit is refused before anything is mounted, and the --check assertions prove model.q4nx matches the source byte count, the copy holds no dangling symlink and no symlink at all.
PR Reviewer Guide 🔍(Review updated until commit b3928aa)Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: CHANGES at f36215f (one assertion is wrong on the exact case cp -RL exists for)
I ran it end to end on throwaway copies of the c20 image (cp --sparse=always work/os/1bit-os.img /var/tmp/ps-*.img, never the stick, /dev/sda never opened).
The blocker: --npu-model fails its own size assertion when the source directory holds symlinks — which is the
build-box case the PR's own rationale names ("on a build box the model directory is symlinks into the FLM install").
source dir: /var/tmp/npu-link-0.6B, 5 files, all symlinks into ~/models/npu/Qwen3-0.6B
$ sudo -n os/mini/prepare-stick.sh /var/tmp/ps-link-tprs.img --npu-model /var/tmp/npu-link-0.6B …
npu models/npu-link-0.6B (5 files, 695260276 bytes) <- the copy is right
PASS models/npu-link-0.6B has no dangling symlink (the bytes are on the stick)
PASS models/npu-link-0.6B is real files, no symlinks
FAIL models/npu-link-0.6B/model.q4nx is missing or a different size
prepare-stick: 1 assertion(s) failed on /dev/loop16p2; this stick would not work as it is rc=1
The copy is provably correct — the file on the stick is 683820936 bytes and holds no symlink. The failure is in the
comparison: stat -c %s does not dereference, so the source side is the link's own length.
$ stat -c %s /var/tmp/npu-link-0.6B/model.q4nx -> 45 (the link target string)
$ stat -Lc %s /var/tmp/npu-link-0.6B/model.q4nx -> 683820936 (the file)
$ stat -c %s <stick>/models/npu-link-0.6B/model.q4nx -> 683820936
os/mini/prepare-stick.sh:412-413 compares those two against each other, so any --npu-model directory built from
symlinks — i.e. every FLM install — prepares fine and then declares the stick broken. One-line fix: stat -Lc %s on
the source in both the if and the ok message. The same one-liner is owed in the GGUF loop at line 401
([ "$(stat -c %s "$f")" = "$(stat -c %s "$m")" ]), where it is pre-existing rather than introduced here: a symlinked
--model x.gguf fails the same way.
Your body's verified case used a real directory (~/models/npu/Qwen3-0.6B-NPU, 5 real files), so this is the one shape
the test did not take — and it is the shape cp -RL is there for.
Everything else checks out, reproduced here:
- real directory (
~/models/npu/Qwen3-0.6B, 5 files):npu models/Qwen3-0.6B (5 files, 695260276 bytes)— the same
byte count the body records — then 14 PASS / 0 FAIL (the body says 13; my count is 14 on two runs) including
model.q4nx is there, same size as the source (683820936 bytes),no dangling symlink,real files, no symlinks,
andregisters as the entry 'Qwen3-0.6B-1bit'. --checkon that prepared image → rc 0, 14 PASS, same assertions, "nothing was written".- refusals, all rc 1 and before the mount (the target does not even have to exist):
--npu-model /tmp→not a directory (the NPU format is a directory, not a file); a directory with no
model.q4nx→carries no model.q4nx;--npu-modelwith no argument →--npu-model needs a directory;
a directory namedfake-1bit→already ends in -1bit … rename it to 'fake' so the entry is 'fake-1bit'. - the registration claim is the code's:
os/mini/init.sh:365-368registers every/data/models/*/model.q4nxas
<dir>-1bitwithrecipe_options.onebit_backend=npu, andrelease-check.sh:498askspick '[^"]*-NPU-1bit', so a
directory named…-NPUis whatchat-npupicks up. That is the mechanism, not a new one. bash -nclean,tools/copyright.py --checkrc 0 (checked on my worktree at this head).
One number to know, not a defect: on an image file the data partition is the image's 2 GiB, so after this model the
run ends at free space on the data partition: 78M. On the stick it grows to the device (#706), so the release flow is
fine, but prepping an image with both the 0.6B GGUF (396 MB) and an NPU model does not fit — worth one line in the
usage block if that is a path anyone takes.
|
Fix verified on this box, so it is a copy-paste rather than a re-derivation. Three one-word changes, all in @@ -398,7 +398,7 @@
- [ -f "$f" ] && [ "$(stat -c %s "$f")" = "$(stat -c %s "$m")" ] &&
+ [ -f "$f" ] && [ "$(stat -c %s "$f")" = "$(stat -Lc %s "$m")" ] &&
@@ -409,8 +409,8 @@
- if [ -s "$f/model.q4nx" ] && [ "$(stat -c %s "$f/model.q4nx")" = "$(stat -c %s "$d/model.q4nx")" ]; then
- ok "models/$name/model.q4nx is there, same size as the source ($(stat -c %s "$d/model.q4nx") bytes)"
+ if [ -s "$f/model.q4nx" ] && [ "$(stat -c %s "$f/model.q4nx")" = "$(stat -Lc %s "$d/model.q4nx")" ]; then
+ ok "models/$name/model.q4nx is there, same size as the source ($(stat -Lc %s "$d/model.q4nx") bytes)"With that applied to
|
…ets checked pi-prs's blocker on #818, reproduced and fixed: the copy was right and the comparison was wrong. stat -c %s does not dereference, so a source that is a symlink reports the link's own length - 45 bytes for model.q4nx, 42 for a gguf - and `--npu-model DIR` declared a perfectly good stick broken. A build box's model directory is exactly that shape (symlinks into the FLM install), which is the case cp -RL exists for. stat -Lc %s on the source side in all three places: the NPU size assertion, the same assertion's ok message, and the pre-existing --model (gguf) assertion, which had the identical bug for a symlinked gguf. The 'model models/%s (%s bytes)' line now reports the file's size too, not the link's. Tested on throwaway copies of the c20 image (/dev/sda never opened): - --npu-model with a directory of 5 symlinks -> copy 695260276 B, 13 PASS, --check rc=0 (was: FAIL + rc=1) - --model with a symlinked gguf -> copy 396705472 B, PASS, message names 396705472 (was: 42) - --npu-model with a real directory -> 13 PASS, unchanged - refusals before the mount -> unchanged (no model.q4nx / not a directory / -1bit) One test note worth keeping: a symlink DIRECTLY in /var/tmp cannot be followed by root at all on this box (fs.protected_symlinks: sticky world-writable dir, symlink owner != follower, dir owner != symlink owner), which looks like a code failure and is not - the symlink must sit in a normal directory to test this path.
|
CHANGES addressed at The blocker was real and it was mine:
Also took your usage-block point: one line now says an image file's data partition is the image's 2 GiB until written Re-tested, throwaway copies of the c20 image,
Two corrections to my own body's numbers, since you counted them: with One thing worth knowing for anyone repeating the blocker test: a symlink placed directly in |
|
Persistent review updated to latest commit c137261 |
|
Preparing review... |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at b3928aab — the blocker is fixed the way it was asked, and I re-ran the case that failed.
c137261 puts stat -Lc %s on the source side of all three comparisons (the NPU assertion, its ok message, and the
pre-existing --model gguf assertion) and in the model models/%s (%s bytes) write line. Verified by running, on
throwaway copies of the c20 image (/dev/sda never opened):
case (head b3928aab) |
result |
|---|---|
--npu-model a directory of 5 symlinks into ~/models/npu/Qwen3-0.6B (the blocker case) |
npu models/npu-sym (5 files, 695260276 bytes), 14 PASS / 0 FAIL, rc=0 — was 13 PASS + FAIL + rc=1 with "this stick would not work as it is" |
--check on that prepared image |
14 PASS, rc=0 |
--npu-model a real directory (control) |
14 PASS, rc=0, unchanged |
--model a symlinked gguf (stat -c %s 42, stat -Lc %s 396705472) |
11 PASS, rc=0, write line now models/g.gguf (396705472 bytes), assertion PASS |
| refusals before the mount | unchanged, all rc=1: "is not a directory (the NPU format is a directory, not a file)", "carries no model.q4nx", "directory name 'bogus-1bit' already ends in -1bit", "does not exist" |
The count depends on flags, as the rewrite now says: 14 rows with --key + --ssh-pubkey + --npu-model, which is
the combination I ran.
Own diff is unchanged in substance: git diff origin/main b3928aab = 1 file, os/mini/prepare-stick.sh, +63/−5, the
two author commits. The head is c137261 plus a merge of main (which had moved to 21c72b8 with #821), and the merged
blob is the same one: c137261:os/mini/prepare-stick.sh = b3928aab:... = fb31471 (main's is 4cd93d4), so the
merge resolution kept this branch's side and nothing was lost.
CI at this head: build pass (5m35s), CodeQL pass, Analyze (actions|rust|c-cpp|python) pass, hw_gate skipped.
pr_agent failed in 18s with litellm.InternalServerError: OpenAIException - Connection error at 15:57:54Z — the
coder-llm endpoint was down across the box's reboot, not a finding on this diff. bash -n rc 0, tools/copyright.py --check rc 0.
One thing for the release lane's notes: #822 is already merged (3702f7831, 15:42:04Z), so the hw-gate npu-hold fix
does not wait for anything.
|
/review Verifying the coder-llm migration to the c20 engine ( |
|
/review |
|
Persistent review updated to latest commit b3928aa |
1 similar comment
|
Persistent review updated to latest commit b3928aa |
Why
The c20 live boot on hardware (2026-11-02) reported
RESULT: PASS (19 checks, no failures)with one row that could not be asked:That is not a fault of the candidate.
os/mini/init.shregisters every onebit model on the stick with Lemonade —/data/models/*.ggufas<name>-1bit(HRX) and/data/models/*/model.q4nxas<directory>-1bitwith"onebit_backend": "npu"— and the stick carried only the 0.6B GGUF, so there was no NPU entry to list. The mechanismalready existed; nothing ever put an NPU model on the stick.
prepare-stickhad--model FILE.ggufand no counterpart forthe engine's NPU format, which is a directory (
model.q4nx+ tokenizer + config).Net effect while it stays that way: the release check never asks the NPU on hardware, so the only NPU evidence a board run
produces is the hw-gate's
npurow — the Lemonade→NPU serve path is unproven there.The change (one file,
os/mini/prepare-stick.sh)--npu-model DIRcopies the directory intodata/models/<name>/, whereinit.shthen registers it as<name>-1bitwiththe NPU backend, and
chat-npupicks it up through the listing it already reads.--npu-model ~/models/npu/Qwen3-0.6B-NPUbecomes the entryQwen3-0.6B-NPU-1bit, which is the catalogue's own id for the NPU build of that model. A name that already ends in-1bitwould register as…-1bit-1bit, so it is refused — in the pre-flight block, before the partition is mountedand before an ssh key or a key line is written.
cp -RL. On a build box the model directory is symlinks into the FLM install; the stick needs the bytes, and adangling link would fail at load time on the console where nobody can see why.
--checkproves it read-only:model.q4nxpresent with the same byte count as the source; the copy holds nodangling symlink and no symlink at all; and it prints the entry the stick will register.
stat -L(pi-prs's review):stat -c %sreports a link's own length, so a symlinkedsource compared wrong and a correct stick was declared broken. Fixed for
--npu-modeland for the pre-existing--modelgguf assertion, which had the same bug; themodel … (N bytes)line now reports the file's size too.directory do not both fit in one image prep; on a stick they do.
--modelpath, no change toinit.shorrelease-check.sh.Verified — end to end on throwaway copies of the c20 image (never the stick)
cp --sparse=always work/os/1bit-os.img /var/tmp/ps-*.img, atc137261:--npu-modela directory of 5 symlinks (the shapecp -RLexists for)695260276 B; 13 PASS / 0 FAIL;--checkrc=0 (was FAIL + rc=1)--npu-modela real directorymodel.q4nx … 683820936 bytes,no dangling symlink,real files, no symlinks,registers as the entry 'Qwen3-0.6B-NPU-1bit'--modela symlinked gguf396705472 B, PASS, and the write line now names the file's size (was42 bytes)--checkon a prepared image--npu-modelname already ending in-1bit--checkafterwards shows no key was writtennot a directory; nomodel.q4nx→carries no model.q4nx; no argument →--npu-model needs a directorypick '[^"]*-NPU-1bit'on{"id":"user.Qwen3-0.6B-NPU-1bit"}→user.Qwen3-0.6B-NPU-1bitRow counts move with the flags: 13 with
--ssh-pubkey+--npu-model, 14 with--keyas well.Testing note for anyone repeating the blocker: a symlink placed directly in
/var/tmpcannot be followed by root onthis box (
fs.protected_symlinks: sticky world-writable dir, symlink owner ≠ follower, dir owner ≠ symlink owner) and lookslike a code failure — the symlinks have to sit in a normal directory to exercise this path.
bash -nclean;tools/copyright.py --checkrc=0. Not yet run: a live boot with the NPU model on the stick — that isthe next hardware run, and the point of the change is that
chat-npuwill then either PASS with a tok/s or FAIL with theengine's own reason instead of skipping.