Skip to content

os/v3/puppy/console-tui: the README documents the two c26 console changes and stops saying 52 tests - #1149

Merged
bong-water-water-bong merged 2 commits into
mainfrom
os/v3-tui-readme
Nov 5, 2026
Merged

bong-water-water-bong merged 2 commits into
mainfrom
os/v3-tui-readme

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

What this is

Docs only, one file: os/v3/puppy/console-tui/README.md. The crate's README describes the frame heal (#1093) but not
the two changes that landed with it, and its test count has been wrong since #1050/#1059.

before after
test count in the README 52 55 (cargo test on this head: 55 passed)
#1118 (text from a log / a catalogue is cleaned before it becomes a cell) absent documented, with the measurement
#1102 (a paint interrupted by a signal repaints instead of ending the console) absent documented, with the reachable trigger

Why it matters rather than being bookkeeping: the README's promise is "the console never goes dead", and these are the two
places where that promise is actually kept — a control byte arriving as data would otherwise be executed by the
terminal, and an ErrorKind::Interrupted on a paint (crossterm installs a SIGWINCH handler, so an ssh user dragging a
window edge) would otherwise have ended the screen.

The measurement behind the #1118 paragraph, and what it does NOT say

Source: the recorded c25 take's own log (34 of its 2039 lines carry CR + ESC — the engine's progress bars).

  • Shipped c26 candidate 5b21a0a8… (heal, no sanitiser): 4 escape bytes in one rendered frame; on a 240x67 pty the
    progress row is cut at the first ESC[K and the wrapped remainder is gone.
  • This build: 0 escape bytes, same row whole.
  • On the appliance's own 160x50 VT it does not lose text on either binary — measured by booting the c26 candidate in
    QEMU, appending a wrapped 245-byte progress line to the guest's log and reading tty1: the last token after the final
    ESC[K was present with the image's own TUI and after installing this build. The once-a-second full paint rewrites
    the cells the erase cleared.

So the README calls the sanitiser defence in depth, not a visible repair. The earlier severity claim ("a visible
glitch on camera") was mine and this measurement is what corrects it.

Acceptance this was checked against (same boot, for the record)

The c26 item-1 acceptance sentence on a copy of the shipping candidate a445b269…, its own TUI: 100 polls at the demo
pages' 3 s cadence + 10 chats answered (0 failed); tty1 read every 30 s → actions=9 stray=0 at all ten samples, the
engine: llamacpp (gpu) row drawn, last request 86.2 tok/s; the frame diff against the clean pre-run frame is the log
pane plus the model actually being loaded and used — no layout movement, no gate string lost.

Checks

cargo test --offline 55/55 · tools/copyright.py --check rc 0 (no new file) · no code, no snapshot, no gate file,
no vm-check.sh string touched. Docs only, so no closes #N.

Co-Authored-By: Claude Fable 5.1 noreply@anthropic.com

…nges and stops saying 52 tests

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: Claude Fable 5.1 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

pi-prs: #1149 ready for review at 36459fbc — docs only (one file, os/v3/puppy/console-tui/README.md), no code, no snapshot, no gate string.

What it fixes: the crate README has said 52 tests since #1050/#1059 (it is 55) and never mentions #1118 (text from a log or a catalogue is cleaned before it becomes a cell) or #1102 (a paint interrupted by a signal repaints instead of ending the console) — i.e. the two places where the README's "the console never goes dead" promise is actually kept.

The one thing worth your attention is that I had to weaken my own earlier claim: measured on the shipping c26 candidate in QEMU, a wrapped progress line carrying ESC[K comes back whole on the appliance's 160x50 VT on both binaries — the once-a-second full paint rewrites the cells the erase cleared. The truncation I reported earlier is a 240x67 pty result. So the README calls the sanitiser defence in depth, not a visible repair, and I said so on the PR rather than shipping the stronger sentence.

Checks on the head: cargo test --offline 55/55, tools/copyright.py --check rc 0. No issue to close (docs).

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

CI note for the record: pr_agent shows red in 12 s but gh api .../actions/runs/42087189309 says conclusion: cancelled — the workflow concurrency group, not a review. Rerun requested; it is not a required context (the ruleset requires build).

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY

The one number in the diff is right, and the two paragraphs describe things that are in the tree.

$ cd os/v3/puppy/console-tui && cargo test --offline
test result: ok. 55 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.14s

55, not the 52 the README said, and not a hand-count: the same run names the two tests the new paragraphs point a reader at — facts::tests::a_log_line_or_a_model_name_cannot_carry_an_escape_into_the_screen and main::tests::an_interrupted_paint_is_repainted_but_a_dead_terminal_ends_the_console — and facts::screen_text() is at src/facts.rs:731, so the paragraph's claims are about code that exists rather than a plan.

The distinction the README now draws is the useful part and it is the honest direction: the sanitiser is "defence in depth, not a visible repair", because on the appliance's own 160x50 VT the once-a-second full paint rewrites what an ESC[K erased, so the earlier "visible glitch on camera" claim is retracted by measurement rather than quietly dropped. A README that walks a severity claim back and says which measurement did it is worth more than the test count.

I did not re-run the pty/QEMU measurements (240x67 pty escape-byte counts, the c26 candidate's 4 bytes vs 0, the 100-poll acceptance): they need the shipping candidate booted, which is pi-os3's rig. The cargo test line and the code references above are what I could check here, and they hold.

One thing to keep in mind for whoever merges: this README describes the crate at this head, so it goes stale the moment a test is added — the count is stated twice (the prose and the cargo test # 55 tests comment), which is the same trap the docs/kernel.md PR in this queue just fixed for a boot date.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Note for whoever lands these two: #1155 (open, src-only) adds 3 tests, so after it the crate suite is 58, not the 55 this PR's README paragraph now says. No conflict in the files themselves (this is README.md, that is src/), only the number — the second to land should say 58.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1093 - Partially compliant

Compliant requirements:

  • The console-TUI frame is rebuilt in full once a second
  • The log pane no longer drops poll lines but shows them instead
  • Measurements were taken on the c25 artifact

Non-compliant requirements:

  • None

Requires further human verification:

  • None

1050 - Partially compliant

Compliant requirements:

  • The console names the engine of the model that is answering, not the first one loaded
  • The engine_of function resolves the engine from model_loaded when available, otherwise from the first loaded model
  • The change updates the signature of engine_of to include a serving argument
  • Existing assertions are updated and a new test is added to verify the correct behavior

Non-compliant requirements:

  • None

Requires further human verification:

  • None

1059 - Partially compliant

Compliant requirements:

  • The README documents which model the engine row names
  • The --status command is pinned to carry the engine information
  • The README now states that the engine row names the llm type model from all_models_loaded[] with model_loaded as a fallback
  • The test count in the README is updated to reflect the new tests

Non-compliant requirements:

  • None

Requires further human verification:

  • None
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

README Update Accuracy

The README.md file has been updated to reflect the correct number of tests (55) and document two new features: text sanitization for external inputs and handling of interrupted paints. These changes are accurate and align with the described behavior in the PR.

cargo test                                    # 55 tests: the probe parsers, the QR drawing (incl. a literal known answer), the links, the short-window degradation, the two renders, the engine row's model choice, the escape sanitiser, the interrupted paint

@bong-water-water-bong
bong-water-water-bong merged commit 3c48e23 into main Nov 5, 2026
9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the os/v3-tui-readme branch November 5, 2026 21:50
bong-water-water-bong added a commit that referenced this pull request Nov 6, 2026
…runs (#1183)

#1155 ("a continuation row spends its indent out of the wrap") added three tests —
`facts::tests::a_continuation_fits_the_row_it_is_drawn_on` and
`ui::tests::{a_log_continuation_fits_the_pane_it_is_drawn_on,
a_packed_status_row_fits_the_pane_it_is_drawn_on}` — and did not touch the line that
states the count. #1149 had set it to 55, which was the exact number of `#[test]`
functions at 3c48e23, so the sentence was right when written and is 3 short now.

Measured: `cargo test --offline` in os/v3/puppy/console-tui at origin/main ->
"test result: ok. 58 passed; 0 failed; 0 ignored". The enumeration gains the new
class the three tests cover (the rows that have to fit the pane).

Co-authored-by: bong-water-water-bong <bong-water-water-bong@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant