Repository navigation
os/v3/puppy: the console-TUI frame is rebuilt in full once a second, and its log pane is not poll noise - #1093
Conversation
|
For review, and one thing that is not this diff: the branch reads The artifact for the next cut is |
PR Reviewer Guide 🔍(Review updated until commit 43861a7)Here are some key observations to aid the review process:
|
|
CI on the head
|
…so a stray line cannot stick The console's screen cannot be made private: anything holding the terminal open can write to tty1, and ratatui writes only the cells whose CONTENT changed since its own last frame - correct only while the screen still holds that frame. So one stray line leaves a wrong screen that a keypress does not fix, and when the write ends on the last row the VT scrolls, which is how the log pane's own lines end up painted across the CONNECT block and the MENU (the cycle-25 screenshot: "engint: o ebit", duplicated MENU blocks). Measured on the shipped c25 artifact 2fc6909c… in real ptys (TERM=linux, one printf of three printk-shaped lines to the pane's tty, then capture the pane): size clean +0.15 s +1.0 s +2.2 s after a keypress 80x25 0 2 2 2 2 240x67 0 4 4 4 - Permanent. Now the frame is rebuilt in full, from a cleared screen, at most once a second: the same write is gone at +1.0 s and stays gone, at both sizes. Cost measured by counting the bytes off a pty for 5 s (idle dashboard): 80x25 1.7 KiB/s (was 0.4), 240x67 4.5 KiB/s (was 1.0). The Clear is one escape and the 240x67 repaint is 4.5 KiB, so the blank is well inside a display frame by arithmetic - NOT measured on the appliance's own VT, which a pty cannot answer; the README says so. Scope: src/main.rs and the README paragraph. The two other halves of cycle-26 item 1 - the log pane's poll noise and the kernel's console level - are #1088's (they were drafted here first and are dropped from this branch in its favour, after their measurements: console_loglevel is overruled to 4 by Ubuntu's sysctl.d, a fact this branch's draft had missed). 52 tests (unchanged - nothing here is unit-testable; the reproducer is in the PR body), clippy --all-targets clean, fmt --check clean, tools/copyright.py --check rc 0, the two committed snapshots byte-identical, the eight gate strings still on the 80x25 frame, the 78-run pty sweep identical to the shipped build's notes. Artifact da5f6763c74a15a5, 1,581,384 B, static. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
f532d0f to
9963252
Compare
|
Re-scoped, and why (also said on #1088, comment
|
|
CI on
|
…e's own VT, not only in a pty The README said the one thing a pty could not answer was the appliance's own console. It has now been answered on a booted frozen c25 image (QEMU, `tty1::respawn:/usr/local/bin/1bit-console`, guest VGA text console at 160x50): the screen read the way the gates read it (1bit-tty1 -> /dev/vcsa1) plus QEMU screendumps of both states. * shipped 2fc6909c...: two stray lines written to /dev/tty1 were still on the screen 4.3 s later AND after a keypress sent through QEMU's own keyboard (9 actions, 2 stray rows at +0.3/+3.3/+4.3 s) - the permanence is real on the VT, not just in a pty; * this branch: the same write was gone by the next frame (0 stray rows at +0.3/+1.5/+3.5 s and after a keypress), and the healed frame is identical to the clean one except the log pane's own new lines (one row: the LOGS follow indicator). Also recorded, because the second run of the harness found it: killing the TUI does NOT bring it back - 1bit-console continues to exec 1bit-console-menu (its documented fallback), so tty1 becomes the dialog menu, which repaints on input and therefore does not heal either (2 stray rows until a key is pressed). That is why the fix belongs in the ratatui screen. What a screendump cannot catch is a sub-frame blank, so "the Clear is invisible" stays arithmetic (one escape + 4.5 KiB). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
6de1ef5 to
3752ebc
Compare
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 3752ebcb — the mechanism is right for the reason given, and the crate's tests pass at this head.
The claim, checked against the code: the loop ticks at event::poll(Duration::from_millis(250)), so "draw() runs ~4x/s" is accurate, and the new heal is if last_heal.elapsed() >= Duration::from_secs(1) { term.clear() } before the draw. That is the correct instrument for this bug: ratatui diffs against its own previous buffer, so a stray line written by anything else leaves the screen wrong until a cell's content changes again — Terminal::clear() discards that buffer, so the next draw() repaints every cell. Without it, no amount of extra drawing fixes a screen someone else painted on.
cargo test --offline --release in os/v3/puppy/console-tui at this head: 52 passed, 0 failed, including the snapshot/size cases (the_mdns_name_is_on_screen_at_both_sizes, the_qr_is_drawn_beside_the_block_only_where_it_fits, the_menu_sits_between_the_facts_and_the_log). The error paths are handled the same way as the draw (break 1), so a terminal that cannot be cleared still exits the TUI rather than looping.
Two notes, neither blocking:
- The cost is a full repaint every second for as long as the TUI runs, which is the right trade for a screen nobody else may paint on — but it is also the reason the first half of item 1 (#1088: stop the writers) matters, and the two PRs should land together to be worth their cost.
term.clear()clears the backend's screen; if it ever needs to be cheaper, the usual refinement is to clear only when a writer is known to exist. I would not do that now — a stray line you cannot see is worse than a repaint you cannot see.
I did not boot an appliance for this; the repaint is a Rust/VT mechanism and the crate tests plus the code are the evidence available off-device.
|
Persistent review updated to latest commit 43861a7 |
|
Merged as The squash took the branch's tree exactly ( But the squash title is the PR's first title ("…and its log pane is not poll noise"), while the merged diff is only the So c26 item 1 as it stands on |
…tead of ending the console (#1102) The module's first promise is "the console never goes dead ... a paint failure redraws", and the loop ended on the first paint error - which is right for a terminal that cannot be written and wrong for `ErrorKind::Interrupted`, where a signal arrived before the write reached the terminal, nothing was drawn, and the next pass should simply paint again. It is reachable: crossterm's event source installs a SIGWINCH handler (that is how resize events arrive), so an ssh user dragging a window edge is the realistic trigger - and #1093's once-a-second `term.clear()` added a second "one error ends the console" point. `paint_failed()` is now the single classification: `true` for every kind except `Interrupted`. An interrupted clear is ignored (the heal retries next pass); an interrupted draw skips the OSC-8 pass and repaints on the next iteration; anything else still exits 1, which is what makes `1bit-console` show the dialog menu. Measured: 53 tests (52 + `an_interrupted_paint_is_repainted_but_a_dead_terminal_ends_the_console`: Interrupted -> retry; BrokenPipe/PermissionDenied/NotFound/Other -> fatal). 12 window resizes in 2 s while the TUI runs (tmux resize-window = a SIGWINCH storm): session survives, nine actions, `q` still stays on the console. Heal unchanged on the happy path (stray write gone at +1.0 s at 80x25), both committed snapshots byte-identical, the eight gate strings still on the 80x25 frame, clippy --all-targets and cargo fmt --check clean, tools/copyright.py --check rc 0. Artifact 906705b04783a87d, 1,581,568 B, static. Not measured: the EINTR itself (a pty cannot be made to interrupt crossterm's write on demand), so the classification is unit-tested and the path reasoned - the resize storm and a fatal error's exit are the two ends that could be demonstrated. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
… console heals) c26 item 1 was "nothing may paint over the console", and #1093 made the console survive it - but nothing judged it afterwards, so the first regression would be found by the owner's eyes again. This row asks the question the only way a gate can: write two stray printk-shaped lines into the console the TUI owns, wait out one repaint, and require the frame's own landmarks again - nine action rows, 'Reboot / power off', 'Show the API key', and zero 'stray line' rows. The action count greps `^ ?[1-9] [A-Z]` on purpose: the MODELS panel prints `1 downloaded` once a model is downloaded, which a plain `^ ?[1-9] ` counts as a tenth action (measured on a booted image). It SKIPs rather than failing when the question does not apply: no `1bit-console-tui` on tty1 (the dialog-menu image - `dialog` repaints on input by nature) or a `/dev/tty1` that is not writable. Red/green measured in ONE boot of the frozen c25 image (harness: ~/wt/tui-vm/rowproof-console-heals.sh, report ~/wt/tui-vm/report-rowproof.txt): the SHIPPED console TUI (2fc6909c…): actions=9 stray=2 -> console-garbled (RED) the merged artifact (da5f6763…): actions=9 stray=0 -> console-heals (GREEN) `bash -n` clean, tools/copyright.py --check rc 0, one file. Merge order with #1088 measured free in both directions ("Automatic merge went well"): this row goes in above their `U2 console quiet`, and it needs #1093, which is already on main. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… console heals) (#1111) * os/v3 puppy: a vm-check row for the console that holds its screen (U2 console heals) c26 item 1 was "nothing may paint over the console", and #1093 made the console survive it - but nothing judged it afterwards, so the first regression would be found by the owner's eyes again. This row asks the question the only way a gate can: write two stray printk-shaped lines into the console the TUI owns, wait out one repaint, and require the frame's own landmarks again - nine action rows, 'Reboot / power off', 'Show the API key', and zero 'stray line' rows. The action count greps `^ ?[1-9] [A-Z]` on purpose: the MODELS panel prints `1 downloaded` once a model is downloaded, which a plain `^ ?[1-9] ` counts as a tenth action (measured on a booted image). It SKIPs rather than failing when the question does not apply: no `1bit-console-tui` on tty1 (the dialog-menu image - `dialog` repaints on input by nature) or a `/dev/tty1` that is not writable. Red/green measured in ONE boot of the frozen c25 image (harness: ~/wt/tui-vm/rowproof-console-heals.sh, report ~/wt/tui-vm/report-rowproof.txt): the SHIPPED console TUI (2fc6909c…): actions=9 stray=2 -> console-garbled (RED) the merged artifact (da5f6763…): actions=9 stray=0 -> console-heals (GREEN) `bash -n` clean, tools/copyright.py --check rc 0, one file. Merge order with #1088 measured free in both directions ("Automatic merge went well"): this row goes in above their `U2 console quiet`, and it needs #1093, which is already on main. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * os/v3 puppy: a skip marker is a SKIP, in one rule with a self-test (pi-prs' CHANGES on #1111) `U2 console heals` reports why it cannot run with `console-heals-skip-no-tui` / `-no-tty1`, and the driver's verdict was `want in body` — so both skip markers CONTAIN the row's own want string (`console-heals`) and a row that never ran printed **PASS**. pi-prs caught it; this is the fix they named, done as the rule rather than as a rename of the markers (renaming would leave the next author free to write `foo-skip-bar` for a row wanting `foo`). * **One implementation**: `os/v3/puppy/vm_check_verdict.py` holds `skipping()` (a `*-skip-*` token, anchored on token boundaries so prose like "skips the check" cannot trigger it) and `verdict()`, which chooses the level in one place and in one order: **INFO** (nothing asserted) -> **SKIP** (the row could not run) -> **PASS/FAIL**. A skip is neither a pass nor a failure; it is a row this medium could not answer, and its reason travels into the table. * **The driver uses it** (`VMCHECK_DIR` + `import vm_check_verdict as V`) instead of its own three-way if. It no longer formats either; the module returns the flattened line. * **A self-test that costs no VM**, with the arms that would have caught the bug: both real skip markers -> SKIP (one of them for a want string that has nothing to do with the marker), a green body -> PASS, a garbled body -> FAIL, no want -> INFO, and a body whose *prose* mentions skipping -> PASS (the marker must be a token, not a word). `vm-check.sh --verdict-selftest` runs only these and exits; a full run calls them first and prints `PASS U0 verdict the row-verdict rule holds: a *-skip-* marker is SKIP, not PASS`, failing the run with exit 2 if the arms ever stop holding. Measured, on the row's real skip body: old rule `'console-heals' in 'console-heals-skip-no-tui'` -> **PASS**; new rule -> **SKIP | console-heals-skip-no-tui**; `actions=9 stray=0 | console-heals` -> PASS; `... stray=2 | console-garbled` -> FAIL. Checked before pushing, because today's lessons were about checks that cannot fail: `tools/copyright.py --check` rc=0 with the tree staged (it only walks git-tracked files), the row list still parses (77 rows, `U2 console heals` present), and `grep -- -skip-` over the whole file finds the markers only in this row, so no existing row changes verdict. Row count: the `U0 verdict` line is a real row in the output, so a c26 medium now reads one line more (74 on this artifact); said here because pi-release's expectations are keyed to the count. --------- Co-authored-by: t <t@t> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: pi-os3 <pi-os3@1bit.monster>
…nges and stops saying 52 tests (#1149) The crate's README describes the frame heal (#1093) but not the two changes that arrived with it, so a reader of the shipped crate cannot see why a control byte from Lemonade's log is harmless, or why an interrupted paint does not end the screen: * #1118 - facts::screen_text() cleans text from outside (CSI and OSC removed whole, TAB to a space, other C0/DEL dropped) before it becomes a cell. Measured against the recorded take's own log: the shipped c26 candidate (5b21a0a8, heal and no sanitiser) leaves 4 escape bytes in one rendered frame and on a 240x67 pty cuts its progress row at the first ESC[K; this build is 0 bytes. On the appliance's own 160x50 VT (booted c26 candidate, QEMU) the same wrapped progress line came back whole on BOTH binaries - the once-a-second full paint rewrites the cells the erase cleared - so the README calls this defence in depth rather than a visible repair, which is what the measurement says. Test: a_log_line_or_a_model_name_cannot_carry_an_escape_into_the_screen * #1102 - a paint that fails with ErrorKind::Interrupted repaints instead of ending the console (crossterm's SIGWINCH handler makes an ssh resize the realistic trigger); every other kind still exits to the dialog menu. Test: an_interrupted_paint_is_repainted_but_a_dead_terminal_ends_the_console Also corrects the test count: cargo test is 55, not the 52 the README has said since #1050/#1059. Docs only - no code, no frame, no gate string touched. cargo test 55/55, tools/copyright.py --check rc 0. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…ree rows that could skip themselves (#1218) c29 ships /usr/local/bin/1bit-selftest (engine #1166) - the one-command proof the install sheet asks the operator to run - and no row ever executed it. The file was proved present and byte-identical to os/mini/selftest.sh, and proved by nobody that it runs in the appliance: the same class the U4 release-check row was added for in cycle 21. The new row does not demand 'RESULT: PASS'. The image boots with an empty /data/models, where the tool's own documented answer is 'RESULT: FAIL - no model to test', so demanding PASS would red a healthy image - and a row that reds a healthy image gets deleted. It requires a VERDICT: PASS with a model, SKIP naming the no-model reason without one, FAIL when the binary is absent or dies without a RESULT line. Its first version printed selftest-broken-missing and came back SKIP, and that is how the second half of this commit was found: Puppy's shell echoes the typed command into the row's own output, vm_check_verdict.skipping() matches a *-skip-* token anywhere in the body, and the command carried the marker literally - so the row was skipped by the echo of itself. Two rows already in the tree carry a literal marker and are exposed to the same thing. U1 shipped shells (#1186, merged tonight) was measured self-skipping, i.e. the row that exists to catch an unparsable script had never run; U2 console heals (#1093) is latent. All three now assemble the marker from m/sk, so the token exists only in the output. No row's semantics change. Proved: four arms under bash and sh on fixtures (absent / no-model / pass / dies silently); the module's own verdict mapping on the echoed command before and after (SKIP with token m-skip-no-model -> no token, so the output decides); and end to end on two shipped artifacts, c28 0c371992 (no such file -> FAIL selftest-broken-missing, and U1 now RUNS: 'shells-parse: 45 scripts') and c29 8266aff9 (SKIP selftest-skip-no-model on its blank models dir, 77 PASS / 0 FAIL / 4 SKIP). Co-authored-by: pi-os3 <os3@1bit.invalid>
Closes #1092. Cycle 26 item 1, the console half (
~/briefs/TOS3-C26.md: tty1 came back garbled from the cycle-25 take -"
1EShow the API key", "engint: o ebit", duplicated MENU blocks).Two defects in
os/v3/puppy/console-tui, both measured on the take's own evidence, neither a substitute for the other.1. The frame is rebuilt in full once a second
draw()runs ~4x/s, but ratatui writes only the cells whose content changed since its own last frame. That is correctonly while the screen still holds that frame, and nothing keeps other writers off tty1 on this appliance:
console=ttyS0,115200 console=tty0with nologlevel=, sokern.infoand up are painted on tty1 - the c25 take'sdmesg.loghasusbcore: deregistering interface driver snd-usb-audioat t=1759 and anamdxdnaTDR dump at t≈6971bit-release-checknpu_note()writes "\n 1bit check: is holding the NPU" to/dev/tty11bit-console-menu --action Nruns adialogprogram on tty1Reproduced on the shipped c25 artifact (
2fc6909c…, what the c25 image embeds): a real pty (tmuxpane,TERM=linux), oneprintfof three printk-shaped lines to the pane's own tty, then capture the pane:Permanent, and a keypress does not heal it: the diff has nothing to notice. That is the owner's screenshot - and because
the stray write ends with a newline on the last row, the VT scrolls and the log pane's LEM lines are left painted over the
CONNECT rows and the MENU, which is what "
engint: o ebit" looks like:After (same pty, same single write, this branch's artifact): the stray text is gone at +1.0 s and stays gone, at
both 80x25 and 240x67.
Cost, measured by counting the bytes off a pty for 5 s (
TERM=linux, idle dashboard):2fc6909c…Clearis one escape the repaint lands behind, so the VT shows no visible blank (a 240x67 frame is 4.5 KiB, i.e.sub-frame on the console) - and a full frame per second is worth it: it bounds what any stray line can cost the person
standing at the box to one second.
2. The log pane drops the poll lines
The take's own log (
/data/logs/lemond-c25-take.log, 2039 lines) is 549 ×GET /api/v1/models- one every ~3 s fromthe open demo pages - against 61 chats, 12
/internal/config, 6 image generations. The pane is three rows at 80x25 and~40 at 240x67, so the polls were the pane, and every chat/load/error had scrolled out of it.
facts::poll_linedrops the timer-driven GETs (/api/v1/models,/api/v1/health,/api/v1/stats). The method and theexact path are both required, so a
POSTto the same path and/api/v1/models/<name>/optionsare kept. A tail that isnothing but polls is shown unfiltered rather than blanked - an empty pane would print "no Lemonade log and no kernel
GPU/NPU lines yet" over a log that exists.
Measured on that same file, rendered at 240x67 (the take log staged at
/var/log/lemond-onebit.log, then removed):2fc6909c…GET /api/v1/modelsPOST /api/v1/images/generations - 200, thevae decode graph … 11.12s/generate_image completed in 15.75slines, thePOSTsTests
cargo test53/53 (52 +the_log_pane_drops_the_polls_and_keeps_the_chats, which uses lines copied out of that log,including two that only look like polls, and asserts the fallback for an all-poll tail). Removing the filter makes it red
(measured, then restored).
Also on the artifact this branch builds (
0995cc95dffaa48c, 1,582,424 B, static, noPT_INTERP):--snapshot 80x25 --fixtureand--snapshot 240x67 --fixtureare byte-identical to the committed snapshots - theheal changes when cells are written, not what the frame says, so no baseline churn for the next cut;
vm-check.sh/menugategrep on tty1 are all on the 80x25 frame (CONNECT,web UI http://,api http://,ssh root@,Lemonade Mobile,Show the API key,Reboot / power off,1-9 actions);and no new ones -
diffof the two runs is empty, so nothing about the degradation floors (os/v3/puppy: the console TUI's MENU rule gives way before an action does #1014/os/v3/puppy: a console-TUI message costs the legend a word, not an action #1026/os/v3/puppy: a console-TUI dialog loses its question before its hint #1028) moved;qover ssh still exits 0; on the consoleqand Ctrl-C still stay, with their message, and nine actions drawn;--all-targetsclean,cargo fmt --checkclean,tools/copyright.py --checkrc 0, no new files, no gate filetouched.
What this does NOT fix, on purpose
The writers. This is insurance for the console, not a licence to paint tty1: silencing printk (
loglevel=on thecommand line,
mkimage.sh) and the two console notes (1bit-release-check,puppy-check.sh) areos/v3's files and arebeing reported to pi-os3/pi-release with these measurements rather than edited from here. One measured fact worth their
time: the take's
config.jsonhaslog_level: debugbut nolog_file, and Lemonade'slog_file: automeans itsfile sink is off (
resolve_logging_targets:auto→targets.file = false), so thelog_max_file_size_mb: 8/log_max_files: 3keys in that file are inert and the only copy of lemond's log is10lemonade's>>redirect.Notes
git log 2fc6909c…origin/main -- os/v3/puppy/console-tuiis empty between thec25 artifact and this base.
:18097andnpu-serve-keeperuntouched.