feat(web): the title bar is painted, a click survives an editor, and the twin door is gone

improvise-5jna, improvise-0x9u, improvise-seik — three browser-page defects with
one shape: a thing that looks done from every check except running it.

improvise-5jna. `doc_name`/`doc_dirty`/`doc_stored_as` were projected by z754 and
painted by nothing. `render_surface.test.js` could not see it — it is a SUBSTRING
SEARCH over the page's source, so a field named by a function nothing calls reads
as painted, which is also how `model.minibuffer` shipped dead. So the guard is a
harness assertion that loads the page and reads `#doc-name`, proved by writing it
first (`element(s) not found`). The joining is `shell.js::documentTitle`, the
TUI's own `name (stored_as) [+]` minus the chrome; the `[+]` is a node test, not a
harness one, because this deployment persists on every accepted batch and clears
the flag in the same round trip, so the marker is transient. UNPAINTED is now
empty.

improvise-0x9u. From an EDITING state a cell click did nothing at all: the
mousedown moved focus, the input's blur ran `close`, `close` repainted,
`replaceChildren` destroyed the `<td>` the mousedown landed on, and no `click`
was ever dispatched. The old spec pressed Escape between the two clicks, so it
only ever clicked from a non-editing page — this repo's characteristic defect.

Worth recording: moving to `mousedown` is NECESSARY AND NOT SUFFICIENT, and the
first attempt failed the same assertion with the same symptom. Removing a focused
`<input>` fires `blur` SYNCHRONOUSLY, so `close` rebuilt the table from inside
itself and detached the `<td>` again. `close` therefore no longer repaints, the
`blur` arm checks `closed` first, and the handler `preventDefault()`s so no focus
change happens at all — otherwise the default action focuses `#grid` after
`input.focus()` and blurs the new editor. `close` restores the DRAWN text, since
nothing repaints behind it any more.

improvise-seik. `Client::viewport` was a wasm-bindgen door with no caller, kept
for a reason `web/app.js` falsified in the same wave. It is one letter from
`set_viewport` and fails SILENTLY — identical frame, forgotten window, every
cursor key an upstream black hole. Deleted, along with the exported `init()` free
function that duplicated `new Client()`. The guard is not the deletion:
`wasm_surface.test.js` now reads the generated `.d.ts` and requires every
declared method to be one `app.js` calls or to be named in `UNCALLED` with its
reason. Proved by injecting a spare `static viewport(...)` into the built `.d.ts`
and watching it fail by name.

