review: the "grid, not mode" clause was argued and unchecked

Adversarial review of d124f18 found that the clause the fix was argued on had
nothing behind it. Both mode-shaped guards were injected and the WHOLE WORKSPACE
stayed green at 1659:

    if state.view.mode.is_records()   1659 passed, 0 failed
    if mode.is_records()              1659 passed, 0 failed

`the_tile_bar_route_into_records_mode_does_not_strand_the_user` cannot see it,
and not by accident: it enters records mode the way a user does, where the grid
and the mode AGREE. It is standing next to the question rather than on it —
a_careful_comment_is_weak_evidence_that_anything_checks_it, with the comment
being the one that argued hardest.

The separating input is a client whose mode has DRIFTED off its own records
grid, which is the fixture `a_records_commit_is_declined_off_the_grid_and_not_off
_the_mode` already built for `commit`. This adds its twin for the door, with the
same regular-grid control so the decline is not merely "this client never edits".
Both injections were re-run against it and both now fail at its first assertion.

Two prose corrections from the same review, each an overclaim:

  - "the toggle-records-mode decline is now TRUE" — it is not. The wording is
    "records mode is not available", and the tile bar still puts a browser in
    RecordsNormal with a records grid drawn. The fix closes the SILENCE, not the
    route; what is now true is that EDITING there is unavailable. So the
    stranding rule is still violated by the tile bar, and this guards the exit
    rather than the door — deliberately, since the room is worth being in and
    only the writes are missing. Saying otherwise was the same defect one layer
    up.
  - The CellId::Create arm's justification named a state that cannot exist:
    project.rs::cell_id emits Create only inside `if layout.is_records_mode()`,
    so the entrance guard takes every such cell and the arm change is a
    functional no-op with no test behind it. Kept, but no longer read as covered.

And the evidence file's column table stopped at 2-3 for a five-column fixture,
so a reader reconstructing it got four columns. The `+` row is restored.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH

context/code/clients.md

@@ -630,24 +630,58 @@ walks every column of the row for exactly that reason.
 It tests `state.grid.records` rather than `state.view.mode.is_records()` for the
 reason `commit` already did: the grid is the **observed** fact and the mode is
 settled from it (`settled_mode`), so a mode-shaped guard would be a second
-description of records-ness free to disagree with the first.
+description of records-ness free to disagree with the first — one arm of
+`doctrine(a_substitution_is_a_defect_at_every_site_that_makes_it)`.
+
+**And that clause needed its own test, because the route test cannot state it.**
+`the_tile_bar_route_into_records_mode_does_not_strand_the_user` enters records
+mode the way a user does, where the grid and the mode *agree*, so it cannot
+separate them: review injected both mode-shaped guards — `state.view.mode` and
+the keymap's own `mode` argument — and the **whole workspace stayed green**.
+That is this file's own `a_careful_comment_is_weak_evidence_that_anything_checks_it`
+caught in the act, and the more carefully the paragraph above argued, the more
+it was standing in for a check. The separating input is a client whose mode has
+**drifted off its own records grid**, which is exactly the fixture
+`a_records_commit_is_declined_off_the_grid_and_not_off_the_mode` already built
+for `commit`; `opening_an_editor_is_declined_off_the_grid_and_not_off_the_mode`
+is its twin for the door, with the same regular-grid control so the decline is
+not merely "this client never edits".
 
 **What this costs, stated rather than discovered later.** `create_column`
 (improvise-gcvb) is now **explicitly** dead rather than accidentally so. It was
 already unreachable — `edit_or_drill`'s `CellId::Create` arm returned `None`,
 and `o` is declined at its `add-record-row` step, so nothing opened
 `RecordsEditing` in a browser at all — but the deadness was an accident of two
