Merge pull request #165 from fiddlerwoaroof/docs/kb-fmt-and-collapse

docs: the fmt gate every brief prescribed, and the collapse ruling

AGENTS.md

@@ -105,10 +105,30 @@ Two agents each adding a workspace member will still collide in
    noticed because it was told to look. `git log --oneline -1` against a named
    commit, reset onto `main` if behind. Name the expected commit in the brief.
 2. **The baseline test count**, so "1090 passing" is checkable rather than
-   asserted, and a drop is visible.
-3. **Do not push, do not merge to `main`.** The orchestrator merges.
-4. The commit-message trailers, spelled out.
-5. **What to report**: branch and commits, what changed per ticket, *what could
+   asserted, and a drop is visible. Tell the agent to **measure its own** and
+   cite the machine: the same commit measured `improvise-io`'s binary at 1.4s on
+   one machine and 3.7–5.2s on another, so a figure with no machine is a claim
+   about the suite, which it is not.
+3. **The gate list, and it must not say `cargo fmt --all`.** That reaches only
+   **one** of this repo's two workspace roots — `harness/opfs-contract` declares
+   its own `[workspace]` — and the obvious repair is worse: `cargo fmt
+   -p opfs-contract` from the repo root exits **1**, *"not a member of the
+   workspace"*, which is indistinguishable from a real formatting failure
+   (improvise-b4ks). Both spellings fail in the direction that looks like
+   success. The gate is:
+
+   ```
+   direnv exec . ./scripts/fmt-workspaces.sh   # exit 2 = the gate did not run
+   ```
+
+   Every gate goes through `direnv exec .` — `./scripts/checkers.sh` scrubs the
+   environment to an allowlist, so a bare invocation drops `tpl` off `PATH` and
+   reports spurious failures. And **never pipe a gate**: `$?` must be the gate's
+   own, and `check-comment-runs.py --check-evidence` prints two lines with the
+   comment cap **first**, so `| tail -1` silently drops half of it.
+4. **Do not push, do not merge to `main`.** The orchestrator merges.
+5. The commit-message trailers, spelled out.
+6. **What to report**: branch and commits, what changed per ticket, *what could
    not be verified and why*, and proposed repo-map/`kb.pl` text. Ask explicitly
    for the thing the agent learned that the ticket did not predict — that is
    consistently the most valuable part of a hand-back, and agents omit it unless

prolog-sessions/kb.pl

