Merge pull request #30 from fiddlerwoaroof/docs/reconcile-after-wave-1
CI / Test and lint (host) (push) Has been cancelled
CI / wasm32 check (browser bundle crates) (push) Has been cancelled
CI / Knowledge base consistency (push) Has been cancelled
CI / Browser harness (OPFS contract + shell) (push) Has been cancelled
CI / Browser bundle + static site (push) Has been cancelled
CI / Deploy to GitHub Pages (push) Has been cancelled

Reconcile the docs with the code after wave 1.

Gotchas 1, 11, 18, 21, 42, 43, 44, 45, 46, 47, 48 corrected against the tree; the File Inventory regenerated rather than patched; kb.pl's seven now-false invariants replaced in place and the counts up (doctrine 48->55, invariant 44->51, location 31->34); the dead-ID audit recipe widened, anchored and made runnable.

Includes the repair of a false claim this campaign propagated: improvise-xps8 described a watchdog bug the merged tree does not have, because a partial simulation of PR #21's fix reproduced the original by construction. The open-then-dropped harness test now runs past the handshake timeout and asserts the false message never appears — proved discriminating by removing setShellFailure's clearTimeout.
This commit is contained in:
Ed Langley
2026-09-20 01:31:36 -07:00
committed by GitHub
12 changed files with 359 additions and 179 deletions
+39 -4
View File
@@ -210,12 +210,47 @@ session before an agent tried one and found nothing. Read the ID back after
filing, and before pushing docs run the audit:
```bash
for id in $(grep -ohE 'improvise-[a-z0-9]{3,4}\b' context/repo-map.md context/adr/*.md \
| sort -u | grep -v 'improvise-core\|improvise-wasm'); do
bd show "$id" >/dev/null 2>&1 || echo "MISSING: $id"
done
grep -rohE 'improvise-[a-z0-9]{3,8}\b' \
context README.md AGENTS.md CLAUDE.md crates src tests examples web harness docs \
| sort -u \
| grep -vE '^improvise-(core|formula|io|protocol|command|gp|ws|worker|wasm|harness|registry|side)$' \
> /tmp/bd-audit-ids.txt
bd show $(cat /tmp/bd-audit-ids.txt) 2>&1 \
| grep -iE 'not found|no such issue' || echo "all resolve"
```
Three things the earlier version of this recipe got wrong, each of which let a
real defect through:
- **It scanned two paths** (`context/repo-map.md context/adr/*.md`), so it
found one dead id in the repo map and missed both copies of it in
`harness/tests/shell.spec.js`. Ticket IDs are cited in code comments,
in `README.md` and in `web/`, not only in `context/`.
- **`[a-z0-9]{3,4}` cannot see a commit SHA wearing the prefix.** A seven-hex
commit sha cited as `improvise-<sha>` survived in
`crates/improvise-core/src/workbook.rs` precisely because it was too long
to match. An ID-shaped non-ID is worse than a dead ID, because the
diagnosis is identical and the repair is not.
- **The exclusion list named only `core` and `wasm`**, so every other crate
name would have been reported missing. The exclusions above also cover the
non-ticket tokens this repo really contains: a package name, a temp file
name, and the prose word "improvise-side". **They are anchored `^…$` for the
same reason the recipe is read as code**: unanchored, the `gp` alternative is
a *prefix* match, so a future real ticket whose id happens to begin `gp` is
silently excluded and the audit reports nothing while skipping it. An
exclusion list is a filter, and a filter that over-matches fails in the
direction that looks like success. (Anchoring it is also what caught the
example id this very paragraph originally spelled out — see the note below.)
Note the recipe's own prose deliberately spells no live or dead id: a document
that lists ids as examples makes itself a source of audit noise.
One environment note: in a worktree-isolated agent the shell guard refuses a
loop that passes a runtime-computed argument to `bd`, so the recipe above
materialises the id list into a file and hands `bd show` the whole list in one
call rather than looping. If `bd show` in your build does not take several ids,
run the ids through `xargs -n1 -I{} bd show {}` from a plain shell instead.
A dead ID is worse than no ID: it reads as a promise that the follow-up exists.
### Before filing a ticket, search
+19 -9
View File
@@ -199,6 +199,14 @@ To run the same page locally:
python3 -m http.server 8080 --directory web
```
The page and the OPFS backend are exercised by a Playwright harness in
`harness/` — `npm ci` and `./harness/build.sh`, then `npx playwright test` from
that directory. `tests/shell.spec.js` runs `web/app.js` under Chromium against a
mocked peer; `tests/opfs.spec.js` runs the `Storage` contract against a real
`OpfsFs` in a real Web Worker under Chromium and Firefox (WebKit provides no
OPFS and is skipped on a runtime probe). CI's `browser` job runs both and can
fail the build.
To open a document hosted by a separate process instead — which is how a
document open in the TUI is shared — start the websocket server and pass
`?ws=`:
@@ -232,7 +240,8 @@ from a public-network origin to a loopback or private-network address behind a
permission prompt; per Chrome's own documentation WebSockets were **not yet**
covered when the prompt began shipping, with coverage planned, so whether a
given browser and version gates *this* socket is not something this project can
state — and there is no browser here to test it in. Two gaps are structural
state — and nothing here can test it: the Playwright harness serves a loopback
origin, so Local Network Access never engages in it. Two gaps are structural
rather than version-dependent: it is a Chromium feature, so other browsers need
not behave the same; and it only engages for a **public** origin, so the
locally-served page this section is about is outside it entirely.
@@ -310,8 +319,8 @@ So the honest controls are the ones that need **no credential from the page**:
own hostname, which the handshake then carries by itself; so it needs the
user to visit that hostname first, and it is
subject to the browser's third-party-cookie rules. Plausible and *not
verified here* — there is no browser in this repository — so it is named, not
written out as a recipe.
verified here* — the Playwright harness mocks its peer and reaches no
identity-aware proxy — so it is named, not written out as a recipe.
What would make this genuinely easy is a **token in the URL**, because the URL
is the one place the page can put a credential *deliberately* — everything else
@@ -352,7 +361,8 @@ caddy run # terminates TLS, proxies to
```
On the same terms as the browser caveat below: **this configuration has not
been executed in this repository** — there is no Caddy and no browser here. It
been executed in this repository** — there is no Caddy here, and the browser
harness that does exist never puts a proxy in front of the socket. It
is the form Caddy documents for `remote_ip`, and the reasoning about what a
browser can satisfy is checked against `web/app.js`, which opens
`new WebSocket(url)` and can therefore send no header. Verify the deny actually
@@ -412,10 +422,10 @@ binary whose job is a pivot table". If sharing a TUI-hosted document to a
remote browser ever becomes a common case rather than an occasional one, this
is worth revisiting on that evidence.
Caveat worth stating plainly: **no line of the browser JavaScript has ever
run.** There is no browser and no headless DOM in the development environment,
so `web/app.js`, `web/worker.js` and the OPFS backend are compile-checked,
syntax-checked and read, never executed.
Caveat, narrowed: `harness/` runs `web/app.js` under Chromium and `OpfsFs` under
Chromium and Firefox, in CI. What is still never executed is `bindings.rs`, the
grouped and multi-level DOM, the CSS and the `ResizeObserver` arithmetic — and
Safari, whose Playwright build provides no OPFS at all.
## What's interesting about the architecture
@@ -426,7 +436,7 @@ things that mutate app state, and each one is a small, debuggable struct.
This means commands are testable without a terminal, effects can be logged or
replayed, and the 40+ commands and 50+ effect types are all polymorphic —
dispatched through trait objects and a registry, not a central match block.
The keybinding system gives each of the 14 modes its own keymap, with
The keybinding system gives each of the 17 modes (`ModeKey`) its own keymap, with
Emacs-style prefix keys for multi-stroke sequences.
## Expectations
+8 -1
View File
@@ -301,7 +301,14 @@ candidate for the store.
Aim for **~80% line and branch coverage** on logic code. This is a quality floor —
go higher where the code is tricky or load-bearing, but don't pad coverage by
testing trivial getters or chasing 100% on rendering widgets. The test suite
should remain fast (under 2 seconds). Slow tests erode the habit of running them.
should remain fast, and the rule is **two numbers** because the ambiguity
between them is what let the old single "under 2 seconds" 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. Excludes compilation, and excludes `harness/`,
which is Playwright and takes minutes in its own CI job. Slow tests erode the
habit of running them.
### Demonstrate bugs before fixing them
+183 -105
View File
File diff suppressed because one or more lines are too long
+4 -2
View File
@@ -316,8 +316,10 @@ impl GridLayout {
)
}
/// The records-mode columns: the regular categories, then one column per
/// raw measure, then the new-measure column. Formula targets are not
/// The records-mode columns: the regular categories, then the
/// new-category column (which sits where the column it creates will
/// appear, improvise-2zq), then one column per raw measure, then the
/// new-measure column. Formula targets are not
/// included — they are computed, so there is nothing to type into them.
fn records_columns(model: &Model) -> Vec<RecordCol> {
let mut cols: Vec<RecordCol> = model
+2 -2
View File
@@ -27,7 +27,7 @@ impl Workbook {
/// Create a new workbook with a fresh `Model` and a single `Default` view.
/// Virtual categories (`_Index`, `_Dim`, `_Measure`) are registered on the
/// default view. All virtuals default to `Axis::None` via
/// `on_category_added` (see improvise-709f2df), then `_Measure` is bumped
/// `on_category_added` (see commit 709f2df), then `_Measure` is bumped
/// to `Axis::Page` so aggregated pivot views show a single measure at a
/// time (see improvise-kos). Leaving `_Index`/`_Dim` on None keeps pivot
/// mode the default — records mode activates only when the user moves
@@ -174,7 +174,7 @@ mod tests {
let v = wb.active_view();
// Virtual categories default to Axis::None; _Measure is bumped to Page
// so aggregated pivot views show a single measure by default
// (improvise-kos, improvise-709f2df).
// (improvise-kos, commit 709f2df).
assert_eq!(v.axis_of("_Index"), Axis::None);
assert_eq!(v.axis_of("_Dim"), Axis::None);
assert_eq!(v.axis_of("_Measure"), Axis::Page);
+2 -2
View File
@@ -76,8 +76,8 @@
//!
//! **A browser still never sends it**, and that is the right answer rather
//! than a gap: there is no file the user owns and no Ctrl-S semantics to
//! honour, so asking would be the **wrong question**. There is no file the user owns and no Ctrl-S semantics to
//! honour: the document *is* the OPFS entry, and a page that loses an edit
//! honour, so asking would be the **wrong question**: the document *is* the
//! OPFS entry, and a page that loses an edit
//! because nobody pressed a key has simply lost it. So the worker persists on
//! its own behalf: [`Worker::persist`] runs the one existing `Save` effect
//! through `App::apply_effects`, so effects remain the only mutation site and
@@ -15,8 +15,9 @@
//!
//! What it cannot see is OPFS. `MemoryFs` stands in, so what is exercised is
//! the `Storage` *contract* rather than the browser's implementation of it —
//! `OpfsFs` owes `storage_contract` and has never been run (gotcha 28,
//! improvise-xf1 deferred). Everything above the trait is real.
//! `OpfsFs` owes `storage_contract`, and `harness/` now runs exactly that
//! against it in a real browser (gotcha 28, improvise-vvl3/xf1). Everything
//! above the trait is real here.
use std::path::{Path, PathBuf};
+2 -2
View File
@@ -1,8 +1,8 @@
//! Run `improvise_io::storage::contract::storage_contract` against `OpfsFs`,
//! inside a dedicated Web Worker, in a real browser.
//!
//! `OpfsFs` has been compile-checked and never run (gotcha 45). This is the
//! instrument that changes that (improvise-vvl3, absorbing improvise-xf1). It
//! `OpfsFs` had been compile-checked and never run (gotcha 45). This is the
//! instrument that changed that (improvise-vvl3, absorbing improvise-xf1). It
//! is a *harness* crate, not part of the application: it ships in no bundle
//! and is not a workspace member.
//!
+1 -1
View File
@@ -1,6 +1,6 @@
// `storage_contract` against `OpfsFs`, in a browser (improvise-vvl3/xf1).
//
// `OpfsFs` has been compile-checked and never run. Everything found in a
// `OpfsFs` had been compile-checked and never run. Everything found in a
// browser so far was found by a person pressing keys, which is not a
// repeatable check. This is.
//
+64 -35
View File
@@ -5,8 +5,10 @@
// needed a browser. These are those three, plus the watchdog — which was found
// by reading, and only because someone went looking.
//
// `web/app.js` and `web/worker.js` have never been rendered or run (gotcha 48).
// Everything below is the first execution of either.
// Until this file, `web/app.js` and `web/worker.js` had never been rendered or
// run (gotcha 48). Everything below is the first execution of either — so this
// is the one place in the repo where "there is no browser here" is false, and
// the statement does not belong inside the artefact that falsifies it.
import { test, expect } from "@playwright/test";
@@ -83,7 +85,25 @@ test.describe("the shell reports what its peer is doing", () => {
// The other arm, which must keep working when the first one is fixed: a
// socket that really did open and then dropped IS a disconnect, and saying
// so is the whole reason `close` cannot simply be suppressed.
test("a socket that opened and then dropped reads as disconnected", async ({ page }) => {
//
// It runs past HANDSHAKE_TIMEOUT_MS on purpose. On this arm the watchdog IS
// armed — `armWatchdog` runs after `transport.ready` resolves, i.e. on
// `open` — so "disconnected" is written while a timer is pending, and the
// only reason it survives is that `close` goes through `setShellFailure`,
// which clears it. That was argued in a comment and asserted nowhere, and
// the argument was got wrong from the outside: a bug report claimed the
// timer fired 8s later and overwrote "disconnected" with "sent no
// document". It had been measured, but against a tree predating PR #21, and
// the "confirmation" that it survived PR #21 simulated only that PR's
// `opened` flag and not its other half — routing BOTH close arms through
// `setShellFailure`. A simulated fix that is not the fix you named
// reproduces a bug the real code does not have. Hence this assertion rather
// than another paragraph: proved discriminating by making `setShellFailure`
// skip its `clearTimeout`, which makes it fail.
test("a socket that opened and then dropped reads as disconnected, and stays that way", async ({
page,
}) => {
await page.addInitScript(logStatus);
// Closes on the page's first frame, so "it opened first" is true by
// construction rather than by timer.
await page.routeWebSocket("ws://127.0.0.1:8790/", (ws) => {
@@ -92,43 +112,52 @@ test.describe("the shell reports what its peer is doing", () => {
await page.goto("/index.html?ws=127.0.0.1:8790");
await expect(status(page)).toContainText("connected", { timeout: 15_000 });
await expect(status(page)).toContainText("disconnected", { timeout: 15_000 });
// Outlast the watchdog it was armed against, with margin.
await page.waitForTimeout(HANDSHAKE_TIMEOUT_MS + 2_000);
await expect(status(page)).toContainText("disconnected");
const log = await page.evaluate(() => window.__statusLog);
expect(log.some((t) => t.includes("sent no document")), `#status said: ${json(log)}`).toBe(
false,
);
});
// TWO BUGS THIS FILE DELIBERATELY DOES NOT ASSERT, and the reason is worth
// more than the assertions would have been.
// TWO BUGS THIS FILE ONCE DESCRIBED AS LIVE, AND WHAT HAPPENED TO EACH.
// The history is worth more than the assertions would have been, because
// one of them turned out never to have existed in the merged tree.
//
// 1. improvise-m18f: the socket's `close` listener is unconditional, so on
// a connection that never opened it overwrites "cannot reach …" with
// "disconnected".
// 2. On a connection that DID open and then dropped, nothing disarms the
// handshake watchdog, so 8s later "disconnected" is replaced by
// "accepted the connection but sent no document" — which is false. Same
// class as the bug `setShellFailure` fixed (a watchdog must be disarmed,
// not pre-empted, gotcha 48), and it SURVIVES the m18f fix, because that
// fix is about which message `close` writes rather than about the timer
// still running.
// 1. The socket's `close` listener was unconditional, so on a connection
// that never opened it overwrote "cannot reach …" with "disconnected".
// FIXED by PR #21. An `opened` flag is the only discriminator —
// `readyState` at close time cannot tell the two apart, and neither can
// the CloseEvent: both cases are code 1006, `wasClean: false`.
// 2. "On a connection that DID open and then dropped, nothing disarms the
// handshake watchdog, so 8s later 'disconnected' is replaced by
// 'accepted the connection but sent no document'." NOT A BUG, and never
// was on any tree that had PR #21. `close` routes both arms through
// `setShellFailure` (`app.js:248-250`), which clears the timer
// synchronously (`app.js:165-169`). It is now asserted directly by the
// open-then-dropped test above rather than described here.
//
// Both are real and both were measured, on macOS/Chromium:
// WHY THE SECOND ONE WAS BELIEVED, which is the part to keep: it was
// genuinely measured on macOS/Chromium — but against a tree based before
// PR #21. It was then reported as "surviving" that PR on the strength of a
// simulation that re-created only the `opened` flag, not the routing of
// both arms through `setShellFailure`. A simulated fix that is not the fix
// you named reproduces a bug the real code does not have, and it reads
// exactly like a confirmation. That is the same rule as
// doctrine(an_injected_fault_must_be_the_fault_you_named), one level up.
//
// never-opened: connecting… -> cannot reach -> disconnected
// open-then-close: connecting… -> connected -> disconnected
// -> accepted the connection but sent no document
//
// They were asserted here as `test.fail` expected-failures and **had to be
// removed**: neither reproduces on the GitHub runner, where `close` does
// not follow `error` within the window and the watchdog does not land. An
// expected-failure is green when the bug is present and red when it is
// absent, so one whose trigger is platform-dependent does not merely flake
// — it **inverts CI's meaning**, and it failed the build twice saying
// "expected to fail, but passed".
//
// What that leaves is a genuine limit rather than an oversight: the trigger
// for both is the ORDER AND TIMING of `error`/`close`/timer on the host's
// network stack, which is not a property of `web/app.js` and not something
// a harness can pin. The four assertions that remain in this file are the
// portable half — every message the page must write, and the one case where
// the watchdog must stay silent. Filed for the orchestrator with the
// recipe above; fixing them is `web/app.js`, which is not this unit's file.
// Both were once asserted here as `test.fail` expected-failures and **had
// to be removed**: neither reproduced on the GitHub runner, where `close`
// does not follow `error` within the window. An expected-failure is green
// when the bug is present and red when it is absent, so one whose trigger
// is platform-dependent does not merely flake — it **inverts CI's
// meaning**, and it failed the build twice saying "expected to fail, but
// passed". The lesson stands even though the second bug did not: assert the
// PROPERTY THAT MUST HOLD, which is portable, rather than the fault, whose
// trigger is the host's network stack.
// The first-frame watchdog (gotcha 46), found by reading. The condition is
// one no per-transport check would catch: the peer accepted the connection
+32 -14
View File
@@ -18,7 +18,7 @@ layer(rendering,improvise_main).
layer(protocol,improvise_protocol).
layer(fitting,gp_tree).
layer(fitting_bridge,improvise_gp).
location(add_keybinding,'crates/improvise-command/src/command/keymap.rs','default_keymaps()').
location(add_keybinding,'crates/improvise-protocol/src/keymap.rs','Keymaps::default_keymaps()').
location(add_command,'crates/improvise-command/src/command/cmd/registry.rs','default_registry()').
location(add_effect,'crates/improvise-command/src/ui/effect.rs','impl Effect').
location(formula_eval,'crates/improvise-core/src/model/types.rs','eval_formula / eval_expr').
@@ -38,13 +38,16 @@ location(wizard_render,'src/ui/import_wizard_ui.rs','ImportWizardWidget').
location(save_effect,'crates/improvise-command/src/ui/effect.rs','Save, save_to_current_path').
location(abort_chain,'crates/improvise-command/src/ui/effect.rs','AbortChain').
location(apply_effects,'crates/improvise-command/src/ui/app.rs','App::apply_effects').
location(sequence_dispatch,'crates/improvise-command/src/command/keymap.rs','Keymap::dispatch Binding::Sequence').
location(text_entry_builder,'crates/improvise-command/src/command/keymap.rs','TextEntry, buffer_of').
location(panel_keymap_builder,'crates/improvise-command/src/command/keymap.rs','PanelKeymap, bind_panel_toggles').
location(sequence_dispatch,'crates/improvise-command/src/command/keymap.rs','Keymap::dispatch, effects_of, Binding::Sequence flattening').
location(text_entry_builder,'crates/improvise-protocol/src/keymap.rs','TextEntry, TextEntry::with_parent').
location(panel_keymap_builder,'crates/improvise-protocol/src/keymap.rs','PanelKeymap').
location(records_layout,'crates/improvise-core/src/view/layout.rs','RecordRow, RecordCol, build_records_mode, records_display').
location(storage_trait,'crates/improvise-io/src/storage.rs','Storage, PhysicalFs, MemoryFs').
location(effect_kind,'crates/improvise-command/src/ui/effect.rs','EffectKind, ModelChange, Effect::kind').
location(prefix_resolution,'crates/improvise-command/src/command/keymap.rs','KeymapSet::prefix_keymap, SetTransientPrefix').
location(prefix_resolution,'crates/improvise-protocol/src/keymap.rs','Keymaps::prefix_keymap; SetTransientPrefix in improvise-command').
location(grid_frame,'crates/improvise-protocol/src/render.rs','AxisFrame, FrameEntry, header_level_widths').
location(browser_harness,'harness/','playwright.config.js; tests/shell.spec.js (chromium); tests/opfs.spec.js (chromium+firefox; webkit has no OPFS)').
location(storage_contract,'crates/improvise-io/src/storage.rs','contract::storage_contract -- public, not cfg(test)').
location(wasm_ci,'.github/workflows/ci.yml','wasm job').
location(view_state_membership,'crates/improvise-command/src/ui/app.rs','ViewState doc comment states both membership tests; App carries the session-local residue tag').
location(axis_presentation,'crates/improvise-core/src/view/axis.rs','plain_label/glyph/short_label/initial/color/next on Axis; AxisColor semantic palette').
@@ -58,9 +61,9 @@ invariant(stale_layout,'app.layout rebuilds only after a whole effect batch; pre
invariant(wizard_back_is_history_stack,'advance pushes the step it leaves onto ImportWizard::history; previous pops it, so back retraces the path actually taken through the conditional transitions rather than re-deriving a reverse table').
invariant(wizard_validate_where_fixable,'preconditions live in validation_error() and are checked on the step where the user can still act -- never deferred to build_model after the final confirm').
invariant(wizard_done_is_confirm,'NameModel is terminal for advance; Enter there calls build(), which parks the workbook in ImportWizard::built on Done. AcceptWizardModel takes it. previous() from Done discards built').
invariant(failed_save_aborts_batch,'Save sets app.abort_effects on failure (no file path, or write error), so effects chained after it -- the Quit in wq/ZZ -- are skipped and the error stays on screen').
invariant(an_abort_stops_the_chain_not_the_bookkeeping,'Three effects set abort_effects: Save and SaveAs on a failed write or missing path (improvise-8tk, both arms), and AddCategory on refusal (improvise-2zq). An abort at the TAIL of a batch also ate mode restoration and buffer clearing, stranding the user where draw_bottom_bar renders the minibuffer instead of the status line, so the error was never drawn. Tidy-up therefore goes at the HEAD, where an abort cannot reach it -- ordering rather than a runs-anyway flag. An abort is not a rollback.').
invariant(sequence_one_batch,'Binding::Sequence flattens all step effects into one Vec applied by a single apply_effects call, so abort_effects carries across sequence steps').
invariant(minibuffer_spine,'Seven text-entry modes built by TextEntry builder in keymap.rs; SearchMode hand-written').
invariant(minibuffer_spine,'Eight text-entry modes carry a MinibufferConfig. Seven are built by TextEntry::new in improvise-protocol/src/keymap.rs; RecordsEditing is TextEntry::with_parent off Editing, which is why a count taken off the ::new call sites reads seven. SearchMode is hand-written -- no commit key.').
invariant(records_mode_all_none,'ToggleRecordsMode puts every regular category on Axis::None, so EditOrDrill regular_none is always true in records mode and the edit branch depends entirely on is_synthetic').
invariant(buffer_name_single_source,'the buffer a text-entry mode types into is named once, in that mode AppMode constructor MinibufferConfig; TextEntry reads it off the mode so the prompt and the keystrokes cannot address different buffers').
invariant(panel_keymap_spine,'the three panel modes share a spine built by PanelKeymap::new(panel): Esc/Tab to Normal, arrows and j/k to move-panel-cursor for that panel, every panel toggle key, and : for command mode. Content keys come through chained .bind/.bind_args/.delete because they vary in shape not just command name').
@@ -70,10 +73,10 @@ invariant(records_dim_is_column_index,'the synthetic records CellKey stores the
invariant(storage_is_two_ops,'persistence only ever reads or writes one whole document, so the Storage trait is read/write bytes and nothing else; a backend that is not a filesystem implements it without directories, metadata or seeking').
invariant(gzip_is_a_name_property,'the .gz suffix selects gzip in save_to/load_from, so it is a property of the document name and not of the storage backend').
invariant(effect_kind_no_default,'Effect::kind has no default impl, so a new effect cannot forget to classify itself; the failure mode would be silent (a mis-declared ViewOnly edit never reaches connected clients)').
invariant(view_lives_in_model_state,'the View is stored inside model_state.workbook, so touching model_state does NOT imply a model change: axis/page/format are layout_geometry, cursor and scroll are per-client view_only, Save/SaveAs/MarkDirty are bookkeeping').
invariant(view_lives_in_model_state,'The View is stored inside model_state.workbook, so touching model_state does NOT imply a model change: axis/page/format are layout_geometry, cursor and scroll are per-client view_only, Save/SaveAs are bookkeeping and ViewOnly. MarkDirty no longer exists (improvise-gv9r): App::apply_effects raises model_state.dirty for any EffectKind::Model(_), so a document change that omits it is unrepresentable rather than validated.').
invariant(view_state_is_a_projection,'the server does NOT serialize ViewState to maintain a client mirror -- it applies the same commands through reduce_view (improvise-cqq). Steady-state traffic is Commands only; ViewState is serialized solely for the bootstrap Snapshot').
invariant(bootstrap_excludes_host_state,'because the only serialization point is the on-connect Snapshot, host-only state must not live in ViewState: a fresh client should not inherit a half-open import wizard, an armed prefix key, or another session staged drill edits').
invariant(prefix_is_an_address,'ViewState::transient_prefix stores the (KeyCode,KeyModifiers) that opened a prefix; KeymapSet::prefix_keymap resolves it, walking parents. Assumes prefixes are FLAT -- guarded by prefix_keymaps_are_flat').
invariant(prefix_is_an_address,'ViewState::transient_prefix stores the (KeyCode,KeyModifiers) that opened a prefix. The table and Keymaps::prefix_keymap are in improvise-protocol/src/keymap.rs; improvise-command wraps them as KeymapSet::prefix_keymap and owns dispatch. Resolution assumes prefixes are FLAT -- guarded by prefix_keymaps_are_flat.').
invariant(compile_check_is_not_runtime_proof,'the wasm CI gate is cargo check, so it catches nothing that merely panics at runtime: Instant and std::fs both COMPILE for wasm32-unknown-unknown').
invariant(effects_never_touch_fs,'every document read or write in the command layer goes through App::storage; production code in improvise-command contains no std::fs').
invariant(io_keeps_path_wrappers,'improvise-io keeps path-taking wrappers (save, load, export_csv, parse_csv_with_warnings) delegating to _to/_from/_bytes variants, so the native CLI is unchanged while the byte-level path is backend-agnostic').
@@ -87,19 +90,26 @@ invariant(basis_has_no_default_constructor,'a zero-valued Basis is not \"no clai
invariant(basis_is_coordination_not_authentication,'Basis derives Deserialize and has public fields, so any claim can arrive on the wire. Rejection::RecordUnstated closes OMISSION, not INVENTION. BasisCache::claim being the only constructor is a statement about clients written against this crate, not about the wire.').
invariant(the_server_decides_which_records_are_checked,'reduce.rs::records_named is an exhaustive read of the ModelCommand, 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 claim would be a force by omission: the record tier skipped, no flag to remove, nothing in the message saying so.').
invariant(grid_extent_is_the_whole_grid,'project::grid_of reads GridExtent from GridLayout::row_count/col_count, never from the clipped viewport ranges. The TUI builds its cache at offsets zero so the two coincide there -- the bug is only visible in a browser.').
invariant(client_frame_commands_is_a_batch,'one keypress resolved client-side is SetSelected plus the offsets viewport_effects returns; a host that folds one and not the other projects the wrong window. Host::commands applies in order and abandons at the first rejection.').
invariant(opfs_flat_root,'OpfsFs::list returns BARE names where MemoryFs and PhysicalFs return dir-joined ones, so Path::new(\"\") is the one dir at which all three agree and the only one at which storage_contract describes OpfsFs. A worker using a subdirectory gets an empty picker in a browser and a full one in every test.').
invariant(async_seam_does_not_survive_boxing,'OpfsFs::open/attach/refresh/commit_deletes are on the struct, not on Storage, and App::storage is a Box<dyn Storage>. Handing the backend to the App puts the seam out of reach; impl Storage for Rc<OpfsFs> is refused by the orphan rule, so each host needs a local newtype.').
invariant(client_frame_commands_is_a_batch,'One keypress resolved client-side is SetSelected plus the offsets viewport_effects returns; a host that folds one and not the other projects the wrong window. Since improvise-955n a frame is answered WHOLE by CheckedBatch::against before anything is applied, so Applied::Rejected is unreachable through Host::commands. Clause 4 (all-or-nothing on an apply-time failure) is NOT delivered: App::apply_effects aborts without undoing.').
invariant(opfs_index_holds_documents_not_entries,'OpfsFs::list joins dir like the other two backends, so storage_contract takes dir as a parameter and the three agree at EVERY directory (improvise-vhcz), not only where joining is the identity. The index holds DOCUMENTS: attach indexes nothing (what it creates is zero bytes), write indexes the name and un-queues a pending delete, refresh rebuilds from OPFS skipping zero-byte entries but counting an unmeasurable one as a document, since the only cause is another tab holding the exclusive handle (improvise-vvl3).').
invariant(async_seam_does_not_survive_boxing,'OpfsFs::open/attach/refresh/commit_deletes are on the struct, not on Storage, and App::storage is a Box<dyn Storage>: handing the backend to the App puts the seam permanently out of reach. The host holds the concrete backend and gives the App a shared handle, which is impl<T: Storage + ?Sized> Storage for Rc<T> in improvise-io (improvise-hmws) -- there because the trait is local there, blanket because nothing about it is OPFS-shaped, and Rc not Arc because wasm32 has one thread.').
invariant(shipped_demo_cells_are_records,'web/demo.improv projects only CellId::Record; guarded twice, in tests/web_demo_model.rs against grid_of and in post_message.rs against the real seed-VFS-open-connect-Snapshot chain.').
invariant(matching_cells_yields_insertion_order,'DataStore::matching_cells/matching_values return the primary IndexMap order. Measured at 18,000 cells: +20% worst case on matching_values, matching_cells 7% FASTER because sorted positions resolve CellKeys with better locality.').
invariant(a_stored_fit_names_only_what_the_document_holds,'fitted_formula refuses a tree splitting on a feature Model::find_item_category cannot resolve. It asks the DOCUMENT, not the name shape: a measure literally named Region=North is legal and indistinguishable from a one-hot as a string.').
invariant(the_frame_is_answered_once_not_the_command,'Since improvise-955n, CheckedBatch::against answers a whole ClientFrame::Commands before anything is applied. The two DOCUMENT tiers (document_generation, layout_version) hoist to the frame; the RECORD tier does not, because which records are at stake is a property of the command (records_named). Hoisting all three was a lost update: [SetCell(North), SetCell(South)] overwrote a South another client had moved, silently, invisible to the other two tiers because a SetCell is CellData. Every mutation answers for its own records against the FRAMEs basis -- never its own field, or a later command could claim a fresher picture than the frame was composed under.').
invariant(the_create_exemption_is_a_property_not_a_list,'A record that NEITHER resolves in the document NOR is claimed by the basis is one the frame is about to create, and only that combination is exempt from the record tier -- because BasisCache::claim filters to records the cache was told about, so [AddItem(East), SetCell(East)] could not claim one however it is ordered. A record that does not resolve but IS claimed is still RecordGone.').
invariant(a_cats_change_reidentifies_every_position,'A cells identity is AxisFrame::cats zipped with its label tuple, so GridCache::invalidate_relabelled blanks the WHOLE axis when old.cats != new.cats -- not the positions whose tuples differ -- and blanks the totals on a recategorization of EITHER axis, since a column total sums over the rows. It is applys job and not the producers, for the reason the whole fold works that way: an update must be correct with no follow-up behind it (gotcha 33b).').
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.').
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,'workspace suite must stay under 2 seconds').
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).').
doctrine(coverage,'aim ~80% line/branch on logic; skip ratatui render paths').
doctrine(test_contract,'test the observable contract, not the internal storage').
doctrine(proptest_cases,'keep proptest at default 256 cases; constrain strategies instead of brute force').
doctrine(binding_dump_diff,'when refactoring keymap construction, dump every (mode,key,modifier) lookup for all 16 modes before and after and diff: most bindings have no direct test, so a green suite is not evidence').
doctrine(binding_dump_diff,'when refactoring keymap construction, dump every (mode,key,modifier) lookup for all 17 ModeKeys before and after and diff: most bindings have no direct test, so a green suite is not evidence').
doctrine(derive_then_verify,'when classifying many items from source, extract facts mechanically and reason in Prolog, but VERIFY the extraction: multi-line method chains and free-function helpers defeated naive regexes and mis-classified SetCell, SortData and ApplyAndClearDrill as view-only. A line-oriented awk over effect.rs likewise reports 30 Model effects where brace-matching the impl blocks gives the true 29 -- HelpPageSet is ViewOnly and the running variable bleeds across the #[cfg(test)] boundary.').
doctrine(mentions_are_not_edges,'never bulk-add dependency edges from prose mentions; direction must be read per pair, and blind addition can invert real edges or create cycles').
doctrine(kb_is_code,'the knowledge base is executable, so audit it like code: check rules actually reject what they should, and check location/3 symbols still exist in the tree').
@@ -129,6 +139,7 @@ doctrine(a_documented_door_with_no_caller_is_the_dangerous_kind,'reduce_full was
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(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.').
doctrine(an_order_test_needs_two_histories,'a single-process test cannot tell \"sorted correctly\" from \"hashed that way today, on this machine\". Compare against the insertion-ordered structure itself, or build the same logical data by two DIFFERENT insertion histories and require each to list its own. Corollary of check_the_checker.').
doctrine(a_helper_that_builds_the_world_the_code_wants_tests_nothing,'improvise_eval FABRICATED a _Measure item per feature name, so it proved the emitter and gp_tree::eval agree against a model that could not exist. The fix is not to delete such a test -- it checks real things -- but to state its scope and assert the PRECONDITION separately against a real fixture. Pair a refusal test with an accepting one, or the refusal can pass by refusing everything.').
doctrine(a_session_local_widget_gets_a_cache_and_no_projection,'a Projection is addressed to a widget on EVERY client, which is right exactly when the change is addressed to NO ONE session. A half-open dialogue belongs to precisely the session that opened it, so WizardCache is built and drawn locally with no Projection. Same argument that removed AcceptWizardModel from ModelCommand, applied to rendering rather than to mutation.').
@@ -141,6 +152,12 @@ doctrine(a_watchdog_names_the_condition_not_the_peer,'a worker that catches its
doctrine(a_demo_is_checked_against_the_type_not_the_axes,'ask grid_of what a cell IS -- a CellId variant, per ADR 002 -- never re-derive the rule from the view config: a second derivation can agree with itself while disagreeing with the client. The assertion is written BEFORE the model, and proved discriminating against the model that fails it.').
doctrine(being_the_second_consumer_audits_the_abstraction,'improvise-9x6 framed the worker as a wasm-flavoured analogue of the socket server, and the transport work was nearly free. The four real obstacles were Host in the wrong crate, Save unreachable from the wire, dirty never set by a wire edit, and an async seam unreachable through a Box -- and THREE OF THE FOUR were gaps in the SHARED layer that the socket server also had and had never noticed. A second real consumer is what audits an abstraction; that is a stronger argument for having something to test than any specific check.').
doctrine(a_load_bearing_order_needs_an_order_test,'an ordering documented as load-bearing and held up only by the order of two statements is a check nobody has seen fail. Projection::Basis is emitted after the grid updates whose counters it states; that was asserted in three doc comments and tested nowhere until a test was added and proved by injecting the swap.').
doctrine(a_fact_stated_twice_is_changed_by_grep,'When a fact is stated in more than one place, changing it is a grep and not an edit -- and the obligation attaches to the FACT, not to the edit: writing a NEW statement of an existing fact obliges you to find the places that already state it. A new paragraph restating an existing fact is a second derivation, in prose, where the compiler cannot see it. Evidence at 22d43b2: gotcha 44 was rewritten in design-principles.md and in src/ui/grid.rs and left stale in context/repo-map.md; "there is no browser here" survives in seven places after harness/ made it false, including in the spec file that falsifies it.').
doctrine(a_reviewed_paragraph_is_not_evidence,'A recipe in a doc is code and must be run in the head. The AGENTS.md dead-ID audit scanned only context/repo-map.md and context/adr/*.md, so it found a dead id in the map and missed both copies in harness/tests/shell.spec.js; its [a-z0-9]{3,4} class could not see a commit SHA wearing the improvise- prefix; and its exclusion list named only core and wasm. A verified SPELLING is not a verified BEHAVIOUR: playwright.config.js lists three projects, and exactly one spec runs on more than one engine while a third engine has no OPFS at all.').
doctrine(a_fast_per_position_accessor_invites_the_loop_it_replaces,'AxisFrame::shows_level materializes every item on every call and is deliberately NOT made cheaper, because a cheap per-position form is an invitation to the quadratic draw loop header_labels exists to replace. The per-position accessors survive for one-off lookups and say so in their own doc.').
doctrine(a_derivation_correct_per_element_can_be_wrong_for_a_set,'improvise-kmgs derived ModelCommand from the model effects ONE AT A TIME, which was right per effect and put SetCell on the wire without its MarkDirty companion -- so "save if dirty" was a trigger that never fired and a wire edit was lost silently (improvise-gv9r). The repair was not a second table but raising the fact where it cannot be omitted: App::apply_effects reads dirtiness off the same EffectKind the Dirty and layout_version are read off.').
doctrine(reachability_is_a_reason_not_to_rush_not_a_reason_not_to_notice,'CheckedBatch documents clause 4s hole -- a chain whose later command fails at apply time leaves its predecessors applied, and silently -- rather than closing it, because no client in the workspace can compose it. The lost update the same commit FIXED had exactly that reachability argument and none of the documentation, which is how it shipped.').
doctrine(a_ticket_id_and_a_commit_sha_must_not_share_a_prefix,'crates/improvise-core/src/workbook.rs cited a git commit sha wearing the improvise- ticket prefix. A reader runs bd show, gets nothing, and concludes a follow-up was lost. A dead ID is worse than no ID; an ID-shaped non-ID is worse again, because the audit loop reports it identically.').
work_done('improvise-8tk',bug_fix,'ZZ/:wq quit even when the save failed, silently discarding the model. Save now aborts the effect batch on failure; save_to_current_path/1 returns Result<String,String>; three tests in ui/effect.rs.').
work_done('improvise-907',bug_fix,'Import wizard step machine redesign: validation_error() gate at ReviewProposals (no-category and over-limit), history-stack previous() bound to Left on every step, and Done made a real confirmation step holding the built workbook. Superseded improvise-gfq, improvise-o1c, improvise-c6j.').
work_done('improvise-6sq',wont_do,'Merging EditOrDrill and EnterEditAtCursorCmd into one EnterEdit{mode} is not safe: they differ in WHEN the cell value is read, not in a mode parameter. Added ui/app.rs::tests::i_and_o_diverge_on_an_empty_records_grid as executable evidence.').
@@ -351,6 +368,7 @@ groom_check(unrecorded_dep,'prose naming another issue is a CANDIDATE, never a f
groom_check(stale_citation,'file:line references rot; resolve each against the tree and prefer symbol names when correcting').
groom_check(dissolved_premise,'grep for the pattern an issue targets before working it -- improvise-ont described 11 call sites of a pattern that no longer occurs anywhere').
groom_check(dissolved_premise_test_gap,'a test-gap ticket rots differently from a code ticket: the gap can be closed by someone else work without anyone touching the ticket. Re-run each item against the test names before working it -- 2 of 7yx five items were already covered, by jc8 and by an import fix.').
kb_issue_found(claude_md_tail_is_generated_not_duplicated,'The ~47-line beads tail is byte-identical in AGENTS.md and CLAUDE.md, which reads as the duplication doctrine(a_fact_stated_twice_is_changed_by_grep) forbids. It is not: both copies sit inside <!-- BEGIN BEADS INTEGRATION v:1 profile:minimal hash:ca08a54f --> markers, written by bd setup codex and bd setup claude respectively, and refreshed by bd against that content hash. A process change there is one tool invocation, not two hand edits, and replacing either copy with a pointer makes bd setup <recipe> --check report it modified and the next run re-add it. Checked 2026-09-20 after a doc review proposed the pointer; declining is the right answer and this fact exists so the next reviewer does not re-propose it.').
kb_issue_found(host_rpc_shims,'sleep/delay/console_log/js_eval/future/await_any/future_cancel were the MCP runtime own predicates, captured into this file by a saveSession. Removed: not project knowledge, and the source of an overwriting warning on every consult. If saveSession re-adds them, strip them again.').
kb_issue_found(may_depend_on_type_confusion,'may_depend_on(A,B) :- crate(A,C,D), \\+forbidden(A,B). C and D were singletons and B was never constrained, so may_depend_on(improvise_core, banana) succeeded. forbidden/2 second argument is a CONCERN, not a crate. Replaced by may_use/2 over an explicit concern/1, plus real crate_depends_on/2 facts read from the Cargo.toml files.').
kb_issue_found(before_fixing_dead_guard,'before_fixing(A,B) :- A=C, doctrine(bug_first,B). A=C bound A to a fresh variable and constrained nothing, so the rule succeeded for any first argument. Now requires work_done(Bug,bug_fix,_).').