A reopened cell editor shows the cell it landed on (improvise-fy9o)

The cell-editing commits own their buffer clear at their head instead of
the keymap welding one after them, so the re-seed survives and Enter,
Enter writes nothing.

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

crates/improvise-command/src/command/cmd/commit.rs

@@ -5,7 +5,7 @@ use crate::ui::effect::{self, Effect};
 use crate::view::RecordCol;
 use improvise_protocol::{OpenedEditor, untouched};
 
-use super::core::{Cmd, CmdContext, read_buffer};
+use super::core::{Cmd, CmdContext, read_buffer, write_buffer};
 use super::grid::AddRecordRow;
 use super::navigation::{CursorState, EnterAdvance, viewport_effects};
 
@@ -805,6 +805,72 @@ mod tests {
             "the reopened editor did not capture the cell it landed on"
         );
     }
+
+    /// Every cell of the 2×2 fixture, by its key.
+    fn every_cell(app: &crate::ui::app::App) -> Vec<Option<CellValue>> {
+        use crate::model::cell::CellKey;
+        let mut cells = Vec::new();
+        for region in ["North", "West"] {
+            for measure in ["Cost", "Revenue"] {
+                let key = CellKey::new(vec![
+                    ("Region".into(), region.into()),
+                    ("_Measure".into(), measure.into()),
+                ]);
+                cells.push(app.model_state.workbook.model.get_cell(&key).cloned());
+            }
+        }
+        cells
+    }
+
+    /// BUG (improvise-fy9o) — see `context/test-evidence/command.md`.
+    #[test]
+    fn enter_enter_leaves_every_cell_unchanged() {
+        use improvise_protocol::key::KeyCode;
+        for (records, start) in [(false, (1, 0)), (true, (1, 2))] {
+            let mut app = grid_holding(CellValue::Number(4.0), records);
+            if records {
+                app.view_state.mode = AppMode::RecordsNormal;
+            }
+            app.cursor_mut().selected = start;
+            let before = every_cell(&app);
+            press(&mut app, KeyCode::Char('i'));
+            press(&mut app, KeyCode::Enter);
+            let reopened = app.view_state.buffers.get("edit").cloned();
+            press(&mut app, KeyCode::Enter);
+            assert_eq!(
+                every_cell(&app),
+                before,
+                "records={records}: i, Enter, Enter changed a cell (the reopened editor held {reopened:?})"
+            );
+        }
+    }
+
+    /// The reopened editor shows the cell it landed on, so typing extends it.
+    /// BUG (improvise-fy9o) — see `context/test-evidence/command.md`.
+    #[test]
+    fn typing_after_the_reopen_still_writes() {
+        use crate::model::cell::CellKey;
+        use improvise_protocol::key::KeyCode;
+        let mut app = grid_holding(CellValue::Number(4.0), false);
+        app.cursor_mut().selected = (1, 0);
+        press(&mut app, KeyCode::Char('i'));
+        press(&mut app, KeyCode::Enter);
+        assert_eq!(
+            app.view_state.buffers.get("edit").map(String::as_str),
+            Some("2"),
+            "the editor reopened on (North, Revenue) is not seeded with its value"
+        );
+        press(&mut app, KeyCode::Char('5'));
+        press(&mut app, KeyCode::Enter);
+        let landed = CellKey::new(vec![
+            ("Region".into(), "North".into()),
+            ("_Measure".into(), "Revenue".into()),
+        ]);
+        assert_eq!(
+            app.model_state.workbook.model.get_cell(&landed),
+            Some(&CellValue::Number(25.0))
+        );
+    }
 }
 
 // ── Commit commands (mode-specific buffer consumers) ────────────────────────