@@ -137,6 +137,7 @@ invariant(the_creating_columns_insert_and_so_suppress_the_advance,'A +cat or +me
 invariant(a_reported_share_cannot_overflow_its_scale,'gp_tree::fit::importance_ranking sums and divides in f64 and narrows only the resulting share, which is bounded by 100 and therefore always finite (improvise-k844). TWO overflows, not one: 100.0*imp[j] associates left so a lone importance over f32::MAX/100 is inf before the divide that would have returned it to range; and the f32 TOTAL itself overflows when several are large, which reassociating to 100.0*(imp[j]/total) turns from NaN% into a SILENT 0% FOR EVERY FEATURE. f64 closes both, since no sum of finite f32s can overflow it. The residual guard answers only an importance that ARRIVES non-finite -- an infinite total apportions nothing, so the ranking is empty rather than NaN, and note NaN <= 0.0 is false so the pre-existing early return passed NaN straight through. Reachable PAST improvise-pv3g because both callers score importance in the target OWN units, making it an MSE difference that grows as the target square.').
 invariant(viewport_extent_is_external_on_both_front_ends,'The viewport extent is an EXTERNAL INPUT that reaches the document, and it is not a browser problem. run_headless_commands reads crossterm::terminal::size() into app.term_{width,height} and CmdContext::{visible_rows,visible_cols} derive from it: 80x24 gives visible_rows 16, 200x60 gives 52. Measured on a 40-row grid, the SAME command sequence -- one PageDown then clear-cell at the cursor -- deleted R12 at 80x24 and R39 at 200x60. So Client::set_viewport is not a browser-introduced input; it is the browser SPELLING of one the TUI has always had, and gotcha 47 \"not derivable\" is true of both front ends. Any ADR treating extent as a browser problem produces a log that replays in a browser and not in a terminal. Cheapest of the five external inputs to close -- one pair of integers with an existing wire spelling, ClientFrame::Viewport -- but it must land in the SAME CHANGE as improvise-6uva, which currently masks it. A test must drive TWO sizes; one cannot see this.').
 invariant(aggregate_hash_is_layout_scoped,'A RecordHash falls back to DataStore maintained index and so resolves against the DOCUMENT, which is why a connected client SetCell works from any view. An AggregateHash has no such index AND CANNOT HAVE ONE -- an aggregate has no existence outside the layout that derives it. So App::resolve_aggregate answers only for the HOST active view, which since improvise-jytm is not the client. Guarded by an_aggregate_a_session_was_shown_does_not_resolve_against_the_hosts_view, whose CONTROL is the load-bearing half: the same surrogate resolves once the host is moved onto that view, so a blanket None -- a typo in the hash, an empty layout -- cannot pass.').
+invariant(fmt_gate_exit_two_means_the_gate_did_not_run,'scripts/fmt-workspaces.sh: exit 1 NAMES A FILE and blames the tree; exit 2 names none and blames the invocation. Every exit-2 cause is checked ABOVE the formatter loop, so the two cannot be confused. The contract is deliberately \"did the gate run\" rather than \"what kind of failure was it\": telling a rustfmt diff from a parse error means PARSING RUSTFMT PROSE, which is the check-adjacent-to-its-subject shape this repo keeps losing to. Measured while narrowing it: an unparseable source exits 1, identical to a diff, and an unknown rustfmt.toml option is only a warning at 0. Caught by ChatGPT Codex, which found the script committing its own ticket defect via a blanket \`|| rc=1\`.').
 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).').
