Merge commit '33cf49692781f1bd0f4b7654bce6ac9f7a30cc4f' into fix/wdmp-asker-cursor

context/code/command.md

@@ -3401,18 +3401,41 @@ one run in eight (measured: 46 of 400). So the arguments now include **every
 item of every regular category as a one-coordinate drill**, read off the parsed
 document. A one-coordinate drill spans every other category whole, so it matches
 the whole slice of records under that item, not one record picked by name. In
-the demo that is four records and eight cells for each of the eight drills. The
-test
-also asserts that the widest candidate it actually passes matches at least four
-records. That guard catches a demo with too little data, and it catches the
-derived drills being dropped from the argument list. It does **not** catch the
-comparison losing its wide drills some other way, because it measures what the
-argument lists match, not what the built commands ordered (improvise-b9f2). The
-review showed this with a rival: a `drill-into-cell` parser that refuses fewer
-than two arguments sends every one-coordinate drill to the one-record cursor
-fallback. The guard still reports four records, and with the store's sort
-removed the law passed 38 of 300 runs. With the store's sort removed and the
-parser intact, the law fails 400 of 400 runs.
+the demo that is four records and eight cells for each of the eight drills. With
+the store's sort removed, the law fails 400 of 400 runs.
+
+**The width guard measures what the comparison compared (improvise-b9f2,
+improvise-btyw).** The test asserts that some `drill-into-cell` the comparison
+actually built and compared froze at least four records. For each one it applies
+those very effects to one scratch `App` and counts the records in its
+`drill_state`. It counts them only when their `Debug` text, which carries their
+order, appears in the text the comparison compared. The comparison reads `Debug`
+while the scratch `App` reads the applied state, and those are two
+representations. The review showed they can disagree: give `StartDrill` a
+`Debug` that prints only a row count, and the frozen records are still four
+while the compared text no longer holds their order. With the store's sort
+removed the law then passed 300 of 300 runs, with the guard green. Requiring
+the ordered records in the compared text ties the guard to what the comparison
+could actually see. A first
+version measured what the *argument lists* match in the store instead. That was
+a parallel derivation, and the review beat it with a `drill-into-cell` parser
+that refuses fewer than two arguments. Every one-coordinate drill then went to
+the one-record cursor fallback, the argument lists still matched four records,
+and with the store's sort removed the law passed 38 of 300 runs.
+
+Counting frozen records is not enough by itself. A drill over no coordinates
+(any candidate with no `Cat/Item` argument, such as `["1", "1"]`) freezes
+**every** record, and it reads them through `iter_cells` in insertion order,
+never through the narrowing index where gotcha 18 lives. Under the parser rival
+that whole-store drill still froze 16 records, and a guard counting it stayed
+silent. So only a drill that **narrowed** the store counts: one that froze fewer
+records than the store holds. With the parser rival in place, the guard fires on
+every run ("froze 1 records"), and it fires on every run under the `Debug` rival
+too ("froze 0 records").
+
+What the guard still cannot see is a narrowing drill whose records are frozen in
+an order that does not come from the store. It checks that the law had a wide
+drill to compare, not why the drill's order would differ.
 
 **What it cannot see.** Only commands and arguments the fixture reaches. A
 command whose nondeterminism needs some other state, such as records mode, a

context/test-evidence/clients.md