@@ -951,6 +1017,11 @@ impl Cmd for CommitAndAdvance {
         // Drop to normal **before** the commit, which can abort: tidy-up goes where
         // an abort cannot reach it (improvise-2zq, gotcha 21).
         effects.push(effect::change_mode(exit_mode_for(&self.edit_mode)));
+        // This command owns its buffer's clear, at the head, so the re-entry
+        // below re-seeds after it rather than being blanked (improvise-fy9o).
+        if let Some(mb) = self.edit_mode.minibuffer() {
+            effects.push(write_buffer(&mb.buffer_key, String::new()));
+        }
         // An untouched editor writes nothing; everything around the write still runs.
         let writes = !untouched(self.opened.as_ref(), &self.value);
         if writes {

crates/improvise-command/src/command/keymap.rs

@@ -690,6 +690,22 @@ mod tests {
     }
 
     /// The buffer `append-char` is bound to in `mode`'s keymap.
+    /// The effects `name` builds in `mode` with `buffer` holding text, as Debug.
+    fn clears_itself(mode: &AppMode, name: &str, args: &[String], buffer: &str) -> String {
+        use crate::command::cmd::test_helpers::*;
+        let wb = two_cat_model();
+        let layout = make_layout(&wb);
+        let reg = make_registry();
+        let bufs: HashMap<String, String> = [(buffer.to_string(), "typed".to_string())].into();
+        let mut ctx = make_ctx(&wb, &layout, &reg);
+        ctx.mode = mode;
+        ctx.buffers = &bufs;
+        let cmd = reg
+            .interactive(name, args, &ctx)
+            .unwrap_or_else(|e| panic!("{name} {args:?} does not build: {e}"));
+        effects_debug(&cmd.execute(&ctx))
+    }
+
     fn append_target(ks: &KeymapSet, mode: &AppMode) -> String {
         let key = ModeKey::from_app_mode(mode, false).expect("a text-entry mode key");
         let km = ks.maps().get(&key).expect("every mode has a keymap");
@@ -764,15 +780,22 @@ mod tests {
             let km = ks.maps().get(&mode).unwrap();
 
             for key in exits {
-                let Some(Binding::Sequence(steps)) = km.lookup(*key, none) else {
-                    panic!("{mode:?}: {key:?} should be a commit/exit sequence");
-                };
-                assert!(
-                    steps
-                        .iter()
-                        .any(|(n, a)| *n == "clear-buffer" && a.as_slice() == [buffer.to_string()]),
-                    "{mode:?}: {key:?} must clear \"{buffer}\", got {steps:?}"
-                );
+                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:?}"
+                    ),
+                    // 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: \"\" }}")),
+                            "{mode:?}: {key:?} runs {name} alone, which must clear \"{buffer}\": {dbg}"
+                        );
+                    }
+                    other => panic!("{mode:?}: {key:?} should commit or exit, got {other:?}"),
+                }
             }
         }
     }

crates/improvise-protocol/src/keymap.rs

@@ -347,6 +347,13 @@ impl TextEntry {
         self
     }
 
