diff --git a/context/code/protocol.md b/context/code/protocol.md index 3b4b5fce..67ad3aea 100644 --- a/context/code/protocol.md +++ b/context/code/protocol.md @@ -629,6 +629,55 @@ typed stored the rounded number in both front ends. Every arm that seeds an editor now goes through `CellValue::edit_text` (`core.md` ยง17). That is the shortest decimal that parses back to the same `f64`. +### An untouched editor writes nothing (improvise-ixxx) + +An exact seed is not enough, because the commit reclassifies it. `typed()` +reads text, not the cell it came from, so `i` then `Enter` on `Text("007")` +stored `Number(7)`. A raw `CellValue::Error` has the edit text `ERR:msg`, and +the same keys stored it as `Text("ERR:msg")`. After umi4 the seed differs from +the stored value only when `typed(edit_text(v)) != v`. In every such case the +right answer is to keep `v`, which is what an untouched commit means. + +So **a commit whose text is the seed the editor captured as it opened writes +nothing**. `resolve::untouched(Option<&OpenedEditor>, &str)` is the one home +for that, beside `typed()`. Every commit path reaches it: + +- **TUI, pivot and records.** `CommitAndAdvance` carries `opened`, which + `commit_at_cursor` takes from `CmdContext::opened_editor`. It skips only + `commit_cell_value`: the mode change, the cursor restore, the advance and the + re-entry all still run, so records Tab still advances. The script form + (`commit-cell-edit `) carries `None` and always + writes, since no editor was opened. The drill's staging goes through the same + `commit_cell_value`, so it is covered too. +- **Browser keyboard.** `resolve::commit` โ†’ `written`, which returns an empty + write for an untouched buffer and leaves `ChangeMode` and `advanced` alone. +- **Browser mouse.** `web/app.js`'s `edit()` already held `original`, captured + when the editor opened. It now passes it to `Client::typed_cell(row, col, + text, seed)`. The decision is `typed_cell_frame`'s, not the page's: the page + hands the seed over and the wasm client decides. + +**The seed is captured at open, never recomputed at commit.** Where the editor +opened is where `OpenedEditor` is written. For the TUI that is `EditOrDrill` +and `EnterEditAtCursor`. For the browser it is `resolve::opened_on`, which +`edit_or_drill` and `advanced` share, and `ClientState::settle_editor` for an +editor that arrived without its buffer. The reason is concurrency. If another +client writes the cell while this editor is open, a seed recomputed at commit +has moved to the new value. The untouched buffer then differs from it, so the +stale text gets written over the other client's value. The browser's basis +claim would not stop that write, because it is taken when the commit is +composed. Comparing against the captured seed avoids that lost update as well. + +`OpenedEditor` lives on `ViewState` (`opened_editor`) and travels as +`Command::SetOpenedEditor`, with the `ViewOnly` effect of the same name. It is +a struct with one field on purpose. What else an open editor must capture, +namely which record it edits (improvise-2wu3) and at what version +(improvise-0j0f), belongs in the same object. `EnterMode` into an editing mode +writes `None`, because an editor opened that way has no seed and must not +inherit an earlier one. + +This does **not** change classification. `007` typed fresh is still +`Number(7)`, and `a_touched_browser_commit_still_writes` pins that. + `numeric` and `error` are decided by the server for the same reason: it is the side that knows which it was, and re-deriving client-side is how the two ends drift apart. `CellDisplay` is deliberately not *drawn from* a `CellValue` diff --git a/crates/improvise-wasm-client/src/socket.rs b/crates/improvise-wasm-client/src/socket.rs index c9234fcb..464a2787 100644 --- a/crates/improvise-wasm-client/src/socket.rs +++ b/crates/improvise-wasm-client/src/socket.rs @@ -277,6 +277,30 @@ mod tests { ); } + /// BUG (improvise-ixxx) โ€” see `context/test-evidence/clients.md`. + #[test] + fn a_mouse_commit_is_judged_against_the_seed_it_opened_with() { + let mut client = client_showing(Some(CellId::Record(record("Revenue")))); + let opened = OpenedEditor { + seed: "1200".into(), + }; + let mut moved = CellDisplay::number("5"); + moved.id = Some(CellId::Record(record("Revenue"))); + client.fold_frame(ServerFrame::Commands(vec![Command::Project( + Projection::Grid(GridUpdate::Cells(vec![((0, 0), moved)])), + )])); + assert_eq!( + client.state().grid.cell(0, 0).and_then(|c| c.edit_text.clone()), + Some("5".into()), + "the other client's write did not reach this cache, so this proves nothing" + ); + assert_eq!( + client.typed_cell_frame(0, 0, "1200", &opened), + None, + "an untouched editor wrote its stale seed over the other client's value" + ); + } + /// The basis a server would publish for the one record this fixture's /// grid names. fn published_basis() -> improvise_protocol::Basis { diff --git a/harness/tests/contract.spec.js b/harness/tests/contract.spec.js index 718a4121..906a0455 100644 --- a/harness/tests/contract.spec.js +++ b/harness/tests/contract.spec.js @@ -287,6 +287,39 @@ test.describe("a pointer event reaches the client and its answer is painted", () await expect(page.locator("#grid tbody td.selected")).toHaveCount(1); }); + // improvise-ixxx. The editor hands wasm the seed it captured at open, and + // only wasm decides an untouched commit writes nothing โ€” JS must pass it. + test("an untouched mouse editor sends no write, and a touched one does", async ({ + page, + }) => { + await page.addInitScript(() => { + window.__posted = []; + const post = Worker.prototype.postMessage; + Worker.prototype.postMessage = function (message, ...rest) { + window.__posted.push(String(message)); + return post.call(this, message, ...rest); + }; + }); + await openApp(page); + const cells = page.locator("#grid tbody td"); + const writes = () => + page.evaluate(() => window.__posted.filter((m) => m.includes("SetCell")).length); + + await cells.nth(4).click(); + await expect(cells.nth(4).locator("input")).toHaveCount(1); + await page.keyboard.press("Enter"); + await expect(page.locator("#grid input")).toHaveCount(0); + await page.waitForTimeout(300); + expect(await writes()).toBe(0); + + // The control: without it a page that never sends anything passes above. + await cells.nth(4).click(); + await expect(cells.nth(4).locator("input")).toHaveCount(1); + await page.keyboard.type("1"); + await page.keyboard.press("Enter"); + await expect.poll(writes).toBe(1); + }); + // improvise-b8ot moved these five surfaces onto one handler per stable root. // The three tests below are the properties a naive delegation loses, each of // which is silent: the page still paints, and only the pointer stops working. diff --git a/web/app.js b/web/app.js index b3c0149c..617725c0 100644 --- a/web/app.js +++ b/web/app.js @@ -563,9 +563,10 @@ function edit(td, row, col, seed) { closed = true; editing = null; if (commit) { - const frame = client.typed_cell(row, col, input.value); - // null only if the grid moved under the editor; the repaint that - // follows restores whatever the server now says is there. + // `original` was captured at open: an untouched editor writes nothing. + const frame = client.typed_cell(row, col, input.value, original); + // null when untouched or the grid moved under the editor; the repaint + // that follows restores whatever the server now says is there. if (frame) transport.send(frame); } td.replaceChildren(drawn);