@@ -4749,3 +4749,58 @@ 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.
+
+### a_locally_resolved_page_scroll_lands_where_the_host_would_put_it
+
+**What was missing (improvise-5xbq).** `tests/lockstep.rs` compared only `j`
+and `G` against the host. The browser's page rule (`resolve::page_rows`) and
+the TUI's (`Move`'s `Page` arm, `visible_rows * 3 / 4`) were two copies, and
+two faults injected into the browser's copy left the whole workspace green:
+`page_rows` returning `window_rows / 2`, and `grid_move` paging by
+`window_cols`. The second was invisible for a further reason: the only window
+the page-rule test used was square, `(8, 8)`.
+
+**The fixture.** 20 rows by 3 columns; the host sees 8 rows and the client
+announces `(8, 3)`. The three candidate rules then give three different
+steps: 6 (three quarters of the rows), 4 (half the rows) and 2 (three quarters
+of the columns). The walk starts off the edge (two `j`), then presses `PgDn`,
+`PgUp`, `Ctrl-d` and `Ctrl-u`, including a `PgDn` that clamps at row 19.
+
+**Why it pins the rows as well as comparing the ends.** `page_rows` is now the
+one home (the TUI's `Move` calls it), so a wrong rule moves host and client
+together, and a comparison alone passes. Each press therefore also asserts the
+row the TUI's rule names.
+
+**Injected, before and after the move to one home.**
+
+| Fault | Two copies | One home |
+|---|---|---|
+| `page_rows` returns `window_rows / 2` | fails at press 2 (`PgDn`), "client and host disagree" | fails at press 2, "not the row the TUI's page rule names"; `app.rs`'s `page_down_scrolls_by_three_quarters_visible` and `page_up_scrolls_backward` fail too |
+| `grid_move` pages by `window_cols` | fails at press 2, "client and host disagree" | fails at press 2, "client and host disagree" |
+
+`Ctrl-d`/`Ctrl-u` are `scroll-rows ±5`, not the page rule. They are in the walk
+because the ticket named them, and they move the start point of the next page
+turn to a row the fixed rules alone would not reach.
+
+### a narrow window still shows how to page through help — `harness/tests/help.spec.js`
+
+**What broke (improvise-1c5m).** Below the width at which the app bar wraps,
+`#help` (fixed, `inset: 2rem`, `z-index: 30`) covers `#hint`, and since PR #180
+Welcome carries no paging instruction of its own. So a new user on Welcome at
+phone width could not see how to reach page two.
+
+**The fix is option (b) from the ticket.** The overlay draws `model.hint` in a
+footer. In Help mode that is already `hint_line(Help)`, the same string `#hint`
+carries, so it adds no derivation, and it sits outside `.help-body`, so
+"Welcome is prose" still holds. The footer is `position: sticky` because the
+overlay itself scrolls and Welcome is longer than a phone-height overlay.
+
+**What the case asserts.** At 600x800 on Welcome, it reads the hint string from
+`#hint`, so the spec spells no keys. It then requires some element showing
+exactly that string to be the topmost element at its own centre, which is the
+`elementFromPoint` measurement the ticket made by hand.
+
+**Red, then a rival.** At base it failed with "the paging hint is under the
+help overlay". With the fix and `position: sticky` changed to `static`, it
+fails the same way: the footer exists but sits below the fold of a scrolling
+overlay, so presence alone cannot satisfy the case.

context/test-evidence/command.md

@@ -5509,14 +5509,41 @@ rivals it catches:
   still pin the store directly.
 - **The derived drills dropped from the argument list** (the
   `all.extend(one_coordinate_drills(model))` line commented out): the test
-  fails before comparing anything, with "the widest drill orders 1 records".
-  That is the only way of losing the wide drills the guard can see. It measures
-  what the argument lists match, not what the built commands ordered, so it can
-  pass while the comparison has lost its wide drills (improvise-b9f2). Review
-  rival: `drill-into-cell`'s parser changed to refuse fewer than two arguments,
-  which sends every one-coordinate drill to the one-record cursor fallback. The
-  guard stays silent and, with gotcha 18 re-injected, the law passed 38 of 300
-  runs.
+  fails with "the widest narrowing drill compared froze 1 records".
+- **`drill-into-cell`'s parser changed to refuse fewer than two arguments**
+  (improvise-b9f2): every one-coordinate drill then goes to the one-record cursor
+  fallback. The first width guard measured what the argument lists match, so it
+  stayed silent. With gotcha 18 re-injected the law passed 38 of 300 runs in
+  review, and 31 of 300 here at `cfcc9bff`. The guard now applies the compared
+  drills' own effects to a scratch `App` and counts the records they froze.
+  Against this rival it fails on every run with "froze 1 records".
+  - A first version of the fix counted all frozen records and **still passed**.
+    A candidate with no `Cat/Item` argument drills the whole store (16 records),
+    and it reads them in insertion order through `iter_cells`, which the
+    injection cannot reach. So the guard counts only drills that froze fewer
+    records than the store holds.
+  - Measured at `7dfa90f1`, test binary in a loop:
+
+    | Faults | Law passes | Guard fired |
+    |---|---|---|
+    | gotcha 18 + parser rival | **0 of 300** | all 300 |
+    | gotcha 18 alone | **0 of 300** | 0 (every failure a real divergence) |
+    | neither | 300 of 300 | 0 |
+- **`StartDrill`'s `Debug` changed to print only its row count**
+  (`StartDrill(4 rows)`, improvise-btyw): the law compares effects by their
+  `Debug` text, and this rival takes the record order out of that text, while
+  the frozen `drill_state` still holds four records. At `7dfa90f1`, with gotcha
+  18 re-injected, the law **passed 200 of 200** runs and the guard stayed green.
+  The guard now counts a drill only when its frozen records, as `Debug` prints
+  them, appear in the text the comparison compared. The same measurements after
+  the repair, 200 runs each:
+
+  | Faults | Law passes | Guard fired |
+  |---|---|---|
+  | gotcha 18 + `Debug` rival | **0 of 200** | all 200 ("froze 0 records") |
+  | gotcha 18 + parser rival | **0 of 200** | all 200 |
+  | gotcha 18 alone | **0 of 200** | 0 (every failure a real divergence) |
+  | neither | 200 of 200 | 0 |
 - **A stale exemption**: `("quit", "stale")` listed in `OUTSIDE_THE_CONTEXT`.
   It fails as "listed … but did not".
 

crates/improvise-command/src/command/cmd/determinism.rs

@@ -1,14 +1,14 @@
 //! A command is a function of its context: two contexts built independently from
 //! equal data give equal effects — `context/code/command.md`, improvise-hzjo.
 
-use std::collections::{BTreeSet, HashSet};
+use std::collections::BTreeSet;
 
 use super::core::{Cmd, CmdContext};
+use super::grid::DrillIntoCell;
 use super::registry::tests::candidate_args;
 use crate::command::key::{KeyCode, KeyModifiers};
 use crate::model::Model;
 use crate::model::cell::{CellKey, CellValue};
-use crate::model::record::record_of;
 use crate::ui::app::App;
 use crate::ui::effect::{self, Effect};
 
@@ -61,27 +61,23 @@ fn one_coordinate_drills(model: &Model) -> Vec<Vec<String>> {
     drills
 }
 
-/// How many distinct records the widest all-`Cat/Item` candidate matches.
-fn widest_drill(model: &Model, candidates: &[Vec<String>]) -> usize {
-    let coordinates = |args: &Vec<String>| -> Option<Vec<(String, String)>> {
-        args.iter()
-            .map(|a| {
-                a.split_once('/')
-                    .map(|(c, i)| (c.to_string(), i.to_string()))
-            })
-            .collect()
-    };
-    candidates
-        .iter()
-        .filter(|args| !args.is_empty())
-        .filter_map(coordinates)
-        .map(|partial| {
-            let cells = model.data.matching_cells(&partial);
-            let records: HashSet<CellKey> = cells.iter().map(|(k, _)| record_of(k)).collect();
-            records.len()
-        })
-        .max()
-        .unwrap_or(0)
+/// Records a compared drill froze, counted only if it narrowed the store and its
+/// ordered records are in `compared`, the text the law compared (improvise-btyw).
+fn records_narrowed(scratch: &mut App, effects: Vec<Box<dyn Effect>>, compared: &str) -> usize {
+    let store = scratch
+        .model_state
+        .workbook
+        .model
+        .data
+        .live_records()
+        .count();
+    scratch.apply_effects(effects);
+    let frozen = scratch
+        .drill_state
+        .as_ref()
+        .filter(|drill| compared.contains(&format!("{:?}", drill.records)))
+        .map_or(0, |drill| drill.records.len());
+    if frozen < store { frozen } else { 0 }
 }
 
 /// The registry's candidates, plus names the demo really has: a drill or a fit
@@ -108,9 +104,9 @@ fn args_the_document_names(model: &Model) -> Vec<Vec<String>> {
     all
 }
 
-fn effects_of(name: &str, args: &[String], ctx: &CmdContext) -> Option<String> {
+fn effects_of(name: &str, args: &[String], ctx: &CmdContext) -> Option<Vec<Box<dyn Effect>>> {
     let cmd = ctx.registry.interactive(name, args, ctx).ok()?;
-    Some(format!("{:?}", cmd.execute(ctx)))
+    Some(cmd.execute(ctx))
 }
 
 /// What running every registered command under `a` and under each of `others` found.
@@ -120,20 +116,34 @@ struct Comparison {
     shown: Vec<String>,
     never_built: Vec<&'static str>,
     compared: usize,
+    widest_drill: usize,
 }
 
 fn compare(a: &CmdContext, others: &[CmdContext]) -> Comparison {
     let mut found = Comparison::default();
     let candidates = args_the_document_names(a.model);
+    let drill = DrillIntoCell {
+        key: CellKey::new(vec![]),
+    };
+    let mut scratch = fixture();
     for name in a.registry.names().collect::<Vec<_>>() {
         let before = found.compared;
         for args in &candidates {
-            let Some(left) = effects_of(name, args, a) else {
+            let Some(effects) = effects_of(name, args, a) else {
                 continue;
             };
+            let left = format!("{effects:?}");
+            if name == drill.name() {
+                found.widest_drill =
+                    found
+                        .widest_drill
+                        .max(records_narrowed(&mut scratch, effects, &left));
+            }
             found.compared += 1;
             for b in others {
-                let right = effects_of(name, args, b).unwrap_or_default();
+                let right = effects_of(name, args, b)
+                    .map(|e| format!("{e:?}"))
+                    .unwrap_or_default();
                 if left != right {
                     found.diverged.insert(name);
                     found
@@ -157,13 +167,12 @@ fn every_command_gives_equal_effects_in_independently_built_equal_contexts() {
         .iter()
         .map(|app| app.cmd_context(KeyCode::Null, KeyModifiers::NONE))
         .collect();
-    let model = contexts[0].model;
-    let widest = widest_drill(model, &args_the_document_names(model));
-    assert!(
-        widest >= 4,
-        "the widest drill orders {widest} records, so the store's order is nearly a coin toss"
-    );
     let found = compare(&contexts[0], &contexts[1..]);
+    assert!(
+        found.widest_drill >= 4,
+        "the widest narrowing drill compared froze {} records, so the store's order is nearly a coin toss",
+        found.widest_drill
+    );
     assert!(
         found.never_built.is_empty(),
         "no candidate args build these in the fixture, so the law never ran them: {:?}",

crates/improvise-command/src/command/cmd/navigation.rs

@@ -118,7 +118,7 @@ impl Cmd for Move {
                 }
             }
             MoveKind::Page(dir) => {
-                let delta = (self.cursor.visible_rows as i32 * 3 / 4).max(1) * dir;
+                let delta = improvise_protocol::resolve::page_rows(self.cursor.visible_rows) * dir;
                 let nr = (self.cursor.row as i32 + delta).clamp(0, row_max) as usize;
                 (nr, self.cursor.col)
             }

crates/improvise-protocol/src/resolve.rs

@@ -620,7 +620,8 @@ enum GridMove {
 }
 
 /// Rows one `page-scroll` step moves: three quarters of the window, at least one.
-fn page_rows(window_rows: usize) -> i32 {
+/// The one home: the TUI's `Move` calls it too (improvise-5xbq).
+pub fn page_rows(window_rows: usize) -> i32 {
     (window_rows as i32 * 3 / 4).max(1)
 }
 

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

@@ -266,6 +266,70 @@ fn a_locally_resolved_cursor_move_lands_where_the_host_would_put_it() {
     );
 }
 
+/// Twenty rows by three columns, the host's window eight rows by three: not
+/// square, so a rule keyed off the wrong axis lands somewhere else.
+fn paging_host() -> App {
+    let mut wb = Workbook::new("Test");
+    wb.add_category("Region").unwrap();
+    for i in 0..20 {
+        wb.add_item("Region", format!("R{i:02}")).unwrap();
+    }
+    for m in ["A", "B", "C"] {
+        wb.add_item("_Measure", m).unwrap();
+    }
+    wb.active_view_mut().set_axis("Region", Axis::Row);
+    wb.active_view_mut().set_axis("_Measure", Axis::Column);
+    let mut app = App::new(wb, None);
+    app.term_height = 8 + 8;
+    app.rebuild_layout();
+    app
+}
+
+/// Page and half-page scrolls land in the same place on both ends, and in the
+/// place the TUI's rule names. Why the fixture and the pinned rows: the evidence file.
+#[test]
+fn a_locally_resolved_page_scroll_lands_where_the_host_would_put_it() {
+    let window = (8, 3);
+    let mut app = paging_host();
+    let mut client = client_of(&app, window);
+    let key = |code, mods| KeyEvent::new(code, mods);
+    let none = KeyModifiers::NONE;
+    let ctrl = KeyModifiers::CONTROL;
+    let presses = [
+        (key(KeyCode::Char('j'), none), 1),
+        (key(KeyCode::Char('j'), none), 2),
+        (key(KeyCode::PageDown, none), 8),
+        (key(KeyCode::PageDown, none), 14),
+        (key(KeyCode::PageUp, none), 8),
+        (key(KeyCode::Char('d'), ctrl), 13),
+        (key(KeyCode::PageDown, none), 19),
+        (key(KeyCode::Char('u'), ctrl), 14),
+        (key(KeyCode::PageUp, none), 8),
+        (key(KeyCode::PageUp, none), 2),
+    ];
+    for (press, (event, want_row)) in presses.into_iter().enumerate() {
+        match resolve(&client, event) {
+            KeyOutcome::Local(cmds) => {
+                for cmd in &cmds {
+                    client.fold(cmd);
+                }
+            }
+            other => panic!("press {press}: the client must answer {event:?}, got {other:?}"),
+        }
+        reduce_key(&mut app, event);
+        assert_eq!(
+            client.view.cursor(),
+            app.view_state.cursor(),
+            "press {press} ({event:?}): client and host disagree"
+        );
+        assert_eq!(
+            client.view.cursor().selected,
+            (want_row, 0),
+            "press {press} ({event:?}): not the row the TUI's page rule names"
+        );
+    }
+}
+
 /// The same for `G`, the key `GridExtent` exists for: it is *defined* as "the
 /// last row", so a window-only client would have nothing to jump to.
 #[test]

harness/tests/help.spec.js

@@ -132,4 +132,30 @@ test.describe("the help overlay draws the shared content", () => {
     await expect(rows_.last().locator("td.selected")).toHaveCount(1);
     await expect(page.locator("#grid tbody td.selected")).toHaveCount(1);
   });
+
+  // improvise-1c5m. Below the width at which the app bar wraps, `#help` covers
+  // `#hint`, and Welcome itself says nothing about paging. The contract: the
+  // mode's own hint is UNCOVERED somewhere while help is open.
+  test("a narrow window still shows how to page through help", async ({ page }) => {
+    await page.setViewportSize({ width: 600, height: 800 });
+    await openHelp(page);
+
+    // The string is the page's own `#hint`, i.e. `hint_line(Help)`; this spec
+    // spells no keys. Asserting it names paging keeps an empty hint out.
+    const hint = (await page.locator("#hint").textContent()).trim();
+    expect(hint).toContain("next-page");
+
+    // An element showing exactly that string, topmost at its own centre: the
+    // overlay covering `#hint` is precisely what `elementFromPoint` sees.
+    const uncovered = await page.evaluate((text) =>
+      [...document.querySelectorAll("body *")].some((node) => {
+        if (node.children.length > 0 || node.textContent.trim() !== text) return false;
+        const r = node.getBoundingClientRect();
+        if (r.width === 0 || r.height === 0) return false;
+        const top = document.elementFromPoint(r.left + r.width / 2, r.top + r.height / 2);
+        return top !== null && (top === node || node.contains(top));
+      }),
+    hint);
+    expect(uncovered, "the paging hint is under the help overlay").toBe(true);
+  });
 });

web/app.js

@@ -348,7 +348,13 @@ function paintHelp(model) {
     }
   }
 
-  el.help.replaceChildren(tabs, body);
+  // The mode's own hint again, as the TUI popup's footer: once the app bar
+  // wraps, the overlay covers `#hint` (improvise-1c5m).
+  const footer = document.createElement("p");
+  footer.className = "help-footer";
+  footer.textContent = model.hint;
+
+  el.help.replaceChildren(tabs, body, footer);
 }
 
 /** The page bar: which item each fixed category is fixed to. Without it a

web/style.css

@@ -736,6 +736,17 @@ td.total {
   font-weight: 700;
 }
 
+/* Sticky, so the paging keys stay in view however long the page scrolls. */
+.help-footer {
+  position: sticky;
+  bottom: -1.25rem;
+  margin: 0.75rem 0 0;
+  padding: 0.5rem 0;
+  border-top: 1px solid var(--line);
+  background: var(--bg);
+  color: var(--dim);
+}
+
 .help-body {
   display: grid;
   grid-template-columns: 16rem 1fr;