+    /// Bind `key` to a commit that clears the buffer **itself**, first, because
+    /// it reopens an editor a trailing clear would blank (improvise-fy9o).
+    pub fn commit_reopening(mut self, key: KeyCode, cmd: &'static str, args: Vec<String>) -> Self {
+        self.km.bind_args(key, KeyModifiers::NONE, cmd, args);
+        self
+    }
+
     /// Replace a spine binding — used only by CommandMode, whose Backspace
     /// exits the mode on an empty buffer instead of popping a character.
     pub fn rebind(mut self, key: KeyCode, cmd: &'static str) -> Self {
@@ -851,8 +858,8 @@ impl Keymaps {
         // edit-mode as an argument, so the command stays mode-agnostic. ──
         let ed = Arc::new(
             TextEntry::new(&AppMode::editing(), "normal", global.clone())
-                .commit(KeyCode::Enter, "commit-cell-edit", vec!["editing".into()])
-                .commit(
+                .commit_reopening(KeyCode::Enter, "commit-cell-edit", vec!["editing".into()])
+                .commit_reopening(
                     KeyCode::Tab,
                     "commit-and-advance-right",
                     vec!["editing".into()],
@@ -867,12 +874,12 @@ impl Keymaps {
             ModeKey::RecordsEditing,
             Arc::new(
                 TextEntry::new(&AppMode::records_editing(), "records-normal", ed)
-                    .commit(
+                    .commit_reopening(
                         KeyCode::Enter,
                         "commit-cell-edit",
                         vec!["records-editing".into()],
                     )
-                    .commit(
+                    .commit_reopening(
                         KeyCode::Tab,
                         "commit-and-advance-right",
                         vec!["records-editing".into()],

crates/improvise-protocol/src/resolve.rs

@@ -896,10 +896,19 @@ 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)?);
     Some(commands)
 }
 
+/// The commit's own clear of `edit_mode`'s buffer, at its head: the keymap
+/// welds none to these keys, so a re-seed survives (improvise-fy9o).
+fn cleared(edit_mode: &AppMode) -> Option<Command> {
+    edit_mode
+        .minibuffer()
+        .map(|mb| write_buffer(&mb.buffer_key, String::new()))
+}
+
 /// The write a commit makes: none when the buffer is the seed the editor
 /// captured as it opened, and only the write is skipped (improvise-ixxx).
 fn written(state: &ClientState, id: &CellId, text: &str) -> Option<Vec<Command>> {
@@ -924,6 +933,7 @@ fn create_column(
     // `exit_mode_for`: the tidy-up goes FIRST, where an aborting
     // `AddCategory` cannot reach it (gotcha 21).
     let mut commands = vec![Command::ChangeMode(AppMode::Normal)];
+    commands.extend(cleared(&edit_mode));
     let name = text.trim();
     if !name.is_empty() {
         commands.push(model(
@@ -3355,18 +3365,28 @@ mod tests {
             }
         );
         assert_eq!(commands[1], Command::ChangeMode(AppMode::Normal));
-        assert_eq!(commands[2], Command::SetSelected { row: 1, col: 0 });
-        assert!(
-            commands.contains(&Command::ChangeMode(AppMode::editing())),
-            "the advance re-enters editing on the cell it landed on: {commands:?}"
-        );
+        // The commit owns its clear, at the head (improvise-fy9o).
         assert_eq!(
-            commands.last(),
-            Some(&Command::SetBuffer {
+            commands[2],
+            Command::SetBuffer {
                 name: "edit".into(),
                 value: String::new(),
-            }),
-            "the keymap welds `clear-buffer` to every commit: {commands:?}"
+            }
+        );
+        assert_eq!(commands[3], Command::SetSelected { row: 1, col: 0 });
+        assert_eq!(
+            commands.last(),
+            Some(&Command::ChangeMode(AppMode::editing())),
+            "the advance re-enters editing on the cell it landed on: {commands:?}"
+        );
+        let last_write = commands.iter().rev().find_map(|c| match c {
+            Command::SetBuffer { name, value } if name == "edit" => Some(value.as_str()),
+            _ => None,
+        });
+        assert_eq!(
+            last_write,
+            Some("900"),
+            "the reopened editor must show the cell it landed on, not blank: {commands:?}"
         );
     }
 

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

@@ -1893,6 +1893,71 @@ fn an_untouched_browser_edit_of_a_misread_value_writes_nothing() {
     }
 }
 
+/// BUG (improvise-fy9o) — see `context/test-evidence/clients.md`.
+#[test]
+fn enter_enter_leaves_every_cell_unchanged_on_both_ends() {
+    let every_cell = |app: &App| -> Vec<Option<improvise_protocol::CellValue>> {
+        [("North", "Cost"), ("North", "Revenue"), ("South", "Cost"), ("South", "Revenue")]
+            .iter()
+            .map(|(r, m)| app.model_state.workbook.model.get_cell(&region_measure(r, m)).cloned())
+            .collect()
+    };
+    let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
+    client_host.cursor_mut().selected = (1, 0);
+    let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
+    key_host.cursor_mut().selected = (1, 0);
+    let before = every_cell(&client_host);
+    let mut client = client_ready_to_edit(&client_host, (2, 2));
+    for key in ["i", "Enter", "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"
+        );
+    }
+    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");
+}
+
+/// BUG (improvise-fy9o) — see `context/test-evidence/clients.md`.
+#[test]
+fn typing_into_the_reopened_editor_extends_the_cell_on_both_ends() {
+    let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
+    client_host.cursor_mut().selected = (1, 0);
+    let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
+    key_host.cursor_mut().selected = (1, 0);
+    let mut client = client_ready_to_edit(&client_host, (2, 2));
+    for key in ["i", "Enter", "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);
+    }
+    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(25.0)),
+            "{end}: the digit typed into the reopened editor did not extend its seed"
+        );
+    }
+}
+
 /// `i` then `Enter` on (South, Revenue) holding `value`, through the resolver
 /// and a real host: the seed is the stored value's, and the cell is unchanged.
 fn untouched_browser_edit(value: improvise_protocol::CellValue) {