-unrelated arms rather than a decision. The `CellId::Create` arm now *declines*
-instead of returning `None`, so that if a creating column ever reaches this
-function from outside records mode it is a door rather than a hole. Reviving
-`create_column` for real means opening the editor on `CellId::Create` while
-still refusing the other columns, which is a change worth making on its own
-evidence and not as a side effect of closing a silence.
+unrelated arms rather than a decision. Reviving `create_column` for real means
+opening the editor on `CellId::Create` while still refusing the other columns,
+which is a change worth making on its own evidence and not as a side effect of
+closing a silence (improvise-x0xd).
 
-And the existing `toggle-records-mode` decline is now **true**, where before it
-was a claim the tile bar contradicted. That is the standing lesson from
-improvise-cfly arriving from the other direction: there, a decline outlived its
-reason; here, a decline was never as wide as its wording.
+**The `CellId::Create` arm is unreachable, and saying otherwise was the third
+thing review caught.** It now declines rather than returning `None`, and a first
+draft of this paragraph justified that as "if a creating column ever reaches this
+function from outside records mode". It cannot: `project.rs::cell_id` emits
+`CellId::Create` only inside `if layout.is_records_mode()`, so the entrance guard
+takes every such cell and the arm change is a **functional no-op** with no test
+behind it. It is kept because an arm that returns `None` is a black hole if the
+producer ever widens, and deleted reasoning is cheaper than a wrong one — but it
+must not be read as covered. `doctrine(a_careful_comment_is_weak_evidence_that_anything_checks_it)`
+applies to a justification as much as to a claim.
+
+**The `toggle-records-mode` decline is still not true, and that is the honest
+reading.** A first draft of this paragraph said the fix made it so. It did not:
+the wording is "records mode *is not available* in the browser yet", and the tile
+bar still puts a browser in `RecordsNormal` with a records grid drawn. What the
+fix closed is the **silence**, not the route, and what is now true is that
+*editing* there is unavailable. So the rule stated above about `stranding` — "no
+path opens a room this client cannot commit" — is still violated by the tile bar;
+this guards the **exit** rather than the door, deliberately, because the room is
+worth being in (a browser can read a records grid, scroll it and page it) and
+only the writes are missing. improvise-cfly's lesson still lands from the other
+direction — there a decline outlived its reason, here a decline was never as wide
+as its wording — but a decline whose *sentence* remains an overclaim is not a
+decline made true, and writing that it was would have been the same defect one
+layer up.
 
 #### The three improvise-e24v listed as free, and what each really costs
 

context/test-evidence/clients.md

@@ -3621,8 +3621,14 @@ the projected row showed them to be different failures:
 | 0 (coordinate) | `None` | nothing at all — `edit_or_drill`'s `cell.id.as_ref()?` short-circuited |
 | 1 (`+cat`) | `Create(Category)` | nothing — the `Create` arm returned `None` |
 | 2–3 (measures) | `Record(_)` | opened the editor; `Enter` then died in `commit`'s `grid.records` early return |
+| 4 (`+`) | `Create(Measure)` | nothing — the `Create` arm returned `None` |
 
-Only the third is the case improvise-bd9o describes. The first is improvise-fdk1
+The fixture's grid is **five** columns wide (`extent = {rows: 1, cols: 5}`), and
+an earlier version of this table stopped at `2–3`, so a reader reconstructing it
+got four. Review caught that; the row above is why the table is worth having at
+all, since it is the only place the fixture's shape is written down.
+
+Only the measure columns are the case improvise-bd9o describes. The first is improvise-fdk1
 seen from the key path: a coordinate column has no surrogate because a rename
 names a record that does not exist yet. A fixture placed on a measure column —
 the obvious one to write, since that is where the ticket's transcript stands —
@@ -3655,3 +3661,41 @@ ticket's transcript spells the route as `T r l c Esc`, which depends on
 `tileable_cats`' order. `tile_to` finds the named category's index in
 `state().tiles` and walks to it, so the test pins *the route exists*, not a
 particular ordering the producer is free to change.
