Merge branch 'fix/help-keys-checks' into fix/w7qd-ssoe-help-guards

This commit is contained in:
Edward Langley
2026-09-22 22:37:19 -07:00
41 changed files with 2717 additions and 460 deletions
+135
View File
@@ -3109,6 +3109,23 @@ The third is the gotcha 48 property, and it needs `timesArmed() > 0` first or it
denies an unreachable event: the precise reason must still be on screen after
the 8s window, not replaced by the watchdog. `page.clock` walks past it.
**Strengthened by improvise-i0v9's second pass.** Every assertion above read the
*final* `#status`, so two failures could still pass it. A watchdog that fired
first and was then replaced by a late notice passed. So did a success claim
shown before the notice. The test now records every status with `logStatus` and
asserts, **before** the page clock is walked, that the log ends in `read-only`
and never contained `did not answer` or the exact `in-page worker · <doc>`. A
reason that arrived after the watchdog would sit in that log behind "did not
answer"; that is the "well before 8s" property, stated without a wall-clock
bound. Measured failing against `gh/main` at `da2e5ab`, whose log was
`["in-page worker · two-tab-….improv", "read-only — …"]`: the premature claim
the ticket measured at 13ms is still there once 1wft's notice exists. The
brief's rival, `onNotice` calling `setShell` instead of `fail` (the watchdog not
disarmed), fails it at the first `toContainText("read-only")`. The Snapshot's
claim replaced the reason at once, which is why the eight-second form of this
rival is now exercised by the invalid-`?doc=` spec instead (see
`context/code/clients.md`, "The watchdog").
It runs on Chromium and Firefox — the two engines with OPFS — rather than
Chromium alone, because the exclusivity semantics are the engine's and the two
word the refusal completely differently.
@@ -4460,6 +4477,37 @@ Records mode is deliberately not covered. There the TUI yanks `None` and its
case there would assert the bug. Since improvise-6p8q's fix it is covered, by
`yy_then_p_in_records_mode_writes_the_record_the_host_writes` below.
### `an_untouched_browser_edit_stores_the_identical_value`
**improvise-umi4**, the browser half. The client seeds `i` from the projected
`CellDisplay::edit_text` (`resolve.rs::edit_or_drill`), which was
`App::display_value_in` and so `CellValue`'s rounded `Display`. The client
resolves `i` and `Enter` itself, and what it composed is applied to the host
through `apply_on` (`Checked::against` plus `apply_command`). The stored value
is then compared by `f64::to_bits`. The fixture puts the value at row 1,
column 1 of a 2×2 pivot grid, with values that `Display` rounds (the list in
`context/test-evidence/command.md`). A records grid is not driven, because the
browser declines records editing (`edit_or_drill`).
Failed before the fix on `Number(1.23456789)` stored as `Number(1.2346)`.
**Rival:** `CellValue::edit_text`'s number arm set back to `Display`. The test
failed on the same case.
The test is in-process, so it cannot see the JSON wire. A float that does not
survive `serde_json` is improvise-6oy4, a separate fix.
### `every_projected_edit_text_commits_back_to_its_value`
The projection contract behind the test above, for **both** grid kinds. Every
`Record` cell's `edit_text`, fed through `typed()`, must give back the cell's
`value`. It runs over the whole projected grid in pivot mode and in records
mode (entered with `R`). A guard asserts that the precise value was projected
at all. This test exists because `project::cell_display`'s records arm spelled
`edit_text` itself, apart from `display_value_in`, and no key-path test reaches
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))`.
### 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
@@ -4474,6 +4522,93 @@ registers to agree; a TUI-only blank filter (`Text("")` to `None`) fails it on
`+`, which is why that filter was removed rather than kept. The evidence entries
for the host-side tests are in `command.md`.
### the worker is not called up until its first frame has arrived
improvise-i0v9. The worker transport wrote `in-page worker · <doc>` when it
spawned the worker, before anything had answered. A worker that then failed
therefore showed a success-looking status over a blank page. The spec records
every `#status` with `logStatus` and requires the whole log to be exactly
`["starting the worker · <doc>…", "in-page worker · <doc>"]`. It pins the order
as well as the final text, since a final read cannot see a claim made too early.
Measured failing against `gh/main` at `da2e5ab`. The rival, restoring the
immediate `setShell(\`in-page worker · ${DOC}\`)` in `workerTransport`, fails it
on the log's first entry. The final read is `toHaveText`, not `toContainText`.
The older specs' `toContainText("in-page worker")` could not tell the starting
text from the claim if the one contained the other, and the starting text
deliberately does not.
### only the first frame claims the peer answered, and not before it
improvise-i0v9, the unit half of the spec above. `arm(message, { answer })`
must write nothing until `sawFrame`, then write `answer` once, however many
frames follow. Fails against `gh/main`'s `createShell`, which has no `answer`
and so never writes the claim at all.
### a reason given before the first frame is not replaced by the frame's claim
The order `worker.js` really produces on a locked document: the `ReadOnly`
notice is posted after `start()` and **before** the Snapshot. So `fail` then
`sawFrame` must leave the notice's text as the only thing ever shown. The rival
is the obvious fix, `sawFrame` writing `answer` unconditionally. It fails both
this test and the one above: the first on its exact log, which is then written
twice. This test passes against `gh/main` by construction, since no claim exists
there. It is not evidence of the bug; it guards against the naive fix.
## Numbers across the serialized wire (improvise-6oy4)
No manifest enabled serde_json's `float_roundtrip`, so its default f64 parser
could read a number back one ULP away from the one written. Found by the
adversarial review of PR #178. That review measured 139,787 of 1M ordinary
decimals and 296,493 of 1M random-bit f64s coming back wrong. Every test that
passes `Command`s or caches **in-process**, which includes all of
`tests/lockstep.rs`, never serializes, so none of them could see it.
`LOSSY = 0.24815714285714285` is a value the default parser reads back as
`0.24815714285714283`. Both tests compare `f64::to_bits`, never `==`.
### `a_yanked_number_crosses_the_serialized_wire_exactly`
South/Revenue holds `LOSSY` at row 1, so the source is off index 0. The page
presses `j y y l p` through `press`, which serializes each frame to a real
`Host::handle` and folds the serialized replies. The value crosses the wire
twice: down as `CellDisplay::value` in the projection (#178), then up as the
`SetCell` the paste composes. Each hop has its own assertion, so a fix to one
hop cannot hide the other. `set_viewport` is called first. Without it the
client records no window and the cursor keys do nothing, which an early
version of this test found out.
**Fault injected:** no `float_roundtrip` in any manifest, the pre-fix tree at
`a6e3651`. Fails at the host-to-client assertion: yanked bits
`4598108823619341467` against `…468`.
### `a_typed_number_crosses_the_serialized_wire_exactly`
`typed_cell(1, 1, "0.24815714285714285")`. The client parses exactly with
`str::parse`, and the host re-parsed the JSON lossily. This path lost precision
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.
### resolve::tests::every_grid_move_lands_where_its_binding_says
improvise-6562, found reviewing PR #180 at 949810c, and it is the same