fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge #295 (65d6a3e1, improvise-7mro projected reads) into the editor integration branch

Browse Source 
Resolutions against gh/main 8d870c15:
- client.rs: kept main's settle_status (1x9h) and #295's
  settle_declined_editor (5m9z) side by side.
- resolve.rs reopened_at: took #295's declined_at shape and moved #292's
  CellId::NonItem arm into declined_at, which reopened_at consults before
  any seed choice, so a NonItem landing still re-opens nothing.
- lockstep.rs: both sides appended at EOF; kept main's tail then #295's.
  Both defined sent_to_host with different semantics (main's panics on a
  non-local key, #295's lets an upstream key reach a silent host), so
  #295's copy is renamed sent_or_upstream with its three callers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-24 12:10:53 -07:00
parent 8d870c15d0 65d6a3e163
commit 2fbb43b9ee
5 changed files with 936 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/code/clients.md
+99 -7
View File
@@ -520,17 +520,109 @@ looks wrong and is exactly what the TUI does; mirroring it is the point.
The re-entry is `reopened_at`, and it declines where the host's
`EnterEditAtCursor` declines (improvise-enz6). The host asks
`core::uneditable` of its layout. The browser asks the landing cell's projected
`CellId`, which is the same answer, because `cell_id` is what fills it. Two
limits follow from asking the cache:
`CellId`, which is the same answer, because `cell_id` is what fills it. Asking
the cache has two consequences:
- **The wording differs on a fold.** The host can tell a fold from any other
aggregate and names the fold. The browser sees only `CellId::Aggregate`, so it
gives the host's generic aggregate text. `lockstep.rs` compares the mode, the
cursor and the buffers here, never the status text.
- **A landing past the window opens anyway.** The cache holds only the
announced window, so a `Down` from the window's last row lands on a cell with
no projected id. The browser then opens the editor, where the host does not.
No write can follow it: once the host has projected that cell,
`record_at_cursor` finds no record there and the commit composes nothing.
- **A landing past the window is decided when the host answers**
(improvise-5m9z). The cache holds only the announced window, so a `Down` from
the window's last row lands on a cell with no projected id yet. `reopened_at`
opens the editor there provisionally, and the fold settles it:
`ClientState::settle_declined_editor` runs after every fold. It closes an
**untouched** editor whose cursor cell the host has now projected as one `i`
declines, and says why. The client then ends where the TUI does. Both ends
ask the same `resolve::declined_at`, so the immediate decline and the late
one cannot disagree.
The decline waits for the host's answer, as #227's awaiting seed does. It does
not choose a seed, which stays `seed_at`'s and #227/#272's.
**A touched editor is kept.** If the user typed before the answer came, the
editor stays open with their text. Closing it would throw input away. Its
commit is then refused out loud (improvise-ik6l): `refused_here` declines the
`commit-cell-edit` or `commit-and-advance-right` **step**, which takes the
whole binding with it. That includes the welded `clear-buffer`, as the TUI's
failed `committable_cell_key` does, so the status is said and the mode and
the typed text stay. Before this fix the commit composed nothing, and Enter
went `Upstream`, which no host answers: a silent dead key.
**`declined_at` reads the cache through the frame's own origin**
(`AxisFrame::first`), never the cursor's offsets (improvise-y5eb). The settle
also runs when the client folds its **own** outgoing frame. By then the
offsets have moved but the cache still holds the old window. Indexing it with
the new offsets read the wrong cell. A `Down` that wraps from the bottom row
to the next column's top read the old window's top row, and closed an editor
on Revenue/Feb with Profit's formula refusal. Read against the frame's
origin, a stale cache answers `None` ("not projected yet"), and the host's
own reply decides. The earlier version of this section called that aliasing
unreachable. It is reachable, and the wrap is the case that reaches it.
The settle is not specific to advancing. An untouched editor also closes
when the host re-projects its cell as declined for any other reason, such as
another client adding a formula over it.
**Every cache read goes through the frame's origin, not only
`declined_at`** (improvise-7mro). `cell_at_cursor` and `seed_at` read through
`projected_at` too, and the cursor-offset read `cell_at` is gone. Before
this, every other read had the y5eb aliasing. A client folds its own scroll
at once, but its cache holds the old window until the host answers, so a key
resolved in that gap read another cell. Pipelined `$ i 7 Enter` in a
one-column window wrote **Jan/Revenue** where the TUI wrote Feb/Revenue: the
seed and the write's target both came from the cell at the old window's
index. `G i` and `gg i` refused an editable cell with a formula's refusal.
The aliasing had a second form, inside the held window: after `k k j` the
cursor is on a cell the cache holds, but the offsets no longer match the
frame's origin, so index arithmetic through the offsets named the next row.
Reading at the frame's origin answers both forms with one rule.
**A cell of the grid outside the held window is `unprojected`**: unknown,
never "the cell at the same index". Inside the grid's extent but outside
either frame's range. An empty grid has nothing to wait for, so it is not
unprojected. What each key does on such a cell:
- **`i` opens an editor awaiting its seed** (`OpenedEditor::awaiting()`).
The fold settles it once the host projects that cell
(`settle_awaited_seed`), and never over typing. The settle waits for a cell
with an **id**. Between a frame and its cells, a relabel leaves a blank
`CellDisplay` at the position, and settling on that seeded `""` before the
cell arrived. A declined cell closes through `settle_declined_editor`,
which runs first.
- **A commit, `x`, `yy` and `p` wait out loud.** `refused_here` declines the
step with "waiting for the host to show this cell", which keeps the mode
and the typed text, as ik6l's refusal does. Nothing is composed: the write
target is unknown, and guessing it is the bug. Pressing the key again after
the host answers does the write. This is the Codex finding on #282.
- **An advance's reopen keeps #282's shape.** It reads `seed_at`, which is
`""` for an unprojected landing. On this base the keymap's welded
`clear-buffer` empties the reopened buffer anyway, in the TUI too, so an
awaited seed there would be written into a buffer the TUI has cleared.
**What a held reply still costs.** With every reply held, the browser cannot
know a seed the TUI shows, so `G i 7` types `7` into an empty editor, where
the TUI types `47`. The write lands on the TUI's record once the host has
answered, never on another. Matching the TUI exactly would need the client
to defer keys until the projection arrives. That means `on_frame` handing
queued frames back to `web/app.js`, which is a change to the page's surface
and is not made here.
**Merging with #227/#272 and #274.** #272 adds the same `OpenedEditor::awaiting`
field and an `awaiting_seed` for the editor an advance reopens after a write,
settled once no write is unanswered. Merged, the settle must require both:
no write unanswered, and the cell projected (`projected_seed`). With #227's
commit-owned clear, the reopen then awaits for an unprojected landing too.
#274 captures the committed cell in `settle_opened` through
`opened_at_cursor`, which reads `cell_at_cursor`. Without this fix that
capture aliases as well: a horizontal wrap with replies **delivered** writes
Jan/Revenue on #274's head. With `cell_at_cursor` reading at the origin, the
capture waits for the projected cell, because `settle_opened` retries on
every fold.
**When `CellId::NonItem` lands (#265), both `declined_at` and
`core::uneditable` need an arm for it.** Both matches are exhaustive, so the
compiler will ask. The trap is answering in the `None` arm: a non-item cell
would then open an editor whose commit writes a ghost record.
`GridMove` is a hand-spelled mirror of `navigation.rs`'s `MoveKind`, because the
command crate is not on a browser's dependency list and never will be. The
context/test-evidence/clients.md
+168 -3
View File
@@ -4890,9 +4890,104 @@ Rivals:
- The host opening everywhere: fails, the client now declining where the host
opens.
One limit is not covered, because the window is 6×6 and every landing is
inside it. A landing past the announced window has no projected id, so the
browser still opens the editor there (`clients.md`).
The window is 6×6, so every landing here is inside it. A landing past the
announced window is improvise-5m9z, covered by
`advancing_the_editor_past_the_window_settles_where_the_tui_does`.
### advancing_the_editor_past_the_window_settles_where_the_tui_does
improvise-5m9z, filed from PR #277. `reopened_at` can decline only a cell
whose id the client holds. An advance past the announced window lands on one it
does not hold yet, so the browser opened the editor where the TUI declines.
This test runs a real `Host` and a real `ClientState`. Each key is resolved on
the client, folded with `fold_outgoing`, sent through `Host::handle`, and the
whole reply is delivered. A TUI `App` presses the same keys through
`reduce_key`. Three cases, each landing one step past the window:
- `fold_host`, 2×6 window: `i` on Feb (1,1), then `Enter` onto the Q2 fold at
row 2.
- `yank_host`, 6×3 window: `i` on North/Cost (1,2), then `Tab` onto Profit at
column 3.
- The control, `fold_host` with Q2 expanded: `Enter` lands on Apr, a record,
and must stay editing on both ends. A settle that closed every editor landing
past the window cannot pass it.
The test asserts both ends land on the same cell and end in the same mode.
It was red at #277's head `aab62a36`, on the fold case: the client was
`Editing` and the TUI `Normal`.
Rivals:
- No `settle_declined_editor`: fails on the fold case.
- `declined_at` letting a formula's result through: fails, as does the enz6
lockstep test.
### a_wrapping_advance_settles_on_the_cell_it_lands_on_not_a_stale_one
improvise-y5eb, found by the adversarial review of #282 at `942f57e5`. The
reviewer's repro, made permanent.
Setup (`wrapping_host`):
- `_Measure` is on rows: [Revenue, Profit, Cost]. Profit is an explicit item
and also `= Revenue - Cost`.
- Months [Jan, Feb] are on columns.
- The window is 2×6, scrolled to rows 1–2, with the cursor on Cost/Jan (2,0).
`i` then `Enter` wraps to (0,1), Revenue/Feb, which is editable, and the TUI
edits it. The client first folds its own outgoing frame: the row offset drops
to 0, but the cache still holds rows 1–2. `settle_declined_editor` then read
the cache at `(0 - 0, 1 - 0)`, which is Profit/Feb, and closed the editor with
Profit's formula refusal, naming a cell that was not under the cursor. The
host's reply did not repair it, because the settle only ever closes.
- **Red at `942f57e5`:** the client was `Normal`, the TUI `Editing`.
- **Fix:** `declined_at` reads through `AxisFrame::first`.
- **Rival:** reading through `cell_at` (the cursor's offsets) again fails this
test and nothing else.
The same test passes at #277's head, so #282 introduced the bug. My hand-back
for #282 said the aliasing was unobservable through a one-step advance. That
was true for one step and false for the wrap.
### enter_on_an_editor_over_a_declined_cell_declines_out_loud_on_both_ends
improvise-ik6l. The editor is seated with `7` typed in and `touched: true`, on
two declined cells: the Q2 fold (`fold_host`, (2,1)) and Profit (`yank_host`,
(1,3)). Each is tried with `Enter` and with `Tab`, so four cases in all.
The TUI refuses at `committable_cell_key` and names the reason. The whole
sequence goes with it: the welded `clear-buffer` does not run, so the text stays
and the mode stays `Editing`. The browser composed nothing, and the key went
`Upstream`, which no host answers: nothing happened and nothing was said.
The test asserts four things:
- the key resolves locally;
- no write is composed;
- the mode and buffers equal the TUI's;
- both status lines are non-empty.
Red at `100627fd`: the `(2,1)` fold case went upstream.
Rivals:
- No `refused_here`: this test fails, going upstream.
- Refusing inside `commit` instead of at the step: this test fails on the
buffers (`""` against `"7"`). A step-level decline is what takes the welded
`clear-buffer` with it. That was my first attempt.
- Refusing `commit-cell-edit` only: this test fails on the `Tab` cases.
### typing_before_the_host_answers_keeps_the_editor_past_the_window
The half of improvise-5m9z that the TUI cannot see. The TUI never waits, so
nothing in it can be compared with this. On the same 2×6 `fold_host` as
above, `i`, `Enter` and `7` are all folded and sent **before** any of the
host's replies is delivered. The `7` is typed into the provisional editor while
the landing cell is still unprojected. Then the replies arrive in order. The
editor must still be open with `7` in its buffer. The fold under it is declined,
but the user's input is not thrown away.
Rival: the settle ignoring `touched`, closing any editor on a declined cell.
This test fails and the one above passes, so the two tests pin the two
directions of the `touched` guard.
### the worker is not called up until its first frame has arrived
@@ -5872,6 +5967,76 @@ in-flight check. mdfc had put that check inside `advanced` alone.
commit being resolved is not in `in_flight` yet, so `opened_on` cannot see its
write.
### keys_sent_before_a_scroll_is_answered_agree_with_the_tui
improvise-7mro, the offset-aliasing matrix from the review of #282. The fixture
is `wrapping_host`: `_Measure` rows Revenue, Profit (stored and a formula),
Cost, and Month columns Jan, Feb. Seven cases: the wrap, a horizontal wrap that
scrolls both axes, `G`, `$`, `gg`, `$ x` and `yy gg p`. Each runs three ways:
every reply delivered before the next key, every reply held to the end, and
replies held until the last two keys. Each run compares the host's document,
the cursor and the mode with the TUI. The delivered and held-then-typed runs
also compare the whole document text and the edit buffer.
With every reply held, the browser cannot know the seed, so the held run asserts
something narrower. No write is composed before its target is projected. The
last key waits out loud, and an editor is open exactly when the keys contain
`i`. After the replies land, repeating the last key writes the cell the TUI
wrote.
Red at `6942e53e` (#282), at gh/main `202e7eac` and at #274's `8d9de94e`. `$`
held wrote **Jan/Revenue**. `G` and `gg` held refused Cost with Profit's formula
refusal. The wrap and the horizontal wrap sent Enter `Upstream`, silently. At
#274's head, the horizontal wrap with replies **delivered** also wrote
Jan/Revenue, because its capture reads `cell_at_cursor`.
**Rivals**, each failing here:
- `edit_or_drill` without the `unprojected` arm fails `G` held: Normal, no
status.
- `refused_here` without the wait on a commit fails the wrap held: Enter
silent.
- `refused_here` without the `clear-cell`/`yank`/`paste` arm fails `$ x` held.
- No `settle_awaited_seed` fails `G` held-then-typed: `7` where the TUI has `47`.
- A settle that accepts a cell with no id fails the same case. It seeded the
blank a relabel leaves between the frame and its cells.
- `cell_at_cursor` through the cursor offsets fails `G` held-then-typed here,
and the test below.
### a_held_cell_is_read_at_its_position_and_not_its_cache_index
improvise-7mro, the second form of the aliasing. `four_by_two_host` has four
regions by two measures, and no two cells are equal. The client's window is
rows 2 and 3. `k k j` puts the cursor on row 2, which the cache holds, with row
offset 1, so the offsets differ from the frame's origin. Every reply is held.
Two key runs, `i 7 Enter` and `i Tab`, are compared with the TUI: the document,
the cursor, the buffer and `opened_editor`.
**Rivals:** `cell_at_cursor` through the cursor offsets fails on
`i 7 Enter`: it seeds and writes R3 where the TUI writes R2. `seed_at` through
the cursor offsets fails on `opened_editor`: the reopened editor's seed comes
from R3. The `#282` settle masks a bare `i` here: a Profit-like decline would
close it. That is why the fixture has no formula and the test goes on to
commit.
**Amended when #227 (with #272, improvise-p6v6) merged #295.** Under p6v6 the
editor that `i 7 Enter` reopens awaits the seed, because its own write is still
unanswered. With every reply held, its buffer is `""` where the TUI's is `"13"`.
So the test now reads the editor twice:
- **Before delivery**, only when the editor is not awaiting. That is the `i Tab`
run, whose reopen reads `seed_at` at once.
- **After delivering the held replies**, always.
The `seed_at` rival still fails, on `i Tab` before delivery (`"23"` against
`"22"`), and fails nothing else.
### resolve::tests::a_cursor_on_an_empty_grid_waits_for_nothing
improvise-7mro. `unprojected` asks whether the position is inside the grid's
extent, not only outside both frames. An empty grid has empty frames, so
without the extent every position is outside them. **Rival:** drop the extent
guard. `i` then opens an editor awaiting a seed that no cell will ever supply.
### tab_in_an_editor_commits_and_lands_one_column_right
improvise-rq3f, found in the adversarial review of PR #246. Changing `advanced`'s
crates/improvise-protocol/src/client.rs
+25 -3
View File
@@ -248,10 +248,11 @@ impl ClientState {
self.view.opened_editor = Some(crate::OpenedEditor::on(seed));
}
self.settle_awaited_seed();
self.settle_declined_editor();
}
/// An editor opened after a write takes its seed from the grid the host
/// answered with, never from the cache the write made stale (improvise-3uzg).
/// An editor opened after a write, or on a cell not projected yet, takes its
/// seed once both are answered, and never over typing (improvise-3uzg/7mro).
fn settle_awaited_seed(&mut self) {
let waiting = self.view.mode.is_editing()
&& self
@@ -260,7 +261,8 @@ impl ClientState {
.as_ref()
.is_some_and(|opened| opened.awaiting && !opened.touched);
let answered = !self.in_flight.iter().any(|sent| sent.awaited);
if let (true, true, Some(seed)) = (waiting, answered, crate::resolve::edit_seed(self)) {
if let (true, true, Some(seed)) = (waiting, answered, crate::resolve::projected_seed(self))
{
self.view.buffers.insert("edit".to_string(), seed.clone());
self.view.opened_editor = Some(crate::OpenedEditor::on(seed));
}
@@ -278,6 +280,22 @@ impl ClientState {
}
}
/// An untouched editor on a cell `i` declines closes once the host projects
/// that cell: an advance past the window opens before it can know (5m9z).
fn settle_declined_editor(&mut self) {
let (row, col) = self.view.cursor().selected;
let untouched = self.view.mode.is_editing()
&& self
.view
.opened_editor
.as_ref()
.is_some_and(|opened| !opened.touched);
if let (true, Some(refusal)) = (untouched, crate::resolve::declined_at(self, row, col)) {
self.view.mode = settled_mode(&crate::AppMode::Normal, self.grid.records);
self.view.status_msg = refusal;
}
}
/// The widget dispatch: a projection names the widget it is addressed to.
/// The `CategoryTreeCache`/`TileBarCache` overlap is deliberate (gotcha 32).
fn apply_projection(&mut self, projection: &Projection) {
@@ -577,6 +595,10 @@ mod tests {
fn answered_with(state: &mut ClientState, edit_text: &str) {
let mut cell = CellDisplay::number(edit_text);
cell.edit_text = Some(edit_text.into());
cell.id = Some(crate::CellId::Record(crate::RecordCell {
record: serde_json::from_str("\"fe4c8e5e9cbd578e0d388f4b2b5a7879\"").unwrap(),
measure: Some("Revenue".into()),
}));
for cmd in [
GridUpdate::RowFrame(AxisFrame::flat(["North"])),
GridUpdate::ColFrame(AxisFrame::flat(["Revenue"])),
crates/improvise-protocol/src/resolve.rs
+106 -45
View File
@@ -313,7 +313,7 @@ enum Step {
fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> Step {
match requested(name) {
Some(request) => Step::Request(request),
None => match declined(name, args) {
None => match declined(name, args).or_else(|| refused_here(state, name)) {
Some(said) => Step::Declined(vec![Command::SetStatus(said)]),
None => match composed(state, event, name, args) {
Some(commands) => Step::Local(commands),
@@ -323,6 +323,18 @@ fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> St
}
}
/// A commit onto a cell `i` declines is refused as `committable_cell_key` refuses
/// it, out loud with the binding (ik6l); a cell op on an unprojected cell waits (7mro).
fn refused_here(state: &ClientState, name: &str) -> Option<String> {
let (row, col) = state.view.cursor().selected;
let waiting = unprojected(state, row, col).then(|| NOT_PROJECTED.to_string());
match name {
"commit-cell-edit" | "commit-and-advance-right" => declined_at(state, row, col).or(waiting),
"clear-cell" | "yank" | "paste" => waiting,
_ => None,
}
}
/// The command names this client answers by **asking its host**, rather than
/// by composing anything. See `context/code/clients.md` (improvise-jz6w).
fn requested(name: &str) -> Option<Request> {
@@ -835,13 +847,11 @@ fn tile_cursor(state: &ClientState, delta: i32) -> Option<Vec<Command>> {
// ── editing a cell ──────────────────────────────────────────────────────
/// The cell under the cursor in this viewport-indexed cache. `None` when the
/// selection is outside the window — reachable only by a projection race.
/// The cell under the cursor, as the host projected it. `None` outside the
/// window the cache holds, which the cursor's own offsets may have left (7mro).
fn cell_at_cursor(state: &ClientState) -> Option<&crate::CellDisplay> {
let cursor = state.view.cursor();
let row = cursor.selected.0.checked_sub(cursor.row_offset)?;
let col = cursor.selected.1.checked_sub(cursor.col_offset)?;
state.grid.cell(row, col)
let (row, col) = state.view.cursor().selected;
projected_at(state, row, col)
}
/// `EditOrDrill::execute`, with ADR 002 answering the branch, and drill
@@ -852,6 +862,10 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
if state.grid.records {
return Some(vec![Command::SetStatus(not_yet("editing in records mode"))]);
}
let (row, col) = state.view.cursor().selected;
if unprojected(state, row, col) {
return Some(awaiting_seed(mode));
}
let cell = cell_at_cursor(state)?;
match cell.id.as_ref()? {
// `edit_text`, never `text`: the drawn text has been through
@@ -898,6 +912,19 @@ fn seeded(seed: String, mode: AppMode) -> Vec<Command> {
]
}
/// Open the cell editor with its seed still to come — after a write, or on a cell
/// not shown yet — settled by the fold once both are answered (3uzg/7mro).
fn awaiting_seed(mode: AppMode) -> Vec<Command> {
vec![
Command::SetBuffer {
name: EDIT_BUFFER.to_string(),
value: String::new(),
},
Command::SetOpenedEditor(Some(OpenedEditor::awaiting())),
Command::ChangeMode(mode),
]
}
/// One direction of the view history. An empty stack is answered out loud:
/// `reduce_view`'s own arm is silent there — `context/code/clients.md`.
fn step_history(stack: &[crate::ViewFrame], step: Command, empty: &str) -> Vec<Command> {
@@ -1104,19 +1131,6 @@ fn advanced(
Some(commands)
}
/// Open the cell editor with its seed still to come: the client settles it from
/// the host's answer, once nothing this client sent is unanswered (improvise-3uzg).
fn awaiting_seed(mode: AppMode) -> Vec<Command> {
vec![
Command::SetBuffer {
name: EDIT_BUFFER.to_string(),
value: String::new(),
},
Command::SetOpenedEditor(Some(OpenedEditor::awaiting())),
Command::ChangeMode(mode),
]
}
/// 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> {
@@ -1136,19 +1150,18 @@ pub(crate) fn edit_seed(state: &ClientState) -> Option<String> {
cell_at_cursor(state)?.edit_text.clone()
}
/// The display text at an absolute grid position, as the cache holds it.
fn seed_at(state: &ClientState, row: usize, col: usize) -> String {
cell_at(state, row, col)
.and_then(|cell| cell.edit_text.clone())
.unwrap_or_default()
/// What an editor awaiting its seed takes once the host projects the cell under
/// the cursor: `None` until then, a relabel's blank included (improvise-7mro).
pub(crate) fn projected_seed(state: &ClientState) -> Option<String> {
let cell = cell_at_cursor(state).filter(|cell| cell.id.is_some())?;
Some(cell.edit_text.clone().unwrap_or_default())
}
/// The projected cell at an absolute grid position, `None` outside the window.
fn cell_at(state: &ClientState, row: usize, col: usize) -> Option<&crate::CellDisplay> {
let cursor = state.view.cursor();
let r = row.checked_sub(cursor.row_offset)?;
let c = col.checked_sub(cursor.col_offset)?;
state.grid.cell(r, c)
/// The edit text the host projected at an absolute grid position.
fn seed_at(state: &ClientState, row: usize, col: usize) -> String {
projected_at(state, row, col)
.and_then(|cell| cell.edit_text.clone())
.unwrap_or_default()
}
/// `EnterEditAtCursor`: re-open the editor where the advance landed, unless `i`
@@ -1160,19 +1173,48 @@ fn reopened_at(
mode: AppMode,
writes: bool,
) -> Vec<Command> {
match cell_at(state, row, col).and_then(|cell| cell.id.as_ref()) {
Some(CellId::Aggregate(_)) => vec![Command::SetStatus(AGGREGATE_DECLINE.to_string())],
Some(CellId::Derived { target }) => vec![Command::SetStatus(derived_decline(target))],
Some(CellId::NonItem { category, item }) => {
vec![Command::SetStatus(crate::non_item_refusal(category, item))]
}
Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate) | None => match writes {
true => awaiting_seed(mode),
false => opened_on(state, seed_at(state, row, col), mode),
},
match (declined_at(state, row, col), writes) {
(Some(refusal), _) => vec![Command::SetStatus(refusal)],
(None, true) => awaiting_seed(mode),
(None, false) if unprojected(state, row, col) => awaiting_seed(mode),
(None, false) => opened_on(state, seed_at(state, row, col), mode),
}
}
/// Why `i` declines the cell the host projected at `(row, col)`. `None` for an
/// editable cell, and for one not projected yet, which the fold settles later.
pub(crate) fn declined_at(state: &ClientState, row: usize, col: usize) -> Option<String> {
match projected_at(state, row, col)?.id.as_ref()? {
CellId::Aggregate(_) => Some(AGGREGATE_DECLINE.to_string()),
CellId::Derived { target } => Some(derived_decline(target)),
CellId::NonItem { category, item } => Some(crate::non_item_refusal(category, item)),
CellId::Record(_) | CellId::Create(_) | CellId::Coordinate => None,
}
}
/// The cell the host projected at an absolute position, read against the frame's
/// own origin: offsets the client has scrolled ahead of the cache cannot alias it.
fn projected_at(state: &ClientState, row: usize, col: usize) -> Option<&crate::CellDisplay> {
let r = row.checked_sub(state.grid.row_frame.first)?;
let c = col.checked_sub(state.grid.col_frame.first)?;
state.grid.cell(r, c)
}
/// Whether `(row, col)` is a cell of the grid that the window the cache holds
/// does not cover: unknown, never the cell at the same cache index (7mro).
pub(crate) fn unprojected(state: &ClientState, row: usize, col: usize) -> bool {
let GridExtent { rows, cols } = state.grid.extent;
let outside = |frame: &crate::AxisFrame, at: usize| {
!(frame.first..frame.first + frame.len()).contains(&at)
};
row < rows
&& col < cols
&& (outside(&state.grid.row_frame, row) || outside(&state.grid.col_frame, col))
}
/// What a cell op says while its cell is still unknown to this client.
const NOT_PROJECTED: &str = "waiting for the host to show this cell";
/// What the host says of an aggregate, the fold of a collapsed group included.
const AGGREGATE_DECLINE: &str =
"Cannot write to an aggregate — it combines several records, so there is no one cell to write";
@@ -1189,8 +1231,8 @@ pub struct OpenedEditor {
pub seed: String,
/// Whether the user has edited the buffer since it opened (improvise-2054).
pub touched: bool,
/// The seed is still to come from the host's answer (improvise-3uzg). Not
/// `serde(default)`: `false` is a claim — `protocol.md` §3, improvise-mgis.
/// The seed is still to come, from a write's answer or an unprojected cell
/// (3uzg/7mro). Not `serde(default)`: `false` is a claim (`protocol.md` §3, mgis).
pub awaiting: bool,
}
@@ -3583,7 +3625,7 @@ mod tests {
Projection::Grid(GridUpdate::ColFrame(AxisFrame::flat(["Revenue"]))),
Projection::Grid(GridUpdate::Cells(vec![
((0, 0), first),
((1, 0), CellDisplay::number("900")),
((1, 0), CellDisplay::number("900").with_id(record_cell())),
])),
Projection::Grid(GridUpdate::Extent(GridExtent { rows: 2, cols: 1 })),
Projection::Basis(published()),
@@ -3894,7 +3936,7 @@ mod tests {
Projection::Grid(GridUpdate::Cells(vec![
((0, 0), CellDisplay::number("900")),
((1, 0), cell),
((2, 0), CellDisplay::number("800")),
((2, 0), CellDisplay::number("800").with_id(record_cell())),
])),
Projection::Grid(GridUpdate::Extent(GridExtent { rows: 3, cols: 1 })),
Projection::Basis(published()),
@@ -4524,6 +4566,25 @@ mod tests {
`KeyOutcome::Request` has nowhere to put one"
);
}
/// improvise-7mro: an empty grid has no cell to wait for, so nothing a cell op
/// says there is a wait. See `context/test-evidence/clients.md`.
#[test]
fn a_cursor_on_an_empty_grid_waits_for_nothing() {
let mut state = in_mode(AppMode::Normal);
state.window = (2, 2);
for key in ['i', 'x'] {
let said = match resolve(&state, ev(KeyCode::Char(key))) {
KeyOutcome::Local(cmds) => cmds,
_ => Vec::new(),
};
assert!(
!said.contains(&Command::SetStatus(NOT_PROJECTED.to_string()))
&& !said.contains(&Command::SetOpenedEditor(Some(OpenedEditor::awaiting()))),
"{key}: waits on a cell no grid has: {said:?}"
);
}
}
}
#[cfg(test)]
crates/improvise-wasm-client/tests/lockstep.rs
+538
View File
@@ -2277,6 +2277,262 @@ fn fold_host() -> App {
app
}
/// Measures on Row (`Profit` both stored and a formula's), months on Column,
/// so a `Down` from the bottom row wraps to the top of the next column.
fn wrapping_host() -> App {
let mut wb = Workbook::new("Test");
wb.add_category("Month").unwrap();
for month in ["Jan", "Feb"] {
wb.add_item("Month", month).unwrap();
}
for measure in ["Revenue", "Profit", "Cost"] {
wb.add_item("_Measure", measure).unwrap();
}
wb.model.add_formula(
improvise_command::formula::parse_formula("Profit = Revenue - Cost", "_Measure").unwrap(),
);
wb.active_view_mut().set_axis("_Measure", Axis::Row);
wb.active_view_mut().set_axis("Month", Axis::Column);
for month in ["Jan", "Feb"] {
for (measure, n) in [("Revenue", 10.0), ("Cost", 4.0)] {
let key = improvise_command::model::cell::CellKey::new(vec![
("Month".to_string(), month.to_string()),
("_Measure".to_string(), measure.to_string()),
]);
wb.model
.set_cell(key, improvise_protocol::CellValue::Number(n));
}
}
let mut app = App::new(wb, None);
app.rebuild_layout();
app
}
/// improvise-y5eb: the settle must not read the cache through offsets it has not
/// caught up with. See `context/test-evidence/clients.md`.
#[test]
fn a_wrapping_advance_settles_on_the_cell_it_lands_on_not_a_stale_one() {
let mut tui = wrapping_host();
tui.cursor_mut().selected = (2, 0);
tui.cursor_mut().row_offset = 1;
for k in ["i", "Enter"] {
reduce_key(&mut tui, BrowserKey::new(k).to_key_event().unwrap());
}
assert_eq!(
tui.view_state.cursor().selected,
(0, 1),
"setup: Enter wrapped"
);
assert!(
tui.view_state.mode.is_editing(),
"setup: Revenue/Feb is editable"
);
let mut host = improvise_command::ui::host::Host::new(wrapping_host());
let (session, welcome) = host.connect(2, 6);
let mut client = ClientState {
window: (2, 6),
..ClientState::default()
};
deliver(vec![welcome], session, &mut client);
let placed = vec![
improvise_protocol::Command::SetSelected { row: 2, col: 0 },
improvise_protocol::Command::SetRowOffset(1),
];
client.fold_outgoing(&placed);
deliver(
host.handle(session, ClientFrame::Commands(placed)),
session,
&mut client,
);
for k in ["i", "Enter"] {
let event = BrowserKey::new(k).to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("{k}: the client must answer this key locally");
};
client.fold_outgoing(&cmds);
deliver(
host.handle(session, ClientFrame::Commands(cmds)),
session,
&mut client,
);
}
assert_eq!(client.view.cursor().selected, (0, 1));
assert_eq!(
format!("{:?}", client.view.mode),
format!("{:?}", tui.view_state.mode),
"the client settled on a cell not under its cursor: {:?}",
client.view.status_msg
);
}
/// improvise-ik6l: Enter on a typed-into editor over a cell `i` declines says
/// so on both ends. See `context/test-evidence/clients.md`.
#[test]
fn enter_on_an_editor_over_a_declined_cell_declines_out_loud_on_both_ends() {
type Case = (fn() -> App, (usize, usize), &'static str);
let cases: [Case; 4] = [
(fold_host, (2, 1), "Enter"),
(yank_host, (1, 3), "Enter"),
(fold_host, (2, 1), "Tab"),
(yank_host, (1, 3), "Tab"),
];
for (make, at, key) in cases {
let mut app = make();
app.cursor_mut().selected = at;
app.view_state.mode = AppMode::editing();
app.view_state.buffers.insert("edit".into(), "7".into());
app.view_state.opened_editor = Some(improvise_protocol::OpenedEditor {
seed: String::new(),
touched: true,
awaiting: false,
});
let before = document_text(&app);
let mut client = client_ready_to_edit(&app, (6, 6));
let event = BrowserKey::new(key).to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("{at:?}: {key} went upstream, where nothing answers it");
};
for cmd in &cmds {
client.fold(cmd);
}
reduce_key(&mut app, event);
assert_eq!(document_text(&app), before, "setup: the TUI wrote nothing");
assert!(
cmds.iter()
.all(|c| !matches!(c, improvise_protocol::Command::Model { .. })),
"{at:?}: the browser composed a write"
);
assert_eq!(
format!("{:?}", client.view.mode),
format!("{:?}", app.view_state.mode),
"{at:?}: the two ends disagree about the mode"
);
assert_eq!(client.view.buffers, app.view_state.buffers, "{at:?}");
assert!(
!app.view_state.status_msg.is_empty() && !client.view.status_msg.is_empty(),
"{at:?}: a silent decline: TUI {:?}, browser {:?}",
app.view_state.status_msg,
client.view.status_msg
);
}
}
/// improvise-5m9z's other half: what the user typed before the host answered
/// is not thrown away. See `context/test-evidence/clients.md`.
#[test]
fn typing_before_the_host_answers_keeps_the_editor_past_the_window() {
let mut host = improvise_command::ui::host::Host::new(fold_host());
let (session, welcome) = host.connect(2, 6);
let mut client = ClientState {
window: (2, 6),
..ClientState::default()
};
deliver(vec![welcome], session, &mut client);
let placed = vec![improvise_protocol::Command::SetSelected { row: 1, col: 1 }];
client.fold_outgoing(&placed);
deliver(
host.handle(session, ClientFrame::Commands(placed)),
session,
&mut client,
);
let mut held = Vec::new();
for k in ["i", "Enter", "7"] {
let event = BrowserKey::new(k).to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("{k}: the client must answer this key locally");
};
client.fold_outgoing(&cmds);
held.push(host.handle(session, ClientFrame::Commands(cmds)));
}
assert_eq!(client.view.cursor().selected, (2, 1), "setup: on the fold");
for reply in held {
deliver(reply, session, &mut client);
}
assert!(
client.view.mode.is_editing(),
"the typed 7 was thrown away: {:?}",
client.view.mode
);
assert_eq!(
client.view.buffers.get("edit").map(String::as_str),
Some("7")
);
}
/// improvise-5m9z: an advance past the client's window lands on a cell it holds
/// no id for, and must settle as the TUI does. See `context/test-evidence/clients.md`.
#[test]
fn advancing_the_editor_past_the_window_settles_where_the_tui_does() {
let expanded = || {
let mut app = fold_host();
let view = app.model_state.workbook.active_view_mut();
view.toggle_group_collapse("Month", "Q2");
app.rebuild_layout();
app
};
type Case = (
fn() -> App,
(usize, usize),
(usize, usize),
&'static str,
bool,
);
let cases: [Case; 3] = [
(fold_host, (2, 6), (1, 1), "Enter", false),
(yank_host, (6, 3), (1, 2), "Tab", false),
(expanded, (2, 6), (1, 1), "Enter", true),
];
for (make, window, at, key, editing) in cases {
let mut tui = make();
tui.cursor_mut().selected = at;
for k in ["i", key] {
reduce_key(&mut tui, BrowserKey::new(k).to_key_event().unwrap());
}
assert_eq!(tui.view_state.mode.is_editing(), editing, "setup: the TUI");
let mut host = improvise_command::ui::host::Host::new(make());
let (session, welcome) = host.connect(window.0, window.1);
let mut client = ClientState {
window,
..ClientState::default()
};
deliver(vec![welcome], session, &mut client);
let placed = vec![improvise_protocol::Command::SetSelected {
row: at.0,
col: at.1,
}];
client.fold_outgoing(&placed);
deliver(
host.handle(session, ClientFrame::Commands(placed)),
session,
&mut client,
);
for k in ["i", key] {
let event = BrowserKey::new(k).to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("{k}: the client must answer this key locally");
};
client.fold_outgoing(&cmds);
deliver(
host.handle(session, ClientFrame::Commands(cmds)),
session,
&mut client,
);
}
assert_eq!(
client.view.cursor().selected,
tui.view_state.cursor().selected,
"{key}: the advance must land past the window on both ends"
);
assert_eq!(
format!("{:?}", client.view.mode),
format!("{:?}", tui.view_state.mode),
"{key} from {at:?} in a {window:?} window: the client and the TUI disagree"
);
}
}
/// improvise-enz6: a commit that advances onto a cell `i` declines re-opens no
/// editor there, on either end. See `context/test-evidence/clients.md`.
#[test]
@@ -3367,3 +3623,285 @@ fn a_move_sent_before_a_pruning_commit_is_answered_writes_where_the_tui_writes()
"the browser wrote a different record from the one the TUI wrote"
);
}
/// One scroll-then-edit case: the window, where the cursor starts, and the keys.
type Pipelined = (
&'static str,
(usize, usize),
((usize, usize), usize, usize),
&'static [&'static str],
);
/// When the host's replies reach the client.
#[derive(Debug, Clone, Copy, PartialEq)]
enum Replies {
/// Each before the next key.
Delivered,
/// All held to the end, so every key reads the first window.
Held,
/// Held until the last two keys, which then run delivered.
HeldThenTyped,
}
/// `wrapping_host`'s cells that differ from how it was built, as `Month/Measure`.
fn written_cells(app: &App) -> Vec<String> {
let model = &app.model_state.workbook.model;
let mut written = Vec::new();
for month in ["Jan", "Feb"] {
for (measure, built) in [
("Revenue", Some(10.0)),
("Profit", None),
("Cost", Some(4.0)),
] {
let key = improvise_command::model::cell::CellKey::new(vec![
("Month".to_string(), month.to_string()),
("_Measure".to_string(), measure.to_string()),
]);
let now = model.get_cell(&key).cloned();
if now != built.map(improvise_protocol::CellValue::Number) {
written.push(format!("{month}/{measure}"));
}
}
}
written
}
/// One key on the client, its frame sent to the host; the reply comes back for
/// the caller to deliver or hold. An upstream key reaches a host that answers nothing.
fn sent_or_upstream(
host: &mut improvise_command::ui::host::Host,
session: improvise_command::ui::project::SessionId,
client: &mut ClientState,
key: &str,
) -> Vec<improvise_command::ui::host::Addressed> {
match resolve(client, BrowserKey::new(key).to_key_event().unwrap()) {
KeyOutcome::Local(cmds) => {
client.fold_outgoing(&cmds);
host.handle(session, ClientFrame::Commands(cmds))
}
_ => Vec::new(),
}
}
/// A host over `make()` and a client on it, the cursor placed and answered.
fn placed_client(
make: fn() -> App,
window: (usize, usize),
(at, row_offset, col_offset): ((usize, usize), usize, usize),
) -> (
improvise_command::ui::host::Host,
improvise_command::ui::project::SessionId,
ClientState,
) {
let mut host = improvise_command::ui::host::Host::new(make());
let (session, welcome) = host.connect(window.0, window.1);
let mut client = ClientState {
window,
..ClientState::default()
};
deliver(vec![welcome], session, &mut client);
let placed = vec![
improvise_protocol::Command::SetSelected {
row: at.0,
col: at.1,
},
improvise_protocol::Command::SetRowOffset(row_offset),
improvise_protocol::Command::SetColOffset(col_offset),
];
client.fold_outgoing(&placed);
deliver(
host.handle(session, ClientFrame::Commands(placed)),
session,
&mut client,
);
(host, session, client)
}
/// improvise-7mro: the offset-aliasing matrix, with the host's replies delivered,
/// held, and held until the typing. See `context/test-evidence/clients.md`.
#[test]
fn keys_sent_before_a_scroll_is_answered_agree_with_the_tui() {
let cases: [Pipelined; 7] = [
(
"wrap",
(2, 6),
((2, 0), 1, 0),
&["i", "Enter", "8", "Enter"],
),
(
"horizontal wrap",
(2, 1),
((2, 0), 1, 0),
&["i", "Enter", "8", "Enter"],
),
("G", (2, 6), ((0, 0), 0, 0), &["G", "i", "7", "Enter"]),
("$", (2, 1), ((0, 0), 0, 0), &["$", "i", "7", "Enter"]),
("gg", (2, 6), ((2, 0), 1, 0), &["g", "g", "i", "7", "Enter"]),
("$ x", (2, 1), ((0, 0), 0, 0), &["$", "x"]),
("gg p", (2, 6), ((2, 0), 1, 0), &["y", "y", "g", "g", "p"]),
];
for (name, window, start, keys) in cases {
let mut tui = wrapping_host();
tui.cursor_mut().selected = start.0;
for &k in keys {
reduce_key(&mut tui, BrowserKey::new(k).to_key_event().unwrap());
}
assert_eq!(
written_cells(&tui).len(),
1,
"setup: {name}: the TUI wrote one cell"
);
for replies in [Replies::Delivered, Replies::Held, Replies::HeldThenTyped] {
let what = format!("{name} ({keys:?}, replies {replies:?})");
let (mut host, session, mut client) = placed_client(wrapping_host, window, start);
let typed_from = match replies {
Replies::Delivered => 0,
Replies::Held => keys.len(),
Replies::HeldThenTyped => keys.len().saturating_sub(2),
};
let mut held = Vec::new();
for (i, &k) in keys.iter().enumerate() {
if i == typed_from {
deliver(std::mem::take(&mut held), session, &mut client);
}
held.extend(sent_or_upstream(&mut host, session, &mut client, k));
if i >= typed_from {
deliver(std::mem::take(&mut held), session, &mut client);
}
}
if replies == Replies::Held {
assert_eq!(
written_cells(host.app()),
Vec::<String>::new(),
"{what}: a write was composed before its target was projected"
);
assert!(
client.view.mode.is_editing() == keys.contains(&"i")
&& !client.view.status_msg.is_empty(),
"{what}: the last key must wait out loud, any editor open: {:?}, {:?}",
client.view.mode,
client.view.status_msg
);
deliver(std::mem::take(&mut held), session, &mut client);
let retry = keys.last().expect("a key");
let reply = sent_or_upstream(&mut host, session, &mut client, retry);
deliver(reply, session, &mut client);
}
assert_eq!(
written_cells(host.app()),
written_cells(&tui),
"{what}: the browser wrote a different cell from the TUI"
);
if replies != Replies::Held {
assert_eq!(document_text(host.app()), document_text(&tui), "{what}");
assert_eq!(
client.view.buffers.get("edit"),
tui.view_state.buffers.get("edit"),
"{what}: the editor's buffer"
);
}
assert_eq!(
client.view.cursor().selected,
tui.view_state.cursor().selected,
"{what}: the cursor"
);
assert_eq!(
format!("{:?}", client.view.mode),
format!("{:?}", tui.view_state.mode),
"{what}: the mode; browser status {:?}",
client.view.status_msg
);
}
}
}
/// Four regions by two measures, every cell stored and no two alike, so a cell
/// read at the wrong row or column is a different value.
fn four_by_two_host() -> App {
let mut wb = Workbook::new("Test");
wb.add_category("Region").unwrap();
for region in ["R0", "R1", "R2", "R3"] {
wb.add_item("Region", region).unwrap();
}
for measure in ["A", "B"] {
wb.add_item("_Measure", measure).unwrap();
}
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
for (r, region) in ["R0", "R1", "R2", "R3"].into_iter().enumerate() {
for (base, measure) in [(10.0, "A"), (20.0, "B")] {
let key = improvise_command::model::cell::CellKey::new(vec![
("Region".to_string(), region.to_string()),
("_Measure".to_string(), measure.to_string()),
]);
wb.model
.set_cell(key, improvise_protocol::CellValue::Number(base + r as f64));
}
}
let mut app = App::new(wb, None);
app.rebuild_layout();
app
}
/// improvise-7mro: a cell inside the held window, read while the client's offsets
/// differ from the frame's origin. See `context/test-evidence/clients.md`.
#[test]
fn a_held_cell_is_read_at_its_position_and_not_its_cache_index() {
for keys in [
&["k", "k", "j", "i", "7", "Enter"][..],
&["k", "k", "j", "i", "Tab"][..],
] {
let mut tui = four_by_two_host();
tui.cursor_mut().selected = (3, 0);
for &k in keys {
reduce_key(&mut tui, BrowserKey::new(k).to_key_event().unwrap());
}
let (mut host, session, mut client) =
placed_client(four_by_two_host, (2, 6), ((3, 0), 2, 0));
let mut held = Vec::new();
for (i, &k) in keys.iter().enumerate() {
if i == 3 {
let cursor = client.view.cursor();
assert_eq!(
(
cursor.selected,
cursor.row_offset,
client.grid.row_frame.first
),
((2, 0), 1, 2),
"setup: inside the held window, the offsets not its origin"
);
}
held.extend(sent_or_upstream(&mut host, session, &mut client, k));
}
assert_eq!(document_text(host.app()), document_text(&tui), "{keys:?}");
assert_eq!(
client.view.cursor().selected,
tui.view_state.cursor().selected
);
// An editor reopened after a write awaits the host's seed (p6v6): read it
// only once the held replies arrive; an unawaited one is read now as well.
let awaiting = client
.view
.opened_editor
.as_ref()
.is_some_and(|o| o.awaiting);
if !awaiting {
assert_editor_agrees(&client, &tui, keys, "held");
}
deliver(held, session, &mut client);
assert_editor_agrees(&client, &tui, keys, "delivered");
}
}
fn assert_editor_agrees(client: &ClientState, tui: &App, keys: &[&str], when: &str) {
assert_eq!(
client.view.buffers.get("edit"),
tui.view_state.buffers.get("edit"),
"{keys:?}, {when}"
);
assert_eq!(
client.view.opened_editor, tui.view_state.opened_editor,
"{keys:?}, {when}: the editor opened on another cell's seed"
);
}