Tab at the rightmost column reopens on the value just written; pin the +cat clear (improvise-3tt4, improvise-5wai)

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

context/test-evidence/clients.md

@@ -5035,3 +5035,40 @@ 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`.
+
+### `tab_at_the_rightmost_column_reopens_on_the_value_just_written`
+
+**improvise-3tt4**, found by the #227 review at `a1b23c05`. `resolve::advanced`
+re-seeded the reopened editor from the client's cache, on the argument that the
+advance always lands on a *different* cell. That is false for Tab at the
+rightmost column: `Advance::Right` clamps back onto the cell the frame has just
+written, and that cell's cache entry is the pre-write value.
+
+The test uses the lockstep 2×2 fixture and starts at (0, 1), which is North /
+Revenue and holds 2. It presses `i 9 Tab 5 Enter` on a browser client and on a
+key-driven host, and compares the buffers after every key. It failed at
+`a1b23c05` on `Tab`: the browser buffer was `"2"` and the host's was `"29"`.
+Pressing `5 Enter` would then have written 25 over the 29 the user had just
+committed. Both ends must end at 295.
+
+**Fix:** `landed_seed` seeds the landing cell from the committed value's edit
+text when the advance lands on the cursor's own cell and the commit wrote. In
+every other case it uses the cache. No other advance lands on the written cell:
+`Down` moves to the next row or the next column, or reopens nothing at the
+corner.
+
+### `tab_at_the_rightmost_column_reopens_on_the_stored_spelling`
+
+This pins what "the committed value" means in the fix above: the stored value's
+edit text, as `typed()` would store it, not the raw typed string. `i Backspace 0
+0 7 Tab` at (0, 1) must leave `"7"` in both buffers. **Rival:** seeding from the
+raw text fails with `"007"` against the host's `"7"`.
+
+### `a_category_committed_at_plus_cat_leaves_no_buffer_behind_on_both_ends`
+
+**improvise-5wai**, found by the #227 review. `+cat` reopens nothing, so the
+commit's own head clear (`resolve::cleared` in `create_column`) is the only
+thing that empties the buffer. The test types `Season` at the `+cat` column of
+the records pair, presses Enter on both ends, and requires the category to have
+landed and both buffers to be `""`. **Rival:** `let _ = cleared(&edit_mode)` in
+`create_column` fails only this test, with the browser keeping `"Season"`.

crates/improvise-protocol/src/resolve.rs

@@ -898,7 +898,8 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<V
     let mut commands = written(state, id, &text)?;
     commands.push(Command::ChangeMode(AppMode::Normal));
     commands.extend(cleared(&edit_mode));
-    commands.extend(advanced(state, edit_mode, advance)?);
+    let wrote = (!untouched(state.view.opened_editor.as_ref(), &text)).then_some(text.as_str());
+    commands.extend(advanced(state, edit_mode, advance, wrote)?);
     Some(commands)
 }
 
@@ -955,13 +956,18 @@ fn create_column(
     if matches!(kind, CreateKind::Category) {
         return Some(commands);
     }
-    commands.extend(advanced(state, edit_mode, advance)?);
+    commands.extend(advanced(state, edit_mode, advance, None)?);
     Some(commands)
 }
 
 /// The advance and the re-entry `CommitAndAdvance` ends in. `None` where the
 /// host would emit `AddRecordRow`, which this client cannot name (gotcha 1).
-fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<Vec<Command>> {
+fn advanced(
+    state: &ClientState,
+    edit_mode: AppMode,
+    advance: Advance,
+    wrote: Option<&str>,
+) -> Option<Vec<Command>> {
     let GridExtent { rows, cols } = state.grid.extent;
     let row_max = rows.saturating_sub(1);
     let col_max = cols.saturating_sub(1);
@@ -983,12 +989,20 @@ fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option
         return Some(Vec::new());
     };
     let mut commands = scrolled_to(state, nr, nc);
-    // `EnterEditAtCursor`: seed from where we landed, then change mode. The
-    // new cell is a different cell, so a stale cache is correct here.
-    commands.extend(opened_on(seed_at(state, nr, nc), edit_mode));
+    // `EnterEditAtCursor`: seed from where we landed, then change mode.
+    commands.extend(opened_on(landed_seed(state, (nr, nc), wrote), edit_mode));
     Some(commands)
 }
 
+/// The seed at the cell an advance lands on. The cache is stale only where the
+/// frame just wrote, which is the cursor's own cell (improvise-3tt4).
+fn landed_seed(state: &ClientState, (row, col): (usize, usize), wrote: Option<&str>) -> String {
+    match wrote.filter(|_| state.view.cursor().selected == (row, col)) {
+        Some(text) => typed(text).map(|v| v.edit_text()).unwrap_or_default(),
+        None => seed_at(state, row, col),
+    }
+}
+
 /// The surrogate naming the record under the cursor, or `None` when the
 /// cursor is not on a writable cell.
 fn record_at_cursor(state: &ClientState) -> Option<crate::RecordHash> {

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

@@ -1680,6 +1680,30 @@ fn a_locally_committed_creating_column_agrees_with_the_host() {
     }
 }
 
+/// improvise-5wai: `+cat` reopens nothing, so its commit's own clear is all
+/// that empties the buffer, on both ends. Evidence file.
+#[test]
+fn a_category_committed_at_plus_cat_leaves_no_buffer_behind_on_both_ends() {
+    let (mut app, mut twin) = records_pair();
+    let client = furnished_client(&app, (12, 12));
+    let at = creating_cell(&client, improvise_protocol::CreateKind::Category);
+    let mut client = client;
+    typing_at(&mut app, &mut twin, &mut client, at, "Season");
+    client = furnished_client(&app, (12, 12));
+    client.view = app.view_state.clone();
+    assert_eq!(read_edit(&client.view.buffers), "Season", "nothing was typed, so this proves nothing");
+
+    both_ends(&mut app, &mut twin, &mut client, "Enter");
+
+    assert!(document_text(&app).contains("Season"), "the commit did not land");
+    assert_eq!(read_edit(&app.view_state.buffers), "", "the host left the name behind");
+    assert_eq!(read_edit(&client.view.buffers), "", "the browser left the name behind");
+}
+
+fn read_edit(buffers: &std::collections::HashMap<String, String>) -> &str {
+    buffers.get("edit").map(String::as_str).unwrap_or("")
+}
+
 /// The twin of the commit test above, for the door rather than the exit
 /// (improvise-bd9o). Evidence file.
 #[test]
@@ -2228,6 +2252,65 @@ fn a_commit_at_the_corner_leaves_no_buffer_behind_on_both_ends() {
     }
 }
 
+/// BUG (improvise-3tt4) — see `context/test-evidence/clients.md`.
+#[test]
+fn tab_at_the_rightmost_column_reopens_on_the_value_just_written() {
+    let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
+    client_host.cursor_mut().selected = (0, 1);
+    let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
+    key_host.cursor_mut().selected = (0, 1);
+    let mut client = client_ready_to_edit(&client_host, (2, 2));
+    for key in ["i", "9", "Tab", "5", "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_eq!(
+            client.view.buffers, key_host.view_state.buffers,
+            "{key}: client and host disagree about the buffers"
+        );
+    }
+    for (end, app) in [("browser", &client_host), ("host", &key_host)] {
+        assert_eq!(
+            app.model_state
+                .workbook
+                .model
+                .get_cell(&region_measure("North", "Revenue")),
+            Some(&improvise_protocol::CellValue::Number(295.0)),
+            "{end}: the reopened editor did not extend the value Tab had just written"
+        );
+    }
+}
+
+/// The reopened seed is the STORED value's edit text, not the typed string:
+/// `007` is stored as 7 on both ends (improvise-3tt4). Evidence file.
+#[test]
+fn tab_at_the_rightmost_column_reopens_on_the_stored_spelling() {
+    let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
+    client_host.cursor_mut().selected = (0, 1);
+    let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
+    key_host.cursor_mut().selected = (0, 1);
+    let mut client = client_ready_to_edit(&client_host, (2, 2));
+    for key in ["i", "Backspace", "0", "0", "7", "Tab"] {
+        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_eq!(key_host.view_state.buffers.get("edit").map(String::as_str), Some("7"));
+    assert_eq!(client.view.buffers, key_host.view_state.buffers);
+}
+
 /// BUG (improvise-fy9o) — see `context/test-evidence/clients.md`.
 #[test]
 fn typing_into_the_reopened_editor_extends_the_cell_on_both_ends() {