fix(tests): a launch default disarms every test that asserts what it supplies
USER RULING (2026-09-20): the category sidebar is the launch default in the TUI as well as the browser. `improvise model.improv` opening with the sidebar showing is intended, not collateral. The behaviour is unchanged; what changes here is that it is now a stated, tested decision. Three tests asserted `category_panel_open` was TRUE after doing something to make it true. The default now supplies that, so all three passed without the code under them working. **`set_panel_open_and_cursor` is the serious one.** With it vacuous, `effect::SetPanelOpen`'s Category arm could be replaced by a no-op — `C` doing nothing in the terminal — and the whole workspace stayed green at 1391 tests. Now closes the panel first and asserts it closed, so the effect has to move the flag. **`panel_commands_address_the_named_panel_only` is the subtle one, and my first repair of it was ALSO vacuous.** Folding `[open:false, open:true]` and asserting the final state is true still passes against a reducer that folds nothing: the default already supplies `true`, so neither command needs to land. Only asserting the FALSE direction discriminates. Caught by injection reporting NONE failing after the "fix" — it would otherwise have shipped looking repaired. The rule this leaves, now in `context/code/clients.md`: **assert the direction the default cannot supply.** `tests/tui_launch_state.rs` gives the ruling a guard on the front end it newly changes — the TUI's launch frame had no test at all. Not a rendered frame: §7 says not to test `Widget::render` and the repo has no `TestBackend`, so the target is `App::new`. One of the three walks `Panel::ALL` comparing `App::new` against `ViewState::default()`, which is what would catch a TUI-only launch state — the divergence wsaw closed. **The view-row click test was vacuous for the property it is named for.** `Default` is index 0 and holds `view_panel_cursor`, so clicking it made `index` and the cursor the same number. Injecting the cursor-vs-index fault into `click_view_row` passed the contract suite 3/3 — the exact fault the ticket, the evidence and the Rust test exist for was invisible to the browser half. Now switches to Default by keyboard, then clicks Plan, and restores the two assertions the relocation had dropped. Inherited from the deleted `pointer.spec.js`, not introduced here. Filed rather than fixed: improvise-56ek (`SetPanelOpen` maps a Panel to a field twice, in two crates — each copy needed repairing independently, and the harness passing against a no-op'd effect is correct rather than a gap) and improvise-qf57 (the formula row's dropped `toHaveCount(0)`). improvise-4ibz is closed by the ruling. Every claim here proved by injection under `--no-fail-fast`, which is the method note: `cargo test --workspace` is fail-fast and understates blast radius. Gates: 1394 Rust tests, 26 + 3 Node, 16 Playwright (3 WebKit OPFS skips), clippy -D warnings, fmt, eight wasm crates, comment runs, 516 evidence entries. `.beads/` untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+23
-7
@@ -669,14 +669,30 @@ change to `SwitchViewAtCursor` was forced into the keyboard arm by
|
||||
`tests/lockstep.rs` and left the click behind **silently**. A helper that returns
|
||||
only the *name* rather than the whole activation is what invites that.
|
||||
|
||||
### The category panel opens at launch
|
||||
### The category panel opens at launch, in both front ends
|
||||
|
||||
`ViewState::default()` sets `category_panel_open: true` — the user's ruling, and
|
||||
it is the **shared** default rather than a browser one. A browser-only default
|
||||
would be the same defect as the bug improvise-wsaw fixed, wearing different
|
||||
clothes: "the page ignores the flag" and "the page overrides the flag" are one
|
||||
mistake. The consequence is that the **TUI** opens with the panel too, since
|
||||
`src/draw.rs` draws it off the same field.
|
||||
`ViewState::default()` sets `category_panel_open: true`. The user ruled this
|
||||
twice: first that the sidebar be visible at launch, then — asked whether the
|
||||
terminal was collateral — that it applies to **both**. `improvise model.improv`
|
||||
opening with the sidebar showing is intended, not a side effect of sharing a
|
||||
default. It is the **shared** default for the reason wsaw gives: a browser-only
|
||||
default would be "the page overrides the flag", which is the same defect as "the
|
||||
page ignores the flag" wearing different clothes.
|
||||
|
||||
**A launch default disarms every test that asserted the thing it now supplies**,
|
||||
and that is the expensive part. Three tests asserted `category_panel_open` was
|
||||
*true* after doing something to make it true; all three then passed without the
|
||||
code under them working. The worst was `set_panel_open_and_cursor`: with it
|
||||
vacuous, `effect::SetPanelOpen`'s Category arm could be replaced by a **no-op**
|
||||
— `C` doing nothing in the terminal — with the whole workspace green. The rule
|
||||
this leaves: **assert the direction the default cannot supply.** Closing the
|
||||
panel is the discriminating move; opening it proves nothing.
|
||||
|
||||
Note the two mappings that had to be repaired separately. `effect::SetPanelOpen`
|
||||
and `reduce_view`'s `Command::SetPanelOpen` are two three-way matches in two
|
||||
crates — the TUI folds one and the browser the other — so a guard on either says
|
||||
nothing about the other. That is a derivation done twice (§5), pre-existing and
|
||||
filed rather than fixed here.
|
||||
|
||||
Two tests, because either alone passes against the bug wsaw fixed: the panel is
|
||||
visible with no keypress, *and* `C` still hides it. Injecting "the page ignores
|
||||
|
||||
@@ -1255,18 +1255,35 @@ is `improvise_protocol::resolve`'s and tested there. The assertions were merged
|
||||
into that file's own click test rather than restored, because resurrecting a
|
||||
file a merge deleted is how a deliberate removal gets undone by reflex.
|
||||
|
||||
They press `Escape` after opening the panel, leaving it **open but unfocused**
|
||||
— the only state in which the *focus* third of the click is observable.
|
||||
They leave the panel **open but unfocused** — the only state in which the
|
||||
*focus* third of the click is observable.
|
||||
|
||||
Proved against the bundle built immediately before the fix: both fail there (the
|
||||
view row does not switch, the formula row does not select) while the
|
||||
pre-existing pointer assertions pass, so the failure is specific to the two
|
||||
panels rather than to the harness.
|
||||
**History, not current evidence:** the original pair was proved against the
|
||||
bundle built immediately before the fix, both failing there while the
|
||||
pre-existing pointer assertions passed. That run was against
|
||||
`harness/tests/pointer.spec.js`, which no longer exists. What holds the
|
||||
relocated assertions honest is the injection below, run in their new home.
|
||||
|
||||
What the view test asserts is deliberately three things — `#view-name`, the
|
||||
`active` marker and the `selected` cursor — because switching ends in `NORMAL`
|
||||
and a test that only checked the mode would pass against a click that did
|
||||
nothing but leave the panel.
|
||||
The view test asserts three things — `#view-name`, the `active` marker and the
|
||||
`selected` cursor — because switching ends in `NORMAL` and a test that only
|
||||
checked the mode would pass against a click that did nothing but leave the
|
||||
panel. **The relocation initially dropped two of the three**, and a reviewer
|
||||
caught it; they are restored.
|
||||
|
||||
**And the fixture was vacuous in a way the prose had already warned about.**
|
||||
`Default` is index 0 and holds `view_panel_cursor` by default, so clicking it —
|
||||
which both the old `pointer.spec.js` and the first relocated version did — makes
|
||||
`index` and the cursor the same number, and reading either gives the same name.
|
||||
A reviewer injected `switch_view_at(state, state.view.view_panel_cursor)` into
|
||||
`click_view_row` and **the whole contract suite passed 3/3**: the exact fault
|
||||
this test, the ticket and the Rust test all exist for was invisible to the
|
||||
browser half. The fixture now switches to `Default` by **keyboard**, leaving the
|
||||
cursor there, then **clicks `Plan`** — so the two readings name different rows
|
||||
and the outcomes differ. Re-running that injection now fails it.
|
||||
|
||||
The blindness was inherited rather than introduced: the deleted file clicked
|
||||
`Default` too. It became worth fixing because the relocated test is now the
|
||||
*only* browser assertion on that surface.
|
||||
|
||||
**One fact the reviewer's report supplied and this fix leans on**: shipping the
|
||||
cure for one panel and not its two siblings *reproduces the original report*. A
|
||||
@@ -1388,6 +1405,64 @@ does not mutate" would hold only for the first click of a session, since every
|
||||
click after the first is a "second click" in the loose reading. Clicking A then
|
||||
B must select B.
|
||||
|
||||
### the_terminal_app_launches_with_the_category_panel_open
|
||||
|
||||
The user ruled (2026-09-20) that the category sidebar is visible at launch in
|
||||
the **TUI as well as** the browser, so `improvise model.improv` opening with it
|
||||
showing is intended. This is the guard on the front end the ruling newly
|
||||
changes: `a_fresh_session_opens_with_the_category_panel` asserts
|
||||
`ViewState::default()` in the *protocol* crate, and nothing downstream of it.
|
||||
|
||||
Why not a rendered-frame test: design-principles §7 says not to test ratatui
|
||||
`Widget::render`, and there is no `TestBackend` anywhere in this repo. The
|
||||
honest target is the TUI's own constructor. `src/draw.rs`'s one-line read of the
|
||||
flag stays argued-not-tested, as gotcha 48 has it.
|
||||
|
||||
Three injections. Putting the shared default back to `false` fails this test and
|
||||
`a_fresh_session_opens_with_the_category_panel`. Making `App::new` override the
|
||||
default (`app.view_state.category_panel_open = false`) fails this test **and**
|
||||
`the_terminal_constructor_inherits_the_shared_default_rather_than_setting_it` —
|
||||
which is the one that would catch a TUI-only launch state being reintroduced, the
|
||||
divergence improvise-wsaw closed. `the_other_two_panels_are_shut_at_launch`
|
||||
stops "open everything at launch" passing the first.
|
||||
|
||||
### set_panel_open_and_cursor
|
||||
|
||||
**The launch default silently disarmed this test, and that is the finding.** It
|
||||
did `SetPanelOpen { Category, open: true }` then asserted the flag was true — but
|
||||
`test_app()` builds from `ViewState::default()`, where it is *already* true. Its
|
||||
Formula and View siblings still default to `false`, so only the category arm
|
||||
went vacuous.
|
||||
|
||||
What that cost, demonstrated: replacing `Panel::Category =>
|
||||
app.view_state.category_panel_open = self.open` with a **no-op** — so no effect
|
||||
could open or close the TUI's category sidebar at all — left the entire
|
||||
workspace green. `C` could be made to do nothing in the terminal with 1391 tests
|
||||
passing.
|
||||
|
||||
The fix is the fixture stating its own precondition: close the panel first and
|
||||
assert it closed, so the effect has to move the flag before the `true` direction
|
||||
is asserted. Injecting the no-op now fails this test.
|
||||
|
||||
Note the harness cannot see this one, and that is correct rather than a gap: the
|
||||
browser folds `Command::SetPanelOpen` through `reduce_view`, a *different*
|
||||
three-way match in a different crate from `effect::SetPanelOpen`'s. Two
|
||||
derivations of one mapping — which is why `panel_commands_address_the_named_panel_only`
|
||||
needed the same repair independently.
|
||||
|
||||
### panel_commands_address_the_named_panel_only
|
||||
|
||||
The `reduce_view` half of the one above, and **the first repair of it was still
|
||||
vacuous** — worth recording because the mistake is subtle. Folding
|
||||
`[open: false, open: true]` and asserting the final state is *true* passes
|
||||
against a reducer that folds nothing at all: the default is already true, so
|
||||
neither command needs to land. Only asserting the **`false`** direction
|
||||
discriminates, because that is the one the default cannot supply.
|
||||
|
||||
Caught by injection rather than by reading — the no-op fold reported NONE
|
||||
failing after the first fix. Now a separate `closed` fold asserts the flag
|
||||
moved, before the original assertions run.
|
||||
|
||||
### a_fresh_session_opens_with_the_category_panel
|
||||
|
||||
The user's ruling that the category panel is visible at launch, implemented as
|
||||
|
||||
@@ -2263,6 +2263,15 @@ mod tests {
|
||||
.apply(&mut app);
|
||||
assert_eq!(app.view_state.formula_cursor, 3);
|
||||
|
||||
// Closed first: the category panel is OPEN at launch, so asserting
|
||||
// `true` straight away would pass against an effect that does nothing.
|
||||
SetPanelOpen {
|
||||
panel: Panel::Category,
|
||||
open: false,
|
||||
}
|
||||
.apply(&mut app);
|
||||
assert!(!app.view_state.category_panel_open);
|
||||
|
||||
SetPanelOpen {
|
||||
panel: Panel::Category,
|
||||
open: true,
|
||||
|
||||
@@ -305,6 +305,14 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn panel_commands_address_the_named_panel_only() {
|
||||
// Closing it is what discriminates: the category panel is OPEN at
|
||||
// launch, so any assertion that it is open passes against a no-op.
|
||||
let closed = fold(&[Command::SetPanelOpen {
|
||||
panel: Panel::Category,
|
||||
open: false,
|
||||
}]);
|
||||
assert!(!closed.category_panel_open, "the fold moved the flag");
|
||||
|
||||
let view = fold(&[
|
||||
Command::SetPanelOpen {
|
||||
panel: Panel::Category,
|
||||
|
||||
@@ -170,15 +170,24 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
|
||||
]);
|
||||
await page.keyboard.press("Escape");
|
||||
|
||||
// A view row (improvise-iioi): the same three, with `switch-view-at-cursor`
|
||||
// as the activation. The SECOND row, so a click on the row already under
|
||||
// the cursor cannot pass by accident.
|
||||
// A view row (improvise-iioi). The activation must read the CLICKED index,
|
||||
// not `view_panel_cursor` — so the cursor must not already be on the row
|
||||
// clicked, or both readings give the same answer. `Default` is index 0 and
|
||||
// holds the cursor by default, so move to it and click `Plan`.
|
||||
await page.keyboard.press("Shift+V");
|
||||
await expect(page.locator("#views li.active")).toContainText("Plan");
|
||||
await page.keyboard.press("Escape");
|
||||
await page.locator("#views li").filter({ hasNotText: "Plan" }).first().click();
|
||||
await expect(page.locator("#views li.selected")).toContainText("Default");
|
||||
// Switch to Default by KEYBOARD, leaving the cursor on it. The click then
|
||||
// names a different row than the cursor does, which is the whole point.
|
||||
await page.keyboard.press("Enter");
|
||||
await expect(page.locator("#view-name")).toHaveText("Default");
|
||||
|
||||
const plan = page.locator("#views li").filter({ hasText: "Plan" }).first();
|
||||
await plan.click();
|
||||
// Reading the cursor instead of the index re-selects Default and fails.
|
||||
await expect(page.locator("#view-name")).toHaveText("Plan");
|
||||
await expect(page.locator("#views li.active")).toContainText("Plan");
|
||||
await expect(page.locator("#views li.selected")).toContainText("Plan");
|
||||
|
||||
// A formula row: navigation only, because FormulaPanel binds no Enter and
|
||||
// a click mirrors Enter. The Rust guard is on the KEYMAP — clients.md.
|
||||
await page.keyboard.press("Shift+F");
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
//! The terminal app's first screen carries a decision (improvise-4ibz).
|
||||
//! Why this is not a rendered-frame test: `context/test-evidence/tui.md`.
|
||||
|
||||
use improvise::ui::app::{App, Panel};
|
||||
use improvise::workbook::Workbook;
|
||||
|
||||
fn launched() -> App {
|
||||
App::new(Workbook::new("launch"), None)
|
||||
}
|
||||
|
||||
/// The user's ruling, 2026-09-20: the category sidebar is visible at launch in
|
||||
/// the TUI as well as the browser. `src/draw.rs` draws it off this flag.
|
||||
#[test]
|
||||
fn the_terminal_app_launches_with_the_category_panel_open() {
|
||||
let app = launched();
|
||||
assert!(
|
||||
app.view_state.category_panel_open,
|
||||
"improvise model.improv opens with the category sidebar showing"
|
||||
);
|
||||
}
|
||||
|
||||
/// The ruling names one panel. Without this, "open everything at launch" would
|
||||
/// pass the test above, and the sidebar would be three panels tall.
|
||||
#[test]
|
||||
fn the_other_two_panels_are_shut_at_launch() {
|
||||
let app = launched();
|
||||
assert!(!app.view_state.formula_panel_open, "formulas stay shut");
|
||||
assert!(!app.view_state.view_panel_open, "views stay shut");
|
||||
}
|
||||
|
||||
/// `App::new` is the TUI's own constructor, so it could override the shared
|
||||
/// default without the protocol-level test noticing. It must not.
|
||||
#[test]
|
||||
fn the_terminal_constructor_inherits_the_shared_default_rather_than_setting_it() {
|
||||
let shared = improvise::ui::app::ViewState::default();
|
||||
let app = launched();
|
||||
for panel in Panel::ALL {
|
||||
let (in_app, in_default) = (open(&app.view_state, panel), open(&shared, panel));
|
||||
assert_eq!(
|
||||
in_app,
|
||||
in_default,
|
||||
"{} differs between App::new and ViewState::default: a TUI-only \
|
||||
launch default is the divergence improvise-wsaw closed",
|
||||
panel.name()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn open(view: &improvise::ui::app::ViewState, panel: Panel) -> bool {
|
||||
match panel {
|
||||
Panel::Formula => view.formula_panel_open,
|
||||
Panel::Category => view.category_panel_open,
|
||||
Panel::View => view.view_panel_open,
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user