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

Merge remote-tracking branch 'gh/main' into wave/km71-edit-formula

Browse Source 
# Conflicts:
#	context/test-evidence/clients.md
#	context/test-evidence/command.md
This commit is contained in:
Edward Langley
2026-09-22 23:43:39 -07:00
parent a91db1d857 ea119bd9d1
commit 77f059c852
30 changed files with 1418 additions and 287 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/test-evidence/clients.md
+127 -18
View File
@@ -4498,6 +4498,42 @@ that arm from a browser. **Rival:** that arm restored to `Some(text.clone())`,
the display text. It failed with `records=true: edit text "1.2346" does not
commit back to Some(Number(1.23456789))`.
### an_edit_writes_the_cell_the_user_clicked_after_the_rows_renumber
improvise-2wu3. `web/app.js` freezes the screen while an inline editor is open
(`repaint` returns early), but `on_frame` goes on folding. The editor kept its
viewport `(row, col)`, and `typed_cell` resolved it against the *current*
cache at commit.
The test follows the ticket's acceptance criteria against a real `Host`:
1. Client A reads the North/Revenue record at `(0, 0)`.
2. Client B's `HideItem("North")` is applied.
3. Only the frames addressed to A are folded into A, with no repaint, just as
the page does it.
4. A precondition asserts the rows really renumbered.
5. The write A composes must name the record A clicked.
**Red first**, at HEAD after improvise-ktlk (PR #169), which moved the editor's
seed but not its commit. Written against the position API as it stood
(`typed_cell(0, 0, "42")`), it failed. The write named `RecordHash(161215…)`,
which is South's record now sitting at row 0, where North's `RecordHash(246296…)`
was expected. So the ticket still reproduced. The frame would have passed the
basis check, because A had folded the fresh `Projection::Basis` along with the
rows.
**The fix makes the fault unwritable in the page.** The page's surface no
longer takes a position at commit. `edit_target(row, col)` is read when the
editor opens and returns the `RecordCell` as an opaque string, and
`typed_cell(target, text)` composes against it. The same test, rewritten for
that surface, passes. There is no position left at commit for a fault to
re-introduce. Rust-side `typed_cell_frame(row, col, …)` remains for tests
that compose at read time.
The page path is exercised end to end by `harness/tests/shell.spec.js`'s
"committing an edit sends one frame and shows the host's value", which goes
through `edit_target` and the new `typed_cell`. The whole harness passes
locally (69 passed, 4 skipped).
### yy_then_p_in_records_mode_writes_the_record_the_host_writes
improvise-6p8q in lockstep. The yank test in this file deliberately stayed on a
@@ -4580,24 +4616,12 @@ before #178 too, as the review noted.
**Fault injected:** the same pre-fix tree. Fails: the host stored
`4598108823619341467` in South/Cost.
### `every_build_that_parses_a_frame_enables_float_roundtrip`
The two tests above run in **one** feature-unified test build, which includes
`improvise-ws-server` and `improvise-command` as dev-dependencies. So they pass
whenever **any** crate in that build enables the feature. The shipped browser
bundle is built from one crate alone, so it would still be lossy. That was
demonstrated: with the feature removed from `improvise-wasm-client` only, both
tests stayed green while `cargo tree` showed no `float_roundtrip` in the wasm
bundle. This test asks cargo instead, once per crate that parses a frame:
`cargo tree --locked --edges normal,features --invert serde_json --package
<crate>`, with `--target wasm32-unknown-unknown` for the two bundles. It uses
normal edges only, because a dev edge would unify the feature back in
(`context/repo-map.md`, `## Wasm`).
**Faults injected:** the feature removed from `improvise-wasm-client` alone,
which fails naming that crate while the two tests above pass. Then removed from
`improvise-ws-server` alone, which fails naming it. The native path runs
without `--target`, so the two arms are different invocations.
**Retired: `every_build_that_parses_a_frame_enables_float_roundtrip`.** It asked
`cargo tree` about three crates listed by hand. improvise-dz8c replaced it with
`every_crate_that_can_parse_json_enables_float_roundtrip`, whose entry is in
`tooling.md`. The injections recorded for it still hold: with the feature
removed from `improvise-wasm-client` alone, the two tests above stay green, so
they cannot guard the bundle alone.
### resolve::tests::every_grid_move_lands_where_its_binding_says
@@ -4653,3 +4677,88 @@ updated"). This test fails on the **document**, which is the property. It also
pins a real divergence: on a rename the host says what was renamed and the
client cannot, because `FormulaId` is opaque and it does not know the old
target.
**improvise-w7qd, and the floor was the hole.** As first written the test
covered `move-selection` and a four-entry jump table, behind `pressed >= 14`.
Two things passed the whole workspace, both found in re-review of #180 at
544744c: negating the `scroll-rows` and `page-scroll` arms, so `Ctrl-d`/`Ctrl-u`
and `PgDn`/`PgUp` all scroll the wrong way (advertised, and still "acting" from
the centre); and dropping the jump table *with* `$` sent to the first column,
because the sixteen `move-selection` sequences met the floor on their own. The
test now walks **every** command Normal binds. `scroll-rows` and `page-scroll`
state their landing from their own argument exactly as `move-selection` does —
from the centre of a 3x3 grid any step of at least one lands at `1 + d`
clamped, so a sign is visible whatever the page size — and a command that moves
the cursor (in the same view) without a stated landing **fails** rather than
being skipped. So a dropped table entry is caught by derivation, not by a count:
under the second rival it fails naming every jump as moving with nothing stating
where. The first rival fails on `PgUp` (`page-scroll ["-1"]`) and, alone, on
`Ctrl-u` (`scroll-rows ["-5"]`). A last assertion that each kind was pressed at
least once guards the other direction — a binding vanishing — and is a named
list on purpose, since that is the one thing no walk over bindings can see.
**improvise-5kdr: the 3x3 fixture saw only a sign.** From the centre of a 3x3
grid every step of one or more clamps to an edge, so the w7qd version stated
`page-scroll` as `1 + d` — one row, which is not what it does — and passed
because the true landing clamped to the same edge. Measured on #198 at 8772db2:
`page-scroll` ignoring the page size together with `scroll-rows` moving one row,
or `move-selection` moving `2d`, left the test green. The walk now stands at
(10,10) of a 21x21 grid with an 8x8 window, far from every edge, so a landing
shows magnitude: `10+d` for `move-selection` and `scroll-rows`, `10 + page·d`
for `page-scroll`, 0/20 for the jumps. The page size is not written into the
test: `grid_move`'s page rule moved into `page_rows(window_rows)` and the test
asks it, so the rule has one home. Rivals, each run: `move-selection` doubled
fails on `Up` (`move-selection ["-1","0"]`); the page-size-and-one-row pair
fails on `PgUp` (`page-scroll ["-1"]`); the one-row `scroll-rows` alone fails on
`Ctrl-u` (`scroll-rows ["-5"]`); the jump-table rival still fails naming every
jump. The residual, stated: because the test reads `page_rows`, a change to the
page rule itself moves the test with it — whether that rule is the TUI's is
`tests/lockstep.rs`'s question, not this one's.
### an_edit_is_refused_when_the_cell_was_written_while_the_editor_was_open
improvise-0j0f, found by the review of PR #188 at 48f52e7 and confirmed
against a real `Host`. #188's first version captured the edited record's
*identity* when the editor opened, but claimed its *basis* at commit. Because
the fold runs on while the screen is frozen, the claim was the fresh counter.
The test runs the steps in this order:
1. A opens an editor on North/Revenue, which shows 1200.
2. B writes 999 to the same cell.
3. A folds only its own frames and does not repaint.
4. A commits "42".
It asserts that the host answers `Rejected(RecordMoved)` and still holds 999.
**Red first:** against the claim-at-commit version it failed, because the
commit was accepted. **Rival injected after the fix:** `typed_target_frame`
back to `basis.claim([record])`, which claims at commit. Only this test failed
(`12 passed; 1 failed`), and the 2wu3 test beside it stayed green. That is
right, because the two are separate properties: *which* record the commit
names, and *which version* of it the claim states. Reverted.
An earlier version of this entry said the reviewer's clear variant would be
`RecordGone`. It is not, and that claim was reasoning and not a test. The
clear variant is now tested by
`an_edit_is_refused_when_the_cell_was_cleared_while_the_editor_was_open`,
below, and the answer is `RecordMoved` (improvise-4img).
### an_edit_is_refused_when_the_cell_was_cleared_while_the_editor_was_open
improvise-4img: the clear variant of the 0j0f race above. The steps are the
same, except that in step 2 B *clears* North/Revenue instead of writing 999.
The test asserts that A's commit of "42" is refused with
`Rejected(RecordMoved)`, and that the cell is still absent afterwards.
**Why `RecordMoved` and not `RecordGone`.** After the clear, North has no cell
left, so `DataStore::resolve_record` answers `None`. But `App::resolve_record`
falls back to the layout, and the grid still draws the North row because
`prune_empty` is off. So the record still resolves, and the counter A claimed
at open has moved. `RecordGone` is for a record that nothing resolves at all;
this one is still on screen.
**Rival injected:** `typed_target_frame` back to `basis.claim([record])`, which
claims at commit. The commit was then *accepted*. With the refusal assertion
disabled, the host held `Some(Number(42.0))`, so A's commit had re-created the
cell B cleared. Under that rival, the 0j0f test above failed as well. Both
files were restored.