From a47bf5ed3a2dedb1162c45266201fe0fd0f58caa Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:12:46 +0700 Subject: [PATCH 1/2] test(tui): assert nothing drawn depends on colour, and that NO_COLOR proves it #58 was filed expecting a colour-only cue to vanish under NO_COLOR. Two things measured before writing any of it turned out otherwise. **The geometry half of the issue was already done.** It claimed the suite runs at one geometry, `size(110, 32)`. On main it runs nine tests at 80x24, two at 60x30, and reaches 110x32 by live resize, plus `a_refusal_reason_survives_a_narrow_terminal_whole`. #49 changed that after I measured. Corrected on the issue; nothing to do here. **The TUI emits no colour at all.** Every style in `app.rs` is `Modifier::BOLD` or `BOLD | REVERSED`; `Color::` appears nowhere in the crate, and the `styles:` block of the shipped golden names only `bold` and `reverse`. Bold and reverse are SGR attributes, not colour, and NO_COLOR does not ask anyone to drop them. So there was no cue to lose. That makes the honest test the stronger claim rather than the one asked for: - `no_view_of_the_tui_uses_colour` walks every cell of all four views at 80x24 and asserts `fg` and `bg` are the terminal default -- structurally, through `Cell::style()`, not by matching rendered strings. This is the guard that matters: the day someone marks a refusal red, it fires and names the cells, and they have to add a cue that survives without it. Verified by colouring the field heading: "overview: 24 cell(s) carry colour", listing the first eight. - `the_metal_banner_keeps_its_emphasis_under_no_color` re-runs the existing banner assertion under `NO_COLOR=1`, cell by cell across the whole banner. It is the cue the issue named and the one most likely to be reached for with colour later. - `no_color_changes_not_one_cell` compares styled frames with and without the variable and requires them byte-identical. If that ever fails, either colour was added or the app grew a NO_COLOR branch that changes layout -- and a layout depending on an environment variable needs its own golden. `spawn_in_with_env` keeps `env_clear`, so the child sees only what a test names. Sync policy unchanged: one predicate per instant, on view-body content rather than the footer, which names every view on every view. Closes #58 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- crates/launchbound-tui/tests/tui.rs | 148 ++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/crates/launchbound-tui/tests/tui.rs b/crates/launchbound-tui/tests/tui.rs index 348d4d4..9bd10a4 100644 --- a/crates/launchbound-tui/tests/tui.rs +++ b/crates/launchbound-tui/tests/tui.rs @@ -125,6 +125,43 @@ fn spawn(size: (u16, u16)) -> Terminal { spawn_in(fixture_run(), size) } +/// `spawn_in` with extra environment. The base is still `env_clear`, so the +/// only variables the child sees are the ones named here. +fn spawn_in_with_env(run_dir: PathBuf, size: (u16, u16), env: &[(&str, &str)]) -> Terminal { + let mut builder = Terminal::builder() + .size(size.0, size.1) + .env_clear() + .timeout(TIMEOUT) + .arg(run_dir); + for (k, v) in env { + builder = builder.env(k, v); + } + let mut t = builder + .spawn(env!("CARGO_BIN_EXE_launchbound-tui")) + .expect("failed to spawn the TUI in a PTY"); + t.wait_until(|s| s.to_string().contains("launchbound")) + .expect("first frame"); + t +} + +/// Every cell whose foreground or background is not the terminal default, +/// as `row:col fg=.. bg=..`. +fn coloured_cells(screen: &termlens::Screen) -> Vec { + let mut out = Vec::new(); + for row in 0..screen.rows() { + for col in 0..screen.cols() { + let Some(cell) = screen.cell(row, col) else { + continue; + }; + let style = cell.style(); + if style.fg != termlens::Color::Default || style.bg != termlens::Color::Default { + out.push(format!("{row}:{col} fg={:?} bg={:?}", style.fg, style.bg)); + } + } + } + out +} + fn quit(mut t: Terminal, context: &str) { t.send(Key::Char('q')).expect("send q"); let status = t.wait_exit().expect("TUI did not exit after q"); @@ -440,6 +477,117 @@ fn a_refusal_reason_survives_a_narrow_terminal_whole() { /// /// The fixture is `runs/reduce-stable-metal` copied verbatim into the crate /// (see `fixture`). +/// **Nothing this TUI draws depends on colour**, in any view, so a reader on +/// a monochrome terminal loses nothing. +/// +/// #58 was filed expecting the opposite — that a colour-only cue would +/// vanish under `NO_COLOR`. Measuring first says there is no such cue to +/// lose: every style in `app.rs` is `Modifier::BOLD` or +/// `BOLD | REVERSED`, and `Color::` appears nowhere in the crate. Bold and +/// reverse are SGR attributes, not colour, and `NO_COLOR` does not ask +/// anyone to drop them. +/// +/// So this asserts the stronger and truer claim, structurally rather than by +/// matching strings. It is the guard that matters: the day someone marks a +/// refusal red, this fires and they have to add a cue that survives without +/// it. +#[test] +fn no_view_of_the_tui_uses_colour() { + let mut t = spawn((80, 24)); + t.wait_frame(ready).expect("the first complete frame"); + // One predicate per instant, and body content rather than the footer: + // the help line names every view on every view. + for (key, view, marker) in [ + (Key::Char('2'), "ranking", "ranking ("), + (Key::Char('3'), "rejections", "all refused configurations:"), + (Key::Char('4'), "progress", "measured 11 of"), + (Key::Char('1'), "overview", "the field, fastest first"), + ] { + t.send(key).expect("switch view"); + let screen = t + .wait_frame(|s| s.to_string().contains(marker)) + .unwrap_or_else(|e| panic!("{view}: waiting for the view body: {e}")); + let coloured = coloured_cells(&screen); + assert!( + coloured.is_empty(), + "{view}: {} cell(s) carry colour, which a NO_COLOR reader would lose; \ + add a cue that survives without it (bold, reverse, or a glyph): {:?}", + coloured.len(), + &coloured[..coloured.len().min(8)] + ); + } + quit(t, "no_view_of_the_tui_uses_colour"); +} + +/// The metal banner is the one cue #58 named, and the one most likely to be +/// reached for with colour later. It carries its emphasis through +/// `NO_COLOR=1` because the emphasis was never colour. +#[test] +fn the_metal_banner_keeps_its_emphasis_under_no_color() { + const BANNER: &str = + "NO convergence gate exists on the Metal path: the same bug class is NOT checked"; + + let mut t = spawn_in_with_env(fixture("run-metal"), (80, 24), &[("NO_COLOR", "1")]); + let frame = t.wait_frame(ready).expect("the first complete frame"); + + let at = frame + .find(BANNER) + .unwrap_or_else(|| panic!("the no-gate banner is missing under NO_COLOR:\n{frame}")); + assert_eq!( + at, + (1, 0), + "the banner is still the second line of the header" + ); + + // Every cell of it, exactly as the non-NO_COLOR test checks: an + // emphasis that survives halfway is the failure worth catching. + for col in 0..BANNER.chars().count() as u16 { + let style = frame + .cell(1, col) + .unwrap_or_else(|| panic!("cell (1, {col}) is off the grid")) + .style(); + assert!( + style.bold && style.reverse, + "banner cell (1, {col}) {style:?} lost its emphasis under NO_COLOR \ + — the notice can be read past:\n{frame}" + ); + } + assert!( + coloured_cells(&frame).is_empty(), + "nothing is coloured under NO_COLOR either" + ); + quit(t, "the_metal_banner_keeps_its_emphasis_under_no_color"); +} + +/// `NO_COLOR=1` produces a byte-identical frame, because there was no colour +/// to suppress. If that ever stops being true, one of two things happened: +/// colour was added (see `no_view_of_the_tui_uses_colour`), or the app grew +/// a `NO_COLOR` branch that changes the layout — and a layout that depends +/// on an environment variable needs its own golden, not this assertion. +#[test] +fn no_color_changes_not_one_cell() { + let mut plain = spawn_in_with_env(fixture_run(), (80, 24), &[]); + let a = plain + .wait_frame(ready) + .expect("a whole frame without NO_COLOR") + .with_styles() + .to_string(); + quit(plain, "no_color_changes_not_one_cell (plain)"); + + let mut flagged = spawn_in_with_env(fixture_run(), (80, 24), &[("NO_COLOR", "1")]); + let b = flagged + .wait_frame(ready) + .expect("a whole frame with NO_COLOR") + .with_styles() + .to_string(); + quit(flagged, "no_color_changes_not_one_cell (NO_COLOR)"); + + assert_eq!( + a, b, + "NO_COLOR changed the frame; it should have nothing to change" + ); +} + #[test] fn the_metal_banner_is_bold_and_reversed_and_nothing_else_is() { const BANNER: &str = From 97677c9b5725edce3e8b186bccb1f3372e5971f6 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:14:22 +0700 Subject: [PATCH 2/2] docs: say that launch_bounds and register budgeting are not validated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The README says what launchbound does and LIMITATIONS is honest about the gate's inheritance, but one question went unanswered anywhere: does this validate `#[launch_bounds]` against register limits? It does not. It never reads a register count at all, and it does not check `#[launch_contract]` against grid limits either. The confusion is earned rather than hypothetical. The corpus *narrates* register pressure without checking it -- `stencil-1d/kernel.toml` opens with "the tuning story is UNROLL x RADIUS x launch_bounds against the register file" -- `lb_max` is a real tuning dimension in two kernels, `.maxntid` is emitted by cuda-oxide, and the tool is called *launchbound*. A reader arriving from cuda-oxide's docs, where `#[launch_bounds]` is a register-budgeting tool, will assume otherwise. Sharper than the issue put it: the one `.maxntid` relationship the corpus enforces, `exprs = ["block_x <= lb_max"]` in `stencil-1d`, holds because the kernel author wrote it as a constraint and the evaluator does what it is told. launchbound attaches no meaning to `lb_max`. Omit that expression and nothing catches a block larger than its own `.maxntid`. A "Not in scope" paragraph in README §"What it is, and is not", a matching LIMITATIONS section, and each links to the other -- both anchors checked against GitHub's slug rules rather than eyeballed, since a broken link in the sentence that says "we do not do this" is worse than no sentence. What would close the gap is named, so the next reader does not have to work it out: the PTX from `cargo oxide inspect` already carries `.maxntid` and register counts. A rule reading them is a new rule with its own measured result and its own calibration entry -- not a documentation change. Closes #59 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- README.md | 14 +++++++++++++- docs/LIMITATIONS.md | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 95edef8..ee31087 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,17 @@ gap is this product. **It is:** the autotuner for Rust GPU kernels that treats a convergence-unsafe configuration as disqualified rather than fast. +**Not in scope: `#[launch_bounds]` and register budgeting are not +validated.** `lb_max` is a tuning dimension in this corpus and `.maxntid` is +emitted by cuda-oxide, but launchbound never reads a register count, so it +cannot tell you a configuration will spill or fail to achieve its requested +occupancy — and it does not check `#[launch_contract]` against grid limits +either. The one `.maxntid` relationship the corpus does enforce +(`block_x <= lb_max` in `stencil-1d`) holds because the kernel author wrote +it as a constraint, not because launchbound knows what `.maxntid` means. A +reader arriving from cuda-oxide's docs will assume otherwise; see +[docs/LIMITATIONS.md](docs/LIMITATIONS.md#launch_bounds-and-registers-are-not-validated). + ## The pipeline ```mermaid @@ -197,7 +208,8 @@ The honest list lives in [docs/LIMITATIONS.md](docs/LIMITATIONS.md) — read it before trusting a result. Highlights: a clean gate is **not a proof of correctness** (reconverge's documented limits are inherited wholesale, and the launch-shape classifier recognizes the measured `warp_id()` family); -the Metal path has **no gate at all**; model output is an estimate carrying +the Metal path has **no gate at all**; `#[launch_bounds]` and register +pressure are **not validated**; model output is an estimate carrying its measured per-kernel Spearman correlation (0.00–0.94 on this corpus, see model-calibration.toml); results are valid only for the recorded GPU, driver, and compiler and do not port between `sm_75` and `sm_86`; and diff --git a/docs/LIMITATIONS.md b/docs/LIMITATIONS.md index 95bb0b1..5ef235f 100644 --- a/docs/LIMITATIONS.md +++ b/docs/LIMITATIONS.md @@ -85,6 +85,39 @@ entry are reported as UNCALIBRATED. Every estimate carries the `estimated` label and this correlation; an estimate presented as a measurement is a release-blocking defect. +## `launch_bounds` and registers are not validated + +launchbound never reads a register count. It does not know how many +registers a candidate uses, so it cannot tell you that one will spill to +local memory, or that a `#[launch_bounds(N)]` request will fail to achieve +the occupancy it asks for. It does not check `#[launch_contract]` against +grid limits either. + +This matters because the corpus *narrates* register pressure without +checking it: `stencil-1d/kernel.toml` opens with "the tuning story is +UNROLL × RADIUS × launch_bounds against the register file", and `lb_max` is +a real tuning dimension in two kernels. A reader arriving from cuda-oxide's +documentation, where `#[launch_bounds]` is a register-budgeting tool, will +reasonably assume the autotuner named *launchbound* validates it. It does +not, and the name does not help. + +The one `.maxntid` relationship the corpus enforces — +`exprs = ["block_x <= lb_max"]` in `stencil-1d` — holds because the kernel +author wrote it as a constraint and the constraint evaluator does what it +is told. launchbound attaches no meaning to `lb_max`; omit the expression +and nothing catches a block larger than its own `.maxntid`. The occupancy +model reads `block_threads` and shared memory, and nothing else about the +launch. + +What would close the gap: the PTX is already available from +`cargo oxide inspect`, and it carries `.maxntid` and register counts. A +rule reading them would be a new rule — with its own measured result, its +own calibration entry, and its own row in this file. It is not a +documentation change. + +The README states the same boundary in its +["What it is, and is not"](../README.md#what-it-is-and-is-not) section. + ## The model's device table is narrower than the gate's `DEVICES` (`launchbound-model`) carries capacity figures for **7.5, 8.0,