# Conflicts: # context/test-evidence/clients.md # context/test-evidence/command.md # crates/improvise-wasm-client/tests/lockstep.rs
335 KiB
improvise-command — test evidence
Why each of these tests exists, what bug it reproduced, and what fault was
injected to prove it can fail. Keyed by exact function name; a rename
orphans the entry, and scripts/check-comment-runs.py --check-evidence reports
that (improvise-9c8c, design-principles §7).
Entries cover tests and the helpers whose shape is the evidence.
ui/reduce.rs
a_view_slice_command_does_not_move_the_authoritative_view_state
This test used to assert the opposite, under the name
view_slice_commands_land_in_the_authoritative_view_state, and it passed against
a path no host walks (improvise-s17o): apply_command's fallback arm folded a
wire view command into app.view_state, the authoritative slice. Nothing in
production reached it — Sessions::apply_checked folds a view command into the
asking client's mirror — so the behaviour it pinned was wrong and safe only
because it was dead. The model is shared, the UI is not (principle 5, gotcha 36);
a client's SetStatus must not move the host's status line.
The fixture writes "host" into the status line first, so the assertion
distinguishes "the fold was removed" from "the field happened to be empty" — with
the default empty string, deleting the fold and leaving it in place look the same
for SetStatus. Restoring reduce_view(&mut app.view_state, other) fails on the
first field.
Helper — checked
Not a test — the fixture every reduce.rs test below reaches the door through.
For the tests whose subject is not staleness: a command the current state
accepts, which every command built by caught_up is by construction. The tests
whose subject is staleness call Checked::against themselves and assert on the
Rejection, so neither kind can pass by accident through the other's
convenience.
switch_view_moves_the_client_and_leaves_the_documents_default
The successor to switch_view_moves_the_document_and_the_client_together, which
asserted the pre-improvise-jytm lockstep. Which view a client is on is now that
client's; Workbook::active_view is the default a fresh client opens and what
.improv writes. The fault it rules out is the old behaviour returning by inertia
— any writer that moves the workbook's copy from a client's switch fails it.
Fault injected 2026-09-20: restoring App::switch_to_view's
workbook.switch_view(name) call fails this and
the_documents_default_view_outlives_this_clients_switches.
a_wire_view_switch_records_history_the_way_a_key_switch_does
A wire-driven view switch owes the same history a key-driven one leaves behind.
effect::ViewBack pops view_state.view_back_stack, and the only thing
that ever pushes onto it is effect::SwitchView. While apply_command performed
the switch itself — calling App::switch_to_view directly rather than running
the effect — the stack stayed empty, so a client that switched over the wire
could not go back, though the same two keypresses could. Wave 4 is what made that
reachable: before ModelCommand there was no wire way to ask.
Nothing else in the suite could see it.
switch_view_moves_the_client_and_leaves_the_documents_default checks where a
switch landed; this checks what it recorded.
Bug demonstrated 2026-09-19 (Copilot, PR #11): the ViewBack left the document
on "Second". Since improvise-jytm the step back is driven through effect::ViewBack
rather than ModelCommand::ViewBack, which left the wire with that ticket — the
stack it pops is one client's history, so a nullary wire command could not address
it (gotcha 38). What the test asserts is unchanged.
switching_views_projects_the_axis_widgets_only
Switching views changes what the axis-bearing widgets read, so they are told; the list of views itself did not change, so it is not.
an_inbound_projection_is_ignored
Projections travel one way. A client that sends one back must not be able to write it into the server's state.
a_keypress_that_changes_the_model_projects_it
The key path is the model's door, and it reports the widgets it touched so a host can project them.
a_view_only_keypress_projects_nothing
A keypress that moves only this client's cursor reaches no other client (gotcha 3).
every_model_command_names_an_effect_that_declares_a_model_change
The derivation, stated as a check. This is what "derived, not invented" means
operationally (doctrine(derive_the_enum_from_the_effects)). A variant pointed
at a ViewOnly effect would be a wire message that changed the document for the
client that sent it and for nobody else — Dirty::of would report NOTHING, and
no projection would ever correct the others (gotcha 35).
Helper — model_app
Not a test — the fixture the three every_model_command walks run against.
app() with a cell actually stored, so a surrogate for it resolves through the
index rather than only through the layout. The walks over every_model_command
run against this: a SetCell whose record cannot be resolved produces no
effects, and every one of those tests asserts on the effects, so the fixture has
to name something real or they would all fail for a reason that is not theirs.
no_model_command_commits_another_clients_session_local_state
improvise-3x3u: the wire vocabulary is the addressable model effects, not all of them.
Sessions::apply routes a Command::Model with no SessionId, which is right
exactly when the change is addressed to no one session (gotcha 36).
ApplyAndClearDrill and AcceptWizardModel are addressed to precisely one, and
it is not the sender: an unaddressed ApplyAndClearDrill commits another
client's private typing (improvise-tom1) and an unaddressed AcceptWizardModel
swaps the document out of a dialogue the sender never opened. Both were
demonstrated against Sessions::apply before the variants were removed.
Re-adding either variant means making it addressed — carrying a SessionId,
with the staging map moved onto the session — not restoring the nullary form this
test forbids.
SESSION_LOCAL_EFFECTS is the list of the two effect names.
the_session_local_effects_are_real_model_effects
The guard above is only worth having while the effects it names still exist and
are still model changes — otherwise it would pass by naming nothing
(doctrine(check_the_checker)).
the_effect_name_reader_reads_all_three_debug_shapes
The reader above is itself a check, so it gets the injection too
(doctrine(check_the_checker)): one effect of each Debug shape, because the
leading identifier is followed by a different delimiter in each — nothing, (,
and {.
Helper — effect_type_name
Not a test — the reader that test injects against, used by the two guards above.
The type name of an effect, read off the leading identifier of its derived
Debug. Effect is a trait object, so std::any::type_name is not reachable
through it; every effect derives Debug, and a derived Debug opens with the
type's own name for a unit struct, a tuple struct and a braced struct alike. So
the name is already on the value — this only reads it back.
every_model_command_names_the_effect_of_the_same_name
The mapping is name-for-name by construction, so state that: every
ModelCommand variant names the effect that carries its own name.
This is the check that separates an effect from its inverse.
every_model_command_names_an_effect_that_declares_a_model_change cannot:
HideItem and ShowItem, AddItem and RemoveItem, CreateView and
DeleteView, SetCell and ClearCell are each two effects of the same
EffectKind, so a swapped pair clears that bar while inverting what the wire
message does. And a_model_command_lands_where_its_keypress_lands cannot reach
them either — most variants have no single keypress to compare against, so it
drives three of twenty-eight.
Proven by injection 2026-09-19: HideItem → effect::ShowItem, CreateView →
effect::DeleteView and AddItem → effect::RemoveItem all left
cargo test --workspace at 918 passing, 0 failing. This test fails on each.
The name-for-name rule is a design constraint, not a coincidence, and it is what
makes the derivation checkable at all
(doctrine(derive_the_enum_from_the_effects)): a variant whose effect must be
named something else is a variant that stopped being derived, and it should fail
here and be argued about rather than pass quietly.
the_enumeration_names_each_variant_once
The every_model_command list covers the enum once. A duplicated entry would
leave a variant untested while the count still looked right.
Helper — every_model_command
Not a test — the enumeration that test, and the three walks above it, cover.
One value per ModelCommand variant. The enum is the derived set, so a test that
walks "the set" has to walk all of it. variant_name is the completeness guard:
it matches exhaustively, so a variant added to the protocol fails the build here
until it is listed in this vector as well.
a_model_command_lands_where_its_keypress_lands
The point of the whole ticket: a model Command applied authoritatively leaves
the document exactly where the keypress bound to the same mutation leaves it.
That equivalence is what lets the two entry points share one door.
The Dirty is compared too, because a host with connected clients projects off
it: a command that produced the right document and the wrong Dirty would leave
every other client stale.
Its pivoted() fixture has two categories on different axes, because the
one-category fixture cannot tell a transpose from a cycle: with only Region on
Row, swapping the axes and rotating Row→Column land on the same view, and an
equivalence test that cannot distinguish the wrong effect from the right one
proves nothing (doctrine(check_the_checker)).
The fixture needs cells and multi-entry view maps too (improvise-djcn). The
test compares format!("{:?}", workbook) across two Apps. It passed only
because the fixture had no cells and every view map had at most one entry, and a
HashMap of one entry has only one order. The fixture now carries:
- three cells;
- two page selections;
- two categories with hidden items, one of them with two;
- two categories with collapsed groups.
That fixture was red in 3 of 3 runs on the old Debug impls: the by-key and
by-command fingerprints differed only in hash order. It was green after the fix.
Each half of the fix was checked by restoring the derived Debug on its own:
View's derivedDebugalone: 3 of 3 runs red.DataStoreandSymbolTable's derivedDebugalone: 3 of 3 runs red.
So neither half is carried by the other. The exhaustive destructure in Model's
Debug has no test of its own, because it is a compile-time guard. It was
checked by adding a probe_field to Model and to Model::new. The build then
failed with E0027: pattern does not mention field probe_field at the Debug
impl, and nowhere else.
a_stale_wire_model_command_cannot_be_applied
A wire Command::Model cannot reach the document without its basis being checked
(ADR 001, improvise-ondj).
Bug demonstrated 2026-09-19: reduce_full and apply_command — the functions
this module's own doc points every future host at, the browser worker included —
took a &Command and matched Command::Model { cmd, .. }, so a batch composed
against a document that had since moved landed anyway. The .. discarded the
basis with no compile error.
The regression guard is the signature, not this test. Both now take a
Checked, whose only constructor runs stale_basis, so the version of this test
that reproduced the bug — reduce_full(&mut app, &wire) with a stale wire — no
longer compiles, and reduce_full itself no longer exists (improvise-s17o: having
no caller was the condition that let the defect hide in the first place). What is left to assert is that the constructor refuses, and that
refusing leaves the document alone: there is no half-applied batch, because the
type is obtained before anything is applied.
a_model_commands_dirty_comes_from_its_effects_kind
The Dirty a model command reports is the one its effect declares, not a second
table kept alongside the enum (gotcha 35).
a_wire_model_command_marks_the_document_dirty
improvise-gv9r. The bug: a client's edit left the authoritative document reporting clean, on every transport.
MarkDirty used to be a separate effect that the command layer composed
alongside the mutation, and improvise-kmgs derived ModelCommand from the
effects one at a time — so the wire got SetCell without its companion. A
derivation correct per element and wrong for a set. "Save if dirty" was therefore
a trigger that never fired, and a host built on it loses every client edit
silently; the browser worker found that out the hard way and had to
over-approximate off the command variant instead.
The fix is the unrepresentable one: dirtiness is raised by App::apply_effects
off the same EffectKind the Dirty and the two basis tiers are read from
(gotcha 40), so no path can compose a document change without it. There is
nothing left to forget, and MarkDirty is gone rather than left as a second
answer.
The three levels are asserted separately because they are three different
effects: an effect that is only ever reached by one of them would otherwise pass
on a sibling's evidence. The negative arm — that a ViewOnly effect leaves the
document clean — is a_view_only_batch_leaves_the_document_clean in ui/app.rs,
and it has to live there: a view-slice Command never reaches apply_effects at
all (apply_command folds it straight into ViewState), so a negative written
against this door would pass however unconditional the raise was.
a_wire_view_switch_leaves_the_document_clean
The inversion of a_wire_view_switch_marks_the_document_dirty, which asserted the
pre-improvise-jytm behaviour. A switch used to move Workbook::active_view, which
persists as the Initial View line, so it really did put the document out of step
with the file. It no longer moves any document state, so there is nothing to
persist and the browser worker's persist trigger (improvise-gv9r) must not fire.
The direction matters: the worker asks the document's dirty flag rather than a
table of variants, so a switch that dirtied spuriously would cost an OPFS rewrite
of the whole document per keystroke (gotcha 41). This is the negative arm — the
positive one, that a real model command does dirty, is the loop above it.
The consequence to keep in view is a product one, filed as improvise-azhb rather
than decided here: a TUI user who switches view and saves now writes the
Initial View the document was loaded with.
ui/app.rs — the view history from the keyboard
the_view_history_walks_both_ways_from_the_keyboard
improvise-htl6: view-forward was bound in no keymap, in either front end, so
the forward half of the view history was reachable only as :view-forward in
the TUI and not at all in the browser. , now binds it. Reasoning:
context/code/tui.md, ", walks the view history forward".
This is the TUI half of the claim and it is deliberately end-to-end through
App::handle_key rather than a keymap-table lookup: the table is what was
wrong, so a test that reads the table cannot be the only witness. It switches
views, presses <, asserts it landed back, then presses , and asserts it
returned.
< is the control and it is load-bearing. Without it, a keymap that bound
nothing would fail on the same assertion as one that bound , wrongly, and
the two are different defects. The rival injected is , bound to
view-back — the misspelling the sibling test's control was originally written
against — which leaves the active view on Default because the back stack is
empty by then, and fails here and in
resolve::tests::the_forward_step_mirrors_its_cmd_and_one_key_reaches_it.
The browser half needs nothing: the keymap is the shared table both ends read,
and resolve.rs has had the view-forward arm since improvise-cfly.
ui/effect.rs
a_switch_the_document_refuses_leaves_the_back_stack_alone
improvise-swzk: SwitchView pushed the leaving frame onto view_back_stack
before calling the guarded App::switch_to_view, so :view NoSuchView buried a
real history frame under a frame naming the view the user was already on. <
then returned to where it already was. Reasoning: context/code/command.md,
"SwitchView — the history records a move that happened".
The test switches to a real second view, then to a name the document has not
got, asserts the back stack is still one deep, then presses < and asserts it
lands on the first view.
Two faults injected, and the second is the one the brief asks for. Making the
push unconditional (if true) fails this test and
switch_view_to_same_does_not_push_stack. Suppressing it entirely (if false) —
the over-reach a fix aimed at this bug could plausibly commit — fails eleven
tests including this one's own first assertion, which is why the assertion that
one frame survives is here and not only the assertion about where < lands.
The fixture is load-bearing and was measured, not assumed. A variant with no
real second view — refuse a switch from Default, then < — passes against
the unfixed code, because the phantom frame it pops names Default and
Default is where you already are. That is the vacuity review demonstrated
twice on PR #48, and the ticket warned about it: a bystander sitting on the view
it would land on anyway cannot see this.
the_document_surface_runs_on_a_non_filesystem_backend
improvise-fuj: every effect that touches a document goes through App::storage,
so the whole save/load/import/export surface works on a backend that is not a
filesystem. This is what a browser worker supplies (improvise-i34); here
MemoryFs stands in for it, and the test would fail if any of these effects
reached for std::fs.
view_only_effects_leave_shared_state_alone
improvise-mae: a declared kind() is only worth having if it matches what the
effect actually does. Rather than restate each declaration, this applies the
effect and checks the shared state it claims not to touch is genuinely unchanged
— a ViewOnly effect that quietly wrote a cell would never reach connected
clients.
No exemptions: since improvise-8uz moved the cursor out of View,
SetSelected/SetRowOffset/SetColOffset touch nothing shared, so every effect
in the list must clear the same bar.
structural_effects_declare_their_kind
The structural effects, each pinned to the kind it must declare.
The bug this guards: AddCategory and RemoveCategory declared CellData, but
both go through Workbook, whose on_category_added / on_category_removed
rewrite every view's axis map. That is a re-layout, so a client told only
"cell data changed" would keep rendering the old grid shape. The two "actually
change" tests beside it cannot see this — a category change moves model data
and view config, so it passes either list. Only pinning the declaration does.
add_category_past_the_limit_reports_the_failure
Bug (improvise-djn): AddCategory discarded the Result from
Workbook::add_category, so pressing n in the category panel with the
12-regular-category limit already reached reported the optimistic
Added category "X" its command had queued and added nothing. The effect runs
last precisely so its failure can overwrite that message (improvise-6t3) — it
just never wrote one.
add_formula_adds_target_item_to_category
Regression: AddFormula must add the target item to the target category so the
grid layout includes cells for it. Without this, a formula like
Margin = Profit / Revenue would be registered but invisible in the grid.
abort_chain_short_circuits_apply_effects
AbortChain must cause subsequent effects in the same apply_effects batch to
be skipped, and the flag must reset so the next dispatch starts clean.
save_without_file_path_aborts_the_chain
BUG (improvise-8tk): Save reported failure only through the status bar, so the
Quit that wq/ZZ chains after it landed anyway and the whole session was
discarded without the user seeing the error. A failed save must abort the rest of
the batch.
failed_save_as_aborts_the_chain
BUG (improvise-lae, found reviewing it): improvise-8tk fixed one arm of a rule
with two. Save aborts the batch when the write fails; SaveAs reported the
error to the status bar and let the batch continue, which is the same silent
discard 8tk closed — the shape
doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only) names,
and the three tests above it are evidence about Save only.
It is reachable today: the : line composes with ., so :w /bad/path . q
builds exactly this batch, and ExecuteCommand concatenates the two commands'
effects into one. improvise-lae's save prompt also commits to SaveAs, so the
arm was about to carry more traffic.
Note what the abort does and does not buy the prompt: the typed path survives for
the user to correct, but the message does not reach the screen, because
draw_bottom_bar draws the minibuffer instead of the status line while a prompt
is open. That is pre-existing and reported separately; the data-loss half is what
this fixes.
successful_save_as_lets_the_chain_continue
The positive arm: a SaveAs that succeeds must not abort, or every :w <path>
would swallow whatever followed it. Without this, "always abort" passes
failed_save_as_aborts_the_chain.
enter_edit_at_cursor_uses_target_mode_not_app_mode
EnterEditAtCursor must use its target_mode field, not whatever
app.view_state.mode happens to be when applied. The previous implementation
branched on app.view_state.mode.is_records(); the parameterized version trusts
the caller (keymap or composing command).
enter_edit_at_cursor_reads_post_batch_layout
improvise-ehz: the edit buffer must be loaded from layout state that already
reflects earlier effects in the same batch. apply_effects only rebuilds after
the batch, so a preceding RebuildLayout is what makes the cursor resolve to the
post-mutation cell. Removing the row-0 item slides a different cell under the
cursor; the buffer must show that cell's value, not the pre-batch one.
set_buffer_does_not_special_case_the_search_name
SetBuffer no longer treats "search" as an alias for the search query
(improvise-gdl): it is a plain buffer write whatever the name says, and the query
is reached through SetSearchQuery. Commands pick between the two in
cmd::core::write_buffer.
apply_and_clear_drill_coord_rename_moves_every_measure_in_the_row
improvise-c5v: in wide format a records row groups one cell per measure, so renaming a coordinate has to move every one of them. The long-format code moved only the cell under the cursor, which would now split the row in half.
import_headless_csv_short_rows_surface_warning_in_status
BUG (improvise-k8i): ImportJsonHeadless parsed CSVs with parse_csv, which
sends short-row warnings to stderr — invisible inside the TUI. Short rows must
surface in the status message, naming the affected column, never silently
succeed.
Note — the drill tests drive apply_effects, not apply
Several ApplyAndClearDrill tests call app.apply_effects(vec![...]) rather
than effect.apply(&mut app). That is deliberate: the dirty flag is raised off
the EffectKind now (improvise-gv9r, gotcha 45), so driving the effect directly
would assert a flag nothing in production sets.
every_file_writing_effect_aborts_the_batch_on_failure
improvise-z05r, the third instance of one rule: improvise-8tk fixed Save and
not SaveAs; PR #27 found SaveAs; ExportCsv was the arm still missing, so a
Binding::Sequence or a : line composing an export with anything after it
continued past a failed write.
The fix is structural rather than a third patch: all three now report through one
report_file_write(app, outcome), which writes the status message either way and
sets abort_effects on Err. That is not sufficient by itself — a fourth writer
could still be written without calling it — so the test names the three, and a
fourth has to come here and be named. Naming is the mechanism; the shared function
is what makes the naming cheap to honour.
Each case drives App::apply_effects with [writer, ChangeMode(Quit)] against a
path under a directory that does not exist, and asserts both last_batch_aborted()
and that the trailing mode change never landed.
Injected fault: drop app.abort_effects = true from report_file_write. Fails by
name, along with the four older abort tests — which is the point: they and this one
now share one mechanism.
Injected fault, narrower: route ExportCsv around the reporter and write its status
directly. Only this test fails, naming ExportCsv, with 460 passing — the exact
state the ticket describes.
a_batch_that_writes_nothing_leaves_the_document_clean
improvise-748f. ViewNavigate { forward: false } emits ApplyAndClearDrill
unconditionally, and that effect returns immediately when drill_state is None.
It declares Model(CellData) regardless — Effect::kind takes no App, so the
classification cannot be state-dependent — so apply_effects raised
model_state.dirty on a document nothing had touched. Press view-back, press :q,
and the quit is refused; autosave arms thirty seconds later.
The older test here, start_drill_and_apply_clear_drill_with_no_edits, pinned the
wrong behaviour in an assertion message arguing it was the same over-approximation a
failed LoadModel gets. It is not: a succeeding LoadModel leaves a document that
really does differ from the file its path names, and that remains declared. This was
a wrong answer to no divergence at all.
a_cell_data_effect_no_raw_cell_records_still_dirties_the_document
The guard against the cure improvise-748f proposed, which is unsound. The ticket
suggested raising dirty off bump_changed_records' per-record fingerprint of the
raw cells, on the reading that CellData means cells. Five of the ten CellData
effects change the document without writing a raw cell: AddItem, AddItemInGroup
and SortData (which reorders — and .improv carries the order), and AddFormula
and RemoveFormula, whose results live in Model::formula_cache and not in the
store. Observing raw cells would have left every one of them reading clean, which is
a lost edit — the opposite direction from the spurious prompt it was fixing.
So the observation is document_content — document_config plus the cells in
iteration order and the formulas — and only the CellData arm reads it. The
LayoutGeometry arm keeps the narrow document_config. A first version pointed both
arms at the wide fingerprint, and review benchmarked what that costs at 18,000 cells
in release over 200 batches after warm-up: one SetCell batch 8.07 ms → 10.21 ms
(+27%, a constant factor on a path that was already O(cells) through
record_fingerprints), but one SetNumberFormat batch 78 µs → 2.63 ms, 33× —
an asymptotic change from O(config) to O(cells + config) on the held-key path that
[/], axis cycle, hide/show, group toggle, transpose and prune toggle all take.
~380/sec where they had been ~12,800/sec. The split costs no correctness because all
five blind effects are CellData; that every LayoutGeometry effect really is
visible to the narrow fingerprint is asserted separately, and not left as a reading.
WholeWorkbook stays declared, and that is the remaining over-report: a LoadModel
whose incoming content happens to be identical still marks the document dirty, and a
failed one does too. Both cost one redundant write.
The SortData case needs two cells inserted out of sorted order, or it reorders
nothing and its case asserts the opposite of what it names. The AddFormula case
removes the fixture's formula first, or adding it is a no-op.
Injected fault: narrow document_content back to the configuration it hashed before.
Fails naming SortData, with five other dirty-flag tests.
every_layout_geometry_effect_is_visible_to_the_narrow_fingerprint
The guard that makes the two-fingerprint split safe rather than a trap. Splitting
CellData (wide) from LayoutGeometry (narrow) is only sound while every effect
wearing LayoutGeometry writes something document_config hashes; an effect it
cannot see would leave the document reading clean forever, which is the same
lost-edit failure the wide fingerprint exists to prevent, arriving by the other door.
The thirteen are enumerated by brace-matching the impl Effect blocks, not by
grepping — the naive grep reports fourteen, picking up HelpPageSet, which is
ViewOnly and is simply the last pub struct before #[cfg(test)] (gotcha 38 names
this exact artefact). Each case asserts its effect really declares
Model(LayoutGeometry) before asserting anything else, so a reclassified effect fails
here rather than silently leaving the list unrepresentative.
Four cases carry a prepare step run directly against the workbook and not
through an effect, because the interesting state is the precondition. What makes such
a case honest is the no-op-fails property: without its precondition the effect
writes nothing, an unwritten document leaves the fingerprint unmoved, and the case
therefore fails rather than passing vacuously.
Measured, by deleting each prepare in turn, two of the four are load-bearing in
exactly that way and two are not:
DeleteViewneeds a second view; without it,dirtyreads false and the case fails.ShowItemneeds a hidden item; without it,dirtyreads false and the case fails.ToggleGroupis not load-bearing here.ToggleGroup::applycallstoggle_group_collapseunconditionally — its membership check gates onlyreset_cursor— socollapsed_groupsmoves whether or not the group is real, and the case passes with thepreparedeleted. The step documents the realistic input; it is the cursor arm, tested elsewhere, that needs a real group.TransposeAxesis not load-bearing either, and its step is literally a no-op:View::on_category_addedalready assigns the first regular categoryRowand the secondColumn, so the fixture arrives withType/Monthdiffering. The step states the precondition rather than establishing it, and the case passes without it.
dirty is cleared after prepare and before the effect. That clear is defensive
hygiene for a future prepare that goes through apply_effects, and it is not what
makes any case honest today: dirty is raised only inside App::apply_effects,
and no prepare in this test runs an effect, so no preparation can set it. Demonstrated
by removing the clear and blinding document_config to 0 together: all thirteen
cases still read dirty == false. An earlier version of this entry claimed the
opposite — that without the clear "all four cases pass against any fingerprint" — and
measurement contradicts it.
Injected fault: document_config returns 0. Fails, with five other dirty-flag tests
across three files. Injected fault, narrower and the one that proves the test is
pointed at the right thing: drop view.number_format from document_config. Fails
naming SetNumberFormat, with only one other test failing anywhere.
ui/app.rs
model_state_and_view_state_are_constructible
improvise-3vr: ModelState and ViewState are the named slices of App state
introduced by the vb4 refactor. Step 1 only requires that the types exist and are
constructible; subsequent steps move fields in.
Note — the ViewState: Send static assertion
improvise-h0b: ViewState is the portable slice — the one thing serialized into
the Snapshot a newly-connected client receives (improvise-q08). Send is the
cheapest compile-time proxy for "portable": a field that cannot cross a thread
boundary certainly cannot cross a serialization one. Before h0b this failed to
compile, because drill_state: Option<DrillState> holds an Rc<Vec<RecordRow>>.
Session-local state lives on App instead. It is a const _: fn() in the test
module, not a named test, so it has no heading of its own.
app_view_state_owns_ui_session_fields
improvise-ew0: ViewState owns the UI session slice — mode, status, search,
panels, navigation, drill, yanked, buffers. App accesses them through
view_state.
view_state_survives_a_json_round_trip
improvise-a2x: ViewState is what a newly-connected client receives as a Snapshot
(improvise-q08), so it has to survive a round trip through the wire format.
Populated with non-default values in every field family — a mode carrying a
MinibufferConfig, a navigation frame, a yanked cell, a keyed buffer, an armed
prefix — because a roundtrip of ViewState::default() would pass while proving
nothing.
the_documents_default_view_outlives_this_clients_switches
The replacement for the_two_active_view_names_never_diverge, and it is a
replacement rather than a deletion on purpose: improvise-jytm makes the two copies
diverge deliberately, and a deliberate split with no guard is how one silently
regresses into the other.
What it pins, all four of which have a silent failure mode. A switch moves the
client's copy and not the document's default. App::view() — what
rebuild_layout and cmd_context read — follows the client, so the split is
observable in the layout and not merely in a string. A switch to a view the
document does not have moves nothing, because the slice keys ViewState::cursors
and grid_in indexes the workbook by it. And install_workbook still seats this
client on the incoming document's default and clears the cursor map, since the
outgoing document's view names mean nothing in the new one (gotcha 24).
Fault injected 2026-09-20: App::switch_to_view calling
workbook.switch_view(name) again fails the second assertion.
moving_the_cursor_does_not_touch_the_shared_workbook
improvise-8uz: moving the cursor must leave the shared document untouched, so that
one client scrolling cannot move another's viewport. Before the move these three
effects wrote into model_state.workbook, and this assertion would have failed.
app_owns_the_session_local_fields_not_view_state
improvise-h0b: the two session-local fields moved off ViewState and onto App.
Each fails both membership tests: ImportWizard is an improvise-io type (not
portable) and a half-open dialog (not snapshot-safe); DrillState holds an Rc
(not portable — see the Send assertion) and stages uncommitted edits (not
snapshot-safe).
staged_drill_edits_are_unreachable_from_view_state
improvise-h0b: a fresh client must not inherit another session's staged,
uncommitted drill edits. Those edits are unreachable from ViewState, so a
Snapshot built from it cannot carry them.
switching_views_restores_each_views_own_cursor
improvise-8uz: the cursor is per view. Written as a characterization test
against the old View-owned fields and kept verbatim across the move to
ViewState::cursors, because a refactor about where state lives must not change
what the app does.
the_improv_format_does_not_persist_the_cursor
improvise-8uz: the ticket asserted these fields were "PERSISTED in .improv". They were not — the grammar has no cursor production and the writer emitted none. That is what made the move free: there was no restore-where-you-left-off behaviour to lose, and no format change to make. The assertion still guards the format going forward.
entering_records_mode_sorts_existing_data
Pressing R to enter records mode sorts existing data by CellKey, so display
order is deterministic regardless of insertion order.
Note: this test's doc comment carried a stray first line from a neighbouring
test ("Regression: pressing o in an empty records view should create the") that
was never finished. It was dropped in the comment-cap wave; the sentence above is
what the test actually asserts.
the_new_category_column_creates_a_category_and_its_column
improvise-2zq step 2: the +cat column creates a category and its column, without
leaving the grid.
Driven through real keys. The assertion that earns its place is the last one: the
new column is usable immediately, which is the whole point —
records_columns filters on kind.is_regular() and never requires items, so a
category with no items is a column the moment it exists.
enter_on_plus_cat_in_the_last_row_does_not_reopen_category_creation
improvise-2zq, found by Copilot on PR #27: creating a category from +cat shifted
the grid under the advance that followed it.
add_category inserts the new coordinate column immediately before +cat, so
every column index from +cat rightwards moves by one. CommitAndAdvance then
computed its advance from ctx, whose layout is the pre-commit one (gotcha 23:
apply_effects rebuilds only after the whole batch). On the last row Enter
wraps to the next column — and the index it wrapped to was +cat again after
the rebuild.
So the failure is not "lands one column off". It reopens category creation: a
user naming two categories in a row never reaches the column they just made, and
gets an unbounded +cat loop instead. The last row is the arm that wraps, so it
is the arm tested.
tab_on_plus_cat_does_not_reopen_category_creation
The same stale index reached Tab, which advances right rather than wrapping.
Both directions read the pre-commit layout, so both are covered — one arm of a
two-arm rule is the shape
doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only) names.
the_new_category_column_creates_the_column_but_writes_no_coordinate
Committing the +cat name creates the column and does not write the
coordinate on the row the cursor was on.
The deliberate answer to the ticket's third question, and the reason is gotcha 1:
setting a coordinate moves every measure cell in the row. That is a data
migration across N cells, and attaching it to the same Enter that made a schema
change — in an app with no undo — is a bigger single keystroke than the measure
+ asks for. The follow-up edit is one column away.
the_new_category_column_refuses_past_the_limit_where_the_user_can_see_it
MAX_CATEGORIES is a reachable refusal on this path, and the refusal has to be
visible.
draw_bottom_bar renders the minibuffer instead of the status line, so a message
written while an editor is open is never drawn. The commit path therefore drops
out of editing before running the add, and AddCategory aborts the batch on
refusal so the trailing EnterEditAtCursor cannot re-open the editor over the
message.
The limit itself is checked in exactly one place — Model::add_category — and
nothing here restates it; the test fills the document to the limit by asking for
categories until it refuses.
naming_a_category_in_records_mode_does_not_eject_the_user
improvise-2zq step 1: naming a category in records mode used to eject the user from the grid they were standing in.
Records mode is _Index on Row and _Dim on Column, with every regular category
on Axis::None. View::on_category_added seated a new regular category on the
first axis no regular category held — Row — and bumped the virtual out of the
way to get there. So N, a name, Enter moved _Index off Row, records
detection stopped matching, and the grid silently became a pivot.
Driven through the real keys rather than through on_category_added, because that
is how the bug was found and it is what makes this evidence rather than a
restatement of the fix.
i_and_o_diverge_on_an_empty_records_grid
improvise-6sq: i and o want opposite behaviour on an empty records grid, which
is why edit-or-drill and enter-edit-at-cursor cannot be merged into one
command. Entering records mode puts every regular category on Axis::None, so
EditOrDrill's aggregation check is true unless the cursor is on a synthetic
record — and on an empty grid there is none, so i reports "cannot drill". o
must still open an edit there (see
add_record_row_in_empty_records_view_creates_first_row), so routing it through
the same drill check would break it.
improvise-ct5 changed the message and deliberately not the divergence. The
outcome — i opens no editor where there is nothing to edit — is right, and o
one key away is the affordance for adding a row. What was wrong is that the
message said "cannot drill", a pivot-mode concept in a mode whose model is "a
table I type into". Making i behave like o here (the ticket's option (b))
would collapse exactly the divergence improvise-6sq exists to record, so the
assertion still pins that i does not enter editing; only the wording assertion
moved, and it now also names what must not appear.
the_empty_grid_message_is_not_the_records_one_in_pivot_mode
The negative arm of improvise-ct5, and the reason the wording is keyed off the mode the keymap supplies rather than reworded once.
Without this, "say the records sentence everywhere" passes
i_and_o_diverge_on_an_empty_records_grid — and it would promise o, which adds
no row in pivot mode, trading one misleading message for another.
The fixture is an empty pivot grid that still counts as aggregated: Region on
Row with no items makes cell_key() None, and Extra cycled to Axis::None is
the regular category that makes EditOrDrill take the drill branch.
blank_model_bootstraps_its_first_measure_from_the_new_measure_column
A blank model has no categories and no measures, so wide format shows exactly the two creation columns. Typing a name into the new-measure one creates the measure, which gives it a real column to type values into — records mode stays self-sufficient for onboarding (improvise-c5v).
improvise-2zq widened that: a blank grid now offers +cat beside +, so both
halves of a schema can be bootstrapped without leaving the grid. This test still
pins the measure half; the category half is
the_new_category_column_creates_a_category_and_its_column.
Inside it, the cursor lands at column 0, which in a blank grid is now +cat
rather than + (improvise-2zq) — a record needs coordinates before it needs
measures, so that order is deliberate. o leaves the test editing column 0, so
the Esc before moving to the new-measure column is load-bearing.
ctrl_w_prompts_for_a_path_and_a_failed_write_reports_it_where_it_shows
improvise-lae end to end: Ctrl-W, a path, Enter.
The failing case is the interesting one, and it pins an ordering (found by
Copilot on PR #27). SaveAs aborts the batch on a failed write, so whatever
CommitSave emits after it is skipped. A first version emitted
[SaveAs, ChangeMode(Normal)], which left the app in SavePrompt — and
draw_bottom_bar renders the minibuffer instead of the status line, so the
"Save error:" the effect had just set was never drawn. The save appeared to do
nothing whatsoever.
Leading with the mode change puts the error in Normal, where the bottom row is
the status bar. This test therefore asserts the mode left the prompt, which is
the opposite of what it asserted before — deliberately, and it is the only thing
keeping the two effects in that order.
every_minibuffer_mode_entered_from_records_returns_to_records_normal
Copilot round 2 reported that Ctrl-W from a records grid returns to Normal,
losing the records keymap while the layout is still a records layout. It is right,
and it is a class: every minibuffer mode reachable from RecordsNormal exited
to Normal, because the exit was a constant baked into
TextEntry::new(..., "normal") and into each commit command.
The class predates SavePrompt: : and Ctrl-E do exactly the same thing
and always have. That is why the fix was not "make SavePrompt remember its
origin" — that would leave two of the three arms broken, which is the shape
doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only) names.
The binding table is not involved and was checked rather than reasoned about: an
IMPROVISE_BINDING_DUMP diff shows RecordsNormal gains Ctrl-W and loses
nothing, so with_parent inheritance (gotcha 13) is intact. What was lost is the
runtime mode.
improvise-sihr fixes it by observation rather than memory, which is what
took the exit out of the keymap entirely: App::apply_effects calls
settle_normal_mode after rebuild_layout, so the mode a batch ends in
matches the grid it ends on and nothing carries an entry mode. The ticket
proposed the opposite — carrying the entry mode on MinibufferConfig — and that
was declined for a concrete reason beyond doctrine: carried generically it would
also return CategoryAdd to CategoryPanel, a behaviour change nobody asked
for, while carried for only three modes it is the one-arm shape again.
This test still drives both exits — Esc and Enter — where the original drove
only Esc, because they reach the mode through different effects
(ChangeMode and ReturnToNormalUnlessMoved) even though one settle now covers
both.
The first version of the fix resolved per-effect and was wrong in a new
direction, which the adversarial review of PR #91 caught. Set at the moment
the mode is written, the resolution reads the batch's opening layout (gotcha
23) — so a batch that crosses settled against the grid it started on. See
a_batch_that_leaves_records_and_then_sets_normal_lands_in_normal.
Injected fault: settle_normal_mode()'s call site removed. Exactly three tests
fail across the whole workspace — this one, the :-line sibling and the
crossing test — which is the per-edit coverage question answered by there being
one edit.
a_colon_line_run_from_records_settles_back_into_records_normal
The sibling above cannot see ReturnToNormalUnlessMoved, and that was found by
injection rather than by reading: forcing only that effect back to
AppMode::Normal left the sibling green. An empty : line — which is what
pressing Enter straight after : gives — takes ExecuteCommand's
raw.is_empty() branch and settles through ChangeMode, so the effect that
settles every line that actually runs was untested.
So this test types a line. page-next rather than yank, because the script
parser requires coordinates for yank and an unparseable line takes the error
branch — which settles through ChangeMode too, and would have made this test
a second copy of its sibling while looking like a new case. The setup assertion
that parse_line_with accepts the line is there for exactly that: a future
rename of the command would otherwise turn this test vacuous silently.
Injected fault (against the per-effect version of the fix):
ReturnToNormalUnlessMoved forced to AppMode::Normal. This test failed,
left: Normal right: RecordsNormal, while the sibling stayed green — which is
the point, and is how this test came to exist. Against the shipped end-of-batch
settle the discriminating injection is removing settle_normal_mode()'s call
site, which fails this test with the same values.
a_batch_that_leaves_records_and_then_sets_normal_lands_in_normal
New wrong state introduced by PR #91's first attempt, found by the
adversarial review and verified by reverting only ui/app.rs and
ui/effect.rs to base and re-running the identical probe:
:toggle-records-mode . enter-mode normal |
base | #91 round 1 |
|---|---|---|
| from a pivot grid | Normal (the sihr bug) |
Normal still |
| from a records grid | Normal (correct) |
RecordsNormal |
The cause is gotcha 23: resolving at the moment the mode is written reads the
batch's opening layout, so a batch that crosses settles against the grid it
started on. :toggle-records-mode is the case the round-1 prose offered as
proof that the staleness was a property — and it is not, because
ToggleRecordsMode emits change_mode(RecordsNormal) explicitly on entry and
ViewBack restores the pushed mode on exit, so the resolution's answer is
overwritten in both directions and the case exercises nothing. That is a careful
comment standing in for a test, in the PR that recorded
doctrine(an_injection_that_does_not_fail_names_an_untested_path).
The fix moves the resolution to one place, settle_normal_mode after
rebuild_layout, where there is no crossing case left to get wrong.
This test drives both directions deliberately. One direction alone is satisfied by a settle that only ever answers one way, and the pivot→records direction was also wrong in round 1 for the mirror-image reason.
a_wizard_abandoned_over_a_records_grid_returns_to_records_normal
CloseWizard writes view_state.mode directly rather than through an effect
that could resolve it, so under round 1's per-effect design it was a third
writer needing its own edit — and reverting that edit alone left the whole
workspace green, which the review filed. The end-of-batch settle removes the
per-edit question entirely (there is one edit), and this test covers the path
the review named: the wizard abandoned over a records grid.
It drives CloseWizard as an effect rather than typing :import, because the
wizard needs a real CSV and the contract under test is one batch's settling, not
the import pipeline.
a_failed_colon_write_lands_in_normal_where_its_error_is_drawn
A failing : command must not strand the user in CommandMode.
Found by Copilot on PR #27, and it is the sharper form of gotcha 21. The abort is
supposed to stop the chain — :w /bad . q must not quit. What it must not also
stop is the bookkeeping: the mode restoration that ExecuteCommand appends.
Aborting before that ran left the app in CommandMode with the stale command line
still drawn, and draw_bottom_bar renders the minibuffer instead of the status
line — so the "Save error:" set one effect earlier was invisible. A failed save
looked exactly like a key that did nothing.
The answer is ordering, not classification: a batch puts its tidy-up where an abort cannot reach it, which is in front. No effect needs a new "runs anyway" flag, and the rule generalises to any command composing bookkeeping with something that can fail.
Both arms are here because they fail in different places: no path at all is
Save's guard (aborting since improvise-8tk, so this arm was broken before that
branch), and a bad path is SaveAs's write (aborting since f0f400d).
a_failed_colon_write_still_refuses_to_quit_after_it
The other half of the same rule: the abort must still stop the chain.
:w <bad> . q is improvise-8tk's data loss spelled on one : line, and moving the
mode restoration to the front must not have bought the fix above at the price of
that one.
a_sequence_whose_first_step_aborts_does_not_run_the_second
Gotcha 21 claims a failed Save aborts across Binding::Sequence steps as well
as within one command's effect list. Nothing tested it: no shipped Sequence has
a first step that can abort, and ZZ is a prefix resolving to the single wq
command, so prefix_z_then_z_reaches_save_and_quit exercises the
within-a-command case only. The claim was structurally true and unguarded.
It is true because Keymap::dispatch flattens a Sequence into ONE effect vec and
App::apply_effects runs that as one batch, breaking on abort_effects. It is
load-bearing, because it is the reason a [w, force-quit] Sequence would be
safe — the argument the grooming note unblocked on.
And it is exactly what improvise-0hb would remove. Command-level transactionality
means applying effects between steps, and apply_effects resets
abort_effects at entry and again on abort, so step 2 would be dispatched into a
fresh batch with the flag cleared — the Quit after a failed save would land,
silently reopening the improvise-8tk P1 data loss. This is the guard that would
catch it.
unmatched_key_after_prefix_is_swallowed
A prefix is consumed whether or not the next key matches, so a stray key after g
does nothing rather than being treated as a fresh normal-mode binding.
renaming_a_coordinate_moves_every_measure_in_the_row
improvise-c5v: a records row groups one cell per measure, so editing a category column has to move every one of them. The long-format code moved only the cell under the cursor, which in wide format would tear the row apart — half its measures under the old coordinate, half under the new one.
leaving_records_mode_cleans_empty_key_cells
improvise-3zq (bug #2): AddRecordRow creates a cell with an empty CellKey when
no Page-axis categories supply coords — that cell serialises as = 0 in
.improv and re-appears on every records toggle. Leaving records mode must clean
up any such meaningless records (the inverse of the SortData that runs on
entry).
enter_at_bottom_right_of_records_view_exits_editing
improvise-3zq (bug #1): Enter on the bottom-right cell of records view should
commit and leave edit mode. Previously CommitAndAdvance pushed an
EnterEditAtCursor effect unconditionally, so the cursor stayed put and editing
re-opened on the same cell.
Helper — records_model_with_two_rows
Not a test — the fixture for every Tab/Enter boundary test in this file.
A records-mode app with two data rows, for testing Tab/Enter behaviour at
boundaries. In wide format a row is one coordinate tuple, so the two rows are two
Region items, each carrying both measures (improvise-c5v).
tab_on_bottom_right_of_records_inserts_below
improvise-hmu: Tab on the bottom-right cell of records view inserts a new record
below and moves to the first cell of the new row in edit mode.
Note — the wizard_key_tests module
Behavioural coverage for the import wizard's key handling (improvise-2wm). The
wizard is driven entirely through handle_key, so these tests pin the
user-visible contract independently of whether the step/key decisions live in an
effect or in a command.
name_model_typing_edits_the_name_and_enter_confirms_the_import
Enter at NameModel builds and parks on the Done confirmation; a second
Enter there installs the workbook and closes the wizard (improvise-907, from
improvise-c6j).
review_proposals_blocks_when_no_category_is_accepted
BUG (improvise-907, from improvise-gfq): an import whose fields all classify as
Measure used to walk the user through DefineFormulas and NameModel before
build_model refused. The refusal now happens at ReviewProposals, where c can
still change a field's kind.
left_walks_back_through_the_steps_taken
BUG (improvise-907, from improvise-o1c): reaching a later step and wanting to
change a field kind meant Esc, which discarded every decision. Left walks back
through the path actually taken (gotcha 22).
done_esc_returns_to_name_model_and_drops_the_built_model
Esc at Done goes back to NameModel rather than discarding the import, and
the workbook built for confirmation is dropped so a later accept cannot install
stale field decisions.
Note — the basis_tests module fixture
The two lower tiers of a batch's basis (ADR 001, improvise-xwi6). Fixture note:
two regular categories, two measures and two records, deliberately. A one-category
fixture cannot tell TransposeAxes from CycleAxis, a one-record fixture cannot
tell "the record it touched" from "every record", and a one-measure fixture cannot
tell a record from a cell — the three distinctions these tests exist to draw.
a_records_two_measures_share_one_record_key
The record is the coordinate tuple with _Measure gone, and both measures of one
coordinate name the same record, which is the whole point of the unit.
a_layout_change_moves_the_layout_version_and_no_record
Tier 2 moves, tier 3 does not. A shape change invalidates every in-flight
CellData batch through layout_version alone; it must not also masquerade as a
write to any record.
Fixture repaired under improvise-75ld: it asked for
SetAxis { category: "Month", axis: Axis::Column } against a fixture where
on_category_added had already seated Month on Column — so the "layout change"
was a no-op, and the test passed only because the tier was bumped off the
declaration. It now asserts the axis before and after, and asks for Axis::Row.
This is the failure that turned "reclassify three effects" into "stop reading the
tier off the classification". Injected fault: the same as
both_shape_counters_move_for_every_reshaping_command — fails naming SetAxis.
a_loaded_document_is_dirty_because_its_path_still_names_the_old_one
A loaded document is dirty, and that is a behaviour change with a reason (improvise-gv9r, raised by the independent review of PR #26).
LoadModel is Model(WholeWorkbook), so apply_effects raises the flag — where
before that branch :e other.improv left the document reporting clean. Clean was
the wrong answer: LoadModel does not move model_state.file_path, so after
it the in-memory document is a different one from the file the path names, and the
bytes on disk and the bytes in memory really have parted company. That is all
dirty claims.
It is worth a test rather than a comment because of what it gates:
autosave_if_needed and the :q refusal both read this flag, so a user who loads
and quits is now asked rather than silently dropping a document whose path is
stale. :q! still discards. The path-staleness itself is pre-existing and is
reported separately; this pins only that the flag follows from it.
a_view_only_batch_leaves_the_document_clean
The negative arm of improvise-gv9r's raise, and it must be aimed at
apply_effects rather than at a Command: a view-slice command is folded
straight into ViewState and never reaches this loop, so a negative written
against the session door would pass however unconditional the raise was. It is a
ViewOnly effect run through the loop that discriminates.
What it pins: a client's cursor, status line or panel does not put the document out of step with the bytes on disk, so it must not arm an unsaved-changes prompt or an autosave.
a_cell_edit_moves_only_the_record_it_touched
Tier 3 moves for the record written and for no other, and tier 2 stays put: a cell edit does not invalidate batches that merely share the document.
a_coordinate_rename_is_one_write_to_each_record_not_one_per_measure
The unit is the record, not the cell. Renaming a coordinate in wide records mode moves every measure cell in the row (gotcha 1), and that is ONE write to the record it left and ONE to the record it arrived at — not one per measure.
an_aborted_batch_bumps_nothing_it_did_not_run
Gotcha 21: a batch that aborts halfway must bump nothing after the abort — in
either tier. apply_effects accumulates the Dirty inside the loop for exactly
this reason, and the versions are read off the same place.
a_write_that_changes_nothing_moves_no_version
A write that lands the value already there invalidates nobody's basis, so it moves
no version. This is a deliberate choice of semantics — observed change, not "a
CellData effect ran" — and the safe direction, because the alternative rejects
batches for nothing.
the_version_tiers_are_not_in_the_view_state
The counters are shared document state on App, never on the portable session
slice (gotcha 30): a client inheriting another's idea of how far the document has
moved is exactly the stale basis this machinery exists to catch.
a_page_flip_under_prune_empty_invalidates_the_cursor
improvise-7kvw, the case improvise-9mo9 decided the other nine and deliberately left
open. SetPageSelection changes which records are visible and not which categories
frame them, so a grid position normally keeps naming the same cell. Under
View::prune_empty that is false: which rows survive pruning is a function of the
page selection, so a page flip renumbers the grid while neither reset_cursor nor
cursor_epoch moved — gotcha 24's failure in both directions at once, the host's own
cursor stale and no client told.
9mo9 did not patch it because [/] paging is constant navigation and sending the
cursor home on every flip is worse than the hole when prune_empty is off, which is
the default. So the reset is conditional on prune_empty, and on the selection
actually moving.
The fixture has three regions and two quarters whose non-empty rows differ, so
position 0 names North on Q1 and South on Q2. The test asserts the row labels
really differ before asserting the epoch moved; the negative arm asserts they really
do not differ with pruning off, so neither case can pass vacuously.
Injected fault: drop the reset. The prune arm fails. Injected fault: make the reset unconditional. The no-prune arm fails.
a_page_flip_without_prune_empty_leaves_the_cursor_alone
The negative arm of the above, and it is load-bearing for the same reason gotcha 24
gives for SetNumberFormat: without it, "reset on every page flip" passes the
positive test and costs the user their position on every [/].
selecting_the_page_already_selected_invalidates_no_cursor
The third arm, and the one the user actually hits. SetPageSelection resets the
cursor only when the selection moved, and that guard shipped untested: deleting
moved && left the whole workspace green at 1416, which review caught.
It is not an edge case. PageNext/PagePrev are odometer-style and emit a
SetPageSelection for every page category on every keypress, so with more than
one page category the non-carrying categories are always no-ops, and a single
one-item category re-selects the same item every time. Unguarded, each such keystroke
bumps cursor_epoch, sends the host's cursor to the origin and broadcasts
Command::ResetCursor to every client on that view — improvise-75ld's "a no-op that
refused everybody else", one tier down and aimed at the cursor instead of the basis.
The test asserts both preconditions before applying the effect — that the fixture
really is on Q1, and that the grid really does prune — and that the row labels did
not move before asserting the epoch stood still. So it cannot pass by re-selecting a
page that was never selected, or by testing a grid that does not prune.
Both before placements are load-bearing, and both were wrong once. The page check
used to run after the effect, which sets the page to Q1, so it could not see a
fixture that started elsewhere: with the fixture moved to Q2 the test did fail, but
at the row-labels assertion, under a message about renumbering. The sentence naming
the property was not the assertion guarding it. The prune_empty check did not exist
at all, and with pruning off the reset never fires whatever moved says — so
paged_model(false) plus the deleted guard left the test green, proving nothing,
with nothing saying so.
Both preconditions read App::view(), the view the effect writes through
app.view_mut(), and not Workbook::active_view(), which is the document's
default and is deliberately allowed to differ from it (gotcha 29). They agree in
this single-client fixture, so the distinction is latent rather than a live bug —
but an assertion about what an effect did must read the view that effect wrote.
Injected fault: delete moved &&. Fails by name, alone.
Injected fault: paged_model(false) and delete moved &&. Fails on the
prune_empty assertion; before that assertion existed, this combination passed.
Injected fault: paged_model's fixture starts on Q2. Fails on the page-selection
assertion, which now names it; before the move it failed on the row labels instead.
a_foreign_layout_is_not_shown_the_hosts_staged_drill_text
improvise-pubt, filed with an argument for unreachability that the test disproves.
project::cell_display fills edit_text from App::display_value_in(layout, key),
and since improvise-jytm that layout may be another client's.
drill_state.pending_edits are keyed by (row index, RecordCol) against the
host's drill layout (gotcha 1), and a row index in one layout names nothing in
another.
The ticket read the arm as unreachable because "a foreign layout built by
GridLayout::new carries no frozen records, so it should not produce a synthetic
key". That is wrong: records mode auto-detects from the axes (gotcha 1), so an
ordinary saved view with _Index on Row and _Dim on Column gives GridLayout::new
a records layout whose cell_key returns exactly the synthetic (_Index, _Dim) key
the drill arm matches on. The test fails on the original code, naming the host's
staged text read out of a layout it was never staged against.
The fix is App::staged_drill_edit, which consults drill_state only when
std::ptr::eq(layout, &self.layout). A pointer comparison rather than a name
comparison because grid_in reuses &app.layout by reference where the view names
agree and derives a fresh one otherwise, so identity is the question being asked.
Whether the host's pending drill text should reach any other client at all is
improvise-tom1 and is untouched here.
The positive arm — the host's own layout still shows STAGED — is in the same test,
or "never consult the drill" passes, which would blank the TUI's own drill editor.
The drill fixture needs a real RecordRow, not an empty Vec: the host's layout
iterates the frozen snapshot, so an empty one gives a zero-row grid and the positive
arm cannot be built.
a_default_that_names_nothing_says_which_invariant_broke
improvise-h074. App::resolve_view falls back to workbook.active_view for a name
the document has not got and then indexes workbook.views[default]. Several of these
call sites previously went through Workbook::active_view(), which carries
.expect("active_view always names an existing view"); IndexMap's Index impl
panics with no message at all, so a workbook whose default names nothing — reachable
only by constructing the struct directly, since create_view/delete_view maintain
it — failed with a bare index panic where it used to name the broken invariant.
Injected fault: restore &workbook.views[default]. The test fails on the expected
substring.
ui/project.rs
a_window_reaching_the_last_row_carries_the_trailing_group_header
A collapsed group at the bottom of the grid folds into one row (improvise-6qet,
context/code/core.md §14), and the header comes above it. A window that reaches
the last row therefore carries both: the frame is [North, South, [Late], Late].
Before 6qet a collapsed group had no row, and this test asserted the header was the
last entry — the premise 6qet changed, so the length moved from 2 to 3.
It must be seen; it is not yet actionable in a browser. web/app.js registers no
listener on a group <th>, and toggle-group-under-cursor is still in
the_command_names_that_still_need_the_host_are_these, so z resolves Upstream
into Host::handle's empty vec — a black hole, not a slow path
(doctrine(an_upstream_key_is_a_black_hole)). The TUI can expand it today; the
browser can only show it.
Helper — app_with_a_trailing_group
Not a test — the fixture for that test and for its stops-short arm.
Region on Row with East in a group that comes last, so the group header is
the final entry on the axis and nothing follows it. That is the shape
axis_frame's trailing rule is about.
a_window_that_stops_short_does_not_carry_what_is_below_it
The other arm, and the one that would make a browser scrolled to the middle of a document draw the bottom of it: a window that stops short of the last row carries nothing below its last item.
a_window_opening_inside_a_group_names_only_the_nearest
A window opening inside a group still names it — otherwise a browser could not
say which group a column or row belongs to — while body_from declines to paint a
header that sits above the window.
And only the nearest one comes. The fixture puts two headers above the
window (Hidden, whose collapsed group is now one row, and Late) because that is
the only way to tell "the nearest header comes" from "every header above comes".
Since improvise-6qet the fold is itself a row, so the window opens at row 2
(East), past both Hidden's header and its folded row; at row 1 the window would
start on the fold and bring Hidden's header with it. With one group above, deleting the
compaction leaves this green — it is a cost property, O(the groups a deep scroll
crossed), and correctness alone cannot see it: group_of still finds the nearest
and body_from still starts at the first item either way.
Helper — app_with_a_collapsed_group_between_rows
Not a test — the fixture for that test and for every_header_between_two_in_window_rows_comes.
Two group headers between North and East: Hidden, collapsed, and Late, open.
It was app_with_consecutive_headers until improvise-6qet: a collapsed group now
contributes one row, so a collapse no longer produces two adjacent headers and
the old name described a shape the layout cannot emit.
every_header_between_two_in_window_rows_comes
Headers between two in-window rows all come, not just the nearest — the frame is
[North, [Hidden], Hidden, [Late], East], each header above the row it opens.
Renamed from consecutive_headers_between_two_rows_all_come for the reason the
fixture was.
Only the nearest wins group_of, so a compaction that kept one would still answer
every question about membership correctly — and would silently lose the Hidden
header, which is the only thing on screen that says those rows exist at all, and
the only thing that will be clickable once the browser can toggle a group. It
cannot today: z is Upstream and answered with nothing.
Above the window the rule is the opposite, and that arm is
a_window_opening_inside_a_group_names_it_without_painting_it.
folding_what_the_server_emits_reproduces_what_the_server_has
The contract of the whole module: a client that folds what the server sends ends up
holding what the server has. Not the fold axiom — the yardstick (server_truth) is
built independently of the projections.
Helper — server_truth
Not a test — the yardstick that test and its siblings measure a fold against.
What the server believes the client should be holding, built straight from the
authoritative state — the yardstick every fold is measured against, and built
independently of the projections so the fold tests are not vacuous
(doctrine(fold_tests_must_test_order_not_the_axiom)).
removing_a_category_empties_the_clients_formula_panel
Removing a category removes every formula that targets it —
Model::remove_category ends with formulas.retain(|f| f.target_category != name).
RemoveCategory is LayoutGeometry, and Dirty::of(LayoutGeometry) reported
formulas: false, so the formula panel was never told: a client's FormulaCache
kept showing a formula the document no longer has, indefinitely, with nothing to
bring it back short of a reconnect. (Gotcha 35.)
a_late_client_converges_with_one_that_followed_from_the_start
Bootstrap is a compaction, not a replay: a client that connects late and folds only the seed lands exactly where one that followed the whole stream is.
the_projected_extent_is_the_whole_grid_not_the_window
The extent is the whole grid, not the window (improvise-bpq2).
A client resolving its own cursor keys clamps against this, so a producer that read it off the projected ranges would stop every browser cursor at the bottom of the screen — and would look right in the TUI, whose cache covers the whole grid at offsets zero (gotcha 44). The fixture is deliberately not square (3 rows, 2 columns) and the window is smaller in both directions, so it separates "the whole grid" from "the window" and from "rows and cols swapped".
a_reshaped_grid_folds_to_the_desired_state
Reshaping is the case the ordering exists for. Growing is the half that bites: cells at positions the client does not have yet are dropped, so the labels that create those positions must go first. Shrinking is here for the other direction — cells that no longer fit must not be sent into a grid that has already been cut down.
a_renamed_column_whose_total_did_not_change_still_gets_its_total_back
improvise-hkt6, found by Copilot on PR 20. The diff must carry the mirror to
desired, and it stopped doing so for one shape: a column renamed while its
total stayed the same number.
apply blanks the total under a relabelled column (gotcha 33(b)), so the client's
total went blank; but the diff compared mirror.totals against desired.totals,
found them equal, and sent no Totals. ClientSession::project_diff then assigns
self.grid = desired, so the server believed the number was still on screen — a
cache wrong forever, with nothing in the stream to correct it, which is
precisely the direction gotcha 35 keeps Dirty::of sloppy to avoid.
The assertion is the one a_reshaped_grid_folds_to_the_desired_state makes — fold
the updates onto the mirror and require the result to be desired — because "a
Totals update was emitted" would pass for a producer that emitted the wrong
totals.
The fixture renames the middle column and keeps every total equal, so it separates "re-sends what the relabel blanked" from "re-sends whenever the labels move at all".
renaming_an_axis_category_resends_every_cell
The producer half of the category-rename rule, and the reason the apply half is
not cosmetic: the two compose into "wrong forever" without it.
grid_updates diffs the cells against the framed mirror — the mirror with the
frame updates already folded in. So an apply that kept the cells through a cats
change made the producer agree they matched and send nothing,
ClientSession::project_diff then assigned self.grid = desired, and the client
held cells answering the old coordinates with no update in the stream that would
ever correct them.
An axis swap is the case that bites: two categories can legally share an item name,
so Region = [North] becoming Department = [North] leaves the labels identical
and the CellIds valid and pointing at the other record.
a_session_that_switches_views_is_projected_the_view_it_switched_to
The original bug: ClientSession::apply folded straight into the session's mirror,
so a Command::SwitchView moved the mirrored active_view while the workbook's
stayed put (gotcha 29). viewport() then read the cursor under the new view's key
and the next project() built the grid from the other view.
improvise-jytm answers it the other way round — the mirror is now authoritative and
grid_in is built from it — so the assertions moved with it: the asker's mirror
names "Second", the document's default is still "Default", and the rows it is sent
are the ones "Second" has. The fixture's two views are transposes of each other, so
"was I sent the right grid" cannot be answered by accident.
Two faults injected 2026-09-20, both of which fail it. Pointing grid_in at
app.view_name() regardless of the session reproduces the original defect exactly.
Removing ClientSession's projected_view observation is the new way to
reproduce it: a switch is ViewOnly, so nothing in Dirty carries it and the
session must notice its own move or never rebuild its grid at all.
Helper — app_with_two_views
Not a test — the fixture for every lockstep and rebase test in this file.
A second view whose axes are the first one's transposed, so the two grids cannot be confused for each other: "Default" rows are regions, "Second" rows are measures.
a_view_switch_moves_only_the_switchers_mirror
The successor to a_view_switch_moves_every_sessions_mirror_not_just_the_switchers,
which pinned the lockstep improvise-jytm removes. The model is shared and the UI is
not, so a bystander keeps its view, its scroll and its grid.
Three assertions, because the old bug can come back through three different doors:
the bystander's mirrored active_view, its row_offset (nonzero, so a mirror reset
shows as a move rather than coinciding with the origin), and whether project_all
names it at all. That third one is the sharpest — a bystander repainted for
somebody else's switch is invisible in the first two, since it would be repainted
with its own view's content and look right.
The scroll is drained through project_all before the switch, so the fan-out the
test reads names only what the switch caused.
Fault injected 2026-09-20: restoring the adopt_active_view loop (push the accepted
view into every mirror) fails it, as does removing the projected_view observation.
a_refused_switch_corrects_the_asker_and_nobody_else
improvise-6c6q. apply_switch relayed Broadcast::ToOthers, so on the refused path
every client but the one that was wrong was told the accepted name. The asker's
own slice kept the name it had folded optimistically while its server-side mirror
held the accepted one — gotcha 24's mismatch, arriving through the asker's own
keystroke. apply_switch's doc comment claimed the hole was closed, which is worse
than the hole.
The test drives the asker's own ViewState, folding the command the way a real
client would before handing it to the door. That is the whole cost of the ticket:
the sibling bug in apply_model shipped green because its test read a
"what do the others hear" helper, which looks away from the asker by construction.
There is deliberately no such helper in this test.
The bystander is asserted from a view it can be moved OFF. A first version left it
on the default, where a wrongly-addressed relay folds to a no-op — so the assertion
read as coverage and was vacuous, which review demonstrated by injecting ToAll and
watching this test pass. It now sits on "Second", and the audience itself is asserted
(Broadcast::To(asker, …)) rather than inferred from what somebody's slice folded
to: the audience is the message (gotcha 36), so it is the thing to read.
Two faults injected 2026-09-20, both of which fail it: relaying ToOthers (the
original improvise-6c6q bug) and relaying ToAll (the over-correction). The second
also fails folding_the_relay_puts_a_refused_asker_back, which reads frames rather
than folded state.
deleting_the_view_this_client_is_on_moves_it_to_the_default
The host-side half of deleting_a_view_moves_the_client_that_was_on_it_and_tells_it,
and a desync that predates improvise-jytm: Workbook::delete_view moved the
document's active view and nothing moved ViewState::active_view, so the TUI's
cursor was read under a key naming a view that was gone. It was invisible while
draw_status read the workbook's copy; improvise-k69y makes it read the client's,
which is what turns it into a wrong badge.
The negative arm is the discriminating one: deleting a different view must leave this client where it is. Without it, unconditionally seating the client on the default would pass the positive assertion. The fixture therefore carries three views, and the first delete is of the one nobody is on.
Fault injected 2026-09-20: dropping the view_state.active_view line from
effect::DeleteView fails it.
deleting_a_view_moves_the_client_that_was_on_it_and_tells_it
A hole improvise-jytm opens and must therefore close. Before the split,
Workbook::delete_view moved the document's active view and adopt_active_view
pushed the result into every mirror, so deleting the view a client was on moved that
client. The split removes both, and without adopt_deleted_views the client is left
naming a view the document has not got: App::resolve_view falls back to the
default, so it is projected the default's grid while its slice says otherwise, and
viewport() reads its cursor under a key nothing will ever write again.
The fixture carries three views and the bystander sits on the one the delete does
not remove. That is the whole discrimination: a first version left the bystander on
the default, where a relay addressed to everybody folds to a no-op, and review
demonstrated the vacuity by replacing Broadcast::To with ToAll and watching the
test pass. The evidence entry had even stated the risk in one sentence and claimed
the assertion covered it in the next — the exact shape this file exists to stop.
So four assertions, over three different surfaces. The audience is read off the
Broadcast itself, because the audience is the message (gotcha 36). The client that
lost its view moved, in its mirror and in its own slice, the latter driven by
folding what the host relayed. And the bystander is still on "Third" — in its slice
and in its mirror — which only means something because there is somewhere for a
wrong relay to have moved it from.
The deletion itself is asserted, because Workbook::delete_view refuses to remove
the last view: a fixture with one view would pass every other line here while
deleting nothing.
Two faults injected 2026-09-20, both of which fail it: dropping the
adopt_deleted_views call from apply_model, and addressing its relay ToAll.
a_session_on_another_view_is_projected_that_views_axes
The hole review found in improvise-jytm's first cut: only the grid half of
"every projection follows the session's view" was checked. category_tree_in and
tile_bar_in are reached through ClientSession::view_in, and replacing that
function's body with app.view() — the host's view — left the whole suite green.
The harm is worse than a wrong pixel. A category's axis is what the tile bar is for: it is the control a user reassigns axes with. Under the fault a client on another view sees the wrong axis for every category, in the widget it would use to change them, while the grid beside it is correct — nothing to notice and nothing that fails.
app_with_two_views is what makes two assertions enough: "Second" is "Default"
transposed, so reading the wrong view swaps Row and Column rather than blurring
them, and the test names both categories rather than one.
Fault injected 2026-09-20: ClientSession::view_in returning app.view() fails it.
walking_the_view_history_moves_no_document_state
The other hole review found: of the three effects improvise-jytm reclassified to
ViewOnly, only SwitchView had a guard (a_wire_view_switch_leaves_the_document_clean).
Putting ViewBack or ViewForward back to Model(LayoutGeometry) left all 1313
tests green — which is gotcha 3's whole point: a mis-declared kind is silent.
It asserts the consequences rather than the declaration, because those are what a
reader can price. App::apply_effects raises model_state.dirty for any
Model(_), and the browser worker persists on that flag — so a mis-declared
view-back rewrites the entire OPFS document (gotcha 41: Storage is whole-document)
for a move that changed nothing. And it bumps layout_version, so every other
client's next Command::Model is refused LayoutMoved (ADR 001) because somebody
pressed a navigation key.
The assert_ne! on the active view before each pair of assertions is the
check-the-checker: an empty history stack makes ViewBack a no-op, and a no-op
dirties nothing whatever its kind.
Both effects are driven, because they are separate kind() implementations and
review injected them separately.
a_fresh_session_opens_the_documents_default_view
The other half of improvise-jytm's contract, and the reason Workbook::active_view
survives at all: it is what a freshly connected client opens. The fixture switches
an existing session first, so a connect that read any client's view rather than
the document's would be visible.
Fault injected 2026-09-20: pushing the accepted view into every mirror in
apply_switch moves the document's default nowhere, but making apply_switch write
workbook.active_view fails it.
Helper — diff
Not a test — the accessor every diff-subject test in this file reads its answer through.
The two halves of an Applied::Diff, for the tests whose subject is not the
rebase. It panics rather than defaulting, so a command that starts replacing the
document is a loud failure here and not a silent Dirty::NOTHING.
Note: before the comment-cap wave this helper's doc comment was preceded by an orphaned paragraph ("One connected client, as its host really drives it…") describing a type that no longer exists at that site. Dropped.
Both a_model_command_that_moves_the_active_view_binds_every_mirror and
a_model_commands_view_switch_reaches_the_asker_too were retired by
improvise-jytm, along with the to_others helper whose blind spot the second one
existed to cover. No ModelCommand moves anybody's active view any more:
ViewBack/ViewForward left the wire because the stack they pop is one client's
history, and a document swap lands in the rebase arm. The property the second test
defended — a correction must reach the client that folded it — is now
a_refused_switch_corrects_the_asker_and_nobody_else, which drives the asker's own
slice rather than a helper.
a_geometry_command_invalidates_every_clients_cursor_including_the_askers
TransposeAxes and CycleAxis call app.reset_cursor() because renumbering every
row and column makes the old indices name different cells (gotcha 24) — but that
reset only reached the authoritative App's own view slice. Each ClientSession
keeps its own cursor in its mirrored ViewState, and nothing reset those, so every
client was left indexing a grid that had changed shape under it: Viewport::window
clips, so what a bystander actually saw was a blank grid, until it happened to
scroll (improvise-gvnb).
The asker is checked alongside the bystander, and that is not symmetry for its own
sake. A Command::Model is the one kind a client cannot fold optimistically, so
the asker is in exactly the same position as everyone else, which is why the reset
is Broadcast::ToAll and not the To(id, …) a refused SwitchView correction is.
a_model_command_that_does_not_reshape_the_grid_leaves_cursors_alone
The other arm. A model command that leaves the grid's shape alone must not throw anyone's cursor away: where a client is looking is its own UI, and only a change to the shared geometry may invalidate it. Without this, "reset on every model command" would pass the test above.
a_bystanders_own_slice_follows_the_invalidation
And the bystander really is shown the new grid from the origin, rather than a
window clipped to nothing. This is the half the mirror reset alone cannot give: the
client's own slice is driven by what the host relays, so the test drives it the way
Host::command does.
Helper — host_dispatch
Not a test — the relay stand-in that test and its siblings drive a client's own slice with.
A stand-in for Host::command's relay, driving one client's own slice. to is the
client being driven and from the one that asked, because that pair is exactly
what Broadcast's audience is about.
a_model_command_that_leaves_the_active_view_alone_broadcasts_no_switch
Renamed in review: it was ..._broadcasts_nothing, and since improvise-9mo9 it
asserts a ResetCursor broadcast, so the name read as a lie about its own
assertion. No client folded an active view, so there is no SwitchView
correction to make.
What the others are told is the Dirty it produced — and, since improvise-9mo9,
the ResetCursor an AddCategory owes, because a new category lands on an axis
in every view. The assertion is the exact broadcast list rather than "contains no
SwitchView": a weaker form would go on passing if the cursor invalidation were
dropped again.
every_reshaping_model_command_invalidates_every_cursor
Only TransposeAxes and CycleAxis reset the cursor, while SetAxis,
HideItem, ShowItem, ToggleGroup, TogglePruneEmpty, AddCategory,
RemoveCategory, AddItem, AddItemInGroup and RemoveItem renumbered the
grid and did not (improvise-9mo9). SetAxis is the case that shows the shape of
the bug: CycleAxis is SetAxis to Axis::next(), so a pair differing only
by an argument disagreed. Because reset_cursor is also the only writer of
cursor_epoch, every one of these left the host's own cursor stale and
broadcast nothing, which gotcha 24 records as strictly worse than doing neither —
the diff then believes each client already holds origin-positioned content and
sends nothing, so they stay stale permanently rather than until the next scroll.
Two faults were injected. Deleting app.reset_cursor() from SetAxis fails the
bystander-cursor assertion (left: 1, right: 0). Changing the fixture so the
item ShowItem reveals was never hidden fails the first assertion — "ShowItem
reshaped nothing, so this case is not evidence of anything" — which is the one
that stops this from becoming a table of commands asserted against a grid that
did not move. That guard matters most for AddItem: the fixture puts two
categories on Row precisely so an append to the inner one inserts a row inside
every outer block, because on a single-level axis an append lands at the end and
moves nothing, and a one-level fixture would have made the reset look unearned.
Helper — app_with_a_reshapeable_grid
Not a test. Two categories on Row, one of them grouped, one item hidden and one
item left with no data — so ShowItem has something to reveal, ToggleGroup has
a group to fold and TogglePruneEmpty has an empty row to prune. Every command in
the table has something real to reshape, which is what the first assertion checks.
drilling_resets_only_the_drill_views_cursor
improvise-9mo9 made SetAxis call app.reset_cursor(), and DrillIntoCell
issues eight or so SetAxis effects to build the _Drill view. The PR claimed
that was safe and did not test it — the claim was that SwitchView precedes
the batch, so reset_cursor writes the active view's cursor, which by then is
the freshly created _Drill one, while the view drilled from keeps its entry
in ViewState::cursors. That is an ordering argument about effects inside a
batch, which is exactly the class of claim gotcha 23 exists because people get
wrong.
Two faults injected. Removing the SwitchView from DrillIntoCell's batch fails
the first assertion — "the drill did not switch views, so this proves nothing
about per-view cursors" — which is the guard that stops the test passing for the
wrong reason if the drill ever stops switching. Changing reset_cursor to clear
the whole cursors map rather than the active entry fails the third:
left: (0, 0), right: (1, 1), the cursor in the origin view thrown away.
It asserts both sides on purpose. That the _Drill view opens at its origin is
half the property; that the other view keeps (1, 1) and row_offset: 1 is
the half that would have broken, and a test asserting only the first would pass
against a map that had been emptied.
an_accepted_no_op_invalidates_no_cursor
The third negative arm, found in review. Gating the reset on Ok is not the same
as gating it on "something changed", and six commands the document accepts
changed nothing and reset every cursor anyway: SetAxis to the axis already
held, HideItem on an already-hidden item, ShowItem on a never-hidden one,
AddItem of an existing item (Category::add_item dedupes and returns Ok),
and RemoveItem/RemoveCategory of something absent.
The first assertion is the non-vacuity guard and it is doing real work here: it
uses grid_shape to prove each case really is a no-op, so a fixture drifting
into one where the command does reshape fails rather than passes.
Review then found five more cases the table could not reach by construction,
because app_with_a_reshapeable_grid had no formula, no category on Axis::None
and no unknown-name case — the one-category-fixture shape the doctrine names. The
fixture now carries a Profit formula, a Channel category parked on
Axis::None, and a Ghost entry hidden under a name the model never had, and
the table covers: SetAxis on an unregistered category, HideItem/ShowItem on
a name the grid never drew, ToggleGroup on a group that does not exist, and
AddItem { _Measure, Profit }.
That last one is the one to remember. The guard it replaced read
category.items, but _Measure's effective items are explicit ∪ formula targets
(gotcha 7), so it called a drawn column "new". Two of the five injections
initially failed to reproduce, and both times the injection was wrong rather than
the guard: the ShowItem case needed a hidden ghost name, since
is_hidden is false for a name nobody hid; and the AddItem case had to compute
its predicate before the add, because afterwards the item really is in
category.items. An injection that does not reproduce the original ordering
tests nothing — the same shape as the simulated-fix confirmation in gotcha 48.
ToggleGroup's first guard was also wrong and this test caught it:
add_item_in_group sets item.group without pushing to Category::groups, so
asking groups said "no such group" about the fixture's real one.
Faults injected, two, both caught by name: removing SetAxis's moved check,
and removing AddItem's fresh check. The TUI repro needs no test harness —
TileSelect's r emits SetAxis with no comparison against the current axis.
naming_a_measure_at_the_bottom_right_leaves_the_cursor_put
A regression this PR introduced and review caught. AddItem gained a
reset_cursor, and CommitAndAdvance{Down} on the trailing + column routes
RecordCol::NewMeasure through add_measure → effect::AddItem.
inserts_a_column matches only NewCategory, so NewMeasure falls through to
the advance — and EnterAdvance at the bottom-right returns AbortChain,
emitting no SetSelected. Nothing restored the cursor. The SetSelected repair
added for +cat covered that arm only.
The fix moves the restore before the advance, which is gotcha 21's rule
(tidy-up goes where an abort cannot reach it) applied to the cursor rather than
to the mode. Injecting the old ordering fails it: left: (0, 0), right: (0, 3).
a_refused_model_command_invalidates_no_cursor
The second negative arm, and the one the first version of improvise-9mo9's fix
got wrong. It was a loop of one until review: it named a class and tested
AddItem alone, and injecting reset_cursor() into AddCategory's Err arm —
the very bug it was written for, one effect over — left it green. It is now a
table over all three refusable effects, and that injection fails it by name. AddItem and AddItemInGroup report a failure — an unknown category
— through status_msg, and the reset was written after the if let Err, so a
refused add bumped cursor_epoch and broadcast Command::ResetCursor to every
client for a change the document never made. AddCategory had been gated on
success in the same commit, which is what made the inconsistency visible.
SetNumberFormat's arm cannot see this: it tests a command that succeeds and
reshapes nothing, where this tests one that reshapes nothing because it was
refused. Injecting the fault — moving the reset back outside the match —
fails it on the bystander's cursor. The first assertion is the non-vacuity
guard: if the category ever became valid, the case would stop being about a
refusal and the test would say so rather than pass.
Helper — grid_shape
Not a test. The row_items/col_items of the layout, as text: which cell each
position names. It is compared before and after rather than counted, because a
cats change can re-identify every position without changing how many there are.
a_clients_view_only_command_moves_only_that_clients_mirror
The other half of the door: a command that is nobody else's business must stay that
way. A client's scroll must not move the authoritative App's own view slice — the
model is shared, the UI is not.
a_document_swap_reseeds_every_session
Replacing the document must clear the cursor map, because the incoming document's
view names have nothing to do with the outgoing one's (gotcha 24).
install_workbook does that for the App's own view slice, but nothing touched the
session mirrors: a connected client went on naming the old document's view and
scrolling by the old document's cursor, so it was projected a window past the end of
a document it had never seen — an empty grid, with nothing that would ever bring it
back.
Helper — other_document
Not a test — that test's incoming document, and the one whose rename hides a bug.
A different document, with a differently-named active view, so a mirror still keyed to the outgoing one is visible rather than coincidentally right.
rebase_hands_every_session_a_snapshot_of_the_new_document
The reseed is reached from the projection path itself, not from a hook a host must
remember to call — Sessions is not reachable from the effect that installs a
workbook, so a convention there would be unenforceable. rebase exists for the
other half: the client's own view slice lives on the client, and only a fresh
Snapshot resets it.
a_document_swap_over_the_wire_rebases_every_client
A document swap asked for by one client must reach every other client's own view slice, and the host must not be able to miss it.
Command::Model put the WholeWorkbook mutations on the wire (improvise-kmgs), so
LoadModel is now something another client can ask for. The server stayed honest —
the generation bump reseeds every mirror on the next project — but a bystander
went on holding cursors["Default"] = row_offset 3 from a document that no longer
exists, and Applied { dirty, broadcast } said nothing that would tell its host to
send a Snapshot. Sessions::rebase, the only thing that produces one, had no
non-test caller in the tree.
That is gotcha 24's cursor-vs-geometry mismatch, arriving through another client's
command — the same shape doctrine(lockstep_binds_every_session_not_the_asker)
names, one layer out: the obligation was addressed to a host in prose instead of
being carried by the value it gets back.
The server's own mirror self-heals off the generation bump
(a_document_swap_reseeds_every_session covers that half); this is the other one,
the client's slice, which only a Snapshot resets.
Helper — a_document_shaped_differently_under_the_same_view_name
Not a test — the fixture that test needs instead of other_document.
A different document whose active view has the same name as the outgoing one, and a different shape: one row where the outgoing document had three.
other_document renames the active view, and that rename is what makes the bug
below invisible: apply_model broadcasts a SwitchView when the name moves, a
client folds it, and the stale cursor is then read under a key nothing has written
yet — right by accident. Same name, different geometry is the case with nothing to
hide behind, and it is the ordinary one: two revisions of a document both call
their first view Default. (Gotcha 39's fixture warning.)
Helper — app_over_a_loadable_document
Not a test — the App that makes a wire LoadModel in that test really replace the document.
An App whose storage already holds that document, so a Model(LoadModel)
arriving over the wire really does replace the document rather than setting an
error status.
a_command_that_leaves_the_document_in_place_is_not_a_rebase
The other arm of the same rule, so the check is not one that always says "rebase"
(doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)). A model
command that mutates the document in place, and a view-only one, are both diffs —
the question apply_model asks is whether the generation moved, not which variant
arrived.
ui::project::tests::a_load_that_fails_is_not_a_rebase
Qualified because ui::host::tests has a test of the same name: this is the
apply_model one, and a bare leaf was satisfiable by either.
A LoadModel naming a document the storage does not hold leaves the document
alone, so it is a diff too. This is why apply_model asks the generation rather
than the variant: "is this one of the three WholeWorkbook commands" is not the
same question as "was the document replaced", and a client would be reseeded onto
the document it was already holding, losing its cursors to a failed load.
Note — the cell_identity module
Emitting and resolving the surrogate a client names a cell by (ADR 002). The two halves are tested together on purpose: an id that is emitted and never resolved, or resolved by re-deriving it the same way it was emitted, would agree with itself while agreeing with nothing else.
the_two_measures_of_a_row_share_a_record_and_differ_by_measure
Gotcha 1's unit, projected: two measure columns of one row are one record and two
cells. Both halves matter — a surrogate that also varied by measure would break
record_versions, and one that did not vary by measure at all would make the two
columns the same cell.
a_projected_record_id_resolves_to_the_cell_it_was_projected_for
The round trip that makes the surrogate usable: what the server projected for a position resolves back to the key that position has. Every row is checked, because one row cannot tell "resolves correctly" from "resolves to the first record".
Helper — raw_app
Not a test — the fixture for every stored-cell surrogate test in cell_identity.
Region on Row, _Measure on Column. Nothing regular sits on None, so every cell
is a stored cell. Three rows and two measure columns, so "the record" cannot be
confused with "the row" or with "the cell".
a_projected_aggregate_id_resolves_to_the_same_constraint
An aggregate names a coordinate constraint, and it must come back as the same
constraint — including the page and filter coordinates baked into the layout's key
(gotcha 27), which is why this resolves against cell_key rather than against the
row and column labels.
Helper — aggregated_app
Not a test — the fixture for the aggregate half of cell_identity.
The same, with a second regular category left on None, so every visible cell sums
over Product rather than being one stored cell. That is the fork EditOrDrill
makes, and the one the wire projects.
a_surrogate_for_a_removed_row_fails_to_resolve_and_the_others_do_not
The first arm of staleness. The identifiers are held constant and only the thing
that makes the id stale is varied (doctrine(a_rename_can_hide_a_stale_key)):
"East" is removed, so its surrogate must fail to resolve — while "North"'s, taken
from the same projection, still resolves to the same cell. Without that second
assertion a resolve_cell that returned None for everything would pass.
an_empty_cell_can_be_written_through_its_projected_id
improvise-1ey is "type a number into a cell and see the value update", and the cell a user types into is usually empty — so it is named by the grid and by nothing in the store. The index cannot answer it, which is why resolution falls back to the layout, and this is the case that makes that fallback load-bearing rather than defensive.
a_write_by_surrogate_lands_on_exactly_that_cell
Two rows and two measure columns, so a write that went to "the first cell", "the whole row" or "the whole column" is visible.
a_write_through_a_dead_surrogate_changes_nothing
The second half of "reject, nothing to write to": a surrogate that does not resolve must apply nothing, not fall back to some other cell. The document is compared whole, because a write to the wrong place is exactly the failure being excluded.
It goes through model_effects rather than the wire form, because the session door
reaches the same conclusion first and typed — Checked::against refuses this
command with Rejection::RecordGone, so a Checked over it does not exist. What
is asserted here is the floor underneath that door: the key path and any host
resolving a command locally get an empty batch rather than a misdirected write.
a_records_mode_cell_is_named_by_its_coordinates_not_by_its_position
The trap gotcha 1 sets. GridLayout::cell_key returns a synthetic (_Index, _Dim)
key here, carrying positions — the very thing ADR 002 rejected _Index for,
because position names a different record after a filter, an insert or a delete.
The projected id must come from RecordRow::coords instead, which is what
resolving it back to the real cell proves.
Helper — records_app
Not a test — the fixture for every records-mode surrogate test.
Wide format: one row per coordinate tuple, a column per raw measure, the coordinate
columns in front and a + column behind. Two records and two measures, so neither
"the row" nor "the column" can stand in for the record by accident.
the_measure_columns_of_a_record_row_share_its_record
Two measure columns of one row are two cells of one record, and the next row is a different record — the same shape the pivot case has, checked here because records mode reaches it by a different path.
a_coordinate_cell_is_named_rather_than_left_as_no_cell
improvise-exdg. This entry replaces a_coordinate_column_is_projected_with_no_identity,
a test that pinned the bug. It asserted that a records-mode coordinate cell
projects id: None. ADR 002 and gotcha 42 define None as "no projected cell",
yet the Region column is drawn and holds text. So a client could not tell it
from a padded slot. Absence was being used to encode a permission ("the host's"),
which is the conflation ADR 002's rejected Option<CellId> draft was about.
improvise-poia removed the same conflation for the creating columns.
The cell now projects CellId::Coordinate. That keeps the old test's real
property: it is still not a SetCell target, because typed_cell_command,
record_cell_at_cursor and render.rs::editable all match Record alone.
The test seats the cursor on the second row, so the first-row and
first-item cases cannot coincide. It then enumerates every cell of the records
grid and checks each one's kind against its column's RecordCol
(names_its_column), which is the ticket's claim with no case picked by hand.
The first version of that enumeration checked only that each id was Some,
which was weaker than the test it replaced. See improvise-w1bu below. Before the fix it failed at a drawn coordinate projected as no cell. Rival injected afterwards: RecordCol::Category(_) => return None
restored. This test failed at the Some(CellId::Coordinate) assertion, and
a_creating_column_is_named_rather_than_left_as_no_cell (whose None-count
now expects zero) failed too.
a_write_moves_the_version_under_the_clients_own_surrogate
The version counter is keyed by the same surrogate the client holds, which is what lets one identity answer both ADR 002's question and ADR 001's. A write must move the counter the client would check.
Helper — apply_caught_up
Not a test — and note it is #[cfg(test)] fn, not #[test] fn.
Sessions::apply for a client that is fully caught up, for the tests whose subject
is not the basis. The staleness tests build the Basis by hand, so neither kind
can pass by accident through the other's convenience.
Note — the basis_check module
ADR 001's basis check: a batch composed against a picture of the document that has since moved is refused, and one composed against the current picture is not (improvise-xwi6, improvise-fkfg).
Both arms, per tier. A check nobody has watched reject is indistinguishable
from one that always accepts, and a check nobody has watched accept is
indistinguishable from one that always rejects (doctrine(check_the_checker)). So
every tier appears twice.
The fixtures hold every identifier constant and vary only what makes the basis
stale (doctrine(a_rename_can_hide_a_stale_key)): the record a batch names is
projected once and reused, so a rejection can never be an artifact of the test
naming something different the second time.
a_cell_edit_is_rejected_when_the_layout_moved_under_it
The class-2 case, and the one test to keep if only one survives.
A CellData batch is stale when the layout moved, even though no record it names
was written: the coordinates its key hangs off changed underneath it. This is A
removing an item while B commits an edit keyed by it, and it is corruption rather
than a lost keystroke — which is exactly what a cell-level lock would not have
covered, because holding a cell does not stop anyone deleting the row.
Helper — basis_for
Not a test — the claim every basis_check test moves exactly one tier of.
The basis a caught-up client holds for one record. Built by hand rather than
through caught_up so each test can move exactly one tier of it and leave the rest
true.
a_cell_edit_composed_after_the_reshape_is_applied
The other arm of the same tier. A batch composed after the reshape is applied — so the rule is "the layout moved since you looked", not "the layout has ever moved".
a_write_is_rejected_when_the_record_it_names_moved
Class 1: somebody wrote the record since this client read it. Data staleness — the record still resolves, so force is coherent and the reason says so. The identifiers do not move, only the value, so a rejection cannot be a renamed key in disguise.
a_write_survives_another_record_moving
The arm that keeps the tier from being "reject whenever anything was written". A record moving does not invalidate a batch that names a different one — which is the whole reason the tier is per-record rather than a single counter.
a_write_to_a_record_that_no_longer_exists_says_so
Class 2 by the other route, and the reason ADR 002 put a maintained index in
DataStore rather than regenerating one from the layout: "the record is gone" and
"the record changed" must not collapse into one answer. There is nothing to write
to here, so nothing to force — force never creates structure.
The test brings the layout claim up to date so that, whatever kind RemoveItem
wears, only the record tier is left to answer. It is CellData today — removing an
item drops its cells and moves no view's axes — so that line is currently a no-op;
it is kept because a reclassification to LayoutGeometry would otherwise reject on
the tier above and hide the case this test is about, silently turning it into a
second copy of a_cell_edit_is_rejected_when_the_layout_moved_under_it.
a_claim_that_omits_the_record_it_writes_is_not_a_free_pass
The force-by-omission hole, closed. Which records are checked is the server's decision, read off the command, so a client that simply left its record out of the claim does not skip the tier — it is told it never stated one. Without this, ADR 001's "force is a re-composition, not a bypass" would have a bypass sitting in the message format.
a_document_swap_invalidates_every_claim
The top tier invalidates everything, whatever the other two say. It is checked first so the answer names the largest thing that moved: a client told "a record moved" after a document swap would go looking for a record that is not there.
a_rejected_batch_changes_nothing_anywhere
Rejection is a normal outcome: nothing is applied, nobody else hears about it, and no basis moves. A rejection that dirtied something would tell every other client to redraw for a change that did not happen.
a_published_basis_follows_the_grid_updates_it_describes
The order ClientSession::basis_update claims is load-bearing, pinned.
The basis names record counters for the cells the grid updates in the same batch
carry, so a Projection::Basis that overtook them would let a client claim a
version for a value it had not been shown — the mirror image of gotcha 33's rule
that labels precede the cells they frame, and unlike that rule it had nothing
holding it but the order of two statements in project_diff. Proved by injecting
the swap (gotcha 43).
a_client_that_folds_the_stream_composes_an_accepted_batch
The check is only satisfiable because the server publishes the basis — which ADR 001 does not mention, and without which a client could only forge one. So: a client that folds what it is sent can compose a batch that is accepted, end to end, with no test-only constructor anywhere in the path.
re_composing_against_the_republished_basis_is_accepted
And the other side of publication: after somebody else writes, the stream carries a new basis, so the client that folds it can re-compose and be accepted. That is ADR 001's force — a re-composition against the current basis — reachable without any mechanism that skips the check.
ui/host.rs
connecting_hands_that_client_a_snapshot_that_draws_the_document
AC 2: the bootstrap is sent on connect, addressed to the new client, and it is not
empty — a Snapshot whose seed carried nothing would satisfy "a snapshot was sent"
while leaving the client blank.
the_reported_extent_bounds_the_snapshot
The extent is what the snapshot is built at, so a client that says it can see one
row is not sent two. This is the property that makes Viewport the handshake rather
than an afterthought.
without_the_command_the_cell_is_empty
doctrine(check_the_checker): the preceding assertion would also pass if the seed
happened to contain the value already. Inject the fault — never send the command —
and it must fail.
a_bystander_is_projected_another_clients_edit
A cell edit is the whole document's, so a client that did not ask for it is told about it too. The projection is per-session because the viewports differ, not because the change does.
Helper — model
Not a test — and note it is #[cfg(test)] fn, not #[test] fn.
A model command from a client that is fully caught up, for the tests whose subject
is not ADR 001's basis. It goes through handle like any other frame; only the
basis is supplied rather than folded, and caught_up can do that because it holds
the App (a client cannot). The staleness tests build their own Basis.
Helper — addressed
Not a test — how every host.rs test names a cell the way a client would.
The same cell as a client names it: the opaque surrogate the server projected on
CellDisplay::id, which is the only handle a client has (ADR 002). A CellKey is
not constructible over there.
a_scroll_reaches_only_the_client_that_scrolled
A view-only command reaches nobody else. The counterpart to the test above: if every command fanned out, that one would pass for the wrong reason.
a_batch_of_commands_is_applied_whole_and_in_order
One keypress resolved on a client is up to three commands, and the host must take all of them (improvise-bpq2).
The fixture is the shape that actually arrives: a cursor moved past the bottom of a
one-row window, so the selection and the offset that follows it travel together.
The assertion is the offset, not the selection, because a host that applied only the
first command would still look right on selected — the window is what goes wrong,
and it goes wrong silently.
a_batch_is_abandoned_at_the_first_rejection
A refused batch applies nothing after the refusal. The stronger half — nothing
before it either — is a_refused_batch_leaves_the_commands_before_it_unapplied;
this is kept because the two halves fail independently.
a_refused_batch_leaves_the_commands_before_it_unapplied
improvise-955n / improvise-08wb. The bug: ADR 001 is emphatic that a stale batch
is never applied, not even partially, and improvise-ondj went to the trouble of a
type (Checked) to make the unchecked path unconstructible. But Checked is per
command, and a ClientFrame::Commands is a sequence of them: Host::commands
applied in order and abandoned at the first rejection, so a frame whose second
command was refused had already applied its first. The guarantee was true of a
single command and false of a batch.
The earlier command is a view command, which is not a concession: it is the
shape the one real producer emits (keys::commit — one mutation among view
commands), and it is still observable, because the host folds it into that client's
mirror. A frame may carry more than one mutation; what it may not do is apply some
of them and refuse the rest, which is clause 4 and open.
A refusal test needs an accepting one
(doctrine(a_helper_that_builds_the_world_the_code_wants_tests_nothing): pair a
refusal with an acceptance, or the refusal can pass by refusing everything). The
second half runs the same leading SetSelected followed by a command whose basis is
current, and requires both to land. Without it, a SetSelected that simply never
worked would clear the first assertion and prove nothing at all.
Inside it, the stale claim is built from caught_up and then has a single tier
bumped, so the refusal can only be about the tier that was moved — a hand-built
basis that is stale in three ways at once cannot tell which check answered.
a_later_write_in_a_chain_still_answers_for_its_own_record
improvise-955n. The regression 90ea26e introduced, and the reason clause 2 cannot
be applied to all three tiers.
Hoisting the basis check to the frame hoisted all of it, so every mutation after
the first was admitted with no record check at all. Two clients, one record: A
composes [SetCell(North), SetCell(South)] against an honest current basis, B
writes South, A's frame arrives — and A's write to South landed on top of B's
silently, which is exactly the lost update ADR 001's record tier exists to
prevent. At the base commit the second command was checked and the frame refused.
The per-command check was doing two jobs and the commit noticed one. "Is the
client's picture of the document current" is a frame-level fact and hoists
correctly. "Is the client's picture of this record current" is not: which records
are at stake is a property of the command, which is why records_named is an
exhaustive match over ModelCommand (gotcha 43). The incapability argument that
motivated clause 3 is true of records a frame creates and was over-generalised
to records it merely writes — and those are the ones the tier is for.
Three arms, because a fix that refuses everything would pass the first alone and a fix that checks nothing would pass the last two.
In arm 2, B moves South with a SetCell, which is CellData: neither the document
nor the layout tier moves, so only the record tier can discriminate — which is what
makes this a test of the record tier and not of the hoist in general.
Arm 3 — the same record twice in one chain. Legitimate and likely (two measures
on one record is one record) and it works for a reason worth pinning rather than
rediscovering: every check happens before any command is applied, so the second
write is compared to the record's pre-frame counter, not to the one its own
sibling just bumped. Were the checks interleaved with the applies, this frame would
refuse itself with RecordMoved.
Read this before closing clause 4. Interleaving is tempting there, because it
would close the silence: [RemoveItem(North), SetCell(North)] under interleaving
is refused RecordGone instead of silently half-applying, which is half of what
clause 4 owes. So check-before-apply is not a free win — it buys the legitimate
double-write and sells the loud failure on a broken chain, and clause 4 has to
pay that back some other way. Measured rather than argued: injecting the
interleaving fails exactly two tests, this one and
a_frame_whose_later_mutation_fails_is_not_yet_undone, and the second failing is
the trade. Arm 3 is what stops someone taking the trade without noticing they made
it.
a_data_dependent_chain_lands_in_order
The capability the contract is for: a chain of data-dependent commands, applied in order, each seeing what the previous one left.
[AddItem("East"), SetCell(the East record)] is the archetype, and it is the case a
per-command basis check cannot admit however it is ordered: a client cannot
claim a counter for a record its own chain is about to create, because
BasisCache::claim filters to records the cache has been told about. So the basis
is answered once, on the first command that carries one, and the rest of the chain
runs against what its predecessors left.
The second assertion is the one that earns the test. Asserting only that the item exists would pass against a host that applied the first command and dropped the second — which is exactly what the previous rule did, loudly, and what the pre-frame check did, silently.
a_frame_whose_later_mutation_fails_is_undone_whole
ADR 001 clause 4, delivered (improvise-yqpd). This entry keeps the hole it characterized, because the inversion is only legible against it.
The three assertions flipped. The removal is undone, the cell reads its
pre-frame value — Some(Number(1.0)), which is neither 5.0 (the frame's own
write landing) nor None (the removal surviving), and that middle value is the
whole difference between a rollback and an abort — and the frame is refused out
loud as Rejection::ChainBroken. A fourth assertion was added that the previous
version could not have: the record counter goes back too, or the client's next
batch is refused for a write that never happened.
The mechanism, and where it deviates from the ticket. The ticket asked for
snapshot-and-restore at App::apply_effects. What landed is App::snapshot_document
/ App::restore_document — the workbook, its dirty marker and the two counted tiers,
the record tier riding inside the workbook's DataStore — driven from
Host::commands, which takes the snapshot before the frame and restores it on
failure. It is not taken inside apply_effects itself, and the reason is cost:
apply_effects runs on every keypress, and an unconditional Workbook clone there
would put a full document copy on the held-key path that gotcha 24's mcq4 work
exists to keep cheap. The primitive is on App, so a future RunScript opens the
same transaction; only the trigger is at the frame.
What counts as a failure, and the split the ticket did not name. Two things:
an effect that sets abort_effects (Rejection::MutationRefused), and a command
whose record no longer resolves (Rejection::ChainBroken). The second is the
interesting one. The ticket says interleaving the record check with the applies
would close clause 4's silence for free but would sell the legitimate double-write —
a frame writing one record twice works only because the second write is compared to
the record's pre-frame counter. That is true of the counter half of the record
tier and not of the structural half: writing a record twice cannot make it
stop resolving. So the record tier splits — does it resolve is asked again before
every mutation (reduce::broken_chain), is its counter what you claimed stays
pre-frame — and clause 4 gets its loud failure without paying the price the ticket
costed. a_later_write_in_a_chain_still_answers_for_its_own_record arm 3 still
passes, which is the guard that the trade was not taken by accident.
Injected faults, each watched to fail. Making undo_frame restore nothing
fails this test alone. Making broken_chain return None unconditionally fails
this test and the mirror test.
A first version of this entry claimed broken_chain's create-exemption was
"correctness-preserving but not currently observable", and that was wrong in both
halves. Review built the fixture it said did not exist and found the exemption
was swallowing a write — see
a_frame_whose_write_would_land_nowhere_is_refused_rather_than_dropped. The
exemption is gone from broken_chain and stays in the pre-frame check, where it
belongs. The lesson worth keeping: "no test fails when I remove it" is evidence
the tests are thin, not evidence the code is inert — the honest next step is to
build the discriminating fixture, and the reasoning that excused not building one
here ("resolve_record falls back to the layout, so the case cannot arise") was
an argument about the common case presented as an argument about all of them.
The hole this replaced, kept because the inversion is only legible against it:
The contract says a frame is all-or-nothing. It was not: App::apply_effects
aborts without undoing (gotcha 21 — an abort is not a rollback), so a chain whose
later mutation fails leaves the earlier one standing.
[RemoveItem("North"), SetCell(the North record)] is the shape: the removal lands,
the write then resolves to nothing, model_effects returns an empty batch, and
the frame is reported accepted having applied half of itself. Both the partial
application and the silence are open.
This test did not pin that, and the way it failed is the repo's own signature defect sitting on the one test whose job is to fail loudly if anyone believes the hole is closed. It asserted that North was removed and that nothing was rejected — both true of any wholly successful frame. The independent reviewer retargeted the second command from the removed North to the live South, making the frame succeed completely, and the test still passed. It could not tell half-applied from fully-applied, so it would never have inverted when clause 4 landed; it would have gone on passing. The missing assertion is the one that names the hole: the second mutation did not land.
It is unreachable from any client here — the only producer of a multi-command frame
is keys::commit, which emits one mutation among view commands — which is what
makes it safe to document rather than rush.
The frame claims North at its current counter, so it passes the check and fails at apply time — which is the only way to reach the hole. A frame refused by the check never applies anything and would be testing clause 2 instead. That fixture note survives the inversion unchanged: it is still the only route to the arm.
a_rolled_back_load_leaves_no_ghost_view_and_no_false_status
Clause 4 was delivered for the document and the mirrors and NOT for the host's
own session slice, found by an integration review of the first two commits.
App::restore said view_state was "deliberately untouched", citing
improvise-yqpd's design note: AddCategory writes its refusal into
view_state.status_msg and THEN aborts, so rolling that back would undo the
explanation along with the damage.
That note is about an apply_effects-level rollback, which is not what
landed. At the frame level the asker receives ServerFrame::Rejected(reason)
— the explanation is delivered on the wire, addressed to the client that earned
it — so the host's status line has no explanatory duty for a client's frame. The
note still governs the host's own key path, which aborts without undoing exactly
as gotcha 21 describes, and is untouched here.
Three measured consequences of a refused frame, all now closed: the TUI host's
cursor still moved and cursor_epoch was still bumped; a rolled-back LoadModel
left view_state.active_view naming a view of the discarded document; and
status_msg kept the rejected frame's sentence, so a frame that did nothing
claimed it had loaded a file.
The fixture had to be renamed before the test could see any of it.
host_over_a_loadable_document built its incoming workbook with
Workbook::new, whose default view is Default — the same name the outgoing
document uses — so a ghost active_view was indistinguishable from a correct
one. This is gotcha 39's trap one level up, and the incoming view is now
Loaded. Checked first: neither existing user of that fixture depends on the
names matching, and gotcha 39's own same-name fixture is
a_document_shaped_differently_under_the_same_view_name in project.rs, which
is untouched.
Why restoring the whole slice is safe rather than clobbering a TUI user's
in-progress typing: Host::commands is synchronous, so the snapshot and the
restore bracket only the frame's own writes. There is no "in between" for a
keystroke to land in.
Injected fault: dropping self.view_state = snapshot.view_state from
App::restore fails this test and only this test.
Addendum (improvise-gho5). Improvise-gho5 found that this
test and a_rolled_back_load_tells_nobody_and_puts_every_mirror_back passed
against a host that never ran the load. The review's fault, reproduced
here, was placed at the top of Host::commands: refuse any multi-command frame
that contains a LoadModel with Rejected(DocumentReplaced), before
CheckedBatch runs. Both tests stayed green. A frame that never got that far
leaves the generation, the view, the status line and every mirror untouched,
which is exactly what the tests asserted.
Both tests now assert that the asker's whole reply is
the_load_ran_then_the_write_broke(), which is Rejected(ChainBroken(North)).
They also assert, before the frame, that North resolves. Together these make
the rejection a witness that the load ran. ChainBroken comes only from
reduce::broken_chain, which runs inside mutating_frame just before each
mutation. It names North only once the document no longer holds that
record, and only an applied LoadModel in this frame can have removed it.
With the same fault re-injected, both tests fail. The left side is
[Rejected(DocumentReplaced)] and the right is
[Rejected(ChainBroken(<North>))]. The fault was reverted.
The residual: a rogue up-front check that forged ChainBroken specifically
would still pass. That is the same forgery a hand-rolled client could commit
against any basis check, and a test cannot guard against it.
a_refused_mutation_rolls_back_its_predecessor_and_the_abort_flag
The Rejection::MutationRefused arm — the one the first hand-back wrongly called
unreachable — plus the batch-outcome flags going back with everything else.
batch_aborted was left true after a rolled-back frame. Inert at the time,
since every Command::Model resets it at the top of apply_effects, but it is
the same shape as the stale-flag bug two commits earlier and App::restore is its
natural home. App's own doc comment names four more "last batch" fields and
says last_reshaped_views is "the shape batch_aborted takes"; restore was
clearing three of the four and neither snapshotting nor restoring any of them.
All six now round-trip through AppSnapshot, which is why the type was renamed
from DocumentSnapshot — it is no longer only the document.
The fixture detail that matters: a duplicate AddCategory is idempotent, so
the obvious way to reach the refusal proves nothing. This fills to
MAX_CATEGORIES (12) and overflows. And the predecessor is an AddItem rather
than a SetCell, because adding eleven categories reshapes the view enough that
a record-addressed predecessor is refused RecordGone first — hiding the arm
under test behind an earlier one.
Injected fault: dropping self.batch_aborted = snapshot.batch_aborted fails this
test and only this test.
a_frame_whose_write_would_land_nowhere_is_refused_rather_than_dropped
Found by reviewing improvise-yqpd's own work, and it was a hole inside the fix
for the hole. The first version of reduce::broken_chain carried a
create-exemption — the same will_be_created_by_the_frame predicate the pre-frame
check uses — on the reasoning that the two must not drift. The hand-back recorded
that arm as "correctness-preserving but unobservable", because removing the filter
failed no test.
That reasoning was wrong, and the fixture that shows it is the one the hand-back
said did not exist. [AddItem("West"), HideItem("West"), SetCell(West)] observed
rejected=false landed=false: the frame reported accepted, with the write
silently gone — precisely the partial-and-silent failure clause 4 exists to
eliminate, surviving inside clause 4's own implementation.
The argument that settles it. App::resolve_cell is gated by
resolve_record, so resolve_record(hash).is_none() is exactly the condition
under which model_effects returns an empty vec and the write vanishes. And
SetCell/ClearCell are the only variants records_named names, so there is no
command for which a non-resolving record still does something useful. Refusing is
therefore strictly better than dropping, everywhere.
Scope: this closes the record-addressed INSTANCE, not the class.
broken_chain asks records_named, which names only SetCell/ClearCell, so a
frame can still apply partially and silently through a command that names no
record. Two measured as accepted: [DeleteView("Scratch"), SetNumberFormat{view:"Scratch"}], where the effect runs and no-ops inside
apply; and [RemoveCategory("Region"), AddItem{category:"Region"}], where the
effect fails and writes its error to the host's status line without
aborting, so the client is never told. Closing the class needs effects to report
failure rather than narrate it — roughly fifty of them — which is a different
change from clause 4 and is stated here rather than implied.
The exemption belongs at CHECK time and nowhere else, and that asymmetry is
the point: before the frame runs, a record the frame will create legitimately does
not exist, so [AddItem(East), SetCell(East)] must pass — kb.pl's
the_create_exemption_is_a_property_not_a_list is unchanged and still describes
stale_records_in_a_frame. At apply time the record has either been created or it
has not, and "has not" means the write is lost.
Injected fault: restoring any exemption to broken_chain fails this test, plus
a_frame_whose_later_mutation_fails_is_undone_whole and the mirror test.
a_frame_of_view_commands_is_not_refused_by_an_earlier_batchs_abort
The second review finding, and a live bug improvise-yqpd introduced.
App::batch_aborted is reset only inside apply_effects (gotcha 21), and
Host::commands read last_batch_aborted() after every command in a frame.
A command that runs no effects — every view command, and Command::SwitchView,
which apply_checked routes to apply_switch rather than through
apply_command — leaves the flag exactly as some earlier batch left it.
So a TUI :w /bad/path raised the flag, and the next frame a connected browser
sent was refused MutationRefused and rolled back, having done nothing wrong.
Reproduced by aborting a pathless effect::Save directly on the App and then
sending [SetSelected].
Two arms, because frame-level gating alone is not the fix. The first arm is a
pure view frame, which the Command::Model-presence gate in commands handles.
The second is the mixed frame [SetSelected, SetCell], which that gate lets
through and which only the per-command mutation && guard answers — the view
command runs no effects, so the flag under it is still the stale one. Injecting
if { let _ = mutation; self.app.last_batch_aborted() } fails this test and only
this test, which is what shows the per-command guard is load-bearing rather than
belt-and-braces.
The same Command::Model-presence predicate is why a frame of pure cursor keys
now pays for neither the Workbook snapshot nor the Sessions clone — the
browser's common path, one frame per keypress.
the_mirror_settles_its_mode_against_every_grid_it_is_later_projected
improvise-el9h. ClientSession::fold settled view.mode against self.grid — the
grid that session was last projected — and project_diff then assigned
self.grid = desired and never settled again. The client does not have that shape:
ClientState::fold calls settle_mode() on every fold including
Command::Project, so when the new view's grid arrives the client re-settles and
the mirror does not.
[SwitchView(records-shaped view), ChangeMode(Normal)] is the shape — precisely the
case PR #98's settle exists to serve. Demonstrated before the fix: the client
ended on RecordsNormal and the mirror on Normal. The fix is settle_mode called
from fold and at the end of project_diff once the grid has moved, which also
closes the three bypass doors (apply_switch, adopt_deleted_views,
adopt_cursor_epoch write the mirror with bare reduce_view): whatever they leave,
the next projection settles.
Two preconditions, and one of them caught a broken fixture on its first run. The
test asserts the client starts on Normal, so the settle has somewhere to move the
mode from. And app_with_a_records_shaped_second_view asserts the second view
really reaches records mode — which failed immediately, because the view was built
with View::new and a fresh View carries no axis entry for _Index/_Dim, so
is_records_mode can never fire on one. Without that assertion the test would have
compared two modes that were both Normal for a reason having nothing to do with
the bug, and passed against the unfixed code.
The yardstick is a real improvise_protocol::ClientState fed the same projections
in arrival order, not a hand-written expectation: the property is that the mirror
and the client run the same function over the same stream.
a_failed_frame_puts_the_askers_mirror_back_on_the_view_it_was_on
Clause 4's least obvious cost, in the ticket's own words: rolling back the App
is not enough. Host::command projects per command and apply_switch /
adopt_deleted_views / adopt_cursor_epoch move mirrors inline, so by the time a
later command fails, every session's mirror has already moved. Host::commands
therefore snapshots the whole Sessions beside the document and restores both.
The first version of this test was vacuous, and the injection is what showed it.
It seated a bystander on a second view and asserted the bystander was told nothing
and its mirror was unmoved — both of which hold when undo_frame restores nothing
at all, because the frame's projections are discarded from out either way and
nothing ever moved that bystander. It answered did the frame return early where it
meant was the mirror put back.
The rewrite aims at the one mirror move a failed frame really makes and that
nothing in the stream can ever correct: the asker's own SwitchView. An
accepted switch is relayed to nobody (gotcha 36), so a mirror left on the switched-to
view diverges from a client that was told the frame was refused, permanently and
silently. The fixture asserts up front that the mirror starts on Default — the
direction the default cannot supply — so the closing assertion is not satisfied by a
mirror nothing ever wrote. Removing self.sessions = mirrors while keeping the
document restore fails this test and only this test.
a_frame_that_creates_the_record_it_writes_is_not_a_broken_chain
The other direction: clause 4 must not fire on [AddItem("West"), SetCell(West)],
which is the exemption ADR 001 states as a property — a record that neither
resolves nor is claimed is one the frame is creating.
Be clear about what this does and does not catch. It catches broken_chain
being asked against the pre-frame document instead of the running one, which is
the plausible implementation error (passing the snapshot where the live App
belongs) and would refuse every data-dependent chain. It does not catch the
exemption filter being dropped, for the reason recorded above: resolve_record
falls back to the layout, so West already resolves once AddItem has run. A
fixture that discriminated the filter would need a created record that never enters
the layout; none exists in this workspace and one was not invented for it.
Helper — frame_basis
Not a test — the frame-level claim that test and the 955n tests state.
The basis a caught-up client would state for a frame naming regions: the two
document tiers plus each named record's current counter.
reduce::caught_up cannot stand in for this, and the difference is exactly what a
frame is: it builds the claim for one command, so a frame's records beyond the
first are simply absent from it and come back RecordUnstated. A real client calls
BasisCache::claim once with every record its frame names, which is what this
reproduces.
Name only records that exist. claim omits a record the cache has never
heard of; this helper always includes one, so pointed at a record that is not there
it fabricates (hash, 0) — a claim no real client could compose, and precisely the
input that flips the create exemption in CheckedBatch. For a record the frame is
creating, leave it out, which is what a client would do.
an_accepted_switch_is_relayed_to_nobody
The successor to a_bystander_is_sent_the_broadcast_and_the_asker_is_not, which
pinned the pre-improvise-jytm relay (Copilot, PR #10). An accepted switch is now
owed to nobody: the asker folded it, and no other client's view moves.
It asserts the bystander gets no frame at all, not merely no SwitchView. A
relay that vanished while the projection fan-out still repainted every client would
pass the narrower check and still be pushing one client's view change at the rest.
a_geometry_command_relays_the_cursor_reset_to_the_asker_too
The other audience, and the reason Broadcast carries one rather than the relay
deciding. A geometry change invalidates every cursor into the document
(improvise-gvnb), and the asker's is not exempt: a Command::Model is the one kind
a client cannot fold optimistically, so nothing has reset it locally and skipping it
would leave the very client that asked positioned in a grid it no longer has.
a_stale_batch_is_refused_to_the_asker_alone
A refused batch is answered to the asker and nobody else (ADR 001). Nothing happened, so there is nothing to tell the others — and the asker is told only which class of staleness it was, because it already holds what is current.
folding_the_relay_puts_a_refused_asker_back
What Broadcast::To is for, asserted end to end through Host rather than
through Sessions: a client that folded a view name the document refused is put
back by the frame the host sends it, so the cursor its next grid is positioned by is
read under a key the document has (gotcha 24).
The client slice is folded before the command is handed to the host, which is
what makes the assertion mean anything: a test that never folded would see
"Default" whatever the relay did. The bystander is asserted silent in the same test,
because a ToAll correction would also put the asker back and would be wrong.
a_document_swap_sends_every_client_a_fresh_snapshot
AC of gotcha 39: a command that replaced the document owes every client a fresh
Snapshot, not a diff — including the client that did not ask.
Helper — host_over_a_loadable_document
Not a test — the host that test and a_load_that_fails_is_not_a_rebase drive.
A host whose document can be replaced out from under its clients: a second document
sits in the storage backend under a name LoadModel can reach.
a_client_can_ask_its_host_to_save
improvise-368h. The bug: effect::Save is EffectKind::ViewOnly — correctly,
since it writes this host's path — so the derivation that builds ModelCommand
out of the EffectKind::Model effects could not reach it (gotcha 38). There was no
save on the wire at all, so no client on any transport could cause its host to
write: a websocket session edited the document in memory and the file on disk
never moved unless a human was at a TUI that was not there.
The derivation was not wrong; it was complete over the mutations and saving is not one. So the variant is added rather than derived, and it carries no path: a client cannot see the host's storage, and which document a host is hosting is the host's own fact.
a_save_with_nowhere_to_write_is_refused_and_told_only_to_the_asker
improvise-368h: the "what if there is no path" half. Not a panic and not a silent no-op — the two answers the ticket ruled out — but the vocabulary that already exists for "nothing happened and here is why", addressed to the asker alone (gotcha 39).
The second client is not decoration: a refusal that leaked to the bystanders would be telling them about a failure of somebody else's host-local request, and this asserts it does not.
a_write_that_fails_is_refused_rather_than_reported_as_saved
improvise-368h: a write that was attempted and failed must not be reported as a
success. Save signals failure by aborting its batch (gotcha 21), and the batch
here is exactly [Save], so an abort is that failure and nothing else.
The fixture fails for a reason that is real rather than mocked: a PhysicalFs
pointed at a directory that does not exist. It creates nothing, so the test leaves
no trace on disk.
every_frame_survives_a_json_round_trip
The wire is JSON to start with (the ticket's choice, kept), so both directions must
survive a round trip through it — including the one point at which ViewState is
serialized (gotcha 25).
The inbound half carries a payload-bearing variant on each side of the envelope
(doctrine(populate_before_roundtrip)): a Viewport and a Command carrying a
CellValue::Text, which is the arm the outbound half cannot reach — a text cell
aggregates to nothing under a None-axis category, so it never becomes a
CellDisplay.
Note: host.rs also carries a second a_load_that_fails_is_not_a_rebase
(doctrine(check_the_checker): inject the fault the test targets — a load that does
not happen — and the snapshot must not appear; a test that saw Snapshot on every
command would pass a_document_swap_sends_every_client_a_fresh_snapshot). It shares
its name with the ui/project.rs test of the same name, which is why there is one
heading for both above.
ui/cat_tree.rs
the_tree_carries_the_virtual_categories_and_measure_includes_formula_targets
improvise-vjms: the panel lists the virtual categories alongside the regular ones,
and _Measure's items are derived — explicit items ∪ formula targets (gotcha 7).
An implementation reading category("_Measure").items passes every other assertion
in the module and silently drops Profit, which is the failure improvise-ete was
closed for proposing.
command/keymap.rs
f1_and_question_mark_both_open_help
improvise-th7m: F1 was bound twice on consecutive lines — first to a bare
enter-mode with no args, then to enter-mode ["help"]. The first was dead,
because Keymap::bind inserts and the later binding wins; had it ever been the
surviving one it would have failed require_args at dispatch and made F1 do
nothing.
The risk in deleting a line of a duplicated pair is deleting the wrong one, so this
pins which binding F1 is meant to carry: the same one ? carries, which is what
identified it as the intended survivor.
every_bound_command_name_is_registered
improvise-9cn: Keymap::dispatch resolves a binding through
registry.interactive(name, args, ctx).ok()?, so a binding is a silent no-op key in
two ways, not one: the name may not be registered, or the args may not parse. Both
walks live in this test.
The args half matters because improvise-s13 moved bindings out of the nullary class
the name check covers: enter-mode ["export-prompt"] and
enter-mode ["formula-edit"] carry a mode name that parse_mode_name must accept,
and a typo like "export_prompt" compiles, registers, and produces a dead
Ctrl-e. (Gotcha 15.)
It builds the context in the mode whose keymap it is walking, because a few parsers
read ctx.mode — commit-buffer item resolves its target category off
AppMode::ItemAdd.
improvise-piyp. This entry is about the test's reach, not its logic — the logic was right and demonstrably so, which is what made the gap hard to see.
It had two blind spots, both found by injection rather than by reading:
- the walk matched
Some(Binding::Prefix(_)) | None => {}, so it never descended into a prefix sub-keymap. Everything underg,yandZwas unchecked, in every mode — andRecordsNormalreaches Normal's three throughKeymaps::prefix_keymap, so it was unchecked twice over. - its key space was a second copy of
tests_support::probe_keys_pub, identical but for the two F-keys the public one carries. So a name bound on any function key was invisible here whiledump_every_bindingprobed it.
Injected at 523d571: one unregistered name under the g prefix and one on
F(5) in Normal. cargo test --workspace came back 1595 passed, 0 failed
— byte-identical to the clean baseline. The positive control that separates
"untested path" from "broken test" is the same bogus name on Char('~'), a
probed non-prefix key: that fails. The guard worked; it could not see there.
Both are closed here. The walk is reachable_names, which recurses into
Binding::Prefix — terminating because prefix_keymaps_are_flat says the
nesting is one deep — and probe_keys now delegates to probe_keys_pub
rather than restating it, which is the part that matters: two lists of "the
keys a test probes" is the same disease as two keymap spines, and the F-key
gap was exactly the drift between them. probe_keys_pub covers F(1)..=F(12)
rather than the two that happen to be bound, since the point is to see a
binding nobody expected.
Re-running the two injections against the fixed test: the prefix one fails
with Normal binds "no-such-command-under-a-prefix", which no registry entry provides, and the F-key one with the matching message for F(5).
Worth noting for contrast, because reading either test alone misleads:
nothing_declined_is_also_composed in resolve.rs already recursed into
prefixes and into parents. The resolution side was guarded where the
registration side was not, and only comparing the two shows which half is
real.
prefix_keymaps_are_flat
improvise-jc8: an armed prefix is stored as the key that opened it and resolved
against the CURRENT MODE's keymap. That is only correct while prefixes are flat — a
Prefix binding nested inside a sub-keymap would be resolved against the mode map,
where it does not exist, and the second half of the sequence would silently do
nothing. Adding a nested prefix must fail here rather than in the terminal.
every_panel_keymap_shares_the_same_spine
improvise-4pg: the three panel keymaps were written out separately, so a spine
binding could be changed in one and forgotten in the others. They share
PanelKeymap now; this pins what "the spine" is, including that each panel's cursor
keys move its own cursor. (Gotcha 12.)
every_mode_with_panel_toggles_binds_all_of_them
Every panel's toggle key is bound in Normal and in all three panel modes, so focus
can move between any two and the focused one closes. A fourth panel gets all four
keymaps from one Panel::ALL entry.
panel_toggle_keys_dispatch_to_real_effects
Panel::name() has to spell what the registry's panel parser accepts. A lookup-only
assertion would not catch a mismatch — dispatch swallows an unresolvable command
with .ok()? — so the toggle key must come back with real effects.
every_text_entry_mode_types_into_the_buffer_its_prompt_names
improvise-k8h: the buffer name used to be written on both sides of the crate —
AppMode's MinibufferConfig names the buffer the prompt describes, and the keymap
names the buffer append-char edits. If those disagree the mode prompts over one
buffer while every keystroke edits another, with nothing to notice. TextEntry now
reads the name off the mode, and this pins the result for every text-entry mode.
(Gotcha 11.)
Helper — text_entry_modes_with_exits
Not a test — the list that test and the two below it walk.
The seven modes that carry a MinibufferConfig, each with the keys that commit or
leave it. SearchMode is not among them: it is Normal plus a flag, so it has no
AppMode variant and no minibuffer config to agree with.
every_typing_keymap_belongs_to_a_text_entry_mode
The list in text_entry_modes_with_exits must stay complete: every keymap that
types into a buffer belongs to a mode whose minibuffer names that same buffer. Only
SearchMode, which has no AppMode variant, is exempt.
text_entry_modes_clear_the_buffer_they_type_into
Every minibuffer mode types into exactly one buffer, and every key that commits or
leaves must clear that same buffer on the way — a buffer left behind pre-fills the
next entry with the last one. SearchMode is excluded on purpose: it has no commit
and its query is owned by exit-search-mode, not by clear-buffer.
The buffer is derived from each mode rather than restated in the test —
every_text_entry_mode_types_into_the_buffer_its_prompt_names owns pinning what it
should be (improvise-k8h).
Amended by improvise-fy9o. A commit key bound to a plain command now passes only if that command, built in
the mode with text in the buffer, emits SetBuffer { name: <buffer>, value: "" }
itself. Rival: CommitAndAdvance without its head clear. Only this test
fails, which makes it the TUI's guard against leftover text.
minibuffer_enter_dispatches_to_a_registered_commit
improvise-6t3: the three commit commands collapsed into a single
commit-buffer <kind>. dispatch swallows an unresolvable command name with
.ok()?, so a lookup-only assertion would still pass had the rename missed the
registry — Enter has to come back with real effects.
dump_every_binding
The instrument doctrine(binding_dump_diff) asks for: every (mode, key, modifier)
lookup across all the modes, written to the file named by IMPROVISE_BINDING_DUMP.
Most bindings have no direct test, so a green suite is not evidence that a keymap refactor preserved them. Dump before, dump after, diff. It is inert without the variable: a test that writes a file nobody asked for is a surprise, not a check.
This is what proved the keymap move to improvise-protocol changed nothing — 4021
resolved lookups, byte-identical either side — and the diff was shown to be
discriminating by renaming one binding and watching eight lines move.
command/cmd/commit.rs
Not an assertion — doctrine(binding_dump_diff)'s instrument, and the
evidence improvise-636d actually rests on, because a green suite is not
evidence for a keymap refactor.
Diffed across the global change: 84 added lookups, 0 removed, 0 changed.
No binding anywhere answers differently than it did, which is the property
that matters for a change that moves bindings between tables. The additions
are 16 rows of ? reaching the three panels and TileSelect, 1 of Help's new
?, 24 of the panel toggles reaching Help and TileSelect, and 40 that are not
a binding change at all — the widened probe_keys_pub hitting ImportWizard's
KeyPattern::Any on F(3)..=F(12).
: produced zero rows, which is the result worth keeping: every mode that
now inherits it already bound it locally, so moving it into the global changed
no answer anywhere. That is what "the duplication was real and consistent"
looks like from the outside — and it is also why nothing in the suite could
have caught the two spines drifting apart, had they ever done so.
invalid_formula_reports_the_parse_error_not_success
improvise-6t3: the commit pushed set_status("Formula added") after AddFormula,
so the parse error that AddFormula::apply writes to the status bar was immediately
overwritten. An unparseable formula was dropped while the UI reported success.
The add effect is the only party that knows whether it worked, so it has to get the last word on the status line.
commit_and_advance_threads_edit_mode_to_enter_edit_at_cursor
CommitAndAdvance must thread its edit_mode through to the trailing
EnterEditAtCursor effect so the post-commit re-edit lands in the mode the keymap
requested. The command never reads ctx.mode.
commit_save_writes_the_model_to_the_typed_path
improvise-lae: the save prompt has to commit to SaveAs and not to ExportCsv,
which is what makes it a separate mode from ExportPrompt rather than a second
binding onto it — ExportCsv would write CSV under whatever .improv name the user
typed.
SaveAs and not Save, because Save writes the path the document already has,
and the whole reason this prompt exists is that there isn't one.
commit_save_on_an_empty_path_writes_nothing
An empty prompt must not reach SaveAs: SaveAs(PathBuf::from("")) asks the
backend to write to "", and the user then reads whatever I/O error that produces
instead of "you typed nothing".
commit_export_bails_on_an_empty_path
improvise-z05r's first arm. CommitExport did not bail on an empty path where
CommitBuffer's three kinds all do (gotcha 20: buffer to trim to bail-on-empty to
dirty to status is the shared spine), so Enter on an empty export prompt asked the
backend to write to "" and the user read whatever I/O error that produced.
The same change reorders the batch: the mode change now comes first, because
ExportCsv aborts on a failed write as of z05r's other arm, and an abort at the tail
would strand the user in ExportPrompt — where draw_bottom_bar renders the
minibuffer instead of the status line, so the export error is never drawn at all
and the operation appears to do nothing (gotcha 21). That ordering is asserted in
commit_export_produces_export_and_normal_mode, by index and not by presence.
command/cmd/fit.rs
the_command_installs_a_formula_the_document_evaluates
The end-to-end claim of improvise-vx3, asserted against a real document: a command
fits, the formula lands through the ordinary add_formula path, and the new measure
evaluates to numbers.
Helper — workbook
Not a test — the document every fit.rs test fits against.
A document a tree can learn exactly — Score steps at Cost 50 — in the units a
real document uses.
Every category is a Label, so the only feature is the numeric measure Cost
and the champion cannot reach an unresolvable one. An earlier version left Region
regular with one item and claimed simplify would prune the constant one-hot; it
does not, because build_categorical records (0.0, 1.0) for every one-hot
whatever the column holds — so these tests were passing on seed luck (Copilot on
PR #25). See improvise_gp::fit's own fixture for the same correction.
the_status_line_reports_quality_against_the_baseline
improvise-vx3 is emphatic that the rule alone is not the deliverable: a tidy tree that generalizes no better than the mean is worse than useless. So the status line carries the held-out comparison, what the rule leans on, and the seed that reproduces it.
a_refusal_names_what_went_wrong_and_stores_nothing
A refusal reaches the user whole. "fit failed" is not actionable; naming the measure that does not exist is (improvise-8xhy's argument, arriving at the place a user meets it).
the_seed_argument_changes_the_search
Gotcha 37b: the seed is half of what reproduces a fit, so it is an argument — and a different one really does search differently.
It asserts on the rule, never on the batch's debug string. The first version
compared effects_debug over the whole vec, which was vacuous: status_line prints
seed {:#x}, so the two strings differ because the seed was printed, and
fit_measure could have ignored request.seed entirely with the test still green
(doctrine(check_the_checker)).
AddFormula has exactly two fields — the formula text and the target category — and
the seed reaches neither, so its Debug cannot differ for the reason the batch's
did. (A substring guard was tried and is wrong: the seeds 1 and 99 occur inside
perfectly ordinary thresholds.)
the_seed_the_status_line_prints_is_a_seed_this_command_accepts
Copilot on PR #25. The status line advertises the seed as {:#x} and tells the user
that is what reproduces the fit, so the one instruction it gives has to work on the
value it just showed them — a decimal-only parser made copying it back an error.
Asserted as a round trip through the status line, not against a hand-written
"0x…": a literal would still pass if status_line changed how it spells the
number, which is the half that breaks.
command/cmd/grid.rs
drill_into_formula_cell_returns_data_records
Drilling into a formula cell (Profit = Revenue - Cost) must return the underlying
data records, not an empty result set. The formula target coordinate is stripped
from the drill key so that matching_cells finds the raw data backing the formula
(gotcha 10).
drilling_fixes_its_coordinates_on_filter_not_page
improvise-dz8: a drilled coordinate is a constraint of the drill, not a dimension
the user may page through, so it lands on Filter. It used to land on Page, which
put it in the tile bar and under [/] as if the user could move it (gotcha 27).
command/cmd/mode.rs
enter_mode_export_prompt_sets_mode_and_clears_its_buffer
The enter-export-prompt shim is gone (improvise-s13); the generic enter-mode
carries the mode name instead, and clears the prompt's buffer on the way in.
enter_mode_clears_a_search_keyed_minibuffer_through_write_buffer
improvise-gdl made write_buffer the single accessor for a runtime-named buffer,
because "search" is not an entry in the buffers map but its own ViewState field.
EnterMode built a SetBuffer directly, so a mode whose buffer_key is "search"
would clear a map entry nobody reads and leave the previous query on screen — the
exact desync gdl closed. No shipped AppMode names that buffer yet, so the mode here
is constructed by hand; that is what makes the test a demonstration rather than a
restatement.
edit_or_drill_passes_records_editing_mode_through
EditOrDrill must trust its edit_mode parameter rather than checking ctx.mode —
the records-normal keymap supplies records-editing, but the command itself never
inspects the runtime mode. This is the parallel of the deleted
enter_edit_mode_produces_editing_mode test for the records branch.
enter_edit_at_cursor_cmd_passes_target_mode_to_effect
EnterEditAtCursorCmd must hand its target_mode straight through to the
EnterEditAtCursor effect — the keymap (records o sequence or
commit-and-advance) decides; the command never inspects ctx. It fronts the pair
with RebuildLayout so the cursor is read against a layout that includes whatever
ran earlier in the sequence (improvise-ehz, gotcha 23).
command/cmd/navigation.rs
page_cycling_skips_filter_coordinates
improvise-dz8: [ and ] page through Page-axis categories. A coordinate the drill
fixed sits on Filter and must be invisible to them, or the user could page out of
the drill they are inside.
enter_advance_at_bottom_right_emits_abort_chain
At bottom-right EnterAdvance has no place to go, so it emits a single AbortChain
effect. Trailing effects in a CommitAndAdvance batch (EnterEditAtCursor) are
then skipped, which is how "Enter at bottom-right commits and exits editing" is
realised.
command/cmd/panel.rs
cycle_panel_focus_advances_from_formula_to_category
Bug improvise-dqn: CyclePanelFocus always picked the first open panel, ignoring
the currently focused panel. With formula + category open and focus on the formula
panel, cycling must move to the category panel.
create_and_switch_view_names_incrementally
improvise-aaw asked for this command to be replaced by a keymap
Binding::Sequence [create-view X, switch-view X, enter-mode normal]. It cannot be:
a Sequence carries literal args, and the name here is computed from ctx — so
a Sequence would hard-code one name and the second n in the view panel would
collide with the first.
The test was called ..._names_incrementally and asserted only that
CreateView/SwitchView/Normal appeared — it never looked at the name, which is
the one thing that makes this command a command and not a composition. Now it does,
at two different view counts, because a single count cannot tell "derived from the
workbook" from "constant".
command/cmd/registry.rs
every_entry_constructs_a_command_with_its_registered_name
improvise-9cn / improvise-61f: every registration derives its name from a prototype
instance, so a typo in the name is impossible. What the types do not catch is the
commands whose name() branches on a field — ViewNavigate, TileAxisOp,
ToggleGroupAtCursor, CommitAndAdvance. Their prototype and the value their
constructor builds are the same type, so registering ViewNavigate { forward: true }
while building forward: false compiles cleanly and files the command under the
wrong name, where dispatch silently misses. Constructing every entry both ways and
comparing names is the only check that reaches that class. (Gotcha 15.)
Some entries are deliberately one-path — commit-and-advance-right reads the edit
buffer and cursor, so its parse fn always errors — but every entry must be reachable
by at least one path, or its name goes unchecked.
registered_names_are_unique
A variant mix-up does not show up as a name mismatch — it shows up as a collision.
Registering ViewNavigate { forward: false } where forward: true belongs files a
second entry under "view-back" and removes "view-forward" from the registry
entirely; parse takes the first match, so every surviving entry still agrees with
its own name. Uniqueness is what catches it.
the_context_reports_each_panels_own_open_flag
improvise-y4xg and improvise-qsmp, two shapes of one defect. registry.rs mapped
Panel to a CmdContext field in two places, and App::cmd_context copies the
three flags out of ViewState in a third. Both were demonstrated unguarded:
swapping Formula and Category at the two registry sites left the workspace at 1400
passing, and hardcoding the category_panel_open propagator to true left it at
1394.
Nothing exercised either, because cmd/mod.rs::test_helpers sets the panel flags
explicitly on the context it builds — the command tests construct the value they then
assert on, so no test ever drove the real propagator.
The registry mapping is gone: both sites call CmdContext::panel_open(panel), the
one mapping on this side of the wall. That leaves two permutable sites, and the test
is aimed at both from a path neither controls — it writes view_state's three fields
by name and reads back through the enum accessor on a context built by the real
App::cmd_context. PR #70's review caught the weaker shape of this: a panel test that
zeroed and read through the same four accessors passed a consistent permutation.
Injected fault: swap Panel::Formula and Panel::Category in panel_open. Fails.
Injected fault: hardcode the category_panel_open propagator to true. Fails.
toggle_panel_visibility_reads_the_panel_it_names
The behavioural half, and the one that reaches the registry. With exactly one panel
open, the command built for each of the three names and executed must emit
SetPanelOpen for its own panel, flipping its own flag. Both injections above
fail here too. Since improvise-yf7p the flag is read in execute rather than copied
in by a registry contextualizer, so the assertion moved from the built command's
Debug to its effects; it also names the panel, so an execute that ignores the
parsed panel (ctx.panel_open(Panel::Formula)) fails it — injected and run.
a_focused_open_panel_closes_and_a_closed_one_opens
improvise-qsmp's own scenario: toggle-panel-and-focus decides close only when the
panel is both open and focused, so the case that separates a right propagator from
a stuck-true one needs the mode set to the panel's own. With the flag stuck true,
pressing a panel's key on a closed panel decides to close it and the panel becomes
unopenable by its own key — which is what this test's second arm names.
Helper — open_only
Not a test. It spells formula_panel_open, category_panel_open and
view_panel_open as fields rather than looping over Panel::ALL through an
accessor, and that is the whole evidentiary value: a permutation in any mapping
between the Panel enum and those three booleans has to disagree with this helper.
command/cmd/text_buffer.rs
writing_the_search_buffer_targets_the_search_query
Writing the "search" buffer means writing the search query, and write_buffer is
the one place that knows it (improvise-gdl). Every buffer-writing command routes
through it, so clear-buffer search clears the query rather than parking an unread
entry in the map.
colon_wq_saves_then_quits
improvise-git proposed deleting the SaveAndQuit command and binding ZZ to a
Binding::Sequence [w, force-quit] instead — the repo's "do not name a command
AAndB" principle.
That premise is incomplete, and this test is why. wq is not only a keybinding: it
is a registered command name, so :wq reaches it through the registry, and
Quit's own unsaved-changes message advertises it ("Use :q! to force quit or :wq to
save+quit"). A keymap Sequence is not reachable from the : line, so deleting the
command deletes the ex-command with it.
Keeping the command and adding the Sequence would leave two independently-written spellings of "save then quit", which is the duplication this codebase keeps curing rather than creating. So this pins the surface, and the conversion is declined on it.
colon_enter_edit_at_cursor_is_not_snapped_back_to_normal
improvise-6sq: ExecuteCommand appends change_mode(Normal) unless some effect
reports changes_mode(). EnterEditAtCursor sets view_state.mode but did not
override it, so :enter-edit-at-cursor editing produced
[RebuildLayout, EnterEditAtCursor(Editing), ChangeMode(Normal)] — the trailing
snap-back undid the edit mode and the user never got a cursor in the minibuffer.
Asserting on changes_mode() alone is not enough here: the appended
ChangeMode(Normal) reports true itself, so the assertion has to name the
snap-back it is ruling out.
colon_import_stays_in_the_wizard_on_success
improvise-toc: StartImportWizard swaps the mode only when the file parses, so a
per-type changes_mode() cannot describe it. Reporting false made
ExecuteCommand append a return-to-Normal on top of the success path:
:import d.json opened the wizard and was immediately snapped back, leaving the
wizard drawn under the Normal keymap. (Gotcha 31.)
colon_import_falls_back_to_normal_on_failure
The other half of the same bug: reporting true unconditionally would strand the
user in CommandMode when the import fails. A failed :import must still fall back
to Normal.
colon_view_back_respects_whether_history_exists
The ViewBack half of improvise-toc: it restores the stored frame's mode only when
the back-stack is non-empty. With history present, :view-back must keep the mode it
restored; with an empty stack it falls through to Normal like any other no-op
command.
command/cmd/wizard.rs
name_model_enter_builds_the_model
Enter on the name step builds the workbook into the wizard rather than advancing the step machine; every other printable key edits the name (gotcha 22).
done_confirms_on_enter_and_steps_back_on_esc
On the confirmation step Enter installs the built workbook and Esc steps back — it does not throw the import away, and no key closes the wizard outright (improvise-907, from improvise-c6j).
a_formula_owned_cell_is_projected_as_derived_and_its_siblings_are_not
improvise-ir7l. project::cell_id decided Record vs Aggregate purely on
whether a regular category sat on Axis::None; it never asked whether the
_Measure coordinate named a formula target. So a formula cell was a Record,
render.rs::editable said true, and a write to it was accepted, stored and
then shadowed forever by formula_cache.
The fault injected: replacing the derived lookup with None, which is the
pre-fix code exactly. The test then reports Record(RecordCell { measure: Some("Profit") }) where it wants Derived { target: "Profit" }. The siblings
are asserted in the same test so a producer that answered Derived for every
cell could not pass.
an_aggregating_formula_cell_is_still_projected_as_an_aggregate
The negative arm of the same change, and the one a careless fix breaks: gotcha
10 says drilling into a formula cell is supported, so Aggregate must still
win where the cell aggregates. Proved discriminating by returning derived
before the aggregate branch — the cell then reads Derived and becomes
undrillable, with nothing else in the suite noticing.
edit_or_drill_refuses_a_formula_owned_cell
The TUI half. EditOrDrill never asked the question either, so the terminal
offered the same editor over the same shadowed cell. Landing only the
projection half would have made the two front ends disagree, which is the
defect design-principles §5 is about. Fault injected by deleting the guard: the
command returns [SetBuffer{edit}, ChangeMode(Editing)] again.
a_save_clears_the_dirty_marker_on_a_client_although_nothing_is_dirty
improvise-z754, and the arm the whole design turns on. effect::Save is
EffectKind::ViewOnly, so Dirty::of reports NOTHING — the test asserts
that first, so it cannot quietly stop being the interesting case if Save is
ever reclassified. A title bar gated on Dirty would therefore leave every
client showing [+] for a document that has just been saved, forever, with
nothing in the stream to correct it.
Fault injected: wrapping document_update in if !dirty.is_empty(), which is
the obvious implementation and the wrong one. The test fails on the cleared
arm. a_title_bar_that_did_not_move_is_not_projected is its counterweight —
without it, "project the title bar every batch" would pass this one.
the_title_bar_names_the_document_without_naming_the_path
The fixture's path is /home/someone/private/books.improv on purpose: a test
whose fixture is a bare file name cannot tell file_name() from the whole
path, and the failure mode is a socket server handing a remote client the
host's directory layout.
a_creating_column_is_named_rather_than_left_as_no_cell
improvise-poia. Both + and +cat projected None, which the protocol
defines as "no projected cell" — the same conflation gotcha 42 names for
aggregates, arriving a third time.
The test walks every column of a records grid and counts, rather than
asserting the two creating columns alone. That matters in the direction a
weaker test fails: a producer that answered Create for everything would pass
a two-position check and fails this one. The fault injected — returning None
for both creating columns, which is the pre-fix code — reported three nameless
columns where one was allowed. Since improvise-exdg the coordinate column is
named too (CellId::Coordinate), so the count of nameless columns is now zero.
See a_coordinate_cell_is_named_rather_than_left_as_no_cell.
a_records_coordinate_is_text_however_it_is_spelled
improvise-tom1. project::cell_display's records arm called records_display
— which returns a String — and then inferred numeric-ness with
text.parse::<f64>(). A records coordinate column holds an item name, so a
Year category with items 2024/2025 projected a coordinate the browser
right-aligns as a number.
Nothing in the suite could see it: every records fixture in the tree names its
categories Region/East/North, so the guess happened to be right. The
fixture here is the whole test — a category whose items are spelled like
numbers — and the measure column beside it is asserted numeric in the same
test, so a producer that answered false for everything would not pass.
Fault injected: the parse::<f64>() guess restored verbatim. It reports the
coordinate as numeric.
a_staged_drill_edit_reaches_its_own_session_and_no_projection
The user's ruling, pinned in both directions (improvise-tom1). A remote client
must NOT see another client's staged drill edits, so grid_of must project the
committed value; the session that staged one must still see its own typing,
through App::display_value_of.
Asserting only the second half would pass against a producer that leaked, and
asserting only the first would pass against one where staging did nothing at
all — so both are here. The fault injected is the leak: teaching
cell_display's records arm to prefer drill_state.pending_edits, which is
the plausible "fix" this ticket exists to forbid. The projected cell then reads
999 instead of 1.
The fixture takes its DrillState::records from the layout the app really
built, because an empty frozen snapshot silently empties the drill grid and the
test then asserts nothing about a cell that does not exist — the first version
did exactly that and panicked on expect.
a_page_axis_carries_its_items_and_a_filter_axis_does_not
The producer half of improvise-p1ym, added during self-review because
a_filter_axis_is_projected_but_not_cycled builds its own PageAxis by hand —
a test that fabricates the very data the code under test is supposed to
produce, which is this repo's characteristic defect. Only this one can say what
grid_of actually emits.
The fixture puts Quarter on Axis::Page and Year on Axis::Filter, which
is the shape a drill makes (gotcha 27), and asserts the whole projected list in
one comparison so a producer that emitted items for everything or for
nothing both fail. Fault injected: let navigable = true, which is
filtering_cats() leaking into the protocol — the exact trap the ticket names.
Year comes back carrying ["2024"].
a_records_grid_has_no_column_for_a_formula_target
A records grid has no column for a formula target that is only a formula
target, because GridLayout::records_columns builds its measure columns from
Category::ordered_item_names — the explicit _Measure items — while
add_formula adds none (gotcha 7). Fault injected: swapping
ordered_item_names for measure_item_names in records_columns, which is a
plausible tidy-up — a Profit column appears.
This test's original premise was FALSE, and the review caught it. It was
written claiming the records branch of cell_id was safe because "a formula
target lives only in measure_item_names". An item can be both an explicit
_Measure item and a formula target, and improvise-poia — in the same PR —
made the + column that builds exactly that reachable. So this test asserted
the accident in the one configuration where the accident holds, and its own
failure message named a repair for a case it could not reach. The real guards
are a_records_column_a_formula_owns_is_derived_not_a_record and
typing_into_a_records_formula_column_is_refused_on_the_key_path; this one now
claims only what it can see.
a_records_column_a_formula_owns_is_derived_not_a_record
The hole improvise-ir7l's first fix left, found by adversarial review. A measure
can be both an explicit _Measure item and a formula target — the records
+ column creates precisely that — and cell_id's records branch asked
formula_for about nothing, so it projected CellId::Record{measure:"Profit"}.
A write there is accepted, stored, and then shadowed forever by formula_cache,
which is the defect the ticket exists to close, surviving in records mode in
both front ends since editable is Record-only.
The repair asks the document about the record's real coordinate —
GridLayout::record_cell_key, the one mapping from a records position to a
model cell — never the synthetic (_Index, _Dim) key, which names no measure
and so can never match a formula. Fault injected: deleting the lookup, which is
the pre-review code. Profit reads back as a Record. The Revenue column is
asserted still writable in the same test, so a producer answering Derived for
everything could not pass.
typing_into_a_records_formula_column_is_refused_on_the_key_path
The same defect through App::handle_key and no test API at all: R, walk to
+, type Profit, Enter, Esc, then i on the column just created. It exists
because the projection half and the EditOrDrill half are separately
breakable, and a unit test of either would have missed the other — proved by
injecting each alone. Restoring cell_id while leaving CmdContext:: owning_formula reading the raw key fails on the mode assertion; the reverse
fails on the identity assertion.
One fixture note that cost a debugging round: CommitAndAdvance re-enters
editing on the cell it advances to, so the mode after Enter is
RecordsEditing and an i there is a literal character. Without the Esc the
test passes its mode assertion for the wrong reason — nothing happened at all.
a_save_tells_the_client_the_document_is_clean
improvise-z754's own stated failure mode, shipped: "every client showing a [+]
for a document that has been saved". Host::save applied effect::Save and
returned only a possible Rejected, never projecting, so the dirty marker
cleared outside any projection and no client was ever told.
The z754 test that was supposed to cover this called sessions.project_all by
hand — which is exactly the call the production path omits. It is not a test
that compares a thing with itself; it is a test that exercises a call the
deployment never makes, which is the subtler shape. This one goes through
Host::handle(ClientFrame::Save).
Two assertions guard the fixture: the edit must first report dirty: true, and
app.model_state.dirty must really be false afterwards — otherwise a host
that saved nothing would pass by projecting nothing. Fault injected: returning
an empty vec from the success arm. No Document update is emitted.
the_automatic_persist_tells_the_client_the_document_is_clean
The worker's half of the same defect, and a different cause: on_frame built
out via frames_for and then called persist(), so the projections were
built while dirty was still true. The browser is the deployment where this
bites hardest, because nothing there ever sends ClientFrame::Save — the worker
persists on its own behalf (gotcha 45), so this was the only path.
Fault injected: dropping the re-projection after persist(). The client's
DocumentCache stays dirty. MainThread grew a document mirror and a
projection dispatch for it, rather than a bespoke reach into the worker — the
point is what a client folding the real stream ends up holding.
A first version asserted only !document.dirty, and DocumentCache::default()
is already clean — so it could not tell "told it cleared" from "never told
anything", and passed under an injection that stopped projecting the widget
entirely. Found in re-review. It now records every marker the client is
told and asserts [true, false] for the edit, with the bootstrap's own clean
marker cleared first so the sequence is about the edit rather than the
handshake. The stronger form fails under both injections.
a_refused_or_duplicate_add_category_moves_no_counter
improvise-75ld. App::apply_effects bumped layout_version and raised
model_state.dirty off the EffectKind before the abort_effects check, so
the refusing effect counted itself. Two arms, and the second is the reachable one:
a 13th category is refused (MAX_CATEGORIES) and aborts, while a duplicate
add-cat Region returns Ok and does not abort at all — a pure no-op that still
refused every other client's next Command::Model with Rejection::LayoutMoved
and made the browser worker rewrite the whole OPFS document (gotcha 41).
Injected fault: put the two bumps back on the declared kind, inside the loop
(if matches!(kind, EffectKind::Model(ModelChange::LayoutGeometry))). Both no-op
arms fail; so do neither_shape_counter_moves_for_an_accepted_no_op,
both_shape_counters_move_for_every_reshaping_command and
a_configuration_change_that_draws_nothing_is_still_a_document_change, with 450
other tests still passing — the build ran.
The positive arm is not decoration: without it, "never bump anything" passes the
two no-op arms. It also fails under a second injection — dropping the
document_config comparison — because AddCategory is LayoutGeometry and the
declared half of dirty no longer covers it.
both_shape_counters_move_for_every_reshaping_command
improvise-pdvm. AddItem, AddItemInGroup and RemoveItem declare
ModelChange::CellData and reshape the grid. Since improvise-9mo9 they call
App::reset_cursor, so cursor_epoch moved while layout_version did not: a
client was told its cursor was invalid while its layout basis tier still read
current, and a batch composed against the old shape was still accepted.
It drives reshaping_commands() — the same table
every_reshaping_model_command_invalidates_every_cursor drives, extracted into a
shared function precisely so the two cannot answer for different cases — and
asserts the grid shape really moved before asserting either counter, so a case
that reshapes nothing fails loudly rather than proving nothing.
Injected fault: as above. Fails naming SetAxis, then AddItem once the earlier
cases are removed.
a_configuration_change_that_draws_nothing_is_still_a_document_change
The third case the shape/config split creates, and the one that forced two
fingerprints rather than one. hide_item records a name whether or not the model
has one, and SetNumberFormat writes the file while moving no row or column: both
are dirty and neither is layout_version. A single fingerprint covering the
persisted configuration made a ghost hide bump the basis; one covering only the
drawn shape let a format change go unsaved.
Injected fault: remove the document_config comparison in observe_reshape.
Fails naming HideItem { category: "Region", item: "NoSuchItem" }.
a_reshape_of_one_view_leaves_a_client_on_another_view_alone
improvise-nynk. improvise-jytm made "which view a client is on" per-client;
Sessions::adopt_cursor_epoch was not updated and still reset every mirror
and returned one Broadcast::ToAll(Command::ResetCursor). A reshape of view
Default therefore threw away the cursor of a client sitting on Second, which
nothing reshaped.
The fixture is two views of one document with different axes, and the bystander
switches to Second and scrolls there before the asker reshapes Default. The
positive arm is kept in the same test — the asker is on the reshaped view, is
reset, and is told — because without it "never invalidate anyone" passes.
Injected fault: drop the cursor_invalidated filter (.filter(|_| true)). Fails
naming the bystander's thrown-away cursor, with 453 other tests passing.
an_unknown_reshape_set_invalidates_everybody
The fail-safe arm of the same narrowing, tested separately because it is the half gotcha 24 calls strictly worse than doing nothing. An empty reshaped set means the batch moved a cursor with no view observed to change shape; the answer must be "everybody", not "nobody", or the diff believes each client already holds origin-positioned content and sends nothing, leaving them stale permanently.
Injected fault: delete the reshaped.is_empty() || disjunct. Fails by name. That
injection is why the predicate is a named function rather than a closure — a
fail-safe nothing can point at is a fail-safe that gets tidied away.
a_batch_that_changes_the_layout_and_then_saves_ends_clean
A regression introduced by the first version of the observed-dirty change and
caught in review of PR #70, not by the suite — the workspace was green at 1400 with
this broken. observe_reshape ran after the effect loop and re-raised
model_state.dirty from the document_config comparison, while Save had cleared
it inside the loop. So gotcha 45's stated ordering property — "Save clears it
in loop order, so a batch that edits and then saves ends clean" — survived for
CellData/WholeWorkbook and was broken for LayoutGeometry.
[SetNumberFormat, Save] reported a successful save and left the document dirty:
the TUI draws "Saved to …" while Quit's check refuses a later :q, and the
browser worker — whose persist trigger is the flag — pays a whole-document OPFS
rewrite. Reachable from :format ,.2f . w and from any Binding::Sequence pairing
a geometry effect with a save.
The fix is position, not logic: the comparison moved inside the loop, taken after
each LayoutGeometry effect against the value held after the previous one. Both
arms are in the test — [SetCell, Save] never left the loop, so the geometry arm
alone could not say whether the rule is general — and each asserts the save really
happened (last_batch_aborted, and a status line starting "Saved") before
asserting the flag, or a failed save would pass the dirty assertion for the wrong
reason.
Injected fault: move the comparison back after the loop. Fails naming the layout arm, with 454 others passing.
The lesson worth carrying: an observation that is correct about what changed can still be wrong about when it is read. Everything above this entry argues that observing beats declaring; this is the cost of that move, and it lands precisely on the one effect that unsets what the observation sets.
the_published_basis_names_the_records_the_grid_draws_and_no_others
improvise-23qd. The set of records a Projection::Basis carries was derived
twice — ClientSession::basis_update, the producer, and server_truth, a
#[cfg(test)] helper that is the yardstick every fold test measures against.
So a change to which records get published moved the thing and its own measure
together, and every test stayed green whatever the change was. Not a helper that
fabricates an input; a helper that fabricates the expected output.
Both now call published_basis. Note what that alone does not buy: the
ticket's stated proof ("alter the filter and watch the basis tests fail") is
impossible once one function feeds both sides — they still move together. A shared
derivation removes the drift and removes the yardstick with it.
So the yardstick is spelled out rather than derived: this test names North, South
and East by hand for app_with_a_reshapeable_grid's three records over six cells,
and asserts that a grid where every cell is an Aggregate publishes none —
because an aggregate names no record, so there is nothing to claim a version for.
Fault injected: .take(2) on the one surviving filter — the ticket's exact "change
which records get published" — fails this test and nothing else in the crate.
A second injection, record_version_of(..) + 1, is caught instead by the two
end-to-end basis_check tests, which is the split worth knowing: the counters have
other guards, the membership had none.
The fixture carries its own premise check. The first version assumed a new category
lands on Axis::None; it lands on Column, and the assertion said so instead of
the test quietly proving nothing.
an_aborted_batch_bumps_layout_version_for_what_it_did_run
improvise-2avl, with its negative twin an_aborted_batch_bumps_nothing_for_effects_it_never_ran.
PR #70 moved the layout_version bump from "once per LayoutGeometry effect,
inside the loop" to "once per batch, after the loop". Gotcha 40 records the in-loop
position as load-bearing — an aborted batch must not claim effects it never ran —
and the author's argument for why end-of-batch is safe for layout_version
specifically (nothing ever clears it, so an end comparison sees only what ran,
whereas dirty must stay in the loop because Save unsets it mid-batch) is sound
and was written into context/code/command.md. An argument is not a test, and
neither arm had one.
Both arms are proved independently, which is the part that earns them: bumping
unconditionally fails only the second test, and skipping observe_reshape on abort
fails only the first. A single injection failing both would not have shown that the
two tests see different things.
Each fixture asserts app.last_batch_aborted() before asserting anything about the
counter, because a batch that did not actually abort would pass both arms while
testing neither.
sorting_the_store_in_records_mode_invalidates_the_cursor
improvise-ni6k names SortData as its sharpest case: it changes neither the
row count nor any label, and every position still names a different record. A guard
written as "did the shape change" misses it; pending_edits are keyed by row index
(gotcha 1), so row 0 being West and then East is gotcha 18's misdirected-write
disease arriving through a different door.
view_frame therefore hashes each record's coordinates alongside the axis
entries, because records-mode row_items are positional — 0,1,2… whatever order
the store is in. Fault injected: deleting those three lines fails this test and
nothing else in the workspace.
The fixture trap, which is the durable part. PR #78 first shipped without the
coords hash, on the stated ground that no fixture could be built where SortData
reorders anything. That was wrong, and the reason is worth more than the fix:
ToggleRecordsMode pushes effect::SortData as its first effect, so a
records-mode fixture built by pressing R has an already-sorted store by
construction and cannot demonstrate anything order-sensitive. The store itself was
fine all along. So this fixture sets _Index/_Dim directly rather than going
through the key path.
The test asserts the count and the labels are unchanged before asserting the cursor moved, so it cannot pass by accidentally becoming an ordinary shape change.
paging_with_nothing_pruned_invalidates_no_cursor
The negative twin of paging_across_a_prune_that_drops_a_row_invalidates_the_cursor,
and the reason view_frame hashes no page selection.
improvise-7kvw is real only under prune_empty: a page flip changes which records
are visible, not which categories frame them, so with pruning off a grid position
keeps naming the same cell. PR #76 ruled that homing the cursor on every [/]
regardless — [/] being navigation a user does constantly — is worse than the
hole. Hashing the page selection does exactly that.
The seductive argument for hashing it anyway is that two pages with identical row
labels really are different data. True, and irrelevant: that is record_versions'
question, and view_frame answers the cursor's. A basis argument applied to a
cursor decision is how this regression got written in the first place.
Fault injected: restoring the three page_selections lines fails this test and
nothing else. The positive arm keeps passing either way, because
with_frozen_records applies prune_empty and so already sees a dropped row in
the axis entries — which is why the negative arm is the one that had to exist.
a_write_that_prunes_a_row_out_of_another_clients_view_invalidates_it
improvise-mcq4's sting, found by the adversarial review of PR #78. The mcq4 fix
narrowed the frame fingerprint from every view to the active one, on the strength
of a measurement showing the narrowing cost nothing. It bought its speed partly
with correctness: a CellData write that prunes a row out of another client's
view became invisible. view_shapes is config-only — it hashes the prune_empty
flag, not which rows survive — so last_reshaped_views is empty, the one-view
frame covers only the host's, and a client on the other view is never sent
ResetCursor while its grid loses a row underneath it. That is gotcha 24's
"strictly worse than doing nothing": the diff then believes the bystander is
current, so it stays stale permanently.
The repair keeps the narrow one-view frame on the LayoutGeometry arm — the
held-key path improvise-748f exists for, where other views cannot reframe because
view_mut() writes only the active one — and restores the all-views frame on the
CellData arm. Measured: 0% on the held-key path, +5.7% on an 18k-cell
two-view SetCell, scaling linearly in view count.
Writing this test found a second bug, which was masking the first. The
precondition failed at first: grid_in built a non-active view with
GridLayout::new, which does not apply prune_empty, so a client on another
view was projected three rows where its own config gives two. Their grid never
pruned, so it never lost a row, so the staleness was unobservable — a bug hiding a
bug. It is the third instance of the new-versus-with_frozen_records trap
(view_frame was the second), and fixing it broke no other test.
Fault injected: removing the all-views arm fails this test, plus
sorting_the_store_in_records_mode_invalidates_the_cursor (SortData is
CellData) and both cursor-audience tests — four in all.
a_switch_then_a_geometry_effect_compares_within_one_view
Found by the same review. frame_basis was taken once at batch open against
view_state.active_view; SwitchView is ViewOnly, so it did not re-base, and
the next model effect hashed the new view and compared it against the old
view's hash. A near-certain mismatch, so a spurious reset_cursor() and a wrong
name in last_reframed_views.
Today's instances are benign — DrillIntoCell and EnterRecordsMode both emit
[CreateView, SwitchView, SetAxis, …] in one batch, but against a view they just
created. The harmful case needs a switch to an existing view with a saved cursor
plus a model effect in the same batch, which a .-chained : line reaches. Low
severity, unsound mechanism.
The basis is now keyed by view name and re-bases rather than comparing when the
name moves. Fault injected: dropping the was == self.view_name() guard fails
this test, with cursor_epoch moving for a batch that reframed nothing.
improvise-9c9o moved where the re-base happens, and this test was unchanged by it because it names the property, not the mechanism — the re-base now fires on the switch rather than on the next geometry effect, and a batch that reframes nothing still invalidates nothing.
Its fixture carried a line that did nothing at all:
views["Second"].clone_into(&mut views["Default"].clone()) clones into a
temporary that is immediately dropped. It read as the thing making the two views
differ, so had create_view ever started copying the active view's config this
test would have gone vacuous with that line standing there looking like it
prevented exactly that. It is deleted and replaced by the assertion it was
pretending to be: the two views' view_frame hashes really differ.
a_geometry_effect_after_a_mid_batch_switch_still_invalidates_the_cursor
The other side of the guard above, and introduced by it. PR #78 round 1 compared
frame hashes across a view change, which threw a cursor away spuriously; round 2
added was == self.view_name(), which fixed that and also exempted the first
geometry effect after a mid-batch SwitchView from the check altogether —
there is no basis for the new view yet, so the code re-based and skipped.
Passes on pre-#78 main and on round-1 head; fails at the merged head.
Direction is why this is worth more than the bug it replaced. Round 1 kept a cursor it should have thrown away; round 2 threw away nothing it should have kept — which is gotcha 35's expensive side, since a stale cursor has nothing in the stream to correct it where a spurious invalidation costs one projection.
The fix is the ticket's: the basis follows active_view, re-based in the
loop as soon as the name moves, rather than at the next geometry effect. It did
not need SwitchView to stop being ViewOnly and it is not a protocol
change — the basis is App's own bookkeeping, and nothing about it is on the
wire. The was == self.view_name() guard is gone from the comparison because it
is now unreachable: by the time the geometry arm runs, the name and the basis
agree by construction.
What it changes elsewhere, and why that is fine: DrillIntoCell and
EnterRecordsMode both emit [CreateView, SwitchView, SetAxis, SetAxis, …], so
they now reset the cursor on the first SetAxis where they used to skip it and
reset on the second. Same outcome, one effect earlier. DeleteView is the one
LayoutGeometry effect that moves active_view itself; the re-base fires for it
too, so the geometry arm compares the new view against itself and resets nothing
— which is what the old skip did.
The fixture measures the row count the transpose must move rather than spelling
it: a first version asserted row_count() == 1 and the real answer is 2, which
is the shape of a precondition that would have gone quietly wrong later. It
fails on the unfixed tree at the epoch assertion, left: 0 right: 0.
a_frozen_drill_does_not_take_its_opening_frame_from_the_held_layout
improvise-mcq4's fix reads the batch's opening frame off App::layout rather than
building a second pruned layout — that is where the 4.1×→1.9× came from. It is
guarded on drill_state.is_none(), because a drill freezes the record list and the
held layout is then not what view_frame would build for that view.
The guard was correct, load-bearing, and tested by nothing: reverting it to the
unconditional form left all 1439 tests passing. Textbook
doctrine(a_careful_comment_is_weak_evidence_that_anything_checks_it), and it was
the most carefully argued comment in the PR.
The fixture freezes one record where the live query finds three — asserted, so a
freeze that did not narrow could not pass — then runs a SetNumberFormat batch,
which reframes nothing. Without the guard the opening basis is the frozen layout
and the in-loop hash is the live one, so they differ and the cursor is reset for a
batch that changed no row. Fault injected: replacing self.drill_state.is_none()
with true fails this test and nothing else in the workspace.
a_clients_grid_does_not_depend_on_which_view_its_host_is_on
improvise-t6db. The one-line grid_in fix landed in PR #78 with no test of its
own; this is the general property the reviewer specced — host-view
independence: grid_in(app_on_A, "B", vp) == grid_in(app_on_B, "B", vp) for
every ordered pair of views. It needs no second derivation of what a view's
config means, which is why it is a property and not a second copy of
GridLayout.
The trap is the fixture, not the assertion. The reviewer's first version made
the two views differ by turning prune_empty off in the second — plenty of
differing config, and vacuous: with nothing to prune, GridLayout::new and
GridLayout::with_frozen_records agree and the property passed on the buggy
tree. So app_with_three_views_and_an_empty_row gives Service no cell at all and
leaves all three views pruning, and the test asserts every view really is two rows
before comparing anything — a fixture that stopped pruning would fail there rather
than passing quietly.
The property is stated with drill_state.is_none() asserted, because a frozen
drill snapshot is the one thing grid_in cannot reproduce for a view the host is
not on (with_frozen_records(.., None) on the elsewhere path).
Fault injected 2026-09-21: restoring GridLayout::new in grid_in — the pre-#78
line — fails this test at the assert_eq!, not at the precondition. It also fails
a_write_that_prunes_a_row_out_of_another_clients_view_invalidates_it and
a_hosts_none_cats_leak_into_another_views_formula_columns: three tests, not
the one an earlier draft of this entry claimed. The #78 fix has more coverage than
it was credited with.
The none_cats asymmetry is REAL, and the first draft of this entry reported it
as unobserved from a fixture that lacked the condition. That correction is worth
more than the test. The claim was that the fixture put Product on None in
Default and on Column in Second; it did not — add_category defaults a new
category to Column and nothing moved it, so all three views had none_cats
{_Index, _Dim}, the stated input to the hazard was absent, and Default and
Second were indistinguishable to grid_in apart from a number_format that is
itself a no-op. An unprobed path reported as a negative result, which is
doctrine(an_injection_that_does_not_fail_names_an_untested_path) applied to a
setup, and the expensive kind: it stops the next person looking.
Setting Product to Axis::None explicitly made the property fail on the first
run, and not on the value of a cell — Second lost both Total columns
entirely when the host stood on Default. Filed as improvise-ac2z, pinned by
the characterization test below, and the property now skips pairs whose
none_cats differ. That skip is a bug being worked around, not a carve-out;
it asserts compared > 0 so a future fixture cannot skip everything and pass.
a_window_narrows_the_cells_and_neither_the_totals_nor_the_widths
improvise-jmjz. project::grid_of evaluated the whole grid roughly three times
per frame — compute_col_widths walked every cell for its text and then every
cell again for the column totals it must size, and grid_of then evaluated the
window a third time. ui/viewport.rs::grid_body now evaluates each cell once and
derives the widths, the totals and the window from that value.
That fusion creates a hazard the three separate passes did not have: one walk now serves three consumers with different scopes. Two faults follow, not one — a window that leaks into the totals makes a column total the sum of the visible rows, and a window that leaks into the widths sizes a column by what happens to be on screen. Neither is visible from the output: the numbers simply get smaller, consistently.
The fixture took three attempts, and what each one was blind to is the entry.
First: rows of 1, 20, 300000 under a measure named Revenue, asserting
col_widths[0] > "300021".len(). It could not see a windowed width at all —
Revenue is 7 characters against a 6-character value, so the assertion passed on
the header alone. Worse, its other half was structurally incapable: comparing
grid_body's widths against compute_col_widths cannot test windowing, because
compute_col_widths is grid_body with an empty window. Windowing degrades
both sides identically, both collapsing to the MIN_COL_WIDTH clamp. The same
function twice is not a check, and it reads exactly like one. Measured by the
PR #92 review: if drawn { widen(..) } left 1476 passing, 0 failing.
Second, the reviewer's: 1, 888888888, -888888888 with a one-character
measure name, so the total cancels to 1.0 and neither it nor the header can
supply the width. This fixed the width half and broke the other: the windowed row
is 1.0 and the cancelled total is also 1.0, so the totals assertion went
blind to the row-window fault, which the widths assertion then happened to catch.
Two assertions, one no longer discriminating, and nothing saying which.
Third, and current: the same values with the large ones first, so the window
holds 888888888 while the total is 1.0. Each injection now fails on the
assertion that names it:
for ri in rows.clone()— the whole walk windowed → totals[888888888.0]against[1.0]if drawn { widen(..) }— widths only → widths[10]against[11]
10 rather than 5 is a smaller gap than the reviewer's fixture gave and a
better witness: the missing character is the minus sign, which only
-888888888 carries and the window does not contain. The test also asserts the
window really is narrower than the grid, so a fixture that stopped windowing
would fail at the precondition.
What this test does not do is prove the speedup, and no test can — the outputs
are identical by construction, which is why the whole suite passed unchanged
across the change. The evidence is the benchmark, before and after, in
context/code/command.md: about 3x, the number of passes removed.
a_formulas_columns_survive_a_host_that_aggregates_differently
improvise-ac2z, fixed. This replaces the characterization test
a_hosts_none_cats_leak_into_another_views_formula_columns, whose message said
that failing meant the bug was fixed; it did, and it is gone.
App::rebuild_layout calls recompute_formulas(&none_cats) for the host's
view, populating Model::formula_cache. project::grid_in on the elsewhere
path built the other view's GridLayout against that same cache, which is keyed
by the coordinates the recompute used: with Product on None in the host's view
the Total cells are keyed without a Product coordinate, a view with
Product on Column asks for (Region, Product, _Measure=Total), finds nothing,
and — prune_empty being on — drops both Total columns. Not a wrong number:
absent columns.
host on Second, projecting Second 6 columns, Total present
host on Default, projecting Second 4 columns, no Total at all
The fix is grid_in::rederived_for: where the target view aggregates over
different categories than the host's, it clones the model and recomputes the
formulas for that view's none_cats. Three conditions gate the clone —
another view, different none_cats, and at least one formula — so the common
case pays nothing and a formula-free document pays nothing ever.
This is an interim, and the cost is the reason. A clone of the whole model
plus a fixed-point recompute, per projection, on exactly the path that is broken
today. The ticket's first candidate — keying formula_cache by none_cats so
one Model can hold more than one view's derived values — is the durable answer
and is filed separately; it is an improvise-core change and did not belong in
this wave's ground. What is bought meanwhile is correctness on a path that was
silently wrong, doctrine(the_elsewhere_path_is_only_as_correct_as_the_inputs_it_can_rebuild).
Fault injected: rederived_for's condition forced to false. Two tests
fail — this one, and
a_clients_grid_does_not_depend_on_which_view_its_host_is_on, whose none_cats
carve-out was removed at the same time and which is the general property this
one is an instance of. The test asserts its two preconditions first — that
Second on its own really draws the formula's columns, and that the two views
really do aggregate over different categories — so a fixture that lost either
fails rather than passing on two empties.
a_view_named_command_lands_on_the_view_it_names
improvise-lltt, the oldest open P1 here, opened by improvise-jytm making
ViewState::active_view authoritative per client.
Nine ModelCommand variants configure a view — SetAxis, CycleAxis,
TransposeAxes, SetPageSelection, HideItem, ShowItem, ToggleGroup,
TogglePruneEmpty, SetNumberFormat — and each named its view implicitly:
model_effects built an effect that wrote App::view_mut(), which is the
host's slice. A client that had switched to Second therefore moved
Default: its own grid did not change, and every other client's did.
The fix is the protocol change the ticket asks for. Each of the nine carries a
view: String, model_effects maps it to ViewTarget::Named, and the nine
effects carry a ViewTarget rather than reading an ambient one. There is no
operandless form left: App::view_mut() is deleted, replaced by
view_mut_of(&ViewTarget), so a future geometry effect has to say which view it
writes. The host's own key path passes ViewTarget::Active, which is right
because the TUI is the host and has no session to ask.
Fault injected: model_effects' SetAxis arm rebuilt with
ViewTarget::Active (the pre-fix behaviour, with view bound and discarded so
it still compiles). The test fails on the asker's own view never moved —
Second's Channel stayed on Row while Default's moved to Page.
The fixture must seat the asker somewhere the host is not, and the two views must
disagree about the category being moved: Channel is Axis::None in Default
and Axis::Row in Second, and the command asks for Axis::Page, so both
assertions separate the fixed code from the broken code in both directions.
a_reframe_of_a_view_the_host_is_not_on_invalidates_its_clients
improvise-lltt's second half, and the part the ticket did not predict. Naming the
view made a reframe of a view the host is not on reachable for the first time,
and Sessions::adopt_cursor_epoch could not see one: it returned early unless
App::cursor_epoch had moved, and the epoch is bumped only by reset_cursor,
which apply_effects calls only for the host's own view. So the clients on
the reframed view were reset by nobody and told nothing — gotcha 24's
strictly-worse-than-nothing state, permanently stale rather than stale until the
next scroll.
App::last_reframed_elsewhere is the second trigger. Its opening frames are
taken lazily inside App::view_mut_of, which is the one door such a write goes
through, so the held-key path still hashes exactly one view: the wide-observation
cost improvise-mcq4 declined is not paid.
Fault injected: the gate restored to if !host_moved { return Vec::new(); }.
The test fails on a client on the reframed view was never told.
Two negative arms, and they are not decoration. A third client left on Default
must keep its offset, or "invalidate everyone" passes; and
paging_with_nothing_pruned_invalidates_no_cursor is what forbids relaying
against last_reframed_views, which is a union with the reshaped set and so
fires for a page selection that pruned nothing. That test failed during
development and is why the elsewhere set is precise.
a_command_naming_a_view_the_document_has_not_got_writes_nothing
App::resolve_view falls back to the document's default for a name the document
has not got, which is right for reading — a slice naming a deleted view must
not panic in rebuild_layout — and wrong for writing, where it would move a
view nobody asked about. view_mut_of therefore looks the name up directly and
returns None.
Fault injected: view_mut_of's Named arm routed through
self.resolve_view(name).0. The test fails on a name the document has not got fell back onto a view that has one.
The window this closes is narrow — Sessions::adopt_deleted_views already moves
a mirror off a deleted view and tells it — but it is a race, not an
impossibility, and the silent alternative is a write to the wrong document state.
Not closed here: the client hears nothing. A Rejection variant for it is filed
rather than smuggled in, since Rejection is wire vocabulary.
the_diff_tells_a_client_the_grid_became_a_records_grid
The producer half of GridCache::records (improvise-e5pf). grid_of setting
the field is exercised by leaving_a_panel_in_records_mode_lands_where_the_host_lands;
the diff is a separate arm, and a client that connects before records mode
is entered learns only through it.
Both preconditions are asserted — the fixture starts on an ordinary grid, and
R really reaches a records one — so the comparison is between two answers
that had something to disagree about.
Fault injected: the records arm deleted from grid_updates. The test
fails on the client is never told. The negative arm (an unchanged flag is not
resent) is what stops "always push it" from passing.
a_batch_that_leaves_records_settles_out_of_records_normal
improvise-4dpj, gap 1. settle_normal_mode matches Normal | RecordsNormal,
and replacing that with a one-way AppMode::Normal left the entire workspace
suite passing. The existing crossing test,
a_batch_that_leaves_records_and_then_sets_normal_lands_in_normal, drives the
layout both ways, but both of its branches end in enter-mode normal, so the
mode handed to the settle is AppMode::Normal in both directions. It
discriminates "settles at all" and "settles against the ending layout", and not
"settles from either mode".
This drives the production repro: from a records grid,
apply_effects([SetAxis { "_Index", Axis::None }]) leaves
layout.is_records_mode() false with the mode still RecordsNormal, so the
RecordsNormal arm is live and not defensive. Both the records-ness of the
grid and the mode before the batch are asserted as preconditions, so a fixture
that stopped entering records mode fails loudly rather than proving nothing.
Injected fault: if matches!(self.view_state.mode, AppMode::Normal). Fails by
name, left: RecordsNormal, right: Normal. It is the only test in the
workspace that fails.
switching_views_across_records_ness_settles_against_the_view_switched_to
improvise-4dpj, gap 2 — raised by ChatGPT Codex against PR #91 and verified
correct on main before being tested. SwitchViewAtCursor emits
[SwitchView(target), ChangeMode(Normal)], and this is a different path
from the one gap 1's siblings drive: that test toggles records mode explicitly,
where a view switch crosses records-ness only because it is auto-detected from
the axes (gotcha 1). Nothing in the batch says "records".
Driven in both directions, with the crossing asserted before and after so a
fixture that stopped crossing fails rather than passes. Injected fault: move
settle_normal_mode() back above rebuild_layout() at the end of
apply_effects — the exact regression PR #91's round 2 fixed. This test fails
by name; so does gap 1's, along with six pre-existing records-mode tests
(measured in review; an earlier draft of this entry said five).
Scope note, stated because an overclaim is worse than a gap: this drives
the effects SwitchViewAtCursor emits, not the view-panel key path that
composes them. Reaching the panel needs a view_panel_cursor on a CmdContext
built in command/cmd/panel.rs, which is another wave's ground this round. The
command's own coverage lives there; what is unexercised is the two joined.
the_layout_geometry_cases_are_every_wire_effect_of_that_kind
improvise-26dm. layout_geometry_effects_actually_change_document_config was
guarded by assert_eq!(cases.len(), 13), which catches a case being deleted
from the list and never one being absent from it — and the two are written
identically as an assert_eq over a number. The adversarial review of PR #78
added a fourteenth pub struct declaring LayoutGeometry, pointed at by
nothing, and the whole suite stayed green.
improvise-5ubs's mechanism cannot reach it: an effect is a struct implementing
a trait, so serde has no derived enumeration to diff against. model_effects
maps each ModelCommand variant back to the effect it came from (gotcha 38),
so the list is diffed against the LayoutGeometry effects reachable through
ModelCommand. The two sets are equal today, which is the evidence the
substitution lost nothing.
The right-hand side is bounded by the derive, and that took a second round.
Review of PR #96 injected the deletion of ModelCommand::TogglePruneEmpty from
every_model_command(): this was the only test in the workspace that
failed, and every_model_command's doc comment claiming the list was
compiler-forced was wrong — variant_name's exhaustive match forces an arm
there, not an entry in the list. ChatGPT Codex raised the same thing on the
PR. the_enumeration_names_every_variant_the_derive_declares closes it, so the
chain is derive → enumeration → model_effects → case list with every link
checked. The assertion here still names both directions, because a short
right-hand side reads as a spurious case on the left and invites deleting it —
degrading both sides together.
Injected fault: drop TogglePruneEmpty from layout_geometry_cases(). The old
count said 12 != 13; this says
right: [… "TogglePruneEmpty" …] and names the effect.
Residual, stated rather than closed: this is total over the wire and not
over the effects. An effect declaring LayoutGeometry that no ModelCommand
names is still invisible — which is precisely PR #78's probe. Closing that
needs an enumeration of effects the compiler maintains, and none exists without
a proc macro or inventory-style registration.
the_enumeration_names_every_variant_the_derive_declares
The bound the entry above rests on. Injection is Codex's own, run end to end: a
ModelCommand::ProbeGeometry whose effect declares LayoutGeometry, with the
model_effects and variant_name arms the compiler forces, omitted from
both lists. Before this test, the whole workspace stayed green. After it,
the failure is staged and each stage names the probe — first
every_model_command() never constructs ["ProbeGeometry"], then, once the
enumeration is repaired and only the case list is short,
the_layout_geometry_cases_are_every_wire_effect_of_that_kind with
ProbeGeometry on the right. That staging is the point: the two tests answer
different questions and the injection separates them.
The name reader is shared, not copied: reduce.rs's effect_type_name and its
own the_effect_name_reader_reads_all_three_debug_shapes are what this leans
on, because a second reader here would be a derivation done twice and could
drift while one copy was repaired. It reads the leading identifier off the
derived Debug, Effect being a trait object that type_name cannot reach.
command/cmd/panel.rs — the TUI half of the category-panel defects
Both of these have a twin in improvise-protocol/src/resolve.rs whose leaf
name is identical, so both headings are module-qualified (gotcha 48). Neither
defect was browser-specific: the same Cmd backs the same key in the TUI, and
confirming that before fixing is what kept the fix out of the client.
command::cmd::panel::tests::deleting_a_categorys_last_item_leaves_the_cursor_inside_that_category
improvise-uej8 in the TUI. The full argument, including why the rival
"always step back" repair is worse than the bug, is under
resolve::tests::deleting_a_categorys_last_item_leaves_the_cursor_inside_that_category
in clients.md; it is not repeated here, because two copies of an argument
are two things to keep true.
What this test adds that its twin cannot: the TUI tree carries the three
virtual categories ahead of the regular ones (improvise-vjms, gotcha 7),
so its indices are the ones a real user's cursor holds. Row 5 is Type's last
item and row 6 is Month's header; the test asserts both before acting, so it
cannot pass by landing somewhere the escalation was never possible. Inject the
fault by deleting the effects.extend(retreat_from_deleted_item(...)) line.
command::cmd::panel::tests::deleting_an_item_with_a_sibling_below_it_leaves_the_cursor_alone
The discriminating input for the rival fix, in the TUI. Fails under an unconditional step-back, passes under the conditional one.
command::cmd::panel::tests::deleting_a_categorys_only_item_does_not_arm_the_next_d_with_remove_category
The singleton arm in the TUI. The enumeration and the landing argument are
under the resolve.rs twin of this name in clients.md; they are not repeated
here, because two copies of an argument are two things to keep true.
What this fixture adds: Month is seated before Type and carries two
items, so the safe landing is found by the backward search — the preferred
direction — and lands on a real item rather than falling off the end of the
tree. Its twin exercises the same case with nothing below. Both assert the
consequence (a second d composes no RemoveCategory) rather than the landing
index, and both fail under the pre-review rule while the two adjacent-case
tests pass.
command::cmd::panel::tests::cycling_an_axis_from_the_category_panel_says_so_as_the_tile_bar_does
improvise-25bf in the TUI. The category panel and the tile bar are two routes
to one operation and only the tile route told the user, which is
design-principles §5 in miniature — a derivation done twice will diverge, and
the divergence is invisible until someone puts them side by side. The wording
now has one home, viewmodel::axis_change_status, called by both routes here
and by both in the browser.
The fixture sets Type to Row explicitly rather than reading the default,
so the expected Type → Col is a literal and not a restatement of the code
under test. It asserts cat_at_cursor() is Type first, because cursor 3 is
only the first regular category while the three virtual ones lead the tree —
an off-by-one would otherwise pin the wrong category's sentence and still pass.
The view stack is the client's (improvise-gd28)
Two user rulings, 2026-09-21: "The state drill manipulates (the view stack) should be local to the client" and "Drilling should be an ephemeral view, not something that gets persisted." The first one is what these three tests serve.
a_client_folding_a_switch_records_the_history_the_host_does
The bug, demonstrated before the fix. effect::SwitchView pushes the view
it is leaving onto ViewState::view_back_stack and clears the forward stack;
reduce_view's Command::SwitchView arm set active_view and nothing
else. Both write the same portable field of the same type, and the fold is
supposed to be the mirror's copy of the effect (improvise-cqq/m91) — so a
connected client's own history was empty forever, however many views it visited.
That is the whole reason view-back reads as unavailable in a browser: not
that stepping back is hard, but that the client has no stack to pop. The
declined() comment attributes it to the drill commit instead, which is a
second, separable reason (see the drill note below).
Written against the unfixed tree it fails with
left: [] right: [ViewFrame { view_name: "Default", mode: Normal }]. It drives
the host through apply_command and the mirror through reduce_view, so it
compares the two writers rather than one writer against itself.
a_client_stepping_back_lands_where_the_host_lands
The lockstep for the two new commands. effect::ViewBack restores
frame.mode as well as the view name, so a fixture left in Normal cannot
tell "restores the mode" from "never touched the mode" — the mode the default
supplies is exactly the mode a broken restore leaves behind. The fixture
therefore enters Help, switches away, drops to Normal, and asserts Help
comes back. Deleting the view.mode = frame.mode line fails it; deleting it
from a Normal-only fixture would not.
stepping_forward_undoes_stepping_back and
a_step_off_the_end_of_the_history_moves_nothing bound the two ends: the
direction is a Rewind enum rather than a bool so the two arms cannot read
each other's stack, and an empty stack is a no-op rather than a panic.
a_step_back_cannot_land_on_a_view_the_document_has_lost
The case the client-local stack creates, stated where a user feels it: a frame on my stack can name a view somebody else deleted while I was away.
This test replaced a guard. The first version answered it inside
Sessions::apply_step, which folded the step and then corrected a dead landing
the way apply_switch corrects a refused switch. Two injections did fail it —
but the correction folds a SwitchView, which pushes the dead view straight
back onto the stack, so pressing < again popped it, was corrected again,
and pushed it again: a back button that never progresses. Pruning the history
when the view vanishes (see the entry above) makes the dead frame
unrepresentable instead, which left apply_step's guard with no reachable
input. It was deleted rather than kept as defence in depth, because a guard
whose fault cannot be injected is indistinguishable from one that always
passes, and apply_checked's existing fold arm is then the whole behaviour.
The fixture visits Third and then Second, so the stack is
[Default, Third, Second] and only the top frame is doomed. Landing on
Third is what discriminates: a fixture whose stack held only the dead frame
would land on Default and could not tell "skipped it" from "gave up and went
home". Injected fault: drop forget_vanished_views from apply_model — the
step lands on Second, a view the document has not got.
deleting_a_view_forgets_every_frame_that_names_it
A defect the client-local stack creates, and a pre-existing one on the host.
A ViewFrame is somewhere you can go back to, so a frame naming a view that
has been deleted is an offer the document cannot honour. effect::DeleteView
moved active_view off the doomed view and left both stacks alone, which was
invisible while no client had a stack at all — the TUI user could already reach
a dead frame with <, and making reduce_view record history would have given
every browser client the same.
Worse than merely dead: without the pruning, Sessions::apply_step corrects
the landing by folding SwitchView(default), which pushes the dead view back
onto the stack. Pressing < again pops it, is corrected again, and pushes it
again — a back button that never progresses, and one that looks from the
outside like a key that does nothing.
Injected fault: delete the forget_view call from effect::DeleteView. The
test names the surviving frame. Its fixture asserts the doomed view is on the
stack before the delete, so it cannot pass by pruning nothing from nothing.
a_deleted_view_leaves_the_history_of_a_client_that_did_not_delete_it
The multi-client half, and the one adopt_deleted_views cannot cover: that
helper looks at mirrors sitting on the deleted view, and a client that
merely visited it is sitting somewhere else entirely. The bystander here is
therefore on Third, holding Second on its back stack — which is also what
makes ToAll the right audience and checkable: every client may have been
there, and the asker is not special.
Which views vanished is observed by diffing the name set across
apply_command, for the reason adopt_cursor_epoch watches a counter and
apply_model watches document_generation (gotchas 24, 39): which effects
drop a view is a per-effect decision, so a table of command variants would be a
second description of it, free to disagree.
Injected fault: drop the forget_vanished_views call from apply_model. The
bystander keeps its dead frame and the test names it.
a_refused_switch_puts_no_phantom_on_the_history
Found by ChatGPT Codex on PR #115, and it is that PR's own regression. Once
reduce_view records history, the correction for a refused switch records
history too. Starting on Default, the asker folds Missing (back gains
Default), then folds the correction back to Default (back gains
Missing) — while the mirror, which never folded either, stays empty. So
< afterwards lands on a view the document never had, and forget_view cannot
help: nothing vanished, the name was never real.
It is reachable, which is why it was fixed rather than argued away. No
shipped client can invent a view name — resolve.rs composes SwitchView from
a ViewSummary in the projected list — but a client can pick a name that
another client deletes before the frame arrives. That race is exactly the case
forget_vanished_views narrows and does not close.
Two things the fix needed, and the test separates them:
- The mirror folds what the client folded, refused or not. A correction can only undo what both ends hold; guarding the mirror is what let the two drift.
- The correction is
[ViewBack, ForgetView(asked)]— the exact inverse of the push, then the refused name off the forward stack it lands on.
Injections, each failing for its own reason:
- Drop
ForgetView: the phantom assertion fires —Missingis on the history. - The rival fix,
SwitchView(leaving)instead ofViewBack(which is what the review's own wording suggests, and what the code did before): no phantom, butleft: [Default, Second]against[Default]— the redundant frame the correction pushed. That injection is the reason the test asserts the back stack is restored rather than merely free of the phantom.
One residue, asserted rather than hidden: the optimistic fold clears the forward stack and no correction can refill it, so a refused switch costs the redo history — exactly as an accepted one does. The test pins it empty, and the fixture loads both stacks first so a lost forward stack is visible at all.
The neighbouring test a_refused_switch_corrects_the_asker_and_nobody_else
compared active_view alone and stayed green through all of this: the check
that answers a neighbouring question, in the place the doctrine warns about.
an_aggregate_a_session_was_shown_does_not_resolve_against_the_hosts_view
The blocker for browser drill, demonstrated rather than argued. The ticket
records drill as blocked on ADR 001 layer 2, on the grounds that
ApplyAndClearDrill is session-local and unaddressable. That reasoning is
about the commit path. Entering a drill needs no commit, and after the
view-stack work above a browser already has <; what is missing is only the
step that builds the _Drill view.
Everything that step needs is on the wire already: CreateView, SetAxis and
SetPageSelection are ModelCommands that name their view (improvise-lltt),
and SwitchView is view-half and per-client. What a browser cannot supply is
the operand — it holds CellId::Aggregate(AggregateHash), an opaque u128, so
it cannot compose the filter coordinates itself. That is ADR 002 working as
designed, and it points at the host resolving the surrogate.
App::resolve_aggregate already does that and is tested — but only where the
host stands. A RecordHash falls back to DataStore's maintained index and
so resolves against the document, which is why a connected client's SetCell
works from any view. An AggregateHash has no such index and cannot have one:
an aggregate has no existence outside the layout that derives it. So it is
layout-scoped, and the layout is app.layout — built from the host's active
view, which since improvise-jytm is not the client's.
This test seats a session on a view the host is not on, takes an
AggregateHash out of the grid grid_in derives for that session, and shows
the host answering None to a surrogate it had itself just projected. The
control is what makes it discriminating: the same surrogate resolves once
the host is moved onto that view, so the failure is the host's view and not the
surrogate. Without the control a blanket None — a typo in the hash, an empty
layout — would pass.
The fixture's second view is not a transpose of the first: it puts Product on
Row and Region on None, so its aggregates constrain on a coordinate the
host's layout never names. A fixture that merely reordered the same axes could
resolve by coincidence.
The first version of this test had the defect it was written to expose, and
review found it. It derived the grid from a hard-coded grid_in(&app, "Other", …), so the session, the switch and apply_checked could all be deleted and
every assertion still passed — it proved a grid built for a name, where its
own name claims a grid a session was shown. The view is now read off
sessions.get(id).view().active_view, asserted to have moved, and asserted to
differ from the host's; the viewport is the session's own. Injected fault:
delete the apply_checked line and the test fails with left: "Default",
which it could not do before.
What this does and does not say about the ticket. It is not ADR 001 layer
2: the mutation a drill performs is addressed to no one session — it configures
the shared _Drill view exactly as CreateView/SetAxis already do
unaddressed — and only the resolution of its operand is session-scoped. Nor is
it improvise-fdk1: a browser cannot edit inside a drill grid at all, because
edit_or_drill declines records-mode editing out loud before reaching the
cell, so no coordinate rename is reachable and no divergent staging scheme is
created. The single missing piece is that the resolution must run against the
view the aggregate was seen in. The cheap shape is the one every other
view-scoped variant already uses: let the command name its view, as
SetAxis { view, .. } does, and resolve against a layout derived for that name
rather than against app.layout.
command/keymap.rs — a bound key that builds nothing (improvise-55l5)
a_bound_key_whose_command_cannot_be_built_says_so
dispatch resolved a bound name with registry.interactive(name, args, ctx).ok()?,
so a command that would not construct collapsed into the same None an
unbound key returns — and App::dispatch_key answers None with
Dirty::NOTHING and no status. The two are different facts and the option
could carry only one of them.
Which errors were invisible, and what each would have caught:
no cell at cursor— six registrations readctx.cell_key():clear-cell,yank-cell,paste-cell,drill-into-cell,commit-cell-editandcommit-and-advance-right. This is the class that reaches a user: it is state-dependent, so no compile-time or registration check can see it.xon a grid whose row axis carries an empty category did nothing and said nothing.Unknown command,require_args,Unknown mode,commit-buffer item requires item-add mode— keymap-authoring errors.every_bound_command_name_is_registeredalready refuses these at test time (gotcha 15 checks both halves for exactly this reason), so they are unreachable from a shipped keymap. They surface now because the mechanism is one rule rather than a list, not because they were the complaint.
The repro is the first class. app_with_no_cells puts an item-less category on
Axis::Row, and the setup asserts layout.cell_key(0, 0) == None — so the
fixture states the precondition rather than hoping for it. The test then asserts
x is bound before pressing it, because a test that pressed an unbound key
would pass against the bug.
Injected rivals, one per test:
Some(built.unwrap_or_default())— returnSomebut swallow the error, the smallest thing that looks like a fix.a_bound_key_…_says_sofails;an_unbound_key_stays_silentpasses, as it should.- Report a failed lookup as well (
NonefromKeymaps::lookupbecomes a status).an_unbound_key_stays_silentfails; the other passes. That is the neighbouring question this pair separates: did a binding match is not did the command build, and a fix that answers both with a message is noise on every typo and every unmapped key.
A Sequence whose later step fails now reports and runs none of it, which
is the existing all-or-nothing shape (try_fold), not a new rule — gotcha 21's
abort is about effects that ran, this is about effects that were never built.
an_unbound_key_stays_silent
The control. None still means, and only means, no binding matched. It presses
a key the Normal keymap does not bind and asserts the status stays empty, and it
asserts that key is unbound first — without that assertion the test would pass
against a keymap in which it was bound to something silent.
command/cmd/grid.rs — o edits the row it added (improvise-o9uq)
o_in_records_mode_edits_the_row_it_added_and_leaves_the_others_alone
o is [add-record-row, enter-edit-at-cursor records-editing], and
AddRecordRow::execute returned only SetCell + a status: it never placed the
cursor. Appending a record changes view_frame's hash, so the frame observation
inside apply_effects (gotcha 24) invalidated the cursor to the origin
mid-batch, and EnterEditAtCursor then opened an editor pre-filled with row
0's Region coordinate. Typing appended to it and Enter went through
commit_plain_records_edit → rename_record_coordinate, moving every measure
cell of an unrelated record (gotcha 1: a category-column edit moves the whole
row, which is why the basis unit is the record — gotcha 40).
Measured before the fix, from Region{East,West} with Revenue 10 and 20, at
sel=(1,0), keys o N o r t h Enter: East's Revenue cell was gone and
the rows read ["0","1","2"] — the East record had become EastNorth.
The fix appends viewport_effects(target_row, 0, ..) after the SetCell,
which is the whole content of the repair: the observation is inside the loop, so
a placement emitted after the reframe survives and one emitted before does not.
Injected rival: move the viewport_effects ahead of the SetCell. Both
tests fail — the reframe then overwrites the placement, which is the exact bug.
That rival is the one worth keeping, because the naive reading of the acceptance
criteria ("append viewport_effects") does not say where, and the wrong order
is indistinguishable from the right one by inspection.
target_row is row_count() — the appended row is last — unless the record
already exists, in which case it is that record's own index, found by scanning
record_row(r).coords. Predicting row_count() unconditionally would put the
cursor out of range in that arm.
o_in_records_mode_opens_the_editor_on_an_empty_coordinate
The same defect one step earlier, and the cheaper witness: after o alone, the
edit buffer must be empty. Pre-filled with another row's coordinate is the
state from which any subsequent keystroke corrupts. It is a separate test
because it fails without needing the commit path to run, so a regression in
placement is diagnosed without reading through rename_record_coordinate.
Two things this fix does not close, deliberately:
otwice with no page-axis category still adds one row. Both presses buildCellKey::new(vec![])— the same record — and the second now says so rather than claiming "Added new record row". Refusing outright was rejected:CommitAndAdvance's Tab-at-bottom-right path calls the same command and relies on seeding an empty-coordinate row.- 1b proper.
view_framehashesentries.len()and every entry, so any append reframes and invalidates the cursor although no existing row was renumbered. Placing the cursor masks it at this call site; distinguishing "appended" from "renumbered" is a separate question and stays filed.
The format a projected cell is drawn with (improvise-e0ex)
a_formulas_format_reaches_its_own_column_and_no_other
context/code/core.md recorded, as the more useful half of improvise-711n's
entry, that ui/project.rs::grid_of "has no test that renders a cell and
asserts its digits". This is that test, and it is why the gap was worth naming:
the per-formula format is resolved in improvise-core and could have been
resolved correctly there while never reaching the grid.
It asserts four things that fail independently: the formula column's cell, the
raw column's cell beside it, and both totals. The totals matter separately
because they are formatted on their own path — GridBody::col_formats, read off
the top row — so a correct cell says nothing about the strip under it.
Two faults injected:
FormatPlan::atreturning the view's format for every key fails the cell assertion (left: Some("0.50") right: Some("0.5000")).- Dropping only the
col_formatswrite inviewport::grid_body, leaving the cells correct, fails the totals assertion (left: "1.50" right: "1.5000").
The second is the one that earns the test its length: without it, the totals strip could round every formula column to the view's format and no assertion in the workspace would move.
a_column_total_does_not_inherit_a_row_local_format
Found by review (ChatGPT Codex, PR #125). The first version of grid_body read
each column's total format off row 0, and context/code/core.md argued that
was safe because "where _Measure is on the row axis a column is not a measure
at all". That argument is wrong: a formula's target category is any category,
so a formula can own a row-axis item, and then row 0's format is a row-local
override applied to a total that has summed that row away — and re-ordering the
rows changes the total's spelling.
The fixture builds the same grid twice with 12.5 FORMAT .4 owning the first
row in one and the last row in the other, and requires the two totals strips
to agree. Every raw cell in raw_workbook holds the same value per measure, so
the two grids' totals are numerically identical and the only thing that can
differ is the format — the assertion has nowhere to hide.
It asserts the owned cell first (12.5000), so a plan that applies no format at
all fails on the fixture rather than passing the comparison vacuously.
Fault injected: plan.at(layout.cell_key(0, ci).as_ref()) in place of
layout.column_key(ci) — the rejected version, restored. Fails with
left: ["14.5000", "16.5000"] right: ["14.50", "16.50"], which is the bug in
one line: row order decided the spelling.
command::cmd::panel::tests::create_and_switch_view_skips_a_name_a_surviving_view_holds
improvise-vw47, the Cmd half. Its sibling
create_and_switch_view_names_incrementally already asserted that the name
follows the workbook rather than being a keymap literal — but every fixture it
uses has the derived name free, so it could not tell len + 1 from "the
first unused name" and the bug lived underneath it.
The fixture is the ticket's own scenario: create View 2, create View 3 with
a ,.4 format, delete View 2. It asserts the count is 2 before asking
anything, because the whole discriminating property is that the count has
fallen below the highest name — a fixture where len + 1 happens to be free
proves nothing, and that is precisely how the sibling test passed for months.
Fails against the unfixed source with
[CreateView("View 3"), SwitchView("View 3"), ChangeMode(Normal)] — the name
of a view that already exists. The consequence is one level further on, in
effect::CreateView → Workbook::create_view, which replaces: the user's
,.4 becomes ,.2 and no view is added. Asserting on the emitted effect
rather than on the applied workbook keeps the test in the Cmd layer where a
command is a pure function of its context; the applied consequence is asserted
in lockstep.rs, where it is also held against the browser's answer.
command/cmd/registry.rs — a refusal names the word that was typed
improvise-z05h. Reasoning: context/code/cli.md §5 and §6.
a_refusal_names_the_command_as_typed_through_an_alias
CmdRegistry::resolve rewrites formula to add-formula before the arity
check runs, so the refusal for formula Product Total = Widget + Gadget named
a command the user had not typed. Both directions are asserted — that the
message opens with formula: and that add-formula does not appear in it,
because prefixing the typed name onto a message that still spells the canonical
one reads as fixed and is not.
The alias is what makes this discriminating; its sibling below covers the case where the two names coincide, so neither can pass by accident.
a_refusal_from_a_canonical_name_is_named_as_typed
The control. add-item is not aliased, so a fix that only rewrote aliased
names would pass the test above and fail here. Together they say the name comes
from the registry in every case, which is what removing 41 name literals from
require_args call sites bought.
a_formula_refusal_states_the_shape_and_asks_for_quoting
The third of z05h's stacked bugs, and the one a user actually needs.
"requires 1-2 argument(s), got 6" is true and unactionable: nothing in it
suggests that the formula must be quoted. Asserts the message names the shape
and contains the word quote, so a refusal that merely restates the arity with
a friendlier name still fails.
every_ex_command_the_help_pages_show_is_one_the_parser_accepts
The surface z05h identified as unguarded. help.rs derives HelpLine::Keys
rows from the keymap and drops any whose command does not resolve, so key rows
cannot go stale — but Literal and Accent rows are prose, and all three
documented spellings of :formula were ones the parser refuses.
This closes the surface rather than the instance: every help line whose trimmed
text opens with : is split on its colons — one row shows two :add-cat
examples side by side — and each piece is handed to parse_line. Run against
bf83294 it fails naming the Welcome page and the exact line.
The checked >= 8 floor is a check-the-checker: a future refactor that stops
pages() emitting Accent/Literal rows, or that indents them differently,
would otherwise leave the test scanning nothing and passing.
What it cannot see, stated because the gap is one edit away from mattering:
draw.rs::draw_welcome builds its lines as a local array inside a ratatui
render function, so no walk reaches them. Its :formula line was corrected by
hand in the same change.
tab_naming_a_measure_lands_on_the_column_it_just_created
BUG improvise-0ygd. Typing a name into the trailing + column and pressing
Tab created the measure and then threw the user across the grid: it added a
record row nobody asked for and parked the cursor at (row + 1, 0), so the
column the commit had just made was never reached.
The cause is a stale index, not an abort and not an ordering. AdvanceDir::Right
computes at_bottom_right from CursorState::col_count, which is the pre-commit
column count — apply_effects rebuilds the layout only after the whole batch
(gotcha 23). Naming a measure inserts a column at the cursor's own index and
pushes +measure one to the right, so after the commit the cursor is no longer at
the right edge and the bulk-entry branch is answering a question about a grid that
no longer exists.
context/code/command.md stated the opposite in prose — "The measure + shares
none of this, and not by luck — it is the last column, so there is no column to its
right for an index to shift into" — which is false: the column that shifts is
+measure itself. That sentence is corrected in the same change.
Two faults were injected against the fix:
- Revert the new-measure arm of
inserts_a_column(back toNewCategoryonly). This test fails on the row count and the cursor; the bulk-entry sibling below still passes, which is the point — a fixture on an ordinary measure column cannot see a+measurebug, because+measureis always the last column and so is the only cell from which the bottom-right branch is reachable at all. - Drop the
created_namehalf, making the predicate value-blind. This test passes; the sibling fails. That is the rival worth naming: it fixes the case the ticket describes while eating the contract the branch exists for.
tab_on_an_empty_new_measure_column_still_starts_a_record_row
The companion, and the one that stops the fix over-firing. +measure is always
the rightmost column, so cursor.col >= col_max in records mode means the cursor
is on +measure and nothing else — the "Tab at the bottom-right starts the next
record" contract is reachable from that one cell. Bulk entry uses it by tabbing off
the end of a row onto +measure with an empty buffer and pressing Tab again.
So the discriminating input is the buffer, not the column: an empty commit
creates nothing, inserts no column, and must still start the next row. Injecting
the value-blind predicate above turns the branch off entirely and this test fails
with row_count 1 where 2 is owed, while the headline test above stays green.
improvise-lm5e — a type name is not a field
Sixteen command tests asserted effects_debug(..).contains("<EffectName>") and
nothing about the effect's fields, so they answered which effect was emitted
where they meant which category, item, view, path, cell or axis it named
(doctrine(check_the_checker)). The ticket's own reproduction step was run
rather than argued: one patch made fourteen command sites emit the right effect
type carrying a wrong constant field — add-category naming "Wrong",
set-cell writing 0.0 at Type/Clothing, set-axis ignoring its argument,
:w <path> saving to /tmp/wrong.improv, paste writing a constant instead of
the yanked value, the tile bar acting on Month whatever the cursor says, page
cycling always landing on the first item, the panel cursor landing on 0 of the
wrong panel, and the formula/view panels deleting and switching to "Other".
Against that command layer, cargo test -p improvise-command --lib reported
525 passed, 6 failed. None of the six were in this list except
commit_item_add_files_the_item_under_the_mode_s_category, which already
checked one field by substring; the other three were in ui/app.rs and two in
ui/reduce.rs, so the layer that would have caught a wrong field was never the
command layer. With the strengthened assertions and the same rival: 510
passed, 21 failed — all sixteen fire, and the five incidental ones remain.
Each strengthened test asserts post-fold App state where the effect is cheap
and safe to apply, and the effect's own fields by Debug where it is not
(SaveAs, RemoveFormula, SetPanelCursor, SwitchView, DeleteView).
Assertions naming a fieldless effect — Quit, Save, RebuildLayout,
TransposeAxes, AbortChain, TogglePruneEmpty — were left alone: for those
the type name is the whole claim, so they are complete rather than smoke.
add_category_cmd_produces_add_category_effect
Folds the effect and asserts the full regular_category_names(), which pins
both that Region arrived and that nothing else moved. See above for the rival.
set_cell_cmd_parses_coords_correctly
The test's own name claimed the coordinates were checked; it asserted only
SetCell. It now folds and reads the cell back at Type/Food, Month/Jan.
set_axis_cmd_recognizes_column_alias
Asserts axis_of("Type") == Column after the fold, and that Month is on
Column with it — so an implementation that moved the wrong category, or that
ignored the col alias and left Type on Row, is separated from one that did
neither.
write_cmd_with_path_saves_as
SaveAs is not folded — it would write a file — so the path is asserted on the
effect's own Debug.
paste_with_yanked_value_produces_set_cell
The fixture seeds Type/Clothing, Month/Feb with 42.0 and yanks 99.0, so
asserting the post-fold value at the command's key separates "wrote the yanked
value" from "wrote whatever was already there".
tile_axis_cycle_produces_cycle_effect
Reading the effects showed what no assertion had: TileAxisOp indexes
tileable_cats(&ctx.model.category_names()), all categories including the
virtual ones, so tile cursor 0 is _Index and not the first regular category.
The test now says so, asserts _Index moves to before.next(), and asserts
Type and Month do not move.
tile_axis_set_produces_set_axis_effect
The same, for the explicit-axis arm: _Index lands on Page, the two regular
categories stay where they were.
page_next_cycles_through_page_items
The name claimed cycling; the assertion was SetPageSelection. Region carries
North, South, East, so the post-fold selection must be South.
page_prev_cycles_backward
Its mirror, and the one that pins the wrap: from the first item [ must land on
East, the last.
move_panel_cursor_down_from_zero
SetPanelCursor carries a panel and a cursor and neither was checked. Both
are now named, so moving the right cursor on the wrong panel fails.
move_panel_cursor_moves_from_the_named_panels_own_cursor
improvise-yf7p. move-panel-cursor formula 2 through the registry's interactive
arm, with the formula cursor at 1 of five formulas and the category and view
cursors at 0, must land on 3. Before yf7p the cursor and bound were copied in
by a registry contextualize, and review showed one that clamped the parsed delta
to delta.signum() left the whole workspace green: every test moved by ±1, where
a clamp is invisible. They are read in execute now, and this test is what sees a
bug there. Rival self.delta.signum() in execute — injected, run, fails (lands
on 2). Reading another panel's cursor fails too, because the fixture seats them at
different values — the index-0 fixture could not separate them.
delete_formula_at_cursor_with_formulas
Asserts the RemoveFormula names the formula the cursor is on, rather than merely
that a formula was removed. PR #148 named the field, but its fixture was one
formula with the cursor at 0, where "the formula under the cursor" and "the
first formula" are the same formula (improvise-y1av). The rival it could not see —
DeleteFormulaAtCursor reading index 0 instead of ctx.formula_cursor — passed
the whole workspace, so d in the formula panel deleting the wrong formula had
no guard anywhere. Now two formulas, cursor at 1: it must remove Tax and must
not name Profit. Rival let cursor = 0usize.min(..) injected, full suite run:
this is the only test that fails.
switch_view_at_cursor_with_valid_cursor
Names the view under the cursor. Same fixture defect as above (improvise-y1av):
two_cat_model has one view, so the cursor at 0 named Default whether or not
the command read it. Now Default, Other with the cursor at 1, asserting
Other. Rival get(cursor).and(first()) — in bounds only, so the out-of-bounds
arm is unchanged and switch_view_at_cursor_out_of_bounds_returns_empty cannot be
what catches it — injected and run: fails here, in
delete_view_at_cursor_deletes_the_view_under_a_nonzero_cursor, and in the two
wasm-client/tests/lockstep.rs tests that were previously the only guard.
delete_view_at_cursor_deletes_the_view_under_a_nonzero_cursor
improvise-y1av. Default, Other, Third with the cursor at 1: deletes Other
and steps the panel cursor back to 0. The same rival as above fails it.
delete_view_at_cursor_zero_does_not_adjust_cursor
Names Default too. Its existing negative half — no SetPanelCursor at
position 0 — is unchanged. Position 0 is this test's subject, so it cannot
separate "the view under the cursor" from "the first view" and does not try to;
the test above does.
commit_category_add_with_name_produces_add_effect
Folds and asserts the whole category list, so the name in the minibuffer is the name that arrives.
commit_item_add_files_the_item_under_the_mode_s_category
This one already checked "Month" by substring, which is why it was among the
six the rival caught. It now folds and asserts Month's items are
Jan, Feb, March and that Type's are untouched — a substring "Month"
cannot tell the category field from the item field.
execute_command_valid_runs_command
:add-category Region must name Region; the assertion said only
AddCategory.
a_sequence_of_moves_folds_in_order_and_the_last_write_wins
Replaces law_sequence_associativity (improvise-gjat), which could not fail. That
test built three effect vectors from one CmdContext and asserted
effects_debug((a ++ b) ++ c) == effects_debug(a ++ (b ++ c)) — the associativity
of Vec::extend, true for any deterministic execute, and
doctrine(fold_tests_must_test_order_not_the_axiom) names exactly this shape. It
never applied an effect, so no command behaviour could move it.
The property that can fail is the one production actually relies on: a
Binding::Sequence flattens into one effect vec (gotcha 21), the batch folds
onto the App in order, and each Move composed against the same ctx emits an
absolute SetSelected — so the moves do not compose and the last write wins.
The test folds one batch per ordering and asserts the resulting cursor, with
c, b, a landing somewhere a, b, c does not, so an implementation that ignores
order is separated from one that honours it.
Two faults were injected, and both leave the old assertion passing:
Move::executereturnsVec::new()when the computed position equals the current one — a plausible "skip a move that moves nothing" optimisation, and one that would breaktests/lockstep.rs, since the browser resolver mirrorsMoveline for line including where it clamps (gotcha 47). The old test still passed (ccontributed nothing to either grouping, so the two remained equal); the new one failedleft: (0, 1) right: (0, 0)—bhad become the last write.App::apply_effectsfolds the batch in reverse. The old test passed untouched, because it formats the vector and never applies it; the new one failedleft: (1, 0) right: (0, 0).
The second fault is why the batch is flattened rather than applied one command at
a time. A first version of this test called apply_effects per command; each
single-Move batch reverses to the same cursor, so the reversed fold was
invisible and the test passed against it. One batch is also the shape a
Binding::Sequence really has.
command/cmd/grid.rs — the toggle-group "involution" law was a tautology, and the law is false (improvise-ub4o)
law_toggle_group_involution called cmd.execute(&ctx) twice against the same
CmdContext and asserted the two Debug strings were equal. A CmdContext is an
immutable borrow, so nothing the first call returned could reach the second: the
assertion is f(x) == f(x) for a deterministic f, which no rival of any kind
can fail. It was vacuous a second time over, because its fixture was
two_cat_model(), which has no groups at all — both sides were the empty string.
Applying the effects between the calls does not rescue the law; it refutes it.
Measured on a fixture with Q1 = {Jan, Feb} and Q2 = {Apr, May} on the row axis,
cursor at data row 0:
znamesQ1and collapses it.- A collapsed group contributes no
DataItem(expand_categoryskips it), so data row 0 is nowApr.znamesQ2and collapses that too. - The row axis is now empty, so
row_group_for(0)resolves nothing andzreturns no effects at all.
So the cursor-addressed toggle was not an involution, and a collapsed group could
not be reopened from the grid. improvise-6qet changed that, and the tests below
are its guards; the two that pinned the old behaviour
(a_second_toggle_at_the_same_cursor_names_the_next_group_not_the_collapsed_one,
a_fully_collapsed_row_axis_leaves_the_command_nothing_to_address) were replaced
rather than kept, since the behaviour they pinned was the bug. The old law stays
deleted; a real determinism law over two independently built contexts is
improvise-hzjo.
improvise-6eqi found the survivors of ub4o all seated the cursor at data row 0, so a
toggle that ignored the cursor (row_group_for(0)) passed the whole workspace. Every
toggle test below starts off row 0; that rival was injected and fails five of them.
toggling_at_the_cursor_names_the_group_the_cursor_sits_in_and_collapses_it
Asserts the effect names category: "Month", group: "Q2" — a category and a group
by name — and that the view's collapsed set actually moved, and that the neighbour
Q1 did not. The cursor sits on Apr (row 2): at row 0 the neighbour and the
subject are the same answer. Fixture asserted expanded first, so the post-condition
is not supplied by the default.
Faults injected: execute returning vec![] after resolving the lookup, and
row_group_for(0) in place of the cursor row (improvise-6eqi). Both fail it.
toggling_a_group_collapses_that_group_and_the_cursor_follows_it
improvise-6qet, driven through the real path (App::cmd_context → execute →
apply_effects). Cursor at row 2 (Apr); z gives rows [Jan, Feb, Q2] with the
cursor on Q2 at row 2, and z again gives [Jan, Feb, Apr, May] with the cursor
back on row 2 — the involution the old law claimed.
Faults injected: the swap (cursor placement emitted before ToggleGroup) fails
it left: (0, 0) right: (2, 0) — the toggle's observed reframe resets the cursor
after the placement. row_group_for(0) fails it too.
a_collapse_from_the_tail_of_a_run_lands_on_the_runs_first_row
Cursor on May (row 3). Collapsing Q2 lands on row 2, the fold. The rival "leave
the cursor where it was" is right for a cursor already on the run's first row and
wrong here (row 3 is past the end of [Jan, Feb, Q2]); injected, it fails this and
the next test only.
collapsing_an_inner_group_lands_where_every_earlier_run_shrank
Region × Month on Row, Month the inner level. Cursor on South/Apr (row 6).
Collapsing Q2 shrinks North's run as well, so South/Q2 is at row 5. The only
test that separates "ask the layout the toggle will produce" from "ask the current
layout": that rival returns 6 (left: (6, 0) right: (5, 0)).
a_collapsed_group_can_be_re_expanded_from_the_grid
Both groups collapsed ([Q1, Q2]), cursor on the Q2 row: z names Q2 and
reopens it, landing on Apr at row 1. Before 6qet a collapsed group had no row and
this was impossible from the grid. The swap fails it at (0, 0).
edit_or_drill_declines_a_collapsed_groups_row
A fold is neither editable nor drillable: no record carries Month=Q2. The same
context at Feb (row 1) still opens the editor, so the decline is about the row,
not the fixture. Rival: the folds_a_group guard removed — the command opens an
editor on the fold, and the test fails.
ui/project.rs — a collapsed group's row is an aggregate (improvise-6qet)
ui::project::cell_identity::a_collapsed_groups_row_is_projected_as_an_aggregate
Nothing regular is on Axis::None, so the none-axis arm of cell_id cannot supply
Aggregate — and North in the same grid is asserted Record to prove it. The
Coastal fold must be Aggregate, and its text 7.00 (East 3 + West 4). Rival:
folds_a_group dropped from cell_id — the fold is projected Record, which
render.rs::editable would offer for a write to the non-item Region=Coastal. It
also fails when group membership is read off Category::groups instead of each
item's group, because a document built with add_item_in_group registers none.
ui/project.rs — a formula lost with its target ITEM (improvise-en4s)
Dirty::of(ModelChange::CellData) reports formulas: true. That was written as
deliberate over-reporting in the cheap direction — a cell write cannot change a
formula, so the entry cost one redundant Projection and bought nothing.
improvise-g3no made Model::remove_item drop the formulas targeting the item,
and effect::RemoveItem is CellData, so a deletion really can take a formula
out of the document. Tightening that arm would leave every connected client's
formula panel showing a formula the document no longer has, forever, with
nothing in the stream to correct it — gotcha 35's expensive direction, and the
exact failure that put formulas on the LayoutGeometry arm in the first place.
removing_a_formulas_target_item_empties_the_clients_formula_panel
The item-level twin of removing_a_category_empties_the_clients_formula_panel,
which covers the LayoutGeometry arm only.
Fault injected: formulas: false on the CellData arm of Dirty::of. The test
fails on the final assertion — the client still names Total = Revenue after the
document has dropped it.
The setup had to be restructured so it could fail there. Written the obvious
way, mirroring its sibling — connect, then AddFormula, then RemoveItem — it
does fail under the rival, but at the setup assertion "the client must be
holding the formula before it can lose it", because AddFormula is itself
CellData, so with the arm tightened the formula never reaches the client at
all. That answers "did AddFormula's projection arrive", which is the
neighbouring question; the property is about a deletion's projection. So the
formula is added before Sessions::connect and the client picks it up from
the bootstrap seed, which no Dirty gates. Both assertions are then reachable
independently and the rival lands on the one that means something.
The sibling does not have this problem against its own rival: formulas: false
on the LayoutGeometry arm leaves AddFormula delivering normally, so its setup
holds and only its final assertion moves. It is left alone.
The test also asserts RemoveItem::kind() is Model(ModelChange::CellData)
before applying it, so reclassifying that effect fails here rather than silently
turning this into a test of the LayoutGeometry arm that its sibling already
covers.
ui::host::tests::a_rolled_back_load_tells_nobody_and_puts_every_mirror_back
improvise-k84e, scoped to what remained after PR #109: the Applied::Rebased
path proper. A frame whose LoadModel succeeds, emits a Snapshot to every
client, and is then rolled back by a later command in the same frame.
a_rolled_back_load_leaves_no_ghost_view_and_no_false_status already covers the
document half — active_view and status_msg go back. This covers the
Sessions half, which nothing touched: mutating_frame clones the whole
Sessions before the batch and undo_frame puts the clone back, so the reseed
rebase performed is undone with it and the computed Snapshots are discarded
rather than sent.
It reproduced nothing — the mechanism is sound, which is what the bead predicted ("should be sound", the phrasing this repo's gotcha list is made of). It is a regression guard, and its value is entirely in the fault it can see.
Fault injected: self.sessions = mirrors; replaced by let _ = mirrors; in
undo_frame. The test failed on the projection assertion, not its setup,
naming the asker's mirror and printing the whole correcting batch — a RowFrame
of Region/North/South and a Document of "Test", i.e. the restored
document being re-sent to a client whose mirror had been left holding the
discarded one. That is precisely the user-visible symptom: a client hearing
about a load that never happened. Reverted.
Three things make it non-vacuous:
- The control is the load-bearing half
(
the_load_this_fixture_performs_rebases_every_client). If the fixture'sLoadModelsilently failed,Applied::Rebasedwould never run and every assertion here would pass over nothing. The control asserts the generation moves and that both clients are owed aSnapshot. - The load is witnessed to have RUN before anything is concluded from the
rollback (improvise-gho5). The asker's whole reply must be
Rejected(ChainBroken(North)).Northis asserted to resolve before the frame, so onlybroken_chaincan give that answer, and only after theLoadModelhas replaced the document.CheckedBatchnever producesChainBroken. This bullet used to say thedocument_generationequality did the job, and that was false: generation equality is exactly what holds when the frame never got that far. - A bystander is seated where a wrong relay could move it. A single-client fixture cannot tell "the mirrors were restored" from "there was one mirror and it happened to be the asker's".
The projections are compared as an emptiness claim over Projections rather
than over Addressed, because project_all can address a client with an empty
ServerFrame::Commands; asserting on the Addressed list would pass for the
wrong reason.
ui::host::tests::the_load_this_fixture_performs_rebases_every_client
The control for the test above, and it is stated separately rather than folded in so that a change making the fixture's load stop rebasing fails here, where the message says what actually broke, rather than turning the rollback test green by emptying it.
ui::project::tests::a_grid_holding_nan_quiesces
improvise-4w09, found by the adversarial review of PR #178. That PR put
value: Option<CellValue> on CellDisplay, and a Number(NaN) compares
unequal to itself under CellValue's PartialEq. The fixture is parsed
from .improv text holding = nan (improvise-55v2), which shows a real
document reaches it. The NaN sits at (1, 1), off the origin. A precondition
asserts the projected cell really carries Number(NaN).
It asserts three things:
grid_updates(&g, &g)is empty.GridCache::folded(&g.seed()) == g, the compaction property of gotcha 32.- A second
project_all(Dirty::EVERYTHING)sends noProjection::Grid. The non-diffed widgets are resent underEVERYTHINGby design, so they are filtered out rather than asserted absent.
Fault injected: same_on_the_wire reduced to f64 ==. Each assertion was
run alone with the ones before it removed, and each fails: the diff returns
Cells([((1, 1), …NaN…)]), the seed does not compact to its cache, and
project_all resends that cell. With bit equality only (no NaN clause), this
test passes. See
render::tests::a_cell_equals_another_exactly_when_the_wire_cannot_tell_them_apart.
ui::host::tests::a_frame_writing_a_view_its_own_first_command_deleted_is_refused
improvise-bok4. This is the reviewer's first measured case:
[DeleteView("Scratch"), SetNumberFormat { view: "Scratch" }]. Before the fix
the asker's reply was [Commands([ForgetView("Scratch")]), Commands([Project…])]
and then Accepted. The second command had done nothing, because
view_mut_of found no view and every view-addressed effect swallowed the
None. Now on_view refuses, the abort becomes
Rejected(MutationRefused), and the assertion that Scratch still exists
proves the deletion was undone with the rest of the frame.
Its sibling, a_frame_adding_an_item_to_a_category_its_first_command_removed_is_refused,
is the reviewer's second case: [RemoveCategory("Region"), AddItem { category: "Region" }]. It was accepted as well, with AddItem's error written only to
the host's status_msg. Both tests failed first against the unfixed effects.
deleting_the_documents_last_view_is_refused_rather_than_accepted covers the
one-command case, which DeleteView's let _ swallowed. The fault injected
was to put the let _ back. The test then failed with a projection batch
where the refusal should have been, and it was reverted.
The control is a_frame_whose_commands_all_land_is_accepted, the same two
commands in an order where both land. Without it, a host that refused every
two-command frame would pass all three tests.
ui::reduce::vocabulary_is_complete::every_effect_is_on_the_wire_or_has_a_verdict
improvise-h3y7, the forward direction of the vocabulary derivation. It is the
direction ViewBack/ViewForward failed: they were missing, so no client ever
had a back history. The test scans this crate's production src/ for
impl Effect for X and simple_effect!(X. That covers both spellings, and the
h3y7 audit's brace-matcher missed nine effects by seeing only the first. It
then requires each name to be a Command or ModelCommand variant (read off
serde's derive) or to carry a NOT_ON_THE_WIRE verdict.
Faults injected, together: HelpPageNext's verdict replaced by one for
SetSelected. This test failed naming HelpPageNext, and
the_verdict_list_names_nothing_that_is_on_the_wire failed naming
SetSelected. Separately, HelpPageSet's verdict was pointed at
Command::HelpPag, and
the_verdict_list_names_only_real_effects_and_real_variants failed.
the_scan_reaches_every_way_an_effect_is_declared pins one effect of each
spelling plus SetTransientPrefix (which lives in command/keymap.rs). A
broken walk therefore fails that test instead of shrinking the set.
an_effect_named_only_in_a_comment_or_string_is_not_an_effect shares
improvise-c7mu's fault. With code_only made the identity, it failed with
["SaveAs", "ExportCsv", "Real", "SetYanked", "AlsoReal"].
improvise-94x2, found by the PR #172 review. The reader took the
identifier immediately after simple_effect!(. The macro's own matcher is
$(#[$meta:meta])* $name:ident(…), so a doc comment before the name (blanked
to whitespace by code_only), an attribute, or a line break after ( left
nothing to read, and the effect was invisible. The reviewer injected a
documented simple_effect!(\n /// Rival.\n SetBogusRival(bool) …) into
effect.rs, and all five tests passed. Re-injected here, it also passed all
five before the fix. After the fix
every_effect_is_on_the_wire_or_has_a_verdict fails naming SetBogusRival.
The rival was removed afterwards. name_after now skips whitespace and every
bracket-matched #[…] before reading the name.
an_attributed_or_wrapped_effect_is_still_an_effect pins that on a fixture:
a doc comment, two attributes (one with nested brackets), and a wrapped call.
With the whitespace skip removed it fails with [].
an_untouched_pivot_edit_stores_the_identical_value
an_untouched_records_edit_stores_the_identical_value
improvise-umi4. App::display_value_in seeded the edit buffer with
CellValue's Display, which writes a non-whole number {:.4}. So i then
Enter with nothing typed stored 1.2346 over 1.23456789, in pivot and in
records mode, and the screen looked the same before and after.
Both tests go through the real key path (App::handle_key, i, Enter) on a
2×2 grid. The cursor sits on (West, Revenue), which is row 1, column 1 in pivot
mode and row 1, column 3 ([Region, +cat, Cost, Revenue, +]) in records mode,
so "the cell under the cursor" is not "the first cell". The values are chosen
so that Display and the value disagree: 1.23456789, 0.1 + 0.2,
98765.4321987, 123456789012345.67 (under the 1e15 integer cutoff, so it
takes the rounding arm), -0.00001234 (rounds to -0.0000), -0.0,
-inf, plus a text value. The comparison is f64::to_bits, so -0.0 against
0.0 counts as a difference.
Both failed before the fix, on Number(1.23456789) stored as Number(1.2346).
Rival injected: CellValue::edit_text's number arm set back to
self.to_string(), i.e. the display seed restored in its one home. Both tests
failed again on the same case. The fix was then restored and the suite re-run.
A text cell that reads as a number (Text("007")) is deliberately not in
the list. Its seed is exact ("007"), but typed() classifies the committed
text as Number(7.0). Probed here: both tests fail on it in both modes. That
is the classifier, not the seed, and it is improvise-ixxx.
loading_a_document_forgets_the_outgoing_documents_view_history
improvise-lrrx: App::install_workbook cleared cursors but not
view_back_stack/view_forward_stack, so < after LoadModel walked the
outgoing document's views inside the incoming one. The history is two deep
(Default, X, then on Z), and the incoming document has a view named X
but opens on Y. Without that, a stale frame is a refused switch and looks the
same as "nothing happened" (the ticket's own fixture warning). Against the unfixed
code, < landed on X, a view this client never visited in the new document.
Rival injected: the two clear() lines removed from install_workbook, with the
improvise-8nd9 fix still in place. This test and its wizard twin failed at the
view-back walked the outgoing document's history assertion.
accepting_an_import_forgets_the_outgoing_documents_view_history
improvise-lrrx, through the second door into install_workbook. A
wizard-built workbook has only Default, so the test creates an X in it after
accepting. Without that, the stale X frame would be refused and could not be
told apart from a cleared history. Same rival and same failure as the load test.
a_step_back_the_document_refuses_leaves_no_frame_behind
improvise-8nd9: ViewBack pushed the current view onto the forward stack before
the guarded switch_to_view, so a frame naming a view the document lacks left a
phantom behind, and , would then "go forward" to where it already was. The
dead frame is pushed by hand, standing for any path that forgets
forget_view, with a live frame under it and the frame's mode set to Help so
that a restored mode is observable. Two rivals. First, the original code, which
pushes first and restores the mode unconditionally: it fails at nothing moved, so no mode. Second, a push-first version that restores the mode only on a move,
so the mode assertion cannot hide the stack assertion: it fails at a refused step left X on the forward stack. The last two assertions check that the live
frame beneath still works, so a fix that drops the whole stack cannot pass.
a_step_back_to_the_view_you_are_on_is_the_step_the_client_folds
improvise-8nd9. This test holds the fix to the client's reduce_view::stepped.
A live frame naming the view you are on (a forget_view collapse leaves one)
is still a step. It restores its mode and leaves a forward frame, exactly as
the client folds it. Rival injected: stepped_to observing whether
active_view changed rather than whether the document accepted the view.
That was the first version of this fix. This test failed at the
mode assertion, as did
command::cmd::text_buffer::tests::colon_view_back_respects_whether_history_exists.
That test's fixture is such a frame, and it is what caught the first version.
a_step_forward_the_document_refuses_leaves_no_frame_behind
The ViewForward half of the test above, with the stacks' roles swapped. It fails
against the unfixed code at nothing moved, so no mode.
yy_then_p_in_records_mode_copies_between_real_records
improvise-6p8q. At gh/main 9e78051 it failed at the yank: left: None, right: Some(Number(700.0)), because YankCell evaluated the synthetic
(_Index, _Dim) key. With the yank alone repaired (rival: paste still writes
SetCell(synthetic key)), it fails at the paste assertion, and the document
carried _Dim=|3|, _Index=|0| = … in ## Data. The test drives yy/p
through App::handle_key and then saves and reloads, because the persisted
line is the harm. The fixture is chosen so no two answers coincide: two records
by two measures with four distinct values, the yank from row 0 Cost and the
paste onto row 1 Revenue, so "the first row", "the first measure" and "the
value already there" are each wrong. The persistence assertion is scoped to the
## Data section: a view section legitimately writes _Index: none, and a
first version of this test failed on that.
yy_on_a_records_coordinate_column_yanks_the_item_as_text
A coordinate column yanks its item as text (West, row 1): the yank reads
GridLayout::records_value, the same value the browser yanks off
CellDisplay::value. Rival: restoring the synthetic-key yank fails it with
None.
p_on_a_records_column_that_stores_no_value_changes_nothing
p on Region, +cat and + at row 1 leaves the document byte-identical and
says why. At 9e78051 it failed on Region with _Dim=|0|, _Index=|1| = 5.5
appended to ## Data. The control — the same paste onto Revenue writes — is
what keeps the declines from being "this command never writes".
p_inside_a_drill_neither_writes_nor_stages
At 9e78051 a drilled paste wrote _Dim=|3|, _Index=|0| = 5.5. The rival that
resolves the real record but ignores the drill (writing straight through) fails
the document-unchanged assertion at the same line. Why declining rather than
staging: context/code/command.md, "Paste lands on a record or nowhere".
p_onto_a_none_axis_aggregate_writes_nothing
improvise-l4mp, None-axis half. At 9e78051 p over the West/Revenue aggregate
(Product on None) wrote Region=West, _Measure=Revenue = 5.5 — a partial key
that is not a record. The cursor is on row 1, not row 0. The control reruns the
same paste with Product paged instead of aggregated and requires the write,
so a paste that declines everywhere fails. Rival: removing the aggregate check
fails the first assertion.
paste_with_an_explicit_full_key_writes_even_where_the_view_aggregates
Pins that "aggregate" is asked of the key, not of the view. It passes at
9e78051 (there was no check at all); its job is the rival where the check
reads only whether the view has a regular category on None, which fails it —
that rival would silently turn every scripted paste with full coordinates
into a no-op on an aggregating view.
the_aggregate_decline_does_not_send_the_user_where_paste_also_declines
improvise-48jd, from the #184 review. The aggregate decline said "drill in to
paste into a record", and inside the drill p answers PASTE_INSIDE_DRILL, so
the advice led to a second refusal. The test follows the advice literally: p
on the West aggregate, > into its drill, cursor onto the Revenue column, p
again. It asserts that the drilled paste really is the decline, so the setup
cannot pass by landing on "no cell at cursor". A first version of the test did
exactly that, because West had no record to drill into. It then asserts that
the first message does not mention drilling. At 0ed2e41 it failed with the
old text. The rival is restoring that text. Real drill paste, which means
staging a typed CellValue, is still open as improvise-48jd.
The drilled paste now declines at the cursor door (value_cell_key) before
PasteCell runs, so the status carries the registry's paste: prefix and a
wording shared with clear-cell. The assertion was re-aimed from equality with
PASTE_INSIDE_DRILL to ends_with(INSIDE_A_DRILL_REFUSAL) (improvise-gw18's
doors). The advice half is unchanged.
drilling_into_a_fold_is_refused_and_o_after_it_writes_no_group_coordinate
improvise-gw18. On gh/main at 800348a, > on the Q2 fold (row 2, column 1)
opened a _Drill with no rows, and o in it stored Month=Q2 in ## Data. The
test presses both keys through App::handle_key, then saves and reloads, and
asserts that no coordinate names a non-item (non_item_coordinates, which asks
the category, not the spelling). It failed on the first assertion: the drill
opened. Rival: the registry's cursor fallback on the reading accessor
(cell_key) in place of unfolded_cell_key. This test and
no_normal_mode_key_pressed_on_a_fold_stores_a_group_coordinate fail. The
control, drilling_into_the_row_above_a_fold_still_opens_a_drill, holds the
refusal to the fold alone.
no_normal_mode_key_pressed_on_a_fold_stores_a_group_coordinate
improvise-gw18's enumeration, made executable. Every key bound in Normal mode,
prefix sub-keys included, is pressed on a fresh app with the cursor on a fold
and a value yanked, and no stored coordinate may name a non-item. The sweep is
enumerated rather than hand-picked, which is how it found p (the fold half
of improvise-l4mp) without being told. The first run failed on
[(Char('p'), NONE)] on a fold stored [("Month", "Q2")]. It asserts that the
enumeration reaches p, so an empty sweep cannot pass. What it cannot see is a
write two keys deep (> then o). That is why the drill test above exists. A
two-key sweep was estimated, not measured, at several seconds and not built.
Widened after PR #197's review, which showed that H without its fold check
passed the sweep: a ghost hidden: entry is not a stored coordinate. Every
stroke must now also leave the ## Data section and the view's hidden set
exactly as they were. With the H guard disabled, this test fails as well as
the yk2s one. What no sweep can see is clear-cell back on the reading
accessor, because clearing a key nothing is stored at changes nothing. The
lx7w tests and the fold lockstep case catch that.
hiding_a_folds_row_is_refused_and_persists_no_hidden_group
improvise-yk2s. H on the fold emitted HideItem { Month, "Q2" }. View
accepts an unknown name, so a ghost hidden: line was saved, with no visible
change and no message. The test asserts no hidden entry, a message, and no
hidden: line in the saved text. The control hides Feb one row up. Rival: the
row_folds_a_group guard disabled (if false && …). Only this test fails. The
guard asks the row half of folds_a_group, so an inner-level fold
(North/Q1) is refused too, although North is a real item. That is a
deliberate over-refusal, taken to keep one predicate.
enter_advancing_onto_a_fold_commits_nothing_there
The write half of improvise-enz6, closed by the commit door. i on Feb, then
Enter, which commits and advances onto the Q2 fold and re-opens the editor
there. That entry is still enz6's open half. Then 7 and Enter. At the
previous commit door (cell_key) the second commit stored Month=Q2. Rival:
commit_at_cursor on cell_key again. This test and
tab_advancing_onto_a_formula_cell_commits_nothing_under_it fail.
x_in_records_mode_clears_the_real_record_under_the_cursor
improvise-lx7w. In records mode x built ClearCell from the synthetic
(_Index, _Dim) key, which names nothing stored, so it was a silent no-op. The
cursor is on West × Cost (row 1, column 3). The test asserts that West/Cost is
gone, that East/Cost is untouched (so a clear of the wrong row cannot pass), and
that nothing synthetic reaches the reloaded file. Rival: clear-cell's cursor
fallback on the old door. This test and the coordinate-column one fail.
x_on_a_records_coordinate_column_changes_nothing_and_says_so
improvise-lx7w's decline arm, the answer #184 gave paste: a coordinate column
stores no value, so x there changes nothing and says so. Before the fix it
changed nothing silently, and it fails on the message.
p_onto_a_formula_owned_cell_writes_nothing_and_names_the_formula
improvise-54g1. p on West × Profit (Profit = Revenue - Cost, row 1,
column 2) stored _Measure=Profit under the formula. The recompute hid it, but
it survived in ## Data. The test asserts that the saved text has no Profit
line, that the reload has no such cell, and that the status names the formula.
The control, one column left, must write, so a door that refused everything
cannot pass. Rival: the old door. It fails on the saved text.
tab_advancing_onto_a_formula_cell_commits_nothing_under_it
A door the tickets did not list, found by working the enumeration. i declines
to edit a formula's cell, but Tab from West/Cost commits and advances onto
Profit, EnterEditAtCursor re-opens the editor there, and Enter stored
7 under the formula. Found by this test failing at 90b93d4, before
committable_cell_key existed. The rival is the one above.
ui::host::tests::every_failing_model_command_refuses_its_whole_frame
improvise-9fpl, found by PR #181's review. Eight of bok4's refusal sites had no
test: reverting any one of them to a status-only write failed nothing. They
were AddItemInGroup, AddFormula's parse error, and all six
ImportJsonHeadless arms. Two improvise-40j4 cases were added to the same
table: DeleteView of an absent view, and AddFormula into an absent
category.
Each case sends a frame of two commands. The first, AddItem { Region, "Landed" }, lands. The second fails. The test asserts that the asker hears
exactly [Rejected(MutationRefused)] and that Landed is gone. The fixture
has a second view, Scratch, because with one view DeleteView("Ghost") is
refused as "the last view" and the absent-name case is never reached. The
first run found exactly that: the case passed before the fix.
Faults injected by a script (/tmp/wire2_inject2.py, not committed): each of
the 14 refusal sites was reverted alone to a status-only write, after the
merge with #185. Every one failed at least one test. The eight that were
unwitnessed fail this test, DeleteView's last-view arm fails
deleting_the_documents_last_view_is_refused_rather_than_accepted, on_view
fails a_frame_writing_a_view_its_own_first_command_deleted_is_refused, and
LoadModel fails both a_load_that_fails_is_not_a_rebase and
a_failed_load_or_import_does_not_run_the_quit_after_it. The file was
restored and compared.
ui::host::tests::a_frame_adding_a_formula_to_a_category_its_first_command_removed_is_refused
improvise-40j4. [RemoveCategory("Region"), AddFormula { "Total = 1", Region }]
was accepted, and the document held a formula for a category that did not
exist; format_md wrote it and parse_md reloaded it. AddFormula ran
let _ = add_item(…) and then add_formula anyway. The test failed first with
the frame accepted. AddFormula now refuses when add_item errs, before
storing anything.
ui::app::tests::a_failed_load_or_import_does_not_run_the_quit_after_it
improvise-cz8d. After #185, App::refuse does not abort. So on gh/main a failed
load refused, and the force-quit after it in the same batch still ran:
load absent.improv . q quit having loaded nothing. bok4's aborting helper had
prevented that, and the merge would have silently lost it. The test binds
[load|import-json <absent>, force-quit] as one sequence and asserts that the
refusal is recorded and the mode is not Quit. import-json failed first,
before refuse_and_stop was applied to its arms. For load, the injected fault
is removing its abort (refuse_and_stop back to app.refuse), and the test
then fails with "load absent.improv . q quit having loaded nothing".
improvise-514o: every refuse_and_stop arm
The case list above covered load and the JSON cannot-read arm only. PR #181's
review reverted each refuse_and_stop to app.refuse one at a time, which
keeps the refusal and drops the abort. Five ImportJsonHeadless arms failed
nothing: CSV cannot-read, CSV parse, JSON parse, array path and build. The wire
table every_failing_model_command_refuses_its_whole_frame cannot see the
abort, because Host::mutating_frame rejects on refused || aborted, and the
refusal alone suffices. The list now has one case per arm, using the fixtures
the wire table uses, which now come from ui::host::import_fixture_storage.
Each case also asserts the arm's own status text, so a fixture that failed in a
different arm cannot stand in for the one it names. The same scripted sweep,
rerun after the change, fails this test at all seven sites. Before, it failed
at two.
ui::app::tests::a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text
improvise-v4hg. In FormulaEdit, Enter on Total = ( reported the parse error
and then did what a successful commit does: CommitBuffer's
ChangeMode(FormulaPanel) closed the editor and the keymap's welded
ClearBuffer("formula") cleared the text. The user saw the error with their
formula gone. Driven through App::handle_key from FormulaPanel: a, the
characters, then Enter. The test failed first with "got FormulaPanel". The fix is
refuse_and_stop on the parse-error arm, and a Binding::Sequence is one
effect vec, so the stop also covers the ClearBuffer step.
The mode assertion alone would be vacuous against a fix that stops the mode
change but still loses the text. That rival (clear the formula buffer, then
refuse_and_stop) was run and fails "and must keep what they typed" with
left: None. The control, Total = 1, asserts the ordinary path still stores
the formula and closes the editor, so a stop on every arm would fail it. After
the fix, the abort sweep (refuse_and_stop back to app.refuse) fails this
test at that site.
ui::app::tests::every_refusal_site_refuses_its_failure_and_only_its_failure
improvise-0v8l. PR #185 split refusals (App::refuse, which the headless exit
code reads) from reports (set_status), and most of the refusal sites were
unpinned. The ticket counted 23 of 27. At gh/main 608ba6a (after #181's wire
table), a script (/tmp/refuse-fu/sweep.py, not committed) reverted each of
the 32 production refusal sites in ui/effect.rs and command/cmd/*.rs
alone to a status-only write, and ran cargo test --workspace --no-fail-fast.
For effect::refuse(x) the revert is effect::set_status(x). For
app.refuse(x) it is app.view_state.status_msg = x.into(). For
refuse_and_stop it is the status write with the abort kept. 13 of 32 failed
a test. 19 survived: AddCategory at the limit, apply_coordinate_rename,
both StartImportWizard arms, the five ImportJsonHeadless arms the wire
table cannot see (the abort alone rejects the frame), rename_record_coordinate,
CommitExport, both ViewNavigate arms, both AddRecordRow arms, both
EditOrDrill arms, SearchNavigate and ExecuteCommand.
This table has one case per survivor. Each case drives the failure through a
: line (run_line, which does what improvise cmd does, one batch per
segment) or through Enter in the site's own minibuffer mode. It asserts that the
batch refused and that the status names the site's own reason. Each case also
has a succeeding twin that must not refuse, so a site that refused
unconditionally fails too. That rival (AddCategory refusing on Ok as well)
was run and fails "its succeeding twin was refused". Rerun after the change,
the sweep catches 32 of 32. The abort half, 10 sites before v4hg's eleventh,
went from 5 caught to 11 of 11.
Three fixture choices. The second was found when the first run failed: the input meant to be refused was accepted.
AddCategorypast the limit needs eleven categories and a two-segment line. The firstadd-categoryfills the twelfth slot and the second is refused, so the succeeding twin is the same fixture with one segment.add-record-rowonto an existing row must page the category on the view the client is actually on. Records mode switches to_Records, so writingWorkbook::active_view_mut(the document's default) changed a view nobody was looking at, and the case "succeeded". It goes throughApp::view_mut_of(&ViewTarget::Active)(gotcha 29).- The drill rename edits record 1, not record 0, following the cursor-at-index-0 rule in the wave agent rules. Nothing failed here: this is a precaution, not a finding.
ui::effect::tests::a_drill_rows_measure_edits_follow_its_rename
improvise-uo2p. ApplyAndClearDrill walked pending_edits, a HashMap, and
applied each edit alone against the model as the previous one left it. A rename
moves the row's cells. A measure edit then wrote row.cell_key(measure), the
snapshot's old key. So when the rename came first, a ghost record appeared at the
old coordinates carrying the new value, and the moved record kept the stale one.
The reviewer saw this in 16 of 40 fresh Apps. The fixture stages two measure edits
and a rename on drill row 1, so a wrong answer needs only one of the three orders
that put the rename last. Each assertion runs over 32 fresh Apps and compares the
whole store, so the ghost and the stale value are both visible. Before the
fix it failed on five of five runs. Rival 3 (the rewrite addresses measure edits
at the unrenamed key) fails it.
ui::effect::tests::a_drill_row_renamed_twice_and_edited_lands_as_one_record
improvise-uo2p. This is the case that fails every time, not with some
probability. Each rename read the snapshot's old keys. The second rename found
nothing there, because the first had moved it, so one rename was always dropped
and the row split into a Clothing/Feb record and a Drink/Jan record. Rival 1
is the ticket's proposed fix direction: measures before renames, each edit
applied alone. It passes the ticket's own case and fails here. So "a fixed
order" is not a fix; the renames have to be composed.
ui::effect::tests::a_drill_that_swaps_two_records_coordinates_swaps_them
improvise-uo2p. Row 0 is renamed onto row 1's coordinates and row 1 onto row 0's.
If the rows are applied in sequence, the first move overwrites the other record
and the second move carries the overwritten values back, so one record's data
is lost in either order. Rival 2 composes each row correctly but writes it
before reading the next row, and it fails here and nowhere else. That is why
RecordRewrite::merged reads every row off the model first, then clears every
old key, then writes.
ui::effect::tests::a_drill_edit_addresses_its_own_record_not_the_one_renamed_onto_its_coordinates
improvise-uo2p. Row 1's staged Cost belongs to row 1's record, even though row
0 is being renamed away from coordinates and row 1 onto them. Before the fix,
some hash orders left Clothing/Jan/Cost = 999 as a ghost. Rival 3 fails it.
ui::effect::tests::two_drill_rows_renamed_onto_one_record_resolve_to_the_later_row
improvise-uo2p. Two rows renamed onto the same coordinates collide per cell. The
rule chosen is that the later snapshot row wins, which is what plain records
mode gives when the same renames are made top to bottom. Before the fix the
winner was the hash order's choice. Rival 4 reverses the row order in merged
and fails here. The rule is a choice, and it still loses data silently. See
context/code/command.md, RecordRewrite.
ui::effect::tests::a_record_rewrite_is_the_same_under_every_order_of_its_edits
improvise-uo2p. The composition is order-independent by construction, so it is asserted over all 24 orders of four edits on one row: two renames, a measure write and a measure clear. The test pins one value as well as the equality, so a rewrite that is the same wrong answer in every order still fails. Rival 3 fails it.
command/cmd/tile.rs — the tile bar's offered set, by name (improvise-jgzw)
the_tile_bar_offers_every_category_by_name_measure_and_records_axes_included
This test pins a decision, not a bug. The argument is in context/code/command.md
under cmd/tile.rs. Before this test, no tile assertion named the whole offered
set. The two older ones named only the category at cursor 0, so a filter that
changed what the bar offers past the cursor was invisible. The fixture puts
_Measure on Column, as every shipped document does. The test then walks the
cursor past the end and asserts the exact list
[_Index, _Dim, _Measure, Type, Month].
Rivals, injected, full workspace run:
regular_category_names()inTileAxisOp, the obvious "no virtual categories in pickers" fix, which drops_Measuretoo. It fails this test (["Type", "Month"]), the two characterization tests above it, andlockstep.rs::a_locally_resolved_tile_axis_change_agrees_with_the_host.View::tileable_catsfilteringis_index_or_dim, the ticket's option (b). With the list asserted this test fails by construction, so the point is what else fails:renders_each_tile_with_its_axis_label(TUI) andthe_tile_bar_route_into_records_mode_does_not_strand_the_user. The second one is improvise-bd9o's own test of the route this decision keeps.
command/cmd/determinism.rs — the determinism law over every command (improvise-hzjo)
The reasoning is in context/code/command.md, "A command is a function of its
context". This entry records what each test can see.
every_command_gives_equal_effects_in_independently_built_equal_contexts
It runs all 89 registrations, 1500+ (command, args) pairs, in four contexts built
from four separately parsed copies of web/demo.improv, and requires equal
effect Debug text. It passed at ae96b8c with no exemptions. It is not a
regression test for a live bug. It restores a law, and the evidence is which
rivals it catches:
command-mode-backspacereadingctx.buffers.values().next()instead of thecommandbuffer: a real command following hash order. Injected, full workspace run: this test is the only failure, because everytext_buffer.rstest builds a one-entry buffer map. With the comparison switched to one reused context (compare(&a, from_ref(&a))) the same rival passes. That is the half the ticket required: the distinct construction is what does the work.DataStore::matching_positionswithout its sort: gotcha 18 re-shipped. This entry first said the test catches it throughdrill-into-cellandfit-measure's record order. That was wrong (improvise-e1sm).fit-measurenever diverged, in 60 failing runs in review and in 354 here. And the catch was a matter of luck. The only real drill wasProduct/Latte, Quarter/Q2, which is one record with two cells, so each other context agrees with probability 1/2. With the sort removed and the test binary run in a loop, the law passed 46 of 400 runs atfc0b13c, anddrill-into-cell ["Product/Latte", "Quarter/Q2"]was the only pair that ever diverged. The arguments now include every item of every regular category as a one-coordinate drill, read off the parsed document (four records, eight cells each in the demo). With the same injection the law passed 0 of 400, and all eight new drills diverged in about 1195 of 1200 pairwise comparisons each. Without the injection it passed 400 of 400. The threecell.rsorder tests still pin the store directly.- The derived drills dropped from the argument list (the
all.extend(one_coordinate_drills(model))line commented out): the test fails with "the widest narrowing drill compared froze 1 records". drill-into-cell's parser changed to refuse fewer than two arguments (improvise-b9f2): every one-coordinate drill then goes to the one-record cursor fallback. The first width guard measured what the argument lists match, so it stayed silent. With gotcha 18 re-injected the law passed 38 of 300 runs in review, and 31 of 300 here atcfcc9bff. The guard now applies the compared drills' own effects to a scratchAppand counts the records they froze. Against this rival it fails on every run with "froze 1 records".- A first version of the fix counted all frozen records and still passed.
A candidate with no
Cat/Itemargument drills the whole store (16 records), and it reads them in insertion order throughiter_cells, which the injection cannot reach. So the guard counts only drills that froze fewer records than the store holds. - Measured at
7dfa90f1, test binary in a loop:Faults Law passes Guard fired gotcha 18 + parser rival 0 of 300 all 300 gotcha 18 alone 0 of 300 0 (every failure a real divergence) neither 300 of 300 0
- A first version of the fix counted all frozen records and still passed.
A candidate with no
StartDrill'sDebugchanged to print only its row count (StartDrill(4 rows), improvise-btyw): the law compares effects by theirDebugtext, and this rival takes the record order out of that text, while the frozendrill_statestill holds four records. At7dfa90f1, with gotcha 18 re-injected, the law passed 200 of 200 runs and the guard stayed green. The guard now counts a drill only when its frozen records, asDebugprints them, appear in the text the comparison compared. The same measurements after the repair, 200 runs each:Faults Law passes Guard fired gotcha 18 + Debugrival0 of 200 all 200 ("froze 0 records") gotcha 18 + parser rival 0 of 200 all 200 gotcha 18 alone 0 of 200 0 (every failure a real divergence) neither 200 of 200 0 - A stale exemption:
("quit", "stale")listed inOUTSIDE_THE_CONTEXT. It fails as "listed … but did not".
A vacuity this test itself had on its first version. The fixture's filler
buffers held v0…v15. The command-mode-backspace rival pops a character,
so every value collapsed to v (or v1), and the rival passed across two
distinct contexts. This is the "two derivations the fixture makes agree" shape.
The values are now value {i} of 16. The comparison was also widened from one
other instance to three, because a take-the-first rival agrees across two seeds
about one time in 21.
only_distinct_instances_can_see_a_command_that_follows_hash_order
The check-the-checker behind the law. A test-only EmitsInHashOrder emits one
SetBuffer per ctx.buffers entry in iteration order. It must equal itself on
one context, which is why the old single-context law was vacuous, and differ
across two independently built ones. If a future refactor made the fixture
share one map, or share one RandomState, the second assertion fails. Without
this test the law would then pass everything and look healthy.
changing_the_target_renames_the_formula_in_its_own_place
improvise-km71. This is the test that separates "edit" from "add". The rival
was the first build: commit-buffer formula was resolved in parse_commit_kind,
where the registry's parse-first rule never reaches the mode, so every edit
became an AddFormula. committing_an_edit_replaces_the_formula_in_place
passed under that rival, because add_formula upserts by
(target, target_category) and an edit that keeps its target lands in the same
slot. Injected on purpose (the commit emits AddFormula for the replace kind):
this test, the rename-status test, the collision test, the parse-error test and
both concurrency-refusal tests fail, and the in-place test passes. A rename is the input the upsert cannot fake. The fixture has three
formulas with the cursor on index 1, so "the one under the cursor" is not also
"the first" or "the last".
an_edit_lands_on_its_formula_after_an_earlier_one_is_deleted_meanwhile
improvise-km71. The capture is made when the editor opens and never re-read
from the cursor. Rival: ReplaceFormula resolves its formula from
formula_cursor at commit. The fixture deletes Profit (index 0) while the
editor is open on Tax (index 1), so cursor 1 now names Margin. The rival
rewrote Margin, and this test fails, along with
an_edit_whose_formula_was_deleted_meanwhile_refuses_out_loud and
an_edit_whose_formula_was_changed_meanwhile_refuses_out_loud. The other nine
formula-edit tests pass under it, because none of them moves the list.
an_edit_whose_formula_was_changed_meanwhile_refuses_out_loud
improvise-km71. Another client rewrote Tax's text but kept its target, so the
id still resolves. Only the was check can see this. Rival: the was filter
deleted. This test is the only one that fails. That is the case for keeping
was after improvise-sboz gives formulas a stable id: a concurrent replace
keeps the id.
a_rename_onto_another_formulas_target_refuses_and_keeps_both
improvise-km71. Rival: Model::replace_formula's collision check disabled.
This test and replace_formula_refuses_to_displace_another_formula fail.
Without the check, renaming Tax to Margin leaves two formulas with one
identity, and formula(&id) then finds the first.
a_fresh_session_has_no_history_to_step_back_through
improvise-lxg0. project.rs::fresh_view built a new session's slice by folding
SwitchView(active) into ViewState::default(). That default's active_view
is the literal Default, so the fold pushed a {Default, Normal} back-frame
for a visit that never happened. The test connects to two documents, and both
open on Second. One has a Default view: there < would take the client
to a view it never visited, which is improvise-lrrx's symptom seen from the
client. One has not: there the frame names a dead view, and
reduce_view::stepped would step onto it anyway. A fixture that opens on
Default is right by accident, because the phantom frame then names where the
client already is. Before the fix both cases failed at the back-stack
assertion with left: [ViewFrame { view_name: "Default", mode: Normal }].
The rival is that unfixed fold. The second assertion folds ViewBack and
checks that the client stays on Second, so a fix that emptied the stack
while still recording a move elsewhere cannot pass.
a_rebased_session_has_no_history_to_step_back_through
improvise-lxg0, through the other door. Sessions::rebase reseeds through
fresh_view too. The session connects to a document opening on Default, then
install_workbook swaps in each of the two documents above. Against the
unfixed code it failed the same way.
The records-grid enumeration checks kinds, not presence (improvise-w1bu)
The review of #196 injected RecordCol::Category(c) if row == 0 => Some(c.clone())
into cell_id. That projects row 0's coordinate as an editable Record with
measure: Some("Region"), and the whole workspace passed, because the
enumeration asked only "is every cell something". It now asks "is every cell
the right thing", column by column. Re-injected here, exactly one test in the
workspace failed, this one, with a records cell of the wrong kind: [(0, 0, Some(Record(RecordCell { .., measure: Some("Region") })))].
Since improvise-hoxy, names_its_column has no (Measure, Derived) arm. The
fixture has no formula, so accepting Derived there only let a bug through.
The review's rival was RecordCol::Measure(m) => Some(Derived { target: m }) in
cell_id. With the arm present, this test passed against that rival.
Without it, the test fails and names all four measure cells as Derived. A
fixture that does carry a formula is
a_records_column_a_formula_owns_is_derived_not_a_record's job, not this one's.
JSON import numbers (improvise-6yvg)
a_json_import_stores_its_numbers_bit_for_bit
ImportJsonHeadless imports [{"Region":"East","Revenue":0.24815714285714285}]
and the test reads Region=East, _Measure=Revenue back by f64::to_bits.
StartImportWizard parses through the same serde_json::from_slice call, so
one test covers the crate's feature.
Red at 1158c23 in cargo test -p improvise-command: stored
0.24815714285714283. That reproduces improvise-6yvg. Limitation: this test
cannot see the feature removed from improvise-command alone, because
improvise-io, a normal dependency, enables it in the same build. That case is
held by every_crate_that_can_parse_json_enables_float_roundtrip (tooling.md).
the_grid_and_a_csv_export_agree_on_a_fold_row
The projection-level pin PR #203's review asked to keep. The review's own
scratch test is made permanent here. Q1 = {Jan, Feb, Mar} with March hidden and
funded (50/50), so a fold that covered hidden members would read 90 / 58 / 0.36.
grid_of (what a client is sent) and export_csv_to (what a file gets) must
both read 40.00, 8.00, 0.80 on the Q1 row. The two go through different
producers (viewport::grid_body and render_csv), and both reach
GridLayout::evaluate. Rival: that evaluation covering every member. This test
fails with ["90.00", "58.00", "0.36"].
p_onto_a_sparse_record_writes_it
improvise-q9eg, a regression introduced by PR #197 and found in its review. In
records mode a category is added (as +cat does), so every existing record
lacks it and it sits on Axis::None. yy on East/Cost, then p on West/Cost
(row 1): gh/main pasted 700, and #197 refused with the aggregate message.
value_cell_key handed PasteCell the real record key, and paste_into then
re-ran aggregates(ctx, key). That key-shape check is a second derivation
beside cell_id, and it reads a record missing a None-axis category as an
aggregate. Every records fixture before this one was fully populated, which is
why nothing saw it. The fix is PasteAt::{Resolved, Spelled}: a resolved key is
written as it stands. Rival: routing Resolved back through paste_into. This
test and p_onto_a_sparse_record_writes_what_the_browser_writes fail.
ui/effect.rs — RecordRewrite's untested branches (improvise-phg1)
renaming_a_drill_row_held_by_a_lone_measureless_cell_moves_that_cell
improvise-phg1(a). A row seeded by o (AddRecordRow) carries its coordinates
on one cell with no _Measure, and RecordRewrite::of moves that cell in its
(false, true) arm. Replacing the arm with Vec::new() left the whole workspace
green, and a renamed o row then left CellKey([]) behind with no coordinates.
The gap predates improvise-uo2p, because rename_record_coordinate had the same
arm and nothing tested it. The fixture is that literal case: an empty-coordinate
cell on drill row 1, renamed. Under the rival it fails with -/-/- = 0 still in
the store, and nothing else in the workspace fails.
a_measure_a_sparse_drill_row_lacks_is_written_at_the_renamed_record
improvise-phg1(b). An edit to a measure the row has no cell for goes through
of's second loop, which must write at the renamed key. Every uo2p fixture
row carried every measure, so that loop only ever ran without a rename, and
writing row.cell_key(measure) unrenamed brought uo2p's ghost record back
unseen. Here drill row 1 has Revenue but not Cost, and stages Cost and a
rename. Under the rival it fails with a Clothing/Jan/Cost = 999 ghost, and
nothing else in the workspace fails.
every_drill_row_reads_the_model_before_any_row_clears_it
improvise-phg1(c). RecordRewrite::merged puts every clear after every read.
The rival lets each row's clears land before the next row reads the model, but
defers every write to the end. The uo2p swap test cannot see that rival: a
row's clears aimed at its own old keys touch nothing another row reads. A wider
rival, where each row's clears and writes land before the next row reads, is
a different fault: the swap test does catch it (uo2p's rival 2, which fails
a_drill_that_swaps_two_records_coordinates_swaps_them alone). So this test
exists for the narrow rival only. What separates it is a clear aimed at
another row's key. A blank edit on a measure the row lacks clears that
measure at the row's renamed coordinates. So row 0 is renamed onto row 1's
coordinates and blanks Profit, which only row 1 has, and row 1 is renamed
away. Row 1's Profit must still arrive at Food/Jan. Under the rival it is
cleared before row 1 reads it, Food/Jan/Profit = 5 is lost, and nothing else
in the workspace fails. This probe is the reviewer's, from the ticket.
a_blank_edit_on_a_measure_a_renamed_row_lacks_clears_it_at_the_renamed_record
improvise-q2mq. RecordRewrite::of's second loop handles the measures a row has
no cell for. Its clear arm must aim at the renamed key, exactly as its
write arm must (improvise-phg1(b)). The phg1 sparse-row test stages a value, so
it covers only the write arm. Aiming the clear arm alone at
row.cell_key(measure), unrenamed, left the whole workspace green.
The probe is the reviewer's. Row 0 (Food/Jan, Revenue = 1) is renamed onto
Clothing, where row 1 already holds Profit = 5, and blanks Profit, which
row 0 lacks. The blank belongs to row 0's final record at Clothing/Jan, so the
store is exactly {Clothing/Jan/Revenue = 1}. Under the rival the clear lands
on Food/Jan/Profit, where there is nothing to clear, and
Clothing/Jan/Profit = 5 survives. Injected alone into a full workspace run,
the rival fails this test and nothing else.
a_wire_write_to_a_collapsed_fold_is_rejected_and_stores_nothing
improvise-ciw8, through a real Host frame. Q2 is collapsed, and a hand-rolled
client sends SetCell (then, separately, ClearCell) for {Month=Q2, _Measure=Cost}
with a caught-up basis. At ea119bd9 both were answered Accepted, and the
SetCell stored Month=Q2, _Measure=Cost = 7, the non-item coordinate
improvise-gw18 exists to prevent. The test asserts Rejected(MutationRefused)
and an unchanged document for both. Rival: the aggregate refusal disabled. At
46e75c41 that failed this test and the None-axis test. Since improvise-b114 a
fold is refused first by the group rule, so this rival fails only the None-axis
test. Disabling both rules fails this test. The control is
a_wire_write_to_a_stored_cell_beside_a_fold_is_accepted, so a host that refused
every wire write cannot pass.
a_wire_write_under_a_formula_is_rejected_and_stores_nothing
improvise-ciw8's Derived arm: SetCell on Feb × Profit, where
Profit = Revenue - Cost. At ea119bd9 it was accepted and stored a value under
the formula. Rival: the formula refusal disabled. Only this test fails.
a_wire_write_to_a_none_axis_aggregate_is_rejected_and_stores_nothing
improvise-ciw8's other Aggregate. With Product on Axis::None, the grid's
key {Region=South, _Measure=Revenue} is a partial key that no stored record
has, so it resolves only through the layout, where cell_id says Aggregate.
At ea119bd9 it was accepted and stored the partial key. It fails under the
same rival as the fold test.
a_wire_write_to_a_record_that_never_existed_is_rejected
The evidence for keeping model_effects' vec![] arm rather than turning it
into a refusal (context/code/command.md, "A wire write reaches only a cell the
host projects as a Record"). A lone write naming a record that never existed is
answered RecordGone before anything runs, both when a caught-up basis claims
it and when a hand-rolled basis omits it. The second half was expected to be
ChainBroken, and the test said otherwise: the create exemption covers only
commands after the first in a frame. That surprise is why it is asserted rather
than argued.
an_empty_cell_beside_a_collapsed_measure_group_is_writable
improvise-sjdb, a regression PR #217 introduced and its review found. _Measure
is on Column with Revenue and Cost in a collapsed group Money and Units
ungrouped, and the Feb row is empty. The host projects Feb × Units as a
Record. A caught-up SetCell on it was answered Rejected(MutationRefused)
at bc060f67. It was accepted before #217. aggregate_refusal found the
unstored record by RecordHash alone, which drops _Measure, so it judged the
record at the first column on Feb's row, the Money fold, which cell_id calls
Aggregate. The setup asserts that the fold comes before Units on the row and
that the target projects as Record, so a fixture where the first column is
harmless cannot pass. The fix looks the cell up by its whole key. The rival is
the old lookup by record, which is this test's red commit, 6e021699.
a_wire_write_naming_a_measure_group_is_rejected_in_any_view
improvise-b114, found in the review of PR #217 at bc060f67 (the reviewer's r3).
_Measure is on Column, Revenue and Cost are grouped as Money, and Units is
ungrouped. Every month is funded. A caught-up SetCell on
{Month=Feb, _Measure=Money} was answered Accepted and stored
Month=Feb, _Measure=Money = 7, a non-item coordinate. Its record {Month=Feb}
is stored, so the stored arm accepted it without asking the projection. The test
runs with Money collapsed, where the host draws that cell as the fold, and
expanded, where the host draws no such cell. It asserts MutationRefused and an
unchanged document in both, so a rule that asks the host's layout cannot pass
the expanded half. Red at 5eaafbae. The fix is App::group_refusal, which asks
the document whether any coordinate names a group rather than an item. Rival:
the group rule removed. Injected into a full workspace run, it fails this test
and nothing else.
a_wire_write_to_a_fold_record_with_an_undrawn_measure_is_rejected
improvise-xur5, found in the delta re-review of PR #217 at 6026768c. It is the
reviewer's review_217b::probe_fold_hidden_measure, widened to all three rows
of the reviewer's table. Q2 is collapsed and every month is stored, so the
record {Month=Q2} is unstored and resolves only through the layout, at the
fold row. A caught-up write names that record with a measure the host draws
nowhere: Cost hidden in the host's view, Bogus, which is no measure at all,
or no measure. At 6026768c all three were accepted and stored Month=Q2,
a group name, as a coordinate. The sjdb fix finds the position by the whole key,
and "not drawn" read as "not refused". Falling back to the by-record lookup for
an undrawn key would close this, but the reviewer showed it wrongly refuses
Feb × Revenue beside a collapsed Money group, so it is not the fix. The
improvise-b114 rule is: no coordinate names a group, asked of the document.
Rival: the group rule removed. Injected into a full workspace run, it fails this
test and the b114 test, and nothing else.
a_stored_record_the_hosts_view_aggregates_is_writable
improvise-sjdb, from the #217 review. South/Revenue is stored, then Product is
added on Axis::None in the host's view, so the host projects South's position
as Aggregate. A stored record is a record wherever it is drawn, because
RecordHash is document-scoped (gotcha 44). A records-mode client, or one on
another view, may write it. The setup asserts the host really does project
Aggregate there, so the test cannot pass on a view that draws South plainly.
Rival: stored records sent through cell_id like unstored ones. That rival
fails this test and nothing else in improvise-command. Before this test,
nothing in the suite saw it.
a_fresh_sessions_slice_is_the_hosts_startup_slice
improvise-v8ab. A fresh session's slice is built twice: App::with_storage
builds the host's own, and project.rs::fresh_view builds a client's. Nothing
asserted they agree. tests/lockstep.rs seeds most of its clients from
app.view_state.clone(), and those could not see improvise-lxg0's phantom
frame or the next drift between the two. There are three exceptions.
switching_between_a_records_view_and_a_pivot_view_settles_both_ways and
a_locally_resolved_page_turn_lands_where_the_host_would_put_it go through
Host::connect, and the published_basis helper uses Sessions::connect.
None of them compares the connected slice with the host's. The test connects to two documents, both opening on Second, one with a Default view
and one without. It asserts the whole ViewState equals the host's, not only
the back stack, so any field the two constructors disagree on fails it. The
rival is the old fold, restored in fresh_view. It failed on the
["Default", "Second"] case: the only difference was view_back_stack: [ViewFrame { view_name: "Default", mode: Normal }].
The scope is connect only. A rebase deliberately resets the client's whole
slice, while install_workbook keeps the host's mode and panels, so equality
is not the contract there. lockstep.rs still copies app.view_state, and
that is deliberate. Its helpers seed a client after mutating the host (mode,
buffers, cursor), so a Snapshot would drop exactly the state those tests
stage. This test is what makes the copy faithful at startup.
ui::effect::tests::recomputing_formulas_leaves_the_model_fingerprint_alone
improvise-pc3b, found by the review of PR #224 at e62a289. That PR moved the
effect-classification fingerprints from serde_json to Debug. The derived
Debug for Model prints formula_cache, a HashMap that recompute_formulas
clears and refills, so it prints in hash order. The reviewer measured a correct
ViewOnly effect (RebuildLayout) changing the fingerprint in 90 of 200 runs
once formulas were present. test_app has none, so the suite was green over a
latent flake.
The test builds formula_app (two measures across a 2×2 grid, plus
Profit = Revenue - Cost and Margin = Profit / Revenue) 32 times. Each build
is a fresh App, so each gets fresh hash states. Each time, it asserts that
RebuildLayout leaves model_fingerprint unchanged. One flip in about two makes
32 independent tries fail near-certainly.
Red before the fix: the two renderings differed only in formula_cache's
entry order. The fix is a hand-written Debug for Model that renders
formula_cache and measure_agg in key order.
Rival, run: keep the hand-written Debug but print &self.formula_cache
unsorted. This test fails. view_only_effects_leave_shared_state_alone now runs
over formula_app too, so every ViewOnly case is checked against a populated
cache. The two earlier injections (a SetStatus that writes the model, and one
that writes a view's number_format) still fail it.
a_document_opening_on_a_records_view_opens_the_host_in_records_normal
improvise-y5eo. App::with_storage set view_state.mode to Normal and never
settled it against the layout. The host settles only at the end of a batch
(settle_normal_mode in apply_effects). A session's mirror settles in
project_diff, so Sessions::connect handed a client RecordsNormal while
the host sat in Normal over the same records grid. The host's first i then
resolved through Normal's keymap and entered plain Editing. The fixture's
active view puts _Index on rows and _Dim on columns, which the .improv
writer does persist. The test asserts the starting mode, then presses i and
asserts the host is still in a records mode. The fix calls
settle_normal_mode once at the end of construction, so the startup mode
comes from the same rule every batch ends with, and so from the same
improvise_protocol::settled_mode a client uses.
Rival injected: that one call removed. The whole workspace had exactly two
failures. This test failed at the mode assertion (left: Normal, right: RecordsNormal), and a_fresh_sessions_slice_is_the_hosts_startup_slice
failed on its new records case: the client's RecordsNormal against the
host's Normal, and nothing else different. install_workbook needs no
change of its own, because both of its callers (LoadModel,
AcceptWizardModel) run inside apply_effects, which settles after the batch.
renaming_a_formula_outside_measure_refuses_and_keeps_the_editor
improvise-kb1d, found by the pre-merge review of #214 and by ChatGPT Codex (P1).
The fixture has a Region-targeted formula at index 1, between two _Measure
formulas, because every other km71 test used _Measure, whose items are
derived from the formula list and cannot show a stranded item. Rival: the guard
in Model::replace_formula disabled. This test fails on the formula list, and
replace_formula_refuses_to_rename_a_target_outside_measure in core fails too.
an_untouched_pivot_edit_of_a_misread_value_writes_nothing
an_untouched_records_edit_of_a_misread_value_writes_nothing
improvise-ixxx. After umi4 the seed was exact, but the commit still ran it
through typed(), which reads text and not the cell the text came from. So
i then Enter with nothing typed stored Number(7.0) over Text("007"),
and Text("ERR:boom") over a raw Error("boom"). The values are the ones
where typed(edit_text(v)) != v: "007", "1e3", "inf", "2024" and a raw
Error. They use no_op_commit, the umi4 tests' own path: the real key path,
on (West, Revenue), which is row 1 and never the first cell.
Both failed before the fix. With the list as written they failed on
Text("007") stored as Number(7.0). With Error("boom") moved to the front
they failed on it, stored as Text("ERR:boom"), so the error case is red on
its own and not only behind the text one. Rival: untouched answering
false always, i.e. the fix removed. Both fail again.
no_op_commit now also asserts that the buffer after i is
value.edit_text(). That assertion is what keeps the umi4 tests above
alive. With ixxx fixed, an untouched commit writes nothing, so a seed
regressed to the rounded Display is never written either, and
an_untouched_{pivot,records}_edit_stores_the_identical_value pass with umi4
reverted. That was run: with the seed assertion removed and
display_value_in seeding from to_string(), all six untouched tests pass.
With the assertion in, both umi4 tests fail.
an_untouched_edit_does_not_overwrite_a_write_made_while_it_was_open
The reviewer's first constraint on improvise-ixxx: the seed is the one
captured when the editor opened. The test opens the editor on
Text("007"), then applies a SetCell(Number(5.0)) directly, standing in for
another client's write that lands while the editor is open. Then it presses
Enter with nothing typed. The cell must still be 5.
Failed before the fix: the commit wrote Number(7.0). Rival:
commit_at_cursor taking the seed from ctx.display_value, which is
recomputed at commit time, instead of ctx.opened_editor. This test fails
alone, with Number(7.0), and every other untouched test passes. So it is the
only guard on the choice of seed, and a fixture where the cell does not move
cannot tell the two seeds apart.
an_untouched_commit_still_advances_and_reopens_the_editor
The reviewer's second constraint: only the write is skipped. The fixture
puts Text("007") at (West, Cost), row 1 and column 0. Enter there advances
to (0, 1) rather than aborting at the corner, which is where (West, Revenue)
sits in pivot mode. The test asserts four things: the cell is unchanged, the
cursor moved to (0, 1), the mode is editing again, and the reopened editor
captured "2", the cell it landed on.
Rival: CommitAndAdvance::execute returning straight after the mode change
when untouched, which skips the advance along with the write. This test fails,
and so do tab_on_an_empty_new_measure_column_still_starts_a_record_row and
ui::app::tests::tab_on_bottom_right_of_records_inserts_below. Those two are
the records Tab contract: the empty + column's seed is "", so an empty Tab
there is now an untouched commit too.
The buffer after Enter is not asserted. The keymap welds clear-buffer
after every commit key, so it empties the buffer EnterEditAtCursor has just
seeded, and the reopened editor opens blank while its captured seed is "2".
That is existing behaviour (enter_commits_the_cell_advances_and_re_enters_editing
in clients.md). An Enter there with nothing typed therefore cleared the landed
cell. It was filed as improvise-fy9o and is now fixed (see
enter_enter_leaves_every_cell_unchanged).
an_editor_typed_back_to_its_seed_still_writes
improvise-2054, found by the #205 review. untouched compared content
only, so an edit that ended back on the seed text was treated as untouched. The
test opens the editor on Number(7) at (West, Revenue), which is row 1 and
column 1. It then applies SetCell(Number(5)) as another client's write,
presses Backspace and 7, and presses Enter. The cell must be 7. After the
Backspace alone, the test also asserts opened_editor.touched is set.
Failed before the fix: the cell stayed Number(5.0), and nothing told the
user. Rivals:
untouchedignoringtouchedfails this test.PopCharnot marking fails this test at the post-Backspace assertion.AppendCharnot marking cannot fail it, because the pop has already marked the editor. That rival failsappending_or_popping_marks_the_editor_touched, which presses each key on its own.
an_editor_opened_by_enter_mode_does_not_inherit_an_earlier_seed
improvise-wv3h. Deleting SetOpenedEditor(None) from EnterMode failed
no test. The reviewer's repro types 007 after :enter-mode editing, and it
no longer discriminates now that improvise-2054 marks the editor on the first
append, so this test commits with nothing typed. The steps:
ion an empty cell, (North, Cost), captures the seed"".- Esc.
clear-bufferdoes not mark the editor, so that stale seed stays untouched. - Move to (West, Revenue), which holds 2.
:enter-mode editing. That is typed into thecommandbuffer, which also does not mark the cell editor.- Enter, with the edit buffer
"".
The cell must be cleared. Rival: the guard deleted. This test fails alone,
leaving Number(2.0): the empty commit was skipped against the earlier
editor's seed.
This test is only possible because clear-buffer and keys typed into another
buffer do not mark the editor. A rule that marked on every buffer write would
make the guard redundant, and no test could pin it (protocol.md §3).
command::cmd::commit::tests::the_editor_an_advance_reopens_starts_untouched
improvise-465t, found by the #205 re-review. Every open resets touched,
but no test pinned the reset on the editor that CommitAndAdvance reopens
through EnterEditAtCursor. The test works on the 2×2 pivot. It presses i
on (West, Cost), row 1 and column 0, types 9, and asserts the first editor
really is touched. Then it presses Enter. The cursor must land on (0, 1), and
the reopened editor must be exactly OpenedEditor::on("2"), which means seeded
from the landed cell and untouched.
Injection: EnterEditAtCursor carrying the previous editor's touched
flag. This test fails alone. Every other test stays green, which was the gap.
Once improvise-fy9o re-seeds the reopened editor, a stale touched would make
every untouched Tab or Enter write its seed.
enter_enter_leaves_every_cell_unchanged
improvise-fy9o. The keymap welded clear-buffer after every commit key, so
it blanked the editor CommitAndAdvance had just reopened. The next Enter then
committed "" and cleared the landed cell. The test presses i, Enter, Enter
on the 2×2 fixture:
- in pivot, from (1, 0);
- in records, from (1, 2), the West row's Cost column.
Every cell must be unchanged. Both cases failed before the fix, clearing
(North, Revenue). The records case was confirmed on its own, because the loop
stops at the first failure. Rival: the editing keys bound through the
welding .commit again fails both.
typing_after_the_reopen_still_writes
After i and Enter from (1, 0), the reopened editor must hold "2", the value
of (North, Revenue). Typing 5 and pressing Enter must then store 25. Before
the fix the buffer was "", so the typed digit replaced the value instead of
extending it.
improvise-i3o5: four declines that set_status hid from every sweep
every_refusal_site_refuses_its_failure_and_only_its_failure gained four cases.
PR #213's review found them by reading, because the sweeps behind this table
revert refuse( calls and cannot see a refusal written as set_status. Each
exited 0 headless and would not reject a frame:
EditOrDrill's collapsed-group arm (folded_quarter, cursor on Q2's fold at row 2). The succeeding twin is on Feb at row 1. Its two sibling arms already refused.paste_into's aggregate decline. The spelled keyRow/A Col/Aomits theHiddencategory on None. The twin spells all three coordinates.paste_into_record's coordinate-column decline:_Index/1 _Dim/0is Region. The twin,_Dim/2, is a measure.paste_into_record's inside-a-drill decline. The same measure key, under a frozen drill. The twin is the same key with no drill.
Red at e406edc, in the order the table reaches the cases. Each failed with
its own text and refused=false, and each went green only when its own site was
converted. The collapsed-group case failed first. Then, one conversion at a
time, the three paste cases failed with the aggregate, coordinate-column and
inside-a-drill text.
Per-site revert: /tmp/refuse-fu/sweep.py, not committed, restricted to the
four sites. It reverts each effect::refuse( to effect::set_status( alone and
runs cargo test --workspace --no-fail-fast. Four of four fail this test, and
nothing else fails. None of the four needs an abort: each is the only effect its
command emits, and p/i are single bindings, not sequences.
a_frame_that_reframes_and_then_places_the_cursor_leaves_the_asker_placed
improvise-wdmp. A browser frame that reframes the grid and then places the cursor,
for example the records +cat commit [ChangeMode, AddCategory, SetSelected], is
folded by the client all at once and before any answer arrives. The host's mirror
folds the same frame in order. adopt_cursor_epoch resets the mirror at the
mutation, and the later SetSelected places it. The relayed ResetCursor
reached the client after its own placement and undid it. So the client sat at
the origin while its mirror, and every projection built from it, sat at the
placement. That is a split, not merely the jump the ticket described. The test
drives a real Host and a real ClientState through fold_outgoing, then folds
every frame addressed to the asker. It seats the cursor off the origin first, so
a reset that never arrived is visible. It runs two shapes: with a placement
after the reframe (expect (1, 0)), and without one (expect (0, 0), gvnb's
reset). In both, the client's cursor must equal its mirror's. At the base the
placed case failed: the client was at (0, 0) and the mirror at (1, 0).
Two rivals. First, the echo disabled, which is the pre-fix behaviour: the placed
case fails. Second, the ticket's suggested fix, which drops the asker's
ResetCursor from the relay: the unplaced case fails, because the mirror was
reset and the client never heard. That rival also fails
a_geometry_command_relays_the_cursor_reset_to_the_asker_too and the
ws-server's the_other_document_would_also_have_a_cursor_to_invalidate.