Tests: 1405 Rust (unchanged), 27 node + 4 bundle (was 26 + 3), 24 chromium
harness (was 23).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
2026-09-20 20:37:44 -07:00
parent 8a559b2686
commit 628f43121d
12 changed files with 232 additions and 46 deletions
+58 -1
View File
@@ -1112,7 +1112,7 @@ reaching it from https means terminating TLS in front of it). From an https page
an explicit `ws://` is **upgraded** rather than passed through, since passing it
through can only produce a failure the user cannot act on.
### `set_viewport`, not `Client.viewport`
### `set_viewport`, and the twin that is gone
Same frame, but it also **records** the window. A client learns its window only
from what it announced — the row frame's own length is the window clipped to the
@@ -1121,6 +1121,23 @@ arithmetic matters. Without it the client cannot clamp, so every cursor key goes
upstream into `Host::handle`'s empty vec and the arrow keys silently do nothing
(improvise-bpq2).
There used to be a frame-only twin, `Client::viewport`, exported beside it and
called by nobody. Its doc said it was kept because `web/app.js` already spelled
it — and `web/app.js` called `set_viewport` in the same wave, so the reason was
false before it was read (improvise-seik). That matters more than an ordinary
unused function, because **the two are one letter apart and the wrong one fails
silently**: it sends an identical frame and simply forgets the window, which
turns every cursor key into the black hole above. It is deleted, along with the
exported `init()` free function that duplicated `new Client()`.
A deletion is not the guard, though — the next export with no caller arrives the
same way. `web/tests/bundle/wasm_surface.test.js` now reads the generated
`.d.ts` and requires every declared method to be one `app.js` calls, or to be
named in an `UNCALLED` table with its reason. `free` and `initSync` are
wasm-bindgen's own; `revision` is there because `on_frame` returns it. Proved by
injecting a spare `static viewport(...)` declaration into the built `.d.ts` and
watching it fail by name.
### `frameCost`: what the grid's own frame costs, in whole cells
One header row and one header column was right until improvise-anr9: the column
@@ -1216,6 +1233,46 @@ Neither arm leaves one armed.
1200 reads `"1,200"` and committing that stores the *string* `"1,200"`. Enter
removes the focused `<input>`, which fires `blur`, which calls `close` again:
harmless, but only by accident.
- **The title bar is three fields and one joining.** `doc_name`, `doc_dirty` and
`doc_stored_as` come off the cache `src/draw.rs::draw_title` folds; the joining
— `name (stored_as) [+]`, in that order — is `shell.js::documentTitle`, which
is the TUI's own spelling minus the `improvise ·` chrome and the right-hand
key strip. The three were projected by improvise-z754 and painted by nothing
for a wave, and `render_surface.test.js` could not see it: that check is a
**substring search**, so a field named by a function nothing calls reads as
painted. What sees it is `contract.spec.js`, which loads the page and asserts
the text (improvise-5jna). The marker itself is a node test rather than a
harness one, because the standalone deployment persists on every accepted
batch and clears the flag in the same round trip, so `[+]` is transient there.
### The pointer takes cells on `mousedown`
A cell's handler is `mousedown` with `preventDefault()`, not `click`, and the
reason is an ordering trap worth the paragraph (improvise-0x9u). From an
**editing** state a `click` never arrived at all: the mousedown moved focus, the
`<input>`'s `blur` ran `close`, `close` repainted, `replaceChildren` destroyed
the `<td>` the mousedown had landed on, and no `click` was ever dispatched. The
pointer was dead until the user pressed Escape or clicked outside the grid.
Three things had to move together, and each is load-bearing:
- **`mousedown`, so the handler runs before any focus change.**
- **`preventDefault()`, so no focus change happens at all.** Otherwise the
default action focuses `#grid` *after* the handler has called `input.focus()`
on the new editor, and blurs it immediately. The keyboard listener is on
`window`, so nothing needs `#grid` focused.
- **`close` no longer repaints, and `blur` checks `closed` first.** Removing a
focused `<input>` fires `blur` **synchronously**, so a `close` that repainted
from inside itself detached the very `<td>` the handler was about to open an
editor on. This is the same repair one level down, and it is why the first
attempt at the fix still failed: the `blur` arm re-entered and rebuilt the
table. `close` restores the **drawn** text rather than the seed, since nothing
repaints behind it any more.
The state the old spec could not see is the whole point: it pressed Escape
before clicking the second cell, so it only ever clicked from a non-editing
page — this repo's characteristic defect, a test that passes while the thing it
names is broken in a neighbouring state.
### The keyboard
+38
View File
@@ -2062,3 +2062,41 @@ done
The other two losses are deliberate: `pointer.spec.js`'s sidebar test was
superseded by the open-at-launch ruling, and the widgets file was merged into
this one.
#### "a click moves the selection from an editing state too"
improvise-0x9u, and the fault was injected by running this assertion against the
bundle as it stood: `cells.nth(4).locator("input")` came back **0**, with the
selection still on cell 0, exactly as the bug report measured in Chromium.
Two things about it are worth more than the assertion. First, **what it does not
do**: it never presses Escape between the two clicks. The sibling test does, and
that single line is why the property it names ("a click moves the selection") was
asserted for a whole wave while the neighbouring state was broken — the click
path from an *editing* page was never exercised.
Second, **the fix's own first attempt failed this same test**, which is the part
to remember. Moving the handler to `mousedown` is necessary and not sufficient:
`close` still repainted, and removing a focused `<input>` fires `blur`
**synchronously**, so `close` rebuilt the table from inside itself and detached
the `<td>` the handler was about to open an editor on. The assertion could not
tell that apart from the original bug — same symptom, different cause — so it is
the debug run rather than the test that separated them. `close` therefore no
longer repaints, and the `blur` arm checks `closed` before dismissing.
#### "the widgets the demo document fills are drawn, in the state that fills them"
The `#doc-name` row is improvise-5jna, and the reason it belongs in a **browser**
test rather than in `render_surface.test.js` is that the latter is a substring
search over the page's source: `doc_name` mentioned by a function nothing calls
reads as painted. That is how `model.minibuffer` once shipped dead, and how
z754's three title-bar fields sat projected and unpainted for a wave while the
static check was green. Proved by adding the assertion first: `element(s) not
found` for `#doc-name`.
The `[+]` marker is deliberately **not** asserted here. The standalone deployment
persists on every accepted batch and `effect::Save` clears the flag in the same
round trip (gotcha 45), so the marker is transient in a browser and any timing
assertion over it would be flaky rather than discriminating. Its spelling — the
order `name (stored_as) [+]`, the `null` `stored_as` writing no empty parens — is
`web/tests/shell.test.js`, over `documentTitle` directly.
+15 -26
View File
@@ -19,9 +19,8 @@ pub struct Client {
state: ClientState,
}
/// Allocate a client: an empty view slice and one empty cache per widget. A
/// client that draws before its bootstrap draws an empty grid, correctly.
#[wasm_bindgen]
/// An empty view slice and one empty cache per widget. Not exported: the
/// page's door is `new Client()` (improvise-seik).
pub fn init() -> Client {
Client::default()
}
@@ -55,14 +54,8 @@ impl Client {
serde_json::to_string(&message).ok()
}
/// The whole client state as JSON: the view slice, every widget cache, and
/// the revision it corresponds to.
#[wasm_bindgen]
pub fn render_state(&self) -> Result<String, JsError> {
self.state_json().map_err(|e| JsError::new(&e.to_string()))
}
/// The cheap half of [`Client::render_state`]: has anything moved?
/// Has anything moved? The cheap question, and the one `on_frame` already
/// answers by returning it.
#[wasm_bindgen]
pub fn revision(&self) -> u64 {
self.state.revision
@@ -71,24 +64,17 @@ impl Client {
// The transport surface: a handful of string calls and the whole harness a
// page needs. See `context/code/clients.md`.
/// The handshake frame, associated rather than a method. **Prefer
/// [`Client::set_viewport`]** — this one forgets the window (clients.md).
/// The handshake frame, *and* the record of what this client can show.
/// The **only** door, and why that matters: `context/code/clients.md`.
#[wasm_bindgen]
pub fn viewport(rows: usize, cols: usize) -> String {
pub fn set_viewport(&mut self, rows: usize, cols: usize) -> String {
self.state.window = (rows, cols);
serde_json::to_string(&Self::viewport_frame(rows, cols))
// Two `usize` in a unit variant; unreachable, and an empty frame
// is skipped by the server rather than crashing the tab.
.unwrap_or_default()
}
/// The handshake frame, *and* the record of what this client can show —
/// which is not derivable from anything the host sends (clients.md).
#[wasm_bindgen]
pub fn set_viewport(&mut self, rows: usize, cols: usize) -> String {
self.state.window = (rows, cols);
Self::viewport(rows, cols)
}
/// Fold one frame, in arrival order, one at a time; returns the new
/// revision. Never reorder or coalesce — a split batch loses cells.
#[wasm_bindgen]
@@ -316,12 +302,15 @@ mod tests {
);
}
/// The handshake frame is the same either way; `set_viewport` also keeps
/// the window, which is what lets a cursor key resolve at all.
/// The handshake frame, and the window it records — one call, because a
/// second door that sent the frame and forgot the window was silent.
#[test]
fn set_viewport_sends_the_same_frame_and_keeps_the_window() {
fn set_viewport_sends_the_handshake_frame_and_keeps_the_window() {
let mut client = init();
assert_eq!(client.set_viewport(24, 8), Client::viewport(24, 8));
assert_eq!(
serde_json::from_str::<ClientFrame>(&client.set_viewport(24, 8)).unwrap(),
Client::viewport_frame(24, 8)
);
assert_eq!(client.state().window, (24, 8));
}
@@ -98,10 +98,10 @@ fn a_browser_client_seeds_types_a_number_and_sees_it_come_back() {
// The handshake. The extent comes first because the snapshot seeds the
// *visible* cells: one built at extent zero is an empty grid.
let handshake: ClientFrame = serde_json::from_str(&Client::viewport(10, 10))
let handshake: ClientFrame = serde_json::from_str(&client.set_viewport(10, 10))
.expect("the viewport frame must be one the server reads");
let ClientFrame::Viewport { rows, cols } = handshake else {
panic!("Client::viewport must produce a Viewport frame");
panic!("set_viewport must produce a Viewport frame");
};
let (id, snapshot) = host.connect(rows, cols);
deliver(&mut client, &[snapshot]);
@@ -305,7 +305,7 @@ fn a_resize_reprojects_the_grid_at_the_new_extent() {
"a one-row viewport is seeded with one row"
);
let frames = send(&mut host, id, &Client::viewport(10, 10));
let frames = send(&mut host, id, &client.set_viewport(10, 10));
deliver(&mut client, &frames);
assert_eq!(
client.model().rows.len(),
+30
View File
@@ -26,6 +26,15 @@ test.describe("every field the render model projects reaches the screen", () =>
await expect(page.locator("#grid tbody th").first()).toHaveText("Espresso");
await expect(page.locator("#grid tbody td").first()).not.toHaveText("");
// `doc_name`, `doc_stored_as`: the title bar. improvise-z754 projected
// these and nothing painted them for a wave, which `render_surface`'s
// substring search cannot see — only a loaded page can (improvise-5jna).
// The dirty marker's own spelling is `web/tests/shell.test.js`: this
// deployment persists on every accepted batch, so `[+]` is transient.
await expect(page.locator("#doc-name")).toHaveText(
"Corner Coffee — 2025 (demo.improv)",
);
// `mode`, `hint`, `view_name`, `status`: the header's four facts.
await expect(page.locator("#mode")).toHaveText("NORMAL");
await expect(page.locator("#hint")).not.toHaveText("");
@@ -212,4 +221,25 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
await expect(page.locator("#mode")).toHaveText("FORMULAS");
await expect(page.locator("#formulas li.selected")).toHaveCount(1);
});
// improvise-0x9u. The neighbouring state the assertion above cannot see: it
// presses Escape first, so it only ever clicks from a NON-editing page.
test("a click moves the selection from an editing state too", async ({ page }) => {
await openApp(page);
const cells = page.locator("#grid tbody td");
await cells.nth(0).click();
await expect(cells.nth(0).locator("input")).toHaveCount(1);
// No Escape between the two clicks — that is the whole test. The blur used
// to repaint and `replaceChildren` destroyed the <td> the mousedown landed
// on, so no click event was ever dispatched and the pointer was dead.
await cells.nth(4).click();
await expect(cells.nth(4).locator("input")).toHaveCount(1);
await expect(cells.nth(0).locator("input")).toHaveCount(0);
await page.keyboard.press("Escape");
await expect(cells.nth(4)).toHaveClass(/selected/);
await expect(page.locator("#grid tbody td.selected")).toHaveCount(1);
});
});
+35 -8
View File
@@ -7,6 +7,7 @@ import {
HANDSHAKE_TIMEOUT_MS,
ROW_PX,
createShell,
documentTitle,
extentOf,
frameCostOf,
gridPlan,
@@ -35,6 +36,7 @@ const el = {
help: document.getElementById("help"),
status: document.getElementById("status"),
viewName: document.getElementById("view-name"),
docName: document.getElementById("doc-name"),
};
await init();
@@ -60,8 +62,8 @@ function sendViewport() {
const box = el.grid.getBoundingClientRect();
const metrics = { rowPx: ROW_PX, colPx: COL_PX };
const { rows, cols } = extentOf(box, frameCost, metrics);
// `client.set_viewport`, NOT the associated `Client.viewport`: the same
// frame, but it also RECORDS the window. Why that matters: clients.md.
// `set_viewport` RECORDS the window as well as framing it; the frame-only
// twin beside it is deleted, so there is nothing to call by mistake (seik).
transport.send(client.set_viewport(rows, cols));
}
@@ -162,6 +164,13 @@ function repaint() {
paintMinibuffer(model);
shell.setModel(model.status);
el.viewName.textContent = model.view_name;
// The title bar, off the cache `src/draw.rs::draw_title` folds — one
// derivation, both front ends (improvise-z754, improvise-5jna).
el.docName.textContent = documentTitle(
model.doc_name,
model.doc_dirty,
model.doc_stored_as,
);
el.grid.replaceChildren(gridTable(model));
// The flag is the client's answer, not the page's: `C` toggles it and the
// TUI draws the panel on exactly the same condition (improvise-wsaw).
@@ -299,7 +308,12 @@ function gridTable(model) {
// Every cell moves the selection; an editable one also opens its editor,
// which is the pointer's `i`. Not repainted in that branch: `edit` has
// set `editing`, and a repaint would take the <input> away.
td.addEventListener("click", () => {
// On MOUSEDOWN, and defaulted away: from an editing state the input's
// blur repainted first and this <td> was gone before `click` could fire
// (improvise-0x9u). See context/code/clients.md.
td.addEventListener("mousedown", (event) => {
event.preventDefault();
if (editing) editing.close(false);
const frame = client.click_cell(cell.row, cell.col);
if (frame) transport.send(frame);
if (cell.editable) edit(td, cell.row, cell.col, cell.editText);
@@ -314,10 +328,12 @@ function gridTable(model) {
* back is the server's, never the string typed here. */
function edit(td, row, col, seed) {
if (editing) return;
editing = { row, col };
// `seed` is `RenderCell::edit_text` -- the STORED value, not the drawn one:
// `td.textContent` has been through the number format.
const original = seed;
// What was on the screen, kept so closing can put it back: `close` no longer
// always repaints, so restoring `seed` would leave `1200` where `1,200` was.
const drawn = td.textContent;
const input = document.createElement("input");
input.value = original;
td.replaceChildren(input);
@@ -327,6 +343,8 @@ function edit(td, row, col, seed) {
// Enter removes the focused <input>, which fires `blur`, which calls
// `close` again. Harmless before, but only by accident.
let closed = false;
// `close` does NOT repaint: a caller about to open another editor on a live
// <td> must not have the table rebuilt under it (improvise-0x9u).
const close = (commit) => {
if (closed) return;
closed = true;
@@ -337,19 +355,28 @@ function edit(td, row, col, seed) {
// follows restores whatever the server now says is there.
if (frame) transport.send(frame);
}
td.replaceChildren(original);
td.replaceChildren(drawn);
painted = -1;
};
const dismiss = (commit) => {
close(commit);
repaint();
};
editing = { row, col, close };
input.addEventListener("keydown", (e) => {
if (e.key === "Enter") close(true);
else if (e.key === "Escape") close(false);
if (e.key === "Enter") dismiss(true);
else if (e.key === "Escape") dismiss(false);
// The editor owns the keyboard while it is open; the page-level handler
// below must not also see these.
e.stopPropagation();
});
input.addEventListener("blur", () => close(false));
// Removing the focused <input> fires `blur` SYNCHRONOUSLY, so a caller that
// already closed this editor must not have the table rebuilt from inside its
// own `close` — that is what detached the <td> in improvise-0x9u's repair.
input.addEventListener("blur", () => {
if (!closed) dismiss(false);
});
}
/** A side panel. An empty row list is what says it is closed — the page asks
+1
View File
@@ -34,6 +34,7 @@
-->
<header>
<h1>improvise</h1>
<span id="doc-name"></span>
<span id="view-name"></span>
<span id="mode"></span>
<span id="hint"></span>
+6
View File
@@ -16,6 +16,12 @@ export function composeStatus(shell, model) {
return [shell, model].filter(Boolean).join(" · ");
}
/** The title bar's one line, in the order and spelling `src/draw.rs::draw_title`
* uses. The three facts are the host's; only the joining is here (clients.md). */
export function documentTitle(name, dirty, storedAs) {
return `${name}${storedAs ? ` (${storedAs})` : ""}${dirty ? " [+]" : ""}`;
}
/** The collapse glyph. `collapsed` is the fact, decided by the view and
* carried on the frame; only this spelling is the page's (gotcha 26). */
export function indicator(collapsed) {
+4
View File
@@ -49,6 +49,10 @@ header h1 {
color: var(--accent);
}
#doc-name {
opacity: 0.85;
}
#mode {
color: var(--accent);
}
+23
View File
@@ -86,6 +86,29 @@ test("the page still reaches the client through a handful of calls", () => {
);
});
/// Methods on `Client` the page does not call, each with the reason it is
/// still exported. A door with no caller and no entry here is improvise-seik:
/// the dangerous kind, because calling the wrong one of a pair is silent.
const UNCALLED = {
free: "wasm-bindgen's own destructor, not ours to remove",
initSync: "wasm-bindgen's own module loader, beside the default export",
revision: "on_frame returns it; a page that stores it can skip a repaint",
};
test("the bundle exports no client door the page has no reason to hold", () => {
const declared = declarations("pkg/improvise_wasm_client.d.ts");
const called = new Set(callsOn("app.js", "client").map((c) => c.name));
// `constructor` is `new Client()`, which `callsOn` cannot see as a call.
const spare = [...declared.keys()].filter(
(name) => name !== "constructor" && !called.has(name),
);
assert.deepEqual(
spare.sort(),
Object.keys(UNCALLED).sort(),
"an exported method nothing calls — delete it, or add it to UNCALLED with the reason",
);
});
test("every worker call web/worker.js makes is declared, with that arity", () => {
const declared = declarations("worker-pkg/improvise_worker_server.d.ts");
const calls = callsOn("worker.js", "server");
+4 -8
View File
@@ -17,14 +17,10 @@ const code = (rel) =>
/// Fields the page deliberately does not read, each with the reason. A field
/// belongs here only with an argument; the list is the debt, written down.
// improvise-z754 put the title bar's facts in one shared cache and taught
// `src/draw.rs::draw_title` to fold it; this page never learned to. That is
// improvise-5jna, open, and this list is what stops it being forgotten.
const UNPAINTED = {
doc_name: "improvise-5jna: the title bar is drawn by the TUI and not here",
doc_dirty: "improvise-5jna",
doc_stored_as: "improvise-5jna",
};
// Empty since improvise-5jna painted the title bar. Note what this check CANNOT
// see, and why the harness asserts the painted text too: it is a substring
// search, so a field named by a function nothing calls reads as painted.
const UNPAINTED = {};
/// The `pub` fields of `RenderModel`, read off the Rust that defines it.
function renderModelFields() {
+15
View File
@@ -7,6 +7,7 @@ import assert from "node:assert/strict";
import {
composeStatus,
createShell,
documentTitle,
extentOf,
frameCostOf,
indicator,
@@ -160,3 +161,17 @@ test("the collapse glyph points right when collapsed", () => {
assert.equal(indicator(true), "▶");
assert.equal(indicator(false), "▼");
});
test("the title bar spells the document the way the TUI's own title does", () => {
// `src/draw.rs::draw_title`: name, then what it is stored as in parens, then
// the dirty marker. The harness cannot pin `[+]` — the standalone deployment
// persists on every accepted batch, so the marker is transient there.
assert.equal(
documentTitle("Corner Coffee", true, "demo.improv"),
"Corner Coffee (demo.improv) [+]",
);
assert.equal(documentTitle("Corner Coffee", false, "demo.improv"), "Corner Coffee (demo.improv)");
// A host with nowhere to write it: `stored_as` is `None` over the wire, which
// arrives as `null`, and an empty pair of parens would be a lie about storage.
assert.equal(documentTitle("Untitled", true, null), "Untitled [+]");
});