Repository navigation
os/v3 puppy: vm-check asserts the console names the engine (#1041), so the row cannot go missing again - #1053
Conversation
…o the row cannot go missing again #1041 made the console name the engine that is serving, and no gate asserted it: vm-check greps /etc/inittab for the 1bit-console respawn line and checks the boot console, but not what the screen says, and release-check has no console row either. The only holders were pi-tui's unit tests and my out-of-tree cut check, both of which run before an image exists - so an image shipping a TUI that lost the engine row passed every gate. The row renders the shipped binary's own frame with its own fixture rather than grepping --status for 'engine:': measured here, a box with nothing loaded prints no engine line at all (the fact is None until a model is loaded), so the obvious row would be red on a healthy appliance. The transcript carries the other half of #1041 too (invented-urls=0). Measured: the c24 image's own /usr/local/bin/1bit-console-tui (5261cf56..., unsquashfs -cat) prints no engine line -> this row FAILs on it; 4b369c10... prints 'engine: llamacpp (gpu)' -> PASSes. vm-check on the shipped c24 image: 58 PASS / 1 FAIL / 3 SKIP, the single FAIL being this row, so nothing else moved. Red on pre-#1041 images is by design and is stated in the PR.
d3a8f7b to
a92c607
Compare
PR Reviewer Guide 🔍(Review updated until commit c2d8232)Here are some key observations to aid the review process:
|
|
Persistent review updated to latest commit a92c607 |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at a92c607e — the row bites on the old binary and passes on the new one, and I reproduced both halves myself.
The control that matters for a gate like this is "does it go red on the image that lacks the behaviour". Built the console at 0c862e64^ (the commit before #1041) and at a head that carries it, and ran the row's own command against each:
PRE-#1041 frame: LEMONADE up 8882 | GPU HRX0 43C | NPU idle | MODELS 2 | FANS controlled
grep -o 'engine: [a-z]* ([a-z]*)' -> (nothing) <- the row FAILs, as claimed
POST-#1041 same grep -> 'engine: llamacpp (gpu)' <- the row PASSes
and the row's second half on this box:
1bit-console-tui --status -> status-rc=0, invented-urls=0
so the transcript next to the assertion carries the other half of #1041 (no http://-: before there is an address) and does not go red on a healthy appliance.
The design decision behind the row is the right one and is stated in the comment: asserting off live facts would have made this row red on every healthy appliance, because engine is None until a model is loaded — measured here, --status with nothing loaded prints no engine line at all. Rendering the shipped binary's own frame from its own fixture asserts what the image can do without a model, a device or a network, which is what a vm-check row is for.
Two notes, neither blocking:
- The row pins the fixture's answer,
engine: llamacpp (gpu). That is correct today (the fixture loads one llamacpp/gpu model) but it means a future fixture that changes the model or its device moves this row too — worth the one comment line it already has, and worth remembering when the fixture is next touched. MERGEABLE BLOCKEDat the moment I read it was just the two CodeQL jobs still running on this head (buildandCodeQLhad not reported); nothing in the diff is waiting on a human.
|
Persistent review updated to latest commit c2d8232 |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at c2d82321 (rebase-only; the review stands).
The PR's own file is unchanged by the rebase — the diff between the two heads is other PRs' content (#1050's facts.rs, #1052's stop-token test) arriving with the new base:
git diff a92c607e…c2d82321 -- os/v3/puppy/vm-check.sh (no output)
So the row is the same one I verified: the frame grep FAILs on the pre-#1041 binary (I built 0c862e64^ and the frame has no engine: segment at all) and PASSes on a head with #1041 (engine: llamacpp (gpu)), with status-rc=0 invented-urls=0 beside it.
os/v3 puppy: vm-check asserts the console names the engine (#1041), so the row cannot go missing again
Root cause (what was unasserted)
#1041(0c862e64) made the console name the engine that is serving — the TUI gains a LEMONADEengine: llamacpp (gpu)row and stops printing
web UI http://-:13305before it has an address. No gate asserted either:vm-check.shgreps/etc/inittabfor the1bit-consolerespawn line, and checks the boot console — but not what thescreen says.
1bit-release-check(release rows) has no console row either.(
~/wt/os3i/c24-tui-check.sh), both of which run before an image exists. An image shipping a TUI that lost the rowwould have passed every gate we run.
That is the same silent-absence class as pi-release's
efivarfs.kofinding: the artifact is checked for presence, notfor the behaviour the cycle added.
The row, and why it is not written off live facts
The obvious row — run
1bit-console-tui --statusand grepengine:— would be red on a healthy appliance, so itwould have cost a cycle a red nobody could act on. Measured on this box, nothing loaded:
No engine line at all: the fact is
Noneuntil a model is loaded, by design (facts.rs:engineisNonewhen nothingis loaded). So the row renders the shipped binary's own frame with its own fixture instead. That asserts what the image
can do without needing a model, a device or a network — and the transcript next to it carries the second half of #1041
(
invented-urls=0).Measured, before and after
Every claim below was measured, not quoted. The "before" arm uses the exact bytes the c24 stick ships, extracted from its
own sfs, so it is the image's behaviour and not a rebuild:
/usr/local/bin/1bit-console-tui5261cf56…(1,567,800 B, extracted withunsquashfs -cat)--snapshot 80x25 --fixture | grep -o 'engine: …'→ no output#10414b369c10…(1,581,392 B)engine: llamacpp (gpu)vm-checkon the shipped c24 image with this patch9355f9f6…)FAIL U2 console engine ... status-rc=0 | invented-urls=0), so nothing else movedvm-checkon an image built from the#1041tree withCONSOLE_TUI_BIN=4b369c10…1bit-os-puppy.img(~/wt/puppy-out-c25test, gate-proof artifact, not a hand-off)vm-check rc=0— the row printsPASS U2 console engine engine: llamacpp (gpu) | status-rc=0 | invented-urls=0, and every other row is unchangedExpected red on pre-#1041 images, by design. The c24 stick carries
5261cf56…, so this row is red there and greenfrom the first image that carries #1041. Rows move with the tree —
U3 mdns outis the same story from c24 — and this isstated so nobody reads a c24 re-run as a regression.
The two images are a controlled comparison, so the delta is attributable: same payload (
opt/1bit-package/1bit/1bit=502d2a81…in both), sameetc/init.d/10lemonade(ba543b6a…), and onlyusr/local/bin/1bit-console-tuidiffers(
5261cf56…→4b369c10…).Checks
bash -nclean,shellcheck -S warningclean,tools/copyright.py --checkrc 0, and the row list still parses (74rows,
U2 console enginepresent) — verified by compiling the driver's own Python block rather than by reading it.Not in this PR
silently SKIPs is worse than no row). When a gate row loads a model, the live assertion is one line on top of this.