Merge pull request #256 from fiddlerwoaroof/fix/rq3f-resolver-gaps

Pin the browser's Tab-commit advance and the tile cursor's upper clamp (improvise-rq3f)

context/test-evidence/clients.md

@@ -5117,3 +5117,36 @@ Rival injected: `claim_as_read` ignoring the captured generation and claiming
 the current one. That is the pre-fix behaviour, restated after the fix. Only
 this test failed in the whole workspace. The layout tier staying current is
 still pinned by `an_edit_writes_the_cell_the_user_clicked_after_the_rows_renumber`.
+
+### tab_in_an_editor_commits_and_lands_one_column_right
+
+improvise-rq3f, found in the adversarial review of PR #246. Changing `advanced`'s
+`Advance::Right` arm to `Some((row, col))` passed the whole workspace (1945
+tests): nothing asserted where the browser's Tab-commit
+(`commit-and-advance-right`) lands. The test opens the editor with `i` on a
+centred cursor, (1, 1) in a three-by-three grid, so no edge clamps the move.
+It then presses Tab and asserts three things: the composed commands contain
+`SetSelected { row: 1, col: 2 }`, the folded cursor is at (1, 2), and the
+editor is open again there.
+
+- **Rival `right_stays`** (the ticket's fault): fails only this test, across
+  the workspace.
+- A unit test and not a lockstep case, because the fault is in the resolver's
+  own arithmetic. Lockstep compares the client with the host; this test
+  compares the client with the answer `CommitAndAdvance` gives, whose
+  `AdvanceDir::Right` arm is `(col + 1).min(col_max)`.
+
+### the_tile_cursor_stops_on_the_last_tile
+
+improvise-rq3f. Changing `tile_cursor`'s upper clamp from `(count - 1)` to
+`count` passed the whole workspace: pressing right on the last tile selected an
+index past the end, and nothing noticed. The fixture is `furnished(TileSelect)`,
+with three tiles and the cursor on the middle one. The control moves right
+once, unclamped, to the last tile. Then both `Right` and `l` must clamp there,
+and each must still emit `SetTileCatIdx(last)`, as `MoveTileCursor` emits even
+when the index does not move (gotcha 47).
+
+- **Rival `clamp_count`** (the ticket's fault): fails only this test, with
+  `SetTileCatIdx(3)` over three tiles.
+- The control matters: without the unclamped move first, a resolver that never
+  moves at all would also pass the two clamped presses.

crates/improvise-protocol/src/resolve.rs

@@ -3454,6 +3454,51 @@ mod tests {
         commands
     }
 
+    /// Tab in an editor lands one column right, off a centred cursor that no
+    /// edge clamps (improvise-rq3f). Evidence: `context/test-evidence/clients.md`.
+    #[test]
+    fn tab_in_an_editor_commits_and_lands_one_column_right() {
+        let mut client = centred_on(record_cell());
+        press_folded(&mut client, KeyCode::Char('i'));
+        assert!(client.view.mode.is_editing(), "setup: the editor opened");
+        let commands = press_folded(&mut client, KeyCode::Tab);
+        assert!(
+            commands.contains(&Command::SetSelected { row: 1, col: 2 }),
+            "Tab must advance right from (1, 1): {commands:?}"
+        );
+        assert_eq!(client.view.cursor().selected, (1, 2));
+        assert!(
+            client.view.mode.is_editing(),
+            "and re-enter the editor there"
+        );
+    }
+
+    /// Right on the last tile stays on it, and still emits, as `MoveTileCursor`
+    /// does (improvise-rq3f). Evidence: `context/test-evidence/clients.md`.
+    #[test]
+    fn the_tile_cursor_stops_on_the_last_tile() {
+        let mut client = furnished(AppMode::TileSelect);
+        let last = client.tiles.tiles.len() - 1;
+        assert_eq!(
+            client.view.tile_cat_idx,
+            last - 1,
+            "setup: one short of the end"
+        );
+        assert_eq!(
+            press_folded(&mut client, KeyCode::Right),
+            vec![Command::SetTileCatIdx(last)],
+            "the control: a move that is not clamped"
+        );
+        for code in [KeyCode::Right, KeyCode::Char('l')] {
+            assert_eq!(
+                press_folded(&mut client, code),
+                vec![Command::SetTileCatIdx(last)],
+                "{code:?} past the last tile must clamp to it"
+            );
+        }
+        assert_eq!(client.view.tile_cat_idx, last);
+    }
+
     /// BUG (improvise-ixxx) — see `context/test-evidence/clients.md`.
     #[test]
     fn an_untouched_browser_commit_writes_nothing_and_still_advances() {