Files
improvise/context/test-evidence/clients.md
T
Edward Langley a8737f3fa5 One page rule for both ends; the help overlay shows how to page
improvise-5xbq: resolve::page_rows is now the one home of the page-scroll
rule; the TUI's Move calls it instead of repeating visible_rows * 3 / 4.
A lockstep case pages (PgDn/PgUp, Ctrl-d/Ctrl-u) on a non-square window,
8 rows by 3 columns, and pins the TUI's landing rows as well as comparing
host and client, since one home moves both ends together. Both faults
the ticket injected (page_rows = window_rows / 2; paging by window_cols)
now fail at the first PgDn, before and after the move to one home.

improvise-1c5m: the help overlay draws the mode's own hint (hint_line,
the string #hint carries) in a sticky footer, so a narrow window whose
app bar wraps under #help still says how to page. Playwright case at
600px, red at base, and red again with the footer made non-sticky.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-23 07:41:03 -07:00

4807 lines
271 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Test evidence: browser client, hosts, harness
What each of these tests proves, what fault was injected to prove it can fail,
and — where it applies — the bug it was written against. Keyed by exact test
name; a renamed test orphans its entry, which
`scripts/check-comment-runs.py --check-evidence` reports.
The design argument each of these belongs to is in `context/code/clients.md`.
---
## `improvise-worker-server/src/worker.rs`
### an_edit_is_persisted_without_the_client_asking_to_save
The persistence half of improvise-9x6's acceptance criterion 2, and the reason
the worker saves on its own behalf: there is no `ModelCommand::Save` for a
client to send, because `effect::Save` is `ViewOnly` and the wire vocabulary is
derived from the *model* effects. So an edit must reach storage with the client
having asked for nothing but the edit. The test writes one cell over the wire
and reopens the document through the same VFS.
### a_partly_refused_batch_does_not_leave_disk_and_memory_disagreeing
improvise-955n's **second-order effect**, which nobody had written down until it
was found while declining the ticket: `on_frame` skipped `persist()` when any
frame in the answer was `Rejected`, so a batch that *did* apply its earlier
command before being refused left the document moved in memory and not written
back — disk and worker disagreeing, silently.
Two changes close it independently, which is why this is worth a test of its own
rather than trusting either: the batch no longer applies partially at all
(improvise-955n), and the persist trigger is now the document's own dirtiness
rather than "was anything rejected" (improvise-gv9r), so a batch that *did*
change something would be written whatever else the answer contained.
The fixture is a refused frame with a command behind the refusal, and the
assertion is that the axes on disk and the axes in memory agree. Under the
settled contract the refusal comes from the **frame's** basis — answered once,
on the first command that carries one — so that is where the staleness is
injected; a later command carries no answer of its own.
### an_explicit_save_is_not_retried_by_the_automatic_write
Found by Copilot on PR #26. A failed write leaves `model_state.dirty` set —
deliberately, so the next frame retries — and the automatic trigger runs after
*every* frame. A save frame therefore attempted the same write twice: extra I/O
on a permanent failure, and on a transient one something worse, because the
answer is composed from the *first* attempt, so the client is told `SaveFailed`
about a document that is now on disk.
The fix is a per-frame question rather than a per-document one, and that is the
right shape precisely because the automatic write exists for edits **nobody
asked** to save; one that was asked for has already been answered.
The fixture is a `Storage` backend whose writes always fail and that counts
them. **Counting is the point**: "it was refused" is true whether the write was
attempted once or twice, so a test asserting only the refusal would pass against
the bug. **Failing is the point too**: a successful save clears `dirty`, so the
redundant second attempt never happens and the bug is invisible from the happy
path.
### a_key_resolved_on_the_host_moves_state_no_client_can_hear
*Why* the `Key` arm answers nothing, demonstrated rather than asserted in prose.
A worker can call `reduce_key` — it holds the authoritative `App`, which is the
thing a socket server lacks. What neither has is a way to tell a client that the
**host's own `ViewState`** moved: a client's slice advances only by folding
`Command`s, and resolving a keypress produces none. So the two slices diverge,
and `ClientSession::viewport` goes on reading the mirror's cursor, which means
the grid does not even follow. That is improvise-bpq2's real cost, and it is a
ticket rather than plumbing. The test asserts all three: the authoritative
cursor moved, the mirror did not, and nothing `reduce_key` emits would let a
client catch up.
---
## `improvise-worker-server/src/vfs.rs`
### an_entry_that_holds_no_document_is_seeded_over
The first browser run of the deployed page failed here, and no host test could
see it: `attach(create: true)` leaves a **zero-byte** entry behind, `documents`
listed it, and `open` therefore took the seed as already applied and read an
empty file. The precondition is asserted before the act, because a test that
only opens cannot tell "the seed landed" from "there was nothing in the way".
The rule it pins is the one gotcha 45 states for the index: what is indexed is
a document, and an entry nobody has written bytes to is not one.
This entry exists because the test cites this file and `--check-evidence` now
requires that (improvise-lyyc); it was written from the test, so read the test
first if the two ever disagree.
---
## `improvise-ws-server`
### distinct_stems_both_open
`doctrine(check_the_checker)` for `colliding_stems_are_fatal`: inject the fault
that test rules out — distinct stems — and both documents must open. Otherwise
an `open_all` that failed on every second file would pass the collision test.
### a_bystander_on_the_same_document_is_projected_the_edit
`doctrine(check_the_checker)` for
`an_edit_is_projected_only_to_the_edited_documents_clients`: put the bystander
on the *same* document and the assertion must flip. Without it, an isolation
test that passed because nobody is ever projected anything looks identical.
### the_other_document_would_also_have_a_cursor_to_invalidate
`doctrine(check_the_checker)` for `a_broadcast_does_not_cross_a_document_boundary`.
The fault to rule out is the other document being *incapable* of producing the
broadcast at all — then a server-wide relay would be silent over there for the
wrong reason and the isolation test would pass without proving isolation. So the
checker drives the same `TransposeAxes` against `beta` and asserts it does produce
a `ResetCursor` there; `beta`'s silence in the isolation test is the routing and
nothing else.
Both tests moved from `Command::SwitchView` to `ModelCommand::TransposeAxes` with
improvise-jytm: a view switch is now addressed to one session and relays nothing,
so it can no longer carry an isolation test. `ResetCursor` is the surviving
`Broadcast::ToAll`.
### a_load_that_fails_rebases_nobody
`doctrine(check_the_checker)` for `a_rebase_stops_at_the_document_boundary`:
inject a load that cannot happen. The generation does not move, so nobody is
rebased — which is also what a routing bug that sent `Snapshot` to nobody would
look like, so the rebase test needs this one beside it to mean anything.
### a_reopened_document_would_have_lost_the_edit_and_the_basis
`doctrine(check_the_checker)` for
`the_last_session_leaving_does_not_close_the_document`: inject the eviction — a
document freshly opened from the same fixture, which is exactly what closing on
last-disconnect would hand the next client. Both assertions in the lifecycle
test fail against it, so neither passes for a reason other than the document
having stayed put.
### a_rebase_stops_at_the_document_boundary
Fixture note, and it is gotcha 39's warning: the swapped-in document keeps the
**same active view name** and changes only its shape. A rename would make
`apply_model` broadcast a `SwitchView`, the client would fold it, and the stale
cursor would then be read under a key nothing has written — right by accident,
with the bug invisible.
### a_client_on_a_socket_is_seeded_and_gets_its_edit_back
(`crates/improvise-ws-server/tests/socket.rs`, and its neighbours in that file.)
Everything the socket test asserts is already asserted in `host.rs` without a
socket, and deliberately so: its subject is the **shell**, not the decisions. It
earns its place by covering what the unit tests structurally cannot — the
handshake rule, JSON framing in both directions, the routing from a `SessionId`
back to the connection it came from, and the one fact that lives only in a URL:
which document the connection asked for. If it ever starts being the place a
behaviour is pinned, that behaviour belongs in `host.rs` or `registry.rs`.
Two disciplines inside it are load-bearing rather than stylistic. The write is
composed from the surrogate the server projected (ADR 002) — a `CellKey` there
would be the test knowing something a browser does not — and the basis is
composed from what the server published, because a basis the test built itself
would be a client claiming to have seen something it was never sent.
---
## `improvise-wasm-client/src/keys.rs`
#### text_entry_modes (a fixture, not a test)
Not a test — a fixture, and its limitation is the evidence. The comment here
used to claim a new mode "shows up as a missing line rather than as untested
behaviour". It does not: this is a hand-written list and nothing checks it
against `AppMode`, so a new text-entry mode is simply absent and silently
uncovered — which is what happened to `save_prompt` (improvise-lae) until it was
added by hand. There is a second hand-written copy in the command crate's
`keymap::tests::text_entry_modes_with_exits`. Deriving both from one place is
filed, not done.
### a_modified_character_now_types_because_the_fallback_is_the_shared_one
The old classifier sent a modified character upstream in a text-entry mode,
because the host's `AnyChar` fallback matches under **any** modifier and a
hand-written rule could only safely be a subset of it. Reading the same table
removes the question: `Ctrl-x` falls through to `AnyChar` here exactly as it
does on the host.
### search_typing_writes_the_query_and_not_a_buffer
Search typing resolves on the client for the first time, and only because the
keymap is shared: `SearchMode` binds `append-char ["search"]`, and the name
`"search"` is `ViewState::search_query` rather than an entry in `buffers`
(improvise-gdl). The old classifier sent every search key upstream into an empty
vec. The negative half of the test matters too: `Esc` stays upstream, because
`exit-search-mode` owns the query and is the host's.
### the_command_names_that_still_need_the_host_are_these
The command **names** this client cannot execute, one by one, so that closing
one of these gaps has to come here and say so. This replaced a list of *keys*,
and that is the whole gain of sharing the keymap: a rebinding no longer
invalidates the list, and a new binding for an already-executable command works
in a browser with no change to this crate. Each entry is driven through the key
that reaches it, because a name that is not bound anywhere would otherwise sit
in the list forever looking like a gap.
**It is a live gate, and improvise-jz6w exercised it.** `save` left the list
when Ctrl-S became a `Request`; putting the entry back fails with *"save
(Char('s')) was resolved locally; if that is deliberate, this list is where it
stops being a gap"*. So the mechanism gotcha 47 asks for was watched failing
rather than assumed.
### a_prompt_this_client_cannot_commit_is_never_entered
The refusal list above is only half the property. `Upstream` says the key
produced no `Command`; it does not say the client stayed put, and the bug being
guarded is precisely *entering a room with no exit*.
Found by Copilot on PR #27. Without `strands_the_user`, `Ctrl-W` resolved
`enter-mode save-prompt` locally — `ChangeMode(SavePrompt)`, a perfectly good
`Command` — and the user then typed a path into a prompt whose Enter goes
nowhere. `Ctrl-E` did the same into `ExportPrompt` and had done since before
that branch.
**The negative arm has two halves, and both are needed.** Half one: a mode that
is *not* refused is still entered — the half that goes through
`strands_the_user`, so the only thing that can fail when the predicate
degenerates to "refuse everything". `?` is honest here where `:` was not: Help
has no commit at all, so nothing a user types into it can be swallowed, and Esc
closes it locally. (Its page keys *are* upstream, which makes page navigation
inert — but inert navigation loses no work, which is the line the predicate
draws.) Half two: the property is about *finishing*, not entering, so something
has to press Enter and observe. The first version of this test asserted only
that `:` entered its mode, and `:` is itself a dead room — so that arm was
certifying the very behaviour it existed to rule out.
### the_command_line_cannot_be_committed_by_this_client
The evidence behind listing `CommandMode` in `strands_the_user`, rather than the
assertion that it is listed. `:` opens locally and typing appends locally, but
Enter is `[execute-command, clear-buffer]` and `step` has no `execute-command`
arm — so the whole sequence fails to resolve, `Host::handle` drops the frame,
and every ex-command behind `:` is inert. That is what makes refusing the mode
cost no working surface. **When improvise-e24v gives this client
`execute-command`, this is the test that should start failing**, and the
`CommandMode` arm of `strands_the_user` is what it points at.
### a_commit_in_the_last_cell_exits_editing_instead_of_advancing
At the bottom-right corner `EnterAdvance` returns `AbortChain`, so the re-entry
is skipped and the commit leaves the user in normal mode. That abort is a fact
about the host's effect chain and it has to be mirrored, or the browser would
sit in an editor on a cell the terminal has left. The fixture is a one-by-one
grid, so the only cell *is* the corner.
### a_move_that_goes_nowhere_follows_the_command_it_mirrors
Which widgets say nothing about a move that goes nowhere is **not a rule of
`keys.rs`** — it is whatever the `Cmd` being mirrored does, and the three
differ: `Move` emits even when it clamps, `MovePanelCursor` returns nothing when
the cursor does not move, `MoveTileCursor` emits regardless. The grid arm
carries its own check-the-checker inline (a `j` that does move), so "emits when
clamped" cannot pass against a fixture that is simply inert.
### i_on_an_aggregate_declines_out_loud
`i` on an aggregate must not go upstream: `Host::handle` answers a `Key` frame
with an empty vec, so that would be a keypress the user sees do nothing at all
(`doctrine(an_upstream_key_is_a_black_hole)`).
### enter_commits_the_cell_advances_and_re_enters_editing
Enter in `Editing` is `[commit-cell-edit editing, clear-buffer edit]`, and every
part of that sequence shows up in what the client composes — **including the
trailing clear that empties the buffer `EnterEditAtCursor` has just seeded**.
That ordering looks wrong and is exactly what the TUI does; mirroring it line
for line is the contract.
---
## `improvise-wasm-client` — lib, socket, state, render
### a_client_with_no_basis_composes_nothing
The fourth `None`, and the one ADR 001 adds: a client that has not been told the
document's basis has nothing truthful to say about what it composed against, so
it composes nothing. That is the property that makes the basis check unforgeable
from a browser — `BasisCache::claim` is the only constructor, and it withholds.
The other three `None` shapes — an aggregate, `id: None`, and a position outside
the grid — are pinned against the same composition in `socket.rs`'s
`typing_into_a_cell_that_is_not_a_record_composes_nothing`. They were asserted
twice, once against each of two compositions, until improvise-r4lo left only one.
### a_refused_batch_is_kept_as_a_rejection_and_not_only_as_a_sentence
A refused batch must reach the person who composed it, and it must reach them as
more than a sentence. ADR 001 makes rejection a *normal outcome*, and the
obligation that comes with it is that the client keeps the typing and can
re-compose. Force needs the `Rejection` itself — `RecordGone` cannot be forced at
all where `RecordMoved` can — so collapsing it into `status_msg` would throw away
the input to the very feature rejection exists to enable. **Both halves are
asserted, because setting the string alone is the plausible half-fix**, and the
revision bump is asserted too: a renderer polls the revision, and a rejection it
cannot see is a rejection it cannot draw.
### every_rejection_message_says_what_it_refused
Found by the PR #12 Copilot review: `DocumentReplaced` read "the document was
replaced — reloading", and nothing in `fold_frame` reloads anything. **A message
that names an action the code does not take is worse than a vague one, because
the user waits for it.**
Renamed and re-scoped by improvise-8uj8, which is the more interesting half. The
test was called `every_rejection_message_says_the_edit_did_not_land` and its own
doc said the fact "is asserted over all of them rather than one at a time" — while
it drove a **hand-written list of five**, written before PR #26 added
`NoDocumentPath` and `SaveFailed`. So two variants were silently exempted from a
rule the test called universal, which is worse than no test: the claim was false
about its own subject.
The re-scope is that the claim, not the messages, was wrong. A save failure did
not fail to *apply* anything, so "the edit did not land" is the wrong sentence for
it. `Rejection::refused_an_edit` (exhaustive) splits the two classes and
`Rejection::all` (a cyclic walk, the same shape `AppMode::all` uses) drives them,
so a new variant fails the build rather than slipping out of the list.
Fault injected: wording `SaveFailed` as `"hmm"` fails the test — the arm the old
five-item list could not have seen. Adding a `Rejection` variant fails the build
at `basis.rs`'s `refused_an_edit` and `after`.
### a_typed_string_composes_the_frame_that_carries_it
Was `a_typed_string_is_classified_as_the_host_classifies_it`, a test that **stated
an obligation in its name and did not discharge it** — it asserted three outcomes
by hand rather than comparing against the host's function, so the two copies of
the rule could drift and stay green. improvise-j7v6 removed the duplication rather
than the drift: `commit_regular_cell_value` now calls `improvise_protocol::typed`,
which the browser's compose path already used.
Per `doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished)`
the test could not survive as a comparison — type identity replaced its claim, and
a hand-written comparison beside the shared function would be the copy coming
back. It is re-scoped to what this crate still owes, which nothing else covers:
the **frame** a typed string composes — basis, cell and command.
The classification's own obligation moved to `tests/typed_string_agreement.rs`,
where it is checked against the thing it could really disagree with: what
`.improv` calls a number (gotcha 34).
### a_cell_projection_that_overtakes_its_labels_is_lost
Gotcha 33, carried across *this crate's* dispatch. The protocol crate pins the
drop inside `GridCache::apply`; what is this crate's is that nothing between the
socket and that cache buffers, sorts or retries — so the same early `Cells` is
still lost after routing. The second half of the test feeds the same three
updates in frame-first order and requires them to land, so "everything is
dropped" cannot pass it.
### bootstrapping_from_a_compaction_lands_where_the_stream_lands
`doctrine(fold_tests_must_test_order_not_the_axiom)`: a client that bootstraps
from a compaction of the host's state must end up where a client that folded the
whole stream is. The seed is the host's *current* state expressed as projections,
not the head of the history.
### the_selection_is_placed_by_subtracting_the_scroll_offset
(`render.rs`.) The conversion that looks right until the first scroll: the grid
is viewport-indexed, the cursor is not. The offsets in the fixture are **nonzero
and unequal**, which is what separates "reconciles" from "ignores the offset"
*and* from "swaps the two offsets".
### column_groups_coalesce_per_header_as_the_tui_does
(`render.rs`.) Was `column_groups_coalesce_by_name_as_the_tui_does`, and its
justification was consumed by the consolidation it predates. It read: the run
boundary must be the name alone, "because that is what `src/ui/grid.rs` uses
(`group_name != prev_group`) and the two ends must not disagree". Since
improvise-8qlr `src/ui/grid.rs` uses `AxisFrame::group_run_heads`, so there is
no second copy to agree with — the rule can change once and both ends follow.
**Its fixture is what settled improvise-h0iv.** It merges `Month/Q1`
(expanded) with `Fiscal/Q1` (**collapsed**), and its own message said the merged
span is "drawn with the FIRST one's indicator". That is a collapsed group
rendered as expanded, with nothing on screen saying otherwise. Whether two
adjacent same-named runs are one span is a question of taste and is why h0iv was
filed rather than fixed; a wrong collapse glyph is not. The expectation is now
three runs, and the collapse flag of each is asserted — which is what makes this
test see the fault at all, since the labels and spans alone cannot distinguish a
merge that keeps the first indicator from a split.
### a_keyboard_edit_in_progress_is_part_of_the_render_model
(`render.rs`.) A keyboard edit has to be *visible*, and what is visible is
decided in Rust rather than in the page. The failure this rules out is quiet: a
client that resolved `i` locally would change its mode label, put the typing into
`buffers["edit"]`, and draw the cell's old value the whole time — so the user
sees a keystroke do nothing, types over it, and commits something they never
read. `None` outside an editing mode is the other half: a stale buffer must not
draw an editor that is not open.
### the_page_bar_names_every_fixed_category_and_what_it_is_fixed_to
(`render.rs`, improvise-p7lh.) `GridCache::page_coords` was projected to every
session, folded correctly by every client, and read by **no** front end — so the
shipped front page (`web/demo.improv` is `Quarter: page, Q1`) drew a grid
filtered to Q1 with nothing anywhere saying so. Worse, `[`/`]` resolve *locally*
in the browser off that same `page_coords`, so a user could press `]`, watch
every number in the grid change, and have no way to tell which quarter they were
looking at or how to get back. The numbers were right and the frame was a lie by
omission.
Proved able to fail by deleting the `page_coords:` line from `render_model`,
which is the state the bug shipped in.
Why this is the assertion and not "the field exists": the bar's whole job is to
name *which item*, so `selected` is what discriminates. A bar that drew the
category name alone would satisfy a presence check and still leave the user
unable to tell Q1 from Q2.
### only_a_navigable_page_axis_offers_a_cycle
(`render.rs`, gotcha 27.) Page is navigable; Filter constrains without being
navigable, and a client must compose no cycle for one. The one home of that
split is `PageAxis::cycle`, and the plausible wrong answer is `items.is_some()`
— which agrees with it on the two common cases and disagrees on the third.
Three axes in one fixture, because two would not separate the readings:
- a Page axis with three items, selected at index 2. **Injected** `items.is_some()`
with `at: 0` and watched it fail on the index — a cycle that always reports
position 1 reads as correct on a freshly-opened document and is wrong the
moment anyone pages.
- a Filter axis (`items: None`). Both readings agree here, which is why it
cannot be the only case.
- an axis with `items: Some(vec![])`. `PageAxis::cycle` filters the empty list
and says `None`; `items.is_some()` says there is a cycle of length zero. This
is the case that makes the test discriminating rather than decorative.
### the_totals_strip_is_relayed_and_an_empty_one_stays_empty
(`render.rs`, improvise-p7lh.) The same shape as the page bar: `GridCache::totals`
projected, folded, and painted by nothing. The TUI draws it (`src/ui/grid.rs`);
the browser had no field for it.
Proved able to fail by replacing `totals_strip`'s body with `Vec::new()` — the
state the bug shipped in — and watching it fail on the relayed text.
Two halves, and the second is the one gotcha 33 is about: an **empty** strip
means "this grid has no totals" (records mode, or an empty grid) and must stay
empty rather than becoming a row of blanks. A client that padded it would draw a
blank total row under a records grid that has none.
What is deliberately **not** asserted anywhere: that the numbers are right. They
are the host's, summed over every row *including ones this viewport scrolled
past*, which is exactly why they are projected. A client-side check of the
arithmetic would be the second derivation the projection exists to prevent — so
the arithmetic is pinned end to end instead, in `contract.spec.js` below, against
a real document where the sum is a fact.
---
## `improvise-wasm-client/tests/browser_round_trip.rs`
The file's scope is improvise-1ey's, quoted: *"open an existing document
server-side, view it in the browser, type a number into a cell, see the server
value update and the projection arrive with the new value."* Everything in that
sentence except the DOM is Rust on both sides, so the test drives the whole of
it: a real `Host` over an authoritative `App`, and a real `Client` reached only
through the string calls the page makes. The socket is the one thing left out,
and it is covered by `improvise-ws-server`'s own `tests/socket.rs`; what this
adds is the half that crate cannot reach — that the **browser client** seeds,
renders, composes and folds correctly against it.
**The discipline that makes it evidence rather than decoration**: the client is
touched **only** through `Client::viewport`, `Client::on_frame`,
`Client::render_model` and `Client::typed_cell` — strings in and out — and it
learns the cell to write from its own render model, never from the fixture. A
`CellKey` anywhere in it would be the test knowing something a browser does not
(ADR 002).
### a_grouped_row_axis_does_not_shift_the_write
A group header is a **row of the render model and not a row of the grid**, and a
write composed against the wrong one lands on a different record — silently,
because both are `usize`. `FirstEditable` therefore returns two numbers.
A fixture with no groups makes them coincide, which is exactly how an earlier
version of the `first_editable` helper handed a *visual* index to `typed_cell`
and passed (Copilot, PR #23). This test is the fixture the main round trip cannot
be: a `Coastal` header sits *between* the two rows, so for the second row
`visual_row` and `data_row` differ. Injecting the confusion — passing
`at.visual_row` to `typed_cell` — leaves `East` unwritten. Note the ordering of
the fixture is load-bearing: `West` comes first and ungrouped, because a header
*before* the first row is dropped by `AxisFrame::body_from` and the two
coordinates would coincide again.
### a_commit_keypress_is_one_mutation_among_view_commands_and_is_accepted
improvise-955n: **the one real producer of a multi-command frame must still be
accepted.** `CheckedBatch::against` answers a frame's basis before applying any
of it, and every mutation answers for its own records. Getting that check right
is the easy half; the expensive half is not tightening it until the browser stops
working. A commit keypress is exactly the shape at risk — one `SetCell` plus the
mode change, the scroll offsets and the re-seeded buffer around it — and it is
produced by `keys.rs::commit`, which no test in the command crate can reach.
So it drives a **real keypress** through a **real `Host`** and asserts the frame
is accepted *and* lands: `ServerFrame::Rejected` anywhere in the answer, or an
unchanged cell, is the regression. Asserting only "no rejection" would pass
against a host that dropped the frame on the floor. The "exactly one mutation"
assertion is a **characterization of what `keys::commit` emits, not a protocol
constraint** — a frame may carry more than one — so that if `commit` ever
started batching two, the test would be exercising something else and should be
re-read rather than silently re-scoped. And the fixture types `888` *after* `i`
has seeded the buffer, so the assertion is on the appended value: `888` alone
would also be produced by a client that ignored the seed.
---
## `improvise-wasm-client/tests/lockstep.rs`
`src/keys.rs` decides a small set of keypresses without the document. The claim
that makes that safe is not "the rule looks the same" but **the two ends land in
the same `ViewState`**, and that is checkable: the host's `reduce_key` runs the
real keymap, registry and effect pipeline against an authoritative `App`, and
`ViewState` is the protocol's own type on both sides. So every local outcome is
driven through both and compared.
This is why the command crate is a dev-dependency. Dev-dependencies are not
built for `--target wasm32-unknown-unknown`, so nothing the host owns reaches the
browser bundle.
### a_locally_typed_character_lands_where_the_host_would_put_it
Driving one mode would say nothing about the others
(`doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)`), so
every text-entry mode is driven, and each with a **non-empty** buffer so an
append that silently *replaced* the buffer would be visible.
### a_forwarded_key_reaches_the_host_unchanged
An unresolvable key must reach the host as a keypress, and what the host then
does to *its* copy of this client's view slice is what the client will be told.
Pinned as the shape the transport owes: the message carries the code and
modifiers, and they survive the round trip. `Enter` rather than an arrow key,
because since improvise-bpq2 the arrows resolve locally and `enter-advance` is
one of the movements deliberately left upstream — it aborts the rest of its batch
at the bottom-right corner, which is a fact about the host's effect chain and not
a cursor position.
### a_locally_resolved_cursor_move_lands_where_the_host_would_put_it
The headline claim for improvise-bpq2's first gap: a cursor key resolved on the
client lands where the host would have put it — **including the scroll that
follows it**, and including the clamp at the end of the grid.
The fixture is six rows in a three-row window, and the keys walk past the bottom
of the window (so `row_offset` must move) and then past the bottom of the *grid*
(so the clamp must hold). A client reading its bound off the row frame's length
would stop at row 2 and never scroll; one that clamped against nothing would run
off the end. Both are visible. Eight presses for six rows: the last three are the
clamp.
**The fixture's one column is deliberate.** The two ends measure their windows
differently: the host's `visible_rows` is `term_height - 8` and its
`visible_cols` is computed from column widths, while a browser announces both
outright. `term_height` can be chosen to match the rows; nothing can be chosen to
match the columns, so the fixture removes the question rather than guessing at an
answer.
### a_locally_resolved_panel_cursor_agrees_with_the_host
improvise-bpq2's second named gap, which turned out not to be one: the category
panel's flattening depends on `expanded_cats`, but the derivation already lives
in `improvise_protocol::viewmodel` and *both* ends call it (improvise-edp). So
this is not two derivations agreeing by luck — it is one derivation, and the test
says so by **expanding** a category, which changes the flattened length the
cursor clamps against. A client that counted categories would stop several rows
early.
### the_buffer_i_seeds_is_the_value_the_host_would_seed
`i` seeds the edit buffer from the cell's **display value**, and the two ends
must agree on what that is: the host reads `App::display_value_at_cursor`, the
client reads the text the server projected into its `GridCache`. Those are two
paths to one string, and this is the assertion that they are the same string.
### entering_typing_and_cancelling_an_edit_agrees_with_the_host
`Esc` rather than `Enter` for the cancel arm, because it is a
`Binding::Sequence` and so exercises the all-or-nothing rule: half of it — a
cleared buffer in a mode the user has not left — is worse than none.
### a_locally_committed_edit_leaves_both_ends_in_the_same_place
What is *not* compared is the document, and deliberately: the client answers with
a `Command::Model`, which `reduce_view` drops, while `reduce_key` writes through
the effect pipeline. The write's composition is asserted in `keys.rs` against
`typed_cell`'s own — the shared one — so what is left to prove here is the half a
unit test cannot see, which is that the advance and the re-entry land in the same
place.
#### client_ready_to_edit (a fixture, not a test)
The published basis a real client folds before it is allowed to compose a write
(ADR 001). The tiers are the ones lockstep never checks — the host applies the
keypress directly rather than the client's `Command::Model` — but
`BasisCache::claim` withholds until it has *something*, so a client without one
resolves no commit at all.
---
## `harness/` — the browser specs
These are Playwright tests, so the checker's `--check-evidence` pass (which
scans Rust `fn` names) cannot see their names. They are recorded here under
their exact `test("…")` titles anyway, because they are where the JavaScript
evidence lives.
#### harness/tests/shell.spec.js — the file's own scope
The three assertions improvise-hwpi could not write. That ticket fixed four
things and said of itself that the fixes were "as unverified as the bugs",
because three of them are one-assertion tests that needed a browser. These are
those three, plus the watchdog — which was found by reading, and only because
someone went looking. Until this file, `web/app.js` and `web/worker.js` had
never been rendered or run.
Two fixtures are load-bearing. `NO_WEBSOCKET_HERE` is a peer that accepts the
TCP connection and then refuses the WebSocket upgrade — the harness's own static
file server — **deterministic where a closed port is not**: a refused port's
`close` timing is a property of the host's network stack, and on the GitHub
runner it did not arrive inside the window. The page cannot tell the two apart
and neither can `web/app.js`: in both cases the socket never opened.
`HANDSHAKE_TIMEOUT_MS` is **imported from `web/shell.js`** and restates nothing
(PR #65), and **one** test outlasts it — this sentence said the opposite of both
for a while, which is gotcha 48's third clause: a fact stated twice is changed by
a grep, not an edit. Since improvise-669b the spec also compares the *running
page's* window against the one it imported, because the single home is in the
TREE and the page loads `harness/serve/app/shell.js` — see
`outlastTheWatchdog` below. `logStatus` records **every distinct `#status`**,
not just the last one, because several of these tests are about a message being
*replaced*, which the final value cannot see.
#### "a frame the client cannot fold says so on screen"
improvise-hwpi: `on_frame` returns a `Result`, which wasm-bindgen throws. Thrown
in a bare listener it reached the devtools console and nothing else — the grid
simply stopped updating with nothing on screen saying why. **The fault is
injected at the *peer*, not in the page** — a mocked socket that answers the
handshake with a frame the client cannot fold — so this exercises the real
`on_frame` and the real catch.
#### "a peer that cannot be reached is named, and not called a disconnect"
improvise-hwpi: without an `error` listener, a connection that never opened left
`#status` blank, and "the server is not running" read exactly like "still
connecting". **This test no longer says anything about the watchdog, and the
paragraph below is why it never could.** `shell.arm` runs only after
`await transport.ready`, which resolves on `open` — so on a socket that never
opens the watchdog is **never armed at all**, and denying its message here
denies an unreachable event.
**That sentence was already in this file, five lines above an assertion that
contradicted it.** PR #65 added the watchdog clause *and edited this very
paragraph*, inserting "`runFor(10s)` advances the watchdog" immediately before
the sentence saying it is never armed. Reverting PR #21's routing, or stripping
`fail()` of its `sawFrame()`, both left that clause **passing**. So a PR whose
subject was "a deleted guard went unnoticed" planted the same shape one level
up: three injections, three distinct test names, and two of them proving
nothing. What the test guards now is the **naming** — `close` on a socket that
never opened must not read as "disconnected" (improvise-taxx) — which is the
half injection 3 really does exercise.
**The mechanism, so it cannot come back.** `outlastTheWatchdog` now asks the
page `timesArmed() > 0` before jumping, and a watchdog denial re-added to this
path **fails by name**. It asks *ever armed* rather than *armed now*: by the
time the disconnect test jumps, a correct page has already disarmed — that is
the property — so `armed()` would have denied the passing case. Building that
guard is what surfaced the distinction, which no amount of reading had.
#### "a socket that opened and then dropped reads as disconnected, and stays that way"
The other arm, which must keep working when the first is fixed: a socket that
really did open and then dropped IS a disconnect, and saying so is the whole
reason `close` cannot simply be suppressed.
**It runs past `HANDSHAKE_TIMEOUT_MS` on purpose** — through `page.clock`, not
through a real wait. On this arm the watchdog IS
armed — `armWatchdog` runs after `transport.ready` resolves, i.e. on `open` — so
"disconnected" is written while a timer is pending, and the only reason it
survives is that `close` goes through `setShellFailure`, which clears it. That
was argued in a comment and asserted nowhere, and **the argument was got wrong
from the outside**: a bug report (improvise-xps8) claimed the timer fired 8s
later and overwrote "disconnected" with "sent no document". It had been measured,
honestly, but against a tree predating PR #21 — and the "confirmation" that it
survived PR #21 simulated only that PR's `opened` flag and **not its other half**,
routing BOTH close arms through `setShellFailure`. *A simulated fix that is not
the fix you named reproduces a bug the real code does not have, and reads exactly
like a confirmation.* The ticket was filed and closed **invalid**. Hence this
assertion rather than another paragraph — proved discriminating by making
`setShellFailure` skip its `clearTimeout`, which makes it fail. Same rule as
`doctrine(an_injected_fault_must_be_the_fault_you_named)`, one level up.
**And it was lost a third time, by the PR that was arguing about exactly this.**
PR #61 split the browser tests into boundary contracts (Playwright) and unit
tests (`node --test`), and merged these two tests into one that kept only the
last-value assertions. `shell.test.js`'s "a loud failure DISARMS the watchdog"
covers `createShell` in isolation and **cannot see whether `app.js` routes the
close event through `fail()` rather than `setShell()`** — which is the half
PR #21 fixed and the half that keeps getting lost. Reverting both socket arms
to `setShell` left the node suite 26/26 and Playwright 13/13 green. Restored
under their original names, and re-proved with that exact revert: the two-arm
revert and the close-arm-only revert each fail "…and stays that way", and
dropping the `opened` flag fails "…the watchdog stays out of it". **Three
injections, three different tests, none of them the unit test.**
**And the helper guards its own premise.** `page.clock.runFor` on a page where
`clock.install()` was never called does **nothing and throws nothing**, so a test
that forgot the install would assert "the watchdog never spoke" against a
watchdog never given the chance — vacuous, and green. `outlastTheWatchdog` reads
`Date.now()` inside the page either side of the jump and requires it to have
moved. Demonstrated both ways: with the install removed the suite passed 6/6,
and with the guard in place the same omission fails by name.
**And it guards its second premise since improvise-669b: that the page runs the
constant the spec imported.** PR #65 single-homed the handshake window in
`web/shell.js`, which closed the *understated spec constant* — but the home is
single in the **tree**, not in what the page runs: the spec imports
`../../web/shell.js` and the page loads `harness/serve/app/shell.js`, the copy
`web/assemble.sh` put there. A stale `serve/` restores the exact vacuity, and
review demonstrated it — shrink the constant in source only, revert both socket
arms, 6/6 green. `createShell` now exposes `timeoutMs()` beside `timesArmed()`
and `outlastTheWatchdog` reads the running page's value and requires it to equal
the import. Injected with the ticket's own recipe (`8000` → `100` in
`web/shell.js`, no rebuild): the run goes to 5 passed / 1 failed, on
"the page's handshake window is not the one this spec imported", where the same
staleness was previously invisible.
Note what this does and does not reach: it guards the tests that *call the
helper*. `"a peer that accepts and then says nothing trips the watchdog"` also
names `HANDSHAKE_TIMEOUT_MS`, but as a Playwright `timeout:` margin rather than
as a window to outlast — a stale value there costs patience, not evidence.
**The window is single-homed, and that is the second half of the same
finding.** `HANDSHAKE_TIMEOUT_MS` lived twice — once in `web/app.js`, once in
the spec — with a comment saying a silent disagreement "would pass for the
wrong reason" and nothing checking it. Setting the spec's copy to 1000 against
the page's 8000 made the whole suite green **against the reverted routing**:
the jump landed inside the window, the watchdog had not yet fired, and the
denial held vacuously. It is now exported from `web/shell.js` and imported by
both, so the two cannot disagree; shrinking it in its one home still catches
the routing revert, which is the demonstration that the guard did not go quiet.
**The faked clock is what makes the restoration cheap without weakening it**,
and the injections are the proof of that too: if `page.clock.runFor` did not
really advance the page's timers, a broken routing could not surface the
watchdog's message and those injections would have passed. They failed. The
test keeps the real page, the real socket, the real listeners and the real
`setTimeout` call — only the *passage of time* is simulated, and 9 seconds of
wall clock became 75 milliseconds.
**The measurement that could not become a CI guard.** Both of the above were once
asserted as `test.fail` expected-failures and **had to be removed**: neither
reproduced on the GitHub runner, where `close` does not follow `error` within the
window. An expected-failure is green when the bug is present and red when it is
absent, so one whose trigger is platform-dependent does not merely flake — **it
inverts CI's meaning**, and it failed the build twice saying "expected to fail,
but passed". The lesson stands even though the second bug did not: **assert the
PROPERTY THAT MUST HOLD, which is portable, rather than the fault, whose trigger
is the host's network stack.**
The first of the two, for the record, was real: the socket's `close` listener was
unconditional, so on a connection that never opened it overwrote "cannot reach …"
with "disconnected". FIXED by PR #21. An `opened` flag is the only discriminator
— `readyState` at close time cannot tell the two apart, and neither can the
`CloseEvent`: both cases are code 1006, `wasClean: false`.
#### "a peer that accepts and then says nothing trips the watchdog"
The first-frame watchdog, found by reading. The condition is one **no
per-transport check would catch**: the peer accepted the connection and then said
nothing. The mock accepts and never answers.
#### "the page draws the demo document out of the worker"
No `?ws=`, so `web/app.js` spawns the in-page worker over OPFS: the whole
application, with no server process. If this draws a grid, then `worker.js`,
`bindings.rs`, `OpfsFs`, `vfs::open` and the seed fetch all ran.
#### "committing an edit sends one frame and shows the host's value"
A commit round-trips through the host, and what lands on screen is the **host's**
value rather than the string that was typed. `20000` is typed and `20,000`
asserted: the number format is applied by the host and relayed back, so a page
that echoed the input would fail.
**This is deliberately NOT named for improvise-hwpi's third fix** (the `closed`
re-entry guard in `edit`), which Copilot asked for on PR #24. That guard is **not
observable from the DOM**, measured rather than assumed: with it removed, `close`
still runs exactly twice, the frame is still sent exactly once, `#grid` still
takes exactly two `childList` mutations, and the final text is identical. The only
difference is one extra *entry* into `repaint()` that produces no output — because
the second close is `close(false)`, so it does not re-send, and everything else it
does is idempotent. That is precisely why hwpi called it "harmless before, but
only by accident". Counting `repaint()` entries would need a test hook inside
`web/app.js`, which would be a page that knows it is being tested. So the test
name says what is checked and this entry says what is not. What *is* observable is
asserted: the editor is gone and did not come back, so no stray `<input>` is left
behind and the host's value is not overwritten by the `original` text the second
close writes before repainting.
#### "the harness can see a failure"
Check the checker, and it runs **first**: a green run against a worker that never
started is the failure this repo has a doctrine about. `run_canary` drives the
*same* reporting path every real check uses, and the spec asserts it is reported
as a failure with its message intact.
#### "every check in the battery runs and passes"
The battery is asserted **complete before any of it is believed**: the report's
names must equal `EXPECTED` exactly, so a build that dropped a check fails rather
than passing with fewer.
#### "what OPFS held survives a reload, and a committed delete does not"
The one thing no in-page check can establish. A delete is visible immediately and
lands in OPFS at the end of the batch — so "`commit_deletes` actually removed it"
is only answered by a fresh page. This is also the storage half of improvise-bck's
persistence criterion.
#### The opfs-contract checks (`harness/opfs-contract/src/lib.rs`)
- **`an_attached_but_unwritten_entry_is_not_a_document`** — `attach` must create
the file to obtain a sync access handle for a document that does not exist yet,
and that created file is zero bytes, which is NOT a document. `Storage::list`
promises document names, so an attached-but-unwritten name must not be listed.
Indexing it is exactly the improvise-9x6 browser failure: the seed was skipped
as "already there" and `load_from` got an empty buffer. The check also opens a
**second instance**, which shares no index, because the filter has to be in
`refresh` and not only in the omission from `attach` — and then writes through
the handle to show it is real and the name IS then a document.
- **`a_directory_entry_is_not_a_document`** — `keys()` yields DIRECTORIES as well
as files, and `getFileHandle` on a directory rejects with `TypeMismatchError`.
Treating that as "could not measure, so assume a document" would list an entry
that can never be attached or read — the same picker bug the zero-byte filter
removes (Copilot, PR #24). `OpfsFs` never creates a directory, so the harness
makes one itself, and asserts a real file beside it IS listed — the
precondition, so the check cannot pass by the directory never having been
created, and evidence that `keys()` walked the root at all.
- **`refresh_walks_every_key_and_sorts_them`** — the hand-rolled `keys()`
async-iterator walk in `refresh` is the least certain code in `opfs.rs`
(improvise-xf1 named it). One entry could pass by accident; several, out of
order, cannot. It wipes first, like every other check: a battery whose steps
depend on the leftovers of the step before breaks when one is inserted, which
is exactly what inserting the directory check did.
- **`read_and_write_are_absolute_at_offset_zero`** — `FileSystemReadWriteOptions`'
`at` handling, the other thing xf1 called out. A `write` that appended instead
of overwriting, or a `read` that started anywhere but 0, is invisible on a
single short write and obvious across a long one followed by a short one.
- **`storage_contract_at_…`** — the point of the whole exercise. `attach` is
hoisted out because acquiring a handle is the async half and `Storage::write`
cannot await; **the contract itself is the shared function, not a transcription
of it**.
- **`a_delete_is_visible_at_once_and_lands_on_commit`** — both halves, because
either one alone would pass against a backend that got the timing exactly
backwards.
### a_locally_cycled_panel_focus_agrees_with_the_host
`cycle-panel-focus` is bound to Tab in Normal only: `PanelKeymap`'s spine binds
Tab to `enter-mode normal` inside every panel (gotcha 12), so the *wrap* arms of
`CyclePanelFocus` — the ones that start from the focused panel — are
unreachable by keypress. The test therefore asserts the one reachable path and
says so, rather than looping three times and passing on a mode that never moved.
The first draft did loop, and failed on step 1 with the mode back at `Normal`;
that failure is the evidence for this paragraph.
### a_locally_resolved_tile_axis_change_agrees_with_the_host
Both of `TileAxisOp`'s names in one walk, because the command names itself off
its `axis` field: `r` takes the `Some` arm (`SetAxis`) and `Enter` the `None`
arm (`CycleAxis`), and a test that drove only one would leave a whole `Cmd`
unmirrored. Injecting `parse_axis("row") => Axis::Column` fails it.
### the_category_panel_bindings_agree_with_the_host
The fixture sits on a **header** row for `Space`/`a` and on an **item** row for
`Enter`/`d`, and then on a header again for a second `d`. Both of
`FilterToItem`'s arms and both of `DeleteCategoryAtCursor`'s arms are therefore
reached. That is not decoration: the first version sat only on the item row,
and injecting a wrong sentence into the *category* arm of
`delete_at_cat_cursor` did **not** fail it. One fixture position, one arm —
and a check that passes against an arm it never enters is the defect this repo
keeps finding.
### the_view_panel_bindings_agree_with_the_host
Two views in the fixture, and the cursor moved to 1 before `Enter`, because
`view_at_cursor` reading `.get(0)` is the natural wrong implementation and a
single-view fixture cannot tell it from the right one. Injecting `.get(0)`
fails it.
It presses `d` as well, and that was added after the fact: the first version
said "create, switch and delete" in its doc comment and pressed only `n` and
`Enter`, so `delete_view_at_cursor` — whose *only* decision is the cursor
step-back it makes away from the top of the list — was never run. The
adversarial review of PR #50 deleted that step-back and the workspace stayed at
1322 passing. `d` is pressed at index 1 so the decision is reached, and
`both_ends` now compares every cursor (see below), without which the deletion
would still be invisible.
### a_locally_committed_minibuffer_agrees_with_the_host
`commit-buffer` for **all three** kinds, driven as keystrokes so the
`TextEntry` spine's `clear-buffer` weld is part of what is compared.
`CommitKind::Item` carries its category, resolved from `AppMode::ItemAdd` —
so the mode the test starts in is load-bearing, not scene-setting. Injecting
`AddCategory` in place of `AddItem` fails it on the document comparison.
Formula was missing from the first version, and it is the kind that matters
most: it is the **only** one whose `on_commit()` is `Some`, so a test of the
other two cannot see the mode a commit returns to. The review of PR #50 broke
both `CommitKind::status`'s and `on_commit`'s Formula arms and nothing failed.
Its text is **seeded rather than typed**, and valid, for the reason the next
entry gives.
### a_formula_the_client_cannot_parse_diverges_on_the_status_line
The one thing a client cannot mirror about `commit-buffer formula`, found by
widening the test above and kept as a **pinned divergence** rather than a fix
(improvise-cl7y).
`CommitBuffer` emits its optimistic status first and the add effect last, so
on the host a parse failure overwrites "Formula added" with "Formula error: …"
(improvise-6t3). The client reproduces the command line for line — which is
right — but it cannot parse, so it folds "Formula added" and stops. The user
sees a browser confidently report a formula it did not add.
This asserts **both** sides, so a fix has to come here and change it. That is
the difference between a pinned divergence and a comment: `a_locally_committed
_minibuffer_agrees_with_the_host` would otherwise have to use a valid formula
forever with no record of why.
### panel_focus_cycles_from_the_focused_panel_not_the_first
`CyclePanelFocus`'s wrap arms, driven directly rather than through a key.
`PanelKeymap`'s spine binds Tab to `enter-mode normal` inside every panel
(gotcha 12), so **no keypress reaches them** — which is why the lockstep test
asserts only the reachable path and why breaking the `start` index survived
the review's injection pass. Mirroring a `Cmd` faithfully includes the arms
the keymap cannot reach today; testing them takes a direct call.
The fixture leaves the **category** panel closed, so "the next open one" and
"the next one" give different answers and a mirror that walked the array
blindly would fail.
### a_locally_resolved_clear_and_paste_agree_with_the_host
The two writes ADR 002 lets a thin client name. Note the fixture's cell key has
**three** coordinates: `Workbook::add_category` puts a new category on
`Axis::Column`, so `Product` is part of the cell's address and a two-coordinate
key names a cell the grid never shows. The first version asserted on the
two-coordinate key, and passed the per-key comparisons while asserting against
a cell nothing had touched.
### walking_off_the_end_of_the_help_pages_agrees_with_the_host
The bug this was written to demonstrate: `effect::HelpPageNext` clamps and
`HelpPagePrev` saturates, while `reduce_view`'s `Command::HelpPage` arm
*wrapped*. Two derivations of one fact, in disagreement, invisible because
`h`/`l` in the help overlay were upstream keys and no client had ever sent the
command. It failed on the first press past the end — client 0, host 4 — and the
wire was changed to match the effect. Whether both should wrap instead is
improvise-trd3.
#### "? opens help, and its rows carry the keys and the words, not empty frames"
This entry was demoted to prose as Retired when PR #61 deleted the spec. PR #72
restored the coverage under a new name, so the claim is live again and the
heading now names the test that exists. What follows is the original evidence;
improvise-igxp's stronger assertions are recorded separately below.
improvise-ktv1's acceptance in one place: **a browser shows help.** `?` had
entered `AppMode::Help` since the keymap was shared, and nothing appeared —
the mode changed and the page had no overlay, which is worse than a key that
does nothing because the user thinks they missed it.
It asserts the shape, not merely that something is visible: the active tab is
the shared module's page title (`Welcome`, then `Navigation` after `l`, which
also exercises `Command::HelpPage` folding on the client), and the body has a
key row. Two faults were injected and each failed it. Dropping the
`paintHelp(model)` call fails `toBeVisible`; removing the `mode != Help` guard
in `help_rows` — so the overlay is never empty and therefore never hidden —
fails `toBeHidden`. The second is the one worth having: an overlay that is
always open passes any check that only ever opens it.
It runs against the **assembled** site like every other test here, so it
covers `render.rs`, the serialized model, `web/app.js` and the CSS together.
**It was gone for a release, and that is why the entry reads oddly.** `36ade35`
deleted it with the rest of `harness/tests/widgets.spec.js` and nothing replaced
it, so the heading promised coverage that did not exist -- exactly the cost
improvise-5ray names, and invisible because the evidence checker then read only
`###` headings and only Rust names. PR #69 taught it to see both; PR #72
restored the coverage under the current name, with the stronger content
assertions improvise-igxp asked for.
The injections above are the original two and still describe the property. The
present test is `harness/tests/help.spec.js`, and its own entry below records
what igxp added: the rows are read as `dt`/`dd` pairs and asserted as text, so
blanking `keys.textContent` fails it where `toBeVisible` passed.
### the_shipped_demo_seeds_a_worker_whose_raw_cells_are_settable
improvise-ir7l renamed this from `..._whose_every_cell_is_settable` and
narrowed what it claims. The old loop called `MainThread::cell` — which panics
on anything but a stored cell — over *every* column including `Profit`, which
`web/demo.improv` defines as `Revenue - Cost`. It passed only because a formula
target projected `CellId::Record`, which is the bug the ticket names: a test
whose name was false for a third of the grid it walked.
It now walks the two raw columns and asserts separately that the `Profit`
column is `CellId::Derived`, so it fails if either half moves. That second
assertion is what makes the narrowing honest rather than a retreat — dropping
the column would have left the page free to offer it again with nothing
noticing.
### the_page_keys_turn_the_odometer_as_the_command_does
improvise-p1ym. `GridCache::page_coords` carried `(category, selected item)`
and nothing else, so `[`/`]` resolved to a command name the client held but
could not compose an argument for. They reached `Host::handle`, which answers
an empty vec — `doctrine(an_upstream_key_is_a_black_hole)` — so an independent
smoke test found them inert against the live page, where `Quarter` sits on the
page axis precisely because paging is a thing a visitor tries.
The test asserts the *shape* composed, not that something resolved: two page
axes, the least significant at Q4, so one press must both wrap `Quarter` and
carry into `Year`. A resolver that moved only the first axis, or carried the
wrong way, fails it.
### a_filter_axis_is_projected_but_not_cycled
The gotcha-27 arm. Page and Filter both constrain which records are visible,
and `GridLayout::page_coords` is deliberately `filtering_cats()` (Page ∪
Filter) because the *layout* does not care which the user can navigate. The
projection copied that verbatim, so a client handed the list would have tried
to cycle a drill's fixed coordinate.
`PageAxis::items` is `Some` only for `Axis::Page`, so the difference is in the
type rather than in a rule each client remembers. The test adds a Filter entry
to a client that is otherwise paging normally and asserts the two Page axes
still move while the Filter one does not — an assertion that only the Filter
axis is skipped, not that paging stopped.
### a_locally_resolved_page_turn_lands_where_the_host_would_put_it
The lockstep claim, and **the first version of it was vacuous**, which is worth
recording because it looked exactly right. It rebuilt the client's cache from
the host after every press and then compared the client's cache against that
same host's document — two readings of one value. Reversing the odometer's
significance in `page_cycle` left it green.
It now drives two independent documents: one `Host` fed the commands the client
composed, one `App` fed the same keypress through `reduce_key`, compared on
`View::page_selection` — the document, never a projection. Proved discriminating
by that same injection (`for i in (0..axes.len()).rev()` → `for i in
0..axes.len()`), which now reports Q2 against Q3. The `assert_ne!` against a
fresh fixture is the second half: without it, a resolver that emitted nothing
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
`web/tests/render_surface.test.js` and `harness/tests/contract.spec.js`. What
is *not* carried over is each key's meaning, which `improvise_protocol::resolve`
decides and tests. Everything below is the history of the bug, and still true.
A user reported three things as broken: "I can't expand categories, and the
Tile + View UIs don't work." None of the three was a regression. Driving the
assembled site under Chromium separated them, and the separation is the point:
two of the three had *every* layer but the last one, and the layer they were
missing was not the same in each case.
**Category expand already worked, by keyboard.** `Shift-C`, `j`, `Enter` folds
`Command::ToggleCatExpand` client-side and the items appear. What does not work
is the *mouse*: `categoryList` builds plain `<li>`s with no listener, so a
click on the triangle does nothing at all. That half is improvise-7ybz, and it
is a product decision about the whole page's pointer model rather than a defect
in an implemented feature — `#grid` offers a click only on an editable cell, so
there is no click-to-select either.
**The tile bar and the view panel had the cache, the keymap, the resolver and
the host round trip, and no DOM.** Before the fix, `render_state()` in the
browser carried five `TileBarCache` entries and both `ViewListCache` views;
`Shift-T` entered `TileSelect` and `ArrowRight` moved `tile_cat_idx`; `r`/`c`/`p`
reached the host and really did move a category between axes. `RenderModel`
simply had no field for either, and `web/index.html` had no element. So the
keys worked and the user was aiming them at nothing — which reads exactly like
"the Tile UI doesn't work". The formula panel was the third of `Panel::ALL` in
the same state and is fixed by the same mechanism.
**The fault injection was the unfixed bundle itself.** Run against the tree at
`a3bef6e`, the category test passes and the other four fail — `#tiles`,
`#views` and `#formulas` resolve to zero elements. That is the evidence the
four tests are discriminating, and it is why the category test stays: it is the
control that says the harness really is driving the keyboard.
Two assertions in the first draft were wrong, and both corrections are worth
keeping. `Axis::short_label()` is **three columns**, so a `Page` tile reads
`Quarter Pag`, not `Quarter Page` — a test asserting the long spelling would
have forced the page to invent a label instead of echoing `Axis`'s own. And
switching views leaves the panel **open**: `switch-view-at-cursor` returns to
Normal and never closes the panel, exactly as `src/draw.rs` keeps drawing it
off `view_panel_open` alone. Asserting it hidden would have pinned the browser
to behaviour the TUI does not have.
The gating rule itself — `None` is what says a panel is closed (it was an empty
list, the rule `help` follows, until improvise-df2w) — is asserted in Rust as well, by
`a_closed_panel_projects_no_rows_and_an_open_one_does` and
`the_tile_bar_is_always_present_and_only_its_cursor_is_modal`, because a browser
test cannot distinguish "closed" from "the cache is empty" and those two tests
fill the caches first.
What these tests do **not** cover: the pointer (improvise-7ybz), and the
inconsistency that `#views`/`#formulas` honour their open flags while
`#categories` is drawn unconditionally because `app.js` has never read
`category_panel_open` (improvise-wsaw).
One assertion was added after the first green run and is worth naming: the tile
test originally asserted only that the *tile label* changed to `Product Col`.
That is the bar reporting itself, which is exactly the shape of test this repo
keeps catching — it would pass against a tile bar that displayed an axis the
document had not taken. It now also asserts the grid reshaped: `Espresso` moves
from a `tbody` row header to a `thead` column header. `.first()` is needed there
because Product then shares the column axis with `_Measure`, so the label
appears once per header level — which is itself a small confirmation that
gotcha 44's multi-level frame reaches the browser.
**Three review findings, applied.** The second is the one worth carrying.
**The page had two spellings of the same triangle, inside the file that
declares it has one.** `indicator()` carries the comment *"only this spelling is
the page's (gotcha 26)"* — singular — and `categoryList` did not call it: it
inlined U+25B8/U+25BE (the SMALL triangles) where `indicator()`, the page's own
grid group headers and `src/ui/category_panel.rs` all draw U+25B6/U+25BC. So the
browser's category panel disagreed with the TUI's *and* with its own grid.
`categoryList` now calls `indicator(!row.expanded)`.
**And the first version of this spec pinned the wrong spelling.** It asserted
`/▸ Product/` and `▾` — the divergent pair — so the obvious cure, which the
file's own comment implies should already be in place, would have *failed a test
this work added*. That is this repo's characteristic defect exactly: not a wrong
line, a test that cements a state the doctrine says is wrong, and it makes the
cure more expensive the longer it sits. The spec now asserts `▶`/`▼`, and the
change was proved discriminating by running it against the pre-fix bundle, where
it fails with `Received string: "▸ Product (4) R"`.
Note what this does **not** settle: whether glyphs should move to the protocol
the way `short_label()` did. That is **improvise-h04z**, not answered here —
what is fixed is the page contradicting itself. `viewRow` still spells `"▶ "`
as a literal duplicating `src/ui/view_panel.rs:62`; the two agree today, which
is precisely the state `categoryList` was in before it drifted.
**And the drift was already spreading, which is the evidence that ticket needs.**
`harness/tests/pointer.spec.js`, written on a different branch, had
*independently* grown the same `▸`/`▾` assertions — so the corrected
`indicator()` broke it too. Three files converging on a spelling nothing
compares is the failure design-principles §5 predicts, observed rather than
argued, and it is what answers "is one duplicated literal worth a protocol
change" with data instead of doctrine.
**`el.tiles.hidden` was a no-op**, removed with its comment. `#tiles` declares
`display: flex`, an author-origin rule that beats the UA sheet's
`[hidden] { display: none }` at any specificity, so the guard never hid
anything and "an empty tile list draws nothing at all" was false. `#views`,
`#formulas` and `#help` declare no `display` and so are genuinely hidden by the
attribute. Worth knowing before anyone "fixes" it: because `#tiles` is always
laid out, its height never changes, so it feeds nothing back into the
`ResizeObserver` — restoring the guard would introduce exactly the first-paint
height change the observer argument assumes away.
**The `ResizeObserver` loop is no longer "argued, not tested", and the proof is
worth more than the measurement.** It was measured: 820 combinations — four
panel states (none / views / formulas / tile-select) × heights 360-440 in 2px
steps × five widths straddling the `COL_PX = 110` boundary, each counting
`#grid` child replacements in a 500ms quiet window. Every size settled; nothing
repainted twice. But the reason is structural, so the measurement is a check on
it rather than the evidence itself: **the `frameCost` dependency graph is
acyclic.** `frameCost.cols` is `row_levels`, the number of row-axis header
*levels*, which does not depend on how many rows are projected.
`frameCost.rows` derives from `col_levels.length` and `col_groups`, which depend
on the projected *column* window, which depends on width and `frameCost.cols`.
So `cols → rows`, and nothing depends on `rows`. There is no cycle to oscillate.
A change that made either term depend on the projected row count would create
one, and that — not the pixel arithmetic — is the thing to check.
**A harness port collision can make a run test somebody else's tree, silently.**
`playwright.config.js` hard-coded 8099 with `reuseExistingServer: true`, so a
`python3 -m http.server` left by another worktree is *adopted*, and the run then
exercises that checkout with no warning. The port is now
`IMPROVISE_HARNESS_PORT`, defaulting to 8099 so CI is unchanged. The tell is a
result that disagrees with the file you just read: check
`lsof -a -p <pid> -d cwd` on the listener before believing it.
**It fires in both directions, and the dangerous one is the false PASS.** Two
agents hit it in one session. One got a false *negative* — a glyph fix reported
absent, because the adopted tree did not have it — which is merely confusing,
since a failure sends you looking. The other got `widgets.spec.js` reporting
**5 passed against a tree it had content-verified as having no `#tiles` and no
`paintTiles` at all**, which is impossible and was caught only because that
agent knew what the tree contained and refused to believe the number. **A false
pass prompts nobody.** That is what makes a hard-coded port with
`reuseExistingServer` a correctness bug rather than an inconvenience, and it is
the same rule as the rest of this file: a green result is evidence only once you
know what it measured.
### a_click_focuses_the_panel_moves_its_cursor_and_activates_the_row
improvise-7ybz's whole difficulty in one assertion, and the reason the click
resolvers take an **index** rather than reading the cursor.
A click means three things at once — focus the panel, move its cursor to the row
under the pointer, activate that row — and the third is resolved in the *same
pure call* as the first two, before any of them has been folded. So
`filter_to_item_at(state, index)` must be asked about `index`; asking
`filter_to_item(state)` reads `view.cat_panel_cursor`, which is still wherever
the keyboard last left it. That is why `cat_tree_entry`, `toggle_cat_expand`,
`filter_to_item` and `tile_axis` each gained an `_at` twin and the cursor-reading
form became a one-line call of it: one definition, two entry points.
**Injected fault**, run: `commands.extend(filter_to_item_at(state, index)…)` →
`filter_to_item(state)`. The test fails with
`ToggleCatExpand("Region")` against `ToggleCatExpand("Product")` — the panel
cursor lands on the row clicked while the *expansion* happens to the row that
was selected before, which is the silent half. The fixture therefore has two
categories and clicks the **second**: with one category, or with a click on the
row already under the cursor, the bug is invisible.
The sibling `a_click_in_an_already_focused_panel_only_moves_and_activates` pins
the other half: `focus` emits only what is not already true, so a click inside
the focused panel is not a mode change on every click. Without it, "always emit
`SetPanelOpen` + `ChangeMode`" passes the first test.
### a_cell_click_is_absolute_even_though_the_page_paints_a_window
The pointer's version of `the_selection_is_placed_by_subtracting_the_scroll_offset`,
and it fails the same way: `GridCache` is viewport-indexed, `ViewState::selected`
is absolute, so a click at painted `(2,1)` under offsets `(12,3)` is `(14,4)`.
Offsets are **nonzero and unequal** for the same reason as its sibling — equal
offsets cannot separate "reconciles" from "swaps the two". The second half
asserts a click past the grid's extent resolves to nothing, which is what stops
a click on the padding below the last row selecting a cell that does not exist.
### a_click_past_the_last_row_resolves_to_nothing
`doctrine(check_the_checker)` for the two above: the empty-answer path. Without
it, a `click` that returned `Vec::new()` unconditionally would pass every
"resolves to nothing" expectation while failing silently everywhere else — so
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
painted — against three surfaces. What each click *means* is
`improvise_protocol::resolve`'s, and its tests are named below. The six tests'
injection stands as recorded.
The user's ruling on improvise-7ybz ("build it — one entry point per widget")
and improvise-wsaw ("honour the flag"), executed in a real browser.
Six tests, and **all six fail against the bundle built immediately before them**
— the tree that already had the three widgets drawn (PR #55) but no click
handlers and no `category_panel_open` read. That is the injection: the sidebar
is visible when it should be hidden, and every `click()` resolves to a `<li>`
that has no listener, so nothing moves. The category test is the sharpest,
because the element it clicks *exists and looks interactive* in the unfixed
tree — which is exactly what the user hit.
Two properties here that the Rust tests structurally cannot reach. **The
pointer and the keyboard are one cursor**: `the keyboard continues from where
the pointer left the cursor` clicks a cell, closes the editor, presses
ArrowLeft, and requires the move to start from the clicked cell — if the click
had only opened an editor (the pre-fix behaviour) the arrow would move from the
origin instead. And **the tile click is asserted against the grid**, not only
against its own label: `Espresso` must appear as a `thead` header, because a
tile bar reporting an axis the document did not take is the failure shape this
repo keeps finding.
One fixture note worth keeping: `openPanelThenLeave` opens the panel with
`Shift-C` and then presses `Escape`, which returns to Normal **without closing
the panel** (`PanelKeymap`'s Esc is `enter-mode normal`, nothing more). That is
the only state in which the "focus" third of the click is observable — from a
freshly opened panel the mode is already `CategoryPanel` and the assertion
would pass without the focus commands existing at all.
#### harness/tests/drill.spec.js — the file's own scope
The fourth widget the user reported, and the one with a **different answer from
the other three**. The tile bar, view panel and formula panel were each one
`RenderModel` field short — cheap. Drill is not: **there is no wire vocabulary
for it**, which is improvise-gd28, and these tests document the refusal rather
than pretending the gap is a bug to be fixed here.
**Where it stops, by layer.** Everything below the command exists and was
verified in the browser, not read: `project::grid_of` emits `CellId::Aggregate`,
the client receives it (`{"Aggregate":"750ab1f5ffa96d5e…"}` in `render_state`),
`render.rs::editable` correctly answers false for it, and `resolve.rs` reaches
the aggregate arm. What is missing is one link: `reduce.rs` contains **no
`Drill` variant at all** — not `Command`, not `ModelCommand`. The seam is ready
and the sentence is not sayable.
**Why it was left out is a decision, not an oversight** (gotcha 38): drilling
creates the `_Drill` view and writes `drill_state`, which gotchas 30 and 1 rule
**session-local** — `pending_edits` are keyed by `(row, RecordCol)` and belong
to whoever staged them. A `Command::Model` is routed with **no `SessionId`**,
which is right exactly when a change is addressed to *no one* session; drill is
addressed to precisely one. That is the same argument that removed
`ApplyAndClearDrill` from the wire in improvise-3x3u, so adding
`ModelCommand::Drill` would reintroduce the bug that removal fixed. It needs the
**addressed** form — ADR 001 layer 2.
**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 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
isolated to the client/wire, not a general regression.
**A fixture fact that is itself part of the answer.** `web/demo.improv` has **no
aggregate cell at all** — gotcha 46 puts exactly one category on each visible
axis precisely so every cell is a `Record`, and `tests/web_demo_model.rs`
asserts it. So on the shipped demo a user cannot even reach the refusal without
first changing an axis. `anAggregateGrid` therefore *makes* one, through the
tile bar, the way a user would: `Quarter` cycles Page → None and every cell
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** — 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. 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.
### an_aggregate_edit_names_the_same_missing_drill_as_the_drill_key
improvise-s8h7. Until it, this test was
`the_drill_key_and_an_aggregate_edit_refuse_for_different_reasons` and asserted
the opposite: that `i` and `>` on an aggregate must refuse with *different*
sentences, "this key does nothing here yet" against "this cell is not one you
may write". The second half was false about the application. `EditOrDrill`
hands an aggregated cell to `DrillIntoCell`, so in the TUI `i` on an aggregate
**is** drill — nothing refuses the write, it drills — and the browser's
separate wording ("drilling into an aggregate needs the host") hid that `i` and
`>` are one missing feature. The reasoning is `context/code/clients.md`.
So the test now asserts four things on one aggregate fixture: `>` names drill
and the browser; `>` does **not** mention aggregates (it is refused whatever the
cell is); `i`'s sentence **contains** `>`'s sentence verbatim; and `i`'s
sentence mentions "aggregate", so it still says why it drilled rather than
edited. The fixture is `client_on(CellId::Aggregate(..))` rather than the old
empty `in_mode` state, because `i` needs a cell under the cursor to reach the
`Aggregate` arm at all — on an empty grid it answers nothing and only `>` was
ever exercised.
**Three injections, each failing at a different assertion.** Run, not reasoned:
- `i` says only `not_yet("drill")` — the "why" dropped. Fails at the last
assertion, "and `i` must still say why". This is the tempting over-tidy fix.
- `>`'s `declined` subject becomes "drilling into an aggregate". Fails at the
anti-aggregate assertion on `>`. Note that `i` still contains `>`'s *old*
sentence here, because `edit_or_drill` calls `not_yet("drill")` itself rather
than going through `declined` — so the containment assertion alone would not
have caught a `>` that started blaming the cell.
- `i` paraphrases ("… drill needs the host"). Fails at the containment
assertion: the same feature named in different words is exactly what the
ticket reported.
The harness sibling (`drill.spec.js`) asserts the same wording in a browser.
Its `not.toContainText("aggregate")` on `>` is still **decoration under the
only injection that exists** — Playwright throws at the positive assertion
above it first — so the Rust assertion is the guard and the harness is
end-to-end confirmation that the string survives the wire.
### the_edit_key_row_says_it_drills_into_an_aggregate
improvise-s8h7, the help-page half. The `edit-or-drill` row on the Editing page
read "Edit current cell", while `i`/`a` on an aggregate drill in the TUI and
name drill in the browser. The row is found by its **keys** — the labels
`keys_for(Normal, "edit-or-drill", ["editing"])` returns — not by its caption,
and exactly one row must carry them, so a caption fix that also moved or
duplicated the row fails on the `let [..]` pattern rather than passing on
whichever copy was found first. Failed at HEAD on the caption; the injection
"Edit current cell, or drill" (drill named, aggregate not) fails it too, since
`>` is also drill and the row must say *when* `i` drills.
### only_the_edit_key_is_captioned_as_editing_the_cell
Found while fixing improvise-s8h7, not predicted by it. The row below the edit
key read `Enter Edit current cell (same as i)`, but Normal-mode Enter is bound
to `enter-advance` alone — not a `Binding::Sequence` — and `EnterAdvance` only
moves the cursor down, wrapping to the next column. The test takes every help
row captioned "Edit current cell" and requires its keys to be a subset of
`edit-or-drill`'s; at HEAD it failed naming `["Enter"]`. The fix moved the row
to the Navigation page's "Cursor movement" section with a caption that says
what it does.
Its reach is stated rather than implied: it checks the one phrase that was
false. A caption that misdescribes Enter in *other* words passes it: a caption
is prose, and nothing checks it against the command it documents. That is a
neighbour of improvise-wk18, not the same gap — wk18 is about `literal()` rows
whose *keys* go unchecked; here the keys were right and the words were wrong.
**improvise-jhdf closed the paraphrase, and the paragraph above is what it
found.** Rewording Enter's caption to "Edit the cell (same as i)" — the same lie
in new words — passed both help tests. The test now asks by **command**, which
wk18's `help::help_bindings()` made possible: every Normal-mode help row whose
caption says "edit" in any case must run `edit-or-drill`. It fails on that
rival naming `(Normal, "enter-advance", None, "Edit the cell (same as i)")`.
Two controls stop it passing by looking away: `edit-or-drill` and
`enter-advance` must each still have a Normal row, or the filter has nothing to
catch. What it still cannot see is a false caption that avoids the word — "Opens
the cell" — which is the honest limit of any check over prose.
### 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 the `reduce_view` guard needed the same
repair independently. **Since improvise-56ek there is one derivation**, so that is
history rather than a standing hazard; see below.
### a_panel_command_writes_that_panel_and_no_other
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.
improvise-56ek then removed the second derivation entirely: `ViewState::panel_open_mut`
and `panel_cursor_mut` (plus their read halves) are the one place a `Panel` becomes
a field, and both `effect::SetPanelOpen::apply` and `reduce_view` call them. The
cure is one home, not a test holding two copies in step
(`doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished)`), so
there is deliberately no "the two agree" test — a wrong field in the one home is a
failure in every caller.
This test replaces `panel_commands_address_the_named_panel_only`, which drove
`Panel::Category` only and so could not tell one panel from another. It keeps that
test's lesson by zeroing every panel before opening one.
Injected fault: point `Panel::Category`'s arm at `formula_panel_open`. Fails by
name for every panel in `Panel::ALL`.
**That injection alone was not enough, and the gap is gotcha 3's shape.** The first
version asserted only through `panel_open`/`panel_cursor` — the read halves of the
mapping it was testing. A *consistent* permutation of all four accessors is a
self-consistent bijection: injecting Formula↔Category into `panel_open_mut`,
`panel_open`, `panel_cursor_mut` and `panel_cursor` together left the test passing,
while two unrelated tests that read raw fields failed. It caught only a
**non-injective** mapping, which is exactly the injection first recorded here. The
test now reads the three field pairs by name once per panel, and the consistent
permutation fails it: "Formula is not the field Formula names".
Also corrected here: the claim that consolidation left "no second copy to agree
with" was false when written. `resolve.rs` carried a fourth copy of the same
`Panel` → `ViewState` three-way match, **in the same crate on the same type**; it
is deleted and its three call sites go through `ViewState::panel_open`. What
remains is *not* the same mapping: `registry.rs` maps `Panel` to `CmdContext`
fields, a different type whose values are copied from `view_state`, and
`resolve.rs`/`registry.rs` each map `Panel` to a (cursor, collection length) pair,
which is a second derivation of something else and is `lockstep.rs`'s subject.
### 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
No `###` headings here on purpose: `--check-evidence` resolves a heading to a
Rust `fn`, and a JavaScript test name under one would read as an orphan. These
are keyed by file.
Every test below was proved able to fail by patching the code it names,
checking the patch was on disk, running the suite and reading the failure —
never by the absence of an error. Thirteen injections, thirteen failures:
**What `shell.test.js` cannot reach, and the PR found out the hard way:** it
tests `createShell` in isolation, so it sees the watchdog RULE and never the
`app.js` wiring that calls it. That distinction is now written into
`context/code/clients.md` and guarded in `shell.spec.js`.
**`shell.test.js`** — `socketUrl` stops upgrading `ws://` on an https page;
`composeStatus` returns the model fact alone (which fails five tests, because
the watchdog's messages are shell facts and vanish with it); `fail()` stops
clearing the timer, so the watchdog *pre-empts* rather than being disarmed —
the bug PR #13 fixed and PR #21 half-lost; `extentOf` stops subtracting the
frame; `frameCostOf` stops counting the column-group line.
**`grid_plan.test.js`, the totals strip** (improvise-p7lh) — `plan.totals` is
`null` where the strip is empty and never an empty row, because an empty strip
means "this grid has no totals" rather than "not caught up yet" (gotcha 33). The
label spans every row-header level, and the cells carry no coordinates at all —
a total is not a cell a click can name, which is what keeps the delegated
`td[data-row]` handler off the `tfoot` without a second rule saying so.
**`grid_plan.test.js`** — the data-row index is taken from the row's position
in `rows`, so a group header consumes one (the click coordinate bug gotcha 1
is about); the group label is tested for truthiness, so a group legitimately
named `""` draws as "in no group"; `typing` is `Boolean(model.editing)`, so an
empty edit buffer reads as no editor open.
**`serial.test.js`** — the chain is replaced by `Promise.resolve().then(...)`,
which is what an `async` listener does: the slow first frame is overtaken. The
suite also keeps that fault as a *passing* test in its own right, asserting the
unchained pair really does come back swapped, so the ordering test cannot be
green for want of a race.
**`render_surface.test.js`** — a `pub sparkline: String` added to `RenderModel`
is reported unpainted; deleting `paintTiles` reports `tiles` unpainted; a
`getElementById` that names an id `index.html` has not got is reported. The
first is the shape of the bug the four widgets shared, and the list it checks
against currently names `doc_name`/`doc_dirty`/`doc_stored_as` — improvise-5jna,
which this test found rather than remembered.
**`dispatch.test.js`** (improvise-b8ot, improvise-p7lh) — the delegated
handler's operands are read back off the node, so the round trip
`cellAction(cellAttrs(cell), …)` is the property, and the fixture puts a group
header between its data rows because that is the one input where "the data row"
and "the table row" differ. Three injections, three failures, each run and read:
- `gridPlan` takes the row's position in `model.rows` rather than counting data
rows, so a group header consumes an index. This is the click-coordinate bug
gotcha 1 is about, and it is **invisible on any ungrouped grid** — which is
why the fixture has a header between its rows. Fails both round-trip tests.
- `whole()` drops its empty-string guard. `Number("")` is `0`, so a header with
no `data-row` reads as row 0 and every click on the frame moves the selection
to the origin — which is exactly why the guard is not a truthiness test.
Fails the two "means nothing at all" tests.
- `pageEntries` reports `navigable: true` unconditionally. Fails the Filter
test alone, and nothing else — the Page case cannot see it, which is the
point of seating a Filter axis in the fixture at all.
**`render_surface.test.js`, the cache direction** (improvise-p7lh) — the
existing guard is closed under *what `RenderModel` chose to expose*, so a fact
that never reaches it is invisible to it. That is precisely how `page_coords`
and `totals` sat projected and unpainted from improvise-n10 onward with a green
suite. The new assertion is one level up: every `pub` field of every widget
cache on `ClientState` must be read somewhere the browser actually runs, or be
named as debt.
Two things it got wrong first, both found by injection rather than by reading:
- The whole-cache escape hatch was a substring test for `&state.grid`, and
`&state.grid.row_frame` **contains** that string — so every `GridCache` field
read as handed-over and the check was vacuous for the one cache the bead is
about. It is now anchored with `(?![.\w])`.
- rustfmt breaks `state.grid.totals` across three lines, so a substring search
reported a field that *is* read as unread. The sources are whitespace-stripped
before the search.
The residual hole, stated because it is real: where a whole cache is handed to a
shared derivation (`flatten_cat_tree(&state.categories, …)`), every field of that
cache counts as reached. A second field added to `CategoryTreeCache` and ignored
by the derivation would pass. Closing that needs the *consumer* checked, not the
call site, and is not attempted here.
**`bundle/wasm_surface.test.js`** — `client.render_model()` renamed to
`client.render_modal()` fails both its tests. It reads the generated `.d.ts`, so
a missing bundle is an assertion failure and never a skip.
**`web/assemble.sh`** — dropping `web/shell.js` from the copy list, with the
page's own imports as the check. The first version of that guard anchored its
`sed` at `^import`, so it could not see the **multi-line** import that is the
only one it exists for, and the injection passed: the guard was written, landed,
and blind. That is what the injection was for.
## `harness/tests/contract.spec.js` — eight injections into the served page
Patched in `harness/serve/app/app.js`, which is what the browser loads, so no
rebuild stands between the fault and the run. The sidebar ignoring
`category_panel_open`; the tile bar, view panel, formula panel and help overlay
each never drawn; `click_cell` sending a constant row; `click_category` sending
a constant index; and the typed buffer drawn without its caret. All eight fail.
The last one did **not** fail at first: the test asserted the `typing` class and
not the text, so a cell drawn blank passed. The assertion was strengthened until
the injection failed, which is the only reason it is worth having — and it is
the same lesson as the eleven tests above: a class is a flag, and a flag is not
a pixel.
### every_advertised_command_answers_in_its_own_mode
improvise-rmvx. Gotcha 47 says an upstream key is a black hole, and
`the_command_names_that_still_need_the_host_are_these` is the guard that was
supposed to keep the list of them honest. It was built with
`in_mode(AppMode::Normal)`, so it only ever asked about Normal-mode bindings:
`add-record-row` (RecordsNormal) and `delete-formula-at-cursor` (FormulaPanel)
were outside its reach entirely and could never have failed it. **A named list
that silently scopes itself to one mode is a guard that looks broader than it
is.**
This asks it of every mode, and it asks it the only way that is not
over-reporting. The naive version resolves every advertised name against an
EMPTY `ClientState` and flags `edit-or-drill`, `commit-cell-edit`,
`create-and-switch-view` and `page-prev`/`page-next` — but those are *handled*
names refused by STATE, which is correct behaviour and not this bug. So the
fixture is `furnished`: every widget has something in it, a basis is published,
and a window is announced, so an `Upstream` can only mean the name is
unhandled.
Two things it does NOT do, each because the first attempt did and was wrong.
It does not call `step` with the advertised args: `advertised` carries `None`
for "any binding of this command", and `step`'s numeric and mode arguments are
mandatory, so that reports `move-selection`, `move-panel-cursor` and
`move-tile-cursor` as gaps in every mode. And it does not tokenise the rendered
hint string: `help::advertised` is a table of command NAMES, so the
cross-reference is exact and anything reading the rendered text is solving a
harder problem than exists. The keys come off `Keymap::keys_running`, which
`labels_for` now shares so "which keys" and "how are they spelled" cannot give
two answers.
**It watches BOTH silent variants, and the first version did not.** `KeyOutcome`
has two ways of saying nothing: `Upstream`, and `Ignored` — which
`KeyOutcome::of(vec![])` produces whenever a resolver composes an empty vector.
To a user they are the same black hole. The first version asserted
`resolve(..) == Upstream(event)`, so **three of the six commands this whole
change exists to make audible could go completely silent with the entire
evidence base green**: degrading only `add-record-row`, `delete-formula-at-cursor`
and `view-back` to `Step::Declined(Vec::new())` left 1397 Rust tests, 19 harness
tests and 26 node tests passing, because no browser spec presses those three
keys. Found on review of PR #71. The check is now `!matches!(.., Upstream(_) |
Ignored)`.
**The generalisation is worth more than the fix.** `KeyOutcome::of` turning an
empty `Local` into an honest `Ignored` is good design and it is also a
**laundering** step: every wrong answer that composes nothing arrives wearing
the variant that is usually innocent. So a guard's discriminator has to be
*state* — did anything the user can see change — and never the shape of the
return value. That is exactly why the `+cat` injection in
`a_locally_committed_creating_column_agrees_with_the_host` failed (it compares
documents and cursors) and the decline injection passed everywhere (nothing
compared a screen).
Widening the check costs ten more entries, all one legitimate boundary case:
`move-panel-cursor` mirrors `MovePanelCursor`, which returns nothing when the
cursor does not move, so a one-row panel or a cursor on an edge answers
`Ignored` for a reason that is not this test's subject. `furnished` therefore
carries **three** of everything with all three panel cursors seated at index 1.
Note what that rules out: seating them at 0 and calling the `Up` results
expected would have hidden the real defect in the same list as the boundary
case.
**The injection**: delete the `"toggle-prune-empty"` arm from `declined`. The
test fails naming `NORMAL: Char('P')` and `RECORDS: Char('P')` — both modes,
which is the widening the ticket was about, and a build that ran rather than a
compile error. The review's own injection is also kept as a check: degrading the
three unpressed declines to empty now fails naming `FORMULAS: Char('d')`,
`FORMULAS: Delete`, `RECORDS: Char('<')` and `RECORDS: Char('o')`.
What it found that the ticket did not predict: `view-back` is advertised in
RecordsNormal (`<`, captioned "back"), so it was a seventh instance of the same
class. The ticket had it filed under the Normal-mode host list instead, where
being upstream is not a defect.
The five that remain are listed by name with a reason each. Three of them —
`commit-export`, `commit-save`, `execute-command` — are reachable only from a
mode `declined` refuses at the door, so a browser never stands in front of the
hint; the other two are improvise-gcvb's records-cell commit, which needs
`record_col`.
**improvise-3t5x and improvise-wk18 widened it again, in two directions, and the
list above is superseded by the one in the test.**
*What "answers" means.* It counted any `Local` as answered, so a lone
`SetStatus` refusal passed — which is why improvise-s8h7's false "Edit current
cell" stayed green. Injected to show it: make `edit_or_drill`'s `Record` arm
return only `SetStatus("RIVAL")`, so `i` refuses on every cell — the old guard
**passed**. The line drawn now: a caption is a promise, so an advertised key
must *act*. A refusal is legitimate — a feature a browser honestly lacks should
say so rather than go silent — but it is not what the caption says, so it has to
be **named** in the list with its reason, like a black hole. The test therefore
sorts every key into three outcomes, `Acts`, `Declines` (said something, did
nothing) and `Silent`, and lists the last two. The same injection now fails
naming `NORMAL: a/i edit-or-drill Declines`.
*Judged by what moved, not by the variant.* The first version of the new
classifier called anything but an all-status `Local` an action, and a rival that
unhooked `jump-first-row` from `composed` **passed** — because a chord's second
key always returns `Local([SetTransientPrefix(None), ..])`, the disarm, even
when nothing else happens. That is the laundering this entry already warns
about, one level down: `KeyOutcome::of` launders an empty vec into `Ignored`,
and `chord` launders a no-op into a `Local`. The disarm is now ignored when
deciding whether a key did anything, and the same rival fails naming `NORMAL:
gg jump-first-row Silent`.
*Which keys.* It read `help::advertised`, the status-bar table, and the help
overlay is a second body of advertising the browser also paints
(improvise-uln5). It now also reads `help::help_bindings()`, which records every
keys row the page builders ask for — the same builders `pages()` renders, so the
two cannot drift — including prefix sequences (`gg`, `yy`, `ZZ`), which it
presses by arming the prefix first. That is what added the five host-only
black holes (`/`, `n`, `H`, `z`, Normal's `Enter`) to the list: each was already
in `the_command_names_that_still_need_the_host_are_these`, and the help page
advertised it anyway.
*Which cells.* `furnished` stood on a record cell only, so `i` on an aggregate
(s8h7's whole subject) was outside its reach. Normal mode is now pressed on a
record, an aggregate and a derived cell, and a non-record cell is reported only
where it changes the answer, so the list does not triple. `furnished_on` also
carries a history in both directions and a yanked value, so `<`, `,` and `p`
are not refused by an empty stack — state refusals are correct behaviour and
not this test's subject, the same argument that gave `furnished` three of
everything.
### a_locally_committed_creating_column_agrees_with_the_host
improvise-gcvb. `resolve`'s commit returned `None` for the whole of records
mode, and the reason given was sound for the coordinate and measure columns —
a category column renames every measure cell in the row, which needs
`record_col` (gotcha 1). The guard was broader than its reason: a cell at the
cursor now projects `CellId::Create`, and committing one is a single existing
`ModelCommand` over the typed NAME that reads no `record_col` and writes no
cell.
Both halves are proved by injection, and the second is the one that matters.
Putting the broad `is_records()` early return back makes the test fail at "the
client must answer this key locally". Deleting the `CreateKind::Category` early
return — so `+cat` advances like any other column — fails with the client in
`RecordsEditing` where the host is in `RecordsNormal`: the TUI returns straight
after a `+cat` commit with a `RebuildLayout` and no cursor move, because an
inserted column renumbers every index to its right and the advance was computed
from the pre-commit layout (gotcha 23). A client that advanced anyway would land
one column off, silently.
`both_ends` compares the document as `.improv` text, the mode, the status and
every cursor, so neither half can pass by agreeing on the wrong thing. The
fixture drives *both* hosts into records mode with the `R` key rather than
hand-building a `_Records` view, because `twin_of` copies only the session
slice and a twin without the view would refuse the commit for the wrong reason.
**Scope, narrowed by improvise-bd9o — read "this key" above as `Enter`, not as a
key a user can reach.** The fixture puts the client in `RecordsEditing` itself;
since bd9o the records decline sits at the *entrance* of `edit_or_drill`, so no
browser key path opens that mode and `create_column` is unreachable in anger
(improvise-x0xd). What this test still pins is `commit`'s own narrowing — that
the exception exists in the function and has the right advance behaviour — so
that reviving the door is a change to `edit_or_drill` alone and not a
rediscovery of gotcha 23. Caught by ChatGPT Codex on PR #128: the bd9o section
of `context/code/clients.md` said "explicitly dead" while this entry and the
"classes that remain" bullet still read as though a user key reached it. **A
fact stated in three places is changed by a grep, not an edit** — and the edit
that created the drift was the one that *added* the third place.
### every_minibuffer_mode_paints_what_is_being_typed
improvise-fdt6. `commit-buffer formula` already resolved on the client — that
half of the ticket was closed by an earlier wave — so the browser could enter
FormulaEdit, type a formula and commit it. What did not exist was any way to
SEE it: `RenderModel::editing` is populated only when `mode.is_editing()`, which
is the two *cell*-editing modes, and it reads the `"edit"` buffer by name. Every
character typed into FormulaEdit, CategoryAdd, ItemAdd, SavePrompt,
ExportPrompt or CommandMode went into a buffer nothing painted. The mode opened,
the user typed, and the screen did not move — which is exactly the dead-room
shape the ticket names, arrived at from the rendering side rather than the
resolution side.
The value is read with `improvise_protocol::read_buffer` rather than
`buffers.get`, so the `"search"` special case (gotcha 11) keeps its one home.
**The injection**: `.filter(|_| state.view.mode.is_editing())` on the new
field, which is the pre-fix behaviour exactly. It fails naming `"formula"`.
**And the static check was not enough, which is the part worth keeping.**
`web/tests/render_surface.test.js` asserts that every `pub` field of
`RenderModel` appears as `model.<field>` somewhere in `app.js`. It passed while
the page was still broken: the string `model.minibuffer` was inside
`paintMinibuffer`, and `paintMinibuffer` was never called and `el.minibuffer`
was never looked up. A field can be *mentioned* by a function nothing invokes.
`harness/tests/keys.spec.js` caught it on the first run, in the browser, which
is the whole argument for that directory existing (gotcha 48).
### nothing_declined_is_also_composed
`step` is `declined` then `composed`, and until this test the claim that the two
lists cannot overlap was a **doc comment**. The dangerous direction is not a
typo: it is a future ticket implementing `view-back` (or `toggle-prune-empty`,
which is one projection field away) in `composed` and not thinking to delete its
`declined` arm. The browser then goes on saying "not available in the browser
yet" while the working code sits unreachable, and nothing notices — the classic
shape of a check passing while the thing it names is broken. Raised on review of
PR #71.
It is **derived from `declined` itself**, not from a list beside it: it walks
every `(name, args)` bound in every mode's keymap — parents and prefix
sub-keymaps included, sequence steps included — and for each one `declined`
answers, asserts `composed` does not. A seventh decline is covered without
coming here, which is the property a hand-written list could not have.
**The injection**: add `"view-back" => Some(vec![Command::SwitchView(..)])` to
`composed`. It fails naming `NORMAL: view-back []` and `RECORDS: view-back []`.
It also found a real overlap on its first run. `enter-mode` is answered by
`declined` for the three stranding modes *and* by `composed` for every mode, so
`composed`'s arm would have opened `SavePrompt`, `ExportPrompt` or `CommandMode`
if `declined` were ever bypassed. Rather than moving the check, **both halves now
hold the rule** — `composed` returns `None` for a stranded mode — so the failure
direction if one is removed is the pre-PR `Upstream`, not a dead room.
---
## `harness/tests/help.spec.js` — the overlay, asserted as words
PR #61 deleted `"? opens help, and its rows are the shared ones"` and nothing
replaced it, so `paintHelp`, `help`, `help_page` and `help_titles` reached no
assertion at all (improvise-78v6). What `contract.spec.js` carried in its place
said only that a `dt` was VISIBLE — and an empty `dt` is a visible `dt`
(improvise-igxp), which is the same shape as the caret bug above: a row is a
frame, and a frame is not a word.
**The injection both tickets name**: blank `keys.textContent` in `paintHelp`,
patched in `harness/serve/app/app.js`. Before this work it left all three
contract tests passing. It now fails both tests in this file (the Welcome
banner reads empty; the advertised key reads empty) **and** `contract.spec.js`,
whose two help lines were strengthened from `toBeVisible` to the banner's text
and the first `dt.help-keys`'s text.
**What the pairs helper is for.** The overlay is a `<dl>`, and the association
between a key row and its description is document order — `dt` then `dd`. Read
as two lists they zip wrong, because a prose row emits **no** `dd` at all
(deliberately: an empty one would leave a blank grid cell). Welcome is prose end
to end, so `dd` count zero there is an assertion about that rule and not a
coincidence.
#### "a key the overlay advertises does what the overlay says it does"
The assertions above are still about a string on a screen; this one asks whether
the string is **true**. The key for `Jump to last row` is read OUT OF THE DOM,
the overlay closed, the key pressed, and the selection asserted onto the last
row. A page drawing a plausible key it does not have — what a hand-spelled table
decays into — fails here, and the spec holds no second copy of the keymap.
It presses the FIRST key the row names because a row is a compaction of every
binding that reaches the command (`h j k l ← ↑ → ↓` is eight).
## `harness/tests/grouped.spec.js` — the first browser run of improvise-anr9
`web/demo.improv` has no groups and one category per visible axis, deliberately
(gotcha 46), so the multi-level header, the group span, the collapse indicator
and the inline group row — exactly the code improvise-anr9 added — had never
been rendered anywhere a test could see (improvise-plcx). The fixture is
`harness/fixtures/grouped.improv`, served beside the shipped demo by
`harness/build.sh`, and the shipped document is unchanged.
**A missing seed is not an error**: `web/worker.js` answers a fetch that 404s
with an empty model. So a typo in the filename yields a page that draws a grid,
and "a grid is drawn" would pass on it. Every assertion here is about labels the
fixture really carries.
**`openGrouped` waits with retrying assertions and the tests then read once.**
The head and the row labels are read in a single `evaluate` each, because what
these tests assert is the RELATION between the lines — and a one-shot read
cannot wait. The grid is painted from the first projection and reframed by the
viewport the page then announces, so the helper gates on the head's shape, its
first group and the first row's innermost label before anything is read.
**Five injections, five distinct failures**, patched in `harness/serve/` so no
rebuild stands between the fault and the run:
- `gridPlan` slash-joins the level tuples into one line (the exact defect anr9
fixed) — fails all three, because `openGrouped` waits on the head's shape
before any test reads it;
- `th.colSpan = 1` on every column group header — fails the span test alone;
- a stray `tr.insertCell()` on the inline group row — fails the group-row test
alone, which is the DOM half of `RenderRow::Group` carrying no cells;
- `indicator(false)` hard-coded — fails on `▶ Upland` while `▼ Inland` still
passes, which is why the fixture has one collapsed group and one open one;
- `collapsed: Region/Upland` removed from the served document — fails on the
rows that reappear, so the assertions see the collapse and not just the glyph.
**A bug this fixture surfaced rather than predicted.** `Coastal` is the row
frame's FIRST entry and its header is never drawn: the spec asserts **two**
group rows where the document has three, with improvise-0o6x named on the line.
That count is a record of an open bug, not an endorsement — when 0o6x lands it
becomes 3 and this line is where the change must be argued.
## `harness/tests/landing.spec.js` — the front door
`docs/index.html` is copied to the site root by `web/assemble.sh`, so the
harness has been serving it all along with nothing asserted about it
(improvise-bgtf).
**Counting the four `.ap-player` elements proves nothing**, and that is the
whole reason this is a spec rather than a one-liner: `autoPlay` is false, so
asciinema-player fetches no cast until play is pressed. A page whose every cast
path 404s constructs four players, throws no page error, and looks identical to
a working one. Injected while the page was being written (`./casts/` ->
`./cats/`) the construction-only check passed on the broken page.
Re-injected into `harness/serve/index.html` now, the spec fails at the first
player: `.ap-line` elements exist only once there is a cast to lay out. The spec
also asserts, BEFORE pressing anything, that no `.cast` has been requested —
without which the network evidence below could be satisfied by a page that
fetched them eagerly, which is not the page we ship.
Two assertions where one looks sufficient: the rendering says a cast played, and
the recorded responses say **four distinct** casts answered 200. Three players
showing the same recording satisfies the first and fails the second.
`"the front door reaches the app it advertises"` follows the footer link rather
than asserting its text, because whether `./app/` resolves is a fact about the
assembled tree. Injecting `href="./aplication/"` fails it.
## `harness/tests/contract.spec.js` — the ninth injection, and a lost guard
**The lost guard.** improvise-78v6 asks for the test-name diff over PR #61's
range, and it finds a fourth loss beyond the help test: `"a click on an item row
filters, as Enter does"`. `contract.spec.js` asserts the CATEGORY arm of
`click_category_row` (the row expands) and nothing asserts the ITEM arm, whose
rule is "do what Enter does here" — filter. Only `#status` distinguishes them,
so a click wired to the wrong arm passed the whole file. Restored inside the
click test, and proved by injecting `click_category(row.is_category ? index :
0)`, which fails that line alone.
The diff that finds this is two commands, and is worth running on any PR whose
subject is a lost guard:
```sh
M=$(gh pr view 61 --json mergeCommit -q .mergeCommit.oid)
for r in "$M^1" "$M"; do
git ls-tree -r --name-only "$r" -- harness/tests web/tests |
while read -r f; do git show "$r:$f" | grep -oE 'test\("[^"]*"'; done
done
```
The other two losses are deliberate: `pointer.spec.js`'s sidebar test was
superseded by the open-at-launch ruling, and the widgets file was merged into
this one.
## `harness/tests/contract.spec.js` — the page bar, and the pointer's delegation
**"the page bar names the quarter, and both it and the totals follow `]`"**
(improvise-p7lh) is the end-to-end half the Rust tests deliberately do not
attempt. It runs against the shipped `web/demo.improv`, where `Quarter` really
is on `Axis::Page` with four items, and asserts the *arithmetic*: Q1's Revenue
total is `63,700` and Q2's is `66,400`, each a sum over all four products. That
is why the totals must be **projected** — the number is a fact about every row,
and a client that summed what it had been sent would be right only by accident
and wrong the moment a viewport scrolled.
Pressing `]` is what makes it a test of the pair rather than of two fields: a
bar that showed a stale quarter over fresh numbers is the precise lie the bead
names, and it passes any assertion that reads the two independently.
**The three delegation tests** (improvise-b8ot) each pin a property a naive
rewrite loses *silently* — the page still paints, and only the pointer stops
working, which no unit test and no render-model contract can see:
- **mousedown, not click.** Asserted by dispatching `mousedown` **alone**, with
no click behind it. A handler bound to `click` does nothing at all here. This
is improvise-0x9u's mechanism from the other side: from an editing state the
input's `blur` repaints first and the `<td>` is detached before `click` would
fire, so the event never arrives. `locator.click()` — which every other
pointer test uses — dispatches both and therefore cannot separate them.
- **preventDefault, and only over a cell.** Read off `window`, which is last in
the bubble path and so sees what `#grid`'s handler did. The second half is
what makes it discriminating: a header carried no listener before delegation,
so a blanket handler on the root that prevents *everything* is a behaviour
change, and asserting only the cell would pass for it.
- **The coordinates come off the node.** The test rewrites a `<td>`'s
`data-row` in the DOM and requires the click to land where the *attribute*
says. A per-node closure over `gridPlan`'s answer ignores the rewrite and
fails. This is the assertion that says the handler is a **lookup** rather than
a second derivation of which data row was clicked — the thing the bead is
actually about, and the thing a "does clicking still work" test cannot tell
apart from the old code.
**The fourth property — closing a live editor first — is not a new test.** It is
`"a click moves the selection from an editing state too"`, already in this file,
which clicks a second cell with no Escape between and asserts the first cell's
`<input>` is gone and the second's is open. Adding a fifth spec asserting the
same ordering would have been a second copy, not a second guard.
**And the repaint-skipping branch is asserted by survival**: after a mousedown
on an editable cell the `<input>` must still be there three hundred
milliseconds later, because a repaint would call `replaceChildren` and take it
away. That is the one of the four whose failure is *most* invisible — the editor
opens and vanishes within a frame, which reads as a click that did nothing.
### the_keys_that_compose_a_switch_view_are_these
improvise-lltt is deferred on a **premise**, not a judgement: its own note says it
is "unreachable today ONLY because no shipped client emits `Command::SwitchView`",
and asks for a guard asserting exactly that, so the silent failure becomes loud
the moment the premise stops holding.
The guard failed on its first run. Two live paths in this client's own resolver
(`resolve.rs`): `switch-view-at-cursor` and `create-and-switch-view`. The other
half was already live — the same resolver composes `ModelCommand::TransposeAxes`.
So the scenario lltt describes is reachable today from the shipped view panel, and
the ticket was escalated P2 → P1 rather than fixed here, the honest fix being the
protocol change it describes.
**It is an `assert_eq` against those two names, not an assert-empty, and the list
is not an exemption list.** It pins what is true so a *third* live path fails, and
its message says so — because a reader who found this passing would otherwise
conclude the premise holds. When lltt is fixed the list empties and this test goes
with it.
Why the enumeration and not a read: the bug was already written into a passing
test's comment elsewhere (`"Aimed at the host's own view… HideItem writes
view_mut()"`) and had been read as a fixture note for as long as it had been there.
#### "a click moves the selection from an editing state too"
improvise-0x9u, and the fault was injected by running this assertion against the
bundle as it stood: `cells.nth(4).locator("input")` came back **0**, with the
selection still on cell 0, exactly as the bug report measured in Chromium.
Two things about it are worth more than the assertion. First, **what it does not
do**: it never presses Escape between the two clicks. The sibling test does, and
that single line is why the property it names ("a click moves the selection") was
asserted for a whole wave while the neighbouring state was broken — the click
path from an *editing* page was never exercised.
Second, **the fix's own first attempt failed this same test**, which is the part
to remember. Moving the handler to `mousedown` is necessary and not sufficient:
`close` still repainted, and removing a focused `<input>` fires `blur`
**synchronously**, so `close` rebuilt the table from inside itself and detached
the `<td>` the handler was about to open an editor on. The assertion could not
tell that apart from the original bug — same symptom, different cause — so it is
the debug run rather than the test that separated them. `close` therefore no
longer repaints, and the `blur` arm checks `closed` before dismissing.
Third — and this is the edit the repair nearly shipped unguarded — **the fix is
three edits, not two**, and the third is the one with the most careful comment:
`close` restores `td.textContent` (the *drawn* value) instead of `seed` (the
*stored* one), because `close` no longer repaints and `repaint()` early-returns
for as long as `editing` is set. Reverting that one line to
`td.replaceChildren(original)` left the whole suite green, and the defect was on
the front page: `web/demo.improv` carries `format: ,.0` over `18400`, and this
test's own two clicks — no Escape between them — leave cell 0 reading `18400`
where the page had drawn `18,400`, for as long as the second editor stays open.
The assertion added for it is `toHaveText("18,400")` after the second click, and
the injection is exactly that revert: it fails with `unexpected value "18400"`,
naming this test and no other.
The pattern is worth carrying past this entry. A repair of N edits whose N−1
guarded ones pass is not a guarded repair, and the unguarded edit is reliably
the one the author understood best — which is why it carries the clearest
comment and why it felt covered. For each distinct edit, name the test that
fails when only that edit is reverted; a line with a comment and no answer to
that question is the line to inject.
#### "the widgets the demo document fills are drawn, in the state that fills them"
The `#doc-name` row is improvise-5jna, and the reason it belongs in a **browser**
test rather than in `render_surface.test.js` is that the latter is a substring
search over the page's source: `doc_name` mentioned by a function nothing calls
reads as painted. That is how `model.minibuffer` once shipped dead, and how
z754's three title-bar fields sat projected and unpainted for a wave while the
static check was green. Proved by adding the assertion first: `element(s) not
found` for `#doc-name`.
The `[+]` marker is deliberately **not** asserted here. The standalone deployment
persists on every accepted batch and `effect::Save` clears the flag in the same
round trip (gotcha 45), so the marker is transient in a browser and any timing
assertion over it would be flaky rather than discriminating. Its spelling — the
order `name (stored_as) [+]`, the `null` `stored_as` writing no empty parens — is
`web/tests/shell.test.js`, over `documentTitle` directly.
#### "a selection driven below the fold scrolls itself into view"
improvise-jq8l, and the expensive half was the **fixture**, not the assertion.
Neither shipped document can reach the state: `web/demo.improv` is four rows and
`harness/fixtures/grouped.improv` is eight, and a probe measured both at
`scrollHeight === clientHeight` in a 484px box — nothing to scroll, so the
assertion would have passed against a page that never scrolls. That is the
vacuity this entry exists to record, and it is why the spec asserts
`scrollHeight - clientHeight > 0` **before** it presses a key.
`harness/fixtures/tall.improv` is thirty rows in three groups, which overflows for
a reason worth stating: `ROW_PX` is 26 and a real row measures 28.6px, and inline
group rows are not charged for at all, so the page renders more rows than its own
box can show. Measured against the unfixed page: 105px of overflow, the selected
cell 96px below the visible bottom, `scrollTop` still 0 after twenty-five
`ArrowDown`s.
Both directions are asserted, and neither alone is enough: the cell being inside
`#grid`'s rect would also hold if the selection had never moved, and `scrollTop >
0` would also hold if the page scrolled to the wrong place.
#### "the casts are legible on a phone, and the page still does not scroll sideways"
improvise-o31u, and **the instrument is the finding**. `getBoundingClientRect`
reports a height for a `content-visibility`-hidden child, and heading text
overflows its box without widening it, so neither "the element is present" nor a
box measurement can see this defect at all. What a reader sees is the **computed
font size**, and that is what is asserted.
Measured against the unfixed page, and it reproduces the ticket's figure to five
digits: `pivot.cast` renders its terminal at **4.83258px** on a 390px viewport.
The arithmetic is not subtle — the player is told `cols: 120`, the container is
~356px inside the page's 16px gutters, and the player DERIVES its font size from
the width it is given. After the fix: **11.9457px**.
Two assertions guard the trade this fix must not make. The page's own
`scrollWidth - clientWidth` must stay **0** — a document that scrolls sideways
on a phone is a worse bug than one unreadable section — and `#cast-pivot` must
be the thing that scrolls, which is what separates "contained the overflow" from
"removed the overflow by shrinking something else".
#### "a desktop viewport is unchanged by the phone fix"
The negative arm, and **it is forward-looking: no fault available today makes it
fail.** The entry previously named two mechanisms and both were measured, at a
1280px viewport, against the shipped page:
- `@media (max-width: 900px)` → `@media all`: this test **passes** (13.1131px,
`scrollWidth === clientWidth`).
- `min-width: 880px` moved from `.player > *` onto `.player`: this test
**passes**; the *phone* arm fails instead, on `scrolls.page === 0`.
The arithmetic is why, and it is worth stating rather than re-deriving: at
1280px `#cast-pivot` measures `clientWidth === scrollWidth === 966`, so an 880px
floor is inert at desktop wherever it is placed. The floor would have to exceed
the ~966px the player actually gets — or the content column would have to
narrow — before either assertion here could move. That is a real future fault
and the reason to keep the test: it pins 13.11px and "nothing to scroll" against
a floor raised past the player's own width. It is not a reason to claim the arm
currently discriminates between the two placements, because it does not.
Note also that the **phone** arm is stronger than this entry used to credit it
with: the misplaced-`min-width` fault, attributed here to the desktop arm, is
caught there. `scrolls.page === 0` is the assertion that separates "contained
the overflow" from "moved it onto the document".
#### "no cast carries the recording machine's shell, clock or geometry"
improvise-2oxt, and **both clauses were injected separately**, which matters
because the first would otherwise mask the second. Serving the pre-2oxt
`pivot.cast` fails on `env` — `Received: {"SHELL": "/bin/zsh", "TERM":
"screen-256color"}` — and stops there. Stripping `env` and `timestamp` from that
same header and serving the body unchanged then fails on `pivot.cast sets a
window title`, which is the clause that actually watches for the leak: an OSC
window-title sequence, emitted by a shell once per prompt and carrying the cwd,
and emitted by `improvise` never.
Asserted over the cast **file** rather than over the rendering, and that is the
finding. A player draws a shell prompt as ordinary terminal text, so a
screenshot or a `toContainText` assertion would have to know what the hostname
and home directory had been — a check that can only be written by someone who
has already seen the leak. The file-level signature can be written by anyone.
What no assertion here reaches, and how it was checked instead: whether each
cast SHOWS what its section claims. That is not a property of the bytes. It was
verified by replaying each recording through a small ANSI reconstruction and
reading the frames as text, which found two scenarios that recorded cleanly and
demonstrated nothing — a drill into a cell with no records behind it, and a `c`
pressed on a category already on the column axis. Both are true frames about the
wrong thing, which is the failure mode this repo keeps meeting, and neither was
visible from the keystroke list.
---
## `improvise-ws-server` — the `Origin` check (improvise-kc7n)
The bug: `grep -rni origin crates/improvise-ws-server/src/` returned nothing.
The server inspected no `Origin` header and accepted every handshake, and a
websocket handshake is exempt from the same-origin policy — no CORS preflight —
so any page a user visited could open a socket from that user's own allowed
address and read and edit every document served. An address check cannot see
it; the argument is `context/code/clients.md` §Origin.
Five edits, five guards. Each was reverted alone and the named test failed;
nothing here is covered only by a comment. Review added a sixth — see
`two_origins_state_none_this_policy_can_answer_for`.
### a_handshake_from_a_foreign_origin_is_refused
The reproduction, written first and watched fail on `2afbf37`: a handshake
carrying `Origin: https://evil.example` connected, and the test's
`connect_async(...).err()` was `None`. After the fix the upgrade is refused with
a 403 and the assertion is on the status, not merely on "an error happened" —
a refused upgrade and a dropped connection are different outcomes and only one
of them is the fix.
**Injection:** replacing the handshake callback's `admitted(&origins, req)` with
`Ok(())` — the transport collecting a policy it never asks — fails this and
`a_named_origin_reaches_the_document`, and nothing else in the workspace.
### a_handshake_with_no_origin_is_accepted
The case most likely to be wrong later, so it is named rather than left to a
match arm. Non-browser clients send no `Origin`: curl, a native client, this
file's own other tests. **Injection:** flipping `OriginPolicy::permits`'s `None`
arm to `false` fails this, `an_absent_origin_is_allowed`,
`the_handshake_refuses_what_the_policy_refuses`, and *both pre-existing socket
tests* — which is the point: the repo's own instrument is the non-browser client
this arm is about.
### a_handshake_from_a_loopback_page_is_accepted
The shipped local recipe — the page served by `python3 -m http.server 8080`,
the socket on another loopback port — must keep working with no flag, or the
default is a different bug from the one being fixed. **Injection:** making
`is_loopback` return `false` fails this and
`a_loopback_page_is_allowed_by_default` and leaves the refusal tests green,
which separates "allows loopback" from "allows everything".
### a_named_origin_reaches_the_document
`--allow-origin` end to end over a real socket, plus the negative arm in the
same test: widening for one page does not widen for its neighbours. Without
that second assertion the test passes against a policy that permits everything,
which is the fault E1a injects.
### allow_origin_widens_the_policy_and_nothing_else_does
The wiring, which no socket test can reach: the other tests build an
`OriginPolicy` directly, so a flag collected by clap and dropped on the floor is
invisible to every one of them. This drives `Args::parse_from` and asserts both
directions — bare refuses the deployed https origin and still allows loopback;
widened allows the named origin and not its neighbours. **Injection:** making
`Args::origin_policy` return `OriginPolicy::default()` fails this test alone.
### two_origins_state_none_this_policy_can_answer_for
Found by review rather than by the ticket. `HeaderMap::get` is **first-wins**,
and the deployment this README recommends puts a reverse proxy in front of the
socket — a proxy that *appends* `Origin` rather than setting it leaves two, and
reading the first lets the second be anything at all. No browser sends two, so
two is not a state any policy can answer for and the handshake is refused.
**Injection:** restoring the first-wins read (`(Some(value), Some(_))` answering
from `value`) fails this test alone, across the whole workspace. The test also
asserts the allowed origin *by itself* is still allowed, so the refusal is
demonstrably the second header rather than the first.
### a_refusal_is_a_forbidden_response
The refusal has to be a refused *upgrade*, and the socket test asserts the
`Error::Http` **kind** and the status rather than matching "403" in a
`Display` string: a dropped connection and a refused handshake are different
outcomes and only one of them is this fix. **Injection:** changing `forbidden()`
to `UNAUTHORIZED` fails this and `a_handshake_from_a_foreign_origin_is_refused`,
which is what shows the socket test reads the status rather than merely
observing that connecting failed.
What the signature covers instead of a test: `transport::serve` **takes** the
policy, so a host cannot forget to pass one — `doctrine(an_invariant_is_made_
unrepresentable_not_validated)`. What remains uncovered and is stated rather
than guarded: a proxy that strips `Origin` turns the check off, which is why the
proxy's own Origin allow-list stays in `README.md` as defence in depth.
### the_fixtures_basis_tracks_a_host_whose_layout_has_moved
improvise-1l9o. `furnished_client` and `client_ready_to_edit` built their
`BasisCache` by walking the client's own grid for `Record` cells and calling
`basis::published(0, 0, records)` with every version hard-coded to zero — a
small re-implementation of `project.rs::basis_update`'s record selection, in a
file whose entire purpose is to rule out re-implementations. Both now take the
`Projection::Basis` out of a real `Sessions::connect` snapshot.
The reason the ticket calls it a nit is the reason it needed a test: a fresh
`App` really is at generation 0, layout 0 and every record at version 0, so the
fabrication and the publication are **indistinguishable** on every existing
fixture. All 23 lockstep tests pass under either. So the change alone is
structural and untested by construction, and this test is what makes it
observable: it moves a tier first — a `HideItem` through `apply_on`, which
`observe_reshape` sees as a shape change — and then asserts the fixture's claim
names the host's current tiers.
Note `SetNumberFormat` does **not** work here and was the first attempt.
It declares `EffectKind::Model(ModelChange::LayoutGeometry)`, but
`layout_version` is **observed** from `view_shapes` rather than read off the
declaration (improvise-pdvm), and a number format is not part of a view's shape
hash. A command being classified `LayoutGeometry` is therefore not a promise
that it moves `layout_version` — which is the observed-not-declared design
working as intended, and easy to misread as a bug.
Fault injected 2026-09-21: re-fabricating the basis inside `published_basis`
(`published(0, 0, real_records)` — keeping the real record set, so only the two
document tiers are forged) fails this test with `left: (0, 0)` against
`right: (0, 1)`, and fails nothing else in the workspace.
**One residual, found in self-review and not closed.** `published_basis` asks
`Sessions::connect` for a window of `window.0 x window.1`, and a fresh session
sits at offsets zero; `client_of` builds its grid at the **cursor's** offsets.
Every fixture here has its cursor at the origin, so the two windows coincide and
the published record set is the one the client's grid names. A future fixture
that scrolls before building its client would get a basis naming records its grid
does not show — which `claim` would then silently omit, and
`Rejection::RecordUnstated` is what the server answers with. The fixtures no
longer fabricate, which was the ticket; they do still assume a cursor at the
origin.
### leaving_a_panel_in_records_mode_lands_where_the_host_lands
improvise-e5pf. `AppMode::Normal` is settled by the host against the layout the
batch ends on; the wire carries `Command::ChangeMode(AppMode::Normal)` and
`reduce_view` folds it verbatim. So a client that opened a panel from records
mode and pressed Esc landed in `Normal` while its host landed in
`RecordsNormal` — and `RecordsNormal` has its own keymap, so the divergence
compounds on the next keypress.
The reachable path, which the ticket did not name: `Tab` in `RecordsNormal`
resolves locally to `ChangeMode(CategoryPanel)`, and `Esc` there resolves to
`ChangeMode(Normal)`. `c`/`f`/`v` are `Ignored` in `RecordsNormal` and `R`
itself is a `stranding` decline, so the panel keys a first draft of this test
used never separated anything — the test panicked on "the client must answer
this key itself" rather than on the drift.
**Why lockstep could not see it before.** Its records coverage (`typing_at`,
`records_pair`) set `host.view_state.mode` by assignment and seeded the client
with `client.view = app.view_state.clone()`, so it started where the mode and
the layout already agree, and every divergence here requires them to disagree —
`doctrine(a_guard_seeded_on_agreement_cannot_detect_drift)`. `in_lockstep`
drives the key on both ends and then refreshes the client's grid from a **real
projection**, never from the host's slice, which is the only seeding that can
show the two apart.
**Fault injected**: `resolve::changed_mode` reduced to
`Command::ChangeMode(mode)`. The test fails on `close: the client and the host
disagree about the mode`, `Normal` against `RecordsNormal`.
Two existing `resolve.rs` fixtures had the same shape and had to be corrected
with the fix: `furnished` and `a_records_mode_commit_is_the_hosts` put the
client in a records *mode* over a non-records *grid*, which is a state no host
can produce. Both now project `GridUpdate::Records` as well. That they had to
change is the evidence that the substitution was load-bearing rather than
incidental.
### a_records_commit_is_declined_off_the_grid_and_not_off_the_mode
improvise-e5pf's damage half. `resolve::commit` is written to **decline** a
records commit it cannot compose (it needs `record_col`, gotcha 1). It asked
`state.view.mode.is_records()`; with the mode drifted to plain `Editing` over a
records grid it did not decline, and composed `typed_cell_command` plus a
`Basis` claim against a grid whose columns are `[coords…, +cat, measures…, +]`.
A wrong write, strictly worse than the black hole it exists to produce.
The test states the drift rather than reaching it — reaching it takes the key
sequence the test above drives — and asserts no `Command::Model` is composed.
**The control is the point.** A first version of this test passed against the
*broken* code, because `client_of` publishes no basis and `BasisCache::claim`
returns `None` until one arrives, so `commit` returned `None` for a reason that
had nothing to do with records mode:
`doctrine(an_injection_that_does_not_fail_names_an_untested_path)`, caught by
injecting the old spelling and watching the test still pass. `records_client`
now publishes a basis naming every record the grid projects, and the test
carries a positive control — the same `Editing` mode over a **regular** grid
must still compose a write — so "this client never writes" cannot pass it.
**Fault injected**: the decline restored to `state.view.mode.is_records()`. The
test fails on `a records commit this client cannot compose was composed anyway`.
### the_normal_pair_is_settled_in_both_directions
`App::settle_normal_mode` settles **both** `Normal` and `RecordsNormal` against
the layout, so a `RecordsNormal` over an ordinary grid becomes `Normal`. The
first version of the client's settling mapped only `Normal → RecordsNormal`,
which left the mirror-image drift live. Found in review of this branch.
The test now aims at `improvise_protocol::settled_mode`, which is the **one**
rule all three ends call — `ClientState::fold`, `ClientSession::fold` and
`App::settle_normal_mode`.
**Fault injected**: the `(Normal | RecordsNormal, false)` arm narrowed to
`(Normal, false)`. The case `records=false, holding RecordsNormal` fails. The
fourth row and the pass-through cases are the negative arms — without them a
rule that answered `RecordsNormal` always, or `Normal` always, would pass, and
the pass-through loop is what pins that `Editing`/`RecordsEditing` are **not**
settled (the reason a records decision asks the grid and never the mode).
### switching_between_a_records_view_and_a_pivot_view_settles_both_ways
**The P1 ChatGPT Codex found on PR #98, and the more useful half is why the
first fix missed it.** That fix settled the mode at each *composition site*
(`resolve::changed_mode`), against the grid the client held when it resolved the
key. `switch_view_at` composes `[SwitchView(target), ChangeMode(Normal)]`, so it
settled against the **outgoing** view: records → pivot emitted `RecordsNormal`
and pivot → records emitted `Normal`, recreating the very drift the PR closed.
The hand-back had *predicted* this as follow-up 3 and concluded it was
unreachable **because `R` is a `stranding` decline in the browser**. That
reasoning is right about the toggle route and wrong about this one: records-ness
is auto-detected from the axes (gotcha 1), so a **view switch** changes it with
nothing in the batch saying "records". One door was enumerated and there were
two — `doctrine(an_unreachability_claim_is_only_as_wide_as_the_routes_enumerated)`.
Codex had flagged the same path on PR #91, where the host side was fixed and the
client side was not, so it was tracking one defect across three PRs.
The remedy is general rather than a patch to `switch_view_at`: `changed_mode` is
deleted, every site emits the plain `AppMode::Normal`, and `ClientState::fold`
settles after **every** fold against the grid it holds. Route-independent, so no
enumeration of composition sites is load-bearing any more.
**The fixture drives the real path**: a `Host` in records mode, a session
connected (which opens the document's *default* view, gotcha 29 — the first
draft asserted the opposite and failed its own precondition), the composed pair
folded locally and sent as a `ClientFrame::Commands`, and every reply frame
delivered in arrival order. `deliver` takes the whole reply rather than a
hand-picked projection.
**The composed `ChangeMode` carries the STALE answer deliberately** —
`settled_mode(&Normal, client.grid.records)` off the outgoing grid — because
that is what `switch_view_at` really emitted, and composing the friendly value
instead would make the records → pivot direction pass under the fault.
**Fault injected**: `ClientState::settle_mode` reduced to a no-op. Both
directions fail, each proved separately because the loop stops at the first:
- pivot → records: `_Records: the client settled against the view it LEFT`,
`Normal` against `RecordsNormal`.
- records → pivot, reached by relaxing the first iteration's expectation under
the same injection: `Default: the client settled against the view it LEFT`,
`RecordsNormal` against `Normal`.
Records-ness and the mode are asserted as **two separate facts** rather than one
derived from the other — a first version computed the records precondition from
the mode expectation, so relaxing the mode silently relaxed the precondition too
and the test failed for the wrong reason.
**It has a host-side twin, and the two are a pair rather than a duplicate.**
improvise-4dpj landed `switching_views_across_records_ness_settles_against_the_view_switched_to`
in `ui/app.rs` while this was being written: the same defect, the same batch
(`[SwitchView(to), ChangeMode(Normal)]`), the same crossing in both directions,
and independently the same phrase in its message — "settled against the view it
LEFT". One is the host settling after the batch against the layout it ends on;
this one is a client settling after the fold against the grid it holds. Neither
is redundant: nothing in the projection stream carries the host's `ViewState`
(gotcha 47), so a host-side guard cannot see a client's mode and the reverse.
Delete either and that side goes unwatched.
### the_mode_a_switch_lands_in_is_the_one_a_host_on_that_view_holds
The lockstep half of the entry above: it asserts the host itself answers
**differently** per view (`Normal` on the pivot, `RecordsNormal` on the records
view). Without it, "the client settles to X" is a claim about the client alone —
a host that answered `Normal` everywhere would make the switch test's
expectations arbitrary rather than lockstep.
## The browser boot diagnosis channel (improvise-1wft and the five it blocked)
Five beads filed by three reviewers were one failure: **nothing carried a boot
failure's REASON from the worker to the page.** `web/worker.js` caught its own
`start()` rejection and logged it, a worker that rejects in `start()` fires no
`error` on the main thread (gotcha 46), and `onFrame`'s `if (!server) return`
dropped every frame after that — so the only survivor was a generic 8s
watchdog, which improvise-vep8 showed did not even arm on one path.
The channel is one rule: **a protocol frame crosses the port as a STRING and a
shell notice crosses as an OBJECT.** Nothing else crosses, so the discriminator
is total and costs no protocol change — `ServerFrame` JSON is a string by
construction of `WorkerServer::on_message`. `shell.js::classifyWorkerMessage`
is that rule and `web/tests/shell.test.js` pins it, including the case that
would break it: a notice *serialized* would classify as a frame and be fed to
the fold.
**A boot failure is a STATE, not a rejected promise.** That is what improvise-i0v9
needs: the page must open and paint read-only when another context holds the
document's exclusive handle. So `bindings.rs::boot` matches on `attach`'s
failure rather than propagating it — a locked document falls through to
`read_only`, which reads the bytes the non-exclusive way and opens over a
backend that refuses every write. `BootFailed` is the only fatal kind;
`ReadOnly`, `SeedMissing` and `SaveFailed` are all open, painting sessions,
which is what `shell.js::isFatal` says and what its test asserts in both
directions.
### The `getFile()` probe (`harness/`, not a cargo test)
**The measurement the read-only path rests on, taken before it was promised.**
`Storage::read` goes through the exclusive sync access handle, so the read-only
session needed a path that does not: `FileSystemFileHandle.getFile()` yields a
`File` snapshot, which is a `Blob`, which reads without a handle. Whether that
works *while another context holds the handle* was unprobed, and the whole
design turns on it, so it was measured rather than assumed.
Measured 2026-09-21, `harness/tests/opfs.spec.js`, both engines that have OPFS:
| engine | `getFile()` + `arrayBuffer()` under a foreign sync handle | the refusal that proves the lock |
|---|---|---|
| Chromium | **32 bytes read, correct** | `NoModificationAllowedError: Failed to execute 'createSyncAccessHandle' on 'FileSystemFileHandle': Access Handles cannot be created if there is another open Access Handle or Writable stream associated with the same file.` |
| Firefox | **32 bytes read, correct** | `NoModificationAllowedError: No modification allowed` |
| WebKit | not measured — Playwright's WebKit provides no OPFS at all | — |
**The precondition is the half that makes it evidence.** Without it the probe
passes vacuously in any engine that does not actually enforce exclusivity: the
read would succeed because there is no lock, not because `getFile()` bypasses
one. So the probe first tries to take a *second* `createSyncAccessHandle` and
`ensure!`s it is **refused**, and reports the refusal text alongside the byte
count. Both halves are in the attached `opfs-probes.json`.
This is filed as a probe rather than a contract check on purpose: it records
what the engines do, so a decision taken on it can be re-checked rather than
re-argued. It runs per engine, so a regression in either is a failure.
### the_exclusive_handle_refusal_is_recognised_in_both_engines
`boot::is_locked` decides whether an `attach` failure means "somebody else has
it" (open read-only) or "something else is wrong" (fail). It matches on
`NoModificationAllowedError`, which is a **string match on a DOMException
name**, and the two engines word the rest of the message completely
differently — so the fixtures are the strings the harness probe above actually
produced, not invented ones. That is the only reason the match is trustworthy.
The discriminating half is the negative: a `SecurityError` from `getDirectory()`
must **not** classify as a lock, or every boot failure would offer a read-only
session over a document it never read. An empty rendering is not a lock either.
### a_read_only_backend_reads_the_document_and_refuses_every_write
`vfs::ReadOnly` is what makes improvise-i0v9's "open and paint" true rather than
aspirational. The test asserts both directions, which matters because a wrapper
that forwarded everything would pass a reads-only assertion: the document opens
and reads back, `write` and `delete` are both refused, the refusal **carries the
reason** rather than a bare error, and the refused writes left the readable
bytes intact.
`delete` is refused as well as `write`, which is easy to miss — it is a write.
### a_document_created_without_its_promised_seed_says_it_was_created
improvise-iobc: a bad or missing `?seed=` produced an empty document and a
normal-looking shell, with the 404 only in the console. The page cannot tell an
*intended* empty document from one that is empty because its starting copy
never arrived — and neither could anything else, because `seedBytes()` returned
`undefined` for both.
The fix is not in the page: `web/worker.js` now returns `{bytes}` or `{error}`
and hands **both** to Rust, and `vfs::open` reports an `Origin` — `Stored`,
`Seeded` or `Created`. Only `Created` **with** a seed error is a notice, so a
reload of an existing document says nothing, which is the common case. That
conjunction is the point: either fact alone over-reports.
The test's discriminating half is that the *same call* reports `Seeded` when a
seed is supplied, so `Created` is not a constant.
### a_name_that_is_not_one_flat_entry_is_refused_rather_than_truncated
improvise-9cma reported `?doc=` values like `` and `///` giving a blank UI and
generic watchdog text. Probing it turned up something worse than the reported
symptom: `OpfsFs::entry_name` takes `Path::file_name`, so **`?doc=a/b/c.improv`
was not rejected at all — it silently opened `c.improv`**, a different document
from the one the URL named. An accepted-and-wrong is worse than a refusal,
because the diagnosis looks identical and the repair is not.
`boot::document_name` answers before anything touches storage, so the reason is
specific and arrives early rather than as a late failure deep in the handle
acquisition.
### a_persist_that_fails_raises_a_notice_naming_the_document
improvise-zre1: the worker auto-persists on its own behalf, and a failed write
left its message in `view_state.status_msg` — which the worker has no UI for —
while `dirty` stayed true. The client saw `dirty: true` and could not tell
"save failed" from "save pending", which is a data-loss risk on page close if
the user trusts autosave.
**The trigger is an observation, not a prediction**, in the same shape gotcha 45
gives for the persist trigger itself: a save that worked clears the flag, so
*the flag surviving `persist()` is* the failure. No second table of which
effects can fail.
The fault is injected with `vfs::ReadOnly` — a real backend that reads and
refuses writes — rather than a mock, and the test asserts the precondition
(`dirty` really did survive) before believing the notice. The notice is
**drained**, not read, so one occurrence is reported once; asserting the second
`take_notice()` is `None` is what pins that.
Its companion `a_persist_that_succeeds_raises_no_notice` is not padding: without
it the first test passes against a worker that reports a failure on every single
persist.
### a connection that hangs forever is still caught by the watchdog
improvise-vep8: `shell.arm` ran **after** `await transport.ready`, so a socket
that never fires `open` never armed the watchdog at all — the page sat on
"connecting…" forever with no timeout. Arming now precedes the await.
Two consequences worth recording, because both were invisible until the move.
**The message had to change.** It said `accepted the connection but sent no
document`, which arming before `ready` makes a claim the page no longer knows
to be true. It now says `never answered`, which covers a peer that never opened
*and* one that opened and went quiet — the one condition the watchdog is really
for, and the reason gotcha 46 gives for it not being per-transport. The existing
spec asserted the old wording and failed, which is the right way round.
**Two comments in `shell.spec.js` stated the opposite of the new code** — both
said `shell.arm` runs only after `transport.ready` and that the watchdog was
"not on this path". Corrected in the same change rather than left, per gotcha
48's third clause: when a fact is stated in more than one place, changing it is
a grep and not an edit.
The disarm half of gotcha 48 is **not** re-broken, and this is the thing to
check first on any future change here: every notice goes through `shell.fail`,
which disarms, so a precise reason is never replaced 8s later. `a socket that
opened and then dropped reads as disconnected, and stays that way` still passes
and still runs past the window.
### an invalid ?doc= is named on screen, and not blamed on the browser
The boundary test for the channel: a `?doc=` Rust refuses produces a
`BootFailed` notice carrying the refusal text, the page renders it, and it
**stands past the watchdog window**. The negative assertion uses
`outlastTheWatchdog`, which asks `timesArmed() > 0` first — mandatory here,
since denying the watchdog's message on a path where it never armed denies
nothing.
### a seed that 404s says so rather than showing a silent empty grid
The other half, and it asserts the thing that separates it from a boot failure:
the page **still draws a grid**. An unfetchable seed is a degraded session, not
a dead one, so `#grid table` must be visible *and* `#status` must say `EMPTY`
and name the seed that failed.
### The silent-drop shape, grepped rather than taken one site at a time
`ociy`, `y01k` and `r4nf` are all "an error swallowed into a value that reads as
'nothing to do'", so the workspace was grepped for the shape rather than only
the sites the tickets named.
- **`serde_json::(to_string|from_str)…\.ok()`** across `crates/` and `src/`:
three instances, all in `improvise-wasm-client/src/lib.rs`, all named by
improvise-r4nf. **Zero remain** after the fix, which is the check that the
grep and the ticket agreed. They now return `Result<Option<String>, JsError>`
— the `Option` still means "nothing to send", and serialization failure
throws, so JS's `if (frame)` is unchanged at every call site.
- **Discarded results in the two hosts**: the remaining `let _ = …` in
`transport.rs` are channel sends whose receiver being gone *is* the normal
disconnect path, and are correctly ignored. The three improvise-y01k named
are now loud.
- **`postMessage` with no guard** in `web/app.js`'s worker transport: not a
second instance of improvise-ociy. `postMessage` on a live dedicated worker
does not throw and the page never calls `terminate()`; the analogous silent
drop is worker-side (`if (!server) return`), and that is improvise-86nw,
answered by the `BootFailed` notice rather than by a send guard.
One thing the send guard had to be split over: a `ResizeObserver` firing after a
drop would have overwritten the precise `disconnected` with `not sent`, breaking
the property gotcha 48 says keeps getting lost. **A viewport announcement is not
a user edit**, so it sends `{ loud: false }` and only user intent reports a
failed send.
### The render circuit breaker (improvise-7gf8)
`repaint()` returned early when `render_model()` threw, leaving `painted`
unchanged — so the `model.revision === painted` guard could never short-circuit
and every subsequent event retried the same broken render forever. It is now
tripped once: a render model that cannot be built is a permanent state, so the
breaker is a flag rather than a counter, and the message says to reload, which
is the only recovery the page has. It routes through `shell.fail`, so it also
disarms the watchdog rather than being replaced by it.
### `OpfsFs::open`'s scope guard (improvise-7gip)
`dyn_into::<WorkerGlobalScope>()` is satisfied by a `SharedWorkerGlobalScope`,
so a shared worker sailed past a check whose message named the right
requirement and failed later as a raw `TypeError` in the glue.
`DedicatedWorkerGlobalScope` is the type that actually means what the message
says.
**The fix needed a `web-sys` feature, which is why it had not been made
cheaply**: `DedicatedWorkerGlobalScope` is feature-gated and absent by default,
so the one-line change does not compile until
`crates/improvise-io/Cargo.toml` names it. That is the whole cost, and it is
worth stating because "just use the narrower type" reads as free.
`a_dedicated_worker_satisfies_the_narrow_scope_guard` is the guard that the fix
did not break the shipped path: in a real dedicated worker the narrow cast
succeeds. It is deliberately **not** a proof that a SharedWorker is rejected —
that needs a SharedWorker page, which this harness does not have, and
improvise-12ha owns the multi-tab question. Recorded as a gap rather than
implied.
### a second tab on the same document opens read-only and says why
improvise-i0v9's headline, end to end, in one browser context: two tabs on one
document, which is how it was reported. Measured before this change, the second
tab read `in-page worker · two-tab.improv` at 13ms — which looks like success —
then sat blank for eight seconds and was told the browser probably would not
allow it, while the code was holding the exact `NoModificationAllowedError`
naming the exact document.
Three things are asserted, and the first is the one the whole design is for:
**the second tab PAINTS.** `#grid table` is visible, because boot now succeeds
without a handle. It is not "dead but apologetic", it is a read-only session
over bytes read through `getFile()`.
The precondition is that tab one really is up and really is holding the handle,
so it waits for tab one's own grid first. Without that the second tab might be
passing because there was never a lock.
The third is the gotcha 48 property, and it needs `timesArmed() > 0` first or it
denies an unreachable event: the precise reason must still be on screen after
the 8s window, not replaced by the watchdog. `page.clock` walks past it.
**Strengthened by improvise-i0v9's second pass.** Every assertion above read the
*final* `#status`, so two failures could still pass it. A watchdog that fired
first and was then replaced by a late notice passed. So did a success claim
shown before the notice. The test now records every status with `logStatus` and
asserts, **before** the page clock is walked, that the log ends in `read-only`
and never contained `did not answer` or the exact `in-page worker · <doc>`. A
reason that arrived after the watchdog would sit in that log behind "did not
answer"; that is the "well before 8s" property, stated without a wall-clock
bound. Measured failing against `gh/main` at `da2e5ab`, whose log was
`["in-page worker · two-tab-….improv", "read-only — …"]`: the premature claim
the ticket measured at 13ms is still there once 1wft's notice exists. The
brief's rival, `onNotice` calling `setShell` instead of `fail` (the watchdog not
disarmed), fails it at the first `toContainText("read-only")`. The Snapshot's
claim replaced the reason at once, which is why the eight-second form of this
rival is now exercised by the invalid-`?doc=` spec instead (see
`context/code/clients.md`, "The watchdog").
It runs on Chromium and Firefox — the two engines with OPFS — rather than
Chromium alone, because the exclusivity semantics are the engine's and the two
word the refusal completely differently.
**What this does not cover, stated rather than implied.** The ticket's note
records a second trigger found independently: the same failure on a *reload*
after a prior session left a handle open, which is a race between context
teardown and handle release and cannot be driven deterministically from a spec.
And nothing here upgrades a read-only session to writable when the other tab
closes — the page has no retry, so "read-only until you reload" is still the
behaviour, now with an accurate message instead of a wrong one.
### an edit sent after the socket dropped says it was not sent
improvise-ociy, and **this test exists because the review of its own branch
found the fix had none** — `web/app.js`'s send guard was written, argued in a
comment, and never run. AGENTS.md is explicit that a fix needs a test that
failed before it.
Proven against the pre-fix silent drop (`if (readyState !== OPEN) return;`
alone): the status stays `disconnected` and the edit vanishes.
**The first version of this test was vacuous and passed for the wrong reason.**
It pressed `ArrowDown`, which on a client that never received a snapshot
composes *no frame at all* — `a_cursor_key_before_the_handshake_produces_no_message`
says so in `improvise-wasm-client` — so `transport.send` was never reached and
the guard under test never ran. It failed, which looked like the injection
working, and it would have kept failing for a reason that had nothing to do
with the guard. `Enter` is forwarded **upstream** and therefore composes a
frame regardless of client state, which is what makes the send actually happen.
That is `doctrine(an_injected_fault_must_be_the_fault_you_named)` seen from the
other side: the *test* must reach the path, not merely fail.
The precondition — the socket really opened and really dropped — is asserted
before the keypress, so this is a send on a CLOSED socket rather than on one
that never connected, which is a different arm with a different message.
And the last assertion is the one that keeps improvise-taxx intact: the drop
must still have been **named first**. The loud send is allowed to add "your
edit did not go", never to replace "disconnected" with something vaguer. This
is why the guard takes `{ loud }` at all — a `ResizeObserver` firing after a
drop sends a viewport announcement, and if that shouted it would overwrite the
precise close message eight seconds of PR #21 went into protecting. **A
viewport announcement is not a user edit.**
---
## `improvise-protocol/src/resolve.rs` — `N`, the panel cursor, and a silent axis
Three defects found by driving the shipped page under Chromium, all in the
category panel or the keys that reach it. Headings are module-qualified: three
of these leaf names are answered by a test in `command/cmd/panel.rs` as well,
and an unqualified heading would be satisfied by whichever the checker found
first (gotcha 48, improvise-rm4s).
### resolve::tests::n_in_normal_opens_the_category_prompt_rather_than_falling_through
improvise-koh6, and the reason it is a P1 rather than a papercut. `N` is
advertised on the browser's **own** help overlay ("Quick-add a new category,
from anywhere") and resolved `Upstream`, which `Host::handle` answers with an
explicit empty vec — so the full DOM was byte-identical before and after.
The user then typed the category name into NORMAL, where `i` is bound to
`edit-or-drill` and opens the grid editor **pre-filled with the cell's current
value**: measured on `web/demo.improv`, `N R e g i o n Enter` turned the cell
`18,400` into the text `18400on`, with no category created.
So the fault to inject is not "does `N` compose the right commands" but
**"does `N` compose anything at all"**: replace the `search-or-category-add`
arm of `composed` with `None` and this test fails at `Upstream(Char('N'))`,
which is exactly the shipped behaviour it was written against. The assertion
names the two commands in order because the arm mirrors `SearchOrCategoryAdd`
line for line, including that it uses a plain mode change rather than
`EnterMode` — so it does **not** clear the minibuffer on the way in, and a
"tidier" arm that added a `SetBuffer` would fail here rather than silently
diverge from the host.
### resolve::tests::search_or_category_add_is_answered_here_whatever_the_search_query_holds
The companion, and the one that pins the *decision* rather than the behaviour.
`SearchOrCategoryAdd::execute` is a conditional: a non-empty `search_query`
delegates to `SearchNavigate`, which a browser cannot compose — it would have
to search a grid it only holds a viewport of. Composing one arm and returning
`None` for the other would make **whether the host is needed** depend on
client state, which `enter_in_an_empty_view_panel_resolves_locally_rather_than_upstream`
exists to forbid.
This test loops both states and demands `Local` from each: the empty query
gets the two commands, the non-empty one gets a decline with its own true
subject through `not_yet`. Inject the fault by returning `Step::Upstream` for
the search arm — the `"North"` iteration fails and the `""` one passes, which
is precisely the asymmetry the rule is about. Note what it deliberately does
*not* assert: that the two arms agree on anything. The **outcome** varying
with state is what `cycle-axis-at-cursor` two arms above already does.
### resolve::tests::deleting_a_categorys_last_item_leaves_the_cursor_inside_that_category
improvise-uej8. `d` on the last item of an expanded category shrank the tree
under a cursor that did not move, so the row beneath it became the **next
category's header** — and one more `d` removes a whole category, which is
`LayoutGeometry`: `Workbook::on_category_removed` rewrites every view's axis
map and `Model::remove_category` drops every formula targeting it (gotcha 35).
High blast radius, one keypress away, no confirmation and no undo.
The test asserts the pre-state it depends on — that row 3 really is a
`Category` entry — before asserting the retreat, so it cannot pass by sitting
somewhere the escalation was never possible. Inject the fault by deleting the
`commands.extend(retreat_from_deleted_item(...))` line.
### resolve::tests::deleting_a_categorys_only_item_does_not_arm_the_next_d_with_remove_category
**Found by a ChatGPT Codex review of PR #114, and it is the PR's own argument
turning against it.** The first fix retreated one row whenever no sibling item
followed. A category with exactly **one** item is the configuration where that
degenerates into the unconditional step-back the PR had just argued was *worse
than the bug*: no sibling below, so it steps back — onto the category's own
header, one `d` from `RemoveCategory`. The escalation the change exists to
prevent, in the one shape the predicate did not exclude.
**The enumeration, which is what makes the second fix total.** `CatTreeEntry`
has two variants and `flatten_cat_tree` emits a header then its items, so for
an item at `i` the row above is a sibling item or *that category's own header*,
and the row below is a sibling item, the *next* category's header, or nothing.
Deleting `i` leaves three cases:
1. below is a sibling → stay at `i`; the list closes up. Safe.
2. above is a sibling → `i-1`. Safe.
3. neither → **the category is now empty, so both neighbours are `Category`
rows.** There is no safe *adjacent* landing at all.
Case 3 is why the adjacent-only rule could not be repaired in place, and why
"land on the own header" is not an option: it is the escalation. The landing is
therefore the nearest `Item` row, preferring above, then below — never a
header. The cost, stated rather than hidden: the cursor may cross into another
category. Held `d` then deletes **one item per press**, each with its own
status line, walking upward; it never reaches `RemoveCategory`, which is
exactly the criterion the review asked for.
**The residual**, unavoidable and stated: if the deleted item was the only item
in the whole tree there is no `Item` row anywhere, `safe_landing` returns `None`
and the cursor does not move. At that point every category is empty, so
`RemoveCategory` carries no cells and no formulas — a non-measure formula
target *is* an item, so an empty category has none.
**The fixture is fenced on purpose.** Both `Region` and `Product` are expanded
so `Product`'s single item has its own header above and the end of the tree
below — the two `Category`-row neighbours case 3 describes. A first version
expanded only `Product`, and the cursor fell off the end of the shortened tree,
where `cat_tree_entry_at` is `None` and the delete arm answers "No category to
delete" — **passing for a reason that has nothing to do with the fix**.
The assertion is not the landing index: it rebuilds the post-deletion tree,
seats the cursor where the command put it, and asks whether a second
`delete_at_cat_cursor` composes a `RemoveCategory`. That is the consequence the
review named rather than a proxy for it. Injecting the pre-review rule
(`closed_up.or(Some(cursor.saturating_sub(1)))`) fails this test on both paths
while the two adjacent-case tests still pass — the singleton is the
discriminating input and nothing else in the suite sees it.
### resolve::tests::deleting_an_item_with_a_sibling_below_it_leaves_the_cursor_alone
The rival fix, which is why this entry exists. The obvious repair is the one
`delete-view-at-cursor` and `delete-formula-at-cursor` already use: step the
cursor back whenever it is above zero. On a **homogeneous** list that is right,
and on this one it is worse than the bug — the first item of a category would
retreat onto that category's own header, which is the same escalation arriving
one row earlier, and holding `d` would walk backwards instead of clearing the
category out. This test fails under that unconditional repair and passes under
the conditional one, so the two are separated by an input rather than by
argument.
What makes the conditional total: within a category's run an item's
predecessor is always either a sibling item or that category's own header —
`CatTreeEntry` has exactly two variants — so `cursor - 1` can never be a
*different* category's header, and `saturating_sub` covers a row 0 the flatten
cannot produce.
### resolve::tests::cycling_an_axis_from_the_category_panel_says_so_as_the_tile_bar_does
improvise-25bf, and it pays for improvise-z336's third finding as well. Four
`Space` presses moved a category through None/Row/Col/Page with `#status`
frozen on a stale `Filter: Product = Espresso` — so the one piece of feedback
about the filter was wrong exactly while the user was trying to get rid of it,
which can manufacture a bug report out of a working code path. The staleness
needed no separate fix: a route that writes a fresh status writes **over** the
stale one.
The test asserts the status is `last`, not merely present, because the
`Command::Model` must precede it. Inject the fault by dropping the `extend`,
or by computing the status from the category's *current* axis rather than
`Axis::next()` of it — the expected `Region → Col` reads `Region → Row` and
fails, so the test sees the rotation and not just the sentence.
### a_locally_resolved_view_step_lands_where_the_host_would_land
improvise-cfly, and the bug is a **wave-level residue** rather than a defect in
either branch that made it. PR #115 put `Command::ViewBack`/`ViewForward` in the
view half, made `reduce_view` fold all three view effects line for line, and
gave every client its own `view_back_stack`. `resolve.rs` was owned by a
different agent that wave, and its `declined()` still refused `view-back`. So
the merged wave held the capability and no key: a browser user pressed `<` and
was told the feature does not exist, over machinery that fully supported it.
**The decline's stated reason was false by the time it shipped**, which is the
half worth more than the code. It read *"it commits a drill first, so composing
it would commit another client's typing"*. Both clauses were checked against the
tree rather than repeated. The first is still true of the `Cmd` —
`ViewNavigate { forward: false }` emits `ApplyAndClearDrill` before `ViewBack`.
The second no longer follows, for two independent reasons: `ApplyAndClearDrill`
is off the wire entirely (gotcha 38), so a resolver **cannot** compose it even
by mistake; and no browser path stages a drill at all — `drill-into-cell` is
declined, and `edit_or_drill`'s `CellId::Aggregate` arm answers with a sentence.
The residual divergence from the `Cmd` is therefore confined to a state this
client cannot reach, which is what `mirror_the_cmd_line_for_line` costs here and
is stated rather than hidden.
**The asymmetry the ticket warned about is real, and it is not the one the
decline named.** `reduce_view`'s `stepped` returns early on an empty stack —
silently. `ViewNavigate` does not: it emits `set_status("No previous view")` /
`"No forward view"`. A client that sent the bare `Command` would therefore agree
with the host on the view and diverge on the status line in exactly the case
where the user needs to be told something. `step_history` mirrors both arms.
Three rival fixes, each of which fails:
- **Emit the step unconditionally**, dropping the sentence. The unit test reads
`[ViewBack]` where it expects `[SetStatus("No previous view")]`; `both_ends`
reports `<: client and host disagree about the status line`, `""` against
`"No previous view"`.
- **Read the other direction's stack** in each arm — the mistake `Rewind`
exists to prevent one level down. `<` with history answers `No previous view`;
the lockstep pair diverges on the mode, because the host's `ViewBack` restores
the frame's mode and the client never steps.
- **Reconstruct the step as `Command::SwitchView(previous)`**, which is the
fix that looks right and is the one improvise-ls3y already paid for. It lands
on the same view and then `switched()` pushes the view being left onto the
**back** stack and clears the forward one, so the next press goes the wrong
way. The unit test names `SwitchView("Default")` against `ViewBack`.
The lockstep test asserts **both stacks**, not the active view, for that last
reason: a step that pops without pushing agrees on where you are and disagrees
on where you can go next, and only the second press shows it. Its empty arm is
pressed first and asserted against the host's own status, so a fixture that
silently had history would be caught rather than quietly testing one arm twice.
### resolve::tests::the_forward_step_mirrors_its_cmd_and_one_key_reaches_it
**`view-forward` is bound in no keymap, in either front end** — not a browser
gap, and not something this ticket predicted. `Keymaps::default_keymaps()` binds
`<` to `view-back` and nothing to `view-forward`; the command is registered, so
the TUI reaches it as `:view-forward` and a browser cannot, because `declined()`
refuses to enter CommandMode at the door. So the forward half of the history has
been unreachable by keypress since it was written, and pressing `<` twice is the
only way to observe the forward stack at all.
The arm is added anyway and tested by name, because `composed` keys on the
command name and the keymap is the *shared* table both ends read: the day a key
binds `view-forward`, nothing else must be touched. What keeps that from being
unreachable code with no guard is the second half of this test, which asserted
that no keymap binds `view-forward` **and** that one binds `view-back`. The
control matters: without it the check passes against a misspelling, which is the
neighbouring-question defect §7 names. When a binding is added, this test fails
and is where the gap stops being one.
**improvise-htl6 is that day, and the assertion was inverted rather than
deleted.** `,` now binds `view-forward` (the argument for that key over `>` and
over `Ctrl-R` is in `context/code/tui.md`), the test is renamed, and the second
half now asserts the binding **exists** — with `view-back` kept as the control,
which now guards the opposite failure: a table that binds everything, or a
sweep that reports names it never read. A third assertion pins the key itself,
`resolve(&state, ev(Char(','))) == Local([Command::ViewForward])`, so the
binding cannot drift onto some other key while the presence check stays happy.
**That third assertion was vacuous as first written, and the paragraph above
said it was not** (improvise-3ij0). It called `composed(&state, ev(Char(',')),
"view-forward", &[])`, and `composed` dispatches on the command **name**: the
`view-forward` arm never reads the event, so the `,` was decoration and the
assertion restated the first half of the test. Rebinding `view-forward` from `,`
to `~` in `keymap.rs` left it green; the key was pinned only by
`ui::app::tests::the_view_history_walks_both_ways_from_the_keyboard`, in another
crate. It now goes through `resolve`, which looks the key up in the shared
keymap before anything reads a name — so the rival fails **here**, at this
assertion (resolve.rs, "and the key is `,`"), and the pre-fix code under the
same rival was run and passes. The general shape: a helper that takes a key
*and* a name, and dispatches on the name, cannot pin the key; only the lookup
can.
**The rival: bind `,` to `view-back` instead** — the misspelling the original
control was written against, and the one a copy-paste actually produces. It
fails this test *and* `ui::app::tests::the_view_history_walks_both_ways_from_the_keyboard`,
because with an empty back stack `view-back` leaves the active view where it is.
Deleting the binding outright is the fault the pre-fix run already
demonstrated.
### "the deployment notice starts collapsed and remembers being opened"
The page's `#backend-notice` was a static `<p>` carrying roughly ten dense
lines of configuration reference — `?doc=`, `?seed=`, `?ws=`, `wss://` versus
`ws://`, and the reverse proxy a plain-`ws://` server needs behind TLS — and it
was rendered above the tile bar, the category tree and the grid on **every**
visit. On the deployed page at 1280×820 it cost about 230px of the 820, which
is the difference between landing on an application and landing on its README.
Reported by the user against `fiddlerwoaroof.github.io/improvise/app/`.
The first-visit default is **collapsed**, and the argument is that nothing is
actually withheld by collapsing it: the one fact a first-time visitor needs —
the document lives in this tab, is saved to this browser's private storage, and
never leaves the machine — is the `<summary>`, which is never collapsed. What
is behind the disclosure is *configuration*, which is looked up rather than
read, and which nobody needs before they have decided they want a second
document or a shared one.
The test asserts **both** directions across a reload, which is what separates
"remembers" from its two neighbours. A page that never reads storage passes the
open-then-reload half only if it also defaults open, and fails the
collapse-then-reload half; a page that always opens fails the first assertion
outright. Inject either fault — drop the `noticeOpen(remembered(...))`
assignment, or drop the `toggle` listener — and exactly one half fails.
What it does **not** establish is that storage is *optional*. A private window,
or one with site data blocked, throws from `localStorage` rather than returning
null, and Playwright's default context stores normally; that path is covered at
the unit level instead, by `noticeOpen` reading every non-spelling as the
default, with the `try`/`catch` in `app.js` argued and not run.
### "the notice opens from the keyboard, which the grid's own handler must not claim"
A `<details>` is a real control and its `<summary>` takes Enter and Space. The
page also installs a `window` keydown listener that hands the key to
`Client::on_key_event` and calls `preventDefault()` whenever the client claims
it — and `Enter` is claimed (it is forwarded upstream, which is why
`shell.spec.js` uses it to compose a frame on a client that has no snapshot).
So without the `el.notice.contains(e.target)` guard, focusing the summary and
pressing Enter would send a frame and suppress the browser's own toggle: the
disclosure would be operable by pointer and dead by keyboard, which is the
whole accessibility argument for using `<details>` in the first place.
Inject the fault by deleting that one guard line; this test fails and nothing
else in the suite moves, because every other keyboard assertion drives `#grid`.
### "the minibuffer is raised over the grid rather than filed under it"
`#minibuffer` was the last flex item in the body column, drawn as a 14px line
in the page's bottom-left corner with a one-pixel top border. In a terminal
that position is a convention with decades behind it; in a browser window it is
where nothing is read, and the user reported exactly that — a prompt reading
"Display a menu" in small text at the very bottom-left, missed. With the grid
scrolled the cell being typed into can be 800px away from the text showing what
is being typed.
The content is unchanged and deliberately so: `RenderMinibuffer` is built in
`improvise-wasm-client/src/render.rs` from `state.view.mode.minibuffer()`, the
same semantic value `src/draw.rs` draws, so there is no second derivation here
of what a prompt says (design-principles §5). What moved is position, size and
contrast, which is paint.
Three assertions, and the middle one is the one that earns the test. Being
inside the viewport is necessary but not sufficient — a short document satisfies
it with the old layout too. **Overlapping `#grid`'s own rectangle** is what
cannot happen for a statically-positioned element that follows `main` in the
flow, because a flex column gives its items disjoint bands: that is the
assertion that separates "raised over the content" from "moved within the
strip", and it fails the moment `position: fixed` is reverted. The font-size
comparison is the only prominence claim a browser can check; it was equal to the
body's before and is now strictly larger.
The precondition matters: `#mode` must read `INSERT` before anything is
measured, because `getBoundingClientRect` on a hidden element returns zeroes
and every comparison below would then be about the origin.
### "an error cell stays an error cell when it is also dimmed or selected"
Found reviewing the styling change that introduced it, before it was pushed.
`td.error` and `td:not(.editable)` used to set **different** properties —
`color: var(--error)` and `opacity: 0.7` — so both applied and a dimmed error
cell was a dimmed red. The restyling made the dimming a `color` too, at the
same specificity, and put it **after** `td.error` in the file, so an error in
an aggregate cell rendered plain grey: measured `rgb(107, 103, 95)` where the
error colour is `rgb(168, 50, 45)`. `td.selected` and `td.typing` had the same
effect by setting `color: var(--fg)`, which is how a *selected* error lost its
colour as well.
That is the whole class of defect a stylesheet has and a program does not:
two rules that were independent become one property, and nothing in any
language fails. `#grid td.error` is the only place in this page where colour
carries meaning rather than emphasis, and it is the one a reader takes at face
value — a circular reference or a `mixed` aggregate reading as an ordinary
number is worse than it reading as nothing.
The fix is ordering plus a rule: the dimming goes **above** the semantic
colours, and a selection is a ring and a tint and never a colour. The test is
aimed at the stylesheet rather than at a document, deliberately — the question
is which rule wins, and a fixture per combination (a dimmed error, a selected
error, an error being typed over) would be three documents testing three
documents. It appends a `<td>` carrying each combination to the real page's
real table, reads `getComputedStyle`, and compares against two controls taken
the same way: a plain unsettable cell (what it must not be) and an error the
dimming cannot reach (what it must be). Reading absolute colours instead would
pin the palette, which is not the property.
Inject the fault by moving the `td:not(.editable)` block back below
`td.error`, or by restoring either `color: var(--fg)`; each makes exactly the
corresponding assertion fail.
## The page bar as a control (improvise-rw6l, improvise-771e)
The user, pointing at the deployed page: *"The page items here (e.g. Quarter)
should open a menu when clicked so you can pick a different option. Also have a
down-arrow indicating there's a menu."*
Almost nothing had to be derived for it. `PageAxis::items` was already
projected, `ModelCommand::SetPageSelection` was already on the wire, and the one
gap was that `RenderPageAxis` **threw the names away**: it kept a position and a
count, which is all improvise-p1ym needed for `[`/`]`. Widening
`RenderPageCycle` with `items` is a pass-through, and the items sit *inside*
`cycle` rather than beside it so there is exactly one predicate — a Filter axis
has no `cycle`, therefore no arrow, no menu and no items, all off the one home
`PageAxis::cycle` already was (gotcha 27).
### render::tests::a_navigable_page_axis_relays_the_items_it_may_be_fixed_to
Fault injected: `items: Vec::new()` in place of `items.to_vec()`. The menu then
paints empty on a real document and nothing in the browser can see it, because
the down-arrow and the chip's `<button>`-ness both follow `cycle`, not the item
count. The Filter row in the same test is what stops the fix being "just always
send `axis.items`".
### render::tests::a_page_tile_reads_as_the_whole_word
improvise-771e: the browser tile bar drew "Quarter Pag". `Axis::short_label`
returns three characters so the TUI's tile bar aligns in a fixed-width terminal
(gotcha 26); a browser has no column to fit, so the abbreviation buys nothing
and reads as a rendering defect. `plain_label` is the other vocabulary already
on `Axis`, so the decision stays in the shared layer and only the *choice* of
vocabulary is local — the `AxisColor → ratatui::Color` shape §5 blesses.
**The fixture is the whole test.** The two vocabularies differ on exactly two
of five axes — `Page` ("Pag") and `None` ("·") — so `client_with_panels`, whose
tiles are `Row` and `None`, could see the `None` half and **not** the `Page`
half the ticket is about. This test projects a `Page` tile and a `Column` one
beside it: the second is what says this is a change of vocabulary rather than a
rewrite, since `Col` is spelled the same either way.
**What actually pinned the truncation was not where the ticket said to look.**
improvise-771e says to check `tests/grid_frame_agreement.rs` before changing
anything. It does not mention the tile bar at all. The assertion that held
"Quarter Pag" in place was `harness/tests/drill.spec.js`, which asserts the tile
text exactly, twice — a Playwright spec, in a directory the ticket never names,
asserting the bug as the expected value. The lesson is the search, not the file:
a *rendered string* is pinned wherever anything reads the screen, so the grep is
for the string, not for the function that produces it.
### a page axis offers its items, and a filter axis offers none
`web/tests/dispatch.test.js`. `pageEntries` relays `cycle.items` and gives a
Filter axis `[]`. Fault injected: `items: axis.cycle ? ... : axis.items`, i.e.
falling back to the raw list — the Filter row then offers items nothing can act
on, which is the "a menu that cannot do anything" case the ticket names as
worse than no menu at all.
### a chip opens the menu and a menu row picks, one attribute apart
improvise-b8ot's shape: one delegated handler per surface on a stable root,
operands off data attributes, and **no structural inference**. A chip carries
`data-axis`; a menu row carries `data-axis` *and* `data-item`. That single
difference is the whole disambiguation, so `pageItemAction` is one function over
both surfaces and `app.js` registers the same listener on each.
The row worth having is `{ axis: "0", item: "" }` → `open`. `Number("")` is `0`,
so a naive read turns an absent item into item zero — which picks Q1 whenever
the user clicks the chip for an axis already on Q1, i.e. silently does nothing,
which is indistinguishable from the menu failing to open.
### the menu answers its own keys and swallows the rest
The menu is the **third** state of the kind `editing` already was: `app.js`'s
window `keydown` handler returns early for it. That is the same treatment and
not a new mechanism, and the reason is the same — the menu is anchored to a
chip above the grid, so `hjkl` reaching the grid would scroll the thing the menu
is pointing at out from under it.
`menuKey` therefore answers **every** key, with `ignore` as the default rather
than a fallthrough: an unlisted key is swallowed, not passed on. The
`count: 0` row is a real state, not defensive padding — a repaint can empty the
axis under an open menu — and it closes rather than wrapping modulo zero.
### the menu is driven by the keyboard and does not leak keys to the grid
`harness/tests/pages.spec.js`, and the assertion that earns the slow test is
the **negative** one: `j` moves the menu's active row *and* leaves
`#grid td.selected` where it was. A spec that only checked the menu moved would
pass with the key leaking, because both handlers would have run.
### the help overlay pages nothing behind itself
improvise-wiew's product call, executed. The overlay is open, `]` is pressed,
and the page bar still reads `Quarter = Q1 (1/4)`. Its sibling — a panel open,
`]` pressed, bar moves to Q2 *and the panel is still open* — is what stops the
first one passing because the key does nothing anywhere.
### a modified key is the browser's, and the menu neither acts on it nor claims it
Found by ChatGPT Codex on PR #123, and correct. The page menu's early return
sat **above** `app.js`'s `metaKey` guard, so with the menu open `Cmd-R` reached
`menuKey`, was answered `ignore`, and then hit
`if (!e.ctrlKey && !e.altKey) e.preventDefault()` — which is true for a Meta
combination. Reload was blocked. `Ctrl-j` and `Alt-Home` had the mirror problem
in the other direction: they *moved* the menu, although the comment two lines
below said Ctrl and Alt combinations stay the browser's.
The repair is ordering plus one bail: the `metaKey` guard moves back above the
menu branch (so the menu is not even consulted), `pageMenuKeydown` returns
immediately on `ctrlKey || altKey`, and what remains then claims
**unconditionally** — every unmodified key including `ignore`, which is the rule
the menu exists to keep.
**Why the assertion is shaped the way it is.** The listener that records
`defaultPrevented` is installed by `page.evaluate` *after* the page has loaded,
so it runs **second** on the same target and observes what `app.js` decided. One
registered first would read `false` for every key and pass against any code at
all. And the test ends by pressing an **unmodified** `j` and asserting
`["j", true]`: without that row, "nothing was defaulted away" is equally
satisfied by a page that claims nothing, so the modifier would not be what the
test is about.
Fault injected: the reviewed code restored exactly — menu branch above the
`metaKey` guard, no `ctrlKey || altKey` bail, conditional `preventDefault`. The
test fails on `Control+j` moving the active row from `✓ Q1` to ` Q2`.
### "no element of the bar moves as the mode changes"
improvise-n6t8. The user's report was that *"the top bar jumps around a lot"*
and that *"the normal/categories/etc. pill changes sizes constantly"*. Four
children of `<header>` were sized by their own content and all four changed on
the **same** event, so a mode change reflowed the bar even with no message on
screen: `#mode` (`help::mode_label`, 4 to 14 characters), `#hint`
(`help::hint_line`, one line per mode), `#doc-name` (gains and loses ` [+]`) and
`#view-name` (`:empty { display: none }`, so it appeared out of nothing).
The walk is **nine** of the seventeen `mode_label` variants, named individually
in the spec rather than described as "every mode": `SavePrompt`, `ExportPrompt`
and `CommandMode` are refused by `resolve::stranding`, `RecordsNormal` and
`RecordsEditing` decline out loud, `ImportWizard` has no browser entry and
`Quit` is not a screen. Each iteration asserts `#mode` **first**, because a key
that failed to reach its mode would leave NORMAL's boxes in place and every
comparison after it would hold vacuously.
Fault injected: `min-width: 21ch` removed from `#mode` in the served stylesheet.
Fails at the first mode change with *"the bar moved in CATEGORIES"*.
**What this found that the ticket did not predict.** On its first run it failed
on the header's *height*, not on any pill. `#status` was `flex: 0 1 auto` beside
a `flex: 1 1 auto` hint, so a longer hint took the shrink out of `#status`,
which **wrapped to two and then three lines** — 52.5px of header becoming 71.0px.
Reserving the pill alone would have left that untouched and the bar still moving.
### "the measurement sees a label longer than the reserve, and not one that fits"
The check-the-checker for the walk above. A reserve sized by eye regresses
silently the day an `AppMode` with a longer label is added — the compiler makes
you write `mode_label` for a new variant, and nothing checks that the CSS still
fits what you wrote — so the measurement has to be able to *see* one.
Both directions are asserted, and both are needed. Sixteen characters (what the
reserve claims to hold, against a longest-today of fourteen) must **not** move
the boxes, or the reserve does not hold what it says. Forty characters **must**
move them, or the walk is measuring something that cannot fail.
This is also why `#mode` carries a `min-width` and not a `width` or a
`max-width`: a cap absorbs the overflow inside a box that never grows, so the
bounding boxes would be identical and the fault invisible. **A reserve that caps
is a reserve that hides its own failure.**
### "a status message changes nothing in the bar and overlaps the grid"
`ViewState::status_msg` reached `#status`, which is inside `<header>`, so every
refusal this client makes — three of them are advertised in Normal mode's own
hint line — reflowed the bar. It is a toast now, inside a `position: fixed`
dock, so it takes part in no layout at all.
The overlap assertion is what separates "out of flow" from "in a strip of its
own": a toast given its own row would have taken its height from `#grid`, and
both the header box and the grid box would have moved.
Fault injected: `onModel` restored to writing `el.status.textContent` beside the
toast, in the served `app.js`. The header box changes and the test fails.
### "a dismissed message does not come back on the next keypress"
**The trap, and the reason a timer alone is not the fix.**
`ViewState::status_msg` is *state*, not an event: `repaint()` calls
`shell.setModel(model.status)` unconditionally and runs on every revision bump,
and nothing in Rust clears the string. So a toast hidden by `setTimeout` is
re-offered — and re-shown — the moment the user presses anything, which reads as
a new event and is worse than a message that simply stays.
`createToasts` keeps what it last **accepted**, and keeps it after the toast
hides, so an equal offer is recognised as a repaint and dropped. The spec drives
the real trap: `:` for the message, `page.clock.runFor(10s)` past the window,
then `ArrowDown`, which moves the selection and so forces a full repaint.
The last assertion — that the element still *contains* the text — is what makes
the two before it about **visibility** rather than about an emptied node. It is
also what `drill.spec.js` needs: its discrimination between two refusals (`>`
must not talk about aggregates) reads `#toast`'s text, and an emptied node would
satisfy `not.toContainText` vacuously the moment the timer fired.
Fault injected: the `if (next === offered) return;` identity check removed from
`createToasts` in the served `shell.js`, leaving dismissal to the timer alone.
The toast is visible again after `ArrowDown` and the test fails.
### "a toast does not cover or displace an open minibuffer"
What happens when both are present, which the ticket asked to be decided rather
than discovered. `#bottom-dock` is anchored at its **bottom** and stacks upward,
so the minibuffer's box is identical with and without a toast above it, and the
toast's bottom edge is at or above the prompt's top edge. A top-anchored stack,
or a toast positioned by an offset from the minibuffer's own height, would both
have moved the prompt.
The toast is **forced** here rather than provoked by a key, and the reason is
stated in the spec: a host message arriving *during* text entry is reachable but
has no keyboard route in this client — in `INSERT`, `:` is bound to `AnyChar`
and types a colon. The rule under test is the stylesheet's, not the route's, so
setting the two DOM states directly is the honest instrument rather than a
contrivance that would test the contrivance.
### "a repaint's model status cannot reach the transport's sink at all"
The unit half of the same rule, and a replacement rather than a deletion.
`composeStatus` joined the transport's fact and the document's with `·` because
a repaint had once silently erased a connection error (gotcha 46); the two facts
now go to two *elements*, so the join is gone. A separation with no guard is how
one regresses into a single sink, so the test moved rather than went.
It asserts the whole sequence each sink saw, not the last value: the fault it
separates from is `onShell` **losing** its text, and a test reading only the
final `#status` would pass against a page that wrote the model's status there
and then happened to be asked again.
### the_tile_bar_route_into_records_mode_does_not_strand_the_user
improvise-bd9o. `declined()` answers `toggle-records-mode` out loud, and the
code read as though that closed the route into records mode. It did not: the
tile bar carries `_Index` and `_Dim` (`View::tileable_cats`), `set-axis-for-tile`
composes locally, and records mode auto-detects from `_Index` on Row plus `_Dim`
on Column — so `T`, a tile walk, `r`, another walk, `c`, `Esc` lands a browser in
`RecordsNormal` with nothing declined on the way.
**What failed before the fix.** At `d46ec59` the test's own five-key preamble
reached `grid.records == true`, and then `i` changed *nothing*: status stayed at
the tile bar's `"_Dim → Col"` and the mode stayed `RecordsNormal`. Verified by
running the test against unmodified `resolve.rs`.
**Two sources, and the fixture walks every column because of it.** A probe of
the projected row showed them to be different failures:
| column | `CellId` | what `i` did |
|---|---|---|
| 0 (coordinate) | `None` | nothing at all — `edit_or_drill`'s `cell.id.as_ref()?` short-circuited |
| 1 (`+cat`) | `Create(Category)` | nothing — the `Create` arm returned `None` |
| 2–3 (measures) | `Record(_)` | opened the editor; `Enter` then died in `commit`'s `grid.records` early return |
| 4 (`+`) | `Create(Measure)` | nothing — the `Create` arm returned `None` |
The fixture's grid is **five** columns wide (`extent = {rows: 1, cols: 5}`), and
an earlier version of this table stopped at `2–3`, so a reader reconstructing it
got four. Review caught that; the row above is why the table is worth having at
all, since it is the only place the fixture's shape is written down.
Only the measure columns are the case improvise-bd9o describes. The first is improvise-fdk1
seen from the key path: a coordinate column has no surrogate because a rename
names a record that does not exist yet. A fixture placed on a measure column —
the obvious one to write, since that is where the ticket's transcript stands —
cannot see either of the others, so the test iterates `0..extent.cols` and
prints the `CellId` in each failure message.
**Two rivals, each failing on a different assertion.**
- *Guard the arm, not the grid*: `CellId::Record(_) if state.grid.records =>
decline`. This is the shape the ticket's own acceptance criterion reads like,
and it is what a fixture on a measure column would have accepted. It fails
here at **column 0 (`None`)** on the status assertion — the coordinate column
never reaches an arm.
- *Say it and open anyway*: return the decline **plus** `ChangeMode(mode)`. It
fails at **column 0** on the second assertion, `!mode.is_editing()`. This is
the rival that matters most, because "the user was told something" is the
assertion one naturally writes, and alone it is satisfied by a client that
speaks and then strands the user in an editor whose `Enter` is still a black
hole.
**Why the status line is cleared with a sentinel and not a keypress.** Each
column needs a fresh "was anything said", and every candidate key for clearing
it (`x`, `Escape`) is itself a key this client might decline or act on — which
would either supply the status the assertion is looking for or move the cursor
out from under it. `state_mut().view.status_msg` is written directly to a value
no code path produces.
**Why the preamble reads the tile bar rather than counting `l` presses.** The
ticket's transcript spells the route as `T r l c Esc`, which depends on
`tileable_cats`' order. `tile_to` finds the named category's index in
`state().tiles` and walks to it, so the test pins *the route exists*, not a
particular ordering the producer is free to change.
### opening_an_editor_is_declined_off_the_grid_and_not_off_the_mode
improvise-bd9o, and it exists because **the route test could not state the
clause the fix was argued on.** `edit_or_drill`'s guard reads
`state.grid.records` and not the mode, and both the commit message and
`context/code/clients.md` make a point of it. Adversarial review injected the two
mode-shaped guards and ran the **whole workspace**:
| injected guard | workspace result |
|---|---|
| `if state.view.mode.is_records()` | **1659 passed, 0 failed** |
| `if mode.is_records()` (the keymap argument) | **1659 passed, 0 failed** |
So a wrong fix that the prose explicitly argues against passed everything,
including `the_tile_bar_route_into_records_mode_does_not_strand_the_user`. That
test enters records mode the way a user does — through the tile bar — where the
grid and the mode **agree**, so it is standing next to the question rather than
on it. This is `doctrine(a_careful_comment_is_weak_evidence_that_anything_checks_it)`
with the comment being the one that argued hardest.
**The separating input is a drifted client**: `records_client(&app)` with
`view.mode` forced to `AppMode::Normal` over a records grid — the state
`doctrine(a_substitution_is_a_defect_at_every_site_that_makes_it)` records as
what the substitution actually produces. Under either injection the guard reads
"not records" and the editor opens; under the committed fix the grid says
records and it declines. Both injections were re-run against this test and both
now FAIL at its first assertion.
**The control is not optional.** A regular grid in the same `Normal` mode must
still open the editor, or "it declined" is satisfied by a client that never
edits anything — the same vacuity its sibling
`a_records_commit_is_declined_off_the_grid_and_not_off_the_mode` guards against,
and the reason this test is that test's twin rather than a new shape.
It asserts on `Command::ChangeMode(m) if m.is_editing()` rather than on the
status string, because the fault is *an editor opening*, and a fix that speaks
and opens anyway is a distinct rival already covered at the route level.
---
### a_new_view_named_over_a_gap_agrees_with_the_host_and_spares_the_survivor
improvise-vw47's cross-front-end half. The new view's name is derived in two
places — `cmd/panel.rs::CreateAndSwitchView` and `resolve.rs`'s
`"create-and-switch-view"` arm — and design-principles §5 says a derivation
done twice is the defect. The previous agent on this ticket refused to fix one
site for exactly that reason and implemented nothing, which was the right call.
**What this test proves that `both_ends` alone cannot.** Against the unfixed
source `both_ends` *passes*: both resolvers computed `format!("View {}", len +
1)` and were wrong **identically**, so the two documents agreed and the
agreement check saw nothing. The bug is therefore caught by the two assertions
after it — the view count must rise, and the surviving `View 4`'s `,.4` format
must be untouched — and it fails with `left: 3 right: 4`. So the test carries
two separate claims and needs both: the ends agree, *and* what they agree on is
right.
The fixture is `furnished_host` plus a gap: create `View 3`, create `View 4`
with `,.4`, delete `View 3`. That leaves three views, so `len + 1` names
`View 4`, which exists. `gapped_host` is built twice rather than cloned because
`App` is not `Clone`, the same reason `twin_of` exists.
The injected rival is the one the ticket was blocked on: fix `panel.rs` and
leave `resolve.rs` spelling the count. `both_ends` then fails on the document
text before either later assertion is reached — the host writes `View 4` with
`,.4` intact plus a new `View 5`, the client asks to replace `View 4` and
resets it to `,.2`. That failure is the evidence that this file is where the
one-derivation claim is witnessed, and that a test per front end would have
passed on both sides of the divergence.
### ctrl_s_in_the_browser_writes_the_hosts_document
improvise-jz6w, end to end and in one test on purpose. `ClientFrame::Save` and
`Host::handle`'s arm for it landed with improvise-368h and were covered by
`a_client_can_ask_its_host_to_save` in `ui/host.rs` — which **constructs the
frame itself**. Nothing anywhere pressed a key and watched a document get
written, so a `KeyOutcome` with no shape for "send this frame" left the feature
complete on both sides of a seam that did not exist. Two half-tests that never
meet is the exact failure mode, which is why this one runs a real `Client`'s
`key()` into a real `Host` over a real `App`.
The fixture is `browser_round_trip.rs`'s own `host()` given a `file_path` and a
`MemoryFs`, so `Host::save` has somewhere to land and the test can read it back
through `App::storage`. The pre-assertion that storage is empty before the key
is pressed is what stops a fixture that was written at construction time from
passing this vacuously.
**Against HEAD it failed** with `left: Key { code: Char('s'), modifiers:
CONTROL } right: Save` — the black hole of gotcha 47, observed rather than
argued.
**Two rivals were injected.** Mapping `Request::Save` to
`ClientFrame::Commands(vec![])` — the wide-variant mistake this ticket declined,
a request smuggled into the batch envelope — fails on the frame assertion with
`left: Commands([])`. That is the one the storage assertion alone could not see
with certainty, because an empty batch writes nothing either way; asserting the
**frame** is what separates "reached the host" from "reached the host as the
right kind of thing". Deleting `requested`'s `"save"` arm restores `Upstream`
and fails identically to HEAD.
### ctrl_s_asks_the_host_to_save_and_composes_nothing
The unit half of the same fix, and the one that states the *negative*: `composed`
must return `None` for `"save"`. A resolver that grew a `"save"` arm composing
some `Command` would satisfy the end-to-end test only by accident of ordering
(`requested` is checked first in `step`), and would be dead code the moment that
order changed. Asserting the absence is what keeps gotcha 45's reason — a
runtime-fallible write does not belong in an all-or-nothing batch — enforced at
the resolver rather than remembered.
It also pins `Request::Save.frame() == ClientFrame::Save`, the one mapping from
a resolver's decision to an envelope.
**Rival:** renaming `requested`'s match arm so `"save"` no longer matches. The
test fails at the first assertion with `Upstream(...)`, which is what the
browser did before this ticket.
### no_binding_puts_a_request_inside_a_sequence
`sequence` refuses a `Step::Request` by returning `None`, which becomes
`Upstream` — a black hole. That is only acceptable because nothing binds a
request inside a `Binding::Sequence`, and "nothing does" is a claim about the
keymap that nothing else checks. This test walks every mode's bindings,
recursing into prefixes and parents exactly as `bound_commands` does, and
requires that no sequence step is a `requested` name.
It asserts the walk found **some** sequence steps before checking any of them:
a walk that silently visited nothing would otherwise pass forever, which is the
vacuous-assertion shape design-principles §7 names. The same test is what makes
the refusal arm safe to leave unreachable instead of inventing a second frame
for it.
---
## The optimistic fold's rollback (improvise-nxah)
The design argument is `context/code/clients.md`, "The optimistic fold is put
back by *the fold*"; the wire half is `context/code/protocol.md` §4.
### a_refused_commit_leaves_the_client_where_the_host_says_it_is
**The bug.** `Client::key` folded a locally-resolved batch before sending it and
`fold_frame` answered `ServerFrame::Rejected` with a sentence and nothing else,
so a refused commit left the client's view slice ahead of the host's. Measured
at `22ab58f` against a real `Host`: the client sat at cursor `(1, 0)` with
`buffers {"edit": ""}` while the host's mirror of that same client was at the
origin with `{"edit": "12007500"}` — the user's typing.
**Why it is not vacuous.** The comparison is against
`Host::sessions().get(id).view()`, the host's **own** mirror, which ADR 001
clause 4 restores to the pre-frame state on a refusal — so the test asserts the
client landed where the authority says it is, not merely that two derivations
agree (improvise-vw47). It is total over `ViewState` rather than a hand-kept
list of fields: everything but `status_msg` is compared, and that one is carved
out with its own assertion because the rejection sentence is this client's own
notification and the mirror has no way to hold one. The fixture first asserts
the two ends **agree before** the refusal, so a test that could never have
disagreed is not passing by accident.
**The fault injected.** Dropping the `self.replay()` call from
`ClientState::refused` — keeping the queue bookkeeping and doing no rollback —
fails it.
**What it does not cover.** That the *ordering* of a rollback is right under
pipelining; that is the next entry.
### refusing_one_of_two_frames_in_flight_keeps_the_other
**What it separates.** A single saved snapshot from a queue. Two mutating frames
are folded before either is answered; the host refuses the first and accepts the
second. Restoring a snapshot taken before the first would discard the second,
which the host *did* apply — so the client would diverge in the other direction.
The assertion is not "the state changed": it is equality against a second,
independently built client that only ever sent the accepted frame, which is what
"right" means here rather than "the two ends match".
**The fault injected.** Making `ClientState::replay` rebuild from `settled` and
replay nothing — the single-snapshot design — fails this test and no other in
the file, which is what shows it is aimed at the ordering and not at the
rollback.
### a_frame_the_host_will_not_answer_settles_without_one
**The invariant the queue rests on.** Only `Command::Model` can be refused, so a
frame carrying none is answered by neither `Accepted` nor `Rejected`; holding
such a frame in the queue would make the next answer pop the wrong entry. The
test sends a `SetStatus`-only frame, asserts nothing is held, then sends a
commit and refuses it, asserting the earlier frame survived.
**The fault injected.** Hard-coding `awaited = true` in
`ClientState::fold_outgoing` fails it.
### a_refused_save_does_not_roll_back_a_frame_it_never_answered
**The second way the queue could pop the wrong entry.** `ClientFrame::Save` is a
request, not a `Command` (improvise-368h), and its two refusals answer no
`Commands` frame. `Rejection::refused_an_edit` is the existing predicate that
separates them, and `refused` consults it before popping.
**The fault injected.** Replacing `if rejection.refused_an_edit()` with
`if true` fails it: a document with nowhere to write undoes an unrelated edit.
**And at the transport.** `a_client_on_a_socket_is_seeded_and_gets_its_edit_back`
(its own entry is above) was **extended, not added to**: after the edit comes back down the socket the test now
asserts the next frame is `ServerFrame::Accepted` — so the acknowledgement is
evidenced at the transport, in its position (last, after everything the frame
produced), and not only in the host's unit tests. Deleting the `Accepted` push
from `Host::mutating_frame` fails it.
---
## A narrow window does not widen the document (improvise-tg27)
`harness/tests/narrow.spec.js`. Reproduced at `22ab58f` before anything was
changed, and the ticket's own diagnosis was wrong in a way worth recording: it
suspected a `min-width: auto` flex trap swallowing `#grid`'s overflow, and
`#grid` has `overflow: auto`, which already zeroes that minimum. The reasoning
is in `context/code/clients.md`.
The measurement that found it: at 360, 420, 520, 600, 700, 768, 820, 900, 950
and 1000 px, `documentElement.scrollWidth` read **1011 at every one of them** —
a floor rather than a slope, which is the signature of a sum of `flex: none`
reserves rather than of a clipped child. `main` was correctly window-width
throughout, so nothing about the grid's own box was wrong.
### "a narrow window does not scroll the document horizontally"
One test over three widths — 360, 520 and 768 — because the property is one
property and a per-width test name would have been three names for it. Three
assertions per width, and the first is a precondition rather than the subject:
`dataCols < 6` states that `grouped.improv` really is clipped there. Without it
every assertion below would hold on a window wide enough to show the whole
grid, which is the "passes on a document with no overflow" shape.
`escaping` is reported as a list of ids rather than a count, so a failure names
which children of `<header>` sat outside the window — at 520 that was
`view-name`, `mode`, `hint` and `status`. Each assertion's message carries the
width, since a loop inside one test otherwise reports only that something
failed.
**Fault injected:** `web/style.css` restored to `22ab58f` and the site
reassembled. Fails at all three narrow widths with *"the document scrolls
horizontally"*, `1011` against the window width.
### "the grid keeps the larger share of a phone-width window"
`#categories` is `width: 15rem` and `flex: none` — 240px of a 360px phone,
leaving the grid 120. The assertion is an **ordering**, grid wider than panel,
and not a ratio: the exact split is a stylesheet number and pinning it would
fail on every future tweak while catching nothing.
**Fault injected:** `web/style.css` restored to `22ab58f` and the site
reassembled. Fails with *"the panel takes more of the window than the grid"*,
120 against 240. 360 and not 520: unfixed, 520 already passes this ordering
(280 against 240), so a cap that bites on a phone needs a phone to see it.
### What this ticket does NOT fix, and a claim that was wrong for an hour
`#hint` — the one line naming the keys that reach a clipped column — is still
squeezed. An earlier version of this spec asserted it became legible, and that
assertion **passed**, which is the part worth recording: it read the hint
immediately after the grid painted, when `#status` had not yet composed the
transport's fact and was narrower, and measured 186px. Settled, the same width
is **1px**.
Measured settled after the fix, `#hint` is 26px at 360, 1 at 520, 81 at 600,
317 at 700, 385 at 768, 67 at 900 and 6 at 1024 — non-monotonic, because it is
whatever the wrap happens to leave once `#view-name`, `#mode` and `#status`
have taken their reserves. No assertion is made about it. `theGrouped` now
waits for `#status` before measuring anything, because a bar that is still
settling is a transient and a spec that measures one fails in the direction
that reads as a pass.
### "at 1280px the bar holds its reserve on one line"
The other direction, and the reason `flex-wrap` was chosen over a `@media`
breakpoint. improvise-n6t8's stability spec runs at 1280, so this is what says
the bar that spec measures is untouched: one line, 38px, nothing wrapped.
It passes **before and after**, which is the point: a no-change assertion, not
a bug reproduction. `docScrollWidth <= clientWidth` holds here on the unfixed
tree too, since 1011 is the floor.
**A first version also asserted it at 1024 and failed**, which is the finding
worth keeping. `#status` composes the document's *name*, so the width at which
the reserve stops fitting depends on the document: at 1024 the fixture `shot`
is one line and `narrow-tg27` is two. The reflow threshold is therefore not a
constant and is deliberately not asserted anywhere — only that 1280 is above
it for a document with a harness-length name.
### "the measurement sees an overflow that is really there"
The check-the-checker. Every assertion above is a negative — *nothing*
overflows — and a negative is vacuous unless the measurement can see a positive.
So one is injected at the same width through the same path: `#doc-name`, which
held the largest of the original reserves, is given `flex: none; width: 1400px;
max-width: none` — the exact shape the fixed reserves had.
Both halves are asserted, because they can fail apart: `docScrollWidth` must
exceed the window (the page-level check works) *and* `escaping` must name
`doc-name` (the per-child check works). A version asserting only the first
would pass against a `escaping` computation that always returned `[]`.
**Fault injected:** the injection removed. The test then fails on its own
`toBeGreaterThan`, which is what says the assertion is not self-satisfying.
### "the keyboard reaches a column the window cannot hold"
Why the clipping is acceptable at all, and the assertion that separates *this*
bug from the one the ticket might be read as describing. At 520px one data
column is projected and the other five are never sent, so scrolling `#grid`
cannot reveal them — the browser's `Viewport` is a real window (gotcha 44/47).
The ticket's "or another explicit navigation" is the keyboard, and it already
worked before this change.
The walk checks **every intermediate step**, alternating `Web`/`Shop`, not only
the final column: a client that jumped straight to the end would satisfy a
last-column assertion alone. It then reads all three header levels, because
`Shop` at the innermost level does not distinguish `Mar/Shop` from `Jan/Shop`.
This test passes before and after. It is here because a future "fix" that made
`#grid` scroll by over-announcing the extent would have to keep it passing, and
because without it the narrow-width specs read as if clipping itself were the
defect.
## improvise-xg1z — an editing overlay with no buffer to draw
`render_model` built `editing` as
`is_editing().then(|| buffers.get("edit").cloned().unwrap_or_default())`, so an
`AppMode::Editing` that arrived without its `SetBuffer` painted `Some("")` — a
blank editor drawn over a cell that still holds a value. The page opens an
`<input>` from that string, so the next Enter commits the empty string and the
value is gone. An **absent** buffer and an **empty** one are two different
facts, and `unwrap_or_default` collapsed them.
Reachable because the pair is two `Command`s, not one: this client's own
resolver emits `SetBuffer` then `ChangeMode`, and a `Snapshot` or a host batch
can carry the mode without the buffer.
Demonstrated at `e7de19d`: the first test below reports `Some("")`.
### `an_editing_mode_with_no_edit_buffer_shows_the_cell_rather_than_a_blank`
**Superseded in part by improvise-ktlk** (entry at the end of this file). As
first landed, the fallback lived in `render.rs`: `editing` fell back to
`edit_seed` while `commit` and `append-char` still read the absent buffer as
`""`, so the overlay showed `1200` and Enter cleared it. The fallback now lives
in the fold (`ClientState::settle_editor`), which seeds the buffer itself, and
`render.rs` reads the same buffer `commit` does. The test's first assertion
changed with it: it used to check that the buffer was still absent after the
fold, and now checks that the fold seeded it.
The test asserts three things in order: what the fold put in the buffer, that
the cell really holds `1200`, and only then what `editing` says. Without the
second, a grid whose cell was blank anyway would pass against the bug.
**Fault injected:** `unwrap_or_default()`, the original expression. Fails with
`Some("")`.
### `an_edit_buffer_the_user_has_emptied_is_not_refilled_from_the_cell`
The half that bounds the fix. An empty buffer is a state the user typed their
way into with backspace, and refilling it from the cell would make the last
backspace un-doable — the editor would spring back to the stored value. This
test passes before *and* after; it is here to fail against the over-broad
repair, which is the one a reader reaches for first.
**Fault injected:** `.filter(|typed| !typed.is_empty())` in front of the
fallback — "empty or absent, refill". It fails **this test alone**, which is
what separates the two questions.
## improvise-hs0u / improvise-lut7 — a prefix chord in the browser
Both tickets have one root cause: `resolve` answered `Binding::Prefix` with
`KeyOutcome::Upstream` unconditionally, and `Host::handle` answers
`ClientFrame::Key` with an explicit empty vec (gotcha 47). So `g`, `y` and `Z`
were black holes, and everything behind them was dead — including
`grid_move`'s `GridMove::ToStart(true)` arm, which was correct and unreachable.
`Command::SetTransientPrefix` was already on the wire and already folded by
`reduce_view`; nothing composed it.
Demonstrated at `e7de19d`: all five tests below fail on the unchanged tree.
### `gg_reaches_the_first_row_the_way_shift_g_reaches_the_last`
improvise-hs0u. `G` reached the last row and nothing could get back, so the
asymmetry read as a bug rather than as an absence. The test arms the prefix by
**folding what `resolve` returns**, not by writing `transient_prefix` directly
— a fixture that sets the field by hand would pass against a `Binding::Prefix`
arm that still went upstream, which is the bug.
**Fault injected:** `Binding::Prefix(_) => KeyOutcome::Upstream(event)`, the
original code. Fails at the arming step.
### `yy_yanks_the_cell_under_the_cursor_and_p_pastes_what_it_yanked`
improvise-lut7. `p` was composed and could never do anything, because nothing
could ever put a value in `ViewState::yanked` — a *state* reason that no
name-level derivation can see, which is why `p` appeared on neither the
declined list nor the host-needed one. The test presses both keys in sequence,
so the paste is evidence about the yank rather than about a hand-seeded field.
**Fault injected:** the `"yank"` arm of `composed` renamed. Fails on the yank.
**Superseded in part by improvise-dsxq** (entry at the end of this file). The
fixture's `1200` is the one kind of value on which the edit text and the value
agree, so this test could not see that the arm yanked the edit text re-parsed.
The arm now yanks `CellDisplay::value`, and the test sets that field on its
hand-built cell. The parity claim lives in the lockstep tests, not here.
### `a_second_key_that_matches_nothing_still_spends_the_prefix`
`App::dispatch_key` `take()`s the prefix before looking the second key up, so
the prefix is spent whether or not it matched. A client that returned
`Ignored` here would leave `transient_prefix` armed forever, and every
subsequent key would be resolved in a sub-keymap the user has forgotten about —
strictly worse than the black hole being fixed.
**Fault injected:** the unmatched arm returns `KeyOutcome::Ignored`. Only this
test fails, which is what says it is not a duplicate of the other four.
### `zz_declines_out_loud_rather_than_stranding_the_prefix`
The same rule where the second key *is* bound to something this client cannot
compose. Every outcome of a chord carries the disarm, so the second key can
never be `Upstream` — there is nowhere to put the disarm. `wq` is therefore
answered by `declined` (improvise-rmvx: silence was worse).
**Fault injected:** the `"wq"` arm of `declined` renamed. Fails because the
chord then composes only the disarm and says nothing.
### `every_name_bound_behind_a_prefix_is_answered_by_this_client`
The structural half, and the reason the `Upstream` arm inside `prefixed` is
unreachable rather than merely unused. A name reachable **only** behind a
prefix used to be unreachable full stop, so nothing checked it: the four names
in the prefix sub-keymaps are invisible to every other derivation here. The
walk enumerates them off the shared keymap table and requires each to be
answered by `requested`, `declined` or `composed`.
**Fault injected:** the `"toggle-col-group-under-cursor"` arm of `declined`
renamed — a name no other test names. This test is the **only** one that
fails, so it is discriminating rather than a restatement of the four above.
### `a_request_bound_behind_a_prefix_would_be_silently_discarded`
Found by ChatGPT Codex reviewing PR #161, and the finding was correct. The
first version of the structural guard counted `requested(name).is_some()` as
"answered" — but `chord` maps `Step::Request(_)` to `None`, so a future
prefix binding for `save` would have **passed the guard while the request was
discarded at runtime**. That is this repo's own worst shape: a green derivation
over a broken thing, which is exactly the defect improvise-lut7 filed.
The guard now **refuses** a request-backed prefix name rather than accepting
it, and this test is why the refusal is the right answer rather than an
omission: it drives `chord` directly with a `Binding::Cmd` naming `save` —
a binding the shared table does not contain, which is why `chord` is split
from `prefixed` at all — and shows the outcome is the disarm **alone**. The
request is gone. There is nowhere to put it: a chord must carry its disarm or
the prefix is stranded (see the sibling test above), and `KeyOutcome::Request`
carries no commands.
It first asserts `requested("save").is_some()`, because if the request class
were empty the guard's refusal clause could never fire and would assert
nothing.
**Two faults injected.** Binding `save` behind the `g` prefix in
`default_keymaps` — the scenario the review described — fails
`every_name_bound_behind_a_prefix_is_answered_by_this_client` naming `save`.
And making `chord` return `KeyOutcome::Request(r)` (the review's other
suggested option) fails **this** test, because the disarm is then lost: it is
the evidence that the alternative fix trades one stranding for another.
### `the_g_chord_returns_to_the_first_row_on_both_ends` (`lockstep.rs`)
`lockstep.rs` is the "host and client agree on a keypress" instrument and it
passed unchanged through all of this, because **no chord was ever driven
through it** — every key it pressed was a single key. That gap is why a dead
resolver arm could sit behind `g` without the agreement suite noticing. The
test presses `G`, `g`, `g` against a real `App` and a real `ClientState` and
compares `transient_prefix` *and* the cursor after **each** press: comparing
only the end state would pass against two ends that disagreed in the middle
and converged.
**Fault injected:** the same `Binding::Prefix` → `Upstream` reversion. Fails on
the second press.
### `a_read_only_session_refuses_an_edit_before_anything_is_painted`
improvise-j4qt. The finder measured this in Chromium and Firefox: two tabs, one
document. The second tab boots read-only. Typing `4242` into a cell applied it
to that tab's in-memory `App` and projected it, so the grid showed
`184,004,242.00`. Only then did the persist fail and `NOT SAVED` replace the
read-only reason. The first tab never saw the edit.
The test builds the read-only boot the way `bindings::read_only` does: the
document's bytes over a `vfs::ReadOnly` backend. It sends a caught-up `SetCell`
and asserts four things:
- the whole reply is `[Rejected(ReadOnly)]`;
- the client's folded grid still shows the old text;
- no notice was raised (no persist ran);
- the document is not dirty.
**Red first**: the same test over `Worker::new`, which was the boot's
constructor before this change, failed. Its reply was a `Commands` frame of
projections carrying the new value.
The control is `a_read_only_session_still_answers_a_view_command`: a
`SetSelected` frame is not refused. Without it, a worker that refused *every*
frame would pass the test above.
End to end: `harness/tests/shell.spec.js`, "a read-only tab refuses an edit and
never paints it". It opens two real tabs on one document in Chromium, edits in
the second, and asserts four things:
- the toast says `read-only`;
- the cell keeps its text;
- the grid contains no `4,242`;
- `#status` still says `read-only` and not `NOT SAVED`.
Fault injected: `bindings.rs` reverted to `Worker::new(app)` for the read-only
boot, then `harness/build.sh` rebuilt. The spec then failed on the toast, which
received `""` because no refusal arrived. Reverted and rebuilt, and the whole
of `shell.spec.js` passed (18).
### `an_open_empty_panel_is_not_a_closed_one`
improvise-df2w. `views` and `formulas` said "closed" by being empty, so an open
formula panel over a document with no formulas built a `RenderModel` identical
to a closed one, and the page hid it while the mode read FORMULAS. The test
compares the two models whole rather than asserting a field, so it does not
depend on how the difference is spelled.
**Reproduced:** fails at HEAD before the fix with `left == right` on the formula
half — the two models were byte-identical. The existing
`a_closed_panel_projects_no_rows_and_an_open_one_does` could not see it, because
it fills both caches before opening the panels.
### the formula panel opens over a document with no formulas
The browser half of improvise-df2w, in `harness/tests/contract.spec.js`, over
`harness/fixtures/tall.improv`, which has no `## Formulas` section. It asserts
**both** directions, because each rival fails a different one.
**Reproduced:** against a bundle built from the unfixed `render.rs` and
`app.js`, fails at `#formulas` `toBeVisible()`, immediately after `#mode` reads
FORMULAS — the exact disagreement the ticket describes.
**Fault injected:** `paintPanel` writing `host.hidden = false` (a page that
stops inferring closed and simply never hides). Fails at the launch-time
`toBeHidden()` before the panel is opened.
## improvise-ktlk — the overlay and the commit read one buffer
PR #162 (improvise-xg1z) changed what the overlay **draws** for an `Editing`
mode that arrived without its `edit` buffer, and not what Enter **commits**.
`render.rs` painted `edit_seed(state)`, while `resolve.rs::commit` and
`append-char` read `read_buffer(&state.view, "edit")`, which is `""` for an
absent key. So the overlay showed the value, Enter composed `ClearCell`, and one
typed character replaced the shown text rather than extending it. #162's tests
only looked at the drawing, and none of them pressed a key.
The fix gives the fact one home. The fold seeds the missing buffer
(`ClientState::settle_editor`, called after every fold, bootstrap and refusal,
beside `settle_mode`), and `render.rs` reads the buffer through `read_buffer`,
as `commit` does. `edit_seed` is `pub(crate)` again and no renderer calls it.
Seeding happens only when there is a cell under the cursor that has
`edit_text`. With no such cell the buffer stays absent, which reads as `""`
on both paths, and `commit` declines anyway for want of a cell.
### `an_editor_entered_without_its_buffer_commits_and_extends_the_cell`
The fixture's display text (`1,200.50`) and edit text (`1200.5`) **differ**.
With `CellDisplay::number("1200")`, as #162 used, a fallback that seeds from the
formatted `text` passes too (gotcha 8). The test folds `ChangeMode(editing)`
with no `SetBuffer`, then presses the keys: Enter must compose
`SetCell(Number(1200.5))`, and after a typed `5`, Enter must compose
`SetCell(Number(1200.55))`.
**Faults injected, each run:**
- The tree before this fix (render-only fallback). The buffer assertion fails
with `""`. With that assertion removed, the Enter assertion fails on its own
with `None`, because the first command is `ClearCell`.
- `edit_seed` returning the drawn `text` instead of `edit_text`. Fails with
`"1,200.50"`.
- The over-broad seed, "absent **or empty**, refill". This test passes, and
`an_edit_buffer_the_user_has_emptied_is_not_refilled_from_the_cell` fails,
as it did against the same rival in `render.rs`.
## improvise-dsxq — the browser's `yy` copies the value, not a string
PR #161 (improvise-lut7) made the browser's `yank` arm compose
`SetYanked(typed(edit_text))`. The TUI's `YankCell` yanks
`evaluate_aggregated(key)`, the value itself. Those differ in three ways, and
#161's only fixture, `1200`, hits none of them:
- `edit_text` is `CellValue::to_string()`, which rounds a non-whole number to
four places.
- `typed()` turns number-like text into a number.
- In a pivot grid `edit_text` is the stored cell only, so an aggregate or a
formula cell has none and the browser yanked `None`.
The fix projects the evaluated value as `CellDisplay::value`, and the arm yanks
that (`context/code/protocol.md` §3). Reproduced at `40b8913` by both tests
below before the fix: the browser yanked `Number(1.2346)` for `1.23456789`, and
`None` for the aggregate.
### `yy_then_p_copies_the_value_the_host_copies`
Three cases, one per disagreement, each on a real `App` and a `ClientState`
seeded by `grid_of`. The host presses `y y p` through `reduce_key`; a twin
host applies exactly what the client composed. Each case asserts the host
yanked the expected value (so a fixture that yanks nothing cannot pass), that
the client yanked the same, that the host's paste landed, and that the two
documents match. Sources sit at row 2 or 3 and targets at row 1, never row 0,
and the first column is an unused `Units` measure, so no cursor is at index 0.
- `1.23456789`, a non-whole number.
- `Text("007")`, number-like text.
- `Profit = Revenue - Cost` over `10.123456` and `0.5`, a formula's value,
pasted into a stored `Revenue` cell.
**Faults injected, each run:**
- The pre-fix arm, `typed(edit_text)`. Fails on the first case with
`Number(1.2346)`.
- The arm yanking `typed(&value.to_string())`, the projected value re-rendered.
Fails on the first case with `Number(1.2346)`.
- The producer coercing number-like `Text` through `typed()`. The first case
passes, and the second fails with `Number(7.0)`.
- The producer projecting the raw `get_cell` instead of the evaluated value.
The first two cases pass, and the third fails with `None`.
### `yy_over_an_aggregate_copies_the_sum_the_host_copies`
`Product` on the None axis with two stored records, `1.23456789` and `1.0`, so
the cell at row 1 is an `Aggregate` with no stored value. Yank only, since an
all-aggregate grid has no record cell to paste into. The host must yank
`Number(2.23456789)`, which rules out a fixture that is not an aggregate.
**Faults injected, each run:** the pre-fix arm (fails with `None`), the
re-rendered value (fails with `Number(2.2346)`), and the raw `get_cell`
producer (fails with `None`).
Records mode is deliberately not covered. There the TUI yanks `None` and its
`p` writes a junk `_Dim`/`_Index` cell, because it resolves the synthetic key
(improvise-6p8q). The browser is right and the TUI is wrong, so a lockstep
case there would assert the bug. Since improvise-6p8q's fix it is covered, by
`yy_then_p_in_records_mode_writes_the_record_the_host_writes` below.
### `an_untouched_browser_edit_stores_the_identical_value`
**improvise-umi4**, the browser half. The client seeds `i` from the projected
`CellDisplay::edit_text` (`resolve.rs::edit_or_drill`), which was
`App::display_value_in` and so `CellValue`'s rounded `Display`. The client
resolves `i` and `Enter` itself, and what it composed is applied to the host
through `apply_on` (`Checked::against` plus `apply_command`). The stored value
is then compared by `f64::to_bits`. The fixture puts the value at row 1,
column 1 of a 2×2 pivot grid, with values that `Display` rounds (the list in
`context/test-evidence/command.md`). A records grid is not driven, because the
browser declines records editing (`edit_or_drill`).
Failed before the fix on `Number(1.23456789)` stored as `Number(1.2346)`.
**Rival:** `CellValue::edit_text`'s number arm set back to `Display`. The test
failed on the same case.
The test is in-process, so it cannot see the JSON wire. A float that does not
survive `serde_json` is improvise-6oy4, a separate fix.
### `every_projected_edit_text_commits_back_to_its_value`
The projection contract behind the test above, for **both** grid kinds. Every
`Record` cell's `edit_text`, fed through `typed()`, must give back the cell's
`value`. It runs over the whole projected grid in pivot mode and in records
mode (entered with `R`). A guard asserts that the precise value was projected
at all. This test exists because `project::cell_display`'s records arm spelled
`edit_text` itself, apart from `display_value_in`, and no key-path test reaches
that arm from a browser. **Rival:** that arm restored to `Some(text.clone())`,
the display text. It failed with `records=true: edit text "1.2346" does not
commit back to Some(Number(1.23456789))`.
### an_edit_writes_the_cell_the_user_clicked_after_the_rows_renumber
improvise-2wu3. `web/app.js` freezes the screen while an inline editor is open
(`repaint` returns early), but `on_frame` goes on folding. The editor kept its
viewport `(row, col)`, and `typed_cell` resolved it against the *current*
cache at commit.
The test follows the ticket's acceptance criteria against a real `Host`:
1. Client A reads the North/Revenue record at `(0, 0)`.
2. Client B's `HideItem("North")` is applied.
3. Only the frames addressed to A are folded into A, with no repaint, just as
the page does it.
4. A precondition asserts the rows really renumbered.
5. The write A composes must name the record A clicked.
**Red first**, at HEAD after improvise-ktlk (PR #169), which moved the editor's
seed but not its commit. Written against the position API as it stood
(`typed_cell(0, 0, "42")`), it failed. The write named `RecordHash(161215…)`,
which is South's record now sitting at row 0, where North's `RecordHash(246296…)`
was expected. So the ticket still reproduced. The frame would have passed the
basis check, because A had folded the fresh `Projection::Basis` along with the
rows.
**The fix makes the fault unwritable in the page.** The page's surface no
longer takes a position at commit. `edit_target(row, col)` is read when the
editor opens and returns the `RecordCell` as an opaque string, and
`typed_cell(target, text)` composes against it. The same test, rewritten for
that surface, passes. There is no position left at commit for a fault to
re-introduce. Rust-side `typed_cell_frame(row, col, …)` remains for tests
that compose at read time.
The page path is exercised end to end by `harness/tests/shell.spec.js`'s
"committing an edit sends one frame and shows the host's value", which goes
through `edit_target` and the new `typed_cell`. The whole harness passes
locally (69 passed, 4 skipped).
### yy_then_p_in_records_mode_writes_the_record_the_host_writes
improvise-6p8q in lockstep. The yank test in this file deliberately stayed on a
pivot grid because a records case would have asserted the TUI bug. Two records
(North, South), the yank from North and the paste onto South on row 1, the
`Revenue` column being neither the first column nor the last. Rivals, each run:
restoring the synthetic-key paste fails `both_ends`' document comparison on
`p` (the twin applied the client's `SetCell` to South's record, the host wrote
a `_Dim`/`_Index` cell); restoring the synthetic-key yank fails the yank
assertion. The tail yanks off the `Region` and `+` columns and requires the two
registers to agree; a TUI-only blank filter (`Text("")` to `None`) fails it on
`+`, which is why that filter was removed rather than kept. The evidence entries
for the host-side tests are in `command.md`.
### the worker is not called up until its first frame has arrived
improvise-i0v9. The worker transport wrote `in-page worker · <doc>` when it
spawned the worker, before anything had answered. A worker that then failed
therefore showed a success-looking status over a blank page. The spec records
every `#status` with `logStatus` and requires the whole log to be exactly
`["starting the worker · <doc>…", "in-page worker · <doc>"]`. It pins the order
as well as the final text, since a final read cannot see a claim made too early.
Measured failing against `gh/main` at `da2e5ab`. The rival, restoring the
immediate `setShell(\`in-page worker · ${DOC}\`)` in `workerTransport`, fails it
on the log's first entry. The final read is `toHaveText`, not `toContainText`.
The older specs' `toContainText("in-page worker")` could not tell the starting
text from the claim if the one contained the other, and the starting text
deliberately does not.
### only the first frame claims the peer answered, and not before it
improvise-i0v9, the unit half of the spec above. `arm(message, { answer })`
must write nothing until `sawFrame`, then write `answer` once, however many
frames follow. Fails against `gh/main`'s `createShell`, which has no `answer`
and so never writes the claim at all.
### a reason given before the first frame is not replaced by the frame's claim
The order `worker.js` really produces on a locked document: the `ReadOnly`
notice is posted after `start()` and **before** the Snapshot. So `fail` then
`sawFrame` must leave the notice's text as the only thing ever shown. The rival
is the obvious fix, `sawFrame` writing `answer` unconditionally. It fails both
this test and the one above: the first on its exact log, which is then written
twice. This test passes against `gh/main` by construction, since no claim exists
there. It is not evidence of the bug; it guards against the naive fix.
## Numbers across the serialized wire (improvise-6oy4)
No manifest enabled serde_json's `float_roundtrip`, so its default f64 parser
could read a number back one ULP away from the one written. Found by the
adversarial review of PR #178. That review measured 139,787 of 1M ordinary
decimals and 296,493 of 1M random-bit f64s coming back wrong. Every test that
passes `Command`s or caches **in-process**, which includes all of
`tests/lockstep.rs`, never serializes, so none of them could see it.
`LOSSY = 0.24815714285714285` is a value the default parser reads back as
`0.24815714285714283`. Both tests compare `f64::to_bits`, never `==`.
### `a_yanked_number_crosses_the_serialized_wire_exactly`
South/Revenue holds `LOSSY` at row 1, so the source is off index 0. The page
presses `j y y l p` through `press`, which serializes each frame to a real
`Host::handle` and folds the serialized replies. The value crosses the wire
twice: down as `CellDisplay::value` in the projection (#178), then up as the
`SetCell` the paste composes. Each hop has its own assertion, so a fix to one
hop cannot hide the other. `set_viewport` is called first. Without it the
client records no window and the cursor keys do nothing, which an early
version of this test found out.
**Fault injected:** no `float_roundtrip` in any manifest, the pre-fix tree at
`a6e3651`. Fails at the host-to-client assertion: yanked bits
`4598108823619341467` against `…468`.
### `a_typed_number_crosses_the_serialized_wire_exactly`
`typed_cell(1, 1, "0.24815714285714285")`. The client parses exactly with
`str::parse`, and the host re-parsed the JSON lossily. This path lost precision
before #178 too, as the review noted.
**Fault injected:** the same pre-fix tree. Fails: the host stored
`4598108823619341467` in South/Cost.
**Retired: `every_build_that_parses_a_frame_enables_float_roundtrip`.** It asked
`cargo tree` about three crates listed by hand. improvise-dz8c replaced it with
`every_crate_that_can_parse_json_enables_float_roundtrip`, whose entry is in
`tooling.md`. The injections recorded for it still hold: with the feature
removed from `improvise-wasm-client` alone, the two tests above stay green, so
they cannot guard the bundle alone.
### resolve::tests::every_grid_move_lands_where_its_binding_says
improvise-6562, found reviewing PR #180 at 949810c, and it is the same
laundering as the disarm, one level over. `heard()` said it judged by what moved
and in fact judged by command **type**: any `Local` command besides a status or a
disarm counted as `Acts`. The fixture sat at every edge — a 2x1 grid with the
cursor at (0,0), one tile, help page 0 — so about twenty movement keys that
clamp to nothing were counted as acting. Rebinding `jump-last-col` to
`GridMove::ToStart(false)`, so `$` and `End` go to the *first* column, left the
whole workspace green.
Two changes, because either alone is not enough. `heard()` now **folds** the
outcome and compares the view slice before and after (status and disarm aside),
counting a `Command::Model` as sent and a `Request` as an act; and `furnished_on`
stands on a 3x3 grid with the cursor at (1,1), three tiles with the cursor on
the middle one, a page axis on its middle item and help page 1, so no move is
clamped by an edge. Injecting `jump-last-col` as a `SetSelected` to where the
cursor already is now fails `every_advertised_command_answers_in_its_own_mode`
naming `NORMAL: $/End jump-last-col Silent`.
**The reviewer's own rival still passes that guard, and should.** From (1,1),
`ToStart(false)` moves to (1,0) — the view moved, so the key "acts". No
did-anything-happen check can see *direction*; only an expected destination can.
This test supplies one without a second table: a `move-selection` binding's
expected landing is read off its own args (the deltas), and the four jumps are
named by what they are called. Every key that runs one is pressed from the
centre — fourteen of them, with a floor so a blind walk fails. Under the rival
it fails naming `End` and `$`. The general point: the fixture that made the
laundering invisible (the edge) is also the one that makes a wrong-direction
jump visible, so moving off the edge fixed one blind spot and opened the other,
and each needs its own question.
**improvise-w7qd, and the floor was the hole.** As first written the test
covered `move-selection` and a four-entry jump table, behind `pressed >= 14`.
Two things passed the whole workspace, both found in re-review of #180 at
544744c: negating the `scroll-rows` and `page-scroll` arms, so `Ctrl-d`/`Ctrl-u`
and `PgDn`/`PgUp` all scroll the wrong way (advertised, and still "acting" from
the centre); and dropping the jump table *with* `$` sent to the first column,
because the sixteen `move-selection` sequences met the floor on their own. The
test now walks **every** command Normal binds. `scroll-rows` and `page-scroll`
state their landing from their own argument exactly as `move-selection` does —
from the centre of a 3x3 grid any step of at least one lands at `1 + d`
clamped, so a sign is visible whatever the page size — and a command that moves
the cursor (in the same view) without a stated landing **fails** rather than
being skipped. So a dropped table entry is caught by derivation, not by a count:
under the second rival it fails naming every jump as moving with nothing stating
where. The first rival fails on `PgUp` (`page-scroll ["-1"]`) and, alone, on
`Ctrl-u` (`scroll-rows ["-5"]`). A last assertion that each kind was pressed at
least once guards the other direction — a binding vanishing — and is a named
list on purpose, since that is the one thing no walk over bindings can see.
**improvise-5kdr: the 3x3 fixture saw only a sign.** From the centre of a 3x3
grid every step of one or more clamps to an edge, so the w7qd version stated
`page-scroll` as `1 + d` — one row, which is not what it does — and passed
because the true landing clamped to the same edge. Measured on #198 at 8772db2:
`page-scroll` ignoring the page size together with `scroll-rows` moving one row,
or `move-selection` moving `2d`, left the test green. The walk now stands at
(10,10) of a 21x21 grid with an 8x8 window, far from every edge, so a landing
shows magnitude: `10+d` for `move-selection` and `scroll-rows`, `10 + page·d`
for `page-scroll`, 0/20 for the jumps. The page size is not written into the
test: `grid_move`'s page rule moved into `page_rows(window_rows)` and the test
asks it, so the rule has one home. Rivals, each run: `move-selection` doubled
fails on `Up` (`move-selection ["-1","0"]`); the page-size-and-one-row pair
fails on `PgUp` (`page-scroll ["-1"]`); the one-row `scroll-rows` alone fails on
`Ctrl-u` (`scroll-rows ["-5"]`); the jump-table rival still fails naming every
jump. The residual, stated: because the test reads `page_rows`, a change to the
page rule itself moves the test with it — whether that rule is the TUI's is
`tests/lockstep.rs`'s question, not this one's.
### an_edit_is_refused_when_the_cell_was_written_while_the_editor_was_open
improvise-0j0f, found by the review of PR #188 at 48f52e7 and confirmed
against a real `Host`. #188's first version captured the edited record's
*identity* when the editor opened, but claimed its *basis* at commit. Because
the fold runs on while the screen is frozen, the claim was the fresh counter.
The test runs the steps in this order:
1. A opens an editor on North/Revenue, which shows 1200.
2. B writes 999 to the same cell.
3. A folds only its own frames and does not repaint.
4. A commits "42".
It asserts that the host answers `Rejected(RecordMoved)` and still holds 999.
**Red first:** against the claim-at-commit version it failed, because the
commit was accepted. **Rival injected after the fix:** `typed_target_frame`
back to `basis.claim([record])`, which claims at commit. Only this test failed
(`12 passed; 1 failed`), and the 2wu3 test beside it stayed green. That is
right, because the two are separate properties: *which* record the commit
names, and *which version* of it the claim states. Reverted.
An earlier version of this entry said the reviewer's clear variant would be
`RecordGone`. It is not, and that claim was reasoning and not a test. The
clear variant is now tested by
`an_edit_is_refused_when_the_cell_was_cleared_while_the_editor_was_open`,
below, and the answer is `RecordMoved` (improvise-4img).
### an_edit_is_refused_when_the_cell_was_cleared_while_the_editor_was_open
improvise-4img: the clear variant of the 0j0f race above. The steps are the
same, except that in step 2 B *clears* North/Revenue instead of writing 999.
The test asserts that A's commit of "42" is refused with
`Rejected(RecordMoved)`, and that the cell is still absent afterwards.
**Why `RecordMoved` and not `RecordGone`.** After the clear, North has no cell
left, so `DataStore::resolve_record` answers `None`. But `App::resolve_record`
falls back to the layout, and the grid still draws the North row because
`prune_empty` is off. So the record still resolves, and the counter A claimed
at open has moved. `RecordGone` is for a record that nothing resolves at all;
this one is still on screen.
**Rival injected:** `typed_target_frame` back to `basis.claim([record])`, which
claims at commit. The commit was then *accepted*. With the refusal assertion
disabled, the host held `Some(Number(42.0))`, so A's commit had re-created the
cell B cleared. Under that rival, the 0j0f test above failed as well. Both
files were restored.
### a_locally_resolved_page_scroll_lands_where_the_host_would_put_it
**What was missing (improvise-5xbq).** `tests/lockstep.rs` compared only `j`
and `G` against the host. The browser's page rule (`resolve::page_rows`) and
the TUI's (`Move`'s `Page` arm, `visible_rows * 3 / 4`) were two copies, and
two faults injected into the browser's copy left the whole workspace green:
`page_rows` returning `window_rows / 2`, and `grid_move` paging by
`window_cols`. The second was invisible for a further reason: the only window
the page-rule test used was square, `(8, 8)`.
**The fixture.** 20 rows by 3 columns; the host sees 8 rows and the client
announces `(8, 3)`. The three candidate rules then give three different
steps: 6 (three quarters of the rows), 4 (half the rows) and 2 (three quarters
of the columns). The walk starts off the edge (two `j`), then presses `PgDn`,
`PgUp`, `Ctrl-d` and `Ctrl-u`, including a `PgDn` that clamps at row 19.
**Why it pins the rows as well as comparing the ends.** `page_rows` is now the
one home (the TUI's `Move` calls it), so a wrong rule moves host and client
together, and a comparison alone passes. Each press therefore also asserts the
row the TUI's rule names.
**Injected, before and after the move to one home.**
| Fault | Two copies | One home |
|---|---|---|
| `page_rows` returns `window_rows / 2` | fails at press 2 (`PgDn`), "client and host disagree" | fails at press 2, "not the row the TUI's page rule names"; `app.rs`'s `page_down_scrolls_by_three_quarters_visible` and `page_up_scrolls_backward` fail too |
| `grid_move` pages by `window_cols` | fails at press 2, "client and host disagree" | fails at press 2, "client and host disagree" |
`Ctrl-d`/`Ctrl-u` are `scroll-rows ±5`, not the page rule. They are in the walk
because the ticket named them, and they move the start point of the next page
turn to a row the fixed rules alone would not reach.
### a narrow window still shows how to page through help — `harness/tests/help.spec.js`
**What broke (improvise-1c5m).** Below the width at which the app bar wraps,
`#help` (fixed, `inset: 2rem`, `z-index: 30`) covers `#hint`, and since PR #180
Welcome carries no paging instruction of its own. So a new user on Welcome at
phone width could not see how to reach page two.
**The fix is option (b) from the ticket.** The overlay draws `model.hint` in a
footer. In Help mode that is already `hint_line(Help)`, the same string `#hint`
carries, so it adds no derivation, and it sits outside `.help-body`, so
"Welcome is prose" still holds. The footer is `position: sticky` because the
overlay itself scrolls and Welcome is longer than a phone-height overlay.
**What the case asserts.** At 600x800 on Welcome, it reads the hint string from
`#hint`, so the spec spells no keys. It then requires some element showing
exactly that string to be the topmost element at its own centre, which is the
`elementFromPoint` measurement the ticket made by hand.
**Red, then a rival.** At base it failed with "the paging hint is under the
help overlay". With the fix and `position: sticky` changed to `static`, it
fails the same way: the footer exists but sits below the fold of a scrolling
overlay, so presence alone cannot satisfy the case.