e09ec75226
The harness fixtures open their groups explicitly now that groups start closed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
1270 lines
66 KiB
Markdown
1270 lines
66 KiB
Markdown
# Test evidence — `improvise-protocol`
|
|
|
|
What each test's bug was, what fault it can see, and why it can fail. Keyed by
|
|
exact test name (design-principles §7). A renamed test orphans its entry;
|
|
`python3 scripts/check-comment-runs.py --check-evidence` catches that.
|
|
|
|
Tests whose name already states the whole property are not listed here. An
|
|
entry earns its place by carrying something the assertion cannot: the bug, the
|
|
injected fault, or the reason a *negative* arm exists.
|
|
|
|
---
|
|
|
|
## `src/render.rs` — the fold
|
|
|
|
### cell_updates_outside_the_cache_are_dropped
|
|
|
|
A projection naming a cell outside the current viewport must be **dropped, not
|
|
panic**: frames and cell updates race on the wire, and a client that crashes
|
|
on a stale index is a client that crashes. This is the totality half of the
|
|
`WidgetCache` contract, stated as an example; the ordering half is
|
|
`a_cell_update_that_overtakes_its_labels_is_lost`.
|
|
|
|
### a_cell_update_that_overtakes_its_labels_is_lost
|
|
|
|
The fold is **not commutative**, and a client must not be written as though it
|
|
were. The frames frame the cells, so a `Cells` update that overtakes the
|
|
`ColFrame`/`RowFrame` naming its positions is dropped. Reordering the
|
|
identical three updates gives a different cache, and the `assert_ne!` is the
|
|
load-bearing line — without it, a fold where order did not matter would pass.
|
|
|
|
### a_compacted_seed_folds_to_the_same_cache_as_the_whole_history
|
|
|
|
Bootstrap is not a second code path, but a server does not replay history to a
|
|
late client either — it sends the state it holds *now*. So the property is
|
|
**compaction**: folding the seed lands exactly where folding the whole history
|
|
landed, including the history's relabels and overwrites.
|
|
|
|
The history is built to be worth compacting — overwritten, relabelled,
|
|
populated and grown, with the extent moving twice so only the last survives —
|
|
and `seed.len() < history.len()` is asserted, because a "seed" that is not
|
|
smaller than the history is not a compaction and the equality would be vacuous.
|
|
|
|
Note the general rule this is an instance of: asserting
|
|
`fold(prefix) + fold(rest) == fold(whole)` is **vacuous**, being the
|
|
definition of a left fold (`doctrine(fold_tests_must_test_order_not_the_axiom)`).
|
|
|
|
### a_snapshot_seeds_every_widget_from_its_compacted_state
|
|
|
|
The same property across the envelope, with two subscribers each folding only
|
|
what is addressed to it — the dispatch a client does for real.
|
|
|
|
### a_totals_update_that_overtakes_its_columns_is_cut_to_fit
|
|
|
|
The totals strip is framed by the columns on the same terms the cells are: a
|
|
strip that overtakes the `ColFrame` naming its positions is cut down, because
|
|
a renderer zips it with the columns and a length mismatch indexes off the end.
|
|
|
|
### an_empty_totals_strip_is_not_padded_into_blanks
|
|
|
|
An **empty** strip says "this grid has no totals" — records mode, or an empty
|
|
grid — and must not be padded into a row of blanks by the re-fit. Padding it
|
|
would draw an empty total row under a records-mode grid that has none, which
|
|
is the one thing the emptiness is for. This is the asymmetry that separates
|
|
`totals` from `cells` and `col_widths`, which are always padded.
|
|
|
|
### a_recategorized_axis_blanks_the_totals_too
|
|
|
|
The totals are **contents, not frame**: a reframing blanks them for the same
|
|
reason it blanks the cells. A stale sum under fresh cells is the one number on
|
|
the screen a user would take at face value. The length assertion is the other
|
|
half — the frame survives, the numbers do not.
|
|
|
|
Renamed by improvise-n89z, which deleted `GridUpdate::Invalidated`. The test
|
|
used to reach the blanking through that arm; it now reaches it through a
|
|
`cats` change, which is the thing that really produces it. Proved
|
|
discriminating by pinning `recategorized` to `false`.
|
|
|
|
### a_relabelled_row_is_blanked_and_the_frame_survives
|
|
|
|
The other half of the same deletion. `invalidation_blanks_contents_but_keeps_
|
|
the_frame` asserted what the retired arm did; this asserts what
|
|
`invalidate_relabelled` does, and it is **strictly stronger**, because the old
|
|
arm blanked everything and so could not tell "blanks what it reframes" from
|
|
"blanks the lot". Row 1's label does not change and row 1 keeps its value.
|
|
Proved discriminating by weakening the `stale` filter from `||` to `&&`.
|
|
|
|
### the_page_bar_outlives_a_reshape
|
|
|
|
`page_coords` is framed by nothing: it names categories, not positions. So it
|
|
survives a reshape that drops every row and column. Without this, "re-fit
|
|
everything on a frame update" would look correct.
|
|
|
|
### the_first_rendered_entry_always_shows_its_level
|
|
|
|
**Bug (improvise-jjx)**, moved here with the rule (improvise-anr9): header
|
|
suppression compared against the entry at `idx - 1` even when that entry was
|
|
scrolled off-screen, so scrolling **blanked the group label at the top of the
|
|
viewport**. The first *rendered* entry always shows its full tuple.
|
|
|
|
The fixture is two-level so the level-0 prefix of index 1 genuinely matches
|
|
index 0's — a single-level frame could not express the case.
|
|
|
|
### a_repeated_prefix_below_the_first_rendered_entry_is_suppressed
|
|
|
|
The negative arm of the above, and it has to stay: without it, "always show
|
|
every level" would pass `the_first_rendered_entry_always_shows_its_level` and
|
|
the multi-level header would repeat its outer label on every row.
|
|
|
|
### a_data_index_skips_group_headers_and_still_names_its_group
|
|
|
|
Group headers do not consume a data index, and a data index resolves to the
|
|
group whose header precedes it — which is what makes a column group span
|
|
drawable by a client that holds no `GridLayout`. The collapse state travelling
|
|
with the group is the third assertion, and it is why `FrameGroup::collapsed`
|
|
is on the wire rather than re-derived.
|
|
|
|
### shared_ancestry_widens_as_the_level_rises
|
|
|
|
The underline rule: at the leaf level only the selected column, at a higher
|
|
level every column under the same ancestor. **Both levels are checked**
|
|
because a version comparing whole tuples would pass the leaf arm and light
|
|
only one column at the outer level, and a version comparing only the outermost
|
|
label would pass the outer arm and light a sibling at the leaf.
|
|
|
|
### body_from_starts_at_the_item_not_at_the_header_above_it
|
|
|
|
A window opening mid-group must be able to *name* its group without painting
|
|
that group's header as a row inside the window. The second assertion —
|
|
`body_from(0)` is everything, headers included — is what stops the fix being
|
|
"drop all group headers".
|
|
|
|
### header_widths_are_per_level_and_measured_in_display_cells
|
|
|
|
The one width rule (improvise-1125): per level, the widest label plus a gap,
|
|
clamped. Measured in **display cells**, so `日本` counts four and not two — a
|
|
`chars().count()` would make level 1 two cells wide, clamp it to
|
|
`MIN_ROW_HEADER_W`, and draw the header short. Both clamps are exercised
|
|
(`"ab"` is under the floor, `"x" * 100` over the ceiling), because a rule
|
|
whose clamps are never reached is a rule nobody has tested.
|
|
|
|
### a_column_without_its_width_yet_is_measured_in_display_cells
|
|
|
|
**Raised by Copilot on PR #23.** The fallback width a column is drawn at
|
|
between its `ColFrame` and its `ColWidths` must be in the same unit the
|
|
authoritative widths are in. `chars().count()` would draw a wide label half as
|
|
wide as it needs. The fixture is deliberately a two-scalar / four-cell string,
|
|
which is the smallest input that separates the two counts.
|
|
|
|
### an_axis_with_no_categories_still_has_one_level
|
|
|
|
An axis carrying no category still draws one header line, and its item tuples
|
|
are empty — reading a level off one must answer `""` rather than panic. This
|
|
is the shape a view with nothing on Row produces, so it is a live layout and
|
|
not a degenerate fixture.
|
|
|
|
### a_relabelled_column_does_not_keep_the_old_columns_value
|
|
|
|
A column dropped from the **middle** used to slide the cells to its right one
|
|
place left, because `fit_to_labels` truncates by position: `[A,B,C]` shrinking
|
|
to `[A,C]` left the column now labelled `C` showing B's value. Nothing on the
|
|
wire ever forced a producer to declare it first, so `apply` blanks what it
|
|
reframes.
|
|
|
|
### a_label_change_blanks_only_the_positions_it_reframes
|
|
|
|
The same rule on the row axis, and the case a bare label shrink gets right by
|
|
accident: only the *relabelled* positions are blanked, so an unchanged row
|
|
keeps what the server already sent for it.
|
|
|
|
### renaming_a_row_category_invalidates_every_cell_under_it
|
|
|
|
Renaming the **category** re-identifies every cell on the axis, even where
|
|
every label is byte-identical: a coordinate is `cats` zipped with the item
|
|
tuple, so `Region = [North]` and `Department = [North]` are different cells
|
|
wearing the same name.
|
|
|
|
**Frame-only, and that word is the test.** No `Cells` and no `Totals` follow
|
|
the frame update, because the producer's follow-up is exactly what would hide
|
|
the bug — `apply` has to be right with nothing behind it, the same shape as
|
|
`a_cell_update_that_overtakes_its_labels_is_lost`.
|
|
|
|
The `id` assertion is the sharp half and is separate from the text on purpose.
|
|
A stale `CellId::Record` under a re-identified coordinate is an editable cell
|
|
whose write addresses the record the **old** coordinate named — silent,
|
|
invisible in any rendering, and the same family of defect as gotcha 18's drill
|
|
snapshot.
|
|
|
|
The precondition assertion (the *items* are identical) is load-bearing:
|
|
without it the test is just the ordinary relabelling case, and `cats` proves
|
|
nothing.
|
|
|
|
### renaming_a_column_category_invalidates_every_cell_under_it
|
|
|
|
The column half of the same rule. Here the per-position arm already blanks
|
|
each total as its column goes stale, so this pins the cells and the ids; the
|
|
row arm above is the one that needed the totals handled separately.
|
|
|
|
### an_unchanged_category_leaves_the_cells_that_still_fit
|
|
|
|
**The negative arm, and it is load-bearing**: without it, "blank everything on
|
|
every frame update" would pass both renaming tests above, and the fold would
|
|
resend the whole grid after a group collapse.
|
|
|
|
### a_relabelled_column_drops_its_total
|
|
|
|
**improvise-hkt6.** `invalidate_relabelled` blanked the cells under a renamed
|
|
column and left its **total** standing, so a client that heard `ColFrame`
|
|
without a `Totals` behind it drew the old column's sum under the new column's
|
|
blanks.
|
|
|
|
Unreachable from `grid_updates`, which always sends the changed totals in the
|
|
same batch — **which is the point**: the rule is that `apply` does not trust a
|
|
producer to remember, and the only way to test it is to send the update the
|
|
producer never sends alone.
|
|
|
|
Three columns and a rename in the *middle*, so "blanked the right one" is
|
|
separable from "blanked all of them".
|
|
|
|
### a_relabelled_row_leaves_the_totals_alone
|
|
|
|
The row half is the **negative** arm and has to stay: a total spans every row,
|
|
so no single row frames it. Without this, "blank the totals on any relabel"
|
|
would pass `a_relabelled_column_drops_its_total`.
|
|
|
|
### col_widths_always_span_the_column_labels
|
|
|
|
`col_widths` is framed by the columns: a renderer zips the two, so a cache
|
|
where they disagree in length is one that indexes off the end. `ColWidths` is
|
|
its own update and may be a frame behind the frames.
|
|
|
|
### a_col_widths_update_is_fitted_to_the_labels_it_is_framed_by
|
|
|
|
The same invariant seen from the other arm. `ColWidths` used to be assigned
|
|
verbatim, so a widths update that disagreed with the frame put the cache
|
|
straight back into the state `col_widths_always_span_the_column_labels` exists
|
|
to forbid — the frame arm was fitted and the widths arm was not.
|
|
|
|
A **short** update is the ordinary case: the server resized a column the
|
|
client has not been told about yet. A **long** one means the widths overtook
|
|
the `ColFrame` that frames them, and the fold drops what overtakes its frame —
|
|
the same rule `Cells` is already held to. Both directions are asserted,
|
|
because fitting only one of them is exactly the bug.
|
|
|
|
### item_updates_touch_only_the_named_category
|
|
|
|
Plus the dropped-update arm at the end: an `Items` update for a category the
|
|
cache has never heard of is dropped, not inserted — the totality rule again,
|
|
on a different widget.
|
|
|
|
### an_axes_update_moves_several_categories_at_once
|
|
|
|
A transpose moves two axes at once, which is why `Axes` takes a list rather
|
|
than a single pair. The third assertion — membership is undisturbed — is what
|
|
stops the fix being "resend the whole category list".
|
|
|
|
### every_projection_round_trips_through_json
|
|
|
|
One **populated** value per `CellDisplay` shape: `numeric` decides the
|
|
alignment and `error` the styling, so a round trip that only ever carries them
|
|
at their default proves nothing about either
|
|
(`doctrine(populate_before_roundtrip)`).
|
|
|
|
### a_populated_snapshot_round_trips_through_json
|
|
|
|
The `Snapshot` is the only thing on the wire that is not an event, and the
|
|
only point at which `ViewState` is serialized (improvise-q08) — so it is the
|
|
one message whose round trip nothing else covers. Populated on **both**
|
|
halves: a default `ViewState` beside an empty seed would round-trip through
|
|
almost no code.
|
|
|
|
---
|
|
|
|
## `src/basis.rs`
|
|
|
|
### a_client_that_has_folded_nothing_cannot_claim_a_basis
|
|
|
|
The floor. A client that has been told nothing cannot compose a basis, so it
|
|
cannot compose a write. `Basis` has no `Default` and no public constructor, so
|
|
`BasisCache::claim` is the *only* door — which is what this asserts is shut
|
|
before the server has published anything.
|
|
|
|
### an_unknown_record_is_omitted_rather_than_guessed
|
|
|
|
A record the client has never been told about is **left out**, not invented.
|
|
Defaulting it to zero would be a claim the client cannot support, and the
|
|
server would read it as "untouched" — the exact forged-basis case
|
|
`Rejection::RecordUnstated` exists to catch.
|
|
|
|
### a_claim_carries_the_counter_for_each_record_it_names
|
|
|
|
The second assertion is the one that earns its place: a claim naming one
|
|
record must not carry another's counter. A `claim` that returned the whole
|
|
cache would pass the first assertion and fail this.
|
|
|
|
### the_published_record_list_is_stable_under_traversal_order
|
|
|
|
The published list is deduplicated and ordered, so a client is not sent a
|
|
fresh basis every time a row moves on screen. Two maps built in opposite
|
|
insertion order must compare equal — which is the property the `BTreeMap`
|
|
parameter buys and a `Vec` would not.
|
|
|
|
---
|
|
|
|
## `src/frame.rs`
|
|
|
|
### the_retired_spellings_do_not_parse
|
|
|
|
The names the **deleted** envelopes used, which must not resurrect as serde
|
|
aliases: an envelope accepting two spellings of one variant is two homes
|
|
again, wearing one type.
|
|
|
|
Not a hypothetical rename — `Bootstrap` and `Command` are exactly what the
|
|
worker pair (`improvise-wasm-client::wire`) spelled these two payloads before
|
|
improvise-xks6. `ClientFrame::Command` joined the list with improvise-bpq2,
|
|
and it is the sharper of the two: the singleton spelling was this crate's own
|
|
until that commit, so an alias would be a *revival* rather than a
|
|
resurrection, and the payload shapes differ only by a pair of brackets —
|
|
`{"Command":{...}}` against `{"Commands":[{...}]}` — which is exactly the
|
|
difference a permissive alias would paper over.
|
|
|
|
**The injected-fault discipline is the part to keep**
|
|
(`doctrine(check_the_checker)`): the mutated input is asserted **well-formed
|
|
JSON first**, and the failure is asserted to be about the *name* — serde's
|
|
`unknown variant` — not merely that an error occurred. A rewrite that also
|
|
breaks the JSON makes this pass against an enum that really does have the
|
|
variant.
|
|
|
|
---
|
|
|
|
## `src/reduce.rs`
|
|
|
|
### a_model_command_leaves_the_view_slice_untouched
|
|
|
|
A model command is the document's business, not this client's. Folding one
|
|
into the view slice would be a client writing state it does not own — the
|
|
document's answer comes back as projections. The cursor is set to a non-default
|
|
value first, so "the whole state is still default" cannot stand in for "the
|
|
state did not move".
|
|
|
|
### folding_the_same_stream_twice_gives_the_same_state
|
|
|
|
Both ends fold the same commands, so the same stream must produce the same
|
|
state — otherwise the server's mirror drifts from the client it is mirroring
|
|
(improvise-m91). A `Projection` is included in the stream, to pin that it
|
|
passes through `reduce_view` untouched rather than being a case it handles.
|
|
|
|
### every_command_round_trips_through_json
|
|
|
|
One populated value per payload **family**
|
|
(`doctrine(populate_before_roundtrip)`), not per variant: the exhaustive walk
|
|
over the 26 `ModelCommand` variants is `every_model_command` in
|
|
`improvise-command`, where the effect each one names can also be checked.
|
|
|
|
The `modelled()` helper composes its basis through a seeded `BasisCache`
|
|
rather than building a `Basis` literal — not because this test is about the
|
|
basis, but because **that is the only constructor there is**, and a test that
|
|
needed a second one would be evidence the door had been left open.
|
|
|
|
### cursor_commands_land_in_the_active_views_entry
|
|
|
|
The second assertion is the point: the view we switched *away* from never
|
|
gained an entry. An implementation writing a single cursor and copying it per
|
|
view would pass the first assertion.
|
|
|
|
### switching_views_restores_each_cursor
|
|
|
|
The point of keying the cursor map: switching back restores where you were,
|
|
and the client can do that **without a `Workbook`**.
|
|
|
|
### help_pages_stop_at_both_ends_and_set_clamps
|
|
|
|
Both bounds and the clamp, because going back from page 0 is the one that
|
|
underflows if written as `help_page - 1`, and `Set` out of range is the one
|
|
that panics if written as an index.
|
|
|
|
It used to be named `..._wrap_in_both_directions_...` and asserted wrapping,
|
|
which was a **second derivation of the effects it came from and disagreed with
|
|
them**: `effect::HelpPageNext` clamps at the last page and `HelpPagePrev`
|
|
saturates at zero, while this arm wrapped modulo `HELP_PAGE_COUNT`. Nothing
|
|
compared the two, because no client had ever sent `Command::HelpPage` — the
|
|
help overlay's `h`/`l` were still upstream keys. improvise-e24v made them a
|
|
client's, and `walking_off_the_end_of_the_help_pages_agrees_with_the_host`
|
|
failed on the first press past the end (client 0, host 4). The wire was
|
|
changed to match the effect rather than the other way round: the effect is
|
|
what ships in the TUI, and changing *that* is a product decision nobody asked
|
|
for. Whether wrapping is wanted on both ends is filed separately.
|
|
|
|
---
|
|
|
|
## `src/lib.rs`
|
|
|
|
### the_wire_types_need_nothing_but_core_and_serde
|
|
|
|
improvise-cqi AC 2: this crate crosses a process boundary, so it must not drag
|
|
a terminal backend with it. A `cargo tree` check would be external; this is
|
|
the part the **compiler** can hold — the types here resolve with only
|
|
`improvise-core` and `serde` in scope, and adding a `ratatui` or `crossterm`
|
|
dependency would have to be written into `Cargo.toml` deliberately rather than
|
|
arriving through a `use`. The CI `wasm` job is the other half: a crate pulling
|
|
in crossterm would stop building for wasm32.
|
|
|
|
---
|
|
|
|
## `src/viewmodel.rs`
|
|
|
|
### expanding_an_absent_category_is_harmless
|
|
|
|
An expanded name the cache does not carry must not shift the flattening:
|
|
`expanded_cats` is per-client and survives a document swap, so it **routinely**
|
|
names categories that are gone. A flattening that indexed by expanded-name
|
|
would go off the end here.
|
|
|
|
### only_quit_advertises_nothing
|
|
|
|
improvise-2hi, carried over from `only_quit_has_no_hint` when the hints moved
|
|
out of `AppMode` (improvise-k099). The old `hint_text` had a `_ => ""` arm, so
|
|
the modes without hints were invisible; `ExportPrompt` was blank too
|
|
(improvise-1c9), and the status bar went empty at the one moment the user has
|
|
to be told what to type. Only `Quit` may say nothing — it is never rendered.
|
|
|
|
What it asserts now is one step stronger, because `hint_line` is derived: a
|
|
mode goes quiet not by someone forgetting a string but by every command it
|
|
advertises becoming unbound, which
|
|
`every_advertised_hint_names_a_binding_that_exists` catches first.
|
|
|
|
### every_advertised_hint_names_a_binding_that_exists
|
|
|
|
The replacement for `every_hint_key_resolves_in_its_mode` (improvise-2hi),
|
|
which is **deleted**, and the deletion is the point
|
|
(`doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished)`).
|
|
|
|
That test existed because `AppMode::hint_text` was a hand-written string
|
|
promising what the keys do, with nothing tying it to the keymaps. It walked
|
|
every `Key:verb` token in every hint through a 60-line tokenizer that had to
|
|
know the hint dialect — named keys, `Ctrl-<c>`, `F/C/V` alternatives, runs like
|
|
`hjkl`, the em-dash prose tail, CommandMode's ex-commands — and panicked on a
|
|
token it could not classify, because a token it skipped was exactly the one
|
|
that would go unchecked.
|
|
|
|
None of that is needed now. A hint names a **command**, and its keys are looked
|
|
up in `Keymaps::default_keymaps()`, so "the hint names a key that resolves" is
|
|
true by construction and cannot be otherwise. What remains checkable, and is
|
|
what this test checks, is the other direction: a mode advertising a command
|
|
that nothing binds. That was never checkable before.
|
|
|
|
Note what neither version reaches, and gotcha 14 stated for the old one: that
|
|
the *caption* is accurate. `i:edit` where `i` enters records mode still passes.
|
|
The caption is English beside a command name, and only a reader can judge it.
|
|
|
|
### the_retired_invalidated_spelling_does_not_parse
|
|
|
|
improvise-n89z deleted `GridUpdate::Invalidated`, and the one way it can come
|
|
back without anyone noticing is as a serde **alias** — a wire spelling that
|
|
parses and that nothing produces, which is the state the deletion was for.
|
|
|
|
It keeps the discipline `the_retired_spellings_do_not_parse` earned in
|
|
`frame.rs`: assert the mutated input is still **well-formed** first (a
|
|
`ColWidths` projection of the same shape parses), and assert the **error
|
|
kind** — serde's `unknown variant` — rather than merely that an error
|
|
occurred, because a rewrite that also broke the JSON would make the check pass
|
|
against an enum that really did have the variant.
|
|
|
|
Proved discriminating by re-adding the variant with an empty `apply` arm: the
|
|
retired spelling parses again and the test fails on `expect_err`.
|
|
|
|
|
|
### boundary_tests::the_app_mode_cycle_reaches_every_variant_serde_declares
|
|
|
|
### basis::tests::the_rejection_cycle_reaches_every_variant_serde_declares
|
|
|
|
improvise-5ubs. Both `all()` functions walk a cyclic `after()`, and both doc
|
|
comments claimed a new variant could not be omitted. Answering `after()` for a
|
|
new variant is compiler-forced; pointing an existing arm at it is not, so a
|
|
variant nothing names is never walked. The reviewer of PR #78 measured it:
|
|
`AppMode::all().len()` came back 17 against 18 variants, suite green.
|
|
|
|
The independent enumeration is serde's — the `Deserialize` derive builds the
|
|
variant-name list from the enum definition and publishes it in its
|
|
`unknown variant ... expected one of ...` message. `variants.rs` reads it back
|
|
and diffs it against the names the cycle reached, each name taken off the
|
|
variant's own serialization rather than off `Debug`.
|
|
|
|
Fault injected 2026-09-21: a `Rejection::Probe` variant added to the enum, with
|
|
an `after()` arm returning `DocumentReplaced` and **nothing pointing at it** —
|
|
precisely the shape the reviewer probed. `the_rejection_cycle_reaches_every_variant_serde_declares`
|
|
fails naming `["Probe"]`; the other 120 tests in `improvise-protocol`'s lib pass,
|
|
including `the_rejection_cycle` half of every basis test. (The workspace does not
|
|
compile with the probe, because `improvise-wasm-client/src/socket.rs` matches
|
|
`Rejection` exhaustively — which is a *different* guard and does not catch the
|
|
omission, since the omitted variant there is one it was forced to name.)
|
|
|
|
### variants::tests::a_type_that_is_not_an_externally_tagged_enum_is_refused
|
|
|
|
### variants::tests::a_message_with_no_expected_clause_is_refused
|
|
|
|
### variants::tests::a_message_that_lists_something_other_than_variants_is_refused
|
|
|
|
### variants::tests::a_list_that_collapses_to_one_name_is_refused
|
|
|
|
### variants::tests::a_well_formed_message_yields_its_names
|
|
|
|
The checker's own checker. A parse of serde's message that silently failed would
|
|
return an empty name list, and an empty list is a subset of everything — every
|
|
cycle would pass and the guard would be worse than absent.
|
|
|
|
**The first version named one thing and exercised another**, found by the PR #92
|
|
review. It pointed `declared` at `Basis`, a **struct**, so serde answers
|
|
`invalid type … expected struct Basis` — which reaches only the
|
|
`split_once → None` branch. The `starts_with("unknown variant")` and
|
|
`names.len() > 1` refusals were exercised by nothing, and all three panics shared
|
|
the literal `"serde no longer says"`, so `expected = …` could not discriminate
|
|
which had fired. **A `should_panic` whose message matches several panics is a
|
|
check that cannot say what it caught.**
|
|
|
|
The parse is now `parse_expected(&str)`, a seam fed hand-written mutated
|
|
messages, each refusal carrying its own wording:
|
|
|
|
- no `expected one of` clause → `serde stopped listing the variants`
|
|
- an `unknown field` list rather than a variant list → `serde stopped saying
|
|
\`unknown variant\``
|
|
- a list that collapses to one name → `the variant list did not parse`
|
|
|
|
plus a happy-path case, so the three refusals are not the only evidence the
|
|
parser runs at all. `a_type_that_is_not_an_externally_tagged_enum_is_refused`
|
|
keeps the end-to-end shape through a real `Deserialize`.
|
|
|
|
|
|
### render::tests::a_frame_that_lost_its_origin_does_not_claim_to_lead_the_axis
|
|
|
|
improvise-1wl0, found by the adversarial review of PR #77. `AxisFrame::first`
|
|
carried `#[serde(default)]`, so a frame whose `first` was dropped in transit
|
|
deserialized as `0` — which is not an absence but the claim that the frame
|
|
*leads the axis*, and exactly what `body_from` acts on:
|
|
`match self.first + idx == 0`. A scrolled browser would then draw the carried
|
|
group header above its first row, at every scroll position, silently: the
|
|
mirror image of the improvise-0o6x bug the field was added to fix.
|
|
|
|
The test serializes a frame built for a scrolled window (`first: 3`), removes
|
|
the key, and asserts the refusal. Three things make it discriminating rather
|
|
than a check that something went wrong:
|
|
|
|
- It asserts the key **was there** before removing it, so a rename of the field
|
|
turns this into a failure rather than a vacuous pass.
|
|
- It asserts the mutated value is **still a well-formed AxisFrame minus one
|
|
field** (`cats` and `entries` present), so the refusal cannot be about broken
|
|
JSON — the check-the-checker hole gotcha 42 records from
|
|
`the_retired_spellings_do_not_parse`.
|
|
- It asserts the **error kind** is `missing field`, not merely that an error
|
|
occurred. A `first` that had been retyped would fail differently and this
|
|
would notice.
|
|
|
|
Run against the pre-fix tree 2026-09-21: it fails at the `expect_err`, printing
|
|
`AxisFrame { .., first: 0, .. }` — the forged claim itself. The fix is the
|
|
removal of one attribute; the whole workspace stays green (1464), which is also
|
|
the evidence that nothing was relying on the permissive parse.
|
|
|
|
|
|
### render::tests::two_runs_under_two_headers_of_one_name_are_two_runs
|
|
|
|
### render::tests::the_two_headers_are_equal_as_values_so_comparing_them_cannot_split_them
|
|
|
|
improvise-h0iv. `group_runs` coalesced adjacent positions by group **name**, so
|
|
two runs under two different headers sharing a name read as one and both front
|
|
ends drew one span across the boundary.
|
|
|
|
The first test is the demonstration, at the level the rule lives: a frame with
|
|
`[Group(Month/Q1), North/Jul, North/Aug, Group(Month/Q1), South/Jul, South/Aug]`
|
|
and nothing ungrouped between the runs. Before the fix it returns
|
|
`[(Q1, 4)]`; after, `[(Q1, 2), (Q1, 2)]`. It asserts the frame really holds
|
|
**two** `FrameEntry::Group` entries first, so a fixture that lost one would fail
|
|
at the precondition rather than pass with a single run.
|
|
|
|
The second test exists because the ticket proposed the wrong fix and a reader
|
|
will propose it again. h0iv said `FrameGroup` carried `cat` and that "comparing
|
|
the pair would fix it". But its own reachability case is a single category
|
|
nested under an outer one, so both headers are `(Month, Q1)`, equal as values.
|
|
The test asserts that equality directly. Since improvise-7xyx removed
|
|
`FrameGroup::cat`, it asserts equality of the whole header, which is stronger:
|
|
no field-wise comparison can separate two equal values. What actually separates them is that they are
|
|
two distinct entries in `self.entries`, which `std::ptr::eq` on the borrowed
|
|
header states exactly.
|
|
|
|
Fault injected 2026-09-21: restoring `prev.map(|p| p.name.as_str()) == ...`
|
|
fails the first test with `left: [(Some("Q1"), 4)]`, and fails
|
|
`two_abutting_runs_of_one_group_name_are_two_spans_at_both_ends` end to end. The
|
|
second test passes under both rules, deliberately — it is a statement about the
|
|
data, not about the comparison.
|
|
|
|
|
|
### render::tests::a_client_never_holds_widths_of_a_different_length_from_its_labels
|
|
|
|
improvise-44ad's replacement test, landed while the move it was written for was
|
|
**declined**. A renderer zips `col_widths` with the column labels, so a length
|
|
mismatch indexes off the end (gotcha 33(c)). The existing guards assert the
|
|
re-fit on individual arms; this one asserts the invariant across a **fold**, at
|
|
every step, over a stream that widens the frame before its widths arrive, lands
|
|
a cell, catches the widths up, narrows the frame, lands totals, and catches up
|
|
again.
|
|
|
|
It asserts the first update really widens past the widths held, so a fixture
|
|
that stopped exercising the gap would fail at the precondition rather than pass.
|
|
|
|
Fault injected 2026-09-21: deleting the width re-fit from `fit_to_labels`
|
|
(`self.col_widths.truncate(cols); if self.col_widths.len() < cols {` replaced by
|
|
`if false {`) fails this at **step 0** — 3 widths for 5 columns.
|
|
|
|
**What that injection also established, and it corrects improvise-44ad.** Only
|
|
four tests in the whole workspace fail: this one and three that already existed
|
|
(`a_col_widths_update_is_fitted_to_the_labels_it_is_framed_by`,
|
|
`a_column_without_its_width_yet_is_measured_in_display_cells`,
|
|
`col_widths_always_span_the_column_labels`). The ticket's premise was that
|
|
`a_client_on_a_socket_is_seeded_and_gets_its_edit_back` is gotcha 33(c)'s
|
|
standing evidence and that rewriting it would destroy the evidence for a rule.
|
|
It is not, and it does not fail: it asserts a reframe *carried* widths, which is
|
|
a statement about the producer's diff. The reason for declining is therefore not
|
|
the one the ticket gives — see `context/code/protocol.md` §3.
|
|
|
|
### a_chord_is_recognised_by_its_modifiers_and_not_by_its_label
|
|
|
|
`help::compact` dropped a chorded key from a status-bar hint by testing whether
|
|
the formatted label started with `"Ctrl-"` or `"Alt-"` — parsing a string it had
|
|
just formatted. Finding 3 of the adversarial review of PR #50, filed as
|
|
improvise-thpe and deliberately not fixed there: the reviewer enumerated every
|
|
`format_key_label` output and none begins with either, so there was **no false
|
|
positive**. The defect was the shape.
|
|
|
|
**The regression guard is the signature**, as it is for `Checked` (gotcha 43):
|
|
`compact` now takes `&[(KeyCode, KeyModifiers)]`, so the string it used to parse
|
|
is not in scope to parse. `keys_for` returns the pairs and `Hint::keys` carries
|
|
them; `help.rs` formats last, through `Hint::labels` and the one spelling in
|
|
`keymap::key_label`. `Keymap::labels_for` — the "throw the modifiers away into a
|
|
string" step the ticket names — has **no caller left and is deleted**, because a
|
|
consolidation that keeps its old path keeps the defect available
|
|
(`doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished)`).
|
|
|
|
The rule also got wider, and that is the part a test can state: a chord is any
|
|
modifier **beyond shift**, which is total over the bitset, where the string test
|
|
enumerated two prefixes. Shift alone is not a chord — it is how a key is named
|
|
rather than how it is held, which `S-Tab` is the case for.
|
|
|
|
**This test cannot fail under the rule it names**, and its doc comment says so
|
|
rather than claiming otherwise. `CONTROL|ALT|SHIFT` on `F(5)` spells
|
|
`Ctrl-Alt-F5`, which the old prefix test drops too. It pins the rule `is_chord`
|
|
states; the *discrimination* is the injection below and the signature.
|
|
|
|
**Injected fault, run rather than argued.** A `KeyModifiers::SUPER` bit and a
|
|
`key_label` arm spelling it `Super-x` were added temporarily. The old filter
|
|
keeps it — `["T", "Super-x"]`, two survivors where one is wanted — while
|
|
`compact` returns `"T"`. That is the ticket's own reproduction, executed, and it
|
|
is the only way to see the difference: with `KeyModifiers` as shipped, every
|
|
chord's label *does* begin with `Ctrl-` or `Alt-`, so no permanent test can
|
|
separate the two rules. Both the bit and the probe were removed afterwards.
|
|
|
|
One behaviour that had to be preserved deliberately: `keys_for` used to `dedup`
|
|
**labels**, and deduping the pairs instead would let two patterns that spell the
|
|
same key both reach the status bar. It now dedups by label explicitly.
|
|
|
|
The binding dump is byte-identical across this change (4406 lines, `diff`
|
|
clean), which is what `doctrine(binding_dump_diff)` asks of anything touching
|
|
the keymap: the dump's prefix rows go through `Hint`, so they had to be checked
|
|
rather than assumed.
|
|
|
|
### a_group_end_stops_the_membership_walk
|
|
|
|
### a_collapsed_group_reaches_the_frame_with_no_item_under_it
|
|
|
|
The same property, asserted in the crate that **owns** the type. Integration
|
|
review found `AxisFrame::group_of` had no coverage of its `GroupEnd` arm at
|
|
all: replacing it with `FrameEntry::GroupEnd => None` left all **133**
|
|
`improvise-protocol` lib tests passing. Only `tests/grid_frame_agreement.rs`
|
|
in the root crate caught it, and that file does no pruning.
|
|
|
|
So improvise-gd83's property reached the protocol **by construction through
|
|
`project.rs`, never by assertion** — a derivation living in two crates with
|
|
only one of them checked, which is what gotcha 44 and design-principles §5
|
|
exist to prevent. The two frames here are the exact shapes `prune_axis_items`
|
|
now produces: the re-opened run closed by a `GroupEnd`, and the collapsed
|
|
header travelling with no item under it.
|
|
|
|
The collapsed one also asserts `collapsed: true` reaches the frame, because a
|
|
client that receives the header without the flag cannot offer re-expansion —
|
|
the header would be there and inert, which is improvise-lhi4 again one layer
|
|
out.
|
|
|
|
Fault injected: `FrameEntry::GroupEnd => None` in `group_of` now fails **both**,
|
|
each with `Some("Q1")` against `None`, where before it failed nothing in the
|
|
crate. What these two do **not** cover is the composition — that
|
|
`prune_axis_items` → `axis_frame` → `group_of` really produces these frames.
|
|
That assertion needs `improvise-command/src/ui/project.rs`; it was measured by
|
|
hand and is filed rather than landed.
|
|
|
|
### wide_cell_clamps_without_overflow
|
|
|
|
improvise-yae4. `grid_body` narrowed each measured width to `u16` and only then
|
|
added the one-character gap and clamped to `MAX_COL_WIDTH`, so the arithmetic
|
|
ran at full width before the cap that makes it safe. A stored 65535-column text
|
|
value panicked at `w + 1` in debug; in release the sum wrapped to `0` and the
|
|
column was drawn at `MIN_COL_WIDTH` — the *narrowest* column, for the widest
|
|
value. A 65536-column value never reached the addition at all: `as u16`
|
|
truncated it to `0` first, so it wrapped in debug builds too, silently.
|
|
|
|
Widths are now capped while still `usize`, in the one place every measurement
|
|
enters (`widen`), which is also why `header_level_seed` folds through it rather
|
|
than keeping a second `as u16`. The answer is unchanged for every input that
|
|
did not overflow: `(min(w, 32) + 1).clamp(5, 32)` and `(w + 1).clamp(5, 32)`
|
|
agree everywhere, because both saturate at 32 for any `w >= 31`.
|
|
|
|
`improvise_protocol::header_level_widths` carried the identical defect on the
|
|
row-header axis — a 65535-column *item label* — and is fixed the same way. It
|
|
is the one home for that rule (gotcha 44), so the row header and the column
|
|
widths do not get two answers.
|
|
|
|
The test asserts `widths == vec![expect]`, not `all(<= 32)`: the length pins
|
|
that the fixture really produces **one** column, and the ordinary 20-column arm
|
|
pins that the single column is the one carrying the value — a fixture in which
|
|
no column was wide would read `MIN_COL_WIDTH` there and pass a `<= 32` check.
|
|
|
|
Faults injected, each against the fixed code:
|
|
- `width as u16` in `widen`, debug: the 65535 arm panics with
|
|
`attempt to add with overflow`.
|
|
- the same, release: the 65536 arm fails with `left: [5]`, `right: [32]` —
|
|
the wrap reported as the minimum width.
|
|
|
|
---
|
|
|
|
## `src/keymap.rs` — the global (tests live in `improvise-command`)
|
|
|
|
The keymap's tests sit with `dispatch` in
|
|
`improvise-command/src/command/keymap.rs`, because checking a bound name needs
|
|
a registry (`context/code/protocol.md` §6). The table itself is this crate's,
|
|
so its evidence is here.
|
|
|
|
### every_modes_keymap_chain_ends_at_the_same_global
|
|
|
|
Before improvise-636d `Keymap::with_parent` had **two** call sites in
|
|
seventeen modes, and `?` therefore resolved in **2 of 17** — not in any panel,
|
|
not in TileSelect, SearchMode or ImportWizard, and not in Help itself.
|
|
|
|
The comparison is `Arc::ptr_eq`, and that is the whole point: the defect being
|
|
removed is *several tables that agree today*, which any by-value comparison
|
|
would accept. The test also asserts no mode **is** the global, so a keymap set
|
|
that made Normal the root — inheritance "working" with sixteen children and
|
|
one privileged parent that is also a mode — fails rather than passes.
|
|
|
|
Fault injected against the fixed code: `SearchMode` rebuilt as
|
|
`Keymap::new()`. It fails with `SearchMode does not reach the global — its
|
|
chain roots elsewhere`. Note what that injection also showed, which is why the
|
|
sibling test below is not redundant:
|
|
`every_global_key_resolves_in_every_mode_and_is_shadowed_where_it_must_be`
|
|
stayed **green** under it, because SearchMode's own `AnyChar` answers `?` with
|
|
`append-char` either way — the expected answer. Resolution alone cannot see a
|
|
missing inheritance edge behind a catch-all; only the chain can.
|
|
|
|
### the_global_binds_only_unmodified_char_keys
|
|
|
|
`KeyPattern::AnyChar` matches `KeyCode::Char` and nothing else, so the
|
|
shadowing that makes the global free in the eight text-entry modes is a
|
|
property of the **key kind**, not of the binding. A global `F1` would reach
|
|
the parent mid-edit and throw away a half-typed cell; a global `Ctrl-x` would
|
|
miss `lookup_local`'s `Char`-with-`NONE` retry, which is what a shifted `?`
|
|
relies on. The test asserts the shape rather than a list of keys, plus a count
|
|
derived from `Panel::ALL` so a fourth panel does not need editing here.
|
|
|
|
It also refuses a `Binding::Prefix` in the global, because a prefix reached
|
|
through a parent would be resolved by `Keymaps::prefix_keymap` in seventeen
|
|
modes at once — `prefix_keymaps_are_flat` checks nesting, not location.
|
|
|
|
Fault injected: `km.bind_args(KeyCode::F(1), none, "enter-mode", vec!["help"])`
|
|
added to `global_keymap`. This test fails naming `F(1)`, and — separately and
|
|
for the better reason — the shadowing test fails with
|
|
`Editing: F(1) must reach append-char, not the global`.
|
|
|
|
### no_mode_binds_a_panel_toggle_of_its_own
|
|
|
|
`bind_panel_toggles` was called **twice** (Normal and `PanelKeymap::new`)
|
|
while `:` was written into two spines and `?` into one. Nothing held those
|
|
three in agreement; the asymmetry between them was the only evidence they had
|
|
drifted, and it was in the source rather than in a test.
|
|
|
|
This is the mechanical check that the duplication is gone, expressed as a
|
|
property rather than as a grep: a mode's **own** `bindings()` may not contain a
|
|
toggle key, so re-adding a second call site fails here whichever keymap it is
|
|
added to. A grep for the call count could not say that.
|
|
|
|
Fault injected: `bind_panel_toggles(&mut normal)` restored in
|
|
`default_keymaps`. Fails with `Normal binds 'F' itself; the global already
|
|
does`. Note `every_mode_with_panel_toggles_binds_all_of_them` — which already
|
|
existed — stays green under that injection, because it goes through `lookup`,
|
|
which walks parents and so cannot tell a local binding from an inherited one.
|
|
|
|
### every_global_key_resolves_in_every_mode_and_is_shadowed_where_it_must_be
|
|
|
|
The half no existing test could state: `every_hint_key_resolves_in_its_mode`
|
|
checks a mode's hint line against its own map and is blind to a key no hint
|
|
mentions, which is every global key in sixteen of seventeen modes.
|
|
|
|
The keys are read **off the global** rather than listed again here, so adding
|
|
a sixth global key extends the check with no edit. The expectation table is
|
|
derived too — the typing modes come from `AppMode::text_entry()`, not from a
|
|
literal list — so the one thing stated by hand is Help's `?`, which is the
|
|
only decision.
|
|
|
|
Help's arm asserts **both** that `?` closes the overlay and that the binding is
|
|
Help's **own**: without the second assertion the arm would also pass if Help
|
|
had no `?` at all and something upstream happened to answer `enter-mode
|
|
normal`, which is precisely the state the ticket found.
|
|
|
|
The fallback arm was the review's find in this branch's own work. It was
|
|
`_ => None`, meaning the six non-shadowing modes asserted only that the key
|
|
resolved to *some* plain command — *is this key bound*, where the question is
|
|
*does this key reach the global*. It now asserts the global's **own** command
|
|
name, read off the global rather than restated, so the only hand-written
|
|
expectation left is Help's `?`, which is the only decision.
|
|
|
|
Faults injected against the fixed code:
|
|
- Help's `?` override deleted: fails with ``?` in Help should close the
|
|
overlay, not re-enter it`.
|
|
- the **rival fix** — `TileSelect` given `Arc::new(global_keymap())`, its own
|
|
equal-but-separate copy. This test stays **green**, because every global key
|
|
does resolve, to exactly the right command. Only
|
|
`every_modes_keymap_chain_ends_at_the_same_global` fails, and that is the
|
|
clearest statement of why it compares by pointer: a per-mode copy is the
|
|
duplication improvise-636d removes, and it is invisible to every check that
|
|
asks what a key *does*.
|
|
|
|
|
|
## `src/keymap.rs` — the layer between the global and the modes (improvise-wiew)
|
|
|
|
The user asked for `[`/`]` "probably in the global keymap", and then ruled on
|
|
the question that ask left open: *"maybe we need an intermediate keymap: the
|
|
help display (and import wizard, etc.) shouldn't respond to these keys, but the
|
|
'normal' modes like categories/tiles/formulas/etc. should."*
|
|
|
|
So the distinction is **is the document the subject right now**, and it is a new
|
|
distinction rather than the global's. Every key the global already binds — `?`,
|
|
`:`, the three panel toggles — is a **UI** action. `page-next`/`page-prev`
|
|
resolve to `ModelCommand::SetPageSelection`, which is
|
|
`EffectKind::Model(LayoutGeometry)`: the first key on a shared layer that
|
|
*changes the document*. Under the Help overlay that is a real mutation the user
|
|
cannot see, because the overlay is covering the grid it just moved.
|
|
|
|
`document_keymap` is therefore a third level: `global → document → {Normal, the
|
|
three panels, TileSelect}`, with Help, ImportWizard and the text-entry modes
|
|
taking `global` directly. RecordsNormal reaches it **through Normal**, which it
|
|
already parents — parenting it directly as well would be a second edge to keep
|
|
in step, and `records_normal_inherits_the_document_layer_once_through_normal`
|
|
asserts the chain is exactly four long.
|
|
|
|
Two things about the name. It is `document_keymap` and not `common`/`shared`
|
|
deliberately: the name is the only thing that tells the next person whether a
|
|
new mode should parent it, and a name that describes *who inherits it* invites
|
|
everything to join. And the layer is **derived** in every test off
|
|
`Normal.parent()` rather than exported, so a test cannot restate a membership
|
|
the table disagrees with.
|
|
|
|
### the_page_keys_reach_the_document_modes_and_no_overlay
|
|
|
|
The six document modes answer `]`/`[` with `page-next`/`page-prev`; Help
|
|
resolves **neither at all** (`lookup` is `None`, not "shadowed into something
|
|
harmless"), ImportWizard answers `handle-wizard-key` through its `Any`, and
|
|
CategoryAdd answers `append-char` — so typing a bracket into a name still types
|
|
it, which is the fallback order (`AnyChar` before the parent) doing its job.
|
|
|
|
Fault injected: parenting Help to `document_keymap` instead of to `global`.
|
|
`]` in Help then resolves to `page-next` and the test fails. Note the assertion
|
|
that discriminates is the Help one — the six positive rows pass either way.
|
|
|
|
### records_normal_inherits_the_document_layer_once_through_normal
|
|
|
|
Fault injected: `Keymap::with_parent(document.clone())` for RecordsNormal in
|
|
place of `with_parent(normal)`. The test above **still passes** — RecordsNormal
|
|
answers `page-next` either way — and this one fails on the parent identity.
|
|
That pair is the reason it is a separate test: a duplicated inheritance edge is
|
|
invisible to every behavioural assertion on the day it is added, and only shows
|
|
up the first time Normal and the layer disagree.
|
|
|
|
### the_document_layer_binds_only_unmodified_char_keys
|
|
|
|
The sibling of `the_global_binds_only_unmodified_char_keys`, and it is owed for
|
|
exactly the same reason: the layer is reached by **fallthrough**, so a non-Char
|
|
binding on it would fire while a descendant mode's `AnyChar` was typing. No
|
|
descendant binds `AnyChar` today, which is precisely why the constraint has to
|
|
be written down rather than relied on — an unconstrained new layer is where the
|
|
next "just put it in the shared one" lands.
|
|
|
|
### resolve::tests::picking_a_page_item_moves_that_axis_and_only_that_axis
|
|
|
|
improvise-rw6l. `Click::PageItem { axis, item }` is the browser's "the user
|
|
picked straight off the bar", and the property that separates it from `]` is
|
|
that it names **one** axis: `page_cycle` is an odometer that restates every
|
|
navigable axis and carries between them, so on a two-axis fixture `]` from
|
|
`Year=2024, Quarter=Q4` emits both `Year=2025` and `Quarter=Q1`. The click emits
|
|
`Quarter=Q1` alone. A fixture with one page axis cannot tell the two apart.
|
|
|
|
The basis it claims is `state.basis.claim([])` — the same claim `page_cycle`
|
|
makes, because `SetPageSelection` names no record (gotcha 43). It is written as
|
|
that call rather than as the `published()` fixture so the test says *which*
|
|
claim it means; the fixture carries a record tier this command must not state.
|
|
|
|
### resolve::tests::a_filter_axis_offers_no_item_to_pick
|
|
|
|
Gotcha 27, from the click side. A Filter axis is on the page bar and carries no
|
|
items, so `PageAxis::cycle` is `None` and the click composes nothing — the same
|
|
one home the arrow and the menu follow. The two out-of-range rows beside it
|
|
(an axis past the end of the bar, an item past the end of the list) are there
|
|
because the page names both by index, so a stale bar under a repaint is the
|
|
realistic way either arrives.
|
|
|
|
Fault injected: resolving `items` off `axis.items` directly instead of through
|
|
`cycle()`. The Filter arm then indexes an empty `Some(vec![])` for a Page axis
|
|
with no items — and, more to the point, the `items.is_some()` spelling is the
|
|
one gotcha 27 says is *not* the split.
|
|
|
|
### vocabulary_is_composed::every_command_variant_is_composed_by_production_code
|
|
|
|
improvise-m0sx. `Command::ClearBuffer(String)` was declared on the wire and
|
|
composed by **nothing** in production: its only occurrences were the enum, the
|
|
fold arm, and two of `reduce.rs`'s own tests. Worse, its fold arm
|
|
(`view.buffers.remove(name)`) contradicted the only implementation of its own
|
|
name — `clear-buffer` composes `effect::SetBuffer { value: "" }`, which
|
|
*inserts*, and `resolve.rs::write_buffer` composes the same `Command::SetBuffer`.
|
|
So the variant was invented rather than derived, and it disagreed with both
|
|
ends. It is deleted; see `context/code/protocol.md` §2 for why deletion beat
|
|
giving it an effect.
|
|
|
|
This test is the guard for the class. It reads the variant list off serde's
|
|
`unknown variant` message (`variants::declared`) — the one enumeration the
|
|
compiler maintains — and scans non-test workspace source for each name in a
|
|
value position.
|
|
|
|
Fault injected: `Command::SetTileCatIdx` rewritten to `Command::SetRowOffset` at
|
|
its three production sites in `resolve.rs`, leaving only its test occurrences.
|
|
The test then failed naming `["ClearBuffer", "SetTileCatIdx"]` — i.e. it failed
|
|
the *property*, not its own setup, and named precisely the variant that lost its
|
|
composer. Reverted.
|
|
|
|
Why it cannot pass vacuously: `the_scan_reaches_production_code_at_all` fails if
|
|
the source walk returns nothing recognisable, which is the direction a broken
|
|
walk fails in (a broken walk otherwise reads as "everything is composed" only
|
|
for the *second* test, never this one — this one gets stricter, not weaker, when
|
|
the scan finds less).
|
|
|
|
### vocabulary_is_composed::the_uncomposed_list_names_nothing_that_is_composed
|
|
|
|
The other direction of the same list. `NOT_COMPOSED` names two variants that are
|
|
genuinely derived — `effect::SetYanked` and `effect::SetSearchMode` both exist —
|
|
but that no resolver composes yet, so they are resolver gaps rather than
|
|
inventions: improvise-lut7 (nothing resolves yank, so `p` reads a permanently
|
|
empty slot) and improvise-fo0w item 1 (search, deliberately held back until
|
|
search-navigate can ship with it).
|
|
|
|
Both are open and one is in progress, so the entries are expected to be removed.
|
|
This test is what forces that: an allowlist entry whose variant *is* composed
|
|
fails, so closing either ticket without deleting its line breaks the build
|
|
rather than leaving a stale claim. `the_uncomposed_list_names_only_real_variants`
|
|
covers the third direction — an entry naming no variant at all, which a rename
|
|
would produce.
|
|
|
|
Fault injected: adding `"SetBuffer"` to `NOT_COMPOSED`. `SetBuffer` is composed
|
|
in `resolve.rs`, so the test failed naming it.
|
|
|
|
### reduce::tests::buffers_are_set_and_cleared_by_name
|
|
|
|
Rewritten with improvise-m0sx. It previously folded `Command::ClearBuffer` and
|
|
asserted `!view.buffers.contains_key("edit")` — pinning the divergence, since
|
|
neither implementation of "clear this buffer" removes the key. It now folds
|
|
`SetBuffer { value: "" }`, which is what both ends compose, and asserts the key
|
|
is **present and empty**.
|
|
|
|
That assertion is the discriminating one: a fold arm that removed the key would
|
|
give `None` and fail. Asserting only that `"command"` survived would pass under
|
|
either behaviour, which is what the old spelling amounted to for the cleared
|
|
key.
|
|
|
|
### render::tests::a_cell_equals_another_exactly_when_the_wire_cannot_tell_them_apart
|
|
|
|
`CellDisplay`'s equality is what the grid diff and the seed property run on
|
|
(improvise-4w09). It compares `value` by `same_on_the_wire`: bits, except that
|
|
every NaN is equal. It pins both clauses, since `a_grid_holding_nan_quiesces`
|
|
can see only one of them. That test compares a NaN with its own bits,
|
|
in-process.
|
|
|
|
- A NaN with the sign bit set, the shape `0.0 / 0.0` produces on x86, equals
|
|
`f64::NAN`, the shape `parse_nonfinite("nan")` gives a client. Both go out as
|
|
the one token `"nan"`.
|
|
- `0.0` differs from `-0.0`: serde_json writes them as `0.0` and `-0.0`, so a
|
|
change between them is real and must be sent.
|
|
- `Number(7.0)` differs from `Text("7")`, and a value differs from none.
|
|
|
|
**Faults injected:**
|
|
- `f64 ==` (the pre-fix semantics). Fails at the NaN case.
|
|
- Bit equality only. Fails at the NaN case, while `a_grid_holding_nan_quiesces`
|
|
**passes**, which is why this test exists.
|
|
- `f64 ==` plus the NaN clause. Fails at the `0.0`/`-0.0` case.
|
|
|
|
### vocabulary_is_composed::a_variant_named_only_in_a_comment_or_string_is_not_composed
|
|
|
|
improvise-c7mu, found by the post-merge review of PR #160. `constructed_in` ran
|
|
over raw source text, so any `Command::X` counted as a composer, including one
|
|
inside a comment or a string. The review deleted the real `Command::SetYanked`
|
|
composer from `resolve.rs`, left `// TODO: Command::SetYanked(value)` in its
|
|
place, and all four tests passed. The test feeds the whole per-file pipeline a
|
|
line comment, a block comment and a string that each name a variant, and
|
|
asserts that none of them counts. A positive control asserts that a real
|
|
construction still does.
|
|
|
|
Fault injected: `code_only` made the identity function. This test failed with
|
|
`["SetYanked", "SetSearchMode", "ChangeMode"]`, and so did
|
|
`source_scan::tests::comments_and_literals_are_blanked_and_code_is_kept`. The
|
|
four whole-tree tests stayed green under that fault. This confirms the review's
|
|
finding that no live comment gives a wrong verdict *today*, and it is the
|
|
reason the property has to be pinned on a fixture rather than on the tree.
|
|
|
|
The fault on the real tree (deleting `resolve.rs`'s composer) was not rerun
|
|
here. `resolve.rs` is outside this lane, and the edit was refused as another
|
|
agent's ground.
|
|
|
|
### source_scan::tests::a_test_only_module_file_is_not_production_code
|
|
|
|
improvise-c7mu, the second half. `#[cfg(test)] mod x;` makes a whole file test
|
|
code, and nothing inside the file says so. The old scan saw it as production,
|
|
and that included this checker's own file. The test builds a fixture tree:
|
|
`lib.rs` declares `mod p;` and `#[cfg(test)] mod t;`, and `t.rs` declares a
|
|
nested `mod deeper;`. It asserts that `production_code` returns exactly
|
|
`lib.rs` and `p.rs`.
|
|
|
|
Fault injected: the file filter in `production_code` made always-true. This
|
|
test failed with `["lib.rs", "p.rs", "t.rs", "t/deeper.rs"]`. Before this
|
|
fixture existed, the `improvise-protocol` and `improvise-command` lib suites
|
|
**stayed green** under the same fault, and so did
|
|
`test_only_items_and_modules_are_not_production`, which checks the helpers one
|
|
by one. The wiring between them was untested until this fixture existed.
|
|
|
|
### reduce::tests::every_model_command_field_is_position_free
|
|
|
|
improvise-hkbc. improvise-6nsd's "a stale layout tier cannot misdirect a
|
|
mutation" holds only while no `ModelCommand` carries a row or column. The guard
|
|
is the signature of `position_free`. It destructures every variant with no `..`,
|
|
down into `RecordCell`, and hands each field to a function that accepts only
|
|
`&str`, a path, a `RecordHash`, a `CellValue`, an `Axis` or an optional name.
|
|
The `#[test]` only keeps that signature compiled and reached.
|
|
|
|
Faults injected, one per half, each compiling only `improvise-protocol`:
|
|
|
|
- A new variant `ScrollTo { row: usize }`, also added to `records_named`'s
|
|
or-pattern the way an author would. The lib built. The test build failed
|
|
with `non-exhaustive patterns: ScrollTo` **at `position_free` and nowhere
|
|
else**.
|
|
- The same variant classified as `ScrollTo { row } => name(row)`. The test
|
|
build failed with `expected &str, found &usize`.
|
|
|
|
A field added to an existing variant is caught too, but not in isolation,
|
|
because every constructor of that variant also fails. Reverted with
|
|
`git restore`.
|
|
|
|
### source_scan::tests::a_test_only_item_hides_nothing_after_it
|
|
|
|
improvise-59q8, found by the PR #172 review. The scanner's line-based
|
|
`strip_test_items` looked for `}` at the attribute's indentation, or for an item
|
|
line ending in `;`. So three shapes were never bounded, and it dropped
|
|
**everything to the end of the file**:
|
|
- a test-only `const … = &[\n … \n];`;
|
|
- a `#[cfg(test)] let x = foo(\n 1,\n);` statement;
|
|
- a one-line `#[cfg(test)] use foo::bar;`, where the attribute's own line is
|
|
also the item line.
|
|
|
|
In `vocabulary_is_complete` that silently hides every effect after such an
|
|
item. Each shape is a case here, asserting that the production code after it
|
|
(`Hidden1`, `Visible`) survives. Against the line scan all three failed; the
|
|
first printed `"impl X {\n"` and nothing else. They now pass because items are
|
|
bounded by balanced brackets on the blanked code.
|
|
`an_unbounded_test_only_item_is_refused_not_swallowed` pins the other half of
|
|
the acceptance criteria: an item that cannot be bounded panics instead of
|
|
consuming the rest of the file. Against the line scan it failed, because
|
|
nothing panicked.
|
|
|
|
### source_scan::tests::a_test_only_cfg_is_recognised_whatever_its_spelling
|
|
|
|
improvise-59q8, the other direction: test code counted as production, which
|
|
makes `vocabulary_is_composed` weaker. `cfg(all(test, not(target_arch =
|
|
"wasm32")))` was kept because the old rule rejected any line containing
|
|
`not(`. A leading `#![cfg(test)]` was kept because only `#[cfg(` was
|
|
recognised. An attribute split across lines was not seen at all. All three
|
|
failed against the old scanner. The predicate is now parsed and evaluated in
|
|
three-valued logic (`context/code/protocol.md` §2). The controls
|
|
(`cfg(not(test))`, `cfg(any(test, feature = "serde"))` and `cfg(target_arch =
|
|
…)`) must stay production, so a scanner that dropped every `cfg` item could
|
|
not pass.
|
|
|
|
A consequence to know about: `cfg(any(test, feature = "x"))` for an ordinary
|
|
feature is now production. Only `feature = "test-support"`, the one test
|
|
feature this repo has, counts as a test build.
|
|
`test_only_items_and_modules_are_not_production` was updated to that spelling.
|
|
|
|
### source_scan::tests::a_c_string_literal_is_blanked
|
|
|
|
improvise-59q8's minor case. In `cr#"x" Command::InRawC(2) "y"#` the `r` has a
|
|
word character before it, so the literal was lexed as two ordinary strings,
|
|
and the text between the inner quotes leaked. It failed as
|
|
`InRawC survived`. `c"…"` and `cr#"…"#` are now literals. A first version of
|
|
this test used a C string with no inner quotes and passed against the bug,
|
|
because the ordinary lexing happened to blank it too.
|
|
|
|
### help::tests::no_help_row_spells_a_key_by_hand
|
|
|
|
improvise-wk18. The help pages had one kind of row whose keys the keymap never
|
|
saw: `literal("gg", …)`, `literal("yy", …)`, `literal("ZZ", …)`, and a dim tip
|
|
reading "press Tab or l/n". Every other key row comes off `keys_for`, so a
|
|
rebinding reaches it; these would have gone on saying `gg` after `g g` stopped
|
|
meaning anything, and no guard could see them because a string carries no
|
|
command name. At HEAD the test failed naming all four.
|
|
|
|
The fix is structural rather than a second scanner. The three prefix rows are
|
|
ordinary `keys(..)` rows now — `keys` also asks `Keymap::prefixed_keys_running`
|
|
for two-key sequences. The tip is **gone** rather than derived: made a `keys` row
|
|
in `ModeKey::Help`, it gave the Welcome page its one row with a description,
|
|
and `harness/tests/help.spec.js` ("? opens help, and its rows carry the keys and
|
|
the words") failed in CI on `welcome.every(row => row.description === null)` —
|
|
Welcome is prose by contract. The Help mode's own status hint already derives
|
|
`next-page` from the keymap, so the literal was a second table (improvise-rr8n's
|
|
rule: keep the one derived from the thing it describes).
|
|
`literal()` and `dim()` are gone; the only way left to build a
|
|
`HelpLine::Literal` is the `ex!` macro, which writes the leading colon itself, so
|
|
what remains hand-written is ex-command text — checked against the parser by
|
|
`every_ex_command_the_help_pages_show_is_one_the_parser_accepts`, which reads
|
|
exactly the text a user would type. This test is what stops a direct
|
|
`HelpLine::Literal { keys: "gg", .. }` coming back past the macro.
|
|
|
|
### help::tests::every_keys_row_the_pages_ask_for_finds_its_keys
|
|
|
|
A `keys` row whose lookup finds nothing is **dropped**, by design — a page never
|
|
advertises a key that does not exist. The cost of that design is that a broken
|
|
lookup looks like a shorter page: `every_help_page_key_row_names_keys_that_exist`
|
|
only checks the rows that survived, plus a floor of twenty. So this asks the
|
|
question from the other end, over `help_bindings()` — every row the builders
|
|
*asked for* — and requires each to find a key.
|
|
|
|
The injection: comment out the `prefixed_labels` extension in `keys`. The three
|
|
prefix rows vanish from the page, every other help test stays green, and this
|
|
fails naming `Normal jump-first-row`, `Normal yank` and `Normal wq`.
|
|
|
|
### help::tests::an_ex_row_can_name_placeholders_and_nothing_else
|
|
|
|
improvise-v29m, found reviewing PR #180 at 949810c. `no_help_row_spells_a_key_by_hand`
|
|
read only `HelpLine::Literal`, and its premise — that `keys()` is the only way to
|
|
make a key row — was a convention: `HelpLine::Keys { keys: Vec<String>, .. }`
|
|
was a public variant with public fields, and `help_bindings()` records only what
|
|
the builders *ask* for, so `HelpLine::Keys { keys: vec!["Q"], description: "Quit
|
|
instantly" }` typed straight into `panels()`, with `Q` bound to nothing, passed
|
|
the whole workspace. So did `ex!("q or press Q", ..)`: the parser test splits on
|
|
colons and `parse_line` accepts trailing words.
|
|
|
|
Both are now **compile errors**, which is why this test is not where the guard
|
|
lives. `HelpLine::Keys` carries a `KeyRow` whose fields are private to a module
|
|
whose one constructor, `KeyRow::looked_up`, does the keymap lookup; the injected
|
|
row fails with E0559/E0451. `ex!` evaluates `ex_usage_is_placeholders` in a
|
|
`const` item, and the injected usage fails with `evaluation panicked: assertion
|
|
failed: ex_usage_is_placeholders("q or press Q")`. The rule cost one row:
|
|
`:add-items <cat> a b c ...` had three bare words and is now
|
|
`:add-items <cat> <item> ...`. This test pins the rule the macro asks, in both
|
|
directions, including that shipped usage and the rival.
|
|
|
|
### help::tests::every_keys_row_drawn_is_one_the_builders_asked_for
|
|
|
|
improvise-ssoe, found in re-review of #180 at 544744c. `KeyRow` closed
|
|
hand-typed keys, but `KeyRow::looked_up` was `pub(super)`, so a page builder
|
|
could call it directly instead of through its `keys` parameter — and
|
|
`help_bindings()` records only what goes through the parameter. Injected into
|
|
`editing()`: `KeyRow::looked_up(n, "clear-cell", None, "Edit the current
|
|
cell")`, which captions `x` as editing the cell — the exact lie `jhdf`'s test
|
|
exists for — and the whole workspace stayed green, because both guards that
|
|
would object read `help_bindings()`.
|
|
|
|
Two changes. `looked_up` is now private to the `key_row` module, whose one
|
|
caller is the module's own `keys` — the lookup `pages()` hands every builder —
|
|
so the reviewer's injection is `E0624: associated function looked_up is
|
|
private`. That narrows the door without closing it: a builder can still reach
|
|
the module-level `keys` by path (`self::keys(..)`) rather than through its
|
|
shadowing parameter. So this test states the premise directly — the number of
|
|
keys rows drawn equals the number of rows the builders asked for (every asked
|
|
row finds its keys, by `every_keys_row_the_pages_ask_for_finds_its_keys`) —
|
|
and the `self::keys` bypass fails it `51 != 50`.
|
|
|
|
**improvise-aszt: a count is not the premise.** The test above compared
|
|
*counts*, so a bypass that also drops one real row keeps them equal. Injected in
|
|
`editing()`: call the parameter `keys(e, "clear-buffer", …)` and discard it with
|
|
`let _ =`, then return `self::keys(n, "clear-cell", None, "Edit the current
|
|
cell")` in its place. The asked row is recorded and never drawn, the drawn row
|
|
is never recorded, 50 = 50, and the whole workspace stayed green. The test now
|
|
compares **content in order**: the drawn keys rows must equal
|
|
`help_bindings()` looked up through the same `keys`, row for row. The same
|
|
injection now fails. What still passes is a bypass that draws exactly what the
|
|
builders asked for, in the order they asked — which is harmless by definition.
|
|
|
|
### tile_select_opens_on_a_regular_category_even_when_a_virtual_holds_row
|
|
|
|
improvise-52od. `opening_tile`'s rule is "the best-ranked tile", and in every
|
|
earlier fixture its top rank, the first Regular tile, was also the first tile on
|
|
Row. So the rival `position(|t| t.axis == Axis::Row).unwrap_or(0)`, which needs
|
|
no wire field at all, passed the whole workspace. The fixture is a records-shaped
|
|
bar: `_Index` on Row, `_Dim` on Column, `_Measure` on None, `Region` on Page and
|
|
`Product` on Row. The rule answers 3 (`Region`). The Row rival answers 0
|
|
(`_Index`), and a "first Regular on Row" rival answers 4 (`Product`), so all
|
|
three are separated. Injected: the Row rival fails this test.
|
|
|
|
### with_no_regular_category_tile_select_opens_on_measure_not_index
|
|
|
|
improvise-bqm0. #231's rule, `position(regular).unwrap_or(0)`, fell back to tile
|
|
0, which is `_Index`, when a view has no Regular category, which is exactly the
|
|
bug #231 fixed. The fixture is `_Index`, `_Dim`, a Label and `_Measure`. The rule
|
|
answers 3 (`_Measure`), then 2 (the Label) with `_Measure` removed, and 0 for an
|
|
empty bar. Injected: #231's rule answers 0 and fails. The Row rival fails too.
|
|
|
|
### a_frame_whose_group_level_names_no_category_does_not_deserialize
|
|
|
|
improvise-e265, ChatGPT Codex's P2 on #250. A `FrameGroup.level` arrives over
|
|
the wire, and `AxisFrame::groups` sizes its open-run stack with
|
|
`open.resize(g.level, None)`. A frame carrying `level: 4294967295` therefore
|
|
drove a multi-gigabyte allocation in the client instead of being refused. At
|
|
`e44b1491` the frame deserialized without complaint. The fix checks every level
|
|
against `cats.len()` as the frame arrives: `context/code/protocol.md` §3b.
|
|
|
|
The test serializes a well-formed two-category frame and checks that it reads
|
|
back equal. That is the control. It then injects two faults into the JSON, one
|
|
at a time: an oversized group level on the **second** marker, and a `GroupEnd`
|
|
one level past `cats`. For each fault it asserts two things. The mutated text is
|
|
still well-formed JSON. And the refusal names the level ("names no category"),
|
|
so the frame is refused for its level and not for its shape. The collapsed,
|
|
item-free frame keeps the fixture independent of `FrameEntry::Item`'s shape,
|
|
which #275 changes.
|
|
|
|
Rivals, each run:
|
|
- The `try_from` attribute removed: `unwrap_err` panics, because the frame
|
|
parses.
|
|
- Only the first marker checked (`.next().filter(..)` instead of `.find(..)`):
|
|
it fails the same way, because the first marker is in range.
|
|
|
|
### every_composed_cell_write_names_the_view_the_client_is_on
|
|
|
|
improvise-wpxr: a `SetCell`/`ClearCell` now names the view whose grid drew the
|
|
cell, because the host resolves an empty record against that view's grid. Every
|
|
existing `resolve.rs` fixture sits on `Default`, which is also
|
|
`ViewState::default().active_view`. So a composer that named a hard-coded
|
|
`"Default"`, or the host's default, passed all of them. This test puts the
|
|
client on `Other` and checks all three compositions: `clear-cell`, `paste` and
|
|
the Enter commit.
|
|
|
|
- **Rival** (`typed_cell_command` ignoring its `view` and naming `"Default"`),
|
|
run across the whole workspace: fails this test at the Enter commit and
|
|
`socket::tests::a_mouse_commit_names_the_view_its_editor_opened_on`, and
|
|
nothing else.
|
|
- **Rival** (`clear-cell` naming `"Default"`): fails **only** this test, at the
|
|
`clear-cell` assertion. `clear-cell` and `paste` build their command in
|
|
`resolve` itself rather than through `typed_cell_command`.
|
|
- **After the merge with gh/main `08ad7b81`,** the Enter commit writes the cell
|
|
its editor OPENED on (improvise-8xq0), so the test now opens the editor on
|
|
`Other`, moves the client to `Later`, and only then commits. The view comes
|
|
from `OpenedCell::view`, captured at open.
|
|
- **Rival** (the commit naming `state.view.active_view`, the view at
|
|
`Enter`), run across the whole workspace: fails **only** this test, with
|
|
`["Later"]` against `["Other"]`.
|
|
|
|
### a_quarters_end_leaves_its_year_open_on_the_same_level
|
|
|
|
improvise-felo.6: runs inside one category nest by `(level, depth)`. `Direct`
|
|
sits in `2024` after `Q1`'s `GroupEnd { level: 0, depth: 1 }`, and `Undated`
|
|
in nothing after `2024` ends. Rival: `groups()` closing by level alone, as it did
|
|
before depth existed. Only this test fails in `improvise-protocol`: every other
|
|
frame fixture puts one group per level.
|