From c6ec923392dc12bbeecd9ea4a45291839acd98bb Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Sat, 5 Sep 2026 21:11:09 +0700 Subject: [PATCH 1/5] fix: read what the analyzer wrote, and refuse what cannot be read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six findings that share a shape: something was consumed on a contract other than the one it is written to. **JSONL.** reconverge prints one findings document per analyzed *target*, and this reader handed the whole of stdout to a single `from_str` — so a kernel crate with a `src/main.rs` beside its library, the ordinary shape of a GPU crate, hard-stopped every candidate with `trailing characters at line 2 column 1`. The fail-safe held; it held against a format the analyzer documents. Stdout is read as JSONL now and the findings are unioned: a deny finding in any target is a reason to refuse, and the bin target's document is harmless to merge. A parse failure carries the first two hundred bytes of what was received, because the serde message told the reporter everything and would tell a user nothing. **The scratch copy.** It took only the entries of `src/` that are files, so `mod util;` with `src/util/mod.rs` produced a scratch crate that could not compile — and the gate reported `error: could not compile` against a crate whose own `cargo check` is clean, telling its author to fix errors they do not have or reinstall their toolchain, and never saying that what it compiled was not what they were looking at. It copies recursively now, carries `build.rs` (or whatever `package.build` names), and a tool error names the scratch directory so the next person can go and look. **The diagnosis filter.** rustc's primary diagnostics begin `error[E0583]:` — a code before the colon — so a filter on `error:` kept cargo's summary and dropped the line that names the failure. "See the errors above": the one above was the one the filter removed. Both forms are accepted, in the prune runner and in the compile executor, which had the tail-eight heuristic #19 removed next door for exactly this reason. **`--budget`.** `split_at(text.len() - 1)` on a trimmed-empty argument is `0usize - 1`, so `--budget ""` panicked at exit 101. Worse, `NaN` and `1e400` parsed: the guard is `elapsed >= budget`, which is false for every value against NaN and never true against infinity, so a value that looked accepted produced an unbounded sweep on real silicon — the one failure a budget exists to prevent. Rejected now, with the accepted forms named, and `min`/`hr` accepted alongside `m`/`h` because that is what people type. **`apply`.** Verification ran *after* the params.rs was printed, so "refusing to emit" arrived after the emission. It now decides first. On a Metal run — which has no convergence gate, deliberately — it refuses by name and points at `--no-verify` or `prune --cc `, instead of handing the `metal` sentinel to reconverge and reporting the correct answer to the wrong question as a regression. `--verify` is a real switch with a `--no-verify` counterpart, so a machine without the analyzer can emit, with the notice carried into the output beside the Metal one. And `Verdict` has a `Display`, so no user-facing message is a struct literal. **`results.json`.** A truncated file, an empty one, `null`, `[]`, a `results.v2` and a *directory* all read as "the box has not run yet": exit 0, nothing on stderr, a schema-valid report saying `unmeasured`. The run directory is the hand-off between two machines and those two conditions call for opposite actions — wait, or go and look. Only `NotFound` is `Ok(None)` now; everything else names the path and the cause, the way `verdicts.v1` already did fifteen lines away. Also: the reconverge pin moves to 0.5.0 at all seven sites, and scripts/check-pins.sh asserts they agree with no network — 2.0.0 moved four of six, which left the watcher measuring drift from a version nothing installs and #17 describing a pin two releases old. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- .claude/skills/termlens/SKILL.md | 452 +++++++++++++++++++++++ .github/workflows/pins.yml | 2 +- .github/workflows/prune.yml | 2 +- CONTRIBUTING.md | 2 +- Cargo.lock | 4 +- action/README.md | 2 +- action/action.yml | 2 +- crates/launchbound-bench/src/run.rs | 64 +++- crates/launchbound-build/src/compile.rs | 88 ++++- crates/launchbound-build/src/scratch.rs | 85 ++++- crates/launchbound-cli/src/main.rs | 269 ++++++++++++-- crates/launchbound-prune/src/decide.rs | 35 ++ crates/launchbound-prune/src/findings.rs | 115 ++++++ crates/launchbound-prune/src/runner.rs | 40 +- crates/launchbound-report/src/build.rs | 7 +- crates/launchbound-tui/Cargo.toml | 2 +- docs/LIMITATIONS.md | 2 +- justfile | 9 +- rust-toolchain.toml | 2 +- scripts/check-pins.sh | 94 +++++ 20 files changed, 1201 insertions(+), 77 deletions(-) create mode 100644 .claude/skills/termlens/SKILL.md create mode 100755 scripts/check-pins.sh diff --git a/.claude/skills/termlens/SKILL.md b/.claude/skills/termlens/SKILL.md new file mode 100644 index 0000000..311e12a --- /dev/null +++ b/.claude/skills/termlens/SKILL.md @@ -0,0 +1,452 @@ +--- +name: termlens +description: Write, fix or review headless terminal tests for a Rust CLI or TUI (ratatui, crossterm, cursive, plain println) with the termlens crate — spawn the real binary in a PTY, wait on the rendered screen without sleeping, snapshot it with insta, assert on cells and styles. Use whenever a test spawns a terminal program, whenever a terminal test is flaky, sleeps or times out, and whenever someone asks how to test a TUI end to end. +--- + +# Testing terminal programs with termlens + +Written against **termlens 0.9.0**. Every `rust` block below is a complete +integration test that is compiled against the crate in CI, so the API it +shows is the API that exists. The recipes spawn a binary called `myapp` +that draws a list with a `> ` highlight, a status line ending in +`Ready: j/k move, q quits`, and prints usage on `--help`; substitute your +application's own texts where the comments say so. + +## 1. What termlens is, and when to reach for it + +termlens spawns your **real binary** in a **real pseudo-terminal**, drains +its output on a reader thread through a VT emulator into an in-memory +**screen grid**, and lets a test wait on and assert against that grid — +Playwright for the terminal. Unix only (Linux, macOS). + +Use it for the things an in-process mock cannot see: + +- raw-mode and alternate-screen entry and exit, and whether the terminal is + left broken after a panic or a `q`; +- what the user actually sees — box drawing, colours, wide characters, + cursor position — after the bytes have been through a terminal; +- key, mouse, paste and resize handling as the terminal really encodes them + under the modes the application enabled; +- a CLI's `--help`, its exit code, its behaviour when the terminal is 40 + columns wide; +- anything printed outside the framework: a stray `println!`, a logger, a + panic message. + +Keep using `ratatui::backend::TestBackend` (or plain unit tests) for widget +layout and rendering logic: it is faster and finer-grained. termlens is the +second layer — a small number of end-to-end flows through the real binary. + +## 2. The model in sixty seconds + +```text +your binary ──PTY──▶ reader thread ──▶ VT emulator ──▶ Screen (immutable snapshot) + ▲ +your test ── send(Key) · click · paste · resize ──▶ PTY └── wait_until / snapshot_after / wait_exit +``` + +- **A `Screen` is one consistent instant.** Every accessor on it reads the + same snapshot; two `screen()` calls are two instants. +- **Every wait is deadline-bounded** (5 s by default) and **every failure + embeds the screen**, so a timeout in CI shows what the application was + displaying. There is no unbounded wait, on purpose. +- **Nothing sleeps.** The reader thread drains continuously; waits wake on + new output. This is what makes the tests fast when green and readable + when red. +- **Nothing is claimed that the emulator cannot see.** Terminal queries the + application sends (cursor position, device attributes, mode probes) are + answered truthfully or left unanswered and named in the next timeout. + +## 3. Golden rules — read these before writing a test + +1. **Never `thread::sleep`.** A sleep is either too short (flaky under CI + load) or too long (slow every run), and it hides *what* you were waiting + for. Wait on the screen instead: `wait_until(|s| …)` for a fact, + `snapshot_after(|s| …)` for a fact followed by a whole-screen snapshot, + `wait_stable(quiet)` for "the picture stopped changing". The single + sanctioned delay is `send_after(delay, key)`, which exists because `Esc` + followed immediately by another key is byte-identical to an `Alt` chord. + +2. **Snapshot only a settled screen.** `wait_until(pred)` guarantees the + bytes that made `pred` true were processed — and nothing more. A repaint + has no end marker, so the predicate can fire on a half-painted screen, + including half a row. Either wait on the **last** thing the application + paints, or use `snapshot_after`, which waits for the predicate and then + for the picture to hold still for 100 ms before handing you the screen. + +3. **One predicate per instant.** Everything you assert about one moment + goes into one closure: `wait_until(|s| s.contains("NORMAL") && + s.contains("Tasks 1/10"))`. `wait_until(a)` followed by + `assert!(screen().b)` is a race between two instants. + +4. **Spawn your own binary with `termlens::bin!("myapp")`.** It expands to + the builder chain every test otherwise repeats — `size(80, 24)`, + `env_clear()`, `timeout(5 s)`, `spawn(env!("CARGO_BIN_EXE_myapp"))` — and + a misspelled name is a **compile error**, not a spawn failure at run + time. Builder calls after the name override any default: + `bin!("myapp", size(120, 40), env("NO_COLOR", "1"))`. + +5. **Geometry is `(cols, rows)`, from 2 to 1000 per axis.** `size(0, 0)` and + `size(1, 1)` are refused with `Error::Size`: one column panics the + emulator on a double-width character and one row panics it on a line + that wraps. Grids past 1000 per axis are refused because every snapshot + costs one entry per cell. 80x24 is the default and is what you want. + +6. **Two coordinate orders exist; do not mix them.** Everything that + addresses a cell is **row-first**: `find` → `(row, col)`, `cell(row, + col)`, `row_text(row)`, `cursor()` → `(row, col, visible)`. Everything + that speaks of terminal geometry or a pointer is **column-first**: + `size()` → `(cols, rows)`, `resize(cols, rows)`, `click(col, row)`, + `scroll(col, row, …)`, `drag(button, (col, row), (col, row))`. Never + pass a `find` result straight into `drag` — the tuple types match and + the axes do not. + +7. **Always finish the process.** Send the quit key, `wait_exit()?` and + assert on the `ExitStatus` (`success()`, `code()`, `signal()`), then + assert `!t.screen().alternate_screen()` so an application that leaves + the user's terminal in the alternate screen fails the test. `Drop` kills + and reaps whatever is left, so a failing test never leaks a process. + +8. **`wait_frame` only works for applications that emit DEC 2026 + synchronized updates.** Stock ratatui 0.30 with crossterm does **not** + (measured: `repaints()` stays 0), so `wait_frame` times out against it + with a message saying exactly that. Default to `snapshot_after`. Use + `wait_frame` only if the application brackets its repaints in + `BeginSynchronizedUpdate` / `EndSynchronizedUpdate`. + +9. **Return `termlens::Result<()>` from the test and use `?`.** The + `Display` of every error carries the screen, so a failing wait prints + the grid the application was showing instead of `called unwrap() on Err`. + +10. **Snapshot the `Screen`, not its text.** `insta::assert_snapshot!(screen)` + records the header (`size: 80x24 cursor: 3,5` or `cursor: hidden`) and + the grid; `screen.with_styles()` adds a `styles:` block that catches a + colour regression. `.text()` drops the header and `format!("{:?}")` is + the same as `Display`. Review changes with `cargo insta review`; never + blind-accept with `INSTA_UPDATE=always`. + +11. **The environment is hermetic by default — set what the app reads.** + Under `env_clear()` (which `bin!` applies) the child sees only + `TERM=xterm-256color`, `SHELL=/bin/sh` and what you set with `env(…)`. + No `HOME`, no `LANG`, no `COLORTERM`, no `NO_COLOR`, no `PATH` — so a + bare program name cannot resolve (use an absolute path or `bin!`), and + an application that checks `NO_COLOR` or `COLORTERM` needs them set + explicitly for the case under test. + +12. **The grid is Unicode-aware; think in cells.** A double-width character + (CJK, most emoji) occupies two cells: the leading one `is_wide()`, the + next `is_wide_continuation()`. `find` reports real terminal columns. + `contains` and `find` fold both sides to NFC and search the **visible + screen only** — text that scrolled off is in `full_text()`, and a line + that wrapped is two rows, so a needle spanning the wrap is not found. + +## 4. Setup + +```toml +[dev-dependencies] +termlens = "0.9" +insta = "1" # for the snapshot recipes; termlens also re-exports it as `termlens::insta` +``` + +- Put the tests in `tests/` **of the package that owns the `[[bin]]`**: + Cargo sets `CARGO_BIN_EXE_` only there, and `bin!` needs it at + compile time. For a binary in a sibling crate, build it and pass the path + to `Terminal::builder().spawn(path)` instead. +- The binary is built by `cargo test` before the tests run. Tests run in + parallel by default; each spawns its own PTY, which is fine. +- Gate the test file with `#![cfg(unix)]` if the crate must also build on + Windows. +- `add --features decode` only if you assert on the pixels of inline images. + +## 5. Recipes + +### Recipe A — hermetic CLI snapshot (`myapp --help`) + +```rust +use termlens::Terminal; + +#[test] +fn help_renders_and_exits_zero() -> termlens::Result<()> { + // 80x24, cleared environment, 5 s deadline, compile-time-checked path. + let mut t = termlens::bin!("myapp", args(["--help"]))?; + + // Wait for the LAST line of the help text before waiting for exit. A + // program that prints and exits within a millisecond can, rarely and + // under load on macOS, lose its tail to PTY teardown; waiting on the + // tail first turns that into a loud timeout instead of a truncated + // snapshot that passes. + t.wait_until(|s| s.contains("q quit"))?; // your help's last line + let status = t.wait_exit()?; + assert!(status.success(), "exit status: {status}"); + + // The header records the size and cursor; the body is the grid. + insta::assert_snapshot!(t.screen()); + Ok(()) +} + +#[test] +fn a_bad_flag_is_reported_with_an_exit_code() -> termlens::Result<()> { + let mut t = Terminal::builder() + .size(80, 24) + .env_clear() + .timeout(std::time::Duration::from_secs(5)) + .args(["--definitely-not-a-flag"]) + .spawn(env!("CARGO_BIN_EXE_myapp"))?; + // stderr lands on the same screen as stdout: it is one terminal. + t.wait_until(|s| s.contains("unexpected argument"))?; + let status = t.wait_exit()?; + // Assert what your CLI promises; clap exits 2 on a usage error. + assert_eq!(status.code(), Some(2), "status: {status}"); + assert_eq!(status.signal(), None, "exited, not killed: {status}"); + Ok(()) +} +``` + +### Recipe B — interactive ratatui navigation and keystrokes + +```rust +use termlens::Key; + +#[test] +fn moving_the_highlight_and_quitting_cleanly() -> termlens::Result<()> { + let mut t = termlens::bin!("myapp")?; + + // Predicate, then a 100 ms settle, then the screen: the safe sequence + // for a whole-screen snapshot. Name the last thing the app paints. + let first = t.snapshot_after(|s| s.contains("Ready"))?; + assert!(first.alternate_screen(), "a TUI should be on the alternate screen:\n{first}"); + assert!(first.contains("> Alpha"), "{first}"); + insta::assert_snapshot!("initial_frame", first); + + // Send a key, then wait on what the key CHANGES — not on text that was + // already true before the key, or the wait returns the old screen. + t.send(Key::Char('j'))?; + let moved = t.snapshot_after(|s| s.contains("> Beta"))?; + assert!(!moved.contains("> Alpha"), "{moved}"); + + // Arrow keys and chords encode as the terminal would (DECCKM-aware). + t.send(Key::Down)?; + t.wait_until(|s| s.contains("> Gamma"))?; + t.send(Key::Up)?; + t.wait_until(|s| s.contains("> Beta"))?; + + // Finish: quit, assert the exit, and assert the terminal was restored. + t.send(Key::Char('q'))?; + let status = t.wait_exit()?; + assert!(status.success(), "status: {status}"); + assert!(!t.screen().alternate_screen(), "the app left the terminal in the alternate screen"); + Ok(()) +} +``` + +If the flow needs `Esc` followed by another key, use the one sanctioned +delay — `t.send_after(Duration::from_millis(20), Key::Char('j'))?` — so the +application's read boundary falls between the two writes and it sees two +presses rather than one `Alt-j` chord. + +### Recipe C — overriding the defaults (size, environment, deadline) + +```rust +use std::time::Duration; +use termlens::Color; + +#[test] +fn honours_no_color_at_a_custom_size() -> termlens::Result<()> { + // Builder calls after the name override bin!'s defaults; the rest stay. + let mut t = termlens::bin!( + "myapp", + size(100, 30), // (cols, rows) + env("NO_COLOR", "1"), // the app reads this at startup + timeout(Duration::from_secs(10)), // every wait's default deadline + )?; + let s = t.snapshot_after(|s| s.contains("Ready"))?; + + assert_eq!(s.size(), (100, 30), "{s}"); + + // With NO_COLOR the title is drawn in the default colour, not cyan. + let (row, col) = s.find("myapp").expect("title is on screen"); + let title = s.cell(row, col).expect("in range"); + assert_eq!(title.style().fg, Color::Default, "{}", s.with_styles()); + assert!(!title.style().bold); + + // One slow step gets its own deadline instead of a slower suite. + t.wait_until_for(|s| s.contains("Ready"), Duration::from_secs(30))?; + Ok(()) +} +``` + +### Recipe D — targeted screen and style assertions + +```rust +use termlens::Color; + +#[test] +fn cells_styles_and_wide_characters() -> termlens::Result<()> { + let mut t = termlens::bin!("myapp")?; + let s = t.snapshot_after(|s| s.contains("Ready"))?; + + // Text: visible screen, NFC-folded, trailing padding never matched. + assert!(s.contains("Alpha") && s.contains("Beta"), "{s}"); + assert_eq!(s.find("Ready"), Some((23, 0)), "status line sits on the last row: {s}"); + + // Cells and styles: the highlighted row is drawn in reverse video. + let (row, col) = s.find("> Alpha").expect("highlight"); + let cell = s.cell(row, col).expect("in range"); + assert!(cell.style().reverse, "{}", s.with_styles()); + // A coloured, bold title: ratatui's Cyan is ANSI colour 6. + let (trow, tcol) = s.find("myapp").expect("title"); + let title = s.cell(trow, tcol).unwrap().style(); + assert_eq!((title.fg, title.bold), (Color::Indexed(6), true)); + + // Find a cell by a property of the cell rather than by its text. + assert_eq!(s.find_by(|c| c.style().reverse), Some((row, col))); + + // Wide characters: one glyph, two cells, real columns reported. + let (crow, ccol) = s.find("東京").expect("CJK item"); + assert!(s.cell(crow, ccol).unwrap().is_wide()); + assert!(s.cell(crow, ccol + 1).unwrap().is_wide_continuation()); + assert_eq!(s.row_text(crow).trim_matches(['│', ' ']), "東京"); + + // Regions and the cursor. rect_text is (cols, rows), like size(). + let list_pane = s.rect_text(0..20, 0..6); + assert!(list_pane.contains("Gamma"), "{list_pane}"); + let (_, _, visible) = s.cursor(); + assert!(!visible, "a list view hides the cursor: {s}"); + + t.send(termlens::Key::Char('q'))?; + assert!(t.wait_exit()?.success()); + Ok(()) +} +``` + +## 6. Reading a failure + +Every error's `Display` ends with the screen, under a header that says +which screen it is (`--- screen at timeout ---`, `--- final screen ---`). +Read the first line for the cause: + +| First line says | Meaning | Do | +|---|---|---| +| `timed out after 5s while waiting for the screen predicate to hold` | the predicate never became true | look at the embedded grid; the text is usually spelled differently, on another row, or scrolled off (the note says how many rows scrolled) | +| `… note: N rows have scrolled off the top` | the text went into history | assert with `full_text()` / `scrollback_text()` | +| `… note: the application queried the terminal (^[[?u …) and received no answer` | the app is blocked on a probe termlens deliberately does not answer | the app needs a fallback; see the termlens README's Known limitations | +| `terminal closed (EOF) while waiting for …` | the app exited before the predicate held | check `wait_exit()` first, or the app crashed — the final screen shows why | +| `the application never emitted a DEC 2026 synchronized update` | `wait_frame` against an app without synchronized output | use `snapshot_after` / `wait_until` (rule 8) | +| `input not receivable: the application has not enabled mouse tracking` | `click`/`drag`/`scroll` before the app enabled the mouse | `wait_until(|s| s.mouse_mode() != MouseMode::None)` first | +| `input not receivable: mouse at (50, 2) is outside the 20x5 grid` | coordinates swapped or out of range | rule 6 | +| `failed to spawn \`sh\`: \`sh\` is a bare program name and env_clear() removed PATH` | bare program name under `env_clear` | absolute path, `bin!`, or `.env("PATH", …)` | +| `invalid terminal size: a terminal needs at least 2 columns and 2 rows` | geometry below 2x2 (past 1000 has its own message) | rule 5 | +| `the terminal emulator failed and the screen stopped advancing` (`Error::Emulator`) | a bug in the emulation, not in your app | report it to termlens with the detail it names | + +## 7. API cheat sheet + +**Spawn** — `termlens::bin!("name" $(, method(args))*)` or +`Terminal::builder()`: + +| Builder method | Meaning | +|---|---| +| `.size(cols, rows)` | 2..=1000 each; default 80x24 | +| `.timeout(Duration)` | default deadline for every wait (5 s) | +| `.arg(a)` / `.args([..])` | program arguments | +| `.env(k, v)` / `.envs([..])` / `.env_clear()` | environment; `env_clear` keeps `TERM` and `SHELL` pinned and drops the rest | +| `.current_dir(path)` | default: the test process's directory | +| `.scrollback(rows)` | history retained (default 1000, text only) | +| `.spawn(program) -> Result` | program is a path or a name on `PATH` | + +**Wait** (all return `termlens::Result`, all embed the screen on failure, all have a `_for(…, timeout)` twin): + +| Method | Returns | Use for | +|---|---|---| +| `wait_until(\|s\| bool)` | `()` | a fact about the screen | +| `snapshot_after(\|s\| bool)` | `Screen` | a fact, then a settled whole-screen snapshot | +| `wait_stable(quiet)` | `Screen` | the picture unchanged for `quiet`; bells and no-op repaints do not reset it | +| `wait_idle(quiet)` | `()` | no *bytes* for `quiet` — a weaker, older sibling of `wait_stable` | +| `wait_frame(\|s\| bool)` | `Screen` | complete DEC 2026 frames only (rule 8) | +| `wait_exit()` | `ExitStatus` | the child's exit; `success()`, `code() -> Option`, `signal() -> Option<&str>` | + +**Drive**: `send(Key)`, `send_str("text")` (no Enter — send `Key::Enter` +yourself; `"\n"` would send LF, not CR), `paste("text")` (bracketed if the +app enabled it), `send_after(delay, Key)`, `click(col, row)`, +`click_with(MouseButton::Right, col, row)`, `drag(MouseButton::Left, (c, r), +(c, r))`, `scroll(col, row, Scroll::Down)`, `resize(cols, rows)`, +`focus_in()` / `focus_out()`, `signal(Signal::Term)` (Unix), `pid()`. + +**Keys**: `Key::Char('j')`, `Enter`, `Esc`, `Tab`, `BackTab`, `Backspace`, +`Delete`, `Insert`, `Up`/`Down`/`Left`/`Right`, `Home`/`End`, +`PageUp`/`PageDown`, `F(1..=12)`, `Ctrl('c')`, `Alt('x')`; chords on any key: +`Key::Right.ctrl()`, `Key::F(5).ctrl().shift()`. + +**Screen** (immutable; every accessor reads one instant): + +| Accessor | Returns | +|---|---| +| `contains(&str)` / `find(&str)` | `bool` / `Option<(row, col)>` — visible grid, NFC-folded | +| `find_by(\|&Cell\| bool)` | `Option<(row, col)>` | +| `cell(row, col)` | `Option<&Cell>`: `contents()`, `style()`, `is_wide()`, `is_wide_continuation()` | +| `row_text(row)` / `text()` / `rect_text(cols, rows)` | `String` | +| `full_text()` / `scrollback_text()` / `scrollback_rows()` | history + screen / history / count | +| `size()` / `cols()` / `rows()` | `(cols, rows)` | +| `cursor()` | `(row, col, visible)`; `cursor_shape()`, `cursor_blink()` | +| `alternate_screen()`, `bracketed_paste()`, `application_cursor()`, `focus_events()` | mode flags | +| `mouse_mode()` / `mouse_modes()` | reporting protocol / the set the app enabled | +| `title()`, `clipboard()`, `links()`, `bells()`, `repaints()`, `graphics()` | out-of-band state | +| `with_styles()` | `Display` with a `styles:` block; snapshot this to catch colour regressions | + +**Style** (`Copy`, public fields): `fg`, `bg` (`Color::Default` / +`Color::Indexed(u8)` / `Color::Rgb(u8, u8, u8)`), `bold`, `dim`, `italic`, +`underline`, `reverse`, `blink`, `conceal`, `strikethrough`. Overline and +double underline are not modelled. + +**Errors** (`termlens::Error`, `#[non_exhaustive]`): `Timeout { waiting_for, +timeout, screen }`, `Eof { waiting_for, screen }`, `Spawn { command, reason }`, +`Size(String)`, `Input(String)`, `Write { what, screen }`, `Emulator { +detail, screen }`, `Pty(String)`, `Io(std::io::Error)`. `err.screen()` returns +the embedded screen when there is one. + +## 8. Pitfalls an agent falls into, and the fix + +| You are about to write | Write instead | +|---|---| +| `std::thread::sleep(Duration::from_millis(500)); let s = t.screen();` | `let s = t.snapshot_after(\|s\| s.contains("…"))?;` | +| `t.wait_until(\|s\| s.contains("title"))?; insta::assert_snapshot!(t.screen());` | `let s = t.snapshot_after(\|s\| s.contains("…last painted…"))?; insta::assert_snapshot!(s);` | +| `t.wait_until(a)?; assert!(t.screen().b);` | `t.wait_until(\|s\| a(s) && b(s))?;` | +| `.size(0, 0)` / `.size(1, 1)` | leave the 80x24 default, or `.size(cols, rows)` with both in 2..=1000 | +| `Terminal::builder().spawn("myapp")` | `termlens::bin!("myapp")?` — a name on `PATH` is not your binary, and under `env_clear` there is no `PATH` | +| `t.click(row, col)` / `t.drag(b, s.find("x").unwrap(), …)` | `t.click(col, row)`; destructure the `find` result and swap | +| `t.send_str("quit\n")` | `t.send_str("quit")?; t.send(Key::Enter)?;` | +| `t.wait_frame(…)` against a ratatui app | `t.snapshot_after(…)` unless the app emits synchronized updates | +| `t.click(3, 4)?` as the first thing after spawn | `t.wait_until(\|s\| s.mouse_mode() != MouseMode::None)?;` first | +| `.timeout(Duration::from_secs(60))` to stop a flake | find the race (rules 2 and 3); use `_for` on the one slow step | +| `insta::assert_snapshot!(t.screen().text())` | `insta::assert_snapshot!(t.screen())` or `.with_styles()` | +| `assert!(t.screen().contains("done"))` after the app printed a lot | `t.screen().full_text().contains("done")` — it scrolled | +| `.unwrap()` everywhere in a `fn test()` | `-> termlens::Result<()>` and `?`, so the failure prints the screen | +| a test that never quits the app | send the quit key, `wait_exit()?`, assert `!alternate_screen()` | +| `INSTA_UPDATE=always cargo test` | `cargo insta review`, and read every diff | + +## 9. Pairing with insta + +- `insta::assert_snapshot!(screen)` — text grid with header. Stable across + runs as long as the application draws nothing volatile. +- `insta::assert_snapshot!(screen.with_styles())` — adds `styles:` runs like + `0: 1-5 fg=6 bold`; use it where a colour or a highlight is the point. +- `insta::assert_snapshot!("name", screen)` — several snapshots in one test. +- Inline snapshots work: `insta::assert_snapshot!(screen, @"")`, then + `cargo insta review` fills the literal. +- `termlens::assert_screen_snapshot!(screen)` is the same call through the + `insta` termlens re-exports, for crates that do not want their own `insta` + dev-dependency. +- Volatile content (a clock, a PID, a spinner) breaks whole-screen + snapshots. insta's text filters are not grid-aware — a shorter replacement + shifts every column after it — so prefer asserting the stable region with + `rect_text(cols, rows)` and the volatile field with `contains`/`find`, + and snapshot the whole screen only when nothing on it moves. +- Snapshot files live in `tests/snapshots/`; commit them. Review every + change with `cargo insta review`; a diff you cannot explain is a bug. + +## 10. A checklist before you finish + +- [ ] No `sleep` anywhere; every wait names what it waits for. +- [ ] Every whole-screen snapshot comes from `snapshot_after` or a wait on the last-painted text. +- [ ] Each test quits the app, asserts the exit status, and asserts `!alternate_screen()`. +- [ ] Coordinates: `(row, col)` from `find`/`cell`, `(col, row)` into `click`/`size`. +- [ ] Environment set explicitly for anything the app reads; `bin!` used for own binaries. +- [ ] Tests return `termlens::Result<()>`; snapshots reviewed with `cargo insta review`. diff --git a/.github/workflows/pins.yml b/.github/workflows/pins.yml index 574b517..4d9a359 100644 --- a/.github/workflows/pins.yml +++ b/.github/workflows/pins.yml @@ -21,7 +21,7 @@ permissions: env: CUDA_OXIDE_PIN: 50d07314eb8b7d5ec821ba02b0048a753c20dd4e CUDA_OXIDE_REPO: NVlabs/cuda-oxide - RECONVERGE_PIN: v0.1.11 + RECONVERGE_PIN: v0.5.0 RECONVERGE_REPO: vyncint/reconverge jobs: diff --git a/.github/workflows/prune.yml b/.github/workflows/prune.yml index 31695ca..39e3a11 100644 --- a/.github/workflows/prune.yml +++ b/.github/workflows/prune.yml @@ -17,7 +17,7 @@ permissions: # pins.yml issue body lists every site. env: PINNED_TOOLCHAIN: nightly-2026-04-03 - RECONVERGE_VERSION: "0.4.0" + RECONVERGE_VERSION: "0.5.0" jobs: gate: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c1d656c..e13947f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -171,7 +171,7 @@ never mixed with a behaviour change, and re-runs the affected stage gates. The `pins.yml` workflow reports upstream movement by opening an issue; it never bumps anything. Its weekly cron is commented out, so upstream movement is noticed when you dispatch it. Current pins: nightly-2026-04-03, -cuda-oxide 50d07314, reconverge 0.1.11 (installed from crates.io). +cuda-oxide 50d07314, reconverge 0.5.0 (installed from crates.io). ## 10. License diff --git a/Cargo.lock b/Cargo.lock index d765f17..7246cf1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2472,9 +2472,9 @@ dependencies = [ [[package]] name = "termlens" -version = "0.6.1" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b76566a0782145bfc8034bfb806beda713e750f6f902d74eb6a83d6bc4a52675" +checksum = "0fca989672430e13284b48504c44b499d88f5b39cb36b0e8dc5b45f06df50b09" dependencies = [ "libc", "portable-pty", diff --git a/action/README.md b/action/README.md index 53a3c45..1829e6c 100644 --- a/action/README.md +++ b/action/README.md @@ -63,7 +63,7 @@ the gate specializes per candidate. | `cc` | — | target compute capability, e.g. `"8.6"` (required; verdicts do not transfer across parts) | | `fail-on` | `tool-error` | `never`, `refused`, or `tool-error` | | `version` | `latest` | launchbound-cli release to install | -| `reconverge-version` | `0.4.0` | reconverge release from crates.io — moves in lockstep with `toolchain` | +| `reconverge-version` | `0.5.0` | reconverge release from crates.io — moves in lockstep with `toolchain` | | `toolchain` | `nightly-2026-04-03` | the nightly that built that reconverge | | `summary` | `"true"` | write the verdict table to the job summary | diff --git a/action/action.yml b/action/action.yml index c80b1d7..ee7114c 100644 --- a/action/action.yml +++ b/action/action.yml @@ -51,7 +51,7 @@ inputs: reconverge-driver). Must be built by the toolchain below — the pins move together. required: false - default: "0.4.0" + default: "0.5.0" toolchain: description: >- The pinned nightly that matches reconverge-version; the pair moves diff --git a/crates/launchbound-bench/src/run.rs b/crates/launchbound-bench/src/run.rs index b760641..44d469e 100644 --- a/crates/launchbound-bench/src/run.rs +++ b/crates/launchbound-bench/src/run.rs @@ -89,15 +89,19 @@ pub fn run_plan( )); } - let mut results = match Results::load(results_path) { - Some(existing) if existing.schema == "results.v1" => { + // A checkpoint that cannot be read is not "start over": resuming would + // silently discard measurements that cost GPU time, and the file is the + // only record of them. Say what is wrong and stop. + let existing = Results::load(results_path)?; + let mut results = match existing { + Some(existing) => { progress(&format!( "resuming: {} candidates already measured", existing.candidates.len() )); existing } - _ => Results { + None => Results { schema: "results.v1".into(), kernel: plan.kernel.clone(), entry: plan.entry.clone(), @@ -364,9 +368,57 @@ fn deterministic_u32(len: u64, modulo: u64) -> Vec { } impl Results { - pub fn load(path: &Path) -> Option { - let text = std::fs::read_to_string(path).ok()?; - serde_json::from_str(&text).ok() + /// Read `results.v1` from a run directory. + /// + /// `Ok(None)` means one thing only: **the file is not there**, so the + /// measurement box has not run yet. Everything else is an error. + /// + /// This used to be `read_to_string(path).ok()?` then `from_str().ok()`, + /// so a truncated file, an empty one, `null`, `[]`, a `results.v2` from + /// a newer runner and a *directory* named `results.json` all collapsed + /// into that same `None` — and `report` rendered "nothing measured yet", + /// exit 0, nothing on stderr, with a JSON report that validated. The run + /// directory is the hand-off between two machines, and the two + /// conditions call for opposite actions: wait, or go and look. Nothing + /// told them apart. + /// + /// `verdicts.v1` two lines away in the report builder already checked + /// its schema tag by name; this is the same check, so a future + /// `results.v2` is refused by name rather than read as nothing. + /// + /// # Errors + /// + /// Any I/O error that is not `NotFound`, any parse failure, and any + /// document whose `schema` is not `results.v1` — each naming the path. + pub fn load(path: &Path) -> Result, String> { + let text = match std::fs::read_to_string(path) { + Ok(text) => text, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(e) => return Err(format!("{}: {e}", path.display())), + }; + // The tag first, so a wrong-schema document is named as one rather + // than as whatever field happens to be missing from it. + let value: serde_json::Value = serde_json::from_str(&text) + .map_err(|e| format!("{}: not JSON: {e}", path.display()))?; + let declared = value.get("schema").and_then(|s| s.as_str()); + match declared { + Some("results.v1") => {} + Some(other) => { + return Err(format!( + "{}: unsupported results schema `{other}` (expected `results.v1`)", + path.display() + )); + } + None => { + return Err(format!( + "{}: not a results.v1 document (no `schema` field)", + path.display() + )); + } + } + serde_json::from_str(&text) + .map(Some) + .map_err(|e| format!("{}: not a results.v1 document: {e}", path.display())) } /// Atomic checkpoint: write to a temp file, then rename. diff --git a/crates/launchbound-build/src/compile.rs b/crates/launchbound-build/src/compile.rs index a2592e7..d7c5e8e 100644 --- a/crates/launchbound-build/src/compile.rs +++ b/crates/launchbound-build/src/compile.rs @@ -124,12 +124,17 @@ impl Compiler { let output = self.executor.run_script(&script)?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); - let tail: Vec<&str> = stderr.lines().rev().take(8).collect(); - let tail: Vec<&str> = tail.into_iter().rev().collect(); + // A missing subcommand is not a compile failure, and cargo's own + // help for it sends the reader to `cargo search cargo-oxide` — + // a package that is not on crates.io. This tool knows the pin + // and can answer properly. + if is_missing_subcommand(&stderr) { + return Err(BuildError::Compile(missing_oxide_message())); + } return Err(BuildError::Compile(format!( - "cargo oxide inspect {stem} failed (exit {:?}):\n{}", - output.status.code(), - tail.join("\n") + "cargo oxide inspect {stem} failed (exit {}):\n{}", + exit_label(output.status.code()), + diagnosis(&stderr) ))); } let artifact = scratch.join(format!("{stem}.ptx")); @@ -142,6 +147,79 @@ impl Compiler { } } +/// Did cargo report that it has no `oxide` subcommand? +fn is_missing_subcommand(stderr: &str) -> bool { + stderr.contains("no such command: `oxide`") || stderr.contains("no such subcommand: `oxide`") +} + +/// The pinned cuda-oxide commit, kept beside the pin sites the policy names. +/// +/// Named here so a pin bump moves the message with it — `check-pins.sh` +/// asserts this constant against `rust-toolchain.toml` and the workflows, +/// because drift between recorded pins is the failure this repository keeps +/// hitting. +pub const CUDA_OXIDE_PIN: &str = "50d07314eb8b7d5ec821ba02b0048a753c20dd4e"; + +/// What to say when `cargo oxide` is not installed. +fn missing_oxide_message() -> String { + format!( + "`cargo oxide` is not installed — the gate can compile nothing \ + without it\n\n \ + cuda-oxide is pinned to {pin} and is NOT published to crates.io, so \ + `cargo search cargo-oxide` (which cargo suggests) finds nothing. \ + Check it out beside this repository, as CI does, and install its \ + cargo subcommand:\n\n \ + git clone https://github.com/NVlabs/cuda-oxide ../cuda-oxide\n \ + git -C ../cuda-oxide checkout {short}\n \ + cargo install --path ../cuda-oxide/crates/cargo-oxide\n\n \ + `launchbound prune` needs none of this — it is the whole pipeline a \ + laptop can run.", + pin = CUDA_OXIDE_PIN, + short = &CUDA_OXIDE_PIN[..8], + ) +} + +/// An exit code, or what to say when there was not one. +/// +/// `{:?}` on an `Option` printed `exit Some(101)` at a user. +fn exit_label(code: Option) -> String { + match code { + Some(code) => code.to_string(), + // Unix only, but the string is honest anywhere. + None => "killed by a signal".to_string(), + } +} + +/// The lines of a failing compiler's stderr that say what went wrong. +/// +/// The same reasoning as `launchbound_prune::diagnosis`, and the same bug: +/// the tail of a failing compile is `error: could not compile … due to N +/// previous errors`, and the N errors are above the cut. rustc's primary +/// diagnostics carry a code — `error[E0583]:` — so both marker forms are +/// accepted, and the fallback is the head rather than the tail. +fn diagnosis(stderr: &str) -> String { + let marked: Vec<&str> = stderr + .lines() + .filter(|line| { + let line = line.trim_start(); + line.starts_with("error:") || line.starts_with("error[") + }) + .collect(); + if !marked.is_empty() { + return marked.join("\n"); + } + let head: Vec<&str> = stderr + .lines() + .filter(|l| !l.trim().is_empty()) + .take(8) + .collect(); + if head.is_empty() { + "(no output on stderr)".to_string() + } else { + head.join("\n") + } +} + /// Extract the PTX document (from the NVPTX header or `.version`) out of /// mixed build output. pub fn extract_ptx(stdout: &str) -> Option { diff --git a/crates/launchbound-build/src/scratch.rs b/crates/launchbound-build/src/scratch.rs index a18351f..5c17a9b 100644 --- a/crates/launchbound-build/src/scratch.rs +++ b/crates/launchbound-build/src/scratch.rs @@ -28,17 +28,90 @@ pub fn prepare_scratch(spec: &KernelSpec, scratch_root: &Path) -> Result) -> Result<(), BuildError> { + let canonical = from.canonicalize().unwrap_or_else(|_| from.to_path_buf()); + if visited.contains(&canonical) { + return Ok(()); + } + visited.push(canonical); + + std::fs::create_dir_all(to).map_err(|e| BuildError::Scratch(e.to_string()))?; + for entry in std::fs::read_dir(from).map_err(|e| BuildError::Scratch(e.to_string()))? { + let entry = entry.map_err(|e| BuildError::Scratch(e.to_string()))?; + let name = entry.file_name(); + if name == "target" { + continue; + } + let path = entry.path(); + let target = to.join(&name); + if path.is_dir() { + copy_tree(&path, &target, visited)?; + } else { + std::fs::copy(&path, &target).map_err(|e| BuildError::Scratch(e.to_string()))?; + } + } + Ok(()) +} + +/// The build script this manifest declares, or `build.rs`. +/// +/// Read from the text rather than from `cargo metadata`: `prepare_scratch` +/// runs once per candidate and a metadata call per candidate is not worth +/// the one field. `build = false` disables the script, and returning a path +/// that does not exist is harmless — the caller only copies what is there. +fn build_script_path(manifest: &str) -> PathBuf { + for line in manifest.lines() { + let line = line.trim(); + let Some(value) = line.strip_prefix("build") else { + continue; + }; + let Some(value) = value.trim_start().strip_prefix('=') else { + continue; + }; + let value = value.trim(); + if value == "false" { + return PathBuf::from("(disabled)"); + } + if let Some(quoted) = value.strip_prefix('"').and_then(|v| v.split('"').next()) { + return PathBuf::from(quoted); + } + } + PathBuf::from("build.rs") +} + /// Default scratch root for a kernel: inside its own target dir /// (gitignored). Absolute, because compile executors may run with a /// different working directory (or inside a container). diff --git a/crates/launchbound-cli/src/main.rs b/crates/launchbound-cli/src/main.rs index 3dd3b7e..90d1c0c 100644 --- a/crates/launchbound-cli/src/main.rs +++ b/crates/launchbound-cli/src/main.rs @@ -155,24 +155,80 @@ enum Command { /// Corpus root used to resolve kernel names. #[arg(long, default_value = "corpus")] corpus: PathBuf, - /// Re-verify the emitted specialization with cargo reconverge. - #[arg(long, default_value_t = true)] + /// Re-verify the emitted specialization through the gate. On by + /// default; `--no-verify` emits without it, and says so in the + /// output. Verification shells out to `cargo reconverge`, so a + /// machine that does not have the analyzer and the pinned toolchain + /// needs `--no-verify` to emit at all. + #[arg(long, action = clap::ArgAction::Set, default_value_t = true, + num_args = 0..=1, require_equals = false, default_missing_value = "true")] verify: bool, + /// Emit without re-verifying through the gate. The output carries a + /// notice saying nothing was checked. + #[arg(long, conflicts_with = "verify")] + no_verify: bool, /// Directory containing the cargo-reconverge binary. #[arg(long)] reconverge_dir: Option, }, } -fn parse_budget(text: &str) -> anyhow::Result { +/// A wall-clock budget in seconds, as typed at the command line. +/// +/// Parsed rather than indexed. `split_at(text.len() - 1)` on a trimmed-empty +/// argument is `0usize - 1`, so `--budget ""` and `--budget " "` panicked at +/// exit 101 inside `core::str`. +/// +/// The important half is what is *rejected*. `"NaN".parse::()` succeeds +/// and so does `1e400` (as `inf`), and the guard that stops a sweep is +/// `elapsed >= budget` — false for every value against NaN, and never true +/// against infinity. So a value that looked accepted produced an +/// **unbounded** measured sweep on real silicon, which is the one failure a +/// budget exists to prevent. `is_finite()` and `> 0.0` close NaN, infinity +/// and negatives together. +/// +/// `--budget 0` stays valid and is not the same thing: the guard fires +/// immediately and the run reports `budget exhausted after 0.0s`. +fn parse_budget(text: &str) -> Result { + const ACCEPTED: &str = "expected a positive number of seconds, or a number with a \ + unit: `s` seconds, `m` minutes, `h` hours (e.g. `90s`, \ + `30m`, `1h`, or `45` for seconds)"; + let text = text.trim(); - let (digits, unit) = text.split_at(text.len() - 1); - match unit { - "s" => Ok(digits.parse::()?), - "m" => Ok(digits.parse::()? * 60.0), - "h" => Ok(digits.parse::()? * 3600.0), - _ => Ok(text.parse::()?), + if text.is_empty() { + return Err(format!("--budget needs a value — {ACCEPTED}")); + } + + // Longest suffix first, so `min` is not read as `m` with `i` left over. + let (digits, multiplier) = [ + ("hr", 3600.0), + ("h", 3600.0), + ("min", 60.0), + ("m", 60.0), + ("sec", 1.0), + ("s", 1.0), + ] + .into_iter() + .find_map(|(unit, multiplier)| text.strip_suffix(unit).map(|d| (d.trim(), multiplier))) + .unwrap_or((text, 1.0)); + + if digits.is_empty() { + return Err(format!("`{text}` is a unit with no number — {ACCEPTED}")); + } + let value: f64 = digits + .parse() + .map_err(|_| format!("`{text}` is not a valid --budget — {ACCEPTED}"))?; + if !value.is_finite() { + return Err(format!( + "`{text}` is not a finite budget — a sweep bounded by NaN or infinity is \ + not bounded at all, which is the opposite of what --budget is for. \ + {ACCEPTED}" + )); + } + if value < 0.0 { + return Err(format!("`{text}` is negative — {ACCEPTED}")); } + Ok(value * multiplier) } /// A compute capability, as typed at the command line. @@ -275,8 +331,9 @@ fn run() -> anyhow::Result { kernel, corpus, verify, + no_verify, reconverge_dir, - } => cmd_apply(&run, &kernel, &corpus, verify, reconverge_dir), + } => cmd_apply(&run, &kernel, &corpus, verify && !no_verify, reconverge_dir), Command::Tune { kernel, corpus, @@ -330,6 +387,33 @@ fn cmd_apply( ) })?; + // Decide about verification BEFORE anything reaches stdout. + // + // This ran after the `params.rs` was printed, so "refusing to emit" + // arrived *after* the emission and a reader who had piped stdout to a + // file had the file. A refusal has to be a refusal. + // + // The Metal path has no convergence gate at all — deliberately, and + // `report` says so on every render — so a Metal run records + // `gate_cc: "metal"`, a sentinel rather than a compute capability. + // Handing that to reconverge got the correct answer to the wrong + // question ("`metal` is not a compute capability") dressed as a + // regression ("no longer passes the gate"). Nothing regressed: on this + // path the gate never ran and cannot. + let ungated = !report.gate_cc.contains('.'); + if verify && ungated { + anyhow::bail!( + "this run was measured on the {} path, which has no convergence gate \ + (docs/SAFETY.md) — there is no gate verdict to re-verify, and \ + `{}` is not a compute capability to run one at.\n\n \ + Emit anyway, with the notice, using `--no-verify`; or re-run the \ + configuration through `launchbound prune --cc ` for the \ + part you will deploy on.", + report.gate_cc, + report.gate_cc, + ); + } + // Render the winning params.rs through the same specializer that built // the measured artifact: what you paste is what was measured. let scratch_root = launchbound_build::scratch::default_scratch_root(&spec); @@ -337,6 +421,34 @@ fn cmd_apply( launchbound_build::scratch::write_params(&spec, &config, &scratch)?; let params = std::fs::read_to_string(scratch.join("src/params.rs"))?; + if verify { + eprintln!("verifying the emitted specialization with cargo reconverge --strict ..."); + let verdicts = launchbound_prune::prune_kernel( + &spec, + &launchbound_prune::PruneOptions { + cc: report.gate_cc.clone(), + reconverge_dir, + scratch_root: None, + }, + )?; + let cv = verdicts + .iter() + .find(|cv| cv.config.id().as_str() == chosen.id) + .ok_or_else(|| anyhow::anyhow!("chosen config missing from prune output"))?; + match &cv.verdict { + launchbound_prune::Verdict::Clean => { + eprintln!("verified: clean under the gate at cc {}", report.gate_cc) + } + launchbound_prune::Verdict::AdmittedWithCaveats { .. } => { + eprintln!("verified: admitted with caveats (see the report)") + } + // `{other}`, not `{other:?}`: this reaches a person. + other => anyhow::bail!( + "the chosen configuration does not pass the gate — refusing to emit:\n{other}" + ), + } + } + let block: Vec = ["block_x", "block_y", "block_z"] .iter() .filter_map(|d| config.get(d).map(|v| format!("{d} = {v}"))) @@ -365,35 +477,24 @@ fn cmd_apply( spec.domain ); println!("// This result is valid only for the part above; it does not port across parts."); + if !verify { + // Carried into the output the way the Metal notice is, so the + // qualification travels with the thing it qualifies: a `params.rs` + // pasted into a repository outlives the terminal it was printed in. + println!("// *** NOT VERIFIED: verification was turned off, so the gate did not"); + println!("// *** re-check this configuration. Run `launchbound prune --cc `"); + println!("// *** before you rely on it."); + } + if ungated { + println!( + "// *** NO convergence gate exists on the {} path: the same bug class is", + report.gate_cc + ); + println!("// *** NOT checked (docs/SAFETY.md)."); + } println!("// ---- src/params.rs ----"); print!("{params}"); - if verify { - eprintln!("verifying the emitted specialization with cargo reconverge --strict ..."); - let verdicts = launchbound_prune::prune_kernel( - &spec, - &launchbound_prune::PruneOptions { - cc: report.gate_cc.clone(), - reconverge_dir, - scratch_root: None, - }, - )?; - let cv = verdicts - .iter() - .find(|cv| cv.config.id().as_str() == chosen.id) - .ok_or_else(|| anyhow::anyhow!("chosen config missing from prune output"))?; - match &cv.verdict { - launchbound_prune::Verdict::Clean => { - eprintln!("verified: clean under the gate at cc {}", report.gate_cc) - } - launchbound_prune::Verdict::AdmittedWithCaveats { .. } => { - eprintln!("verified: admitted with caveats (see the report)") - } - other => anyhow::bail!( - "the chosen configuration no longer passes the gate: {other:?} — refusing to emit" - ), - } - } Ok(ExitCode::SUCCESS) } @@ -409,7 +510,18 @@ fn cmd_tune( seed: u64, reconverge_dir: Option, ) -> anyhow::Result { - let budget_secs = budget.map(parse_budget).transpose()?; + let budget_secs = budget + .map(parse_budget) + .transpose() + .map_err(|e| anyhow::anyhow!("{e}"))?; + // On any backend: an exhaustive sweep makes no random choice, so there + // is nothing for a seed to control. + if backend != "model" && order == "exhaustive" && seed != 0 { + eprintln!( + "note: --seed has no effect with --order exhaustive — every candidate is \ + measured, in order, so there is no sampling to seed." + ); + } let dir = resolve_kernel_dirs(Some(kernel), corpus)?.remove(0); let spec = KernelSpec::load(&dir)?; let explicit_out = out.is_some(); @@ -460,6 +572,10 @@ fn cmd_tune( "model" => { // Nothing is written on this path, so an --out the caller took // the trouble to type is worth answering rather than ignoring. + // The same reasoning for the three below: a flag accepted and + // silently ignored is a promise the tool does not keep, and + // `--budget` is the one that matters — somebody who passes + // `--budget 30m` reasonably believes something is bounded. if explicit_out { eprintln!( "note: --out is unused with --backend model — it prints the ranking and \ @@ -467,6 +583,19 @@ fn cmd_tune( `launchbound report` can read." ); } + if budget.is_some() { + eprintln!( + "note: --budget is unused with --backend model — nothing is measured, \ + so there is no sweep to bound. `--backend cuda` and `--backend metal` \ + honour it." + ); + } + if order != "exhaustive" || seed != 0 { + eprintln!( + "note: --order and --seed are unused with --backend model — the ranking \ + is analytic and always in cost order." + ); + } use launchbound_model::{device, estimate}; use launchbound_prune::{PruneOptions, Verdict, prune_kernel}; let verdicts = prune_kernel( @@ -599,8 +728,17 @@ fn cmd_model( } if let Some(results_path) = results { + // The cause, not "cannot read": a truncated file, a `results.v2` + // and a path that is simply not there are three different problems + // and used to share one message. let measured = launchbound_bench::Results::load(&results_path) - .ok_or_else(|| anyhow::anyhow!("cannot read results.v1 at {results_path:?}"))?; + .map_err(|e| anyhow::anyhow!("{e}"))? + .ok_or_else(|| { + anyhow::anyhow!( + "{}: no results.v1 there — the measurement box has not written one yet", + results_path.display() + ) + })?; let mut xs = Vec::new(); // model cost let mut ys = Vec::new(); // measured median for est in &estimates { @@ -1046,3 +1184,58 @@ fn cmd_space( } Ok(ExitCode::SUCCESS) } + +#[cfg(test)] +mod budget_tests { + use super::parse_budget; + + #[test] + fn accepted_forms_are_seconds() { + for (input, seconds) in [ + ("45", 45.0), + ("90s", 90.0), + ("30m", 1800.0), + ("30min", 1800.0), + ("1h", 3600.0), + ("1hr", 3600.0), + ("2sec", 2.0), + (" 90s ", 90.0), + // Zero is a budget, and a meaningful one: the guard fires at + // once and the run says how far it got. + ("0", 0.0), + ] { + assert_eq!(parse_budget(input), Ok(seconds), "{input}"); + } + } + + #[test] + fn nothing_unbounded_or_unparseable_is_accepted() { + // The first two used to panic at exit 101; the two after them used + // to be accepted and produce an unbounded sweep. + for input in ["", " ", "NaNs", "1e400s", "inf", "-5s", "abc", "s", "min"] { + let err = parse_budget(input).unwrap_err(); + assert!( + err.contains("--budget") || err.contains(&format!("`{}`", input.trim())), + "{input}: the message must name the flag or the value: {err}" + ); + assert!( + err.contains("expected a positive number of seconds"), + "{input}: and say what would have been accepted: {err}" + ); + } + } + + #[test] + fn a_budget_that_parses_can_actually_bound_a_sweep() { + // The property the guard needs, stated where it can fail: `x >= NaN` + // is false for every x, and nothing is ever `>= inf`. + for input in ["0", "45", "90s", "30m", "1h"] { + let seconds = parse_budget(input).unwrap(); + assert!(seconds.is_finite() && seconds >= 0.0, "{input}"); + assert!( + f64::MAX >= seconds, + "{input}: an elapsed time must be able to reach it" + ); + } + } +} diff --git a/crates/launchbound-prune/src/decide.rs b/crates/launchbound-prune/src/decide.rs index d88911e..d8813d6 100644 --- a/crates/launchbound-prune/src/decide.rs +++ b/crates/launchbound-prune/src/decide.rs @@ -41,6 +41,41 @@ pub enum Verdict { ToolError { detail: String }, } +impl std::fmt::Display for Verdict { + /// Prose, because these reach a user. + /// + /// `apply` printed `ToolError { detail: "…\n…" }` — a Rust struct + /// literal with escaped newlines — in a message somebody is meant to act + /// on. A `Debug` dump is a fine thing to log and a poor thing to read. + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Verdict::Clean => write!(f, "clean"), + Verdict::AdmittedWithCaveats { caveats } => { + write!(f, "admitted with {} caveat", caveats.len())?; + if caveats.len() != 1 { + write!(f, "s")?; + } + for caveat in caveats { + write!(f, "\n {} {}", caveat.rule, caveat.message)?; + } + Ok(()) + } + Verdict::Disqualified { records } => { + write!(f, "refused by the gate")?; + for record in records { + write!(f, "\n {} ", record.rule)?; + if let Some(span) = &record.span { + write!(f, "at {span}: ")?; + } + write!(f, "{}", record.reason)?; + } + Ok(()) + } + Verdict::ToolError { detail } => write!(f, "{detail}"), + } + } +} + #[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct RejectionRecord { pub rule: String, diff --git a/crates/launchbound-prune/src/findings.rs b/crates/launchbound-prune/src/findings.rs index 26a8f6a..68dcf7a 100644 --- a/crates/launchbound-prune/src/findings.rs +++ b/crates/launchbound-prune/src/findings.rs @@ -1,6 +1,18 @@ //! Serde model of reconverge's `findings.v1` document. Tolerant of unknown //! fields: reconverge may grow the schema, and the gate must not silently //! pass on a parse failure (the caller treats one as a tool error). +//! +//! **The contract is JSONL, one document per analyzed *target*.** A package +//! with a lib and a bin compiles twice and prints two lines; before 2.1.0 +//! this reader handed the whole of stdout to one `from_str`, so a second +//! line was `trailing characters at line 2 column 1` — a tool error, a hard +//! stop, for every candidate of a crate that has nothing wrong with it. A +//! `src/main.rs` beside a kernel library is the ordinary shape of a GPU +//! crate: the host launcher lives there. +//! +//! reconverge 0.5.0 added `target` to distinguish the documents; it is +//! optional here because an older analyzer on someone's PATH does not write +//! it, and the union below does not depend on it. use serde::{Deserialize, Serialize}; @@ -9,10 +21,42 @@ pub struct FindingsDoc { /// Schema tag; expected `findings.v1`. #[serde(default)] pub schema: String, + /// The compiled target's crate types (`lib`, `bin`, …). reconverge + /// 0.5.0 and later; absent from an older analyzer's output. + #[serde(default)] + pub target: Option, #[serde(default)] pub findings: Vec, } +/// Why a stream of findings documents could not be read. +#[derive(Debug)] +pub enum ReadError { + /// A line was not a findings document at all. + Parse { + line: usize, + error: serde_json::Error, + }, + /// A line parsed but declared a schema this build does not implement. + Schema { line: usize, declared: String }, + /// The analyzer printed nothing where a document was expected. + Empty, +} + +impl std::fmt::Display for ReadError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ReadError::Parse { line, error } => { + write!(f, "findings.v1 parse failed on line {line}: {error}") + } + ReadError::Schema { line, declared } => { + write!(f, "unexpected findings schema `{declared}` on line {line}") + } + ReadError::Empty => write!(f, "the analyzer printed no findings document"), + } + } +} + #[derive(Debug, Clone, Deserialize, Serialize)] pub struct Finding { /// Rule ID, e.g. `RC001`. @@ -61,3 +105,74 @@ impl FindingsDoc { serde_json::from_str(json) } } + +/// Read reconverge's stdout as JSONL and take the union of the findings. +/// +/// The union is the decision rule, not a convenience: a deny finding in +/// *any* target of the crate is a reason to refuse, and the bin target's +/// document — usually empty, since the kernels live in the lib — is +/// harmless to merge. It is also what makes a multi-crate kernel workspace +/// possible later without touching `decide`. +/// +/// A line that is not a findings document is still an error, and still a +/// hard stop: `docs/SAFETY.md` §2 is explicit that unreadable analyzer +/// output is never a pass. +/// +/// # Errors +/// +/// The first line that does not parse, or that declares another schema, +/// naming which line it was. +pub fn read_stream(stdout: &str) -> Result, ReadError> { + let mut findings = Vec::new(); + let mut documents = 0; + for (index, line) in stdout.lines().enumerate() { + let line = line.trim(); + if line.is_empty() { + continue; + } + let number = index + 1; + let doc = FindingsDoc::parse(line).map_err(|error| ReadError::Parse { + line: number, + error, + })?; + if doc.schema != "findings.v1" { + return Err(ReadError::Schema { + line: number, + declared: doc.schema, + }); + } + documents += 1; + findings.extend(doc.findings); + } + if documents == 0 { + return Err(ReadError::Empty); + } + Ok(findings) +} + +/// The first `limit` bytes of what was received, for a tool-error detail. +/// +/// "trailing characters at line 2 column 1" told the person who reported +/// this everything and would tell a user nothing. Truncated on a character +/// boundary and with control bytes escaped, because this is foreign output +/// on its way to a terminal. +pub fn received_excerpt(stdout: &str, limit: usize) -> String { + let mut out = String::new(); + for ch in stdout.chars() { + if out.len() >= limit { + out.push('…'); + break; + } + match ch { + '\n' => out.push_str("\\n"), + '\t' => out.push_str("\\t"), + c if c.is_control() => out.push('\u{fffd}'), + c => out.push(c), + } + } + if out.is_empty() { + "(nothing)".to_string() + } else { + out + } +} diff --git a/crates/launchbound-prune/src/runner.rs b/crates/launchbound-prune/src/runner.rs index f157b3f..1c90c5d 100644 --- a/crates/launchbound-prune/src/runner.rs +++ b/crates/launchbound-prune/src/runner.rs @@ -7,7 +7,7 @@ use crate::PruneError; use crate::decide::{AnalyzerOutcome, Verdict, decide}; -use crate::findings::FindingsDoc; +use crate::findings; use launchbound_build::scratch::{default_scratch_root, prepare_scratch, write_params}; use launchbound_space::{Config, KernelSpec, enumerate}; use std::collections::BTreeMap; @@ -61,6 +61,12 @@ pub fn prune_kernel( /// Run `cargo reconverge check` in `dir`. Exit 2 or unparseable output is a /// tool error — a hard stop, never a pass by omission (docs/SAFETY.md). +/// +/// A failure names `dir`, which is the *scratch* copy rather than the +/// kernel crate. The distinction is the whole diagnosis when the two +/// differ: the gate used to report `error: could not compile` against a +/// crate whose own `cargo check` is clean, and never said that what it +/// compiled was not what the reader was looking at. `cd` there and see. fn run_reconverge(dir: &Path, options: &PruneOptions) -> AnalyzerOutcome { let mut cmd = Command::new("cargo"); cmd.args([ @@ -97,23 +103,26 @@ fn run_reconverge(dir: &Path, options: &PruneOptions) -> AnalyzerOutcome { let exit_code = output.status.code().unwrap_or(-1); if exit_code == 0 || exit_code == 1 { let stdout = String::from_utf8_lossy(&output.stdout); - match FindingsDoc::parse(stdout.trim()) { - Ok(doc) if doc.schema == "findings.v1" => AnalyzerOutcome::Findings { + // JSONL, one document per analyzed target — reconverge's documented + // contract, and the shape a package with a lib and a bin produces. + match findings::read_stream(&stdout) { + Ok(findings) => AnalyzerOutcome::Findings { exit_code, - findings: doc.findings, - }, - Ok(doc) => AnalyzerOutcome::ToolError { - detail: format!("unexpected findings schema `{}`", doc.schema), + findings, }, Err(e) => AnalyzerOutcome::ToolError { - detail: format!("findings.v1 parse failed: {e}"), + detail: format!( + "{e}\n received: {}", + findings::received_excerpt(&stdout, 200) + ), }, } } else { let stderr = String::from_utf8_lossy(&output.stderr); AnalyzerOutcome::ToolError { detail: format!( - "cargo reconverge exited {exit_code}:\n{}", + "cargo reconverge exited {exit_code} in {}:\n{}", + dir.display(), diagnosis(&stderr) ), } @@ -136,10 +145,21 @@ fn run_reconverge(dir: &Path, options: &PruneOptions) -> AnalyzerOutcome { /// /// Falls back to the *head* rather than the tail when nothing is marked — /// a tool that prints a reference puts the reason before it. +/// +/// The marker has to be **both** forms. rustc's primary diagnostics begin +/// `error[E0583]:` — a code in brackets before the colon — so a filter on +/// `error:` alone kept cargo's summary line and dropped the line that names +/// the failure. "See the errors above": the one that was above was the one +/// the filter removed. reconverge's own `error:` lines came through, so the +/// filter worked for the analyzer and failed for the compiler, which is the +/// common case of a gate tool error on a kernel that has one. fn diagnosis(stderr: &str) -> String { let marked: Vec<&str> = stderr .lines() - .filter(|line| line.trim_start().starts_with("error:")) + .filter(|line| { + let line = line.trim_start(); + line.starts_with("error:") || line.starts_with("error[") + }) .collect(); if !marked.is_empty() { return marked.join("\n"); diff --git a/crates/launchbound-report/src/build.rs b/crates/launchbound-report/src/build.rs index 33e3fee..6cb686f 100644 --- a/crates/launchbound-report/src/build.rs +++ b/crates/launchbound-report/src/build.rs @@ -32,7 +32,12 @@ impl RunDir { ))); } let plan = BenchPlan::load(&dir.join("plan.json")).ok(); - let results = Results::load(&dir.join("results.json")); + // A results.json that exists but cannot be read is an error, not + // "the box has not run yet". The two call for opposite actions — + // wait, or go and look — and rendering both as `unmeasured` at + // exit 0 told them apart for nobody. Same treatment `verdicts.v1` + // gets fifteen lines above. + let results = Results::load(&dir.join("results.json")).map_err(ReportError::RunDir)?; Ok(RunDir { verdicts, plan, diff --git a/crates/launchbound-tui/Cargo.toml b/crates/launchbound-tui/Cargo.toml index acdeb03..62abfe6 100644 --- a/crates/launchbound-tui/Cargo.toml +++ b/crates/launchbound-tui/Cargo.toml @@ -22,4 +22,4 @@ crossterm = "0.29" ratatui = "0.30" [dev-dependencies] -termlens = { version = "0.6.1", default-features = false } +termlens = { version = "0.9", default-features = false } diff --git a/docs/LIMITATIONS.md b/docs/LIMITATIONS.md index 7d54b18..d5f6e3f 100644 --- a/docs/LIMITATIONS.md +++ b/docs/LIMITATIONS.md @@ -6,7 +6,7 @@ launchbound's, with numbers where we have them. Everything here was true on ## The gate inherits reconverge's limits, wholesale -A clean gate is **not a proof of correctness**. `reconverge` (v0.4.0) is +A clean gate is **not a proof of correctness**. `reconverge` (v0.5.0) is summary-based and interprocedural, handles reducible control flow only, cannot evaluate non-literal masks, and puts data races entirely out of scope. Its own documentation is the authority; launchbound adds no analysis diff --git a/justfile b/justfile index 1602ded..5530ded 100644 --- a/justfile +++ b/justfile @@ -3,7 +3,7 @@ default: ci # The full local gate. Never push a commit that fails this. -ci: fmt-check clippy test deny schemas +ci: fmt-check clippy test deny schemas pins # Cargo errors on a memberless virtual workspace, so the cargo recipes no-op # until the first crate lands in S1. `grep -c` prints 1 when packages is empty. @@ -38,6 +38,13 @@ gate: prune cc="8.6": cargo run -q -p launchbound-cli -- prune --cc {{ cc }} +# The six recorded pin sites agree with each other. No network: the +# dispatch-only `pins.yml` asks upstream, this asks ourselves — and a +# watcher whose own baseline is stale reports drift from a version nothing +# installs, which is how #17 came to describe a pin two releases old. +pins: + ./scripts/check-pins.sh + # Golden + JSON Schema validation of report documents (S4). schemas: cargo test -p launchbound-report --test schema_and_golden diff --git a/rust-toolchain.toml b/rust-toolchain.toml index e113f35..a14a8c0 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -6,7 +6,7 @@ # Current lockstep set: # nightly nightly-2026-04-03 # cuda-oxide 50d07314eb8b7d5ec821ba02b0048a753c20dd4e -# reconverge v0.1.11 (43780b58) +# reconverge v0.5.0 [toolchain] channel = "nightly-2026-04-03" components = ["rustfmt", "clippy"] diff --git a/scripts/check-pins.sh b/scripts/check-pins.sh new file mode 100755 index 0000000..cf06d83 --- /dev/null +++ b/scripts/check-pins.sh @@ -0,0 +1,94 @@ +#!/usr/bin/env bash +# CI gate: the six recorded pin sites agree with each other. +# +# The policy is "every site, or not at all" (CONTRIBUTING §9), and 2.0.0 did +# four of six: it moved the gate to reconverge 0.4.0 in `action.yml`, +# `action/README.md`, `prune.yml` and `docs/LIMITATIONS.md`, and left +# `rust-toolchain.toml`, `CONTRIBUTING.md` and `pins.yml` recording 0.1.11. +# +# The cost was not cosmetic. `pins.yml` measures upstream drift against its +# own `RECONVERGE_PIN`, so with a stale baseline its weekly signal reports +# movement away from a version nothing installs — which is why #17 sat open +# describing a pin the gate had not used for a release. A watcher whose +# baseline is wrong is worse than no watcher: it produces noise that looks +# like a finding. +# +# This asks nothing of the network, so it runs in the ordinary CI job rather +# than in the dispatch-only watch. +set -euo pipefail + +ROOT=$(cd "$(dirname "$0")/.." && pwd) +cd "$ROOT" + +status=0 + +fail() { + echo "PIN DISAGREEMENT: $1" >&2 + status=1 +} + +# --- reconverge. Recorded with a leading `v` in two places and without it in +# four, so compare the bare version. +declare -A RECONVERGE=( + ["rust-toolchain.toml"]="$(sed -n 's/^# reconverge v\([0-9][0-9.]*\).*/\1/p' rust-toolchain.toml)" + ["CONTRIBUTING.md"]="$(sed -n 's/.*reconverge \([0-9][0-9.]*\) (installed from crates.io).*/\1/p' CONTRIBUTING.md)" + [".github/workflows/pins.yml"]="$(sed -n 's/^ RECONVERGE_PIN: v\([0-9][0-9.]*\).*/\1/p' .github/workflows/pins.yml)" + [".github/workflows/prune.yml"]="$(sed -n 's/^ RECONVERGE_VERSION: "\([0-9][0-9.]*\)".*/\1/p' .github/workflows/prune.yml)" + ["action/action.yml"]="$(sed -n '/^ reconverge-version:/,/^ [a-z]/s/^ default: "\([0-9][0-9.]*\)".*/\1/p' action/action.yml)" + ["action/README.md"]="$(sed -n 's/^| `reconverge-version` | `\([0-9][0-9.]*\)`.*/\1/p' action/README.md)" + ["docs/LIMITATIONS.md"]="$(sed -n 's/.*`reconverge` (v\([0-9][0-9.]*\)).*/\1/p' docs/LIMITATIONS.md)" +) + +# --- cuda-oxide, recorded as a full SHA in three places. +declare -A CUDA_OXIDE=( + ["rust-toolchain.toml"]="$(sed -n 's/^# cuda-oxide \([0-9a-f]\{40\}\).*/\1/p' rust-toolchain.toml)" + [".github/workflows/pins.yml"]="$(sed -n 's/^ CUDA_OXIDE_PIN: \([0-9a-f]\{40\}\).*/\1/p' .github/workflows/pins.yml)" + # The "cargo oxide is not installed" message quotes the pin at the reader + # and tells them to check that commit out. A message that names a stale + # commit is worse than the cargo help it replaced. + ["crates/launchbound-build/src/compile.rs"]="$(sed -n 's/^pub const CUDA_OXIDE_PIN: &str = "\([0-9a-f]\{40\}\)".*/\1/p' crates/launchbound-build/src/compile.rs)" +) + +# --- the nightly, which `rust-toolchain.toml` owns. +TOOLCHAIN=$(sed -n 's/^channel = "\(.*\)"/\1/p' rust-toolchain.toml) + +agree() { + local -n table=$1 + local label=$2 expected="" site ok=1 + for site in "${!table[@]}"; do + local value="${table[$site]}" + if [ -z "$value" ]; then + fail "$label: could not read a pin from $site (has its shape changed?)" + ok=0 + continue + fi + if [ -z "$expected" ]; then + expected="$value" + elif [ "$value" != "$expected" ]; then + fail "$label: $site records $value, but another site records $expected" + ok=0 + fi + done + # Only claim agreement when there was some: a summary line contradicting + # the failure printed above it is the exact shape of bug this gate exists + # to catch, and printing one here would be embarrassing. + if [ "$ok" -eq 1 ] && [ -n "$expected" ]; then + echo " $label $expected (${#table[@]} sites agree)" + fi +} + +echo "lockstep pin set:" +agree RECONVERGE reconverge +agree CUDA_OXIDE cuda-oxide +echo " nightly $TOOLCHAIN" + +# The nightly is also named in the action, which installs it. +action_toolchain=$(sed -n '/^ toolchain:/,/^ [a-z]/s/^ default: "\(nightly-[0-9-]*\)".*/\1/p' action/action.yml) +if [ -n "$action_toolchain" ] && [ "$action_toolchain" != "$TOOLCHAIN" ]; then + fail "nightly: action/action.yml records $action_toolchain, rust-toolchain.toml records $TOOLCHAIN" +fi + +if [ "$status" -eq 0 ]; then + echo "every recorded pin site agrees" +fi +exit "$status" From 88251ec8ec6680c291f809d6f69799a7501c1c5d Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Sat, 5 Sep 2026 21:15:53 +0700 Subject: [PATCH 2/5] fix(tui): stop cutting values and sentences at the panel border MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #24 fixed this on the chosen line and left it standing in the two views below. The ranking lost every closing bracket at eighty columns, so each interval read as a number with no upper bound and a dangling comma. The rejections view — the one the README calls the point of the tool — lost the half of the reason that says what to do: the reader got as far as `splits a 64-threa` and never reached `safe only at one warp (<= 32 threads)`. Nothing marked the cut, so it read as the whole reason. The two want different treatment and that is the substance of the fix. An interval is a field, so the ranking drops it whole when it does not fit, through the same helper the chosen line uses — shared so the two views cannot disagree about it again, which is how this came to be fixed once and missed twice. A rejection reason is a sentence whose actionable clause is at the end, so it wraps instead, and the panel spends the two or three rows. The guard is the scan that found both: no golden line may reach the panel border with a character that means the value continued — a digit, `,`, `[`, `(`, `=`, `-` — or with a cut word, unless an ellipsis marks it. Confirmed red against the shipped pre-fix golden, which is also what it would have done to #24. Alongside: `prune`'s verdict line now says what it checked and what it did not. The gate answers convergence and shared-memory capacity at a `--cc`; it has no view of instruction availability, so a crate using an `sm_80+` intrinsic under `needs_cc = "7.5"` prunes to `12 clean` at `--cc 7.5` and fails only when something finally lowers it. That is defensible and was nowhere stated, so "3 clean" read as "this kernel is fine at cc 7.5". docs/LIMITATIONS.md carries the long form, including why the two stronger fixes were not built. Tests: the JSONL union and every way it can fail, a scratch copy over a crate with `src/util/inner.rs` and a manifest-named build script, a `error[E0583]:` line surviving the diagnosis filter, and each of the six unreadable `results.json` shapes. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- .../launchbound-build/tests/scratch_tree.rs | 100 ++++++++++++++ crates/launchbound-cli/src/main.rs | 20 +++ crates/launchbound-prune/src/findings.rs | 123 ++++++++++++++++++ crates/launchbound-prune/src/runner.rs | 18 +++ .../tests/results_shapes.rs | 88 +++++++++++++ crates/launchbound-tui/src/app.rs | 62 ++++++++- .../tests/golden/ranking-scrolled-80x24.txt | 10 +- .../tests/golden/rejections-80x24.txt | 24 ++-- crates/launchbound-tui/tests/tui.rs | 63 +++++++++ docs/LIMITATIONS.md | 29 +++++ 10 files changed, 515 insertions(+), 22 deletions(-) create mode 100644 crates/launchbound-build/tests/scratch_tree.rs create mode 100644 crates/launchbound-report/tests/results_shapes.rs diff --git a/crates/launchbound-build/tests/scratch_tree.rs b/crates/launchbound-build/tests/scratch_tree.rs new file mode 100644 index 0000000..ad47406 --- /dev/null +++ b/crates/launchbound-build/tests/scratch_tree.rs @@ -0,0 +1,100 @@ +//! `prepare_scratch` copies the crate, not a subset of it. +//! +//! It took only the entries of `src/` that are files, so `mod util;` with +//! `src/util/mod.rs` — how Rust code is organised past one file — produced a +//! scratch crate that could not compile. The gate then reported +//! `error: could not compile` against a crate whose own `cargo check` is +//! clean, and never said that what it compiled was not the reader's crate. +//! +//! The six corpus kernels are single-file because `corpus/README.md` asks +//! them to be, which is why nothing here caught it. + +use launchbound_space::KernelSpec; +use std::fs; +use std::path::{Path, PathBuf}; + +fn write(path: &Path, body: &str) { + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, body).unwrap(); +} + +/// A kernel crate with a module directory, a nested module below it, and a +/// build script — the three things the old copy dropped. +fn kernel_with_a_module_directory(tag: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("lb-scratch-{tag}-{}", std::process::id())); + let _ = fs::remove_dir_all(&dir); + write( + &dir.join("Cargo.toml"), + "[package]\nname = \"probe\"\nversion = \"0.0.0\"\nedition = \"2024\"\n", + ); + write(&dir.join("build.rs"), "fn main() {}\n"); + write(&dir.join("src/lib.rs"), "mod params;\nmod util;\n"); + write(&dir.join("src/params.rs"), "pub const TILE: usize = 128;\n"); + write(&dir.join("src/util/mod.rs"), "pub mod inner;\n"); + write(&dir.join("src/util/inner.rs"), "pub const K: u32 = 1;\n"); + write( + &dir.join("kernel.toml"), + "[kernel]\nname = \"probe\"\nentry = \"probe\"\nneeds_cc = \"7.5\"\ndomain = 1\n\n\ + [dims.tile]\nrole = \"spec\"\nvalues = [128]\n", + ); + dir +} + +#[test] +fn a_module_directory_reaches_the_scratch_copy() { + let dir = kernel_with_a_module_directory("moddir"); + let spec = KernelSpec::load(&dir).expect("the probe kernel.toml loads"); + let root = dir.join("target/launchbound-scratch"); + let scratch = launchbound_build::scratch::prepare_scratch(&spec, &root).expect("scratch"); + + for rel in [ + "src/lib.rs", + "src/params.rs", + "src/util/mod.rs", + "src/util/inner.rs", + // One directory up, and the same omission: a build script the + // manifest would have run. + "build.rs", + "Cargo.toml", + ] { + assert!( + scratch.join(rel).is_file(), + "{rel} must be in the scratch copy — the gate compiles this, not your crate" + ); + } + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn a_build_script_named_by_the_manifest_is_copied_from_where_it_lives() { + let dir = kernel_with_a_module_directory("buildpath"); + let manifest = dir.join("Cargo.toml"); + let text = fs::read_to_string(&manifest).unwrap(); + fs::write( + &manifest, + text.replace("edition", "build = \"tools/gen.rs\"\nedition"), + ) + .unwrap(); + write(&dir.join("tools/gen.rs"), "fn main() {}\n"); + + let spec = KernelSpec::load(&dir).unwrap(); + let root = dir.join("target/launchbound-scratch"); + let scratch = launchbound_build::scratch::prepare_scratch(&spec, &root).unwrap(); + assert!(scratch.join("tools/gen.rs").is_file()); + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn a_target_directory_under_src_is_not_dragged_along() { + let dir = kernel_with_a_module_directory("skiptarget"); + write(&dir.join("src/target/huge.bin"), "not source\n"); + let spec = KernelSpec::load(&dir).unwrap(); + let root = dir.join("target/launchbound-scratch"); + let scratch = launchbound_build::scratch::prepare_scratch(&spec, &root).unwrap(); + assert!(scratch.join("src/lib.rs").is_file()); + assert!( + !scratch.join("src/target").exists(), + "build output is not source" + ); + let _ = fs::remove_dir_all(&dir); +} diff --git a/crates/launchbound-cli/src/main.rs b/crates/launchbound-cli/src/main.rs index 90d1c0c..c01e6d2 100644 --- a/crates/launchbound-cli/src/main.rs +++ b/crates/launchbound-cli/src/main.rs @@ -44,6 +44,11 @@ enum Command { /// Target compute capability for RC004 shared-memory context /// (docs/SAFETY.md): 8.6 for A10G, 7.5 for T4. A verdict at one /// --cc does not transfer to another. + /// + /// The gate checks thread convergence and static shared-memory + /// capacity at this capability. It does NOT check that the device + /// code can be lowered for it: instruction availability is + /// `needs_cc` in kernel.toml and is the author's claim. #[arg(long, value_parser = parse_cc)] cc: String, /// Directory containing the cargo-reconverge binary (else @@ -1059,6 +1064,21 @@ fn cmd_prune( println!( " => {clean} clean, {caveats} with caveats, {refused} refused, {errors} tool errors" ); + // What "clean" means, on the line that says it. The gate answers + // the convergence question and the shared-memory one; it has no + // view of instruction availability, so a kernel using an + // `sm_80+` intrinsic under `needs_cc = "7.5"` is admitted at + // `--cc 7.5` and only fails when something finally lowers to PTX + // for that part. `needs_cc` is the author's claim and is taken + // on trust — which is defensible, and was nowhere stated. + if clean > 0 || caveats > 0 { + println!( + " (checked: thread convergence and static shared memory at cc {cc}. \ + NOT checked: whether the device code can lower for that part — \ + instruction availability is `needs_cc` in kernel.toml, the author's \ + claim, taken on trust.)" + ); + } } if verdicts .iter() diff --git a/crates/launchbound-prune/src/findings.rs b/crates/launchbound-prune/src/findings.rs index 68dcf7a..9487e6c 100644 --- a/crates/launchbound-prune/src/findings.rs +++ b/crates/launchbound-prune/src/findings.rs @@ -176,3 +176,126 @@ pub fn received_excerpt(stdout: &str, limit: usize) -> String { out } } + +#[cfg(test)] +mod stream_tests { + use super::*; + + fn doc(krate: &str, target: &str, codes: &[&str]) -> String { + let findings: Vec = codes + .iter() + .map(|code| { + serde_json::json!({ + "code": code, + "confidence": "deny", + "kernel": "k", + "message": "m", + "explain": code, + }) + }) + .collect(); + serde_json::json!({ + "schema": "findings.v1", + "tool": { "name": "reconverge", "version": "0.5.0" }, + "crate": krate, + "target": target, + "findings": findings, + }) + .to_string() + } + + /// The shape that hard-stopped every candidate of a crate with a + /// `src/main.rs` beside its library: two documents, one per target. + #[test] + fn a_lib_and_a_bin_are_two_documents_and_their_findings_union() { + let stdout = format!( + "{}\n{}\n", + doc("k", "bin", &[]), + doc("k", "lib", &["RC001"]) + ); + let findings = read_stream(&stdout).expect("two documents are the contract, not an error"); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].code, "RC001"); + } + + /// A deny finding in *any* target is a reason to refuse. + #[test] + fn a_finding_in_either_target_reaches_the_decision() { + let stdout = format!( + "{}\n{}\n", + doc("k", "lib", &["RC001"]), + doc("k", "bin", &["RC003"]) + ); + let mut codes: Vec = read_stream(&stdout) + .unwrap() + .into_iter() + .map(|f| f.code) + .collect(); + codes.sort(); + assert_eq!(codes, ["RC001", "RC003"]); + } + + #[test] + fn one_document_still_works_and_blank_lines_are_skipped() { + let stdout = format!("\n{}\n\n", doc("k", "lib", &["RC002"])); + assert_eq!(read_stream(&stdout).unwrap().len(), 1); + } + + /// An older analyzer writes no `target`; the union does not need one. + #[test] + fn a_document_without_a_target_field_still_parses() { + let stdout = r#"{"schema":"findings.v1","crate":"k","findings":[]}"#; + assert!(read_stream(stdout).unwrap().is_empty()); + let doc = FindingsDoc::parse(stdout).unwrap(); + assert_eq!(doc.target, None); + } + + /// Unreadable output is still a hard stop — `docs/SAFETY.md` §2 — and + /// now says which line, so a two-document stream with one bad line is + /// diagnosable. + #[test] + fn a_line_that_does_not_parse_is_an_error_naming_the_line() { + let stdout = format!("{}\nnot json\n", doc("k", "lib", &[])); + let err = read_stream(&stdout).unwrap_err().to_string(); + assert!(err.contains("line 2"), "{err}"); + assert!(err.contains("parse failed"), "{err}"); + } + + #[test] + fn another_schema_is_refused_by_name() { + let stdout = r#"{"schema":"findings.v99","crate":"k","findings":[]}"#; + let err = read_stream(stdout).unwrap_err().to_string(); + assert!(err.contains("findings.v99"), "{err}"); + } + + #[test] + fn no_output_at_all_is_an_error_rather_than_a_clean_pass() { + // The direction that matters: nothing must ever read as "no + // findings", which is a pass. + for stdout in ["", " ", "\n\n"] { + assert!(read_stream(stdout).is_err(), "{stdout:?}"); + } + } + + /// "trailing characters at line 2 column 1" told the reporter + /// everything and would tell a user nothing. + #[test] + fn a_failure_shows_what_was_received() { + let excerpt = received_excerpt("{\"schema\":\"findings.v1\"}\nsecond line\n", 200); + assert!(excerpt.contains("findings.v1"), "{excerpt}"); + assert!( + excerpt.contains("\\n"), + "newlines are escaped, not printed: {excerpt}" + ); + assert_eq!(received_excerpt("", 200), "(nothing)"); + // Bounded, and marked when it is cut. + let long = received_excerpt(&"x".repeat(500), 200); + assert!( + long.ends_with('…') && long.chars().count() <= 201, + "{}", + long.len() + ); + // Control bytes never reach a terminal from foreign output. + assert!(!received_excerpt("a\u{1b}[2Jb", 200).contains('\u{1b}')); + } +} diff --git a/crates/launchbound-prune/src/runner.rs b/crates/launchbound-prune/src/runner.rs index 1c90c5d..ba8fdd8 100644 --- a/crates/launchbound-prune/src/runner.rs +++ b/crates/launchbound-prune/src/runner.rs @@ -193,6 +193,24 @@ mod diagnosis_tests { ); } + /// rustc's primary diagnostics carry a code, and the filter dropped + /// them: `error[E0583]:` does not start with `error:`. So the message + /// the filter was built to preserve was the one it removed, for exactly + /// the class of failure a kernel author actually hits. + #[test] + fn a_rustc_diagnostic_with_a_code_survives() { + let stderr = "error[E0583]: file not found for module `util`\n\ + error: could not compile `reduce-flip` (lib) due to 1 previous error\n"; + let out = diagnosis(stderr); + assert!( + out.contains("error[E0583]: file not found for module `util`"), + "the line that names the failure must survive: {out}" + ); + // #19's line still comes through: cargo's summary is not the + // diagnosis, but dropping it would be a different bug. + assert!(out.contains("could not compile"), "{out}"); + } + #[test] fn every_marked_line_is_kept() { let stderr = "error: first\nnoise\nerror: second\n"; diff --git a/crates/launchbound-report/tests/results_shapes.rs b/crates/launchbound-report/tests/results_shapes.rs new file mode 100644 index 0000000..cc133df --- /dev/null +++ b/crates/launchbound-report/tests/results_shapes.rs @@ -0,0 +1,88 @@ +//! A `results.json` the report cannot read is an error, not "unmeasured". +//! +//! The run directory is the hand-off between two machines: `stage` writes +//! the plan, the measurement box writes `results.json`, and the file comes +//! back by whatever copies it. A partial copy, the wrong file, or a +//! `results.v2` from a newer runner all used to read as "the box has not run +//! yet" — exit 0, nothing on stderr, and a JSON report that validated and +//! said `unmeasured`. Those two conditions call for opposite actions: wait, +//! or go and look. + +use launchbound_report::RunDir; +use std::fs; +use std::path::PathBuf; + +/// A run directory with a valid `verdicts.json` and whatever `results.json` +/// the caller wants. +fn run_dir(tag: &str, results: Option<&str>) -> PathBuf { + let dir = std::env::temp_dir().join(format!("lb-results-{tag}-{}", std::process::id())); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).unwrap(); + fs::write( + dir.join("verdicts.json"), + serde_json::json!({ + "schema": "verdicts.v1", + "kernel": "probe", + "cc": "8.6", + "candidates": [], + }) + .to_string(), + ) + .unwrap(); + if let Some(body) = results { + fs::write(dir.join("results.json"), body).unwrap(); + } + dir +} + +#[test] +fn a_missing_results_file_is_still_simply_unmeasured() { + let dir = run_dir("missing", None); + let run = RunDir::load(&dir).expect("a run that has not been measured still loads"); + assert!(run.results.is_none()); + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn every_unreadable_shape_is_an_error_that_names_the_cause() { + // The six shapes from the report, each of which rendered identically. + for (tag, body, expect) in [ + ("truncated", "{", "not JSON"), + ("empty", "", "not JSON"), + ("null", "null", "no `schema` field"), + ("array", "[]", "no `schema` field"), + ( + "v2", + r#"{"schema":"results.v2"}"#, + "unsupported results schema", + ), + ( + "shape", + r#"{"schema":"results.v1"}"#, + "not a results.v1 document", + ), + ] { + let dir = run_dir(tag, Some(body)); + let err = RunDir::load(&dir) + .err() + .unwrap_or_else(|| panic!("{tag}: an unreadable results.json must be an error")) + .to_string(); + assert!( + err.contains(expect), + "{tag}: the message must name the cause, got: {err}" + ); + assert!(err.contains("results.json"), "{tag}: and the path: {err}"); + let _ = fs::remove_dir_all(&dir); + } +} + +#[test] +fn a_directory_named_results_json_is_an_error_too() { + let dir = run_dir("isdir", None); + fs::create_dir(dir.join("results.json")).unwrap(); + let err = RunDir::load(&dir) + .err() + .expect("a directory is not a document"); + assert!(err.to_string().contains("results.json"), "{err}"); + let _ = fs::remove_dir_all(&dir); +} diff --git a/crates/launchbound-tui/src/app.rs b/crates/launchbound-tui/src/app.rs index 611b002..ac2cff4 100644 --- a/crates/launchbound-tui/src/app.rs +++ b/crates/launchbound-tui/src/app.rs @@ -7,7 +7,7 @@ use ratatui::Frame; use ratatui::layout::{Constraint, Direction, Layout, Rect}; use ratatui::style::{Modifier, Style}; use ratatui::text::{Line, Span}; -use ratatui::widgets::{Block, Borders, List, ListItem, Paragraph}; +use ratatui::widgets::{Block, Borders, List, ListItem, Paragraph, Wrap}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum View { @@ -131,11 +131,38 @@ fn chosen_tail( lo_ms: f64, hi_ms: f64, panel_width: u16, +) -> String { + interval_tail( + id, + config, + median_ms, + lo_ms, + hi_ms, + panel_width, + CHOSEN_LABEL.len(), + ) +} + +/// `id config time [lo, hi]`, with the interval only if it fits whole. +/// +/// `prefix` is what the caller puts in front of it — the `CHOSEN ` label, +/// or a one-character marker and a space in the ranking. Shared so the two +/// views cannot disagree about when an interval is dropped, which is how +/// this defect came to be fixed in one view and left standing in the other. +#[allow(clippy::too_many_arguments)] +fn interval_tail( + id: &str, + config: &str, + median_ms: f64, + lo_ms: f64, + hi_ms: f64, + panel_width: u16, + prefix: usize, ) -> String { let without = format!("{id} {config} {median_ms:.4} ms"); let with = format!("{without} [{lo_ms:.4}, {hi_ms:.4}]"); // The panel's two border columns are not text. - let usable = usize::from(panel_width).saturating_sub(2 + CHOSEN_LABEL.len()); + let usable = usize::from(panel_width).saturating_sub(2 + prefix); if with.chars().count() <= usable { with } else { @@ -295,9 +322,22 @@ fn draw_ranking(frame: &mut Frame<'_>, app: &App, area: Rect) { } else { " " }; + // Same precedence as the chosen line, and the same reason: at + // eighty columns every row here lost its closing bracket, so + // each interval read as a number with no upper bound. The id, + // the config and the time outrank the interval; a reader who + // needs it has a wider terminal. ListItem::new(format!( - "{marker} {} {} {:.4} ms [{:.4}, {:.4}]", - c.id, c.config, s.median_ms, s.ci95_lo_ms, s.ci95_hi_ms + "{marker} {}", + interval_tail( + &c.id, + &c.config, + s.median_ms, + s.ci95_lo_ms, + s.ci95_hi_ms, + area.width, + 2, // the marker and the space after it + ) )) }) .collect(); @@ -357,7 +397,19 @@ fn draw_rejections(frame: &mut Frame<'_>, app: &App, area: Rect) { } let visible: Vec> = lines.into_iter().skip(app.scroll).collect(); frame.render_widget( - Paragraph::new(visible).block(Block::default().borders(Borders::ALL).title("rejections")), + Paragraph::new(visible) + .block(Block::default().borders(Borders::ALL).title("rejections")) + // Wrapped, not shortened. A rejection reason is a sentence, and + // its actionable clause is at the end: the reader used to get as + // far as `splits a 64-threa` and never reach `safe only at one + // warp (<= 32 threads)`, which is the only part that says what to + // do. Nothing marked the cut, so it read as the whole reason. An + // interval can be dropped whole because it is a field; a sentence + // cannot, so this panel spends the two or three rows instead. + // + // `trim: false` keeps the leading indentation that distinguishes + // a reason from the configuration line above it. + .wrap(Wrap { trim: false }), area, ); } diff --git a/crates/launchbound-tui/tests/golden/ranking-scrolled-80x24.txt b/crates/launchbound-tui/tests/golden/ranking-scrolled-80x24.txt index 81b48d6..ab10a01 100644 --- a/crates/launchbound-tui/tests/golden/ranking-scrolled-80x24.txt +++ b/crates/launchbound-tui/tests/golden/ranking-scrolled-80x24.txt @@ -2,11 +2,11 @@ size: 80x24 cursor: hidden launchbound — reduce-flip · gate cc 8.6 · measured · NVIDIA A10G 27 candidates · 9 admitted · 18 refused · 10 measured ok ┌ranking (10 measured)─────────────────────────────────────────────────────────┐ -│ c1-0000000000000004 block_x=32 tile=256 unroll=0 0.0520 ms [0.0517, 0.0523│ -│ c1-0000000000000005 block_x=32 tile=256 unroll=2 0.0550 ms [0.0547, 0.0553│ -│ c1-0000000000000006 block_x=32 tile=256 unroll=4 0.0580 ms [0.0577, 0.0583│ -│ c1-0000000000000007 block_x=32 tile=512 unroll=0 0.0610 ms [0.0607, 0.0613│ -│ c1-0000000000000008 block_x=32 tile=512 unroll=2 0.0640 ms [0.0637, 0.0643│ +│ c1-0000000000000004 block_x=32 tile=256 unroll=0 0.0520 ms │ +│ c1-0000000000000005 block_x=32 tile=256 unroll=2 0.0550 ms │ +│ c1-0000000000000006 block_x=32 tile=256 unroll=4 0.0580 ms │ +│ c1-0000000000000007 block_x=32 tile=512 unroll=0 0.0610 ms │ +│ c1-0000000000000008 block_x=32 tile=512 unroll=2 0.0640 ms │ │ │ │ │ │ │ diff --git a/crates/launchbound-tui/tests/golden/rejections-80x24.txt b/crates/launchbound-tui/tests/golden/rejections-80x24.txt index ac6fe08..52fc6b0 100644 --- a/crates/launchbound-tui/tests/golden/rejections-80x24.txt +++ b/crates/launchbound-tui/tests/golden/rejections-80x24.txt @@ -4,22 +4,22 @@ launchbound — reduce-flip · gate cc 8.6 · measured · NVIDIA A10G ┌rejections────────────────────────────────────────────────────────────────────┐ │REFUSED BUT FASTER — a tuner without a convergence gate would hand you one: │ │ c1-000000000000000a block_x=64 tile=128 unroll=0 0.0210 ms — 1.90x faster │ -│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a 64-threa│ +│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a │ +│64-thread block (2 warps) at a block-wide barrier; safe only at one warp (<= │ +│32 threads) │ │ │ │all refused configurations: │ │ x c1-000000000000000a block_x=64 tile=128 unroll=0 │ -│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a 64-thr│ +│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a │ +│64-thread block (2 warps) at a block-wide barrier; safe only at one warp (<= │ +│32 threads) │ │ x c1-000000000000000b block_x=64 tile=128 unroll=2 │ -│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a 64-thr│ +│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a │ +│64-thread block (2 warps) at a block-wide barrier; safe only at one warp (<= │ +│32 threads) │ │ x c1-000000000000000c block_x=64 tile=128 unroll=4 │ -│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a 64-thr│ -│ x c1-000000000000000d block_x=64 tile=256 unroll=0 │ -│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a 64-thr│ -│ x c1-000000000000000e block_x=64 tile=256 unroll=2 │ -│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a 64-thr│ -│ x c1-000000000000000f block_x=64 tile=256 unroll=4 │ -│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a 64-thr│ -│ x c1-0000000000000010 block_x=64 tile=512 unroll=0 │ -│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a 64-thr│ +│ RC001 at src/lib.rs:33:13: divergence source `warp_id()` splits a │ +│64-thread block (2 warps) at a block-wide barrier; safe only at one warp (<= │ +│32 threads) │ └──────────────────────────────────────────────────────────────────────────────┘ 1 overview · 2 ranking · 3 rejections · 4 progress · j/k scroll · q quit diff --git a/crates/launchbound-tui/tests/tui.rs b/crates/launchbound-tui/tests/tui.rs index 1b2ade5..1ba885b 100644 --- a/crates/launchbound-tui/tests/tui.rs +++ b/crates/launchbound-tui/tests/tui.rs @@ -200,3 +200,66 @@ fn stress_100_runs_at_80x24() { quit(t, &format!("run {run}")); } } + +/// No shipped frame is cut mid-value or mid-word without an ellipsis. +/// +/// Three views had this defect and two were fixed one at a time: #24 took +/// the chosen line, and the ranking and rejection views kept it — the +/// ranking losing every closing bracket, so each interval read as a number +/// with no upper bound, and the rejections losing the clause that says what +/// to do about the refusal. A golden is a recording of shipped behaviour, +/// so the goldens are where the scan belongs. +/// +/// The rule is what a rendered field may *end* with at the panel border. A +/// digit, `,`, `[`, `(`, `=` or `-` there means the value continued and was +/// cut; a letter means a word was. An ellipsis is allowed: a marked +/// shortening is a choice, and an unmarked one is a bug. +#[test] +fn no_golden_line_is_cut_at_the_panel_border() { + let dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/golden"); + let mut checked = 0; + for entry in fs::read_dir(&dir).expect("tests/golden must exist") { + let path = entry.unwrap().path(); + let name = path.file_name().unwrap().to_string_lossy().into_owned(); + let text = fs::read_to_string(&path).unwrap(); + for (number, line) in text.lines().enumerate() { + // Only rows that reach a right border can be cut by one. + let Some(inner) = line.strip_suffix('│') else { + continue; + }; + let Some(inner) = inner.strip_prefix('│') else { + continue; + }; + let Some(last) = inner.chars().next_back() else { + continue; + }; + // A box-drawing row is the border itself, not content. + if inner.chars().all(|c| c == '─' || c == ' ') { + continue; + } + if last == '…' { + continue; + } + let cut = last.is_ascii_digit() || matches!(last, ',' | '[' | '(' | '=' | '-'); + assert!( + !cut, + "{name}:{}: a value is cut at the panel border (ends {last:?}):\n{line}", + number + 1 + ); + // A word cut mid-way. A field that legitimately ends in a letter + // (`ms`, a kernel name) is indistinguishable from a truncated + // one by the last character alone, so this only fires when the + // row is full to the border AND the last word is long enough to + // be a sentence rather than a unit. + let full = inner.chars().count() >= 76; + let tail = inner.split_whitespace().next_back().unwrap_or(""); + assert!( + !(full && last.is_alphabetic() && tail.len() > 6), + "{name}:{}: a word is cut at the panel border ({tail:?}):\n{line}", + number + 1 + ); + } + checked += 1; + } + assert!(checked > 0, "no goldens found in {dir:?}"); +} diff --git a/docs/LIMITATIONS.md b/docs/LIMITATIONS.md index d5f6e3f..10b0009 100644 --- a/docs/LIMITATIONS.md +++ b/docs/LIMITATIONS.md @@ -20,6 +20,35 @@ project's corpus reproduces. A launch-shape-dependent hazard from a source the classifier does not recognize would be admitted **with a caveat**, not refused. +## `--cc` is a convergence and capacity question, not a lowering one + +The gate answers two questions at a compute capability: does the launch +shape make a barrier or a collective non-convergent, and does the static +shared memory fit. It has **no view of instruction availability**, so a +kernel whose device code cannot be lowered for that part at all is admitted +without a word. + +The reported case: every float intrinsic in cuda-oxide's catalog at the +current pin (`ex2`, `lg2`, `rcp`, `tanh` approx variants) is `sm_80+`. A +crate using one of them with `needs_cc = "7.5"` in `kernel.toml` prunes to +`12 clean` at `--cc 7.5`, and only fails when something finally lowers it: + +``` +$ cargo oxide inspect --arch sm_75 +error: CUDA target sm_75 cannot lower generated intrinsic `ex2_approx_f32`; + requires sm_80 or newer +``` + +`needs_cc` is the author's claim and the gate takes it on trust. Since 2.1.0 +the verdict line says so, so "3 clean" no longer reads as "this kernel is +fine at cc 7.5". What would close the gap is a `cargo oxide build --arch +sm_XY` probe per candidate — which needs a toolkit, and `prune` is +deliberately the part a laptop can run. A static scan of the crate against +the catalog's `Available on sm_NN+` lines is the cheaper half and is not +built: it would need the catalog, which is the sibling checkout `prune` +exists not to require, and an embedded copy of it would go stale silently — +which is the failure mode this document is about. + ## The Metal path has no gate at all `reconverge` analyzes cuda-oxide kernels. No equivalent exists for MSL and From f3ddbdb927a2de1d8bcbda4989b48bcf007e2432 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Sat, 5 Sep 2026 21:18:07 +0700 Subject: [PATCH 3/5] test(tui): assert the wrapped reason survives a narrow terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A property of the rendered grid rather than of a string: the reason is wrapped across rows now, so the sentence exists only as a sequence of cells. Sixty columns is where wrapping has to do real work and no golden covers it — which is also how the test found that `ready` cannot be used at that width, because the footer is cut before it reaches `q quit`. Confirmed red with the wrap removed. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- crates/launchbound-tui/tests/tui.rs | 66 +++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/crates/launchbound-tui/tests/tui.rs b/crates/launchbound-tui/tests/tui.rs index 1ba885b..1c7c12a 100644 --- a/crates/launchbound-tui/tests/tui.rs +++ b/crates/launchbound-tui/tests/tui.rs @@ -263,3 +263,69 @@ fn no_golden_line_is_cut_at_the_panel_border() { } assert!(checked > 0, "no goldens found in {dir:?}"); } + +/// The refusal reason reaches the reader whole, at a width where it does not +/// fit on one row. +/// +/// This is a property of the rendered grid, which is why it is here and not +/// a string assertion: the reason is *wrapped* across rows now, so the +/// sentence exists only as a sequence of cells. At eighty columns the reader +/// used to get `splits a 64-threa` and never reach `safe only at one warp +/// (<= 32 threads)` — the only part that says what to do about the refusal — +/// with nothing marking the cut, so it read as the whole reason. +/// +/// Narrower than the goldens on purpose: 60 columns is where wrapping has to +/// do real work, and the golden suite has no frame there. +#[test] +fn a_refusal_reason_survives_a_narrow_terminal_whole() { + let mut t = spawn((60, 30)); + // NOT `ready`: that predicate looks for the footer's `q quit`, and at + // sixty columns the footer itself is cut before it reaches those words. + // A readiness marker has to hold at the width being tested, which is + // the sort of thing only a narrow-terminal test finds out. + t.wait_frame(|s| s.to_string().contains("candidates ·")) + .expect("the first complete frame"); + t.send(Key::Char('3')).expect("send 3"); + let frame = t + .wait_frame(|s| s.to_string().contains("all refused configurations:")) + .expect("the rejections view"); + + // Rebuild the panel's prose from the grid: wrapping breaks at spaces, so + // joining the rows and collapsing whitespace recovers the sentence. + let joined = frame + .to_string() + .lines() + .map(|line| line.trim_matches(['│', ' '])) + .collect::>() + .join(" "); + let prose: String = joined.split_whitespace().collect::>().join(" "); + + assert!( + prose.contains("safe only at one warp (<= 32 threads)"), + "the actionable clause must reach the reader at 60 columns:\n{frame}" + ); + assert!( + prose.contains("divergence source `warp_id()` splits a 64-thread block"), + "and so must the rest of the reason:\n{frame}" + ); + + // And nothing is cut at the border. The same rule the golden scan + // applies, asserted here against a live grid at a width no golden covers. + for row in 0..frame.rows() { + let text = frame.row_text(row); + let Some(inner) = text.strip_suffix('│').and_then(|t| t.strip_prefix('│')) else { + continue; + }; + if inner.chars().all(|c| c == '─' || c == ' ') { + continue; + } + if let Some(last) = inner.chars().next_back() { + assert!( + last == '…' || !(last.is_ascii_digit() || matches!(last, ',' | '[' | '(' | '=')), + "row {row} is cut at the border (ends {last:?}):\n{frame}" + ); + } + } + + quit(t, "narrow rejections"); +} From 7d7cc9cdff6a68ac2aec8215afbce842ffaedffe Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Sat, 5 Sep 2026 21:22:48 +0700 Subject: [PATCH 4/5] release: 2.1.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thirteen findings, all reported against 2.0.0 with a measured reproduction, and the lockstep pin bump to reconverge 0.5.0 that #46 and #17 were waiting on. Verified end to end against the *published* cargo-reconverge 0.5.0 rather than a local build: the two shapes that hard-stopped the gate — a kernel crate with a `src/main.rs`, and one with a `src/util/mod.rs` — now give `3 clean, 0 with caveats, 8 refused, 0 tool errors` at exit 0, identical to the control with neither. The whole corpus prunes unchanged, and the three gate tests pass, so the pin bump is behaviour-preserving where it should be. Also here: `prune`'s "what was and was not checked" note is printed once for the run rather than once per kernel — over a six-kernel corpus the per-kernel form was six repetitions of the same paragraph, which is the kind of user-facing noise #27 was about. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- AGENTS.md | 21 ++++- CHANGELOG.md | 131 +++++++++++++++++++++++++++++ Cargo.lock | 22 ++--- Cargo.toml | 2 +- README.md | 14 ++- action/action.yml | 2 +- crates/launchbound-cli/src/main.rs | 30 ++++--- 7 files changed, 193 insertions(+), 29 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fd61bfd..5efd03b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,8 +19,9 @@ is the full contributor document and wins wherever the two disagree. ## Build and test ```sh -just ci # fmt, clippy, test, deny, schemas +just ci # fmt, clippy, test, deny, schemas, pins cargo test --workspace # no GPU, no network, no checkout needed +just pins # the recorded pin sites agree — no network just gate # the gate tests — needs cargo-reconverge + cuda-oxide ``` @@ -33,7 +34,23 @@ MSRV 1.88 for everything that does not need it. gate and runs on any laptop; that is why it is its own verb. Do not write a test that needs silicon when the gate does not. - **Goldens:** regenerate with `LAUNCHBOUND_BLESS=1 cargo test -p launchbound-tui - --test tui`, then read every diff. + --test tui`, then read every diff. A golden is a recording of shipped + behaviour, so `no_golden_line_is_cut_at_the_panel_border` scans them all: + a value or a word ending at the panel border, without an ellipsis, is a + truncation bug. Three views had one and two were fixed separately before + that scan existed. +- **PTY tests follow the termlens skill**, vendored at + `.claude/skills/termlens/SKILL.md`: content-based waits only, never a + sleep; one predicate per instant, and one-directional, so a frame cannot + satisfy both waits of a resize; `(cols, rows)` for a size and + `(row, col)` for a cell. A readiness predicate has to hold at the width + under test — `ready` looks for the footer's `q quit`, which is cut at + sixty columns. +- **The pins move together or not at all**, and `just pins` checks that the + recorded sites agree before anything asks upstream. 2.0.0 moved four of + six, which left `pins.yml` measuring drift from a version nothing + installs — a watcher with a stale baseline produces noise that looks like + a finding. - **A model-derived ranking is never presented as a measurement.** Anything estimated says so on every surface it reaches. This is the project's central honesty claim — do not blur it to make output tidier. diff --git a/CHANGELOG.md b/CHANGELOG.md index 308937f..96927e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,137 @@ change measured timings are marked `bench:`. ## [Unreleased] +## [2.1.0] - 2026-09-05 + +Thirteen findings, all reported against 2.0.0 with a measured reproduction. +Most of them reduce to one shape: something was consumed on a contract other +than the one it is written to, and the mismatch was reported as a fault of +whatever it was pointed at. + +### Changed + +- **The lockstep pins move to reconverge 0.5.0**, at every recorded site. + 2.0.0 moved the gate to 0.4.0 and updated four of the six the policy + names, so `rust-toolchain.toml`, `CONTRIBUTING.md` and `pins.yml` still + recorded 0.1.11 — and `pins.yml` measures upstream drift against its own + `RECONVERGE_PIN`, so its weekly signal reported movement away from a + version nothing installs. That is why #17 sat open describing a pin two + releases old. `just pins` now asserts the sites agree with each other, + with no network, before anything asks upstream. Reported in #46 and #17. + +- **`apply --verify` is a real switch.** It was a `bool` with + `default_value_t = true`, which clap gives a `SetTrue` action — so + `--verify` set what was already set, `--no-verify` was an unknown + argument, and the help read as an opt-in for something mandatory. There + is a `--no-verify` now, and the help says verification is on. This is + what makes `apply` usable on a machine that has the run directory but not + the analyzer and the pinned toolchain. Reported in #36. + +- **`prune`'s verdict line says what it checked.** The gate answers + convergence and static shared-memory capacity at a `--cc`; it has no view + of instruction availability, so a crate using an `sm_80+` intrinsic under + `needs_cc = "7.5"` prunes to `12 clean` at `--cc 7.5` and fails only when + something finally lowers it for that part. `needs_cc` is the author's + claim and is taken on trust — defensible, and nowhere stated, so "clean" + read as "this kernel is fine at cc 7.5". docs/LIMITATIONS.md carries the + long form, including why the two stronger fixes were not built. Reported + in #32. + +### Fixed + +- **A kernel crate with a bin target no longer hard-stops the gate.** + reconverge prints one `findings.v1` document per analyzed target — its + documented contract — and this reader handed the whole of stdout to a + single `from_str`, so a `src/main.rs` beside a kernel library, the + ordinary shape of a GPU crate, made every candidate a tool error at + `trailing characters at line 2 column 1`. In the Action `fail-on` + defaults to `tool-error`, so CI went red for a crate with nothing wrong + with it, pointing at the analyzer's tracker. Stdout is read as JSONL and + the findings are unioned: a deny finding in any target refuses, and the + bin target's document is harmless to merge. The fail-safe always held — + it held against a format the analyzer documents. Reported in #42. + +- **The scratch copy is the whole crate.** It took only the entries of + `src/` that are files, so `mod util;` with `src/util/mod.rs` — how Rust + code is organised past one file — produced a scratch crate that could not + compile, and the gate reported `error: could not compile` against a crate + whose own `cargo check` is clean. The message told its author to fix + build errors they do not have, or to reinstall their toolchain, and never + said that what it compiled was not their crate. It copies recursively + now, carries `build.rs` or whatever `package.build` names, skips + `target/`, and a tool error names the scratch directory. Reported in #43. + +- **rustc's diagnostics survive the tool-error filter.** #19 replaced a + tail-six heuristic with a filter on marked lines, and picked the + *secondary* marker: rustc's primary diagnostics begin `error[E0583]:`, + a code before the colon. So what survived was cargo's summary and + reconverge's generic hint — "see the errors above", with the one that was + above removed. Both forms are accepted now, here and in the compile + executor, which still had the tail heuristic #19 removed next door. + Reported in #44. + +- **`--budget` cannot be given a value that fails to bound anything.** + `split_at(text.len() - 1)` on a trimmed-empty argument is `0usize - 1`, + so `--budget ""` panicked at exit 101. Worse: `NaN` and `1e400` parsed, + and the guard is `elapsed >= budget` — false for every value against NaN, + never true against infinity — so a value that looked accepted produced an + **unbounded** measured sweep on real silicon, which is the one failure a + budget exists to prevent. Non-finite and negative values are rejected, + the message names the flag and the accepted forms, and `min`/`hr` are + accepted alongside `m`/`h` because that is what people type. `--budget 0` + stays valid and means what it always did. Reported in #33. + +- **`apply` decides about verification before it prints anything.** It + emitted the `params.rs` and *then* verified, so "refusing to emit" + arrived after the emission and a reader who had piped stdout to a file + had the file. On a Metal run it could never succeed at all: that path has + no convergence gate, deliberately, so the run records `gate_cc: "metal"`, + and handing that sentinel to reconverge produced the correct answer to + the wrong question ("`metal` is not a compute capability") dressed as a + regression ("no longer passes the gate"). Nothing regressed; the gate + never ran and cannot. It refuses by name now, before stdout, and points + at `--no-verify` or `prune --cc `. `Verdict` has a `Display`, so + no user-facing message is a Rust struct literal. Reported in #34. + +- **A `results.json` the report cannot read is an error, not "unmeasured".** + A truncated file, an empty one, `null`, `[]`, a `results.v2` from a newer + runner and a *directory* named `results.json` all rendered as "nothing + measured yet": exit 0, nothing on stderr, and a JSON report that + validated against the schema. The run directory is the hand-off between + two machines and those two conditions call for opposite actions — wait, + or go and look. Only `NotFound` is `Ok(None)` now; everything else names + the path and the cause, the way `verdicts.v1` already did fifteen lines + away in the same function. `model --results` names the cause too, and + `tune`'s end-of-run report inherits all of it. Reported in #45. + +- **Nothing is cut mid-value or mid-word at the panel border.** #24 fixed + this on the chosen line and left it in the two views below: the ranking + lost every closing bracket at eighty columns, so each interval read as a + number with no upper bound, and the rejections view — the one the README + calls the point of the tool — lost the clause that says what to do, + stopping at `splits a 64-threa` and never reaching `safe only at one warp + (<= 32 threads)`. An interval is a field and is dropped whole through the + same helper the chosen line uses; a reason is a sentence and wraps. A + scan over every golden now fails on a value or a word ending at the + border without an ellipsis — confirmed red against the shipped pre-fix + frames, and it would have caught #24. Reported in #35. + +- **A missing `cargo oxide` is diagnosed by name.** cargo's own help + relayed `cargo search cargo-oxide`, and cargo-oxide is not on crates.io: + cuda-oxide is a pinned git checkout. The one actionable-looking line sent + the reader to a package that does not exist, on the first wall of `stage` + and `tune --backend cuda`. The message now names the pin, gives the three + commands CI uses, and says that `prune` needs none of it. The pin is a + constant `just pins` checks, so a bump moves the message with it. In the + same path, `exit Some(101)` is an exit code again and the compile failure + reports the compiler's errors rather than its summary. Reported in #37. + +- **No flag on `tune` is accepted and silently ignored.** `--budget`, + `--order` and `--seed` are inert with `--backend model`, and `--seed` is + inert with `--order exhaustive` on any backend. Each says so once, the + way `--out` has since #22 — somebody who passes `--budget 30m` reasonably + believes something is bounded. Reported in #38. + ## [2.0.0] - 2026-08-26 A major, and both reasons are in the "breaking" list this project keeps in diff --git a/Cargo.lock b/Cargo.lock index 7246cf1..b512204 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1121,7 +1121,7 @@ checksum = "bf36173d4167ed999940f804952e6b08197cae5ad5d572eb4db150ce8ad5d58f" [[package]] name = "launchbound-bench" -version = "2.0.0" +version = "2.1.0" dependencies = [ "anyhow", "launchbound-space", @@ -1135,7 +1135,7 @@ dependencies = [ [[package]] name = "launchbound-build" -version = "2.0.0" +version = "2.1.0" dependencies = [ "launchbound-space", "serde", @@ -1146,7 +1146,7 @@ dependencies = [ [[package]] name = "launchbound-cli" -version = "2.0.0" +version = "2.1.0" dependencies = [ "anyhow", "clap", @@ -1164,7 +1164,7 @@ dependencies = [ [[package]] name = "launchbound-metal" -version = "2.0.0" +version = "2.1.0" dependencies = [ "launchbound-bench", "launchbound-space", @@ -1175,7 +1175,7 @@ dependencies = [ [[package]] name = "launchbound-model" -version = "2.0.0" +version = "2.1.0" dependencies = [ "launchbound-space", "serde", @@ -1185,7 +1185,7 @@ dependencies = [ [[package]] name = "launchbound-prune" -version = "2.0.0" +version = "2.1.0" dependencies = [ "launchbound-build", "launchbound-space", @@ -1198,7 +1198,7 @@ dependencies = [ [[package]] name = "launchbound-report" -version = "2.0.0" +version = "2.1.0" dependencies = [ "insta", "jsonschema", @@ -1210,7 +1210,7 @@ dependencies = [ [[package]] name = "launchbound-runner" -version = "2.0.0" +version = "2.1.0" dependencies = [ "launchbound-bench", "launchbound-search", @@ -1218,7 +1218,7 @@ dependencies = [ [[package]] name = "launchbound-search" -version = "2.0.0" +version = "2.1.0" dependencies = [ "launchbound-bench", "proptest", @@ -1226,7 +1226,7 @@ dependencies = [ [[package]] name = "launchbound-space" -version = "2.0.0" +version = "2.1.0" dependencies = [ "proptest", "serde", @@ -1237,7 +1237,7 @@ dependencies = [ [[package]] name = "launchbound-tui" -version = "2.0.0" +version = "2.1.0" dependencies = [ "anyhow", "crossterm", diff --git a/Cargo.toml b/Cargo.toml index 11cb5f5..9d18737 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ members = [ exclude = ["corpus"] [workspace.package] -version = "2.0.0" +version = "2.1.0" edition = "2024" # MSRV for crates that do not require the pinned nightly (CONTRIBUTING.md); # set by ratatui 0.30. The analysis and compile paths require diff --git a/README.md b/README.md index 4fb9d15..6b66da4 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,7 @@ launchbound prune --cc 8.6 [--json] # reconverge pass only — launchbound model --cc 8.6 # analytical ranking — NOT GATED launchbound tune --cc 8.6 --backend cuda|metal|model [--budget 30m] launchbound report [--json] [--rejected] # includes refused-but-faster configs -launchbound apply # emit the cuda-oxide policy specialization +launchbound apply [--no-verify] # emit the cuda-oxide policy specialization launchbound-tui # the run in four views: the chosen # configuration and the field it beat, # the ranking, the refusals, the progress @@ -145,6 +145,18 @@ compute capability does not transfer to another, and `tune` is the command whose answer you act on. The CUDA spellings work: `--cc 86` and `--cc sm_86` mean `8.6`. +`prune --cc` answers two questions at that capability — does the launch +shape make a barrier or a collective non-convergent, and does the static +shared memory fit — and its verdict line says so. It has **no view of +instruction availability**: whether the device code can be lowered for that +part at all is `needs_cc` in `kernel.toml`, the author's claim, taken on +trust ([docs/LIMITATIONS.md](docs/LIMITATIONS.md)). + +`apply` re-verifies what it emits through the gate before printing anything. +`--no-verify` emits without it — for a machine that has the run directory +but not the analyzer, and for a Metal run, which has no convergence gate to +re-verify against — and the output carries a notice saying so. + `model` ranks the **whole** space and says so: it runs no gate and needs no `reconverge`, so its fastest row may be a configuration that hangs. `tune --backend model` is the gated form of the same ranking. diff --git a/action/action.yml b/action/action.yml index ee7114c..0a9c8a7 100644 --- a/action/action.yml +++ b/action/action.yml @@ -40,7 +40,7 @@ inputs: default: tool-error version: description: >- - Which launchbound release runs the gate, e.g. "2.0.0". The default, + Which launchbound release runs the gate, e.g. "2.1.0". The default, `latest`, installs the newest release on crates.io each run. Pin a number if you want the gate to change only when you say so. required: false diff --git a/crates/launchbound-cli/src/main.rs b/crates/launchbound-cli/src/main.rs index c01e6d2..c8e812b 100644 --- a/crates/launchbound-cli/src/main.rs +++ b/crates/launchbound-cli/src/main.rs @@ -1010,6 +1010,7 @@ fn cmd_prune( }; let mut tool_error = false; + let mut admitted_something = false; let mut json_out = Vec::new(); for dir in &dirs { let spec = KernelSpec::load(dir)?; @@ -1064,20 +1065,8 @@ fn cmd_prune( println!( " => {clean} clean, {caveats} with caveats, {refused} refused, {errors} tool errors" ); - // What "clean" means, on the line that says it. The gate answers - // the convergence question and the shared-memory one; it has no - // view of instruction availability, so a kernel using an - // `sm_80+` intrinsic under `needs_cc = "7.5"` is admitted at - // `--cc 7.5` and only fails when something finally lowers to PTX - // for that part. `needs_cc` is the author's claim and is taken - // on trust — which is defensible, and was nowhere stated. if clean > 0 || caveats > 0 { - println!( - " (checked: thread convergence and static shared memory at cc {cc}. \ - NOT checked: whether the device code can lower for that part — \ - instruction availability is `needs_cc` in kernel.toml, the author's \ - claim, taken on trust.)" - ); + admitted_something = true; } } if verdicts @@ -1089,6 +1078,21 @@ fn cmd_prune( } if json { println!("{}", serde_json::to_string_pretty(&json_out)?); + } else if admitted_something { + // What "clean" means, said once for the run rather than once per + // kernel. The gate answers the convergence question and the + // shared-memory one; it has no view of instruction availability, so + // a kernel using an `sm_80+` intrinsic under `needs_cc = "7.5"` is + // admitted at `--cc 7.5` and fails only when something finally + // lowers it for that part. `needs_cc` is the author's claim and is + // taken on trust — defensible, and nowhere stated, so "clean" read + // as "this kernel is fine at cc 7.5". + println!( + "\nchecked at cc {cc}: thread convergence, and static shared memory against \ + the cap.\nNOT checked: whether the device code can be lowered for that part \ + — instruction\navailability is `needs_cc` in kernel.toml, the author's claim, \ + taken on trust\n(docs/LIMITATIONS.md)." + ); } Ok(if tool_error { ExitCode::from(2) From e9ff1a7f481670d44dd27db12cdce54ec51016a0 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Sat, 5 Sep 2026 21:29:37 +0700 Subject: [PATCH 5/5] fix(ci): make the pin gate run on macOS, and actually check what it claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects in the gate I added, two of which it exists to prevent. `declare -A` and `local -n` are bash 4. macOS ships bash 3.2, where an associative array is silently an indexed one, so the script passed on Linux and died on the macOS runner with `rust: unbound variable` — the same defect as the GNU-only `sed -i` this project's sibling had in its own gate scripts. Rewritten with tab-separated lines and a pipeline, which is portable. `agree` runs in a pipeline and therefore a subshell, so setting `status=1` inside it would have been discarded: the gate would have printed the disagreement and exited 0. It returns a status now and the caller collects it. That is precisely the class of bug this gate is for. And the nightly was never compared at all. The action writes `default: nightly-2026-04-03` unquoted, the extractor required quotes, and the old code treated an unreadable site as nothing to check — so the one pin with only two recorded sites had neither of them verified. An extractor that silently matches nothing is a stale pin one level up, so an empty read is now a failure by name. Each of the three pin families verified red independently, with exit 1. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- scripts/check-pins.sh | 123 ++++++++++++++++++++++++++++-------------- 1 file changed, 82 insertions(+), 41 deletions(-) diff --git a/scripts/check-pins.sh b/scripts/check-pins.sh index cf06d83..8341c28 100755 --- a/scripts/check-pins.sh +++ b/scripts/check-pins.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# CI gate: the six recorded pin sites agree with each other. +# CI gate: the recorded pin sites agree with each other. # # The policy is "every site, or not at all" (CONTRIBUTING §9), and 2.0.0 did # four of six: it moved the gate to reconverge 0.4.0 in `action.yml`, @@ -15,6 +15,13 @@ # # This asks nothing of the network, so it runs in the ordinary CI job rather # than in the dispatch-only watch. +# +# **Portability: no `declare -A`, no `local -n`.** macOS ships bash 3.2, +# where an associative array is silently an indexed one — the first version +# of this script passed on Linux and died on macOS with +# `rust: unbound variable`. That is the same defect as the GNU-only `sed -i` +# in the sibling repository's gate scripts, in a gate written to prevent +# exactly this class of drift. set -euo pipefail ROOT=$(cd "$(dirname "$0")/.." && pwd) @@ -22,71 +29,105 @@ cd "$ROOT" status=0 +# Prints; the caller decides the exit status. `agree` runs in a pipeline and +# therefore in a subshell, so a `status=1` set inside it would be discarded +# — which would print the failure and exit 0, the precise shape of bug this +# gate exists to catch. fail() { echo "PIN DISAGREEMENT: $1" >&2 - status=1 } -# --- reconverge. Recorded with a leading `v` in two places and without it in -# four, so compare the bare version. -declare -A RECONVERGE=( - ["rust-toolchain.toml"]="$(sed -n 's/^# reconverge v\([0-9][0-9.]*\).*/\1/p' rust-toolchain.toml)" - ["CONTRIBUTING.md"]="$(sed -n 's/.*reconverge \([0-9][0-9.]*\) (installed from crates.io).*/\1/p' CONTRIBUTING.md)" - [".github/workflows/pins.yml"]="$(sed -n 's/^ RECONVERGE_PIN: v\([0-9][0-9.]*\).*/\1/p' .github/workflows/pins.yml)" - [".github/workflows/prune.yml"]="$(sed -n 's/^ RECONVERGE_VERSION: "\([0-9][0-9.]*\)".*/\1/p' .github/workflows/prune.yml)" - ["action/action.yml"]="$(sed -n '/^ reconverge-version:/,/^ [a-z]/s/^ default: "\([0-9][0-9.]*\)".*/\1/p' action/action.yml)" - ["action/README.md"]="$(sed -n 's/^| `reconverge-version` | `\([0-9][0-9.]*\)`.*/\1/p' action/README.md)" - ["docs/LIMITATIONS.md"]="$(sed -n 's/.*`reconverge` (v\([0-9][0-9.]*\)).*/\1/p' docs/LIMITATIONS.md)" -) +# One `` line per site. A tab separates them because a path +# cannot contain one and a version certainly cannot. +site() { + printf '%s\t%s\n' "$1" "$2" +} -# --- cuda-oxide, recorded as a full SHA in three places. -declare -A CUDA_OXIDE=( - ["rust-toolchain.toml"]="$(sed -n 's/^# cuda-oxide \([0-9a-f]\{40\}\).*/\1/p' rust-toolchain.toml)" - [".github/workflows/pins.yml"]="$(sed -n 's/^ CUDA_OXIDE_PIN: \([0-9a-f]\{40\}\).*/\1/p' .github/workflows/pins.yml)" - # The "cargo oxide is not installed" message quotes the pin at the reader - # and tells them to check that commit out. A message that names a stale - # commit is worse than the cargo help it replaced. - ["crates/launchbound-build/src/compile.rs"]="$(sed -n 's/^pub const CUDA_OXIDE_PIN: &str = "\([0-9a-f]\{40\}\)".*/\1/p' crates/launchbound-build/src/compile.rs)" -) +# reconverge. Two sites spell it with a leading `v` and the rest without, so +# the bare version is what is compared. +reconverge_sites() { + site "rust-toolchain.toml" \ + "$(sed -n 's/^# reconverge v\([0-9][0-9.]*\).*/\1/p' rust-toolchain.toml)" + site "CONTRIBUTING.md" \ + "$(sed -n 's/.*reconverge \([0-9][0-9.]*\) (installed from crates.io).*/\1/p' CONTRIBUTING.md)" + site ".github/workflows/pins.yml" \ + "$(sed -n 's/^ RECONVERGE_PIN: v\([0-9][0-9.]*\).*/\1/p' .github/workflows/pins.yml)" + site ".github/workflows/prune.yml" \ + "$(sed -n 's/^ RECONVERGE_VERSION: "\([0-9][0-9.]*\)".*/\1/p' .github/workflows/prune.yml)" + site "action/action.yml" \ + "$(sed -n '/^ reconverge-version:/,/^ [a-z]/s/^ default: "\{0,1\}\([0-9][0-9.]*\)"\{0,1\}.*/\1/p' action/action.yml)" + site "action/README.md" \ + "$(sed -n 's/^| `reconverge-version` | `\([0-9][0-9.]*\)`.*/\1/p' action/README.md)" + site "docs/LIMITATIONS.md" \ + "$(sed -n 's/.*`reconverge` (v\([0-9][0-9.]*\)).*/\1/p' docs/LIMITATIONS.md)" +} -# --- the nightly, which `rust-toolchain.toml` owns. -TOOLCHAIN=$(sed -n 's/^channel = "\(.*\)"/\1/p' rust-toolchain.toml) +# cuda-oxide, recorded as a full SHA. The third site is the "cargo oxide is +# not installed" message, which quotes the pin at the reader and tells them +# to check that commit out — a message naming a stale commit is worse than +# the cargo help it replaced. +cuda_oxide_sites() { + site "rust-toolchain.toml" \ + "$(sed -n 's/^# cuda-oxide \([0-9a-f]\{40\}\).*/\1/p' rust-toolchain.toml)" + site ".github/workflows/pins.yml" \ + "$(sed -n 's/^ CUDA_OXIDE_PIN: \([0-9a-f]\{40\}\).*/\1/p' .github/workflows/pins.yml)" + site "crates/launchbound-build/src/compile.rs" \ + "$(sed -n 's/^pub const CUDA_OXIDE_PIN: &str = "\([0-9a-f]\{40\}\)".*/\1/p' crates/launchbound-build/src/compile.rs)" +} +# The nightly, which `rust-toolchain.toml` owns and the action installs. +toolchain_sites() { + site "rust-toolchain.toml" "$(sed -n 's/^channel = "\(.*\)"/\1/p' rust-toolchain.toml)" + # The action quotes some defaults and not others, so the quotes are + # optional here. Requiring them made this site unreadable, and the first + # version of this script treated "unreadable" as "nothing to check" — so + # the nightly was never actually compared. An extractor that silently + # matches nothing is the same failure as a stale pin, one level up. + site "action/action.yml" \ + "$(sed -n '/^ toolchain:/,/^ [a-z]/s/^ default: "\{0,1\}\(nightly-[0-9-]*\)"\{0,1\}.*/\1/p' action/action.yml)" +} + +# Read `` lines on stdin; every value must match. agree() { - local -n table=$1 - local label=$2 expected="" site ok=1 - for site in "${!table[@]}"; do - local value="${table[$site]}" + label=$1 + expected="" + count=0 + ok=1 + tab=$(printf '\t') + while IFS="$tab" read -r site_name value; do + [ -z "$site_name" ] && continue + count=$((count + 1)) if [ -z "$value" ]; then - fail "$label: could not read a pin from $site (has its shape changed?)" + fail "$label: could not read a pin from $site_name (has its shape changed?)" ok=0 continue fi if [ -z "$expected" ]; then expected="$value" elif [ "$value" != "$expected" ]; then - fail "$label: $site records $value, but another site records $expected" + fail "$label: $site_name records $value, but another site records $expected" ok=0 fi done + if [ "$count" -eq 0 ]; then + fail "$label: no sites were read at all" + return 1 + fi # Only claim agreement when there was some: a summary line contradicting # the failure printed above it is the exact shape of bug this gate exists # to catch, and printing one here would be embarrassing. - if [ "$ok" -eq 1 ] && [ -n "$expected" ]; then - echo " $label $expected (${#table[@]} sites agree)" + if [ "$ok" -eq 1 ]; then + echo " $label $expected ($count sites agree)" + return 0 fi + return 1 } echo "lockstep pin set:" -agree RECONVERGE reconverge -agree CUDA_OXIDE cuda-oxide -echo " nightly $TOOLCHAIN" - -# The nightly is also named in the action, which installs it. -action_toolchain=$(sed -n '/^ toolchain:/,/^ [a-z]/s/^ default: "\(nightly-[0-9-]*\)".*/\1/p' action/action.yml) -if [ -n "$action_toolchain" ] && [ "$action_toolchain" != "$TOOLCHAIN" ]; then - fail "nightly: action/action.yml records $action_toolchain, rust-toolchain.toml records $TOOLCHAIN" -fi +# A pipeline's status is its last command's, which is `agree`. +reconverge_sites | agree reconverge || status=1 +cuda_oxide_sites | agree cuda-oxide || status=1 +toolchain_sites | agree nightly || status=1 if [ "$status" -eq 0 ]; then echo "every recorded pin site agrees"