@@ -379,6 +380,9 @@ doctrine(a_disjunctive_assertion_can_accept_the_bug_it_guards,'a_string_that_is_
 doctrine(a_law_asserted_over_an_immutable_context_is_a_tautology,'law_toggle_group_involution called execute twice against the SAME immutable CmdContext, so it asserted f(x)==f(x) and NO RIVAL OF ANY KIND could fail it -- not merely the one the ticket named. Vacuous twice over, since the fixture had no groups and both sides were the empty string. A Cmd is pure over its context BY CONSTRUCTION (design-principles section 1), so any property stated by calling one twice without applying the effects between is restating that purity. Worse, applying the effects here REFUTED the law rather than rescuing it: collapsing removes the group data rows, so the cursor then names the NEIGHBOUR. Applying the effects is the minimum for such a test to say anything, and it may reveal the law is false.').
 doctrine(a_rival_must_fail_the_property_not_the_setup,'When a test setup shares a classification with the thing under test, a rival takes down the SETUP and the red proves the neighbouring question. improvise-en4s: AddFormula and RemoveItem are BOTH EffectKind::Model(CellData), so tightening the CellData arm meant the formula never reached the client and the test failed at \"the client must be holding the formula before it can lose it\" rather than at the deletion property. Fix: move the setup off the shared path -- seed the formula BEFORE Sessions::connect, so it arrives in the bootstrap seed, which no Dirty gates. Check WHERE a rival makes a test fail, not merely THAT it does.').
 doctrine(every_agent_appends_to_the_same_evidence_file_at_eof,'Four evidence-file conflicts in one batch of ten PRs -- command.md twice, gp.md, core.md -- because context/test-evidence/<area>.md is append-at-EOF and every agent touching an area lands there. They are LOUD conflicts and get noticed; the hazard is the RESOLUTION, since --check-evidence cannot see a dropped entry (it checks that every heading names a running test, and that a test CITING the file has an entry -- a test that cites nothing loses its entry silently). So resolve by keeping both sides and VERIFY LINE-BY-LINE against :2 and :3 rather than reading the merged text. Expect this conflict rather than treating each instance as bad luck; it is structural.').
+doctrine(a_family_of_bugs_means_the_question_is_wrong,'improvise-lhi4, improvise-0o6x and improvise-6qet are three beads about the same missing way back out of a collapsed group. The first two both fixed the header VISIBILITY -- and AxisEntry doc comment promise that a GroupHeader is always visible \"so its label and collapse state show\" became true in both halves without the header becoming ADDRESSABLE. Three people re-finding one gap is evidence that each fix answered a NEIGHBOURING question, and the cure was to ask what the user actually expected (the cursor follows the content) rather than to fix a third symptom. So: when a third ticket arrives in one family, stop fixing and re-derive the question -- and expect the answer to be a shape none of the accumulated options list, because the options were all generated under the wrong framing.').
+doctrine(a_stub_must_fail_in_the_shape_of_the_real_fault,'A test double that breaks MORE than the real fault breaks satisfies the test through a DIFFERENT arm and reads exactly like coverage. Found three times on improvise-b4ks, each time only by injecting a rival: a cargo stub that failed for EVERY argument also broke cargo metadata, so the run exited on the metadata arm and the rustfmt probe could be DELETED with the suite green. The stub has to fail where the real thing fails and succeed where it succeeds; otherwise the assertion is answered by a path that will never occur in production.').
+doctrine(a_checker_fixture_must_carry_the_structure_the_hazard_needs,'A simplified fixture tests a repository that does not exist, and a green suite over one is the same non-evidence as a vacuous assertion. On improvise-b4ks the fixture was vacuous three times: with no PATH DEPENDENCY, cargo fmt --all had nothing to leak through; with a VIRTUAL outer manifest, the bare --manifest-path form happened to reach every member. Each time the structural feature the hazard depends on had been simplified away, and only an injected rival showed it. So enumerate what the hazard NEEDS -- here two workspace roots, a path dependency crossing them, and an outer root that is itself a package -- and build the fixture to carry all of it.').
 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.').
@@ -534,6 +538,8 @@ decision('improvise-rr8n',keep_the_table_derived_from_what_it_describes,'The tic
 decision('improvise-t14d',an_affordance_outranks_a_decoration,'draw.rs::fill_line takes a Keeps parameter rather than a global rule, because the three rows sharing a composed line genuinely disagree about which side yields. The title bar keeps the RIGHT -- the ?:help/:q escape hatch outranks an unbounded document name -- while minibuffer_line and draw_status keep the LEFT, which is what they already did, since right-priority there would let a long status message eat the text the user is editing. So the ticket \"give the right segment priority\" was correct ONLY for the title bar. Second fault, and the ticket framed it wrongly: padding by str::len mis-padded EVERY document including pure-ASCII ones, because the separator dot is in draw_title own format string at two bytes and one cell -- not a non-ASCII-NAME problem as filed. Widths now go through ui::text::truncate.').
 decision('improvise-6uva',a_registry_with_two_arms_demotes_the_weaker_caller,'CmdRegistry has three registration arms and NOTHING TYPE-CHECKS WHICH ARM A CALLER DESERVES. Three instances surfaced in one fix, and they do not point the same way, which is the part to carry. (1) The pure arm demoted every Move: parse built CursorState::default() so a script cursor reset instead of moving. (2) register_pure two arms DISAGREED WITH EACH OTHER -- interactive refused empty args, pure accepted them -- so \"w\" parsed one way and refused the other. (3) Found by ChatGPT Codex: the interactive arms of clear-cell/yank/paste/drill-into-cell took |_args, ctx| and DISCARDED EXPLICIT COORDINATES, so after the fix \"clear-cell Region/East\" cleared the origin. HERE THE WEAKER ARM WAS THE ONE THAT HAD THE CONTEXT, which is why \"always prefer the interactive arm\" is not the lesson. Make the arms agree, or make the difference the point; resolved for the cell-key commands by key_from_args_or_cursor -- explicit coordinates win, cursor is the fallback.').
 decision('improvise-55v2',a_borrowed_str_in_a_wire_adaptor_narrows_the_format,'Repr::Nonfinite(&\'a str) in an untagged serde adaptor can only be produced by a deserializer that BORROWS FROM ITS INPUT, so the spelling round-tripped through from_str and from_slice and through nothing else. Measured before fixing: {\"Number\":\"inf\"} parsed while {\"Number\":\"in\\u0066\"} -- THE SAME JSON DOCUMENT, since escaping is not a distinction JSON makes -- was refused, and to_value/from_value did not round-trip the adaptor own output. So the wire accepted a SUBSET of the encodings of the spelling it claimed to share with the .improv file. Cow<\'a, str> fixes all four paths and leaves the serialized form unchanged. THE TESTS COULD NOT SEE IT because both used from_str exclusively, which is precisely the one path that borrows -- a check adjacent to the question it names, arriving inside the change that DEFINES the question. Found by adversarial review, confirmed by injection.').
+decision('improvise-6qet',a_collapsed_group_is_one_aggregate_row,'USER RULING 2026-09-23: \"When collapsing a group, I would generally expect the cursor to move to whichever place the group collapsed into.\" That reframes the bug -- it is not \"the GroupHeader is not addressable\", it is \"THE CURSOR DID NOT FOLLOW THE CONTENT IT WAS ON\", and the ticket four options were all answers to the wrong question. CHOSEN SHAPE, a fifth none of them listed: expand_category stops SKIPPING a collapsed group items (the bare \`continue\` at layout.rs:755, which is the whole mechanism of the bug) and emits ONE DataItem for the group, whose cell is an Aggregate. It DISSOLVES the problem rather than working around it -- the cursor lands on the collapsed group BY ARITHMETIC with no special case, z becomes an involution again without anything being taught about involution, ViewCursor and row_group_for and every cursor-addressed command are untouched, and settability is already answered by ADR 002 since grid_of returns CellId::Aggregate. It is also what a pivot table MEANS by a collapsed group: the totals in one row, where today collapsing shows nothing in their place. Rejected: a cursor that names a visual entry (literally delivers the ruling but changes row_group_for signature and forces every cursor-addressed command to define a header row), and resolving against the visual stream (narrow, but only ACTS as if the cursor had moved).').
+decision('improvise-b4ks',fmt_per_root_by_member,'This repo has TWO workspace roots -- harness/opfs-contract declares its own [workspace] -- and BOTH obvious spellings fail in the direction that looks like success, each measured. \`cargo fmt --all\` (or --manifest-path <root> --all) CROSSES THE BOUNDARY, because --all formats local PATH DEPENDENCIES and harness/opfs-contract depends on improvise-io by path, so the inner root reported src/draw.rs; this is wider than the ticket note, which blamed only cd-then---all. And bare --manifest-path is BLIND TO MEMBERS, since the repo root is a package as well as a workspace -- an injected fault in crates/improvise-core exited 0. The only sound spelling is \`-p <each member>\` scoped by that root own manifest. Note \`cargo fmt -p opfs-contract\` FROM THE REPO ROOT exits 1 with \"not a member of the workspace\", indistinguishable from a real verdict, which is the ticket itself; from INSIDE that root it is correct, which is why CI harness-lint job was always sound while the root-level gate every brief prescribed was checking less than it claimed.').
 wire_carries(command,both_directions,'user intent up, projection commands down').
 wire_carries(snapshot,on_connect_only,'improvise-q08: full ViewState + render cache filled from current viewport').
 wire_carries(command,both_directions,'user intent client->server, projections server->client').