diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 77ccc651..e605a8f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,113 @@ env: CARGO_TERM_COLOR: always jobs: + # --------------------------------------------------------------------------- + # The one place wasm-pack runs (improvise CI dedup). + # + # Before this job there were two: `pages-build` ran `web/build.sh`, and the + # browser harness ran `harness/build.sh`, which CALLS `web/build.sh`. The two + # wasm-pack builds were therefore done twice per run, in parallel jobs, with + # separate rust-cache keys (`add-job-id-key` is on by default, so `browser` + # and `pages-build` never shared a target dir). Measured on run 35535225637 + # (push to main, caches warm): 68s inside `pages-build`, and the same 68s + # inside the browser job's 110s build step. + # + # Everything downstream now DOWNLOADS. The invariant that buys is artifact + # identity: the harness and Pages are served bytes from one wasm-pack run, + # so "the harness runs against the tree Pages publishes" (improvise-vvl3) is + # true by construction rather than because two scripts agree. + # + # The native binary is deliberately NOT built here. Nothing downstream + # consumes one — `cargo test --workspace` in the `test` job builds its own, + # for its own target dir and its own cache key, and a `needs:` on this job + # would serialise a 109s job behind a 126s one for no artifact at all. + build: + name: Build all bundles + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: dtolnay/rust-toolchain@stable + with: + targets: wasm32-unknown-unknown + # Two workspaces, because `harness/opfs-contract` carries an empty + # `[workspace]` table and so has its own target dir, which the default + # single-workspace configuration never cached (it recompiled its whole + # dependency tree every run). + - uses: Swatinem/rust-cache@v2 + with: + workspaces: | + . -> target + harness/opfs-contract -> target + # Pinned release binary rather than `cargo install` (a three-minute + # source build) or a third-party action. wasm-pack runs its own bundled + # `wasm-opt`, which is why improvise-wasm-client's Cargo.toml names + # `--enable-bulk-memory`: without it the optimizer rejects the + # `memory.copy` LLVM emits regardless. + # v0.15.0 is what `nix develop` supplies, and the bundle was built and + # measured against it locally (346 KB, 120 KB gzipped). Pinning the same + # version is what makes that local run evidence about this one. + - name: Install wasm-pack + env: + WASM_PACK_VERSION: v0.15.0 + run: | + set -eu + url="https://github.com/rustwasm/wasm-pack/releases/download/${WASM_PACK_VERSION}/wasm-pack-${WASM_PACK_VERSION}-x86_64-unknown-linux-musl.tar.gz" + curl -sSfL "$url" | tar -xz --strip-components=1 -C /usr/local/bin \ + "wasm-pack-${WASM_PACK_VERSION}-x86_64-unknown-linux-musl/wasm-pack" + wasm-pack --version + + # Both assemblies below generate the prose site with node. The runner + # image ships one, but pinning it is what makes a local run evidence. + - uses: actions/setup-node@v4 + with: + node-version: "24" + + # The same script a developer runs locally, so there is one answer to + # "how is the bundle built" rather than two that can drift. It builds all + # three wasm bundles (client, worker, opfs-contract) and assembles + # `harness/serve` — the deployed arrangement plus /opfs/. + - name: Build the bundles and the harness tree + run: ./harness/build.sh + + # Re-assembles from the pkg dirs the step above already produced — no + # wasm-pack runs here. A second assembly rather than a copy of + # `harness/serve` because the cache-bust token differs: the harness tree + # is stamped `harness` and the published site the commit SHA, which is + # what makes a returning visitor pick up a fresh bundle (improvise-9x6). + - name: Assemble the site + run: ./web/assemble.sh site "$GITHUB_SHA" + + - run: find site -type f | sort + - run: du -sh site + + # The tree the browser harness serves. Uploaded whole rather than + # reassembled downstream: a second assembly path is a second answer to + # "what does the harness serve", which is the thing improvise-vvl3 exists + # to prevent. + - name: Upload the harness tree + uses: actions/upload-artifact@v7 + with: + name: harness-serve + path: harness/serve + retention-days: 1 + include-hidden-files: true + + # For a job that needs the raw wasm-pack output rather than the + # assembled site — the Node bundle suite under `web/tests/bundle/`. + - name: Upload the wasm bundles + uses: actions/upload-artifact@v7 + with: + name: wasm-bundles + path: | + web/pkg + web/worker-pkg + retention-days: 1 + include-hidden-files: true + + - uses: actions/upload-pages-artifact@v5 + with: + path: site + test: name: Test and lint (host) runs-on: ubuntu-latest @@ -130,6 +237,37 @@ jobs: nix shell nixpkgs#trealla --command tpl prolog-sessions/kb.pl \ -g "(kb_consistent -> halt(0) ; (kb_report, halt(1)))" + # --------------------------------------------------------------------------- + # Lints the one crate the `test` job cannot reach. + # + # `harness/opfs-contract` carries an empty `[workspace]` table, so it is its + # own root and `cargo clippy --workspace` at the repo root never sees it. + # + # Its own job rather than a step inside `browser`, for two reasons. It took + # 37s of the browser job's 263s on run 35535225637 while gating nothing the + # harness needs — a lint failure and a harness failure are different reports, + # and here the lint failure wore the harness's name. And `cargo fmt --all` + # from inside that directory formats the WHOLE outer workspace (114 files, + # measured) rather than the one crate, because cargo-fmt's `--all` walks past + # the nested workspace boundary — so a rustfmt violation anywhere in the repo + # failed the *Browser harness* job. `-p opfs-contract` scopes it to 1 file. + harness-lint: + name: Lint the harness crate + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: dtolnay/rust-toolchain@stable + with: + targets: wasm32-unknown-unknown + components: clippy, rustfmt + - uses: Swatinem/rust-cache@v2 + with: + workspaces: harness/opfs-contract -> target + - working-directory: harness/opfs-contract + run: | + cargo fmt -p opfs-contract -- --check + cargo clippy --target wasm32-unknown-unknown --all-targets -- -D warnings + # --------------------------------------------------------------------------- # The browser harness (improvise-vvl3, absorbing improvise-xf1). # @@ -139,55 +277,64 @@ jobs: # by reading or by a person pressing keys, neither of which is repeatable. # # It can fail the build, deliberately: a harness nobody runs is worse than - # none, because it reads as coverage. Playwright's browser download is ~80 MB - # and takes well under a minute on ubuntu-latest, which is affordable next to - # the two wasm-pack builds this job already needs. + # none, because it reads as coverage. + # + # It now needs NO Rust toolchain: the tree it serves is downloaded from + # `build`, which is what makes "the harness runs against the tree Pages + # publishes" (improvise-vvl3) a fact about bytes rather than about two + # scripts agreeing. The download is `harness/serve` whole, so there is still + # exactly one place that arrangement is decided — `harness/build.sh`. browser: name: Browser harness (OPFS contract + shell) + needs: build runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - - uses: dtolnay/rust-toolchain@stable - with: - targets: wasm32-unknown-unknown - # Named rather than assumed: this job lints harness/opfs-contract, - # which the `test` job's workspace-wide fmt and clippy cannot reach. - components: clippy, rustfmt - - uses: Swatinem/rust-cache@v2 - uses: actions/setup-node@v4 with: node-version: 22 - - name: Install wasm-pack - env: - WASM_PACK_VERSION: v0.15.0 - run: | - set -eu - url="https://github.com/rustwasm/wasm-pack/releases/download/${WASM_PACK_VERSION}/wasm-pack-${WASM_PACK_VERSION}-x86_64-unknown-linux-musl.tar.gz" - curl -sSfL "$url" | tar -xz --strip-components=1 -C /usr/local/bin \ - "wasm-pack-${WASM_PACK_VERSION}-x86_64-unknown-linux-musl/wasm-pack" - wasm-pack --version - # Not a workspace member (it only builds for wasm32), so the `test` job's - # workspace-wide fmt and clippy do not reach it. - - name: Lint the harness crate - working-directory: harness/opfs-contract - run: | - cargo fmt --all -- --check - cargo clippy --target wasm32-unknown-unknown --all-targets -- -D warnings - name: Install the harness working-directory: harness run: npm ci + # Keyed on the resolved @playwright/test version, so a version bump is a + # miss and anything else is a hit. The download was 50s on run + # 35535225637 and is pure I/O — nothing about it depends on this commit. + - name: Playwright version + id: pw + working-directory: harness + run: echo "v=$(node -p "require('@playwright/test/package.json').version")" >> "$GITHUB_OUTPUT" + - uses: actions/cache@v4 + id: pw-cache + with: + path: ~/.cache/ms-playwright + key: playwright-${{ runner.os }}-${{ steps.pw.outputs.v }} # All three, because improvise-xf1's acceptance criterion names Chrome, # Firefox and Safari and `createSyncAccessHandle` is implemented # separately by each engine. WebKit is installed even though its OPFS is # absent today: the spec skips on a runtime probe, so the day that build # gains OPFS the check starts running without a CI change. + # `--with-deps` installs apt packages OUTSIDE the cached directory, so it + # runs on a cache hit too; only the browser download is skipped. - name: Install browsers working-directory: harness run: npx playwright install --with-deps chromium firefox webkit - # Builds both site bundles, the harness's own wasm bundle, and assembles - # `harness/serve` — the same tree `pages-build` publishes, plus /opfs/. - - name: Build the harness - run: ./harness/build.sh + # The tree `build` assembled, not one this job rebuilds. Same bytes as + # the Pages artifact, modulo the cache-bust token `web/assemble.sh` + # stamps — the harness tree is stamped `harness` and the site the commit + # SHA, which was already true when both jobs built their own. + - name: Download the harness tree + uses: actions/download-artifact@v7 + with: + name: harness-serve + path: harness/serve + # `web/pkg` and `web/worker-pkg` as wasm-pack left them, for the .d.ts + # the step below reads. The artifact's root is `web/`, so this restores + # both directories at the paths the test resolves relative to itself. + - name: Download the wasm bundles + uses: actions/download-artifact@v7 + with: + name: wasm-bundles + path: web # The JS↔wasm boundary: every call web/app.js makes is declared by the # bundle, with that arity. It needs the bundle and nothing else, so it # runs here rather than in the `js` job, and before Playwright — a @@ -207,12 +354,13 @@ jobs: # --------------------------------------------------------------------------- # GitHub Pages delivery (improvise-d31). # - # Two jobs on purpose. `pages-build` runs on every push and pull request, so - # a change that breaks `web/build.sh` or the bundle fails on the PR that made - # it — the build is a gate. `pages-deploy` runs only on a push to main and - # `needs` the check jobs above, so nothing that failed `cargo test`, the - # wasm32 gate or the browser harness can reach the published site. The deploy - # depends on those checks rather than repeating them (improvise-d31 AC 2). + # The artifact is uploaded by `build`, which runs on every push and pull + # request, so a change that breaks `web/build.sh` or the bundle fails on the + # PR that made it — the build is a gate. `pages-deploy` runs only on a push + # to main and `needs` the check jobs, so nothing that failed `cargo test`, + # the wasm32 gate or the browser harness can reach the published site. The + # deploy depends on those checks rather than repeating them (improvise-d31 + # AC 2). # # Not path-filtered, deliberately. The bundles are `improvise-wasm-client` and # `improvise-worker-server` plus, between them, most of the workspace — the @@ -221,60 +369,15 @@ jobs: # that meant "skip when the bundle cannot have changed" would have to name # most of the workspace, and the failure mode of getting it wrong is a stale # site with no signal that it is stale. A redundant build is the cheaper error. - pages-build: - name: Browser bundle + static site - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v7 - - uses: dtolnay/rust-toolchain@stable - with: - targets: wasm32-unknown-unknown - - uses: Swatinem/rust-cache@v2 - # Pinned release binary rather than `cargo install` (a three-minute - # source build) or a third-party action. wasm-pack runs its own bundled - # `wasm-opt`, which is why improvise-wasm-client's Cargo.toml names - # `--enable-bulk-memory`: without it the optimizer rejects the - # `memory.copy` LLVM emits regardless. - # v0.15.0 is what `nix develop` supplies, and the bundle was built and - # measured against it locally (346 KB, 120 KB gzipped). Pinning the same - # version is what makes that local run evidence about this one. - - name: Install wasm-pack - env: - WASM_PACK_VERSION: v0.15.0 - run: | - set -eu - url="https://github.com/rustwasm/wasm-pack/releases/download/${WASM_PACK_VERSION}/wasm-pack-${WASM_PACK_VERSION}-x86_64-unknown-linux-musl.tar.gz" - curl -sSfL "$url" | tar -xz --strip-components=1 -C /usr/local/bin \ - "wasm-pack-${WASM_PACK_VERSION}-x86_64-unknown-linux-musl/wasm-pack" - wasm-pack --version - - # The same script a developer runs locally, so there is one answer to - # "how is the bundle built" rather than two that can drift. - - name: Build the wasm bundle - run: ./web/build.sh - - # assemble.sh generates the prose site with node. The runner image ships - # one, but pinning it here is what makes a local run evidence about this. - - uses: actions/setup-node@v4 - with: - node-version: "24" - - # `web/assemble.sh`, not an inline copy of it: the browser harness job - # below assembles the same tree, and a harness that runs against a - # different arrangement of files is testing something nobody deploys. - - name: Assemble the site - run: ./web/assemble.sh site "$GITHUB_SHA" - - - run: find site -type f | sort - - run: du -sh site - - - uses: actions/upload-pages-artifact@v5 - with: - path: site - + # + # On improvise-864a (concurrent merges racing on one Pages environment, the + # superseded run failing "Found 0 artifacts"): building once shortens the gap + # between upload and deploy — 182s before, ~115s after, on the run 35535225637 + # shape — so the window in which a superseding run can expire this run's + # artifact is smaller. It does not close the race; it does not widen it. pages-deploy: name: Deploy to GitHub Pages - needs: [test, wasm, js, browser, pages-build] + needs: [test, wasm, js, browser, harness-lint, build] if: github.event_name == 'push' && github.ref == 'refs/heads/main' runs-on: ubuntu-latest permissions: diff --git a/AGENTS.md b/AGENTS.md index f49eb61e..803c4157 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -191,6 +191,17 @@ exact-frame characterization tests that were the PR's only evidence the migration changed nothing. Two right findings, different costs. File the declined one. +**Under merge-as-soon-as-green, a review round can land after its own merge.** +GitHub deletes the branch when a PR merges, so a fix push to the old branch name +**silently succeeds and accomplishes nothing** — it re-creates a branch `main` +will never see, CI runs on it, and the commit is nowhere. The tell is the push +printing `* [new branch]` where you expected an `old..new` range. It happened on +#51: the third review round completed four minutes after the merge, and the +correction it prompted went to a re-created `wave/b-projection-gaps`. The repair +is to branch from `gh/main`, cherry-pick, open a follow-up PR, and delete the +stray re-creation. Check `gh pr view --json state` *before* pushing a fix to +a PR you have not looked at in a few minutes, rather than after. + Merge only once Copilot passes. Prefer a **merge commit** — `AGENTS.md` says merges over rebases, and a wave's commits are individually meaningful. diff --git a/README.md b/README.md index b7ee5844..a9f61f4c 100644 --- a/README.md +++ b/README.md @@ -180,7 +180,7 @@ improvise script setup.txt -f model.improv ## In the browser The browser page is at ****, -built and published from `main` by the `pages-build` / `pages-deploy` jobs in +built and published from `main` by the `build` / `pages-deploy` jobs in `.github/workflows/ci.yml`. A deploy only happens after `cargo test`, the wasm32 check and the Playwright browser harness have passed. diff --git a/context/code/command.md b/context/code/command.md index e72bf073..27cb901f 100644 --- a/context/code/command.md +++ b/context/code/command.md @@ -79,8 +79,7 @@ authoritative `App`. A host that wants to apply a client's batch must face the door a host may use: checking per command is the lost update improvise-955n fixed. It is now `#[cfg(test)]` (improvise-s17o), which makes "no production caller" a fact the compiler keeps rather than a note verified at one commit, and -leaves `apply_checked` as the one door. `context/design-principles.md` §5 names -`Sessions::apply` as "dispatch — the one door"; read that as `apply_checked`. +leaves `apply_checked` as the one door. ### `Checked` — a wire `Command` the authoritative state has answered for diff --git a/context/design-principles.md b/context/design-principles.md index 1900164e..2e3a048d 100644 --- a/context/design-principles.md +++ b/context/design-principles.md @@ -226,7 +226,7 @@ The pieces already exist and are worth naming as what they are: | action | `Command` / `ModelCommand` (`improvise-protocol`) | | reducer | `reduce_view`, and `WidgetCache::apply` per widget | | store | `ViewState` + the per-widget caches | -| dispatch | `Sessions::apply` — the one door (gotcha 36) | +| dispatch | `Sessions::apply_checked` — the one door (gotcha 36) | So the question to ask of any code in a front end is: **would a third front end have to write this again?** If yes, it is behaviour and it belongs in the shared @@ -303,6 +303,16 @@ candidate for the store. A ``` fence in a doc comment is a doctest — a running test, not prose — and is exempt. +- **And an argument must not live only in a commit message.** A message is + write-once and unauditable: it cannot be corrected without rewriting merged + history, and no check in this repo can read one — the ID audit greps + `context`, `README.md`, `AGENTS.md`, `CLAUDE.md`, `crates`, `src`, `tests`, + `examples`, `web`, `harness` and `docs`, and a commit message is in none of + them. So a reason recorded only there is a reason nothing can ever find wrong. + When one turns out to be wrong, correct `context/` and say in `context/` that + the message is superseded; that is the fix rather than a compromise. PR #51's + `7d25762` is the worked example, corrected by #56. + --- ## 7. Testing @@ -336,6 +346,37 @@ catches that: every `### name` heading must name a test that exists. It checks one direction only, deliberately — a test may have no evidence (most do not), but evidence naming a test nobody can find is a dangling explanation. +### E2E pins the boundaries; unit tests work between them + +**E2E and integration tests are rare and pin *boundary contracts*. Unit tests +*assume* those contracts and test between the boundaries** (user, 2026-09-20). + +So the question to ask of a slow test is never "what feature does this cover" — +it is **"which boundary is this, and what is the contract at it?"** The answer +changes what you write. Four browser widgets had never worked — the tile bar, +the view panel, the formula panel, and the category sidebar ignoring +`category_panel_open` — and all four were **one** boundary failure: a +`RenderModel` field with no element painting it. Four per-widget Playwright +specs cost four slow tests and would still have missed the fifth; a single +contract assertion that *every field the render model exposes is painted* +catches all four and the next one. `harness/tests/opfs.spec.js` is the cleanest +instance of the right shape already in the tree: it runs the **storage +contract**, not a feature. + +Three corollaries, in the order they bite: + +- **A unit test may assume the contract rather than re-establishing it.** That + assumption is what makes it cheap, and it is only sound because something + slow pins the contract — so deleting the contract test silently invalidates + every unit test that leaned on it. +- **Never weaken a contract to make it cheap.** If the honest contract needs a + real browser, a real `Worker` or real OPFS, it stays slow. Moving it to a + faster harness that cannot see the thing it names is how a suite goes green + over a broken product. +- **Each slow test must say which contract it pins.** The difference between a + contract test and a feature test is invisible from the code, so it has to be + written down or the next person adds the fifth per-widget spec. + ### Use property tests judiciously Property tests (`proptest`) are for **invariants that must hold across all diff --git a/context/plan.md b/context/plan.md index 2abf1f68..5c430be3 100644 --- a/context/plan.md +++ b/context/plan.md @@ -239,7 +239,7 @@ Each cast should be embedded with `AsciinemaPlayer.create()` configured with `th ### 4.2 Enable GitHub Pages -**Not branch-and-folder.** Pages is served from the GitHub Actions artifact that `pages-build` / `pages-deploy` publish in `.github/workflows/ci.yml` (Settings → Pages → Source: GitHub Actions), and the tree is assembled by `web/assemble.sh`: `docs/index.html` and `docs/casts/` at the root, the browser app under `app/`. +**Not branch-and-folder.** Pages is served from the GitHub Actions artifact that `build` / `pages-deploy` publish in `.github/workflows/ci.yml` (Settings → Pages → Source: GitHub Actions), and the tree is assembled by `web/assemble.sh`: `docs/index.html` and `docs/casts/` at the root, the browser app under `app/`. Setting the source to branch `main` / folder `/docs` instead would serve `docs/` raw. That used to fail loudly — there was no `docs/index.html`, so it was a 404 — and since improvise-e61 it fails **quietly**: the landing page renders perfectly and its "Run improvise in your browser" link 404s, because `app/` exists only in the assembled tree. A page that looks right with a dead front door is worse than a page that is missing. diff --git a/context/repo-map.md b/context/repo-map.md index 4ed7fbbe..2a35525f 100644 --- a/context/repo-map.md +++ b/context/repo-map.md @@ -161,7 +161,7 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text **`prune_axis_items` takes a predicate, not a flag vector.** `prune_empty` hands it `impl Fn(usize) -> bool` over the *data* index (group headers consume no index). The domain is derived from `items` itself, so there is no parallel length a caller could get wrong — the earlier `&[bool]` + `debug_assert` pair could disagree between debug and release if a third caller ever sized `keep` from something other than `row_count()`/`col_count()` (improvise-n3wt). 2. **Effects never touch `std::fs`.** Every document read or write goes through `App::storage: Box` (improvise-fuj). The host picks the backend: `App::new` supplies `PhysicalFs`, `App::with_storage` takes any other — a browser worker will supply OPFS (improvise-i34), and tests use `MemoryFs`. `improvise-io` keeps path-taking wrappers (`save`, `load`, `export_csv`, `parse_csv_with_warnings`) for the native CLI, each delegating to a `_to`/`_from`/`_bytes` variant that takes the backend or the bytes. -3. **Every effect declares an `EffectKind`, and there is no default.** `ViewOnly` never reaches other clients; `Model(CellData | LayoutGeometry | WholeWorkbook)` does (improvise-mae). A new effect that forgets to classify itself fails to compile — that is deliberate, since the failure mode is silent: a mis-declared `ViewOnly` edit would simply never reach connected clients. Watch two traps: the *View* lives inside `model_state.workbook`, so "touches model_state" does NOT mean "is a model change" (axis/page/format are `LayoutGeometry`, cursor and scroll are `ViewOnly` — see improvise-8uz); and `Save`/`SaveAs`/`MarkDirty` touch `model_state` for bookkeeping only and are `ViewOnly`. `AddCategory`/`RemoveCategory` are the third trap and go the other way: they look like data edits, but both go through `Workbook`, whose `on_category_added`/`on_category_removed` rewrite **every** view's axis map, so they are `LayoutGeometry`. The two "an effect declaring X actually changes X" tests cannot separate them — a category change moves model data *and* view config, so it clears either bar; `structural_effects_declare_their_kind` pins the declaration itself. A third trap, now guarded: **`EffectKind` is too coarse to check a *mapping*.** `ui/reduce.rs::model_effects` sends each of 28 `ModelCommand` variants to one effect, and every inverse pair — `HideItem`/`ShowItem`, `AddItem`/`RemoveItem`, `CreateView`/`DeleteView`, `SetCell`/`ClearCell` — declares the *same* kind, so swapping three of them at once left the suite at 918 passing while three wire messages did the exact inverse of what they said. The mapping is name-for-name by construction, so `every_model_command_names_the_effect_of_the_same_name` asserts that, reading the type name off the effect's derived `Debug` (`Effect` is a trait object, so `type_name` is out of reach). A variant whose effect must be named something else is a variant that stopped being derived — it should fail there and be argued about. Their reach is wider than the name suggests: `Model::remove_category` also drops every formula targeting the category, which is why `Dirty::of(LayoutGeometry)` reports `formulas: true` — see gotcha 35. +3. **Every effect declares an `EffectKind`, and there is no default.** `ViewOnly` never reaches other clients; `Model(CellData | LayoutGeometry | WholeWorkbook)` does (improvise-mae). A new effect that forgets to classify itself fails to compile — that is deliberate, since the failure mode is silent: a mis-declared `ViewOnly` edit would simply never reach connected clients. Watch two traps: the *View* lives inside `model_state.workbook`, so "touches model_state" does NOT mean "is a model change" (axis/page/format are `LayoutGeometry`, cursor and scroll are `ViewOnly` — see improvise-8uz; and `SwitchView`, `ViewBack` and `ViewForward` moved from `LayoutGeometry` to `ViewOnly` with improvise-jytm, writing `view_state` and nothing else, which is what took the latter two off the wire — but a sound derivation is not a guard: only `SwitchView` had one, and review showed reclassifying either sibling back was silent across the whole suite); and `Save`/`SaveAs`/`MarkDirty` touch `model_state` for bookkeeping only and are `ViewOnly`. `AddCategory`/`RemoveCategory` are the third trap and go the other way: they look like data edits, but both go through `Workbook`, whose `on_category_added`/`on_category_removed` rewrite **every** view's axis map, so they are `LayoutGeometry`. The two "an effect declaring X actually changes X" tests cannot separate them — a category change moves model data *and* view config, so it clears either bar; `structural_effects_declare_their_kind` pins the declaration itself. A third trap, now guarded: **`EffectKind` is too coarse to check a *mapping*.** `ui/reduce.rs::model_effects` sends each of 28 `ModelCommand` variants to one effect, and every inverse pair — `HideItem`/`ShowItem`, `AddItem`/`RemoveItem`, `CreateView`/`DeleteView`, `SetCell`/`ClearCell` — declares the *same* kind, so swapping three of them at once left the suite at 918 passing while three wire messages did the exact inverse of what they said. The mapping is name-for-name by construction, so `every_model_command_names_the_effect_of_the_same_name` asserts that, reading the type name off the effect's derived `Debug` (`Effect` is a trait object, so `type_name` is out of reach). A variant whose effect must be named something else is a variant that stopped being derived — it should fail there and be argued about. Their reach is wider than the name suggests: `Model::remove_category` also drops every formula targeting the category, which is why `Dirty::of(LayoutGeometry)` reports `formulas: true` — see gotcha 35. 4. **Commands never mutate.** `&CmdContext` is read-only; return `Vec>`. If you want to touch `&mut App`, add or extend an Effect. 5. **`CellKey` is always sorted.** Use the constructors; equality and hashing rely on canonical form. 6. **No `Model::add_item`.** With a `Workbook` in hand use the facade: `wb.add_item("Region", "East")?` / `add_item_in_group` / `remove_item` (improvise-tvt). With only a `Model`, go through the category: `m.category_mut("Region").unwrap().add_item("East")`. @@ -175,7 +175,7 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text 14. **Keybindings are per-mode.** `ModeKey::from_app_mode()` resolves the mode; Normal + `search_mode=true` maps to `SearchMode`. Minibuffer modes bind Enter/Esc as `Binding::Sequence` so `clear-buffer` fires alongside commit/cancel. Hints are checked against the keymaps: `every_hint_key_resolves_in_its_mode` (`command/keymap.rs`) splits each `AppMode::hint_text()` and requires every key it names to resolve in that mode's map, because `only_quit_has_no_hint` asserts only that a hint is *present*. It checks which keys are named, not what they do. Its tokenizer knows the hint dialect explicitly (named keys, `Ctrl-`, `F/C/V` alternatives, character runs like `hjkl`/`[:]`, the em-dash prose tail, CommandMode's `:q` ex-commands) and panics on a token it cannot classify rather than skipping it. 15. **Registry names come from prototypes, not strings.** Every `register*` call reads the name off a prototype `Cmd` instance, so a typo is impossible (improvise-9cn) — there is no name literal in `registry.rs`. What types can't reach: four commands name themselves off a field (`ViewNavigate`, `TileAxisOp`, `ToggleGroupAtCursor`, `CommitAndAdvance`), so a prototype can name the wrong *variant* and still compile. That shows up as a **duplicate** registration, not a mismatch — the wrong-variant entry collides with its sibling and the sibling's name vanishes. `registered_names_are_unique` catches it; `every_bound_command_name_is_registered` catches the keymap side, and checks **both** halves of `dispatch`'s `registry.interactive(name, args, ctx).ok()?` — the name resolves *and* the args parse — because improvise-s13 moved `enter-mode ["export-prompt"]`/`["formula-edit"]` out of the nullary class, where a typo'd mode name compiles into a dead key. It builds the `CmdContext` in each keymap's own `AppMode`, since `commit-buffer item` reads `ctx.mode`. 16. **`effect_cmd!` macro** wraps a pre-built effect as a parseable command. Use it for simple wrappers; reach for a hand-written `Cmd` once there's real decision logic. Its counterpart on the effect side is `simple_effect!` in `ui/effect.rs`, which declares a one-field newtype that writes one place on the `App`. Its **kind argument is mandatory** — `simple_effect!(Name(T) => |app| place, EffectKind::ViewOnly)` — even though every call site so far answers `ViewOnly`: hard-coding it reintroduced exactly the default gotcha 3 does without, since `$place` is an unconstrained expression and a one-line `active_view_mut().number_format` setter would have reported `ViewOnly` with the suite green (improvise-8fy). Likewise, mode entry has no per-mode shim commands — bind `enter-mode ` (improvise-s13); the name is decoded by `parse_mode_name` in `registry.rs`, and `EnterMode` clears the mode's minibuffer on the way in. -17. **Float equality inside formula eval is currently mixed** — `=`/`!=` use a `1e-10` epsilon but the div-by-zero guard checks `rv == 0.0` exactly. Don't assume one or the other; tracked for cleanup. +17. **Float equality inside formula eval has one epsilon, and the div/0 guard shares it.** `FLOAT_EQ_EPSILON = 1e-10` in `model/types.rs` is used by `=`/`!=` **and** by the division-by-zero guard (`rv.abs() < FLOAT_EQ_EPSILON`), in both `eval_expr` and its aggregated twin — six uses across the two evaluators (a div/0 guard plus the `Eq` and `Ne` arms in each), one constant. This gotcha previously recorded the opposite (`=`/`!=` epsilon, div/0 exact) and was stale; the mix is gone. Why an epsilon at all and why this one: `context/code/core.md` §4. 18. **`IndexMap`** preserves insertion order for categories, views, and items. Persistence relies on this — and so, since improvise-shyt, do `DataStore::matching_cells`/`matching_values`: the secondary index **narrows** the candidates, then `matching_positions` resolves each to its position in the primary `IndexMap`, sorts, and reads back. **An index whose values are `HashSet`s is for O(1) lookup, never for iteration.** Walking it gave a different order in every process, which was not tidiness. Records-mode rows reordered between sessions; `SUM`/`AVG` drifted in the last bits because `f64` addition is not associative (`Min`/`Max`/`Count` and the text-unanimity rule of gotcha 7 are order-free, so they were never affected); and worst, the **drill snapshot** froze its record list in hash order while `drill_state.pending_edits` are keyed by *row index* (gotcha 1), so the same drill opened in a second process addressed **different records** — silent misdirected writes, and nothing in the tracker knew. Measured at 18,000 cells: +20% worst case on `matching_values`, and `matching_cells` 7% *faster*, sorted positions resolving `CellKey`s with better locality. `DataStore::live_records` was the open choice and is now decided (improvise-2yc2): it walks `cells` and projects insertion order onto records, deliberately paying O(cells) rather than O(records), because `cells`' order is already the answer `matching_positions` gives and a second answer to "what order is this store in" is the disease. It is derived per call rather than memoised on `RecordEntry`, since a stored position is a copy and a record deleted and re-created would hold its old one. Note what is *not* on offer: an unordered return — every iterator has an order, and a type that merely disclaims one still hands the caller hash order, which is the guard living one level away from the hazard and the shape this whole class of bug returns through. **The second real instance was `import::analyzer::analyze_records`** (improvise-7yx), which built its distinct values through a `HashSet`: `build_model` adds items in exactly that order and `IndexMap` preserves it, so the *same CSV imported twice* produced two different `.improv` files and two different row orders. Now first-seen order, pinned by a five-iteration test because each `HashSet` gets its own `RandomState`. Also answered here since: an **empty `partial`** matches every cell vacuously (improvise-hkt6) — it used to index `sets[0]` and panic, unreachable only because both callers guarded. 19. **Commit paths for cell edits** split across `commit_cell_value` → `commit_regular_cell_value` / `commit_plain_records_edit` in `command/cmd/commit.rs`. Keep the synthetic-records branch in sync with the plain branch when changing behavior. 20. **Minibuffer commits go through one command.** `commit-buffer ` (`CommitBuffer { kind: CommitKind }`) owns buffer→trim→bail-on-empty→dirty→status for all three; the varying parts are methods on `CommitKind`. The add effect is emitted **last** so its failure message wins over the command's optimistic status (improvise-6t3). `CommitKind::Item` carries its target category, resolved from `AppMode::ItemAdd` by `parse_commit_kind` at registry time — so `commit-buffer item` is not constructible from a script line. @@ -193,7 +193,7 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text 28. **The browser backend is sync on purpose, with an async seam.** `OpfsFs` (`improvise-io/src/opfs.rs`, wasm32 only) implements the same synchronous `Storage` trait as `PhysicalFs`, because `FileSystemFileHandle.createSyncAccessHandle()` yields a handle whose read/write/truncate/flush **are** blocking — available only inside a dedicated Web Worker, which is where improvise-9x6 puts the app. So `Storage` never became async. What is still async splits out into four explicit methods the worker awaits from its message loop: `open()`, `attach(path)` (must precede any read/write of that document — acquiring a handle is async even though using it is not), `refresh()` (repopulates the index `list` answers from), and `commit_deletes()`. Consequence: **`delete` is visible immediately but lands in OPFS at the end of the batch** — it drops the name from the index and queues a `removeEntry`. Handles are exclusive per file, so `attach` closes any handle it replaces and `detach` releases the lock. `storage_contract` in `storage.rs` is the behaviour every backend owes; it runs against both host backends and is the written-down spec `OpfsFs` must satisfy in a browser, where the harness cannot reach. -29. **"Which view is active" is stored twice, and the copies must move together.** `Workbook::active_view` is the document's; `ViewState::active_view` is the client's, and it is what keys `ViewState::cursors`. The second copy exists because `reduce_view` runs on a client that holds no `Workbook` (improvise-cqi). Only two writers are allowed to touch either — `App::switch_to_view` and `App::install_workbook` — and `the_two_active_view_names_never_diverge` guards it. Calling `workbook.switch_view()` directly desyncs them and the cursor map is then read under the wrong key; that is exactly how the 8uz characterization test broke during the extraction. Whether the two are *allowed* to diverge — two clients on different views — is improvise-g0f, and the answer is **yes, eventually**: the model is shared, the UI is not, so `Workbook::active_view` becomes the document's *default* (what a fresh client opens) and `ViewState::active_view` is authoritative per client. improvise-jytm owns the split; a "follow mode" binding two UIs together is deliberately not ruled out. Until then `Sessions::apply` keeps them in lockstep by routing `SwitchView` through `App::switch_to_view` — see gotcha 36. +29. **"Which view is active" is stored twice, and the copies are now allowed to differ.** `Workbook::active_view` is the document's **default** — what a freshly connected client opens, and what `.improv` writes as `Initial View`. `ViewState::active_view` is **authoritative for that client**, and it is what keys `ViewState::cursors`. The second copy exists because `reduce_view` runs on a client that holds no `Workbook` (improvise-cqi); the *split* is improvise-jytm answering improvise-g0f, on the user's ruling that **the model is shared and the UI is not**. Two clients may sit on different views of one document. Three things follow. **`App::resolve_view` is the one answer** to what a view name names in this document, falling back to the default so a slice naming a deleted view cannot panic in `rebuild_layout`; `view()`, `view_mut()`, `view_name()`, `project::grid_in` and `ClientSession::view_in` all go through it, and a second copy of that fallback was the first thing review found. **The guard was replaced, not deleted** — `the_two_active_view_names_never_diverge` became `the_documents_default_view_outlives_this_clients_switches` plus `a_fresh_session_opens_the_documents_default_view`, because a deliberate split with no guard is how one silently regresses into the other. And **a deleted view is the case the split creates**: `DeleteView` can take away the view another client is on, so `Sessions::adopt_deleted_views` moves that mirror to the default and tells it, and `effect::DeleteView` does the same for the host — observed ("does this mirror still name a view the document has"), never predicted from the command variant. **Testing any of it needs a bystander that can be moved**: both audience tests first sat theirs on the default, where a wrongly-addressed relay folds to a no-op, and review demonstrated the vacuity by injecting `ToAll` twice. Two gaps remain, both filed: a view-scoped `ModelCommand` names no view, so a connected client's `SetAxis` lands on the host's view (improvise-lltt — deferred on a *condition*, that no shipped client emits `SwitchView`, while the browser already emits `TransposeAxes`), and `Initial View` no longer follows the TUI user (improvise-azhb). Follow mode is deliberately not foreclosed. 30. **Two tests gate `ViewState` membership.** `ViewState` is the *portable* session slice — the one thing serialized into the Snapshot a newly-connected client gets (improvise-q08). A field may live there only if it is **portable** (plain data from the allowlisted crates, no `Rc`/`Arc`) and **snapshot-safe** (a fresh client inheriting the value is correct). `wizard` fails both — `ImportWizard` is an `improvise-io` type, and a half-open dialog is not something to inherit — and `drill_state` fails both — it holds an `Rc`, and `pending_edits` are uncommitted writes belonging to whoever staged them. Both live on `App` under the **session-local** residue tag (improvise-h0b). The `ViewState: Send` static assertion in `app.rs`'s tests is the compile-time proxy for portability: `Rc` is not `Send`, so a re-added shared pointer fails the build (the `Deserialize` derive from gotcha 25 now rejects it too, serde's `rc` feature being off). `Send` does *not* catch the origin-crate or snapshot-safety halves — check those by hand. @@ -205,13 +205,13 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text 34. **A formula target is unquoted, and pipe quoting has one home.** `parse_formula` strips the pipes off a target exactly as it does off an `Expr::Ref` — the target names a `_Measure` item, so a target that kept its delimiters made `add_formula` create an item literally named `|Total Revenue|`: a ghost column the user's real item never matched, and `Expr::Ref("Total Revenue")` never resolved to. Both the TUI formula minibuffer (`AddFormula`) and `.improv` load go through that one call, so both were affected. The writer emits `Formula::raw`, not `target`, so file text is unaffected either way — which is exactly why the round-trip tests never saw it, and why the fix repairs existing files on next load with no migration. Pipe quoting has exactly **one home**: `improvise_formula::quote`, owning both halves — `quote_name`/`is_bare_name`, `pipe_quote` (unconditional, for a `.improv` text cell that must not read back as a number) and `unescape_pipe_body`. `improvise-gp` re-exports it, `improvise-io` calls it from `persistence/{format,parse}.rs`, `parser.rs::unquote_pipe` is a shim over it, and io's own copy is **deleted** (improvise-i2m5). **The keyword guard is canonical — user-confirmed, so do not revert it as over-caution**: an item literally named `SUM` is written `|SUM|`. Bare keywords do parse, but only because `agg_call` needs a `(` and PEG backtracking falls through to `ref_expr` — rule ordering, not a stated rule — and a `## Formulas` line carries formula text verbatim, so a name `format_md` writes is read back by `parse_formula`: one namespace, one spelling rule. One thing the guard does not cover, and one that used to be an exception and no longer is. **A section header takes its name as the whole line, totally** (improvise-pczv): `header_name = { pipe_quoted ~ &NEWLINE | line_name }` — the pipe-quoted arm needs the lookahead, or `|A|B` would parse as `A` and silently drop `B`, and `line_name` catches everything else, so a header name can no longer fail to parse. Four rules share it (`model_name`, `initial_view`, `category_section`, `view_section`), which is why it is a rule and not four copies. Note a header is still *written* raw where a reference is quoted — the grammar accepts both arms, so the quoting asymmetry between a header and a reference is a writer convention, not a format one. And the two former *unquoting* copies differed only on a trailing lone backslash, unreachable from the grammar, where io returned it and `parser.rs` panicked — the surviving `unescape_pipe_body` is total. -35. **`Dirty` over-reports on purpose, and its doc comment is the derivation.** `Dirty::of` maps an `EffectKind` to the widgets that kind can reach, and the two directions do not cost the same: over-reporting pays one redundant `Projection`, under-reporting leaves a client's cache wrong **forever**, with nothing in the stream to correct it. `LayoutGeometry` therefore includes `formulas`, because `RemoveCategory` wears it and `Model::remove_category` ends by dropping every formula targeting the category — a client's formula panel otherwise kept showing a formula the document no longer had. It now coincides with `Dirty::EVERYTHING` and is still spelled out, because the two are derived from different effects and need not stay equal. The doc comment on `Dirty::of` is the **only** record of that derivation, and it went stale once already — claiming `AddCategory` was `CellData` after it was reclassified — so reclassifying an effect means re-reading it. +35. **`Dirty` over-reports on purpose, and the asymmetry is the reason.** `Dirty::of` maps an `EffectKind` to the widgets that kind can reach, and the two directions do not cost the same: over-reporting pays one redundant `Projection`, under-reporting leaves a client's cache wrong **forever**, with nothing in the stream to correct it. `LayoutGeometry` therefore includes `formulas`, because `RemoveCategory` wears it and `Model::remove_category` ends by dropping every formula targeting the category — a client's formula panel otherwise kept showing a formula the document no longer had. It now coincides with `Dirty::EVERYTHING` and is still spelled out, because the two are derived from different effects and need not stay equal. The derivation now lives in `context/code/command.md`, and the doc comment on `Dirty::of` points there rather than restating it; the reason `LayoutGeometry` is spelled out rather than written as `EVERYTHING` is a two-line comment inside the match body. It went stale once already — claiming `AddCategory` was `CellData` after it was reclassified — so reclassifying an effect means re-reading the prose, not just the comment. -36. **A client's command has one door, and the door takes the `App`.** `Sessions::apply(app, id, cmd)` is the only way a connected client's `Command` reaches the server; `ClientSession::fold` and the mirror's `ViewState` are private, so a transport cannot reach the mirror at all. The door takes the authoritative `App` because some commands have a **document half**: `SwitchView` routes through `apply_command`/`App::switch_to_view` (gotcha 29), and **every** mirror then takes what the document *accepted*, not what was asked — so a switch to a view that does not exist leaves the mirrors put. Not just the asker's: `grid_of` builds every client's grid from `workbook.active_view()`, so a bystander left naming the outgoing view has its cursor read under a stale key and is projected the new view's rows positioned by the old view's scroll — gotcha 24's mismatch arriving through somebody else's keystroke. `apply_switch` therefore takes **no `SessionId`**; the switch is addressed to no one session and that absence is the statement. A client's own slice lives on the client and stays in step only by folding the same command, so `Sessions::apply` returns `Applied { dirty, broadcast }`, where `broadcast` is a **`Vec`** and each item names its own audience (improvise-gvnb). **The audience follows from whether the asker folded.** `Broadcast::ToOthers` is a *correction* of something the asker already applied optimistically; `Broadcast::ToAll` is a *consequence* the asker is owed like everybody else. A `Command::Model` is the one kind a client **cannot** fold optimistically — it cannot predict what the document will accept — so **every** relay originating in `apply_model` is `ToAll`: the `ResetCursor` from `adopt_cursor_epoch`, and the `SwitchView` that `ViewBack`/`ViewForward` move. `apply_switch` is the exception, and only because its asker really did fold. **Copying `ToOthers` from `apply_switch` into `apply_model` is the trap, and it is invisible from the bystander's side** — `adopt_active_view` moves the asker's *mirror* either way, so the drift is between the server's picture of the asker and the asker's own slice, and a test asserting through a "what do the others hear" helper looks away from it by construction. (`apply_switch` has the mirror-image hole on its *refused* path, where the asker folded something the document turned down and is the one client not told: improvise-6c6q.) A `Vec` rather than an `Option` because one command can owe two relays with different audiences, and an `Option` would have had to drop one silently. Everything else is that client's alone and never touches the `App`'s own view slice: **the model is shared, the UI is not.** Folding `SwitchView` into the mirror alone was the bug — `viewport()` then read the cursor under the new view's key and the next `project()` built that client a grid out of the *other* view. +36. **A client's command has one door, and the door takes the `App`.** `Sessions::apply(app, id, cmd)` is the only way a connected client's `Command` reaches the server; `ClientSession::fold` and the mirror's `ViewState` are private, so a transport cannot reach the mirror at all. `apply_switch` now **takes a `SessionId`** — it is addressed to exactly one session, which is the opposite of what this gotcha used to say — moves only that session's mirror, and dirties nothing. The one thing it still asks the document is whether the view exists, because a mirror keys `ViewState::cursors` and is read by `grid_in`. The audience is conditional: an accepted switch relays **nothing** (the asker folded it, and no other client's view moves), a refused one is `Broadcast::To(asker, …)` — improvise-6c6q, where `ToOthers` told every client but the one that was wrong. `adopt_active_view` and `Broadcast::ToOthers` are **deleted**; the variants are `ToAll` and `To(SessionId, _)`, the second because the deleted-view case needs a one-session audience that is *not* the asker. The paragraph's warning about the "what do the others hear" helper survives and is why the 6c6q test drives the asker's own `ViewState` — that helper is itself deleted. What survives unchanged: the model is shared and the UI is not, and `Broadcast` is a `Vec` because one command can owe two relays with different audiences. **The trap this paragraph used to describe is gone with `ToOthers`, and the lesson it taught is not.** It was copying `ToOthers` from `apply_switch` into `apply_model`, invisible from the bystander's side because `adopt_active_view` moved the asker's mirror either way — so the drift sat between the server's picture of the asker and the asker's own slice, where a test written through a "what do the others hear" helper looks away by construction. improvise-6c6q closed the mirror-image hole on the refused path. Two rules survive the mechanism: **assert the `Broadcast` itself**, since the audience is the message rather than something to infer from folded state, and **seat a bystander somewhere a wrong relay can move it from** — a bystander already on the relayed value cannot see a wrongly *widened* audience either, because the wrong relay folds to a no-op. Review demonstrated that vacuity twice on PR #48. A `Vec` rather than an `Option` because one command can owe two relays with different audiences, and an `Option` would have had to drop one silently. Everything else is that client's alone and never touches the `App`'s own view slice: **the model is shared, the UI is not.** Folding `SwitchView` into the mirror alone was the bug — `viewport()` then read the cursor under the new view's key and the next `project()` built that client a grid out of the *other* view. 37. **`gp-tree` returns data and never prints, and a fit takes two things to reproduce.** Two rules with one root — the crate is a library a TUI, a wasm worker or a CLI may each host. (a) A decision the user should hear about comes back as **data**: `Dataset::warnings` carries the columns `preprocess` dropped for being entirely missing (improvise-d44s), and `fit.rs` hands back pooled `(prediction, actual)` pairs and a score per feature rather than tables. `examples/fit_csv.rs` owns every `println!`, and that is the only reason its output stayed byte-identical across these moves — adding an `eprintln!` to either module is the regression. (b) **The seed is necessary and not sufficient**: `data::Model` records `seed: Option` *and* `config: Option`, because the same seed under a different population/generations/elite/penalty lands on a different tree (improvise-ksst). Both are `Option` for one reason — every `FitConfig` value and the seed `0` are legal, so nothing can double as "unknown". Note `#[serde(default)]` on an `Option` field is **documentation, not the mechanism**: serde defaults it anyway, so deleting the attribute fails no test, and a backward-compatibility test that only deletes it is not discriminating. Aim such a test at an absent field loading as the *default* instead of as unknown. The GA loop itself is `fit::fit` (improvise-0ai); `breed` remains one fitness-agnostic generation for callers whose fitness is not "predict this column". (c) **A scoring function takes one slice of rows.** `Row` pairs a feature vector with what it predicts, so the parallel `x`/`y` lengths that `zip` used to truncate silently cannot be built (improvise-dkqn, `doctrine(an_invariant_is_made_unrepresentable_not_validated)` — the same answer `prune_axis_items` got, and the reason no `Result` entered five signatures). The pairing is made at the boundary that *builds* the data: `preprocess` walks one row index and the rows live inside `Target`, because a `Dataset` carrying `rows` and `target` separately just moves the zip up one level into every caller. The empty row set is one shape, answered once in `mean_over`. Changing any of this needs **bit-level** evidence: `tests/scoring_known_answers.rs` pins every path as f32 bit patterns plus a `next_u64` witness that catches a changed RNG call order, because the CLI prints three or four decimals and byte-identical output alone cannot see a moved accumulation. (d) **A fit is storable only when the document can *name* its features.** `preprocess` invents `Region=North`, `Cost.missing`, `when.year`; those are **encoding artefacts, not items of the document**, and `Expr::Ref` resolves through `Model::find_item_category` — so a tree splitting on one emitted a well-formed formula whose every cell was `Error("ref:…")`, silently (improvise-8xhy). `fitted_formula` therefore takes the `&Model` and refuses, listing what it could not resolve; `formula_text` still spells it, being the tree's own text. **The check asks the document, never the name's shape**, because a measure literally named `Region=North` is a legal document and the two are indistinguishable as strings — which also means `.missing` and the date parts are covered with no second copy of `preprocess`'s expansion rules to drift from. **This is an interim** (improvise-4zl7): a `WHERE` predicate needs a *scalar coordinate predicate* the formula language does not have — `Filter` gates a formula or an aggregate, it is not an `Expr` — would still leave `.missing` and the date parts, and must first answer what such a predicate means where `context.get(cat)` is `None`, since `Formula::filter` falls through to `aggregate_raw` and a categorical fit would silently degrade in every view that aggregates over the category. The test that missed all this is the shape to remember: `improvise_eval` **fabricates** a `_Measure` item per feature name, so it asserted the emitter and `gp_tree::eval` agree against a model that could not exist. It is kept, with its scope now stated, and the property it could not state is asserted separately against a real document. -38. **The wire vocabulary is derived twice, by the same recipe.** `Command`'s view half is the `EffectKind::ViewOnly` effects targeting the portable slice (`decision(improvise-cqi, command_set_derived_not_invented)`); its model half, `ModelCommand`, is the `EffectKind::Model(..)` effects — **29** of them, one variant apiece, minus `SwitchView`, which `Command` already names and routes (gotcha 36), leaving 28 (improvise-kmgs). Note the obvious `awk` extraction reports **30**: it picks up `HelpPageSet`, which is `ViewOnly`, because that is the last `pub struct` before `#[cfg(test)]` and the test module mentions `ModelChange::` — brace-match the `impl Effect` blocks instead (`doctrine(derive_then_verify)`). Three things follow. **A `ModelCommand` does not mutate**: `ui/reduce.rs::model_effects` maps it back to the effect it came from and `apply_command` runs that through `App::apply_effects`, so effects stay the only mutation site and the `Dirty` is the effect's own `EffectKind`, never a second classification beside gotcha 35's table. **Two model effects are deliberately off the wire.** `ApplyAndClearDrill` and `AcceptWizardModel` were once kept on the grounds that the *command* is portable even where the operand is not; **that was the wrong criterion** (improvise-3x3u). A `Command::Model` is routed with no `SessionId`, which is right exactly when the change is addressed to *no one* session — and these are addressed to precisely one, never the sender: their operands are `drill_state.pending_edits` and the built wizard workbook, session-local residue (gotcha 30) private to whoever staged them (improvise-tom1). Unaddressed, B's `ApplyAndClearDrill` commits A's uncommitted typing and B's `AcceptWizardModel` swaps the document out of a dialogue B never opened — both demonstrated against `Sessions::apply` before removal. So 29 − `SwitchView` − these two = **26**. The effects are untouched and still reached by the key path. Not permanent: multi-client drill wants the *addressed* form, which is where ADR 001 layer 2 goes anyway. Guarded by `no_model_command_commits_another_clients_session_local_state`, plus a companion asserting both effects still exist and are still model changes so the first cannot pass by naming nothing. **`Sessions::apply` routes the model half to `apply_model`**, which owes the same lockstep as `apply_switch`: `ViewBack`, `ViewForward` and a document-replacing `LoadModel` all end in `App::switch_to_view`, so every mirror takes the accepted active view. Its broadcast is conditional where `apply_switch`'s is not, because no client optimistically folded an active view here. A keymap lookup still cannot return a `Command`, and not for want of vocabulary: a binding names an *intent* resolved against `CmdContext` (`move-cursor down`) while a `Command` is the *resolved mutation* (`SetSelected { row, col }`) — improvise-gxi's AC 4 asks for something the split forbids. +38. **The wire vocabulary is derived twice, by the same recipe.** `Command`'s view half is the `EffectKind::ViewOnly` effects targeting the portable slice (`decision(improvise-cqi, command_set_derived_not_invented)`); its model half, `ModelCommand`, is the `EffectKind::Model(..)` effects — **26** of them since improvise-jytm made `SwitchView`, `ViewBack` and `ViewForward` `ViewOnly` — minus `ApplyAndClearDrill` and `AcceptWizardModel`, leaving **24** variants (improvise-kmgs). `SwitchView` no longer needs subtracting separately: it is not a model effect any more. `ViewBack`/`ViewForward` left for the *same* reason as the other two exclusions — their operand is `ViewState::view_back_stack`, session-local residue belonging to whoever navigated. Note the obvious `awk` extraction reports **30**: it picks up `HelpPageSet`, which is `ViewOnly`, because that is the last `pub struct` before `#[cfg(test)]` and the test module mentions `ModelChange::` — brace-match the `impl Effect` blocks instead (`doctrine(derive_then_verify)`). Three things follow. **A `ModelCommand` does not mutate**: `ui/reduce.rs::model_effects` maps it back to the effect it came from and `apply_command` runs that through `App::apply_effects`, so effects stay the only mutation site and the `Dirty` is the effect's own `EffectKind`, never a second classification beside gotcha 35's table. **Two model effects are deliberately off the wire.** `ApplyAndClearDrill` and `AcceptWizardModel` were once kept on the grounds that the *command* is portable even where the operand is not; **that was the wrong criterion** (improvise-3x3u). A `Command::Model` is routed with no `SessionId`, which is right exactly when the change is addressed to *no one* session — and these are addressed to precisely one, never the sender: their operands are `drill_state.pending_edits` and the built wizard workbook, session-local residue (gotcha 30) private to whoever staged them (improvise-tom1). Unaddressed, B's `ApplyAndClearDrill` commits A's uncommitted typing and B's `AcceptWizardModel` swaps the document out of a dialogue B never opened — both demonstrated against `Sessions::apply` before removal. So 29 − `SwitchView` − these two = **26**. The effects are untouched and still reached by the key path. Not permanent: multi-client drill wants the *addressed* form, which is where ADR 001 layer 2 goes anyway. Guarded by `no_model_command_commits_another_clients_session_local_state`, plus a companion asserting both effects still exist and are still model changes so the first cannot pass by naming nothing. **`Sessions::apply` routes the model half to `apply_model`**, which relays `adopt_cursor_epoch`'s `ResetCursor` and `adopt_deleted_views`'s per-client switch; a document-replacing `LoadModel` bumps the generation and lands in the rebase arm. The former claim here — that `ViewBack`, `ViewForward` and `LoadModel` all end in `App::switch_to_view` so every mirror takes the accepted active view — was wrong on all three clauses after improvise-jytm and is deleted. A keymap lookup still cannot return a `Command`, and not for want of vocabulary: a binding names an *intent* resolved against `CmdContext` (`move-cursor down`) while a `Command` is the *resolved mutation* (`SetSelected { row, col }`) — improvise-gxi's AC 4 asks for something the split forbids. 39. **`Applied` is an enum because a document swap is not a diff.** `Sessions::apply` returns `Applied::Diff { dirty, broadcast }`, `Applied::Rebased { snapshots }` or — since ADR 001 layer 1 — `Applied::Rejected { reason }`, and the enum *is* the mechanism: a host that wants the `Dirty` must match, and matching names the case where a `Snapshot` is owed. The server's mirrors self-heal — `ClientSession::project` reseeds on the `document_generation` bump — but `ViewState::cursors` lives on the **client**, keyed by the outgoing document's view names (gotcha 24), and only a fresh `Snapshot` replaces it. `Command::Model` put the three `WholeWorkbook` mutations on the wire (gotcha 38), so the host that must send the snapshots is no longer the host that asked. `apply_model` therefore calls `rebase` itself rather than instructing the host to, exactly as `apply_switch` adopts the accepted view itself. **The trigger is the generation moving, not the command variant**: a `LoadModel` that cannot read its path leaves the document alone, and reseeding on the variant would cost a client its cursors for a *failed* load — `a_load_that_fails_is_not_a_rebase` guards that arm. A `rebase: bool` beside `dirty` was rejected, because the host that forgets to call `rebase` is the host that forgets to read the flag. The **third** case is a *normal outcome*, not an error, and it is in the enum for the same reason the other two are: a host that wants the `Dirty` must match, and matching names it. `ServerMsg::Rejected` is the matching frame, addressed to **the asker alone** — nothing happened, so nobody else is owed anything — and the client keeps it as a `Rejection` rather than a sentence, because force needs to know *which* record and *which* kind (gotcha 43). **Fixture warning.** When testing a document swap, the incoming document's active view must have the **same name** as the outgoing one. A rename makes `apply_model` broadcast a `SwitchView`; the client folds it, and the stale cursor is then read under a key nothing has written — right *by accident*, with the bug invisible. `other_document()` renames, and a first version of this test passed because of it; `a_document_shaped_differently_under_the_same_view_name()` keeps the name and changes only the shape, so there is nothing to hide behind. @@ -230,7 +230,7 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text 43. **A batch carries its basis, and the server had to publish it first.** `Command::Model { basis, cmd }` is a **struct variant** so the basis cannot be forgotten, and `Sessions::apply_model` checks it *before* applying anything — a stale batch is never applied, not even partially (improvise-xwi6/fkfg, ADR 001 layer 1). Four things to know. **`Basis` has no `Default` and no public constructor**: a zero-valued basis is not "no claim", it is the claim that *the document is fresh*, which in a new `App` is true — so a default would be a forgeable bypass rather than an absence. `BasisCache::claim` is the only door a client has and it returns `None` until the server has published one. Note what that does and does not buy: `Basis` derives `Deserialize` because it must cross the wire, and its fields are public, so **a hand-rolled client can state any tiers it likes**. This is an *anti-staleness coordination check, not a security boundary* — `RecordUnstated` closes **omission**, not **invention**, and nothing here tries to close invention, because a client that forges a current basis is indistinguishable from one genuinely caught up. "The only constructor" is a statement about clients written against this crate, which is still worth having: it makes the honest path the easy one. **Which records are checked is the server's decision** — `reduce.rs::records_named`, an exhaustive match with one or-pattern for the variants that name no record, so a new record-writing variant fails the build. Reading the list off the *client's* claim would be a **force by omission**: the record tier skipped, with no flag to remove and nothing in the message saying so. A claim that omits a record the command writes gets `Rejection::RecordUnstated`, a case of its own because "you lost a race" and "you were never in one" are different things to tell a user. **`Rejection` keeps structural and data staleness apart**, which is the whole reason ADR 002 put a maintained `RecordHash` index in `DataStore`: `RecordGone` means nothing resolves and there is nothing to force (**force never creates structure**), `RecordMoved` means it resolves at another version and force is one keystroke — which is also why the wasm client stores the `Rejection` itself rather than collapsing it into `status_msg`, since a string cannot carry which record or which kind. **And the ADR omitted the publication.** It said a batch carries the basis it was composed against and never said how a client learns one; nothing in the projection stream carried any of the three tiers, so the check as written was satisfiable only by forging. `Projection::Basis` + `BasisCache` close that — the server publishes the two document tiers plus the counters for the records *this client's grid names*, diffed against what that client was last told and seeded in the bootstrap — and it is emitted **after** the grid updates in a batch, because the counters describe the cells those updates carry: a basis arriving first would let a client claim a version for a value it had not been shown, the mirror image of gotcha 33. That ordering was documented as load-bearing in three places and **held up only by the order of two statements** until `a_published_basis_follows_the_grid_updates_it_describes` pinned it (proved by injecting the swap) — the same job `a_cell_update_that_overtakes_its_labels_is_lost` does for the mirror-image rule. Two things are unchecked and deliberately so: `Command::SwitchView` carries no basis (it sits outside `Command::Model`; the workbook refuses a view it does not have, so it cannot corrupt, but it is a hole in the rule), and `apply_command`/`reduce_full` do not check at all — **the check is a property of the session door, not of the mutation path**, which is right because a host holding the `App` is the truth rather than a claimant. That second one **was** a door rather than an exemption, and needed a type to stop being one (improvise-ondj): both took a wire `Command` and their `Command::Model { cmd, .. }` arm **discarded the basis with `..`**, while `ui/reduce.rs`'s own module doc pointed every future host — the browser worker included — at exactly those two names. **The struct variant closed omission *from the message*; it did nothing to close ignoring *on arrival*, which is the half a struct variant is usually reached for.** So both now take `Checked<'_>`, a private-field newtype over a `&Command` whose only constructor is `Checked::against(&App, &Command) -> Result<_, Rejection>` — also the only caller of `stale_basis`. `Sessions::apply` answers **once, before the dispatch**, so `apply_switch`/`apply_model` receive the proof rather than each remembering to ask: an invariant made unrepresentable rather than validated, the same answer `prune_axis_items` and `gp-tree`'s `Row` got. Visibility was the weaker option and was rejected for a concrete reason — `pub(crate)` closes only the out-of-crate path, and `reduce_full` has **no production caller anywhere in the workspace**, so it would have become dead code. **The regression guard is therefore the signature**: the test that reproduced the bug no longer compiles, and what remains asserts the constructor refuses and that refusing leaves the document untouched. One residual hole, documented on the type and filed: a `Checked` does not name *which* `App` it was checked against, so a multi-document host could in principle check one and apply to another. That is also why `ui::reduce::caught_up(&App, cmd)` is not a bypass: it takes `&App`, so nothing reachable from a wire can call it. **Since improvise-955n the frame, not the command, is what is answered — and only two of the three tiers hoist.** `CheckedBatch::against` runs before anything in a `ClientFrame::Commands` is applied. `document_generation` and `layout_version` describe the *document*, so answering them once for the frame is exactly right, and a first version hoisted all three — which was a lost update: the record tier asks "is the client's picture of **this record** current", and which records are at stake is a property of the *command* (`records_named`). Skipping it for later commands let `[SetCell(North), SetCell(South)]` overwrite a South somebody else had moved, silently, with the frame reported accepted, and invisible to the other two tiers because a `SetCell` is `CellData` and moves neither. So every mutation answers for its own records **against the frame's basis** — the one the first basis-carrying command stated, never each command's own field, or a later command could claim a fresher picture than the frame was composed under. The one exemption is stated over a *property* rather than a list of commands: a record that **neither resolves in the document nor is claimed by the basis** is one the frame is about to create (`[AddItem(East), SetCell(East)]`, which `BasisCache::claim` cannot possibly claim for), and only that combination is exempt; a record that does not resolve but *is* claimed is still `RecordGone`. That the `Basis` inside a *later* `Command::Model` is ignored rather than compared is an honest interim: clause 1 says the **frame** states one basis, and moving the field from `Command::Model` to `ClientFrame::Commands` would make that unrepresentable-otherwise instead of a convention — a protocol change that belongs with clause 4. Two things worth keeping from the rejected design: **arity is not a property of a variant** (a future `Command::RunScript` is one variant performing unboundedly many mutations), so no count over the vocabulary can enforce "one mutation per frame"; and **the transaction boundary is not the frame**, because one command may already be a chain, which is why clause 4's mechanism belongs at `App::apply_effects`. -44. **The TUI grid draws from its cache, and since improvise-anr9 the cache frames it too.** `GridWidget` holds no `&Model` and no `&View`: cell text, aggregate-ness (`CellId::Aggregate`, ADR 002), column widths, the page bar, the totals strip and now the **frame** all come off `GridCache`. The TUI still builds that cache over the **whole** grid (offsets zero), so a cache position is an absolute position — the browser's `Viewport` is a real window, the TUI's is not, and the two clients therefore use it differently; column totals **must** be projected rather than client-derived, because the sum runs over every row *including the ones a viewport scrolled past*. The frame is an `AxisFrame` per axis — `cats` (the categories, outermost first) plus `entries: Vec` interleaving `Group(FrameGroup)` headers with `Item(Vec)` label *tuples*, exactly as `GridLayout::row_items` interleaves them. That replaced a `Vec` built by `GridLayout::row_label`, which slash-joined the tuple — the exact rendering `two_row_categories_produce_cross_product_labels` says must not appear — so the browser drew `Food/Alice` where the TUI drew two header columns, with no groups and no collapse indicators. It is **one** type and not a `labels` plus a `levels` beside it: two answers to "what are the labels" is the disease (improvise-edp), and this is the second cure, which cost a *type* change rather than an addition. Three things to know before touching it. **`leaf_labels()` is not "the labels"** — it is the innermost label, what a one-line summary calls a row by; a draw loop wants `header_labels(first_rendered)`, `groups()` and `shares_ancestry(sel)`, each of which is one pass, because `item`/`label`/`shows_level`/`group_of` scan `entries` from the start and asking them per position is quadratic in the frame *and grows with the TUI's absolute scroll offset* (found three times on anr9; the per-position forms survive for one-off lookups and say so). **The width rule has one home**, `improvise_protocol::header_level_widths` — widest label per level, plus a gap, clamped — reached by `AxisFrame::header_widths` and by `ui::viewport::compute_row_header_width`, which borrows level tuples off a `GridLayout` rather than building a frame on the keypress path. **A `cats` change re-identifies every position**: a cell's identity is `cats` zipped with its tuple, so `invalidate_relabelled` blanks the whole axis when the categories move, not merely the positions whose tuples differ, and blanks the **totals** on a recategorization of *either* axis — a column total is a sum over the rows, so re-identifying the rows leaves a number answering coordinates the grid no longer has (gotcha 33(b): the rule lives in `apply`, not in a producer's memory). What is still **not** on the cache is `record_col`, and that is correct: it keys pending drill edits and is session-local (gotcha 1). `body_from(idx)` takes an argument only because the two front ends scroll differently — a browser's frame is already windowed and passes `0`, the TUI's covers the whole grid and passes its row offset — not because the rule differs. `group_of` answers "nearest preceding header", which is what the TUI has always painted rather than membership; changing that means changing `AxisEntry` in `improvise-core` and is filed, not smuggled in. And `project::grid_of` evaluates every cell per frame where the widget evaluated only the visible ones — a widening of an existing whole-grid cost, still unbenchmarked. +44. **The TUI grid draws from its cache, and since improvise-anr9 the cache frames it too.** `GridWidget` holds no `&Model` and no `&View`: cell text, aggregate-ness (`CellId::Aggregate`, ADR 002), column widths, the page bar, the totals strip and now the **frame** all come off `GridCache`. The TUI still builds that cache over the **whole** grid (offsets zero), so a cache position is an absolute position — the browser's `Viewport` is a real window, the TUI's is not, and the two clients therefore use it differently; column totals **must** be projected rather than client-derived, because the sum runs over every row *including the ones a viewport scrolled past*. The frame is an `AxisFrame` per axis — `cats` (the categories, outermost first) plus `entries: Vec` interleaving `Group(FrameGroup)` headers with `Item(Vec)` label *tuples*, exactly as `GridLayout::row_items` interleaves them. That replaced a `Vec` built by `GridLayout::row_label`, which slash-joined the tuple — the exact rendering `two_row_categories_produce_cross_product_labels` says must not appear — so the browser drew `Food/Alice` where the TUI drew two header columns, with no groups and no collapse indicators. It is **one** type and not a `labels` plus a `levels` beside it: two answers to "what are the labels" is the disease (improvise-edp), and this is the second cure, which cost a *type* change rather than an addition. Three things to know before touching it. **`leaf_labels()` is not "the labels"** — it is the innermost label, what a one-line summary calls a row by; a draw loop wants `header_labels(first_rendered)`, `groups()` and `shares_ancestry(sel)`, each of which is one pass, because `item`/`label`/`shows_level`/`group_of` scan `entries` from the start and asking them per position is quadratic in the frame *and grows with the TUI's absolute scroll offset* (found three times on anr9; the per-position forms survive for one-off lookups and say so). **The width rule has one home**, `improvise_protocol::header_level_widths` — widest label per level, plus a gap, clamped — reached by `AxisFrame::header_widths` and by `ui::viewport::compute_row_header_width`, which borrows level tuples off a `GridLayout` rather than building a frame on the keypress path. **A `cats` change re-identifies every position**: a cell's identity is `cats` zipped with its tuple, so `invalidate_relabelled` blanks the whole axis when the categories move, not merely the positions whose tuples differ, and blanks the **totals** on a recategorization of *either* axis — a column total is a sum over the rows, so re-identifying the rows leaves a number answering coordinates the grid no longer has (gotcha 33(b): the rule lives in `apply`, not in a producer's memory). What is still **not** on the cache is `record_col`, and that is correct: it keys pending drill edits and is session-local (gotcha 1). `body_from(idx)` takes an argument only because the two front ends scroll differently — a browser's frame is already windowed and passes `0`, the TUI's covers the whole grid and passes its row offset — not because the rule differs. `group_of` answers "nearest preceding header", which is what the TUI has always painted rather than membership; changing that means changing `AxisEntry` in `improvise-core` and is filed, not smuggled in. And `project::grid_of` evaluates every cell per frame where the widget evaluated only the visible ones — a widening of an existing whole-grid cost. Since improvise-jytm `grid_in` may additionally **derive a `GridLayout`** for a session on a view the host is not on; the host's layout is reused where the names agree, which is the common case and the only one that can carry a drill snapshot. 45. **One session loop, two transports — and the browser saves itself.** `Host` is `App` + `Sessions` + `handle(id, ClientFrame) -> Vec` and lives in `improvise_command::ui::host`, **not** in `improvise-ws-server`, which re-exports it. It moved for improvise-9x6: the worker hosts the same authoritative `App` over a `MessagePort` and cannot depend on tokio, and a second copy would have been exactly the mirror improvise-xks6 had just deleted for the frames — a drifting `Applied`/`Broadcast` match is invisible until a client sees it. `registry` stayed on the server, because a worker hosts **one** document. What the worker adds is only what a `MessagePort` differs in: **one client by construction** (the `Addressed` filter is honoured anyway, against a future `SharedWorker`), and **a handshake with no close** — `ClientFrame::Viewport` must still come first, but a pre-handshake frame is *dropped* rather than closing a connection that does not exist. @@ -238,7 +238,7 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text **The async seam does not survive being boxed.** `OpfsFs::open`/`attach`/`refresh`/`commit_deletes` are on the struct, not on `Storage`. `App::storage` is a `Box` and a trait object can name none of the four, so **handing the backend to the `App` puts the async seam permanently out of reach**; the host must hold the concrete backend *and* give the `App` a `Storage`, which means sharing one. That is `impl Storage for Rc`, and it lives in **`improvise-io`** rather than in each worker (improvise-hmws): `impl Storage for Rc` is two foreign types in any other crate, so improvise-9x6 wrote a forwarding newtype that every future worker would have written again. `Rc` and not `Arc` because the one caller is a Web Worker and wasm32 has no other thread. **The flat root is no longer load-bearing**: `OpfsFs::list` joins `dir` like the other two backends, so `storage_contract` takes `dir` as a parameter and the three agree at *every* directory rather than only at `Path::new("")` where joining is the identity (improvise-vhcz). What the index holds is **documents, not entries**, and the split between its three writers is the rule: `attach` indexes nothing (what it creates is zero bytes), `Storage::write` indexes the name and un-queues any pending delete of it, and `refresh` rebuilds from OPFS skipping zero-byte entries — counting an entry it cannot measure as a document, since the one cause of that is another tab holding the exclusive handle (improvise-vvl3). - **What no test here can see** has shrunk, and precisely. `harness/` (improvise-vvl3/xf1) runs `improvise_io::storage::contract::storage_contract` against a real `OpfsFs` inside a real dedicated Web Worker under Chromium and Firefox, and the `browser` job in CI can fail the build — so **`OpfsFs` is no longer compile-checked-and-never-run**, and two bugs fell out of its first execution. That is why `storage_contract` stopped being a `#[cfg(test)]` helper and became a public function returning `Result`: a browser must run *that* sequence, not a second transcription of it, and a failure has to travel out of a worker as a string. Still unseen: **Safari** (Playwright's WebKit has no OPFS, so xf1's criterion is met for two engines of three; `opfs.spec.js` skips on a runtime probe and an unexpected skip on any other engine is a failure), and **`bindings.rs`**, which the harness does not instantiate — `createSyncAccessHandle` exists in no *host* harness, and that shim is now the dark corner where `OpfsFs` used to be. What was already verified stays: `tests/post_message.rs` drives a real `Worker` over a real `App` on nothing but the JSON a `MessagePort` carries. Build note unchanged: the worker bundle needs `--enable-nontrapping-float-to-int` **as well as** `--enable-bulk-memory`, because it carries the whole `App` and so chrono's date arithmetic. + **What no test here can see** has shrunk, and precisely. `harness/` (improvise-vvl3/xf1) runs `improvise_io::storage::contract::storage_contract` against a real `OpfsFs` inside a real dedicated Web Worker under Chromium and Firefox, and the `browser` job in CI can fail the build — so **`OpfsFs` is no longer compile-checked-and-never-run**, and two bugs fell out of its first execution. That is why `storage_contract` stopped being a `#[cfg(test)]` helper and became a public function returning `Result`: a browser must run *that* sequence, not a second transcription of it, and a failure has to travel out of a worker as a string. Still unseen: **Safari** (Playwright's WebKit has no OPFS, so xf1's criterion is met for two engines of three; `opfs.spec.js` skips on a runtime probe and an unexpected skip on any other engine is a failure). **`bindings.rs` is NOT in that list, and said it was for months.** `shell.spec.js`'s "the standalone deployment" loads the app with no `?ws=`, which spawns the worker, which imports the glue, which calls `bindings.rs::start` and `OpfsFs::open` — and the spec then asserts a grid draws, so the whole chain has run under Chromium in CI since improvise-vvl3. The claim survived in **five** places (this line, the File Inventory, two `kb.pl` facts and improvise-bck's grooming note) because each was written before the harness landed and none was re-read after; the spec's own comment says the chain plainly. It is the reason to distrust a "never run" written in prose: nothing checks it, and `kb.pl`'s CI gate checks consistency rather than truth. What was already verified stays: `tests/post_message.rs` drives a real `Worker` over a real `App` on nothing but the JSON a `MessagePort` carries. Build note unchanged: the worker bundle needs `--enable-nontrapping-float-to-int` **as well as** `--enable-bulk-memory`, because it carries the whole `App` and so chrono's date arithmetic. 46. **The page picks a peer, and the demo was built against an assertion.** `web/app.js` holds two transports and no protocol knowledge: `?ws=` selects `improvise-ws-server`, its absence spawns `new Worker('./worker.js?doc=&seed=', { type: "module" })` (improvise-djm). `type: "module"` is mandatory — `worker.js` `import`s the bundle — and there is **no `MessageChannel`**: a dedicated worker's own `postMessage`/`onmessage` *is* the port, so the ticket's channel wording was a date rather than a decision. The websocket path is **kept**, because the ws-server is how a TUI-hosted document is shared and nothing else in the browser can reach it; the cost is contained because the two differ only in *where strings come from* — handshake, fold, repaint and keyboard are all above the seam — and **the presence of the parameter is the whole selection mechanism**, a second knob being a second thing to get wrong. `#status` composes **two** facts, the transport's and the document's, because a single source let a repaint silently erase a connection error. **A worker that rejects in `start()` fires no `error` on the main thread** — `worker.js` catches its own — so the guard is a **first-frame watchdog**, deliberately *not* per-transport: "the peer never answered the handshake" is the one failure that means the same thing over a socket and over a port, and a check aimed at each peer's own failure modes would have missed exactly this one. @@ -449,7 +449,8 @@ wasm-client/tests/lockstep.rs 606 / 13t host and client agree on a ke web/app.js 519 string calls only, no protocol knowledge, no framework. Run under Chromium by harness/tests/shell.spec.js — see gotcha 48 for what that does and does not reach -web/worker.js 100 three string calls, no protocol knowledge. NEVER RUN +web/worker.js 100 three string calls, no protocol knowledge. RUN by + shell.spec.js's no-?ws path, with bindings.rs (gotcha 45) web/index.html, style.css the page; legibility only web/build.sh 38 wasm-pack build → web/pkg (gitignored). Needs wasm-opt = ["-O", "--enable-bulk-memory"] or the bundled @@ -464,8 +465,10 @@ web/demo.improv 57 the document the site ships and seeds from. improvise-worker-server/src/worker.rs 738 / 13t Worker: one port, one client; handshake, Addressed filter, persist trigger (model_state.dirty). Every decision, every test improvise-worker-server/src/vfs.rs 311 / 6t documents/open: seeding is a Storage::write -improvise-worker-server/src/bindings.rs 158 / 0t wasm32 shim: OpfsFs awaits + postMessage. NEVER RUN — - the one dark corner left now that harness/ runs OpfsFs +improvise-worker-server/src/bindings.rs 158 / 0t wasm32 shim: OpfsFs awaits + postMessage. No Rust test + reaches it, but harness/tests/shell.spec.js DOES: the + no-?ws path runs start()/OpfsFs::open under Chromium. + Called NEVER RUN here for months: see gotcha 45 improvise-worker-server/src/lib.rs 114 / 1t the module doc that states exactly what is and is not covered — the one correct copy of that fact improvise-worker-server/tests/post_message.rs 381 / 7t main-thread traffic, strings only diff --git a/crates/gp-tree/src/fit.rs b/crates/gp-tree/src/fit.rs index f39059d3..282a8676 100644 --- a/crates/gp-tree/src/fit.rs +++ b/crates/gp-tree/src/fit.rs @@ -457,7 +457,7 @@ mod quality_tests { #[test] fn the_ranking_is_ordered_shares_with_the_unused_features_dropped() { let ranked = importance_ranking(&[1.0, 0.0, 3.0]); - assert_eq!(ranked.len(), 2, "the zero-importance feature is listed"); + assert_eq!(ranked.len(), 2, "the zero-importance feature is dropped"); assert_eq!(ranked[0].0, 2); assert_eq!(ranked[1].0, 0); assert!((ranked[0].1 - 75.0).abs() < 1e-3, "{ranked:?}"); diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index ce81ef27..8f7d34a7 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -242,6 +242,12 @@ fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> Op cmd: ModelCommand::TransposeAxes, }]), + // The hint line advertises `>`, and an upstream key is a black hole + // (gotcha 47), so it said nothing at all. Interim: improvise-gd28. + "drill-into-cell" => Some(vec![Command::SetStatus( + "drill is not available in the browser yet".to_string(), + )]), + // ── editing ────────────────────────────────────────────────── "edit-or-drill" => edit_or_drill(state, AppMode::from_name(args.first()?)?), "commit-cell-edit" => commit(state, AppMode::from_name(args.first()?)?, Advance::Down), @@ -1222,7 +1228,8 @@ mod tests { (ev(KeyCode::Char('Z')), ""), (ev(KeyCode::Char('H')), "hide-selected-row-item"), (ev(KeyCode::Char('z')), "toggle-group-under-cursor"), - (ev(KeyCode::Char('>')), "drill-into-cell"), + // `>` is NOT here any more: it answers with a status of its own, + // because it is advertised and silence read as broken (gd28). (ev(KeyCode::Char('<')), "view-back"), (ev(KeyCode::Char('/')), "search"), (ev(KeyCode::Char('n')), "search-navigate"), @@ -1259,6 +1266,31 @@ mod tests { } } + /// The advertised key must say what is actually true. Two refusals, two + /// subjects — see `context/test-evidence/clients.md`. + #[test] + fn the_drill_key_and_an_aggregate_edit_refuse_for_different_reasons() { + let mut state = in_mode(AppMode::Normal); + state.grid.extent = GridExtent { rows: 2, cols: 2 }; + state.window = (2, 2); + + let KeyOutcome::Local(drill) = resolve(&state, ev(KeyCode::Char('>'))) else { + panic!("`>` is advertised in the hint line; it must answer"); + }; + let [Command::SetStatus(said)] = &drill[..] else { + panic!("expected one status, got {drill:?}"); + }; + assert!( + said.contains("drill") && said.contains("browser"), + "the status must name drill and say it is the browser that lacks it: {said}" + ); + assert!( + !said.contains("aggregate"), + "a sentence about aggregates is TRUE-SOUNDING about the wrong thing — \ + `>` is refused whatever the cell is: {said}" + ); + } + /// The refusal above is only half the property: the bug guarded is /// *entering a room with no exit*. See `context/test-evidence/clients.md`. #[test] diff --git a/harness/tests/drill.spec.js b/harness/tests/drill.spec.js index d0feca52..193e41ed 100644 --- a/harness/tests/drill.spec.js +++ b/harness/tests/drill.spec.js @@ -48,27 +48,29 @@ test.describe("drill is refused in the browser, not broken", () => { await expect(page.locator("#grid input")).toHaveCount(0); }); - // The silent half, and the one worth pinning. `>` is `drill-into-cell`, - // which `resolve::step` does not name, so it becomes `KeyOutcome::Upstream` - // -> `ClientFrame::Key` -> `Host::handle`, whose Key arm is an empty vec - // (gotcha 47). An upstream key is a black hole, not a slow path. - test("the drill key does nothing at all, and the page is not merely dead", async ({ + // `>` is the key the hint line advertises as ":drill", and it used to be a + // name `resolve::step` did not handle — so `KeyOutcome::Upstream` -> + // `ClientFrame::Key` -> `Host::handle`'s empty vec, a black hole rather than + // a slow path (gotcha 47). It now answers. Interim until improvise-gd28. + test("the drill key says drill is unavailable, not something about aggregates", async ({ page, }) => { await anAggregateGrid(page); - const before = await page.locator("#status").textContent(); await page.keyboard.press(">"); - // Nothing moves: not the mode, not the status, not the grid. + await expect(page.locator("#status")).toContainText( + "drill is not available in the browser yet", + ); await expect(page.locator("#mode")).toHaveText("NORMAL"); - await expect(page.locator("#status")).toHaveText(before); - await expect(page.locator("#grid td.editable")).toHaveCount(0); - // check_the_checker: a test that asserts "nothing happened" passes against - // a page that has stopped listening. This proves the keyboard is live, so - // `>`'s silence is specific to `>` — the refusal below DOES change #status. + // The two refusals have two SUBJECTS, and conflating them would be a + // true-sounding sentence about the wrong thing — worse than silence, + // because it stops the reader looking. `>` must not talk about aggregates. + await expect(page.locator("#status")).not.toContainText("aggregate"); + + // And `i` still gives the OTHER refusal, which is what proves the two are + // distinct rather than one message reached by two keys. await page.keyboard.press("i"); - await expect(page.locator("#status")).not.toHaveText(before); await expect(page.locator("#status")).toContainText( "drilling into an aggregate needs the host", ); diff --git a/prolog-sessions/kb.pl b/prolog-sessions/kb.pl index ac0c0e72..02b09357 100644 --- a/prolog-sessions/kb.pl +++ b/prolog-sessions/kb.pl @@ -8,7 +8,7 @@ crate(gp_tree,'crates/gp-tree','genetic-programming tree fitting; problem-agnost crate(improvise_gp,'crates/improvise-gp','bridge: emits a fitted gp-tree Tree as an improvise formula string and validates it through parse_formula. Exists because gp-tree must stay problem-agnostic and improvise-formula must not drag GP into the wasm bundle.'). crate(improvise_ws_server,'crates/improvise-ws-server','the socket host: tokio + tungstenite transport and a Registry of Hosts keyed by DocumentId. Re-exports Host and the frames; defines neither. Native only.'). crate(improvise_wasm_client,'crates/improvise-wasm-client','the browser client: the fold, the key classifier, and the render model the DOM paints. Non-dev deps are improvise-protocol + serde + wasm-bindgen and nothing else.'). -crate(improvise_worker_server,'crates/improvise-worker-server','the other host: the same App, Sessions and Host as the socket server, behind self.onmessage/self.postMessage with OPFS underneath. worker.rs and vfs.rs are ordinary Rust and hold every decision and every test; bindings.rs is the wasm32 shim and is never run.'). +crate(improvise_worker_server,'crates/improvise-worker-server','the other host: the same App, Sessions and Host as the socket server, behind self.onmessage/self.postMessage with OPFS underneath. worker.rs and vfs.rs are ordinary Rust and hold every decision and every test; bindings.rs is the wasm32 shim, run by harness/tests/shell.spec.js under Chromium and by no Rust test.'). layer(model,improvise_core). layer(view,improvise_core). layer(formula,improvise_formula). @@ -102,7 +102,7 @@ invariant(a_cats_change_reidentifies_every_position,'A cells identity is AxisFra invariant(the_header_width_rule_has_one_home,'improvise_protocol::header_level_widths: per level, the widest label plus a gap, clamped (improvise-1125). It lived twice -- inline in src/ui/grid.rs::render_grid and again in ui::viewport::compute_row_header_width -- and a browser sizing its own row header would have been a third. It takes a borrowed iterator rather than an AxisFrame because its other caller holds a GridLayout on the keypress path and building a frame there would clone every row tuple on every key.'). invariant(saving_is_a_frame_not_a_command,'ClientFrame::Save (improvise-368h) carries no path -- a client cannot see the hosts storage -- and is a FRAME because as a Command it put a runtime-fallible operation inside a batch ClientFrame::Commands promises is whole-or-nothing, so [Save, SetCell] reported a failed write and applied the edit anyway. It also fails the test every Command passes: it mutates no shared state and no clients slice, so reduce_view has nothing to fold and Checked has no basis to answer for. Refused as NoDocumentPath or SaveFailed to the asker alone; success is silent.'). invariant(the_persist_trigger_is_an_observation,'improvise-worker-server persists on model_state.dirty, not on a table of Command variants (touches_document is deleted) -- the same choice as watching cursor_epoch rather than matching CycleAxis, and document_generation rather than matching LoadModel. It is strictly tighter: a refused batch raises nothing, so the "was anything rejected" guard went with it.'). -invariant(opfs_is_run_but_bindings_is_not,'harness/ (improvise-vvl3/xf1) executes improvise_io::storage::contract::storage_contract against a real OpfsFs in a real dedicated Web Worker under Chromium and Firefox, gated by the browser job in CI; two bugs fell out of the first run. Playwrights WebKit provides no OPFS at all (getDirectory rejects with UnknownError), so xf1s three-engine criterion is met for two of three and the project is kept so it starts the day that build gains OPFS. harness/tests/shell.spec.js runs web/app.js under CHROMIUM ONLY -- engine-independent logic, and routeWebSocket is slower across three. Never instantiated by any harness: bindings.rs.'). +invariant(the_harness_runs_the_worker_chain_under_chromium_only,'harness/ (improvise-vvl3/xf1) executes improvise_io::storage::contract::storage_contract against a real OpfsFs in a real dedicated Web Worker under Chromium and Firefox, gated by the browser job in CI; two bugs fell out of the first run. Playwrights WebKit provides no OPFS at all (getDirectory rejects with UnknownError), so xf1s three-engine criterion is met for two of three and the project is kept so it starts the day that build gains OPFS. harness/tests/shell.spec.js runs web/app.js under CHROMIUM ONLY -- engine-independent logic, and routeWebSocket is slower across three. It also runs BINDINGS.RS: its no-?ws test loads the app, which spawns web/worker.js, which imports the glue, which calls bindings.rs start() and OpfsFs::open(), and the test then asserts a grid draws. This invariant was named opfs_is_run_but_bindings_is_not and asserted the opposite in FIVE places for months (here, the crate/3 fact above, two context/repo-map.md lines and improvise-bck) -- every copy written before the harness landed and none re-read after. A never-run claim in prose is unfalsifiable by anything in this repo: the CI job checks this KB for CONSISTENCY, not truth, and the error UNDERSTATED coverage so nothing failed and nobody looked. What is genuinely unrun, BY ENGINE: Safari/WebKit (no OPFS in Playwrights build) and any non-Chromium engine for the shell spec. That is deliberately scoped to engines rather than written as a closed list of everything unreached, because a closed list is how the previous version of this invariant went wrong in the other direction: two wasm_bindgen exports on WorkerServer, documents() and refresh(), have NO caller in web/*.js or harness/ and so are run by nothing (the .refresh( hits in harness/opfs-contract are OpfsFs::refresh, a different method). The lesson the rename is about applies to its own replacement: a coverage claim in prose is unfalsifiable by anything in this repo, so state what IS covered and scope what is not, rather than enumerating an absence.'). doctrine(bug_first,'write a failing test demonstrating the bug before fixing it'). doctrine(unit_over_e2e,'prefer small unit tests over integration/e2e'). doctrine(suite_speed,'Two numbers, because the ambiguity between them is what let this drift: NO SINGLE TEST BINARY may exceed 2 seconds, and the WORKSPACE WALL CLOCK is the sum of them. Measured at 22d43b2: 1309 tests over 33 binaries, ~2.2s wall, longest single binary 1.42s -- so the workspace is already past 2s and the rule that bites is the per-binary one. Re-MEASURE rather than re-assert: cargo test --workspace prints a "finished in" per binary. Excludes compilation, and excludes harness/ (Playwright, minutes, its own CI job).'). @@ -122,11 +122,12 @@ doctrine(two_questions_one_enum,'when a new enum variant shares one property wit doctrine(contract_test_for_the_unreachable_backend,'when one implementation of a trait cannot be exercised by the test harness, write the contract as a function over &dyn Trait and run it against the reachable implementations; it becomes the executable specification the unreachable one is held to'). doctrine(derive_the_enum_from_the_effects,'when a wire vocabulary must mirror an existing mutation set, enumerate the mutations and let that be the enum; inventing variants ahead of the mutations produces a protocol nobody can implement against'). doctrine(force_the_failure,'an unchecked status whose failure yields a usable-looking value is not a style nit: force the failure and look at what the next line does with it. Four on one branch -- bd init exits non-zero but still leaves an empty database behind, an unchecked mv lands the new tracker INSIDE the old directory and reports success, jq status swallowed by a process substitution reads as \"no inputs found\" and exits 0, and a failed mktemp turns \"$TMP/$NAME\" into an absolute path at the filesystem root. Every one looked like success at runtime, so reading the failure paths found none of them; stubbing mv and mktemp to fail is what exposed them. Note chmod a-w proves nothing when the tests run as root -- it returned 0 and looked like a pass. The same shape appears in tooling: requesting a Copilot PR review over REST returns HTTP 200 and silently ignores the reviewer.'). +doctrine(an_injection_that_did_not_compile_reads_as_a_clean_run,'a fault injection is evidence only if the build RAN. A grep for compiler errors that matches ^error[ but not a bare error: line turns a failed compile into a green read -- the identical defect class injection exists to catch, one level up in the instrument. Caught mid-review on PR #48 by the reviewer in its own tooling, one step from a false unguarded finding, and then re-checked against the earlier runs rather than assumed away. The fix is not a wider grep, because a pattern can always be one case short: require a POSITIVE signal that the injected build ran -- the test count moved, or the named test appears in the output. A count cannot be silently empty.'). doctrine(kb_is_maintained_through_the_mcp,'prolog-sessions/kb.pl is the MCP serialized clause database, not a hand-written source file. Add facts by loadSession, then loadProgram, then saveSession -- never by appending text to the file. Note loadProgram REPLACES a whole predicate rather than appending to it, so the program you load must carry every existing clause of every predicate it touches, or the rest are silently dropped. Verify counts with findall/length before saving. assertz is NOT an escape hatch: a consulted predicate is static and assertz raises permission_error, so the whole-predicate reload is the only route.'). doctrine(fold_tests_must_test_order_not_the_axiom,'a test asserting fold(prefix) then fold(rest) == fold(whole) is VACUOUS: that identity is the definition of a left fold and holds for any apply() whatsoever. Proven 2026-09-19 by rewriting GridCache::apply to write every cell update to (0,0) -- both headline fold tests stayed green while the two real tests failed. Test the properties the design actually rests on: ORDER SENSITIVITY (which updates are dropped when they arrive early, i.e. that the fold is deliberately not commutative) and COMPACTION EQUIVALENCE (fold(compact(history)) == fold(history)), which is what \"seeded, then follows the event stream\" means in production. A seed taken as the literal head of the history presupposes the conclusion.'). doctrine(a_macro_must_not_supply_a_forced_classification,'invariant(effect_kind_no_default) makes the compiler demand an EffectKind per effect precisely because a wrong one is silent. A macro that hard-codes the kind reintroduces the default under another name, for a category defined by SYNTAX (one field, one place) rather than SEMANTICS (per-client). Proven 2026-09-19: a simple_effect! instantiation writing the active view number format -- a textbook LayoutGeometry change -- compiled, reported ViewOnly, and passed all 294 command tests. If a macro covers a classified thing, make the classification a mandatory argument even when the answer is always the same.'). doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only,'when a rule must hold on several arms -- both directions of a fit, every variant that touches one collection, each side of a symmetry -- a test driving ONE arm says nothing about the others, and will sit there green while the rule is violated next door. Worked example 2026-09-19: improvise-rkeq fixed col_widths never being re-fitted when ColLabels changed, and fixed only the label arms; the ColWidths arm, named after the very collection it fits, kept assigning verbatim and put the length mismatch straight back. col_widths_always_span_the_column_labels passed the entire time because it only ever drove the label arm. Copilot caught it on the PR. Enumerate the arms and test each, or assert the rule over a generated set of arms rather than one chosen by hand.'). -doctrine(lockstep_binds_every_session_not_the_asker,'when two copies of a value must move together and one copy is per-client, moving it for the client that ASKED is not lockstep -- it is a desync with a polite name. Proven 2026-09-19 (improvise-kvyt, found by Copilot): Sessions::apply moved workbook.active_view and then only the addressing session mirror, but grid_of builds EVERY client grid from workbook.active_view(), so a bystander left naming the outgoing view had its cursor read under a stale key and was projected the new view rows positioned by the old view scroll. The fix gives every mirror the ACCEPTED value -- not the requested one, which also corrects a client that optimistically folded a name the document turned down -- and apply_switch takes no SessionId at all, because the change is addressed to no one session and that absence is the statement. Ask of any shared value: who else derives something from it?'). +doctrine(lockstep_binds_every_session_not_the_asker,'when two copies of a SHARED value must move together and one copy is per-client, moving it for the client that ASKED is not lockstep -- it is a desync with a polite name. Proven 2026-09-19 (improvise-kvyt, found by Copilot) on active_view, and the reasoning generalises past the example that taught it: ask of any shared value, who else derives something from it? improvise-jytm then REMOVED active_view from the doctrine scope by making it per-client (the model is shared, the UI is not), so what remains under it is the document half -- cursor_epoch, document_generation -- and the doctrine is now a test for whether a value is shared AT ALL before it is a test for who hears about it.'). doctrine(an_invariant_is_made_unrepresentable_not_validated,'when a reviewer asks a function to validate a precondition -- matching lengths of parallel slices, a count supplied alongside the collection it counts -- prefer changing the type so the bad state cannot be built. Proven twice on one branch 2026-09-19: prune_axis_items stopped taking &[bool] plus a debug_assert and took a predicate over the data index instead, so no parallel length exists to get wrong; and the same reviewer request against gp-tree five public scoring functions was declined in favour of giving them one slice of rows rather than parallel x/y, which removes the zip truncation AND the empty-input NaN as one shape rather than four edge cases. Validation also pushes Result into five signatures, which AGENTS.md keeps at the boundaries.'). doctrine(same_kind_is_not_the_same_effect,'a classification shared by an operation and its INVERSE cannot check a mapping between them. Proven 2026-09-19 (improvise-kmgs review): model_effects sends 28 ModelCommand variants to one effect each, and HideItem/ShowItem, AddItem/RemoveItem, CreateView/DeleteView and SetCell/ClearCell all declare the same EffectKind -- so swapping three of them simultaneously left the workspace at 918 passing, 0 failing, with three wire messages doing the exact inverse of what they said. A test asserting only that the target declares the right KIND is therefore no guard at all. Where a mapping is name-for-name by construction, ASSERT THE NAME: it is total, costs nothing at runtime, and a variant whose effect must be named something else is a variant that stopped being derived and should fail loudly.'). doctrine(a_rename_can_hide_a_stale_key,'a fixture that RENAMES a key across the change under test can make a stale entry unreachable rather than wrong, so the test passes for the wrong reason. Proven 2026-09-19: the first document-swap test used a fixture whose incoming active view was renamed Default->Sheet, which made apply_model broadcast a SwitchView; the client folded it and then read its cursor under a key nothing had written -- right by accident, bug invisible. The replacement keeps the view NAME and changes only the shape, so there is nothing to hide behind. General form: when testing that stale state is corrected, hold the identifiers constant and vary only what makes the state stale. Same family as the one-category fixture that could not tell TransposeAxes from CycleAxis.'). @@ -136,7 +137,7 @@ doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished,'two de doctrine(a_missing_variant_is_a_date_before_it_is_a_decision,'when two vocabularies for the same job differ, ask of each difference whether the ARGUMENT for it names the thing they differ in. In improvise-xks6 none of Viewport, Key or batching mentioned a transport -- each was justified by the session model or the fold -- so each difference was an unfinished feature or a stale file. THE TELL FOR STALENESS IS INCOHERENCE RATHER THAN DIFFERENCE: the worker envelope carried a Basis it had no Rejection to answer.'). doctrine(a_struct_variant_closes_omission_not_ignoring,'making a field mandatory in the MESSAGE stops a sender forgetting it. It does nothing to stop a RECEIVER discarding it: the two-dot rest pattern is exactly that, with no compile error. If the receiver must not ignore it, the receiver PARAMETER is the place to say so -- improvise-ondj gave apply_command a Checked newtype rather than a guard.'). doctrine(a_documented_door_with_no_caller_is_the_dangerous_kind,'reduce_full was named by ui/reduce.rs module doc as the entry point every host should use, and used by NONE of them. Nothing exercised its Command::Model arm, so nothing could notice it skipped the basis check. Audit the functions the DOCS ADVERTISE, not only the ones the callers reach.'). -doctrine(the_audience_follows_the_optimistic_fold,'a relay to other clients is ToOthers exactly when the asker folded the command optimistically; otherwise ToAll. A Command::Model is never folded optimistically, so every relay out of apply_model is ToAll. Copying apply_switch ToOthers into apply_model left the asker own slice naming the outgoing view while its server mirror named the incoming one -- and a test asserting through a what-do-the-others-hear helper cannot see it, because that helper looks away from the asker by construction.'). +doctrine(the_audience_follows_the_optimistic_fold,'a relay is addressed by whether the asker folded the command optimistically and by whose UI the change reaches. A Command::Model is never folded optimistically, so every relay out of apply_model is ToAll. A view switch IS folded optimistically and reaches nobody else UI, so an accepted one relays NOTHING and a refused one is To(asker) alone -- improvise-6c6q, where ToOthers told every client but the one that was wrong. Two traps, both proven by injection on PR #48. A test written through a what-do-the-others-hear helper looks away from the asker by construction, which is how the sibling bug in apply_model shipped green. And a test whose bystander is already on the value being relayed cannot see a wrongly WIDENED audience either, because the wrong relay folds to a no-op -- so assert the Broadcast ITSELF, since the audience is the message, and seat the bystander somewhere it can be yanked off.'). doctrine(half_a_lockstep_fix_is_worse_than_none,'resetting the server mirrors without telling the clients makes the DIFF believe they are already current, so it sends nothing: stale FOREVER rather than until the next scroll. When a fix has a server half and a client half, shipping one is not partial progress, it is a worse bug than the original.'). doctrine(force_is_a_recomposition_not_a_bypass,'force never applies a batch whose basis is stale; it re-composes the same INTENT against the current state and submits a fresh batch with a current basis. A mechanism whose only purpose is to violate the invariant is not a feature, it is the invariant deleted with extra steps.'). doctrine(a_simulated_fix_must_be_the_fix_you_named,'The sibling of an_injected_fault_must_be_the_fault_you_named, one level up, and it fails in the opposite direction: an injected FAULT that is not the one named proves a check that does not fire; a simulated FIX that is not the one named proves a BUG THAT DOES NOT EXIST. Worked example: the harness agent measured the 8s watchdog overwrite honestly, on a tree predating PR #21, then reported it "survives improvise-taxx, confirmed by simulating the opened flag". PR #21 is TWO changes -- the flag AND routing both close arms through setShellFailure, which clears the timer. Simulating half of it reproduced the bug and read exactly like a confirmation. improvise-xps8 was filed on that report, propagated into repo-map gotcha 48 and a spec comment, and was closed INVALID. The repair is not a deletion: the property is now executed by a test that outlasts HANDSHAKE_TIMEOUT_MS, proved discriminating by removing setShellFailures clearTimeout and watching "sent no document" appear.'). @@ -200,7 +201,8 @@ decision('improvise-dz8',filter_persists_in_the_format,'added \"filter\" to axis decision('improvise-i34',no_indexeddb_fallback,'user decision 2026-08-31: skip the IndexedDB fallback unless recent Safari needs it. It does not -- Safari 16.4+ has synchronous access handles -- so the fallback is out of scope and the async question it would have forced does not arise.'). decision('improvise-i34',delete_is_deferred,'removeEntry is async and the trait is sync, so delete/1 drops the name from the index and queues the removal. A delete is therefore visible to read and list at once and lands in OPFS when commit_deletes is awaited. Invisible to a single worker; the alternative was making the whole trait async for one operation.'). decision('improvise-cqi',command_set_derived_not_invented,'the client->server half of Command is exactly the set of EffectKind::ViewOnly effects whose target is the portable slice. Effects touching host-local state (wizard, drill_state, storage, layout cache) are absent by construction -- improvise-h0b had already moved those off ViewState, so the eviction did the enumeration.'). -decision('improvise-cqi',view_state_carries_its_own_active_view,'reduce_view runs on a client with no Workbook, and cursors is keyed by view name, so ViewState needs its own active_view. Kept identical to Workbook::active_view by App::switch_to_view and App::install_workbook; whether they MAY diverge is improvise-g0f.'). +decision('improvise-cqi',view_state_carries_its_own_active_view,'reduce_view runs on a client with no Workbook, and cursors is keyed by view name, so ViewState needs its own active_view. Kept identical to Workbook::active_view until improvise-jytm, which answered improvise-g0f: they MAY diverge. Workbook::active_view is now the document DEFAULT (what a fresh client opens, what .improv writes as Initial View) and ViewState::active_view is authoritative per client.'). +decision('improvise-jytm',active_view_is_split_default_from_client,'the model is shared and the UI is not (user, 2026-09-19), so which view you are looking at is UI. Four things worth keeping. The split is made to AGREE WITH EffectKind rather than by a new mechanism: SwitchView/ViewBack/ViewForward became ViewOnly, which took the latter two off ModelCommand for free, because ModelCommand is DERIVED from the Model effects. A switch is therefore invisible to Dirty, so ClientSession OBSERVES its own view move (projected_view) rather than being told -- the cursor_epoch shape. App::resolve_view is the ONE answer to what a view name names in this document; a second copy of that fallback was the first thing review found. And the split CREATES a case it must then close: a DeleteView can take away the view another client is on, answered by observation in adopt_deleted_views rather than by a table of which commands remove views. Follow mode is deliberately NOT foreclosed: per-session UI state stays addressable, so a follow relationship can later be one session DERIVING its ViewState from another.'). decision('improvise-sef',retroactive_ticket,'The three kb commits landed with no beads issue at all. Filed and closed 2026-09-19 during a commit/tracker reconciliation rather than left untracked, because the CI job they add is a durable capability someone will later ask the provenance of. Every other commit back to cc3889d already named a ticket that is closed with a close_reason.'). decision('backlog-groom-2026-09-19',render_cache_name_collision,'improvise-cqi landed RenderCache as a flat grid-only wire type, while six open migration tickets (35e, n10, pca, 764, jb3, edp) were written against a fat per-widget cache of the same name. Two objects, one name, and nothing in the tree complains until someone writes from_app. Filed improvise-rkeq to decide the shape and made it block 35e rather than silently rewriting six tickets to match whichever shape a groomer preferred.'). decision('backlog-groom-2026-09-19',retroactive_tickets_for_untracked_merges,'PR #7 (cloud-session tooling) landed 18 commits with no beads issue, the same gap the kb commits had. Filed improvise-3qz2 closed. Rule being followed: work that adds a durable capability -- a CI job, a hook, a script someone will later ask the provenance of -- gets a ticket even after the fact; a chore commit does not.'). @@ -379,7 +381,7 @@ kb_issue_found(mcp_lenience,'the prolog MCP server accepted \`:- discontiguous f kb_issue_found(static_str_blocks_deserialize,'a borrowed &\'static str field makes Deserialize unimplementable, and rustc reports it on every ENCLOSING enum variant rather than at the offending field -- five near-identical \"lifetime may not live long enough\" errors pointing at MinibufferConfig, none naming buffer_key. Read past the reported site to the borrowed field.'). kb_issue_found(ticket_premise_false_8uz,'improvise-8uz asserted selected/row_offset/col_offset were \"PERSISTED in .improv\". They were not: improv.pest has no cursor production and format_md emitted none. The claim shaped the ticket\'s whole option set, so it was checked with a test before any of the options was weighed.'). kb_issue_found(ticket_premise_false_rml,'improvise-rml called category_panel.rs an \"exact copy\" of tile_bar.rs. It was not: the colors were identical but the labels differed (\"Row ↕\" vs \"Row\", \"None ∅\" vs \"·\"). Only the color table was true duplication; the four label sets are four deliberate vocabularies and were preserved as four methods.'). -kb_issue_found(duplicated_active_view,'adding ViewState::active_view created a second copy of \"which view is active\". The improvise-8uz characterization test caught the desync immediately because it called workbook.switch_view directly. Two writers only (switch_to_view, install_workbook), guarded by the_two_active_view_names_never_diverge.'). +kb_issue_found(duplicated_active_view,'adding ViewState::active_view created a second copy of "which view is active". The improvise-8uz characterization test caught the desync immediately because it called workbook.switch_view directly. Held in lockstep by two writers until improvise-jytm, which made the divergence deliberate: the guard test the_two_active_view_names_never_diverge was REPLACED, not deleted, by the_documents_default_view_outlives_this_clients_switches and a_fresh_session_opens_the_documents_default_view -- a deliberate split with no guard is how one silently regresses into the other.'). functional(invariant/2). functional(location/3). functional(doctrine/2).