Merge pull request #272 from fiddlerwoaroof/fix/p6v6-editor-opener-awaits

improvise-p6v6: every cell-editor opener awaits its seed while a write is unanswered (stacked on #227)

context/code/protocol.md

@@ -2045,6 +2045,20 @@ stated once, over the write, never over a list of cases:
   in-flight write, not the newest frame: a frame that writes nothing stays in
   the queue behind an unanswered write, so the newest frame can be one of
   those.
+- **Every opener asks the in-flight half, because it lives in `opened_on`**
+  (improvise-p6v6). mdfc first put the whole test inside `advanced`, which left
+  `i`/`a` (`edit_or_drill`) seeding from the cache with no in-flight check.
+  `i 9 Tab Esc i 5 Enter`, typed faster than the answer, stored 15 in the
+  browser where the TUI stores 195. The two halves now live in different
+  places, for a reason. "A write is still in flight" is true of the *client*,
+  so `opened_on` asks it, and no opener can forget. "This frame writes" is
+  true only of the commit being resolved, which is not in `in_flight` yet, so
+  `advanced` still asks it itself. Folding both into `opened_on` would lose the
+  second: the 3tt4 and 3uzg tests fail when `advanced` defers entirely.
+  The click-edit path (`socket.rs::edit_target_at`, improvise-ugj4) also seeds
+  from the cache, but it claims the record at the version it *read*. Behind an
+  unanswered write the host therefore refuses its commit as `RecordMoved`: a
+  loud failure, not a silent overwrite.
 - **The seed settles only into an open editor** (improvise-4cb5). An Esc typed
   before the answer arrives leaves the awaiting `opened_editor` in place. The
   late seed would then fill the edit buffer in `Normal` mode, where the TUI's is

context/test-evidence/clients.md

@@ -5278,3 +5278,29 @@ JSON.
 **Rival:** `#[serde(default)]` restored. The frame then parses, with
 `awaiting: false`, and the test fails at `expect_err`. Before this test existed,
 deleting the attribute left the whole workspace green, so nothing pinned it.
+
+### `an_editor_opened_with_i_before_the_last_write_is_answered_opens_on_that_write`
+
+**improvise-p6v6**, found by the #227 review at `de4a7d78`. It predates #227:
+the reviewer reproduced it against gh/main `202e7eac`. Fixture: `precise_host`,
+cursor (0,1) on North/Revenue = 2 (the rightmost column), window 2×2.
+
+1. `i 9` are answered one at a time.
+2. `Tab` (writes 29), `Escape` and `i` go out through `sent`, and none is
+   answered until all three are out.
+3. The browser's buffers must then equal the TUI host's.
+4. `5 Enter` must store 295 on both ends.
+
+Red at `de4a7d78`: the browser buffer was `"2"` where the TUI's was `"29"`. `i`
+reaches `edit_or_drill`, which called `opened_on(cell.edit_text)` with no
+in-flight check. mdfc had put that check inside `advanced` alone.
+
+**Rivals:**
+- **The fix reverted** (the code at `de4a7d78`): fails this test only.
+- **`advanced` defers wholly to `opened_on`** (its own `writes` half dropped,
+  on the reasoning that `opened_on` now checks): fails
+  `a_commit_that_prunes_its_row_reopens_on_the_row_that_moved_up`, both
+  `tab_at_the_rightmost_column_*` tests, and
+  `resolve::tests::enter_commits_the_cell_advances_and_re_enters_editing`. The
+  commit being resolved is not in `in_flight` yet, so `opened_on` cannot see its
+  write.

crates/improvise-protocol/src/resolve.rs

@@ -781,7 +781,11 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
     match cell.id.as_ref()? {
         // `edit_text`, never `text`: the drawn text has been through
         // `format_f64`. `tests/lockstep.rs` found exactly that.
-        CellId::Record(_) => Some(opened_on(cell.edit_text.clone().unwrap_or_default(), mode)),
+        CellId::Record(_) => Some(opened_on(
+            state,
+            cell.edit_text.clone().unwrap_or_default(),
+            mode,
+        )),
         CellId::Aggregate(_) => Some(vec![Command::SetStatus(format!(
             "an aggregate is edited by drilling in — {}",
             not_yet("drill")
@@ -796,9 +800,18 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
     }
 }
 
-/// Open the cell editor on `seed`: the buffer, what it captured, then the mode.
-/// `EditOrDrill`'s and `EnterEditAtCursor`'s shared shape (improvise-ixxx).
-fn opened_on(seed: String, mode: AppMode) -> Vec<Command> {
+/// Open the cell editor on `seed`, or await it while any write is unanswered: the
+/// cache predates that write (improvise-mdfc/p6v6, `protocol.md` §3).
+fn opened_on(state: &ClientState, seed: String, mode: AppMode) -> Vec<Command> {
+    match state.in_flight.iter().any(|sent| sent.awaited) {
+        true => awaiting_seed(mode),
+        false => seeded(seed, mode),
+    }
+}
+
+/// The buffer, what the editor captured, then the mode: `EditOrDrill`'s and
+/// `EnterEditAtCursor`'s shared shape (improvise-ixxx).
+fn seeded(seed: String, mode: AppMode) -> Vec<Command> {
     vec![
         Command::SetBuffer {
             name: "edit".to_string(),
@@ -1009,12 +1022,11 @@ fn advanced(
         return Some(Vec::new());
     };
     let mut commands = scrolled_to(state, nr, nc);
-    // After a write — this frame's or an unanswered earlier one's — the cache may
-    // be stale, so the seed waits for the host (improvise-3uzg/mdfc, `protocol.md` §3).
-    let stale = writes || state.in_flight.iter().any(|sent| sent.awaited);
-    commands.extend(match stale {
+    // This frame's own write makes the cache stale too; `opened_on` answers for
+    // earlier ones (improvise-3uzg/mdfc/p6v6, `protocol.md` §3).
+    commands.extend(match writes {
         true => awaiting_seed(edit_mode),
-        false => opened_on(seed_at(state, nr, nc), edit_mode),
+        false => opened_on(state, seed_at(state, nr, nc), edit_mode),
     });
     Some(commands)
 }

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

@@ -2637,6 +2637,55 @@ fn a_tab_sent_before_the_last_write_is_answered_reopens_on_that_write() {
     }
 }
 
+/// BUG (improvise-p6v6) — see `context/test-evidence/clients.md`.
+#[test]
+fn an_editor_opened_with_i_before_the_last_write_is_answered_opens_on_that_write() {
+    let window = (2, 2);
+    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, window);
+    for key in ["i", "9"] {
+        let event = BrowserKey::new(key).to_key_event().unwrap();
+        sent_and_answered(&mut client, &mut client_host, event, window);
+        reduce_key(&mut key_host, event);
+    }
+    // Tab writes 29; Esc and i reopen before its answer comes back.
+    let answers: Vec<Unanswered> = ["Tab", "Escape", "i"]
+        .into_iter()
+        .map(|key| {
+            let event = BrowserKey::new(key).to_key_event().unwrap();
+            reduce_key(&mut key_host, event);
+            sent(&mut client, &mut client_host, event, window)
+        })
+        .collect();
+    let awaited: Vec<bool> = answers.iter().map(|a| a.awaited).collect();
+    assert_eq!(awaited, [true, false, false], "setup: only the Tab writes");
+    answers
+        .into_iter()
+        .for_each(|answer| answered(&mut client, answer));
+    assert_eq!(
+        client.view.buffers, key_host.view_state.buffers,
+        "i opened on the cache the unanswered write had made stale"
+    );
+    for key in ["5", "Enter"] {
+        let event = BrowserKey::new(key).to_key_event().unwrap();
+        sent_and_answered(&mut client, &mut client_host, event, window);
+        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(295.0)),
+            "{end}: the browser overwrote the value Tab had just written"
+        );
+    }
+}
+
 const FORMULAS: [&str; 3] = [
     "Profit = Revenue * 2",
     "Tax = Revenue * 0.1",