From cad914faab2cce9be595a9751251c43c57533735 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Sat, 19 Sep 2026 17:40:36 -0700 Subject: [PATCH] docs(repo-map): gotcha 42 corrected by the review Three corrections. The selection is reconciled by ADDING the offset to the render index, not subtracting it from the selection -- which is why there is no underflow path; and the test that earns its place uses nonzero, unequal offsets so it separates 'reconciles' from 'ignores' and from 'swaps'. The mirrored framing is kept honest by TWO tests: the round trip catches a rename, and an exhaustive match over the server's enums catches an addition the hand-written fixture list would miss. And the check-the-checker was itself broken -- it injected malformed JSON, so is_err() held for a syntax error and would have held against an enum that really did have the variant. Plus what a careful read of web/app.js established and what it could not: filed as improvise-jm5v. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EoW5BB87ZZdm96SvBTWawH --- context/repo-map.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/context/repo-map.md b/context/repo-map.md index 310729bf..bf64758f 100644 --- a/context/repo-map.md +++ b/context/repo-map.md @@ -215,11 +215,13 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text 41. **`Storage` is not a VFS, and that closes the SQLite question.** The trait is whole-document `read`/`write` bytes (gotcha 2, `invariant(storage_is_two_ops)`) — no offsets, seeking, handles or locking — so an `sqlite3_vfs`, which needs `xRead(offset,len)`/`xWrite`/`xTruncate`/`xLock`, has nothing to bind to. SQLite *can* still sit behind it, as an in-memory database serialized to a blob (`rusqlite` `serialize`/`deserialize_read_exact`, proven with a probe before declining — declining on a capability that actually exists would have been the wrong decline). It is worth nothing: measured on 18,000 cells the SQLite container is **22% larger** than `.improv`, **4.7× larger gzipped**, ~**160×** larger at the floor (24 KB against ~150 bytes), and still rewritten whole on every save, so the one dimension SQLite normally wins — incremental page writes — is unavailable. improvise-9ix was declined on that evidence. Two things generalise past SQLite. **A second serialization format is a second implementation of the grammar**, obliged to track `improv.pest` forever; and **its own round-trip test cannot detect drift from `.improv`**, because it never compares the two — that missing cross-format equality test is what made the ticket look cheap. If a structured-query story is ever wanted it is a one-way `export_sqlite` beside `export_csv`, native-only and feature-gated: `libsqlite3-sys`' C build fails under this repo's nix cc-wrapper for `wasm32-unknown-unknown`, which would break the `wasm` CI gate the day it landed. -42. **The browser client's render decisions are made in Rust, and its socket framing is duplicated on purpose.** `improvise-wasm-client/src/render.rs` builds a `RenderModel` in which every question with a *silent* wrong answer is already answered, because the DOM half (`web/app.js`) is untested JavaScript and a wrong answer there shows up as a wrong pixel rather than a failing test. Three such answers. **`editable` is a fact about the `CellId` variant** — `Record` only, per ADR 002 — and the trap is the third case: `None` means "no projected cell", **not** "aggregate", so conflating them either hides a cell or offers a write the type system exists to make unconstructible. **The selection is placed by subtracting the scroll offset**, because `GridCache` is viewport-indexed while `ViewState::selected` is absolute; omitting the subtraction looks right until the first scroll, and a selection outside the window highlights nothing. **The category panel is `viewmodel::flatten_cat_tree`**, never a second walk of `CategoryTreeCache` — that second walk was improvise-edp. The page is four string calls: `Client::viewport` (the handshake — the `Snapshot` seeds the *visible* cells, so one built at extent zero is an empty grid and a resize is a fan-out with nothing dirty), `on_frame` (one frame, one fold, arrival order — gotcha 33 holds *inside* a batch, so never coalesce), `render_model`, `typed_cell`. +42. **The browser client's render decisions are made in Rust, and its socket framing is duplicated on purpose.** `improvise-wasm-client/src/render.rs` builds a `RenderModel` in which every question with a *silent* wrong answer is already answered, because the DOM half (`web/app.js`) is untested JavaScript and a wrong answer there shows up as a wrong pixel rather than a failing test. Three such answers. **`editable` is a fact about the `CellId` variant** — `Record` only, per ADR 002 — and the trap is the third case: `None` means "no projected cell", **not** "aggregate", so conflating them either hides a cell or offers a write the type system exists to make unconstructible. **The selection is placed by reconciling the scroll offset**, because `GridCache` is viewport-indexed while `ViewState::selected` is absolute. `render.rs` does it by *adding* — `r + cursor.row_offset == sel_row` — rather than subtracting, which is why there is no underflow path and why a selection above the window simply never matches. Omitting the offset entirely looks right until the first scroll, so the test that earns its place uses **nonzero and unequal** offsets: at `selected=(5,1)`, `row_offset=4`, `col_offset=1` it separates "reconciles" from "ignores" *and* from "swaps the two offsets". **The category panel is `viewmodel::flatten_cat_tree`**, never a second walk of `CategoryTreeCache` — that second walk was improvise-edp. The page is four string calls: `Client::viewport` (the handshake — the `Snapshot` seeds the *visible* cells, so one built at extent zero is an empty grid and a resize is a fan-out with nothing dirty), `on_frame` (one frame, one fold, arrival order — gotcha 33 holds *inside* a batch, so never coalesce), `render_model`, `typed_cell`. - **`socket.rs` is not `wire.rs`, and there are three envelopes for two deployments.** `improvise-ws-server` speaks `Viewport`/`Command` and `Snapshot`/`Commands(Vec<_>)`, and has **no `Key` variant** — so key forwarding does not work over that socket, which is part of why improvise-bpq2 is open; a forwarded keypress is silently skipped by `next_client_msg`. `improvise-wasm-client/src/wire.rs` frames the *worker* deployment (improvise-tm6) with a third vocabulary again. The socket pair is mirrored rather than shared because **a native crate cannot be a wasm dependency** (tokio, tungstenite, `Box`), so `improvise-ws-server` is a **dev**-dependency — the same argument the crate already makes for `improvise-command` — and `the_socket_framing_matches_the_server` round-trips every variant through the server's own types in both directions, with `a_frame_named_the_other_envelopes_way_does_not_parse` as its check-the-checker (it renames `Commands`→`Command` and `Snapshot`→`Bootstrap`, the names the *sibling* envelope actually uses, and asserts neither parses). The real fix is `improvise-protocol` owning the framing: improvise-xks6. + **`socket.rs` is not `wire.rs`, and there are three envelopes for two deployments.** `improvise-ws-server` speaks `Viewport`/`Command` and `Snapshot`/`Commands(Vec<_>)`, and has **no `Key` variant** — so key forwarding does not work over that socket, which is part of why improvise-bpq2 is open; a forwarded keypress is silently skipped by `next_client_msg`. `improvise-wasm-client/src/wire.rs` frames the *worker* deployment (improvise-tm6) with a third vocabulary again. The socket pair is mirrored rather than shared because **a native crate cannot be a wasm dependency** (tokio, tungstenite, `Box`), so `improvise-ws-server` is a **dev**-dependency — the same argument the crate already makes for `improvise-command` — and `the_socket_framing_matches_the_server` round-trips every variant through the server's own types in both directions, kept honest by **two** tests, because one is not enough. `the_socket_framing_matches_the_server` round-trips every variant through the server's own types in both directions and catches a **rename** on either side — but its fixture list is written by hand, so it is blind to a variant added on the *server*; `every_server_variant_is_one_this_crate_mirrors` matches exhaustively over `ClientMsg`/`ServerMsg` so that addition is a compile error here instead of a surprise at the far end. - **What no test in this repo can see:** `web/app.js` has never been rendered. There is no browser and no headless DOM here; it is syntax-checked and every call it makes is confirmed present in the generated `.d.ts` with the right arity, and that is all. Click-to-edit, the `ResizeObserver` extent arithmetic and the CSS are unexercised. What *is* verified end to end is the Rust half: `tests/browser_round_trip.rs` drives improvise-1ey's stated scope against a real `Host` over a real `App` touching the client only through the four string calls. One deployment note that is not a bug: `examples/demo.improv` leaves `Customer`/`Date` on the None axis, so **every** cell is an aggregate and nothing is editable — correct behaviour, bad demo. Pick a model with no regular category on None. + Its check-the-checker, `a_frame_named_the_other_envelopes_way_does_not_parse`, renames `Commands`→`Command` and `Snapshot`→`Bootstrap` — the names the *sibling* envelope actually uses — and **was itself wrong until it was injected against**: the rewrite deleted the opening `[` along with the variant name, leaving `{"Command":{...}]}`, which is not JSON, so `is_err()` held for a *syntax* error and held identically against an enum that really did have a `Command` variant (proven by adding one). Two corrections generalise past this case: **assert the mutated input is still well-formed** before feeding it in, and **assert the error kind** — serde's `unknown variant` — not merely that an error occurred. A check that fails for a reason other than the one under test is `doctrine(check_the_checker)`'s own failure mode wearing the name of its cure. The real fix is `improvise-protocol` owning the framing: improvise-xks6. + + **What no test in this repo can see:** `web/app.js` has never been rendered. There is no browser and no headless DOM here; it is syntax-checked and every call it makes is confirmed present in the generated `.d.ts` with the right arity, and that is all. Click-to-edit, the `ResizeObserver` extent arithmetic and the CSS are unexercised. What a careful read *did* establish: the observer cannot feed back into itself (`#grid` is a flex child with `overflow: auto`, so `replaceChildren` cannot resize it), and `ROW_PX`/`COL_PX` under-estimate, so the worst case is a row of over-fetch. What it cannot establish, and is filed as improvise-jm5v: **a malformed frame is quiet, not loud** — `on_frame` returns a `Result`, wasm-bindgen throws it, and it is thrown inside an unguarded `message` listener, so it reaches the devtools console and nothing else; the socket has no `error` listener; `close()` has no re-entry guard; and **no `keydown` is registered at all**, so `on_key_event` and the whole `keys`/lockstep machinery are unreached from the browser. What *is* verified end to end is the Rust half: `tests/browser_round_trip.rs` drives improvise-1ey's stated scope against a real `Host` over a real `App` touching the client only through the four string calls. One deployment note that is not a bug: `examples/demo.improv` leaves `Customer`/`Date` on the None axis, so **every** cell is an aggregate and nothing is editable — correct behaviour, bad demo. Pick a model with no regular category on None. ---