fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #60 from fiddlerwoaroof/fix/web-panel-clicks

Browse Source 
fix(web): a click is all of its intents, never mutates the document, and the panel opens at launch
This commit is contained in:
Ed Langley
2026-09-20 14:48:11 -07:00
committed by GitHub
parent 74ccd6f030 ec03f950b3
commit 01ad91a033
12 changed files with 1087 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/code/clients.md
+119 -6
View File
@@ -552,12 +552,16 @@ gotcha 47 asks for — closing e24v has to come to that line and delete it.
`text`**: the drawn text has been through `format_f64`, so a stored 1200 reads
`"1,200"` under a comma-grouping format and committing it would store the
*string* `"1,200"`. `tests/lockstep.rs` found exactly that.
- **Drill does not go upstream.** Every `Upstream` is a black hole, and a key
that visibly does nothing is the worst of the three answers. Drill is
session-local residue and reaching it needs a `CellKey` this client cannot
build, so the honest answer is to say so: a `SetStatus`, which is a view-half
command — `Sessions::apply` folds it into the asker's own mirror and
broadcasts nothing, so it reaches this user and no one else.
- **Drill does not go upstream — by *either* door.** Every `Upstream` is a black
hole, and a key that visibly does nothing is the worst of the three answers.
Drill is session-local residue and reaching it needs a `CellKey` this client
cannot build, so the honest answer is to say so: a `SetStatus`, which is a
view-half command — `Sessions::apply` folds it into the asker's own mirror and
broadcasts nothing, so it reaches this user and no one else. This argument
arrived as `edit_or_drill`'s, i.e. about `i`/`a` on an aggregate, and for a
while it read as doctrine for `>` while `>` was still silent. Both doors now
refuse, and **they refuse with different sentences on purpose**: "this key is
not built yet" and "this cell is not yours to write" are different facts.
- **The commit's write is not spelled again here.** It is
`Client::typed_cell`'s, the same one click-to-edit uses, so a browser keyboard
commit and a browser mouse commit cannot disagree about what `""` or `"42"`
@@ -587,6 +591,115 @@ applied it (`"A"`, not shift+`"a"`), and the shared keymap's `lookup_local`
retries character keys without modifiers for that very reason, so keeping the
flag would only make an exact binding miss.
### The pointer: `Click`, and the four rules it obeys
A click is resolved here for the same reason a keypress is — a front end paints
and wires, it does not decide (design-principles §5). `resolve::click` takes a
`Click` naming a widget and an index, and returns the `Command`s. `web/app.js`
attaches listeners and knows nothing else; there is one entry point per widget
on `Client`, which was the user's ruling on improvise-7ybz.
**Rule 1 — a click navigates to what was clicked, and activates it where
activation is something the client can express.** Every arm composes the same
shape: `focus` the panel, `SetPanelCursor` to the row, then whatever `Enter`
does on that row. The activation is resolved **against the clicked index, never
against the cursor**, because the `SetPanelCursor` ahead of it has not been
folded when the resolver runs — the trap that reads as "the cursor lands on
`Plan` and the document switches to `Default`", silent and wrong in the half a
user does not watch.
The second clause of rule 1 is what makes the formula panel look inconsistent
and is in fact the rule holding. `ModeKey::ViewPanel` binds `Enter` to
`switch-view-at-cursor`; `ModeKey::FormulaPanel` binds `a`/`n`/`o` and
`d`/`Delete` and **no `Enter` at all**. So a view-row click switches and a
formula-row click only navigates. Opening the editor on a formula click would
be an activation *invented for the pointer that the keyboard does not have* —
a second derivation of what a formula row means, which is the disease §5 names.
**The guard for this must be aimed at the keymap, not at the click**: asserting
that the click emits no `FormulaEdit` only restates the author's reading, and a
reviewer demonstrated the gap by adding `Enter -> enter-mode formula-edit` to
`FormulaPanel` and watching the whole workspace stay green while the click and
the keyboard disagreed. `the_formula_panel_has_no_enter_to_mirror_and_the_view_panel_does`
asserts the binding's absence and names `click_formula_row` as what must change
if it ever appears.
**Rule 2 — a click is all of its intents or none of them** (improvise-hkr8).
Each `click_*` returns `Option<Vec<Command>>` and `click` does the single
`unwrap_or_default`. The case that matters is an activation the client cannot
compose: `filter_to_item_at` goes through `model`, which goes through
`BasisCache::claim`, which returns `None` until the server has published a basis
(gotcha 43). Before the fix the `None` was swallowed per-intent, so the click
kept its navigation prefix, dropped its activation, and reported success — while
the *keyboard* handled the identical `None` correctly by answering
`KeyOutcome::Upstream`. Note that `None` at the `click` level means something
different and benign: the whole click composed nothing, which is the invariant
rather than a breach of it.
**Rule 3 — a click on an *unselected* tile never mutates the document.** The
user's ruling on the tile bar. The cost accepted is two clicks for the common
action and a **deliberate divergence from the keyboard**, where `Enter` cycles
directly — record it as chosen, not as an inconsistency to tidy, or someone will
read the binding and "fix" the pointer to match. What it buys is worth more than
the keystroke: a tile cycle is a `LayoutGeometry` change, which bumps
`cursor_epoch` and therefore resets **every connected client's cursor** (gotcha
24), so a stray click used to cost every other viewer their position.
**State the rule this precisely, because the looser wordings are false.** The
implemented rule is "clicking the already-selected tile cycles it; clicking any
other selects it", where *already selected* is `mode == TileSelect` **and**
`tile_cat_idx == index`. The tempting summaries — "a click never mutates on its
own", "a **second** click cycles" — both have the same counterexample, found on
review: `T` enters `TileSelect` leaving `tile_cat_idx` at **0**, so a click on
the **leftmost** tile is a click on the already-selected one and cycles on the
*first* gesture. Two adjacent tiles therefore answer the identical first click
differently, and that is correct rather than a hole — a click does what `Enter`
does there, and `Enter` in `TileSelect` cycles the tile under the cursor. The
resolver cannot see *how* the state arose and must not try to.
The mode conjunct is still load-bearing in the other direction: without it,
`tile_cat_idx == 0` in **Normal** mode would read tile 0 as already selected, and
the very first click of a session on the leftmost tile would cycle it.
**Rule 4 — the shared helper is the idiom, and breaking it is how the two arms
drift.** `click_category_row` calls `filter_to_item_at`, `click_tile` calls
`tile_axis_at`, `click_view_row` calls `switch_view_at` — each is `foo_at(index)`
with the keyboard's `foo_at_cursor` calling `foo_at(cursor)`. `click_view_row`
originally re-spelled `SwitchView + ChangeMode(Normal)` inline, so a host-side
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, in both front ends
`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
the flag" fails the second; injecting "the default is false" fails the first.
A test that only asserts the sidebar is visible cannot tell the flag being
honoured from the flag being ignored.
---
## `improvise-wasm-client` — the crate, the fold, the render model
context/test-evidence/clients.md
+440 -13
View File
@@ -920,6 +920,11 @@ at all would pass by moving neither.
#### harness/tests/widgets.spec.js — the file's own scope
**Both files were deleted by PR #61**, which moved the pointer's and the
widgets' browser halves into `harness/tests/contract.spec.js` and the rest into
`node --test web/tests`. What follows is the history of how these assertions
were arrived at; the assertions themselves now live there.
**The file is gone; its subject is not.** All four widgets it named were one
failure at one boundary — a `RenderModel` field with no element painting it —
and that is now asserted once, from both sides, by
@@ -1112,6 +1117,11 @@ this one is only meaningful *because* its neighbours assert non-empty answers.
#### harness/tests/pointer.spec.js — the file's own scope
**Both files were deleted by PR #61**, which moved the pointer's and the
widgets' browser halves into `harness/tests/contract.spec.js` and the rest into
`node --test web/tests`. What follows is the history of how these assertions
were arrived at; the assertions themselves now live there.
**The file is gone; one test carries it.** `contract.spec.js`'s "each clickable
surface is wired to its own index" keeps what only a browser can say — the
handler is on the element, it passes *that* element's index, and the answer is
@@ -1173,14 +1183,22 @@ addressed to precisely one. That is the same argument that removed
`ModelCommand::Drill` would reintroduce the bug that removal fixed. It needs the
**addressed** form — ADR 001 layer 2.
**The two entry points fail differently, and that asymmetry is the finding.**
`i`/`a` on an aggregate declines **out loud**: `edit_or_drill` answers the
`CellId::Aggregate` arm with a status the user can read. But `>`
(`drill-into-cell`) is a name `resolve::step` does not handle, so it becomes
**The two entry points used to fail differently, and that asymmetry was the
finding.** `i`/`a` on an aggregate declines **out loud**: `edit_or_drill`
answers the `CellId::Aggregate` arm with a status the user can read. But `>`
(`drill-into-cell`) was a name `resolve::step` did not handle, so it became
`KeyOutcome::Upstream` → `ClientFrame::Key` → `Host::handle`, whose `Key` arm is
an explicit empty vec — **a black hole, not a slow path** (gotcha 47). Pressing
`>` in the browser does nothing whatsoever: no status, no mode change, nothing.
That is why this reads as "never confirmed working" rather than as a refusal.
`>` in the browser did nothing whatsoever: no status, no mode change, nothing.
That is why this read as "never confirmed working" rather than as a refusal.
**That silence is now closed, and the closing is what this file ships.** `step`
answers `"drill-into-cell"` with a `SetStatus` saying drill is not available in
the browser yet — an interim the wording of which is load-bearing, since it must
be true both today and on the day improvise-gd28 lands. The wire vocabulary is
still absent; only the *silence* was fixed. Note for gd28: when the real drill
arrives, a leftover `"drill-into-cell"` arm in `step` would **shadow** it with
nothing failing, because a name that resolves never reaches the host.
**The TUI is the control and it works**: 12 drill tests pass across
`cmd/grid.rs`, `cmd/mode.rs`, `ui/effect.rs` and `ui/app.rs`. So this is
@@ -1196,20 +1214,429 @@ becomes an aggregate. The precondition is then **asserted** (`td.editable`
count 0) rather than assumed, so neither test can pass against a grid of
Records.
**Both injections, run.** Replacing the aggregate arm with `None` — silencing
the honest refusal — fails **both** tests, and the second fails on its
*`check_the_checker` control*, which is the point: with nothing answering, the
control can no longer tell "`>` is silent" from "the page is dead", and the test
correctly refuses to pass. Adding `"drill-into-cell" => Some(vec![SetStatus(…)])`
to `step` — making `>` resolve — fails **only** the second test and leaves the
**Both injections, run** — against the tree *before* the interim, which is what
made them two faults rather than one. Replacing the aggregate arm with `None` —
silencing the honest refusal — failed **both** tests, and the second failed on
its *`check_the_checker` control*, which is the point: with nothing answering,
the control can no longer tell "`>` is silent" from "the page is dead", and the
test correctly refuses to pass. Adding `"drill-into-cell" => Some(vec![SetStatus(…)])`
to `step` — making `>` resolve — failed **only** the second test and left the
first green. Two faults, two different failure signatures, neither test passing
for the other's reason.
for the other's reason. That second injection is now the shipped code, which is
why the test it broke was rewritten rather than kept.
That control is load-bearing and not decoration: a test asserting *nothing
happened* passes against a page that has stopped listening, so it presses `i`
afterwards and requires `#status` to change. Without that, this file would be
folklore with a green tick.
### the_drill_key_and_an_aggregate_edit_refuse_for_different_reasons
The Rust half of the interim, and the assertion that carries the whole weight.
Two keys on the same aggregate cell must both refuse and must **not** refuse
with the same sentence: `>` says drill is unavailable, `i` says drilling into an
aggregate needs the host. Collapsing them into one message would be the
tempting tidy-up and would lose the distinction between "this key does nothing
here yet" and "this cell is not one you may write".
**The injection, and why it is not the obvious one.** Replacing `>`'s sentence
with the *aggregate* sentence looks like it should fail on the first assertion —
and does not: that sentence contains both "drill" and "browser", so it clears
the substring check. What fires is the anti-aggregate assertion lower down. The
claim that the second assertion is the load-bearing one was doubted on review,
tested, and confirmed; record it here because the first assertion reads like the
discriminating one and is not.
The harness sibling (`drill.spec.js`) asserts the same distinction in a browser,
but its `not.toContainText("aggregate")` is **decoration under the only
injection that exists** — Playwright throws at the positive assertion above it
first, so that line is never reached. The Rust assertion is the guard; the
harness one is end-to-end confirmation that the string survives the wire.
### a_view_row_click_focuses_moves_and_switches_to_the_row_clicked
improvise-iioi, and it is the **same trap** as the category click one level
over: the activation must be resolved against the row **clicked**, never
`view_panel_cursor`, because the `SetPanelCursor` ahead of it has not been
folded when the resolver runs. Injecting exactly that — reading the name at the
cursor instead of at `index` — yields
`SwitchView("Default")` beside `SetPanelCursor { cursor: 1 }`, i.e. the cursor
lands on `Plan` and the document switches to `Default`. Silent, and wrong in the
half a user does not watch.
The fixture therefore holds **two** views and clicks the **second**. With one
view, or with a click on the row already under the cursor, the bug is invisible
— the same reason `a_click_focuses_the_panel_moves_its_cursor_and_activates_the_row`
clicks Product rather than `_Index`.
### a_formula_row_click_navigates_and_activates_nothing
The negative arm, and it is a statement about the *rule* rather than about the
formula panel. The rule is "go there, then do what `Enter` does there";
`ModeKey::FormulaPanel`'s keymap binds `a`/`n`/`o` to `enter-mode formula-edit`
and `d`/`Delete` to delete, and **no `Enter` at all**. So the rule yields
navigation and nothing else, and that is the rule applying rather than the fix
stopping half way.
This test asserts the *exact* command vector, which is the whole of what it can
say. It once carried a second, explanatory `assert!` that no `FormulaEdit` mode
change appears — **which could never have been the failing assertion**, because
the `assert_eq!` above it pins the same three commands and aborts first. A
reviewer demonstrated it by injecting a `ChangeMode(FormulaEdit)` and watching
the panic land on the `assert_eq!` with the explanatory message never printed.
It was a comment wearing an `assert!`, and the argument now lives in
`context/code/clients.md` where a comment run cannot hold it.
### the_formula_panel_has_no_enter_to_mirror_and_the_view_panel_does
The guard that the removed assertion was pretending to be. The asymmetry — a
view-row click activates, a formula-row click does not — is a fact about the
**keymap**, and nothing tied the click to it. The reviewer proved the gap by
adding `Enter -> enter-mode formula-edit` to `ModeKey::FormulaPanel` and running
the whole workspace: **1381 passing, nothing failed**, while `click_formula_row`
still activated nothing. The click and the keyboard then disagree about what a
formula row does, silently.
Injecting that same binding now fails this test and only this test. The failure
message names `click_formula_row` as what must change, because the person who
adds an `Enter` binding is not the person reading `resolve.rs`'s click arms. The
positive half (`ViewPanel` *does* bind `Enter`) is there so the test cannot pass
by asserting that no panel binds anything.
#### The view and formula row clicks in the browser (improvise-iioi)
These began as two tests in `harness/tests/pointer.spec.js`. **That file no
longer exists**: PR #61 deleted it and moved the pointer's browser half into
`harness/tests/contract.spec.js`, whose stated scope is the wiring — the handler
is on the element and passes *that* element's index — while what a click MEANS
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 leave the panel **open but unfocused** — the only state in which the
*focus* third of the click is observable.
**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.
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
user who has just learned category rows are clickable clicks a view row and
reads the silence as broken — which is literally what "I can't expand
categories" meant.
#### The harness port: reuse is now opt-in
`reuseExistingServer: !process.env.CI` became `=== "1"` on
`IMPROVISE_HARNESS_REUSE`. A default-on reuse adopts a server another checkout
left on the port and then tests **that tree**, which fired in both directions in
one session — a false negative and, worse, a false *positive*. A path-hash
default was considered and rejected: it still silently reuses when two agents
share a worktree path, where an explicit opt-in cannot. CI is unaffected (it
never reused) and a developer wanting the old speed sets the variable.
### a_click_that_cannot_compose_its_activation_composes_nothing
improvise-hkr8. `click_category_row` ended with
`filter_to_item_at(state, index).unwrap_or_default()`, and that `None` arm is
not "nothing to do" — it is `BasisCache::claim` answering that the server has
published no basis yet (gotcha 43), so the document change cannot be composed.
The click therefore kept its **navigation prefix** (focus the panel, move the
cursor), dropped its activation, and reported success. The keyboard handles the
identical `None` correctly, by mapping `step(..) == None` to
`KeyOutcome::Upstream`.
The fixture has to do two things a careless one would not. It **expands
`Region`** so that row 1 is an `Item` row — a header row activates via
`ToggleCatExpand`, which needs no basis and so cannot show the bug — and it
asserts both preconditions (`cat_tree_entry_at` is an `Item`, `claim` is `None`)
rather than assuming them, because an empty `ClientState` makes every
state-dependent refusal look like the one under test.
Injecting the old `unwrap_or_default()` fails this test and only this test.
### the_same_click_composes_everything_once_a_basis_is_published
The positive arm of the one above, and it is what stops the fix from being
"return nothing more often". With a basis published, the same click composes all
six commands: open, focus, move the cursor, two `Command::Model`s and the
status. Without this, deleting the activation entirely would pass hkr8's test.
### a_first_tile_click_selects_without_touching_the_document
The user's ruling: a click selects, a second click cycles. The fixture publishes
a basis **deliberately**, so the absence of a `Command::Model` is a decision
rather than a client that could not compose one — pointed at an unpublished
basis this test would pass for hkr8's reason instead of its own.
Both halves are asserted, because "the axis did not change" passes against a
page where nothing happened at all: the positive half requires `SetTileCatIdx`
plus `ChangeMode(TileSelect)`, the negative half requires no `Command::Model`.
**Four injections, four distinct signatures.** "First click also cycles" fails
this test, `the_first_click_on_tile_zero_selects_it_like_any_other` and
`clicking_a_different_tile_selects_it_rather_than_cycling_it` — and **not**
`a_second_click_on_the_selected_tile_cycles_its_axis`. "Second click does not
cycle" fails only that last one. "`already_selected` reads the index alone"
fails only the tile-zero test. Reverting hkr8 fails only the hkr8 test. No
injection passes for another's reason.
### t_then_a_click_on_the_leftmost_tile_cycles_it_immediately
Found on review, and it is a defect in the *wording* rather than the code. `T`
enters `TileSelect` leaving `tile_cat_idx` at **0**, so a click on the leftmost
tile is a click on the already-selected one and cycles on the **first** gesture.
Both loose summaries of the ruling — "a click never mutates on its own" and "a
**second** click cycles" — are false against that probe.
The behaviour is right: a click does what `Enter` does there, and `Enter` in
`TileSelect` cycles the tile under the cursor. The resolver cannot see how the
state arose and must not try to. So the test exists to **name** the case rather
than to change it, and it asserts the neighbouring tile answers the identical
gesture by selecting — because the surprising half is that two adjacent tiles
differ, and a reader who meets that in the wild should find it written down.
Note what this means for its sibling `a_second_click_on_the_selected_tile_cycles_its_axis`:
that test sets the state directly rather than by clicking, so it **already was**
the keyboard-entry case. Nothing was untested; the name and the argument were
missing.
### enter_in_an_empty_view_panel_resolves_locally_rather_than_upstream
`switch_view_at` returns `Option`, and its two callers answer the `None`
differently on purpose. **From a `click_*`, `None` means "this gesture composed
nothing"** — a dynamic fact, and hkr8's rule. **From `step`, `None` means "this
client cannot execute this command *name*"** — a *static* fact about the
vocabulary, which `the_command_names_that_still_need_the_host_are_these` exists
to keep static. Two questions sharing one type, which is why the asymmetry reads
wrong at first. (`delete_view_at_cursor` already answered `Some(Vec::new())` the
same way, so this follows the file rather than inventing.)
Nothing held the keyboard side to it until this test. A reviewer removed the
`unwrap_or_default()` so out-of-range propagates `None` — making `Enter` in an
empty view panel `KeyOutcome::Upstream`, gotcha 47's black hole — and **the whole
workspace passed under `--no-fail-fast`**. That would make membership of the
host-needed list state-dependent, the one property that list denies. Injecting
it now fails this test and only this test.
The assertion is `Ignored`, not `Local(vec![])`: `KeyOutcome::of` maps an empty
vec to `Ignored` by construction, so `Local` is never empty. What matters is
that it is not `Upstream`.
### the_first_click_on_tile_zero_selects_it_like_any_other
The trap in the ruling's own wording. "A second click on the already-selected
tile" reads as an index comparison, and `tile_cat_idx` is **0 before anything
has selected anything** — so an index-only test would read tile 0 as already
selected and cycle it on its very first click, making the non-mutation
guarantee true of every tile but one. The condition is `mode == TileSelect`
**and** `tile_cat_idx == index`.
### clicking_a_different_tile_selects_it_rather_than_cycling_it
The other half of the same trap, from the opposite side: without it, "a click
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.
#### How to know a launch default has stopped disarming things
The question this left open was **how to know you have exhausted the class**.
Grep does not answer it: grep finds sites that *name* the flag, and the hazard
is a test that merely **starts from a default-built state** and asserts
something downstream, mentioning nothing. Three disarmed tests were found by
grepping `category_panel_open`; that was luck, not method.
**Mutation answers it, because it does not care what a test mentions.** The
recipe, which a reviewer ran across five sites in about twenty minutes:
> For every production site that **writes, propagates or reads** the value,
> replace it with the constant the default supplies, and run
> `--no-fail-fast`. Sites where nothing fails are suspects. Then **re-run each
> suspect with the default inverted.** If it now fails something, the test was
> **vacuous** — the default disarmed it, and the fix is the fixture. If it
> still fails nothing, it was simply **unguarded** — a different problem,
> pre-existing, and a bead rather than a fixture.
**The second step is the one that matters**, and without it the two classes are
indistinguishable: both look like "nothing failed". It is what established that
the class is closed at **two** (`set_panel_open_and_cursor`,
`panel_commands_address_the_named_panel_only`) and that two further silent sites
are older problems this ruling did not cause:
- `ui/app.rs`'s `App::cmd_context` propagator — hardcode it `true` and the whole
workspace passes. `registry.rs` reads it for `toggle-panel-and-focus`'s
open-vs-close decision, so `C` on a closed panel would decide to *close* and
nothing would fail. Invisible because `cmd/mod.rs`'s test helper sets the
field explicitly, so no command test reaches the propagator. Filed.
- `improvise-wasm-client/src/render.rs`'s `RenderModel` field — hardcode it
`true` and **Rust sees nothing at all**; only the Chromium harness catches it,
at `contract.spec.js`'s `toBeHidden`. That is literally improvise-wsaw
reintroduced one layer up, and **a Rust-only CI run cannot see it** — which is
the sharpest argument on file for why the browser job must be able to fail the
build.
**And a repair needs its own probe, because a repair can be shaped like a fix.**
The two-step recipe above finds a *disarmed* test; it does not tell you whether
the repair you then write is real. That gap is not hypothetical — the first
repair of `panel_commands_address_the_named_panel_only` folded
`[open:false, open:true]`, read as a fix in review, and was still vacuous.
The probe that settles it is a **one-directional** fault: make the writer
`if open { set true }`, so the flag can be raised but never lowered. **A vacuous
repair cannot see this**, because a test that only ever asserts the `true`
direction is satisfied by a writer that only ever writes `true`. Injected into
both copies at once, it fails `set_panel_open_and_cursor` **and**
`panel_commands_address_the_named_panel_only`, each in its own copy — which is
what establishes both repairs are genuine rather than merely differently
spelled. Prefer it to re-reading the diff: the diff contained `open: false` in
the version that was vacuous.
Note `resolve.rs`'s `panel_open` reader is **well** guarded by contrast:
hardcoding it `true` fails two click tests. So the picture is not "everything
touching this flag is untested"; it is specifically the propagators.
### the_terminal_constructor_inherits_the_shared_default_rather_than_setting_it
**Prospective rather than present-tense, and the doc comment says so.** With
three panels, `the_terminal_app_launches_with_the_category_panel_open` and
`the_other_two_panels_are_shut_at_launch` already cover every `Panel::ALL`
member, so this test cannot currently fail without one of them failing too. A
reviewer confirmed that by control.
It earns its place the way gotcha 12 describes `Panel::ALL`: a **fourth** panel
reaches it from one `ALL` entry, and at that moment it becomes the only test
asserting that `App::new` did not quietly give the TUI its own launch state —
the divergence improvise-wsaw closed. Recorded here so nobody later reads a
green tick as evidence of something it is not yet checking.
### 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
the **shared** `ViewState::default()` rather than a browser special case — a
browser-only default is "the page overrides the flag", which is the same defect
as the "the page ignores the flag" bug improvise-wsaw fixed. The consequence,
accepted deliberately, is that the TUI opens with the panel too.
The test also asserts the *other two* panels stay shut, so it cannot pass
against a default that simply opens everything.
Two existing tests had to change, and both were pinning the old decision by
**inheriting** it rather than stating it.
`panel_focus_cycles_from_the_focused_panel_not_the_first` is about skipping a
*closed* panel and took "the category panel is closed" from the default; it now
spells that precondition. `with_categories()` is used by tests about a click
*opening* the panel, so it now shuts it explicitly. Neither is the decision
being re-made — the decision is re-made only in the harness, deliberately.
#### The launch state and the tile ruling in the browser
Also merged into `harness/tests/contract.spec.js` rather than left in the
deleted `pointer.spec.js` — see the note above.
Two states, one mechanism: the sidebar is visible with no keypress, **and** `C`
still hides it. Proved by injection in a real browser: setting
`el.categories.hidden = false` (the page ignoring the flag) fails on
`toBeHidden`; setting the shared default back to `false` fails on `toBeVisible`.
Different signatures, different halves.
**Only the second is a guard, and an earlier wording here got that wrong.** It
claimed "either alone passes against the bug wsaw fixed" — but under the new
default, visible-at-launch passes for correct code *and* for a sidebar drawn
unconditionally. The launch assertion states the **ruling**; the `toBeHidden`
after the toggle is the only thing that still catches wsaw.
Note the first `C` now **focuses** rather than opens, since the panel is already
open — `toggle-panel-and-focus` closes only when the panel is both open and
focused — so the test presses it twice and the hide is the second press.
The tile tests were rewritten rather than adjusted, because the previous one
asserted the behaviour the ruling reverses; its failure against the new code was
itself the injection. The first-click test waits out a timeout before asserting
the axis did not move, since a cycle would arrive asynchronously and an
instantaneous assertion would pass against a slow one.
---
## `web/tests/` — the Node suite, and what was injected into it
crates/improvise-command/src/ui/effect.rs
+9
View File
@@ -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,
crates/improvise-protocol/src/lib.rs
+1 -1
View File
@@ -372,7 +372,7 @@ impl Default for ViewState {
search_query: String::new(),
search_mode: false,
formula_panel_open: false,
category_panel_open: false,
category_panel_open: true,
view_panel_open: false,
cat_panel_cursor: 0,
view_panel_cursor: 0,
crates/improvise-protocol/src/reduce.rs
+8
View File
@@ -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,
crates/improvise-protocol/src/resolve.rs
+381 -32
View File
@@ -72,30 +72,62 @@ pub fn resolve(state: &ClientState, event: KeyEvent) -> KeyOutcome {
pub enum Click {
/// A row of the category panel, indexing the flattened tree.
CategoryRow(usize),
/// A row of the view panel, indexing `ViewListCache::views`.
ViewRow(usize),
/// A row of the formula panel, indexing `FormulaCache::formulas`.
FormulaRow(usize),
/// A tile, indexing `TileBarCache::tiles`.
Tile(usize),
/// A grid cell, in **viewport** coordinates as the page painted it.
Cell { row: usize, col: usize },
}
/// One rule for all three widgets: **go there, then do what `Enter` does
/// there.** Why that rather than three answers: `context/code/clients.md`.
/// One rule for all the widgets: **go there, then do what `Enter` does
/// there.** Why that rather than one answer each: `context/code/clients.md`.
pub fn click(state: &ClientState, target: Click) -> Vec<Command> {
match target {
// `None` here means the whole click composed nothing, which is the
// invariant rather than a breach of it (improvise-hkr8) — clients.md.
let resolved = match target {
Click::CategoryRow(index) => click_category_row(state, index),
Click::ViewRow(index) => click_view_row(state, index),
Click::FormulaRow(index) => click_formula_row(state, index),
Click::Tile(index) => click_tile(state, index),
Click::Cell { row, col } => click_cell(state, row, col),
}
};
resolved.unwrap_or_default()
}
/// Focus the panel, move its cursor, then switch — what `V` then `Enter` do,
/// in one gesture. `switch-view-at-cursor` is the activation.
fn click_view_row(state: &ClientState, index: usize) -> Option<Vec<Command>> {
let mut commands = focus(state, Panel::View);
commands.push(Command::SetPanelCursor {
panel: Panel::View,
cursor: index,
});
// Off `index`, never the cursor: `SetPanelCursor` is not folded yet.
commands.extend(switch_view_at(state, index)?);
Some(commands)
}
/// The formula panel binds **no** `Enter`, so the rule yields navigation and
/// nothing else. That is the rule applying, not an omission — clients.md.
fn click_formula_row(state: &ClientState, index: usize) -> Option<Vec<Command>> {
state.formulas.formulas.get(index)?;
let mut commands = focus(state, Panel::Formula);
commands.push(Command::SetPanelCursor {
panel: Panel::Formula,
cursor: index,
});
Some(commands)
}
/// Focus the panel, move its cursor, then activate — the three things the
/// keyboard needs three keystrokes for, and `filter-to-item` is the activation.
fn click_category_row(state: &ClientState, index: usize) -> Vec<Command> {
fn click_category_row(state: &ClientState, index: usize) -> Option<Vec<Command>> {
// A click past the last row is a click on the panel's empty space, which
// is not a row and must not move the cursor onto one.
if cat_tree_entry_at(state, index).is_none() {
return Vec::new();
}
cat_tree_entry_at(state, index)?;
let mut commands = focus(state, Panel::Category);
commands.push(Command::SetPanelCursor {
panel: Panel::Category,
@@ -103,35 +135,41 @@ fn click_category_row(state: &ClientState, index: usize) -> Vec<Command> {
});
// Against `index`, never the cursor: the `SetPanelCursor` above is not
// folded yet, so the cursor still reads the old row. Evidence file.
commands.extend(filter_to_item_at(state, index).unwrap_or_default());
commands
commands.extend(filter_to_item_at(state, index)?);
Some(commands)
}
/// Select the tile and cycle its axis — `TileSelect` plus what `Enter` does
/// in it. A tile the bar does not have is not a click.
fn click_tile(state: &ClientState, index: usize) -> Vec<Command> {
if state.tiles.tiles.get(index).is_none() {
return Vec::new();
/// Select the tile; a **second** click on the already-selected one cycles its
/// axis. Why a click never mutates on its own: `context/code/clients.md`.
fn click_tile(state: &ClientState, index: usize) -> Option<Vec<Command>> {
state.tiles.tiles.get(index)?;
// Mode *and* index, because `tile_cat_idx` is 0 before anything selects
// anything — index alone would cycle tile 0 on its first click.
let already_selected =
state.view.mode == AppMode::TileSelect && state.view.tile_cat_idx == index;
match already_selected {
true => tile_axis_at(state, index, None),
false => {
let mut commands = vec![Command::SetTileCatIdx(index)];
if state.view.mode != AppMode::TileSelect {
commands.push(Command::ChangeMode(AppMode::TileSelect));
}
Some(commands)
}
}
let mut commands = vec![Command::SetTileCatIdx(index)];
if state.view.mode != AppMode::TileSelect {
commands.push(Command::ChangeMode(AppMode::TileSelect));
}
commands.extend(tile_axis_at(state, index, None).unwrap_or_default());
commands
}
/// Move the selection to a clicked cell. Activation is the page's inline
/// editor, which is the pointer's `i` — `context/code/clients.md`.
fn click_cell(state: &ClientState, row: usize, col: usize) -> Vec<Command> {
fn click_cell(state: &ClientState, row: usize, col: usize) -> Option<Vec<Command>> {
let cursor = state.view.cursor();
let GridExtent { rows, cols } = state.grid.extent;
// The page paints a window; `SetSelected` is absolute (gotcha 42).
let (absolute_row, absolute_col) = (row + cursor.row_offset, col + cursor.col_offset);
if absolute_row >= rows || absolute_col >= cols {
return Vec::new();
return None;
}
scrolled_to(state, absolute_row, absolute_col)
Some(scrolled_to(state, absolute_row, absolute_col))
}
/// Open a panel and focus it, emitting only what is not already true — so a
@@ -273,13 +311,9 @@ fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> Op
"delete-category-at-cursor" => delete_at_cat_cursor(state),
// ── the view panel ───────────────────────────────────────────
"switch-view-at-cursor" => Some(match view_at_cursor(state) {
Some(name) => vec![
Command::SwitchView(name),
Command::ChangeMode(AppMode::Normal),
],
None => Vec::new(),
}),
"switch-view-at-cursor" => {
Some(switch_view_at(state, state.view.view_panel_cursor).unwrap_or_default())
}
"create-and-switch-view" => {
let name = format!("View {}", state.views.views.len() + 1);
Some(vec![
@@ -834,6 +868,16 @@ fn view_at_cursor(state: &ClientState) -> Option<String> {
.map(|summary| summary.name.clone())
}
/// `SwitchViewAtCursor::execute`'s whole answer, at an arbitrary index. The
/// keyboard passes its cursor and a click passes the row — clients.md.
fn switch_view_at(state: &ClientState, index: usize) -> Option<Vec<Command>> {
let summary = state.views.views.get(index)?;
Some(vec![
Command::SwitchView(summary.name.clone()),
Command::ChangeMode(AppMode::Normal),
])
}
/// `DeleteViewAtCursor::execute`, including the cursor step back that only
/// happens away from the top of the list.
fn delete_view_at_cursor(state: &ClientState) -> Option<Vec<Command>> {
@@ -1005,10 +1049,28 @@ mod tests {
}
}
/// The user's ruling: the category panel is open at launch. It is the
/// *shared* default, so both front ends open with it — clients.md.
#[test]
fn a_fresh_session_opens_with_the_category_panel() {
let fresh = ViewState::default();
assert!(
fresh.category_panel_open,
"a client that has pressed nothing can already see its categories"
);
assert!(
!fresh.formula_panel_open && !fresh.view_panel_open,
"and only that one — the ruling names the category panel alone"
);
}
// ── the pointer (improvise-7ybz) ────────────────────────────────
fn with_categories() -> ClientState {
let mut state = ClientState::default();
// Explicitly shut: these tests are about a click *opening* the panel,
// which the launch default would otherwise answer for them.
state.view.category_panel_open = false;
state.fold(&Command::Project(Projection::CategoryTree(
crate::CategoryTreeUpdate::Categories(vec![
crate::CategoryRow {
@@ -1081,6 +1143,286 @@ mod tests {
fn a_click_past_the_last_row_resolves_to_nothing() {
assert!(click(&with_categories(), Click::CategoryRow(9)).is_empty());
assert!(click(&ClientState::default(), Click::Tile(0)).is_empty());
assert!(click(&ClientState::default(), Click::ViewRow(0)).is_empty());
assert!(click(&ClientState::default(), Click::FormulaRow(0)).is_empty());
}
/// Two tiles and a **published basis**, so a missing `Command::Model` is
/// a decision rather than a client that could not compose one.
fn with_tiles() -> ClientState {
let mut state = ClientState::default();
state.bootstrap(Snapshot {
view: ViewState::default(),
seed: vec![Projection::Basis(published())],
});
state.tiles.tiles = vec![
crate::TileEntry {
category: "Region".into(),
axis: Axis::Row,
},
crate::TileEntry {
category: "Product".into(),
axis: Axis::Column,
},
];
state
}
/// The user's ruling: a click **selects**, it never mutates. The basis is
/// published here, so nothing stops the cycle but the rule itself.
#[test]
fn a_first_tile_click_selects_without_touching_the_document() {
let state = with_tiles();
let commands = click(&state, Click::Tile(1));
assert_eq!(
commands,
vec![
Command::SetTileCatIdx(1),
Command::ChangeMode(AppMode::TileSelect),
],
"select and enter TileSelect, and stop there"
);
assert!(
!commands.iter().any(|c| matches!(c, Command::Model { .. })),
"a click must not change the document; the axis cycle needs a second one"
);
}
/// `tile_cat_idx` is 0 before anything has selected anything, so the
/// already-selected test needs the **mode** as well as the index.
#[test]
fn the_first_click_on_tile_zero_selects_it_like_any_other() {
let state = with_tiles();
assert_eq!(state.view.tile_cat_idx, 0, "the default index is the trap");
let commands = click(&state, Click::Tile(0));
assert_eq!(
commands,
vec![
Command::SetTileCatIdx(0),
Command::ChangeMode(AppMode::TileSelect),
]
);
assert!(
!commands.iter().any(|c| matches!(c, Command::Model { .. })),
"index alone would read tile 0 as already selected and cycle it"
);
}
/// Entering `TileSelect` by keyboard leaves `tile_cat_idx` at 0, so a click
/// on the leftmost tile cycles on the FIRST gesture. See clients.md.
#[test]
fn t_then_a_click_on_the_leftmost_tile_cycles_it_immediately() {
let mut state = with_tiles();
state.view.mode = AppMode::TileSelect;
assert_eq!(state.view.tile_cat_idx, 0, "what `T` leaves behind");
let commands = click(&state, Click::Tile(0));
assert!(
commands.iter().any(|c| matches!(c, Command::Model { .. })),
"the tile really is selected, and a click does what Enter does there"
);
assert!(
!click(&state, Click::Tile(1))
.iter()
.any(|c| matches!(c, Command::Model { .. })),
"its neighbour answers the identical gesture by selecting — deliberate"
);
}
/// The second click is on the **already-selected** tile, not merely the
/// second click of the session — otherwise the guarantee holds once.
#[test]
fn a_second_click_on_the_selected_tile_cycles_its_axis() {
let mut state = with_tiles();
state.view.mode = AppMode::TileSelect;
state.view.tile_cat_idx = 1;
assert_eq!(
click(&state, Click::Tile(1)),
vec![
Command::Model {
basis: state.basis.claim([]).unwrap(),
cmd: ModelCommand::CycleAxis("Product".into()),
},
// `Column.next()` is `Page` — the *new* axis, as `Enter` says.
Command::SetStatus("Product → Page".into()),
]
);
}
/// Clicking A then B selects B. Cycling B would make "a click does not
/// mutate" true only of the first click in a session.
#[test]
fn clicking_a_different_tile_selects_it_rather_than_cycling_it() {
let mut state = with_tiles();
state.view.mode = AppMode::TileSelect;
state.view.tile_cat_idx = 0;
let commands = click(&state, Click::Tile(1));
assert_eq!(
commands,
vec![Command::SetTileCatIdx(1)],
"already in TileSelect, so only the index moves"
);
assert!(!commands.iter().any(|c| matches!(c, Command::Model { .. })));
}
/// `None` from a `click_*` means "this gesture composed nothing"; `None`
/// from `step` means "not this client's command". See clients.md.
#[test]
fn enter_in_an_empty_view_panel_resolves_locally_rather_than_upstream() {
let state = in_mode(AppMode::ViewPanel);
assert!(state.views.views.is_empty(), "no views to switch to");
assert_eq!(
resolve(&state, ev(KeyCode::Enter)),
KeyOutcome::Ignored,
"an empty panel is a state, not a command the host must answer — \
Upstream here would make the host-needed list state-dependent"
);
}
/// improvise-hkr8: a click is all of its intents or none of them. With no
/// published basis the activation cannot be composed, so nothing is.
#[test]
fn a_click_that_cannot_compose_its_activation_composes_nothing() {
let mut state = with_categories();
state.view.expanded_cats.insert("Region".into());
assert!(
matches!(
cat_tree_entry_at(&state, 1),
Some(CatTreeEntry::Item { .. })
),
"row 1 is an item row, whose activation is a document change"
);
assert!(
state.basis.claim([]).is_none(),
"and this client has been published no basis"
);
assert!(
click(&state, Click::CategoryRow(1)).is_empty(),
"the focus and the cursor move must not survive the activation's failure"
);
}
/// The same click, once the basis has arrived: all four intents.
#[test]
fn the_same_click_composes_everything_once_a_basis_is_published() {
let mut state = with_categories();
state.view.expanded_cats.insert("Region".into());
state.fold(&Command::Project(Projection::Basis(published())));
let basis = state.basis.claim([]).unwrap();
assert_eq!(
click(&state, Click::CategoryRow(1)),
vec![
Command::SetPanelOpen {
panel: Panel::Category,
open: true
},
Command::ChangeMode(AppMode::CategoryPanel),
Command::SetPanelCursor {
panel: Panel::Category,
cursor: 1
},
Command::Model {
basis: basis.clone(),
cmd: ModelCommand::SetAxis {
category: "Region".into(),
axis: Axis::Page
},
},
Command::Model {
basis,
cmd: ModelCommand::SetPageSelection {
category: "Region".into(),
item: "North".into()
},
},
Command::SetStatus("Filter: Region = North".into()),
]
);
}
fn with_panels() -> ClientState {
let mut state = ClientState::default();
state.fold(&Command::Project(Projection::ViewList(vec![
crate::ViewSummary {
name: "Default".into(),
axes: vec![],
},
crate::ViewSummary {
name: "Plan".into(),
axes: vec![],
},
])));
state.fold(&Command::Project(Projection::Formulas(vec![
"Profit = Revenue - Cost".into(),
"Margin = Profit / Revenue".into(),
])));
state
}
/// improvise-iioi: the same three intents the category panel gets, with
/// `switch-view-at-cursor` as the activation.
#[test]
fn a_view_row_click_focuses_moves_and_switches_to_the_row_clicked() {
let state = with_panels();
assert_eq!(
click(&state, Click::ViewRow(1)),
vec![
Command::SetPanelOpen {
panel: Panel::View,
open: true
},
Command::ChangeMode(AppMode::ViewPanel),
Command::SetPanelCursor {
panel: Panel::View,
cursor: 1
},
// `Plan`, the row clicked — not `Default`, where the cursor is.
Command::SwitchView("Plan".into()),
Command::ChangeMode(AppMode::Normal),
]
);
}
/// The formula panel binds no `Enter`, so the rule yields navigation and
/// nothing else. A mode change here would be an invented activation.
#[test]
fn a_formula_row_click_navigates_and_activates_nothing() {
let state = with_panels();
let commands = click(&state, Click::FormulaRow(1));
assert_eq!(
commands,
vec![
Command::SetPanelOpen {
panel: Panel::Formula,
open: true
},
Command::ChangeMode(AppMode::FormulaPanel),
Command::SetPanelCursor {
panel: Panel::Formula,
cursor: 1
},
]
);
}
/// The asymmetry is a fact about the keymap, so it is asserted against
/// the keymap — not against a reading of it. See `context/code/clients.md`.
#[test]
fn the_formula_panel_has_no_enter_to_mirror_and_the_view_panel_does() {
let keymaps = Keymaps::shared();
assert!(
keymaps
.lookup(&ModeKey::FormulaPanel, KeyCode::Enter, KeyModifiers::NONE)
.is_none(),
"if FormulaPanel gains an Enter binding, click_formula_row must \
activate too — the click mirrors Enter, and this is the only \
thing tying the two together"
);
assert!(
keymaps
.lookup(&ModeKey::ViewPanel, KeyCode::Enter, KeyModifiers::NONE)
.is_some(),
"and the view panel's click activates because its Enter does"
);
}
/// The pointer and the keyboard are one cursor: a click resolves to the
@@ -1415,9 +1757,12 @@ mod tests {
/// to `enter-mode normal` inside a panel, so no key reaches them.
#[test]
fn panel_focus_cycles_from_the_focused_panel_not_the_first() {
// Spelled out rather than inherited: this test is about skipping a
// *closed* panel, so the default's own answer must not decide it.
let open_two = |mode: AppMode| ViewState {
mode,
formula_panel_open: true,
category_panel_open: false,
view_panel_open: true,
..ViewState::default()
};
@@ -1436,8 +1781,12 @@ mod tests {
vec![Command::ChangeMode(AppMode::FormulaPanel)],
"and from the last, it wraps"
);
let nothing_open = ViewState {
category_panel_open: false,
..ViewState::default()
};
assert!(
cycle_panel_focus(&ViewState::default()).is_empty(),
cycle_panel_focus(&nothing_open).is_empty(),
"with nothing open there is nowhere to go"
);
}
crates/improvise-wasm-client/src/lib.rs
+13
View File
@@ -118,6 +118,19 @@ impl Client {
self.clicked(Click::CategoryRow(index))
}
/// The user clicked row `index` of the view panel: focus, move, switch.
#[wasm_bindgen]
pub fn click_view(&mut self, index: usize) -> Option<String> {
self.clicked(Click::ViewRow(index))
}
/// The user clicked row `index` of the formula panel. Navigation only —
/// that panel binds no `Enter` (improvise-iioi).
#[wasm_bindgen]
pub fn click_formula(&mut self, index: usize) -> Option<String> {
self.clicked(Click::FormulaRow(index))
}
/// The user clicked tile `index`: select it and cycle its axis.
#[wasm_bindgen]
pub fn click_tile(&mut self, index: usize) -> Option<String> {
harness/playwright.config.js
+4 -3
View File
@@ -2,9 +2,10 @@ import { defineConfig, devices } from "@playwright/test";
// The one place a line of web/*.js is executed, and where a change to either
// file belongs. Why, and why Playwright: context/code/clients.md.
// `reuseExistingServer` will happily adopt a server another checkout started on
// this port, and then you test THAT tree. See context/code/clients.md.
// Reuse is OPT-IN because adopting another checkout's server tests THAT tree,
// and the symptom is a result contradicting the file you just read.
const PORT = process.env.IMPROVISE_HARNESS_PORT ?? "8099";
const REUSE = process.env.IMPROVISE_HARNESS_REUSE === "1";
export default defineConfig({
testDir: "./tests",
@@ -38,7 +39,7 @@ export default defineConfig({
// The APP, not the site root — since improvise-kh8 the root is the landing
// page, which would answer 200 with no bundle assembled at all.
url: `http://127.0.0.1:${PORT}/app/index.html`,
reuseExistingServer: !process.env.CI,
reuseExistingServer: REUSE,
timeout: 30_000,
},
});
harness/tests/contract.spec.js
+45 -6
View File
@@ -41,12 +41,20 @@ test.describe("every field the render model projects reaches the screen", () =>
// `categories` + `category_panel_open`: the flag is the client's answer,
// and the sidebar was drawn always until it read it (improvise-wsaw).
await expect(page.locator("#categories")).toBeVisible();
await expect(page.locator("#categories li").first()).toBeVisible();
// Open at launch by the shared default, so the first press only FOCUSES:
// `toggle-panel-and-focus` closes only when open AND focused.
await page.keyboard.press("Shift+C");
await expect(page.locator("#mode")).toHaveText("CATEGORIES");
await expect(page.locator("#categories")).toBeVisible();
// This is the assertion that catches wsaw — a sidebar drawn unconditionally
// passes the launch one. Visible-at-launch is the ruling, not the guard.
await page.keyboard.press("Shift+C");
await expect(page.locator("#categories")).toBeHidden();
await page.keyboard.press("Shift+C");
await expect(page.locator("#categories")).toBeVisible();
await expect(page.locator("#categories li").first()).toBeVisible();
await page.keyboard.press("Shift+C");
await expect(page.locator("#categories")).toBeHidden();
await page.keyboard.press("Escape");
// `views`: empty unless open, so the page asks no question about the mode.
await expect(page.locator("#views")).toBeHidden();
@@ -133,19 +141,22 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
await page.keyboard.press("ArrowLeft");
await expect(cells.nth(3)).toHaveClass(/selected/);
// A tile: selects and cycles, because a click is the whole interaction.
// A tile: the first click SELECTS and does not touch the document; the
// second, on the same tile, cycles. The user's ruling — clients.md.
const product = page.locator("#tiles .tile").filter({ hasText: "Product" });
await product.click();
await expect(page.locator("#mode")).toHaveText("TILES");
await expect(product).toHaveClass(/selected/);
// Waited out, not asserted instantly: a cycle would arrive asynchronously.
await page.waitForTimeout(500);
await expect(product).toHaveText("Product Row");
await product.click();
await expect(product).toHaveText("Product Col", { timeout: 10_000 });
await page.keyboard.press("Escape");
// A category row: focuses the panel, moves its cursor and activates the
// row — three things from one click, and the index must be the row's own.
await page.keyboard.press("Shift+C");
await expect(page.locator("#categories")).toBeVisible();
await page.keyboard.press("Escape");
const productRow = page.locator("#categories li").filter({ hasText: "Product" });
await expect(page.locator("#categories li.selected")).toHaveText(/_Index/);
await productRow.click();
@@ -157,5 +168,33 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
"Cold Brew",
"Pastry",
]);
await page.keyboard.press("Escape");
// 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.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");
await expect(page.locator("#formulas li").first()).toBeVisible();
await page.keyboard.press("Escape");
await page.locator("#formulas li").first().click();
await expect(page.locator("#mode")).toHaveText("FORMULAS");
await expect(page.locator("#formulas li.selected")).toHaveCount(1);
});
});
tests/tui_launch_state.rs
+55
View File
@@ -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` could override the shared default without the protocol test
/// noticing. PROSPECTIVE with three panels — see the evidence file.
#[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,
}
}
web/app.js
+10 -4
View File
@@ -166,8 +166,10 @@ function repaint() {
el.categories.hidden = !model.category_panel_open;
el.categories.replaceChildren(categoryList(model));
paintTiles(model);
paintPanel(el.views, model.views, viewRow);
paintPanel(el.formulas, model.formulas, formulaRow);
paintPanel(el.views, model.views, viewRow, (i) => client.click_view(i));
paintPanel(el.formulas, model.formulas, formulaRow, (i) =>
client.click_formula(i),
);
// What the frame costs, so the NEXT viewport announcement is honest.
// Inline group ROWS are deliberately NOT counted — context/code/clients.md.
@@ -340,10 +342,14 @@ function edit(td, row, col, seed) {
/** A side panel. An empty row list is what says it is closed — the page asks
* no question about the mode, exactly as `paintHelp` does not. */
function paintPanel(host, rows, build) {
function paintPanel(host, rows, build, clicked) {
host.hidden = rows.length === 0;
const list = document.createElement("ul");
for (const row of rows) list.appendChild(build(row));
rows.forEach((row, index) => {
const li = build(row);
li.addEventListener("click", () => send(clicked(index)));
list.appendChild(li);
});
host.replaceChildren(list);
}
web/tests/bundle/wasm_surface.test.js
+2
View File
@@ -74,7 +74,9 @@ test("the page still reaches the client through a handful of calls", () => {
[
"click_category",
"click_cell",
"click_formula",
"click_tile",
"click_view",
"on_frame",
"on_key_event",
"render_model",