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/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/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..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", @@ -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/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/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..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 @@ -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-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 3dd3b7e..c8e812b 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 @@ -155,24 +160,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 +336,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 +392,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 +426,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 +482,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 +515,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 +577,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 +588,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 +733,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 { @@ -867,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)?; @@ -921,6 +1065,9 @@ fn cmd_prune( println!( " => {clean} clean, {caveats} with caveats, {refused} refused, {errors} tool errors" ); + if clean > 0 || caveats > 0 { + admitted_something = true; + } } if verdicts .iter() @@ -931,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) @@ -1046,3 +1208,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..9487e6c 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,197 @@ 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 + } +} + +#[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 f157b3f..ba8fdd8 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"); @@ -173,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/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-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/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/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..1c7c12a 100644 --- a/crates/launchbound-tui/tests/tui.rs +++ b/crates/launchbound-tui/tests/tui.rs @@ -200,3 +200,132 @@ 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:?}"); +} + +/// 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"); +} diff --git a/docs/LIMITATIONS.md b/docs/LIMITATIONS.md index 7d54b18..10b0009 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 @@ -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 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..8341c28 --- /dev/null +++ b/scripts/check-pins.sh @@ -0,0 +1,135 @@ +#!/usr/bin/env bash +# 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`, +# `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. +# +# **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) +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 +} + +# 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" +} + +# 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)" +} + +# 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() { + 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_name (has its shape changed?)" + ok=0 + continue + fi + if [ -z "$expected" ]; then + expected="$value" + elif [ "$value" != "$expected" ]; then + 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 ]; then + echo " $label $expected ($count sites agree)" + return 0 + fi + return 1 +} + +echo "lockstep pin set:" +# 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" +fi +exit "$status"