From 8ee5fade27dfc6b3b2cae3611373b2fc625d4e2b Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Wed, 23 Sep 2026 07:57:01 -0700 Subject: [PATCH] fy9o: corner lockstep guard, evidence and protocol.md; file improvise-jmti Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/code/protocol.md | 27 ++++++- context/test-evidence/clients.md | 42 +++++++++-- context/test-evidence/command.md | 31 +++++++- .../improvise-command/src/command/keymap.rs | 16 +++-- .../improvise-wasm-client/tests/lockstep.rs | 70 +++++++++++++++++-- 5 files changed, 166 insertions(+), 20 deletions(-) diff --git a/context/code/protocol.md b/context/code/protocol.md index 9dcfe3a5..ceaf182a 100644 --- a/context/code/protocol.md +++ b/context/code/protocol.md @@ -1976,7 +1976,32 @@ edit are the same string **by construction** rather than by agreement (improvise-k8h) — the keymap never spells a buffer name. `commit(key, cmd, args)` welds the command to its `clear-buffer` so no caller -can bind a commit without its clear (improvise-w3q). `rebind` exists for +can bind a commit without its clear (improvise-w3q). + +**The two cell-editing commit keys are the exception, and the clear moved rather +than disappeared** (improvise-fy9o). `commit-cell-edit` and +`commit-and-advance-right` reopen an editor on the cell they advance to, and the +welded clear ran **after** them, so it blanked the re-seeded buffer. The +reopened editor looked empty, and a second Enter committed `""` and cleared the +landed cell. The user's ruling is that the reopened editor shows the landed +cell's current value, so with improvise-ixxx's untouched rule an unchanged Enter +writes nothing. `commit_reopening(key, cmd, args)` therefore binds the command +alone, and the command clears its own buffer **at its head**: +- in the TUI, `CommitAndAdvance` does it right after the mode change; +- in the browser, `resolve::cleared` does it, reached from both `commit` and + `create_column`. + +Being at the head is also gotcha 21's rule: a head clear survives an abort +where the trailing weld did not. The corner commit and the `+cat` insertion +reopen nothing, and they still leave no text behind. + +Reordering the weld to `[clear-buffer, cmd]` was tried first and rejected. Four +readers take a `Sequence`'s **first** step as its identity: `keymap::runs`, +`help.rs`, and two resolver audits. So every hint on those keys started +reporting `clear-buffer`. `text_entry_modes_clear_the_buffer_they_type_into` +keeps its guarantee by asking a plain-`Cmd` commit key's command what it emits. + +`rebind` exists for exactly one case: CommandMode's Backspace, which exits the mode on an empty `:` line instead of popping a character that isn't there. diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md index 69b9bd5e..4fc118e8 100644 --- a/context/test-evidence/clients.md +++ b/context/test-evidence/clients.md @@ -273,11 +273,14 @@ with an empty vec, so that would be a keypress the user sees do nothing at all ### enter_commits_the_cell_advances_and_re_enters_editing -Enter in `Editing` is `[commit-cell-edit editing, clear-buffer edit]`, and every -part of that sequence shows up in what the client composes — **including the -trailing clear that empties the buffer `EnterEditAtCursor` has just seeded**. -That ordering looks wrong and is exactly what the TUI does; mirroring it line -for line is the contract. +Enter in `Editing` used to be `[commit-cell-edit editing, clear-buffer edit]`, +and the trailing clear emptied the buffer the advance had just re-seeded. That +was improvise-fy9o. Since the fix, Enter is `commit-cell-edit` alone, and the +commit clears its own buffer at its head. The test pins that order: the write, +then `ChangeMode(Normal)`, then `SetBuffer(edit, "")`, then `SetSelected`. It +ends with the re-entry, and the **last** write to `edit` is the landed cell's +`"900"`. **Rival:** `resolve::commit` without its head clear fails this test, +and `a_commit_at_the_corner_leaves_no_buffer_behind_on_both_ends` too. --- @@ -4908,3 +4911,32 @@ the keyboard paths a trimmed comparison cannot be seen through the keys, since any whitespace edit also sets `touched`, so this unit test and the mouse path are where it shows. The mouse test now also asserts that `"7 "` against the seed `"7"` is a write. + +### `enter_enter_leaves_every_cell_unchanged_on_both_ends` + +**improvise-fy9o**, browser and host in lockstep. On the 2×2 `precise_host`, +starting at (South, Cost), row 1 and column 0, the test presses `i`, Enter, +Enter. The client resolves each key, folds it and applies it to one host, and +`reduce_key` drives a second host. After each key the client's buffers must +equal the key host's, and at the end every cell must be unchanged on both +hosts. + +Failed before the fix: (North, Revenue) was cleared, because the welded clear +had blanked the reopened editor. **Rival:** the editing commit keys bound +through the welding `.commit` again fails this test, the typing test below, and +both TUI tests. + +### `typing_into_the_reopened_editor_extends_the_cell_on_both_ends` + +The other half of the ruling: the reopened editor shows the landed cell, so +typing extends it. `i`, Enter, `5`, Enter must store `25` at (North, Revenue) +on both ends. It fails under the weld rival, because the digit went into a +blank buffer and stored `5`. + +### `a_commit_at_the_corner_leaves_no_buffer_behind_on_both_ends` + +Moving the clear from the keymap into the command must not leave text behind +where nothing reopens. At the bottom-right corner, `i`, `9`, Enter must leave +`edit` as `""` on both ends, with the mode no longer editing. **Rival:** the +browser without `cleared` leaves `"49"` and fails here. The equivalent TUI rival +fails `text_entry_modes_clear_the_buffer_they_type_into`. diff --git a/context/test-evidence/command.md b/context/test-evidence/command.md index 4037430c..776bfe3a 100644 --- a/context/test-evidence/command.md +++ b/context/test-evidence/command.md @@ -2861,6 +2861,11 @@ 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: , 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 @@ -5636,8 +5641,9 @@ 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 clears the landed cell, -measured here; that is filed as improvise-fy9o rather than fixed here. +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` @@ -5678,3 +5684,24 @@ 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). + +### `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. diff --git a/crates/improvise-command/src/command/keymap.rs b/crates/improvise-command/src/command/keymap.rs index 300e4ef8..5c741c75 100644 --- a/crates/improvise-command/src/command/keymap.rs +++ b/crates/improvise-command/src/command/keymap.rs @@ -781,16 +781,20 @@ mod tests { for key in exits { match km.lookup(*key, none) { - Some(Binding::Sequence(steps)) => assert!( - steps.iter().any(|(n, a)| *n == "clear-buffer" - && a.as_slice() == [buffer.to_string()]), - "{mode:?}: {key:?} must clear \"{buffer}\", got {steps:?}" - ), + Some(Binding::Sequence(steps)) => { + assert!( + steps.iter().any(|(n, a)| *n == "clear-buffer" + && a.as_slice() == [buffer.to_string()]), + "{mode:?}: {key:?} must clear \"{buffer}\", got {steps:?}" + ) + } // A reopening commit owns its clear (improvise-fy9o): ask the command. Some(Binding::Cmd { name, args }) => { let dbg = clears_itself(&app_mode, name, args, &buffer); assert!( - dbg.contains(&format!("SetBuffer {{ name: \"{buffer}\", value: \"\" }}")), + dbg.contains(&format!( + "SetBuffer {{ name: \"{buffer}\", value: \"\" }}" + )), "{mode:?}: {key:?} runs {name} alone, which must clear \"{buffer}\": {dbg}" ); } diff --git a/crates/improvise-wasm-client/tests/lockstep.rs b/crates/improvise-wasm-client/tests/lockstep.rs index cb1b9a34..2b1f95a8 100644 --- a/crates/improvise-wasm-client/tests/lockstep.rs +++ b/crates/improvise-wasm-client/tests/lockstep.rs @@ -1897,10 +1897,21 @@ fn an_untouched_browser_edit_of_a_misread_value_writes_nothing() { #[test] fn enter_enter_leaves_every_cell_unchanged_on_both_ends() { let every_cell = |app: &App| -> Vec> { - [("North", "Cost"), ("North", "Revenue"), ("South", "Cost"), ("South", "Revenue")] - .iter() - .map(|(r, m)| app.model_state.workbook.model.get_cell(®ion_measure(r, m)).cloned()) - .collect() + [ + ("North", "Cost"), + ("North", "Revenue"), + ("South", "Cost"), + ("South", "Revenue"), + ] + .iter() + .map(|(r, m)| { + app.model_state + .workbook + .model + .get_cell(®ion_measure(r, m)) + .cloned() + }) + .collect() }; let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0)); client_host.cursor_mut().selected = (1, 0); @@ -1923,8 +1934,55 @@ fn enter_enter_leaves_every_cell_unchanged_on_both_ends() { "{key}: client and host disagree about the buffers" ); } - assert_eq!(every_cell(&client_host), before, "the browser's Enter, Enter wrote"); - assert_eq!(every_cell(&key_host), before, "the host's Enter, Enter wrote"); + assert_eq!( + every_cell(&client_host), + before, + "the browser's Enter, Enter wrote" + ); + assert_eq!( + every_cell(&key_host), + before, + "the host's Enter, Enter wrote" + ); +} + +/// A commit that reopens nothing still leaves no text behind, on either end: +/// the clear moved from the keymap into the commit (improvise-fy9o). +#[test] +fn a_commit_at_the_corner_leaves_no_buffer_behind_on_both_ends() { + let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0)); + let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0)); + assert_eq!( + key_host.cursor().selected, + (1, 1), + "the fixture is not at the corner" + ); + let mut client = client_ready_to_edit(&client_host, (2, 2)); + for key in ["i", "9", "Enter"] { + let event = BrowserKey::new(key).to_key_event().unwrap(); + let KeyOutcome::Local(cmds) = resolve(&client, event) else { + panic!("{key}: the client must answer this key locally"); + }; + for cmd in &cmds { + client.fold(cmd); + } + apply_on(&mut client_host, &cmds); + reduce_key(&mut key_host, event); + } + assert!( + !key_host.view_state.mode.is_editing(), + "the corner must not reopen" + ); + for (end, buffers) in [ + ("browser", &client.view.buffers), + ("host", &key_host.view_state.buffers), + ] { + assert_eq!( + buffers.get("edit").map(String::as_str), + Some(""), + "{end}: the committed text was left in the buffer" + ); + } } /// BUG (improvise-fy9o) — see `context/test-evidence/clients.md`.