+
+### opening_an_editor_is_declined_off_the_grid_and_not_off_the_mode
+
+improvise-bd9o, and it exists because **the route test could not state the
+clause the fix was argued on.** `edit_or_drill`'s guard reads
+`state.grid.records` and not the mode, and both the commit message and
+`context/code/clients.md` make a point of it. Adversarial review injected the two
+mode-shaped guards and ran the **whole workspace**:
+
+| injected guard | workspace result |
+|---|---|
+| `if state.view.mode.is_records()` | **1659 passed, 0 failed** |
+| `if mode.is_records()` (the keymap argument) | **1659 passed, 0 failed** |
+
+So a wrong fix that the prose explicitly argues against passed everything,
+including `the_tile_bar_route_into_records_mode_does_not_strand_the_user`. That
+test enters records mode the way a user does — through the tile bar — where the
+grid and the mode **agree**, so it is standing next to the question rather than
+on it. This is `doctrine(a_careful_comment_is_weak_evidence_that_anything_checks_it)`
+with the comment being the one that argued hardest.
+
+**The separating input is a drifted client**: `records_client(&app)` with
+`view.mode` forced to `AppMode::Normal` over a records grid — the state
+`doctrine(a_substitution_is_a_defect_at_every_site_that_makes_it)` records as
+what the substitution actually produces. Under either injection the guard reads
+"not records" and the editor opens; under the committed fix the grid says
+records and it declines. Both injections were re-run against this test and both
+now FAIL at its first assertion.
+
+**The control is not optional.** A regular grid in the same `Normal` mode must
+still open the editor, or "it declined" is satisfied by a client that never
+edits anything — the same vacuity its sibling
+`a_records_commit_is_declined_off_the_grid_and_not_off_the_mode` guards against,
+and the reason this test is that test's twin rather than a new shape.
+
+It asserts on `Command::ChangeMode(m) if m.is_editing()` rather than on the
+status string, because the fault is *an editor opening*, and a fix that speaks
+and opens anyway is a distinct rival already covered at the route level.

crates/improvise-wasm-client/tests/lockstep.rs

@@ -1452,3 +1452,44 @@ fn a_locally_committed_creating_column_agrees_with_the_host() {
         );
     }
 }
+
+/// The twin of the commit test above, for the door rather than the exit
+/// (improvise-bd9o). Evidence file.
+#[test]
+fn opening_an_editor_is_declined_off_the_grid_and_not_off_the_mode() {
+    let (app, _) = records_pair();
+    let mut drifted = records_client(&app);
+    // The drift stated rather than reached: a plain `Editing`/`Normal` pair over
+    // a records grid is what the substitution produces.
+    drifted.view.mode = AppMode::Normal;
+    drifted.view.cursor_mut().selected = stored_cell_at(&drifted);
+
+    let i = KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE);
+    assert!(
+        !opens_an_editor(&drifted, i),
+        "an editor whose Enter this client cannot commit was opened anyway: a \
+         guard reading the MODE cannot see this client, whose mode has drifted \
+         off its own records grid"
+    );
+
+    // The control: the same mode over a REGULAR grid must still open, or the
+    // decline above is "this client never edits" and proves nothing.
+    let plain = furnished_host();
+    let mut ordinary = paged_client(&plain);
+    ordinary.view.mode = AppMode::Normal;
+    ordinary.view.cursor_mut().selected = stored_cell_at(&ordinary);
+    assert!(!ordinary.grid.records, "the control must be a regular grid");
+    assert!(
+        opens_an_editor(&ordinary, i),
+        "the control never opened either, so the decline above is vacuous"
+    );
+}
+
+fn opens_an_editor(client: &ClientState, key: KeyEvent) -> bool {
+    match resolve(client, key) {
+        KeyOutcome::Local(cmds) => cmds
+            .iter()
+            .any(|c| matches!(c, improvise_protocol::Command::ChangeMode(m) if m.is_editing())),
+        _ => false,
+    }
+}