main
106 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Edward Langley
|
9976486d69 |
docs: the help entry no longer contradicts itself
Found by review on #75. Renaming the heading to the test PR #72 restored made check-evidence green, and left the body three paragraphs down still saying "The test no longer exists" and "nothing replaced the assertion" -- one entry making mutually exclusive claims about whether the coverage exists. The checker could not see it, and that is not a gap in the checker: it validates heading-to-test and says nothing about the prose beneath, which is what a human reads. A mechanical check licenses exactly the claim it makes and no neighbouring one. The paragraph now records the gap as history -- gone for a release, invisible because the checker then read only ### headings and only Rust names, restored by #72 with improvise-igxp's stronger assertions -- and points at help.spec.js's own entry rather than promising or denying coverage itself. doctrine(a_checker_that_reads_headings_cannot_see_a_contradicting_body): a rename is a read of the whole entry, not an edit of its first line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
d2a54ac320 |
kb: restore six doctrines a hard reset discarded, and record how
Consolidates four doc branches and restores what resyncing lost. I wrote doctrines to kb.pl on local main, pushed each as a branch, then ran `git reset --hard gh/main` twice to resync -- discarding six clauses that existed only in the unmerged local commits. Nothing failed: kb_consistent passed on the reduced file, the counts simply read lower, and the loss was found only by grepping for clause names I happened to remember writing. That is the same shape as everything else this wave found. A count that looks plausible is not evidence, and the check that would have caught it -- diff the clause NAMES, not the total -- is the one this repo already applies to tests after a merge. doctrine(a_hard_reset_discards_knowledge_written_to_the_working_branch) states the two rules: write a fact once, on the branch that will carry it to main, not on a local main that is going to be resynced; and after any reset or force-move of a branch holding knowledge, re-count and diff clause names. Restored: every_wave_agent_gets_its_own_worktree, the_orchestrator_delegates_substantial_work, a_message_assertion_is_a_uniqueness_claim_about_the_whole_vocabulary, run_the_gate_ci_runs, an_observation_correct_about_what_changed_can_be_wrong_about_when_it_is_read, a_siblings_unmerged_pr_is_invisible_to_bd_search. Also carries: the wave skill, the Copilot-gate correction to AGENTS.md, the help evidence rename, and the code-review skill's missing -D warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
65e8f2fb42 | Merge remote-tracking branch 'gh/docs/copilot-gate' | ||
|
Edward Langley
|
debad21556 |
docs: run the gate CI runs, and bindings.rs is run
Two one-line corrections the wave's last review surfaced, both about a claim being stated in more than one place. .claude/skills/code-review/SKILL.md told every reviewer and every wave agent to run `cargo clippy --workspace --all-targets` with no `-D warnings`. Clippy exits 0 on warnings, so the committed local gate was green on exactly the input CI is red on. This is not one person's shell mistake, it is the instruction: a reviewer following the skill as written would report clean and be wrong, which happened on PR #71. Also adds --no-fail-fast, since fail-fast hides blast radius. README.md was a sixth copy of "bindings.rs is never run", which PR #70 corrected in five places -- its grep missed README, and PR #72 then rewrote the sentence four lines above and carried the wrong half forward. Neither PR is at fault individually; it is an interaction between two branches, and it is the exact failure the corrected fact is a monument to. Plus doctrine(a_weak_check_and_a_strong_one_agree_until_the_defect): five instances this wave, one mistake -- a check written against the failure you imagined rather than the outcome the user sees. The weak and strong checks agree on every input anyone tried and diverge only on the input that is the defect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
cd094b6dfb |
fix(kb): the dj1l cases discriminate, and a replaced rule cannot silently become an add
Five findings from the second review round, and the first is this PR's own standard applied to its own P1. **The two fixes improvise-dj1l names had no discriminating test.** Reverting `has_key` to unification, or dropping `canon_key` from the duplicate gate, left kb-test at 27 passed. Case 11 was carried by `check_total` refusing with a DIFFERENT message, and case 10 by the other half of the fix. Both now assert which refusal fired (`said()`), plus two cases aimed at each half: a rm key with a variable in a non-final position must be refused as ABSENT rather than matching everything, and a keyed FACT whose key holds a variable must ADD without excluding the siblings it would unify with. Each proved by reverting exactly one fix and watching exactly that case fail. **Keying a rule on its whole clause made `replace` on an edited rule silently become `add`** -- both arms live, the stale one still firing, and `check_total` cannot see it because `NO + NF` is correct arithmetic for an add. Reachable on the 15 `kb_error/1` arms and the `kb_text/2` rules, so editing the check improvise-2f97 adds is the first thing to hit it. `check_rule_replace` refuses and names the route (rm, then add); the negative arm -- replace of a rule present verbatim -- is tested, or the refusal would just be "replace never works on rules". **The `decision(` arm was dead code.** `cited_name` took the first field, and the repo spells `decision(<ticket>, <name>)`, whose ticket id fails `name_shaped/1` on the hyphen -- so that marker could never fire for a real citation. A second `cited_name` clause takes the second field. All three markers are now exercised. Minor, from the same round: `_JS_TEST` no longer accepts `test.only`, which disables every other test in its file, with a case asserting a heading cannot be satisfied by one; and `node-test-test.sh`'s last case no longer duplicates an earlier one. Filed rather than fixed: improvise-w8ih, extracting `web/assemble.sh`'s gate so its own absence is visible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
67ce2df421 |
docs: the Copilot gate is gone, and AGENTS.md told agents to wait for it
Measured: PRs #12, #21 and #27 carry 2, 8 and 4 Copilot reviews; #61, #66, #68 and #70 carry none. Both request paths report success and register nothing -- the GraphQL mutation returns clientMutationId null, the REST call returns 200 with requested_reviewers empty, and no review_requested event appears. So "merge only once Copilot passes" was an instruction to wait forever on a review that was never requested. An absent review and an unrequested one look identical from outside, which is review_is_requested_not_inherited seen from the requester's side. Review is now an adversarial agent the orchestrator dispatches, naming the head SHA it reviewed; the Copilot mechanics are kept for the day it returns. Found by the PR #70 agent, which noticed its own review request had produced nothing rather than assuming the review was queued. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
5c3c796308 |
docs: a wave skill, so the brief stops being retyped
Four agents were dispatched without worktree isolation and all edited the same working tree: one git status held one agent's effect.rs, another's harness/build.sh and a third's scripts/node-test.sh. Any commit would have swept up three units' half-finished work. The brief had carried that rule before and lost it, which is what a retyped brief does. It is now a skill: worktree isolation first, partition by crate rather than ticket count, the base commit by name, the baseline, the never-write list, the injection discipline, pr-watch --expect, and attribution. Two doctrines: every_wave_agent_gets_its_own_worktree records the tell and the per-agent recovery (copy only what an agent can PROVE it touched -- two units can legitimately both be in improvise-protocol); the_orchestrator_delegates_substantial_work records the user's rule and the non-obvious half of it, that orchestrator work lands in the shared primary tree and collides with agents who own that ground. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
50de0a8fd2 |
fix(kb): keys are compared as variants, and a cited clause cannot be silently retired (improvise-dj1l, improvise-2f97)
dj1l (P1, found by review of PR #68 after it merged). kb-merge.pl matched clause keys by UNIFICATION, which both lost and corrupted clauses. check_self_consistent compared keys with ==, but findall hands back fresh variables, so two clauses with variant heads were never == and passed the duplicate gate; foldl_merge then matched by unification and the second removed the first. Reproduced: 536 before, 2 in delta, 537 after. Worse, adding `probe_fact(X) :- doctrine(X,_).` over a kb holding probe_fact(a) and probe_fact(b) deleted probe_fact(a) AND wrote `probe_fact(a):-doctrine(a,A)` -- the match bound X, so the clause written was not the clause supplied. The round-trip verify cannot see that: it compares the file against an intended list already corrupted by the same unification. same_key/2 (copy_term + numbervars) in has_key, conflicts and absent; canonicalize before duplicate detection; a RULE keys on the whole clause, since one head shape with different bodies is ordinary Prolog; and check_total/2 asserts the arithmetic the tool already PRINTED beside the delta size -- "2 in delta, 537 after" was visible in its own output twice and nothing compared them. 2f97. kb_error(dangling_citation/3): a name cited inside another clause's text must resolve as a doctrine/2, invariant/2 or decision/3, so retiring a clause can no longer dangle every clause naming it. The token rule is narrow on purpose -- a name-shaped token directly inside doctrine(, invariant( or decision( -- so prose cannot false-positive. kb_error/1 is a gate predicate, so this was written with KB_ALLOW_GATE=1 deliberately: the refusal exists so a write cannot disarm its own gate, and an intentional change to the gate is supposed to be a greppable act rather than an impossible one. Seven cases added to kb-test.sh, each proved by injection: two variant-headed rules both land; a non-ground rule deletes nothing and is written as supplied; a duplicate key is still refused; an injected dropping fold is caught by the total; a cited clause cannot be retired; and the citation arm fires on a name that resolves to nothing without firing on one that resolves. scripts/kb gains a KB_MERGE seam so a broken merge can be injected. One pre-existing case moved off doctrine(bug_first), which the new arm correctly refuses to retire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
fc6d229dda |
kb: design against the model before implementing, and fix the key rule it exposed
AGENTS.md has always said Prolog is a design tool for reasoning about tickets; it was the least-used line in the file, because kb.pl was only ever written retrospectively. `kb check --with <design.pl>` merges proposed facts into a throwaway candidate and runs kb_consistent over the union, so a design can be refused before any code exists. Nothing is written either way. The model already derives rather than records -- illegal_dependency, may_use, depends_transitively, portable, snapshot_safe, view_state_admits, record_obligation, and eleven kb_error classes. Three proposals exercised it in one sitting: one added dependency edge produced SIX layering violations through the transitive closure; a proposal to declare decision/3 functional was refused, correctly, because improvise-907 carries two decisions; and a mis-ordered layer/2 argument was named rather than accepted. Trying to model a design found a real defect in the tool. Keys were the functor plus all arguments but the last, which is right for doctrine(Name,Text) and wrong for any RELATION: crate_depends_on(A,B) keyed on A, so a crate depending on two crates read as one key twice and was unaddable. It cannot be read off the KB's functional/1 -- that keys an arity-3 predicate on its first argument, right for crate/3 and wrong for decision/3 -- so key_arity/2 is a table, and anything unlisted keys on the whole head, which is the safe direction. kb-test.sh is 20 cases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
eaaf9d935a |
kb: a write cannot disarm its own gate, and cannot silently lose another's
Second review pass on PR #68. All three findings real; the first two reproduced by injection before fixing. The candidate is validated by CONSULTING it, so the thing being checked got to influence the check. Three ways through, all demonstrated: - `:- halt(0).` as a delta -- fixed last round by refusing directives - REMOVING kb_consistent: it then throws existence_error, and tpl exits 0 when a goal throws before reaching halt, so the gate passed by not existing. The error was printed and the tool still reported success. - REPLACING it with the fact `kb_consistent.`, which passes forever after. CHECK_GOAL now catches, so a throw exits non-zero, and writes touching the gate predicates are refused unless KB_ALLOW_GATE makes it deliberate. That list is hand-maintained and a new gate predicate is unprotected until named -- stated rather than hidden. Writes were read/validate/mv with nothing in between, so two agents -- ordinary here, waves run in parallel -- would both read the same file, both validate, and the later mv would discard the earlier addition. The lost-update class this tool exists to remove, reintroduced in the tool. Closed by stamping content at read time and refusing to commit if it moved, plus an mkdir lock. The stamp is the part that matters; the lock only makes the collision rare. AGENTS.md's top-level instruction still said to save facts through the MCP. The wave section had been fixed; the authoritative line had not. kb-test.sh is 17 cases. The race is deterministic via KB_TEST_RACE. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
7fd57c06b3 |
docs: say what to do; attribute what agents post
Two user rulings. Documentation should not spend itself on history -- that is bloat. Prose docs now lead with the rule and keep only the line that stops it looking arbitrary; the worked examples and the how-we-found-it move into doctrine/2 text, which is what that predicate is for and where a reader goes deliberately rather than while looking for a command. Trimmed: both skills, context/code/kb-tool.md, and the kb paragraph in AGENTS.md. Anything an agent posts to GitHub goes out under the owner's account, so every comment, review and issue body now opens with a line naming the model and that an agent wrote it. A PR body's generated-with footer already did this; comments carried nothing, and an unattributed one reads as his own words to every reader -- including a reviewer deciding how much weight to give it. The reply on PR #68 has been amended in place. improvise-xfsb files the structural fix: a GitHub App or machine user so agents post under their own identity. An attribution line is a convention an agent can forget; a separate identity cannot be. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
805bae7a15 |
kb: a clause can be retired, and the docs stop naming the MCP
Three gaps in the tool as first written, each found by asking what it could not do rather than by running it: - There was no way to REMOVE a clause. Every correction could add or overwrite, so a fact that went false could only be rewritten, never dropped. `kb rm` takes keys and refuses when any is absent -- a typo'd key would otherwise report success and leave the clause in place. - `doctrine(kb_is_maintained_through_the_mcp)` is retired for kb_is_maintained_by_scripts_kb. The NAME was the claim that went false, which is the repair opfs_is_run_but_bindings_is_not needed for the same reason. The replacement records why nobody should go back, and that READING through the MCP is unaffected. - AGENTS.md said kb.pl was "MCP-maintained, whole-predicate replacement" and told agents to strip host_rpc shims after each save. Both described the retired mechanism. This is a_fact_stated_twice_is_changed_by_grep landing inside the PR that depends on that doctrine being true. Also fixes a printed value that was an unevaluated expression: `rm` reported "530-529 removed" rather than 1. kb-test.sh is 11 cases now, both new ones proved by injection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
3405fc4def |
kb: retire the Prolog MCP for a tool that costs the clause, not the predicate
loadProgram REPLACES a whole predicate, so adding one clause meant re-sending every existing clause of it through an agent's context window. Measured on the wave-3 batch: ~113KB read and ~113KB written to deliver ~25KB of new text. That sets the behaviour -- writing becomes a thing you batch and do rarely -- and the batch is where a short payload silently drops the rest of a predicate, which has cost this repo 22 decisions and 19 doctrines once already. scripts/kb reads clauses as DATA and never consults them, so the cost of adding a clause is the clause. Canonical form is read_term in, portray_clause out -- measured, not designed: all 495 existing facts round-trip byte-identically and the form is a fixed point in one pass. Rules move to one line, which is why the reformat lands here rather than earlier: the MCP would re-expand them on its next save and the two would thrash. trealla moves into the devShell. Reaching it through `nix shell` per invocation took the tool's first real use past a two-minute timeout for a write that takes 0.42s with tpl on PATH. Four guards, each added because injecting its fault showed the tool accepting what it should refuse. None was found by reading: - an unparseable delta dropped trealla into its REPL, which HANGS with a terminal and EXITS 0 without one -- a refusal that reads as acceptance - `-g main,halt` exits 0 on a throw AND on a failure, because a goal that fails never reaches the halt beside it; halt(2) alone exits 2, so the default is the hazard. The goal now guarantees a halt on every path (0/2/1) - `42.` parses, is not callable, and landed as a fact whose head is a number; kb_consistent has no opinion about stray terms - fmt discarded the merge's exit code and would have committed a failed canonicalization Plus the two that were designed in: the result is read back and required to be the clauses intended (variant equality -- == compares variable identity and reports every rule as different), and kb_consistent runs on the CANDIDATE, so a bad delta leaves the knowledge base untouched rather than needing a revert. scripts/kb-test.sh proves all nine by injection. context/code/kb-tool.md carries the argument, including what it does not reach. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
2d22b65d1f |
kb: a Record is the settable arm, not the unsettable one
invariant(shipped_demo_cells_are_settable_or_derived) described the CellId::Record arm as "editable false". The boolean in the cited test is expected_derived, not editability, and the assertion message beside it says the two raw columns "must be Records the page offers" -- so a Record is precisely the SETTABLE arm and the clause stated the inverse permission. That is the failure doctrine(a_stale_gotcha_asserts_the_negation) names: a wrong doc does not merely omit, it tells the reader not to assume the thing that is true. Costly here because ADR 002 makes permission a fact about the CellId variant, so a reader trusting this clause had both permissions backwards. The replacement names the error rather than only avoiding it, so it cannot come back quietly. Verified against tests/web_demo_model.rs, not from memory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
9fdf39dda2 |
kb: the wave-3 batch, and three corrections to clauses that had gone false
Applied through the MCP cycle (loadSession, four loadProgram calls, saveSession),
not by editing the file. Counts verified against the live file immediately before
writing, because the first read of them came from a working tree six merges behind
and would have deleted three clauses while reporting a larger total.
doctrine 56 -> 95 (+39)
invariant 51 -> 64 (+13, one of them a rename)
decision 48 -> 64 (+16)
work_done 20 -> 23 (+3)
Verified: kb_consistent passes (exit 0, the exact CI invocation); unique names
equal totals for all three keyed predicates; a name-by-name diff against
|
||
|
Edward Langley
|
6378551641 |
Merge main into docs/wave2-batch-e
Only .beads/issues.jsonl conflicted; restored to main's export so the branch carries no beads diff. kb.pl auto-merged cleanly: doctrine/decision/ invariant counts 56/48/51 before and after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
9d6934604e |
kb: scope the unrun claim by engine, rather than closing the list
Review finding on this PR, and it is the commit's own lesson applied to the commit. The replacement invariant ended with "what is genuinely unrun: Safari/WebKit and any non-Chromium engine for the shell spec" — a CLOSED list, and an incomplete one. Verified: `WorkerServer::documents()` and `WorkerServer::refresh()` are `#[wasm_bindgen]` exports with no caller in `web/*.js` or `harness/`, so they are run by nothing. (The ten `.refresh(` hits under `harness/opfs-contract` are `OpfsFs::refresh`, a different method — I checked rather than counting greps.) So the old invariant understated coverage and my replacement overstated it. The direction flipped; the defect did not. The claim is now scoped BY ENGINE and says why: a coverage claim in prose is unfalsifiable by anything in this repo — the KB-consistency job checks the KB is consistent, not that it is true — so state what IS covered and scope what is not, rather than enumerating an absence. kb_consistent exits 0; 55 doctrines / 47 decisions / 51 invariants, unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
ddfc06ed66 |
docs: follow the active_view split through repo-map and kb.pl
The batched instruction-file edits owed by improvise-jytm (PR #48), applied now that it has merged. Text supplied by the WU-E agent and audited before applying: 97 ticket IDs across both files, all resolve. repo-map gotcha 29 is replaced entirely — the two copies of "which view is active" are now ALLOWED to differ, and the gotcha said the opposite. Gotcha 36's `apply_switch` half is inverted: it now takes a `SessionId` and is addressed to exactly one session, where the paragraph said its taking no SessionId "is the statement". Gotcha 38's counts move 29 → 26 model effects and 28 → 24 ModelCommand variants, and its `ViewBack`/`ViewForward` sentence is deleted for being wrong on all three clauses. Gotchas 3 and 44 gain a clause each. One paragraph needed more than a correction. Gotcha 36's `ToOthers` trap describes a mechanism this PR deletes — but the lesson outlives the mechanism, so it is rewritten in the past tense with the two rules that survive: assert the `Broadcast` ITSELF, because the audience is the message rather than something to infer from folded state; and seat a bystander where a wrong relay can move it FROM, because a bystander already on the relayed value cannot see a wrongly WIDENED audience — the wrong relay folds to a no-op. Review demonstrated that vacuity twice on #48, in tests whose evidence file had stated the risk in one sentence and claimed the assertion covered it in the next. kb.pl: four clauses replaced, two added. `lockstep_binds_every_session_not_the_asker` changes shape rather than wording — active_view was the example that taught it and is now out of its scope, so what remains under it is the document half and the doctrine becomes a test for whether a value is shared AT ALL before it is a test for who hears about it. The addition worth reading is `an_injection_that_did_not_compile_reads_as_a_clean_run`, caught by #48's reviewer in its OWN tooling: a grep for `^error[` that misses a bare `error:` turns a failed compile into a green read — the defect injection exists to catch, one level up in the instrument. The rule is deliberately not "widen the grep", since a pattern can always be one case short: require a POSITIVE signal that the injected build ran. A count cannot be silently empty. Counted before and after, on the predicate that has been silently wiped before: doctrines 55 → 56, decisions 47 → 48, invariants 51 → 51. kb_consistent exits 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
519f0adf23 |
kb: bindings.rs is run, and the invariant's NAME was the false claim
Found by adversarial review of PR #47, which called this the one only the orchestrator can fix. Verified independently before acting: /app/index.html with no ?ws= spawns the worker, which imports the glue, which calls bindings.rs::start and OpfsFs::open — and shell.spec.js's "the standalone deployment" then asserts a grid draws. It has run under Chromium in CI since improvise-vvl3. invariant(opfs_is_run_but_bindings_is_not) is renamed to the_harness_runs_the_worker_chain_under_chromium_only, because the name itself asserted the falsehood and a reader who never opened the body would carry it. The crate/3 fact for improvise-worker-server said the same thing and is corrected. What remains genuinely unrun is now stated: Safari/WebKit, which has no OPFS in Playwright's build, and any non-Chromium engine for the shell spec. The body records how it survived, because that is the reusable part: a "never run" written in prose is unfalsifiable by anything this repo has. The Knowledge base consistency job was green throughout — it checks the KB for CONSISTENCY, not truth — and the claim UNDERSTATED coverage, so nothing failed and nobody looked. Five copies, every one written before the harness landed and none re-read after. Caught while making this edit, and worth recording: the first version dropped the clause-terminating period, so the invariant failed to parse and was SILENTLY LOST — invariants went 51 → 50 and the new name was absent. The findall/length count AGENTS.md prescribes is what caught it, on exactly the predicate that has been silently wiped before. Counting before and after is not ceremony. Verified: kb_consistent exits 0; 55 doctrines / 47 decisions / 51 invariants, the new name present and the old one gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
6f9efcf2e8 |
docs: improvise-xps8 describes a bug that does not exist — turn it into a guard
Review findings on PR #30, all three applied. 1 [P1]. The claim that the handshake watchdog overwrites a genuine mid-session disconnect 8s later is FALSE on any tree that has PR #21. `close` routes BOTH arms through `setShellFailure` (app.js:248-250), which clears the timer synchronously (app.js:165-169). It was believed because it was measured honestly — against a tree predating PR #21 — and then reported as "surviving" that PR on the strength of a simulation that re-created only the `opened` flag and not the other half of the fix, the routing. A simulated fix that is not the fix you named reproduces a bug the real code does not have, and reads exactly like a confirmation. Rather than delete the claim, execute the property it denied: the open-then-dropped test now runs past HANDSHAKE_TIMEOUT_MS + 2s and asserts "sent no document" never appears. Proved discriminating by removing `setShellFailure`'s `clearTimeout` — the assertion then fails with exactly the message xps8 described. gotcha 48 and the spec's comment block corrected; the ticket is cited in neither. Recorded as doctrine(a_simulated_fix_must_be_the_fix_you_named), the sibling of an_injected_fault_must_be_the_fault_you_named that fails the other way: a wrong fault proves a check that cannot fire, a wrong simulated fix proves a bug that does not exist. 2 [P1]. The previous commit created the contradiction it was meant to remove: design-principles recorded the measured ~2.2s while kb.pl still said "under 2 seconds". The ambiguity between per-binary and wall clock is what let it drift, so both now state two numbers — no single binary over 2s, wall clock is the sum, measured 1309 tests / 33 binaries / ~2.2s / longest 1.42s — in kb.pl, design-principles and repo-map alike. Also 16 -> 17 in doctrine(binding_dump_diff). 3 [P2]. The new audit exclusion was unanchored, so `improvise-gp` was a prefix match that would silently exclude a real future ticket. Anchored `^…$`. It immediately caught the example id the explanatory paragraph itself spelled out — a recipe is code, twice over. CLAUDE.md: declined, with evidence. Both copies of the duplicated tail sit inside `<!-- BEGIN BEADS INTEGRATION v:1 profile:minimal hash:ca08a54f -->` markers written by `bd setup claude` and `bd setup codex` and refreshed against that hash. A process change there is one tool invocation, not two hand edits, and a pointer makes `bd setup <recipe> --check` report it modified. Recorded as kb_issue_found(claude_md_tail_is_generated_not_duplicated) so it is not re-proposed. kb.pl: doctrine 54->55, kb_issue_found 11->12; invariant 51, decision 47, location 34 unchanged. kb_consistent passes. cargo test --workspace: 1309 passed, 0 failed, 2 ignored. harness (Playwright): 12 passed, 3 skipped (webkit has no OPFS). Dead-ID audit with the anchored recipe: 157 tokens, all resolve. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
f12a2d3517 |
docs: reconcile the docs with the code after wave 1
An independent review checked every factual claim in the docs against the code at |
||
|
Edward Langley
|
4d5a8b6c0d |
kb: three waves of facts, and the two newest crates enter the model
kb.pl had received NOTHING across the 28 unreviewed commits, while three agents each produced doctrine-grade findings. Now 27->48 doctrines, 32->44 invariants, 30->47 decisions, 8->11 crates. The consistency check earned its keep twice. Adding forbidden/2 for improvise-wasm-client and improvise-worker-server made kb_consistent FAIL -- forbidden_names_unknown_crate -- because known_crate/1 is derived from crate/3 and neither crate had ever been declared. improvise-ws-server was missing too. And saveSession re-captured the seven host_rpc shims exactly as kb_issue_found(host_rpc_shims) predicts; stripped per the recorded audit. Also normalised wasm_bundle_crate/1, which mixed quoted-hyphen and underscore atom forms, so a query over it could only ever match some of its own facts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EoW5BB87ZZdm96SvBTWawH |
||
|
Edward Langley
|
6820eafee5 |
kb: same_kind_is_not_the_same_effect, a_rename_can_hide_a_stale_key
Both from the wave-4 review, both proven by injection. The first records that swapping three ModelCommand mappings for their inverses left the suite at 918 passing, because an operation and its inverse declare the same EffectKind. The second records a fixture that passed for the wrong reason: renaming the active view across a document swap made the stale cursor unreachable rather than wrong. Also extends derive_then_verify with the awk-reports-30-not-29 instance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EoW5BB87ZZdm96SvBTWawH |
||
|
Edward Langley
|
f26464e4d0 |
kb: two doctrines from Copilot's PR #10 findings
lockstep_binds_every_session_not_the_asker — moving a shared value for the client that asked is a desync with a polite name. Proven by improvise-kvyt: a view switch moved only the addressing session's mirror while every client's grid is built from workbook.active_view(). an_invariant_is_made_unrepresentable_not_validated — records why the PR agent declined Copilot's request to validate parallel-slice lengths on five gp-tree functions, and what the real remedy is. Same reasoning as improvise-n3wt, which removed exactly that asserted form in the same PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EoW5BB87ZZdm96SvBTWawH |
||
|
Edward Langley
|
22c8667d80 |
kb: record one_arm_of_a_symmetric_rule doctrine (Copilot's PR #9 finding)
Copilot found that improvise-rkeq's col_widths fix covered only the label arms, leaving the ColWidths arm -- named after the very collection it fits -- assigning verbatim and reinstating the length mismatch. The existing test passed throughout because it only ever drove the label arm. Also extends force_the_failure with the tooling instance: requesting a Copilot PR review over REST returns HTTP 200 and silently ignores the reviewer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EoW5BB87ZZdm96SvBTWawH EOF |
||
|
Edward Langley
|
b759599c6f |
docs(kb,repo-map): record the new crates and the wave-1 review lessons
kb.pl: added crate/layer/depends/forbidden facts for gp_tree and improvise_gp (the review found the kb modelled 6 of 8 workspace crates and so had no opinion on the new one); corrected the improvise_protocol description, which still said RenderCache. Two new doctrines, both proven by fault injection during the review: fold_tests_must_test_order_not_the_axiom a_macro_must_not_supply_a_forced_classification No illegal dependency; kb_consistent passes under trealla. repo-map: gotcha 32 now says bootstrap is a compaction, not a replay; new gotcha 33 for the labels-frame-cells contract; gotcha 25 notes the new Clone/PartialEq derives; protocol inventory counts re-measured from the files rather than copied; registry.rs test count corrected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EoW5BB87ZZdm96SvBTWawH |
||
|
Edward Langley
|
80d0f9fdcf |
chore(beads): drop memories from the tracked export, let auto-export own it
bd 1.3.0's auto-export scope excludes memories even with export.include-memories: true, so while .beads/issues.jsonl carried them every mutating bd command hit the shrink guard and auto-export never updated the file. Set export.include-memories false via bd config set and re-exported without them; auto-export now writes and stages the file unattended, verified by a subsequent bd write landing with no warning. Memories are unaffected -- they live in Dolt and replicate via bd dolt push. Dropped the now-wrong bd-export-must-include-memories memory and replaced it with autoexport-owns-issues-jsonl plus the never-hand-edit rule. kb.pl: replaced the stale export_drops_memories_by_default decision with autoexport_owns_the_tracked_export, memories_in_the_file_freeze_autoexport and plain_export_rewrites_the_bookkeeping. Maintained through the MCP; host_rpc shims stripped again per kb_issue_found(host_rpc_shims). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
715df33a3e |
kb: normalize to MCP canonical form, assert beads-maintenance facts
kb.pl is maintained through the prolog MCP, not by hand. Re-did this session's facts via loadSession/loadProgram/saveSession and saved the canonical form: 563 -> 391 lines. Comments and the discontiguous directives do not survive a round trip, so they are gone; every clause does survive, and kb_consistent still passes under trealla. Checked before normalizing that no substantive prose existed only as a comment -- the cycle-safe closure, MCP leniency, the symbol-existence check and fault injection were all already asserted. Section headers were the only loss. New facts: four under beads-maintenance-2026-09-19 (export drops memories without --include-memories, git hooks bypass the shrink guard, dependency ordering is nondeterministic, flake pin verified against nix build) and three under kb-normalization-2026-09-19, including that loadProgram REPLACES a predicate rather than appending, so a partial payload silently drops the rest. saveSession re-added the MCP's own host_rpc shims, as kb_issue_found(host_rpc_shims) says it would; stripped again per that fact's own instruction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
09296c1681 |
chore: re-export issues.jsonl with memories, ignore generated dolt server config
bd export drops 'bd remember' memories unless --include-memories is passed, and beads' own git hooks run the plain form, so a stray `git checkout` had silently stripped all 15 memory records from the tracked export. Re-export with the flag and record the trap in kb.pl. .beads/dolt-server-config.yaml is machine-local generated state (absolute cfg_dir, ephemeral port) alongside the already-ignored dolt-server.* files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
6752d76fda |
chore: groom the backlog (2026-09-19)
Ran the five checks recorded in kb.pl from the 2026-08-30 groom over the 62 open and deferred issues. Cycles: none. Priority inversion: one -- improvise-g0f (P3) blocks improvise-cqq (P2), raised to P2 so bd ready stays usable in priority order. Unrecorded dependencies: 11 prose mentions between open issues, of which one was real (improvise-lae blocks improvise-git, by that ticket's own "check lae first" sentence); the rest are analogues or siblings and stay prose. Stale citations: two broken paths (nr5 carried a doubled crate prefix; three gp-tree tickets pointed at examples/fit_csv.rs, which lives in crates/gp-tree/) and nine line-number drifts, corrected to symbol references where a symbol exists. Dissolved premises: improvise-ede closed (the early return it asks about is intended, per prune_empty's own doc comment); improvise-toc closed (fixed by a341717/e8dcdb1, merged today, which deleted changes_mode); improvise-ete demoted to P4 (its four production .unwrap().items chains are gone -- only test code is left, so the stated panic-prevention win is void); two of improvise-7yx's five test gaps already covered. Two tickets filed. improvise-rkeq: cqi landed RenderCache as a flat grid-only wire type while six migration tickets were written against a fat per-widget cache of the same name -- it blocks improvise-35e rather than me picking a shape on their behalf. improvise-3qz2: a closed record of PR #7's cloud-session tooling, which landed 18 commits with no issue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
f01a07ada3 |
Merge remote-tracking branch 'gh/main'
# Conflicts: # prolog-sessions/kb.pl |
||
|
Edward Langley
|
5ff8bb8faa |
chore(kb): record the kb-maintenance arc against a retroactive ticket
Reconciled the commits back to |
||
 Claude
|
84c3c701e4 |
chore(kb): record the force_the_failure doctrine
Four findings on this branch were the same shape: an unchecked status on an operation whose failure produces a usable-looking value rather than an error, so the next line carries on and the whole thing reports success. bd init exits non-zero but still leaves an empty database behind mv unchecked, so the new tracker lands INSIDE the old directory jq status swallowed by a process substitution, reads as "no inputs" mktemp empty TMP turns "$TMP/$NAME" into a path at the filesystem root Reading the failure paths found none of them, because each one reads plausibly. Forcing the failure found all four: stubbing mv and mktemp is what exposed the last two. The fact also records that chmod a-w proves nothing when the tests run as root -- that attempt returned 0 and looked like a pass, which is the same failure mode one level up. Sits alongside check_the_checker, which says the same thing about checks rather than about error handling. Verified with trealla from the flake's own pinned nixpkgs, which the seeding script on this branch makes reachable: kb_consistent passes, the new fact is queryable, and an injected bad layer/2 still trips kb_report -- so the check is not passing vacuously. 19 doctrine facts now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DyHK7L8S1XDX3EihTnSKaX |
||
|
Edward Langley
|
d25f3187d3 |
feat(protocol): extract improvise-protocol crate
The crate the browser epic hangs off: Command (the wire format), ViewState, RenderCache/CellDisplay/Snapshot, and reduce_view. It depends on improvise-core and serde and nothing else -- no IO, no command layer, no terminal -- and improvise-command now depends on it. ViewState, ViewCursor, ViewFrame, AppMode, Panel, MinibufferConfig, MinibufColor, HELP_PAGE_COUNT and the key types move here. Everything that moved is re-exported from ui::app and command::key, so no existing path changed. The three tickets before this one are what made the move mechanical: h0b evicted the unportable fields, a2x added the derives, 8uz took the cursor out of the shared View. Command's client->server half is not invented. It is exactly the set of EffectKind::ViewOnly effects whose target is the portable slice -- h0b had already sorted the portable state from the host-local, so the enumeration was done. Effects touching the wizard, staged drill edits, storage or the layout cache are absent by construction. The other half carries projections: already-formatted display values and their labels. One thing had to be added rather than moved. reduce_view runs on a client that holds no Workbook, and ViewState::cursors is keyed by view name, so ViewState needs its own active_view. That makes two copies of "which view is active"; App::switch_to_view and App::install_workbook are the only writers, and the_two_active_view_names_never_diverge guards it. The 8uz characterization test caught the desync the moment it appeared, because it was calling workbook.switch_view directly. Whether the two are *allowed* to diverge -- two clients on different views -- is improvise-g0f, deliberately left open. It changes the shape of improvise-cqq, and nothing here forecloses either answer. reduce_view has 12 tests including a determinism check: folding the same stream twice gives the same state, which is what stops the server's mirror drifting from the client it mirrors. Every Command round-trips through JSON. The crate is in the CI wasm job. Closes improvise-cqi Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
07a29aa33b |
feat(io): OPFS-backed Storage for the browser, plus list/delete
Storage gains list and delete: a browser build has no OS file manager standing behind it, so enumerating and removing documents has to be the backend's job. Both are implemented for PhysicalFs and MemoryFs, and a shared storage_contract test runs against both so they cannot drift. OpfsFs (wasm32 only) implements the same *synchronous* trait. That works because createSyncAccessHandle() yields a handle whose read, write, truncate, flush and close are genuinely blocking -- available only inside a dedicated Web Worker, which is where improvise-9x6 puts the app. So the trait did not have to become async. What is still async is pulled out into four methods the worker awaits from its message loop: open(), attach(path), refresh() and commit_deletes(). attach must precede any read or write of a document, because acquiring a handle is async even though using it is not. Two consequences worth knowing. delete() drops the name from the index and queues a removeEntry, so it is visible to read and list immediately and lands in OPFS when commit_deletes is awaited -- the alternative was making the whole trait async for one operation. And handles are exclusive per file entry, so attach closes any handle it replaces and detach exists to release the lock. Checked before writing it: Safari 16.4+ has synchronous access handle methods (an early spec wrongly called them async, and every current browser implements them sync). So the IndexedDB fallback the ticket called for is not needed, per the user's "not unless recent Safari needs it" -- and the async question it would have forced does not arise. Compile- and clippy-clean for wasm32; there is no browser here, so improvise-xf1 tracks verifying it against one. storage_contract is the sequence it must satisfy. Closes improvise-i34 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
f448c651ce |
feat(axis): add Axis::Filter for drill-fixed coordinates
Drilling into a cell fixed its coordinates on Axis::Page, which put them in the tile bar and under [ and ] as if the user could move them. They are constraints of the drill, not dimensions to page through. They now land on Axis::Filter. The variant splits one question into two, and picking the wrong one is the trap this introduces. "What data is visible" is View::filtering_cats -- Page and Filter both constrain -- and GridLayout and add-record-row ask it. "What can the user act on" is categories_on(Axis::Page) for [/] and View::tileable_cats for the three tile-select sites. Both predicates are named on Axis (filters_data, is_user_assignable) rather than left as tests against the variant, so a third variant does not silently join the wrong side. Axis::next maps Filter to itself and no navigable axis rotates into it, so neither tile select nor cycle-axis can hand out a Filter assignment. The .improv grammar gains a filter keyword and round-trips a selection with it. Filter only ever occurs in the _Drill view, which is never written -- but the alternative to supporting it was silently writing a Filter axis out as some other axis. Worth recording: adding the variant produced exactly one compile error outside axis.rs. improvise-rml had already collapsed the four scattered display matches into one file, so the consolidation paid for itself on the very next ticket. Closes improvise-dz8 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
c9b256889d |
refactor(axis): consolidate axis presentation onto the Axis enum
Axis label and color tables were spread across four sites: the tile bar,
the category panel, the view panel, and the tile command. Adding an axis
variant meant finding all four. They now hang off Axis in view/axis.rs.
The ticket's signature was a layering violation. It asked for
display_short() -> (&'static str, Color), but Color is a ratatui type and
Axis lives in improvise-core, which may not name a UI type. Split into
color() -> AxisColor, a semantic palette, plus the label methods, with
draw.rs::axis_fg resolving the palette to a terminal color -- the same
arrangement minibuf_fg already has for MinibufColor.
The ticket also called the category panel an "exact copy" of the tile
bar. It was not. The colors were identical; the labels were not ("Row ↕"
vs "Row", "None ∅" vs "·"). So only the color table was true
duplication, and that is the part with nothing keeping the two copies in
agreement. The four label sets are four deliberate vocabularies and stay
four methods -- plain_label, glyph (Display appends it), short_label
(three columns; None is "·", not a truncation of "Non"), and initial
(one letter; None is empty). A characterization test pins all four
verbatim so the move could not quietly restyle anything.
next() replaces the rotation match in both cmd/tile.rs and
View::cycle_axis. is_data_axis() from the ticket is omitted: no caller
wanted it.
Not touched: persistence/mod.rs maps Axis to the on-disk keywords. That
is a format concern and belongs in the io layer.
Closes improvise-rml
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn
|
||
|
Edward Langley
|
5430bded86 |
refactor: move the cursor out of the shared View
selected/row_offset/col_offset lived on View, inside the shared workbook, so one client scrolling would move every other client's viewport. They now live in ViewState::cursors: HashMap<view_name, ViewCursor>, reached through App::cursor / cursor_mut / reset_cursor. The ticket's premise was false, and checking it first is what settled the design. It asserted these fields were "PERSISTED in .improv"; they were not -- improv.pest has no cursor production and format_md emitted none. Two characterization tests were written before any change: one pinning that the cursor is per-view and restored when you switch back, one pinning that the format carries no cursor at all. Both still pass, the first verbatim against the new location. That invalidated the whole option set rather than one option. Of the three offered, (a) a single ViewState cursor drops the per-view restore; (b) View keeping "the saved starting position" rests on a persistence that does not exist, so the field would be written and never read; (c) leaving it is not per-client. Keying the map by view name satisfies all three constraints at once. Two behaviours had to move with the fields. View::transpose_axes and View::cycle_axis reset the cursor themselves because they renumber the grid; that reset is now in the TransposeAxes and CycleAxis effects. Workbook::normalize_view_state existed to zero stale offsets after a load, and is deleted as vacuous -- a View cannot carry one now; instead LoadModel and AcceptWizardModel clear the cursor map, since the incoming document's view names have nothing to do with the outgoing one's. The ticket's original symptom is gone: view_only_effects_leave_shared_ state_alone no longer exempts SetSelected/SetRowOffset/SetColOffset, so every ViewOnly effect clears the same bar with no special cases. Closes improvise-8uz Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
850ea584d1 |
feat(command): make ViewState serializable
ViewState is the wire payload of the Snapshot a newly-connected client receives (improvise-q08), so it and its field types now derive Serialize and Deserialize: AppMode, MinibufferConfig, MinibufColor, ViewFrame, KeyCode, KeyModifiers. serde joins improvise-command's dependencies. The ticket did not anticipate the actual blocker. MinibufferConfig held `buffer_key: &'static str`, and Deserialize cannot produce a borrowed 'static value -- so every text-entry AppMode variant that carries a MinibufferConfig failed, five "lifetime may not live long enough" errors pointing at the variants and none naming the field. buffer_key and TextEntry::buffer are now owned Strings. Cow<'static, str> would keep construction allocation-free but puts container plumbing in a public field. A BufferKey enum would close the name set, but drags the buffers map key type, the script parser's buffer args and the hand-written "search" buffer with it. Mode construction is per-keypress; seven small allocations are not worth designing around. Note this does not reopen improvise-k8h's rejection of a Buffer enum: that was the drift argument -- each side would still name a variant -- which is about who writes the name, not what type it has. buffer_of() still reads it off the mode. k8h doesn't forbid an enum here; scope does. The roundtrip test populates one non-default value per field family, including a payload-carrying mode and an armed prefix; a roundtrip of ViewState::default() would pass while proving nothing. Closes improvise-a2x Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
c219bfed6a |
refactor(app): evict wizard and drill_state from ViewState
ViewState is the portable session slice -- the one thing serialized into
the Snapshot a newly-connected client receives (improvise-q08). Two fields
failed that contract on both counts:
wizard -- an improvise-io type (outside the allowlist), and a
half-open dialog nobody else should inherit
drill_state -- holds Rc<Vec<RecordRow>>, and stages uncommitted writes
belonging to whoever staged them
Both now live on App under a new documented residue tag, session-local.
The ticket sketched a HostState wrapper; App already is the host struct
and already carries a residue taxonomy, so this adds a fifth tag instead
of a layer -- which is also what jc8 did with keymap ownership.
Demonstrated first: a `ViewState: Send` static assertion that failed to
compile, naming Rc<Vec<RecordRow>> exactly. It survives as a guard that a
shared pointer cannot come back. Send only proxies one clause of
portability, so a second test pins snapshot-safety directly: staged drill
edits are unreachable from ViewState.
kb.pl gains the decision procedure itself -- session_field/3 plus
view_state_admits/1 -- and two consistency checks over it, both proven to
fire by fault injection. Placing a future session field is now a query.
Follow-up improvise-a2x: the remaining field types still derive no
Serialize. Deferred because nothing builds a Snapshot until cqi.
Closes improvise-h0b
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn
|
||
|
Edward Langley
|
3a58d3c1e8 |
feat(kb): add a maintained consistency check, and fix what it found
kb_report/0 and kb_consistent/0 enumerate kb_error/1 solutions across four classes: domain integrity (layers, concerns and dependency edges name real crates), layering (no inverted crate dependency), rule soundness (rules must REJECT a sentinel drawn from no declared domain), and functional keys (a predicate declared functional/1 must not carry conflicting values for one key), plus cross-references between recorded work. Wired into CI as a third job, since kb.pl is executable knowledge and deserves checking like code. Proving the checker fires found two bugs in it. depends_transitively/2 was a naive transitive closure, so illegal_dependency/2 — whose entire purpose is detecting an inverted edge — looped forever on exactly that input; it now carries a visited set. And the :- discontiguous declarations added in the previous commit are a syntax error in trealla, which wants the parenthesized form; the MCP server had silently tolerated them, so validating kb.pl through the session alone had hidden it. Every check class is now verified by injecting the fault it targets and confirming it is reported, then reverting. A consistency check that has only ever said "consistent" has demonstrated nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
a406b52a37 |
fix(kb): repair two unsound rules and drop captured runtime shims
Audited prolog-sessions/kb.pl, which had been appended to all session without
anyone checking it loads or reasons correctly. Four problems.
may_depend_on/2 was type-confused. It read
may_depend_on(A,B) :- crate(A,C,D), \+forbidden(A,B).
with C and D unused and B never constrained, so may_depend_on(improvise_core,
banana) succeeded. forbidden/2's second argument is a CONCERN (ui, io,
ratatui), not a crate, so the rule compared a crate against a concern and
called it a dependency check. Replaced by may_use/2 over an explicit
concern/1, plus crate_depends_on/2 facts read from the Cargo.toml files and
an illegal_dependency/2 rule that actually checks for inverted layering.
before_fixing/2 had a dead guard: `A=C` bound A to a fresh variable and
constrained nothing, so it succeeded for any first argument. It now requires
the subject to be a recorded bug fix.
The first sixteen lines were the MCP runtime's own predicates — sleep,
js_eval, future, await_any and friends — captured into the file by a
saveSession. Not project knowledge, and the reason every consult warned about
overwriting sleep/1.
Nine predicates are appended chronologically and so are scattered through the
file. Added :- discontiguous declarations rather than reordering, since the
chronological shape is what makes the file readable as a work log.
Also verified every location/3 fact still names a symbol that exists in the
tree, and that no predicate has conflicting values for the same key.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn
|
||
|
Edward Langley
|
3697171c0f |
chore: record backlog-grooming method in the knowledge base
Groomed the backlog 2026-08-30. The durable part is the method: cycle detection, priority-inversion detection, stale file:line citations, and dissolved premises are all mechanically checkable, whereas prose mentions of other issues are candidates that must be read per pair — of 21 one-way mentions only 2 were real dependencies, and several pointed the opposite way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
cc3889d362 |
refactor: inject the storage backend into App (improvise-fuj)
The ticket named two effects reading files directly, but that was the visible symptom. App had no storage backend at all, so Save, SaveAs, ExportCsv, LoadModel and CSV import all hardwired the filesystem through persistence and csv_parser. App now holds a Box<dyn Storage> chosen by the host at construction. App::new still supplies PhysicalFs, so every existing caller is untouched; App::with_storage takes any other backend. A browser worker constructs App with an OPFS backend and nothing else changes. The alternatives on the ticket were a parameter on each effect, which spreads per-session config through every signature, or a thread-local, which hides a dependency App is the natural owner of — it already holds the keymap set and terminal size. improvise-io keeps its path-taking wrappers so the native CLI is unchanged, each delegating to a backend- or bytes-taking variant: export_csv gains export_csv_to over a render_csv that produces bytes, and parse_csv_with_warnings gains parse_csv_bytes_with_warnings. Production code in improvise-command now contains no std::fs. The test is the point rather than the refactor: it drives SaveAs, ExportCsv, LoadModel and StartImportWizard over a MemoryFs with no disk touched, then inspects the backend to confirm the bytes landed there. Any of those effects reaching for std::fs fails it — the property a browser backend depends on, verified now instead of discovered later in wasm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
69e77d6823 |
ci: add wasm32 to the toolchain and gate the bundle crates (improvise-ywd)
The audit could not start because the Nix toolchain shipped no wasm32 std and there is no rustup here. Adding targets = ["wasm32-unknown-unknown"] to the flake's rustToolchain was the whole blocker: all four bundle crates — improvise-core, improvise-formula, improvise-io, improvise-command — then check clean on the first try with no feature flags and no source changes. The root improvise crate does not build for wasm because crossterm does not. That is correct: it is the terminal binary, not part of the browser bundle, so the CI job names the four bundle crates rather than checking the workspace. Adds .github/workflows/ci.yml with that wasm job plus a host job running fmt, clippy -D warnings and the test suite — there was no test or lint CI before, only cargo-dist's release workflow. A check gate cannot catch what merely panics at runtime, and the audit's real finding was that two of the ticket's six named hazards are real while four are not. Instant::now() panics on wasm yet compiles, so improvise-command now uses web-time, which re-exports std::time off-wasm. std::fs likewise compiles and then fails in a browser; StartImportWizard and LoadModel still bypass the Storage trait, filed as improvise-fuj because the fix needs a decision on how a wasm build injects its backend. Verified by compilation on both targets and by the native suite. Nothing here has been run on a wasm host. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
6007f5f38c |
refactor: store the armed prefix as an address, not a keymap (improvise-jc8)
ViewState held transient_keymap: Option<Arc<Keymap>>. Keymap lives in improvise-command, which will depend on improvise-protocol once reduce_full lands there taking &Command — so a ViewState in the protocol crate carrying a Keymap is a hard dependency cycle. Not a serde problem; a graph problem. The keymap was never arbitrary: it is always the sub-keymap of a Binding::Prefix at a specific (mode, key, mods), armed during dispatch and consumed on the very next keypress. So ViewState now stores that address and KeymapSet::prefix_keymap resolves it — the Arc goes back to KeymapSet, which already owns every keymap. Both consumers resolve it: App::handle_key has the keymap set and the mode, and the which-key popup only needs binding_hints(). Prefix keys had almost no coverage, so four characterization tests went in first and passed against the old code before anything moved: gg jumps, ZZ reaches wq, an unmatched key after a prefix is swallowed rather than acting as a fresh binding, and records mode reaches the prefixes through its parent keymap. Resolution looks the prefix up in the current mode's keymap, which is only correct while prefixes are flat — a nested one would resolve where it does not exist and silently do nothing. prefix_keymaps_are_flat fails the build if one is ever added, rather than leaving that as a comment. wizard and drill_state are still in ViewState; neither is a cycle, and they are filed as improvise-h0b. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
3b1607d29b |
feat: classify every effect for projection (improvise-mae)
Adds EffectKind::{ViewOnly, Model(ModelChange)} with ModelChange::{CellData,
LayoutGeometry, WholeWorkbook}, so a server can decide which effects owe
connected clients an update. All 58 effects are classified.
kind() has no default. The design offered a per-struct method; making it
required means a new effect cannot forget to classify itself, and the failure
mode here is silent — a mis-declared ViewOnly edit would simply never reach
connected clients. It paid off at once: the compiler found a 58th Effect impl
living in command/keymap.rs rather than effect.rs.
The classification was derived from what each apply() body touches and
reasoned about in Prolog, not eyeballed. That surfaced the trap: the View is
stored inside model_state.workbook, so "touches model_state" does not mean
"is a model change". Axis, page, grouping, hidden items, number format and
active view are LayoutGeometry; cursor and scroll are per-client ViewOnly;
Save, SaveAs and MarkDirty touch model_state for bookkeeping only. Classifying
by grepping for model_state would have had the projection layer broadcasting
cell updates for cursor moves.
The tests verify the declarations rather than restate them: each applies the
effect to a probe App and fingerprints the shared state it claims not to
touch, in both directions.
Cursor and scroll are the one judgement call — they write the shared,
persisted View but must not move another client's viewport. Filed as
improvise-8uz to resolve before the projection layer builds on this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn
|
||
|
Edward Langley
|
9d73e8f504 |
refactor: route persistence through a Storage trait (improvise-6mq)
save/load talked to std::fs directly, so a non-filesystem backend (browser OPFS, an in-memory fixture) had nowhere to plug in. They now go through a Storage trait, with save/load kept as thin PhysicalFs wrappers so every caller and all 83 persistence tests are untouched. The design section recommended starting with the vfs crate. Measuring the actual surface first argued otherwise: persistence touches the filesystem in exactly two places — write the whole document, read the whole document — and autosave_path is pure path manipulation. That does not justify a dependency plus a VfsPath abstraction, and the ticket's own objection to the vfs crate is decisive: it is sync-only while OPFS, the eventual target, is async, so adopting it would mean writing this trait later anyway. Paths stay opaque document names rather than a directory tree, so a backend implements read and write without pretending to have directories, metadata or seeking. Gzip moves from streaming File+GzEncoder to encode-then-write. The .gz suffix still selects it, which makes gzip a property of the document name and not of the backend; a test pins that both backends write identical bytes. MemoryFs gives persistence a fixture with no tempfile and no disk — the shape a browser backend plugs into. The "isolate dirs behind a stub" criterion turned out to be vacuous: dirs is declared at root Cargo.toml:36 and used nowhere. Noted on improvise-v9w, which already covers unused root dependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
22964f99be |
feat: wide-format records mode, one column per measure (improvise-c5v)
Records mode showed long format: a row per cell, a _Measure column naming which measure it was, and a synthetic "Value" column holding the number. Entering data meant sitting on "Value", and editing the measure column renamed a coordinate rather than a value. Wide format instead: a row is one coordinate tuple — the cells sharing every coordinate but _Measure — and each raw measure gets its own editable column. Two measures on one coordinate are now ONE row. The case matrix came from a Prolog model rather than a read-through, which surfaced the load-bearing consequence: a row now groups several cells, so editing a category column must move all of them. The long-format code moved only the cell under the cursor, which here would leave half a row's measures under the old coordinate and half under the new. Both the plain-records and staged-drill paths implement it. _Dim in the synthetic key now carries the column index rather than a label, because a category and a measure item may share a name and a label could not say which column an edit landed on. GridLayout::record_col resolves it to a RecordCol, and drill pending edits are keyed by (row, RecordCol) — which also frees ApplyAndClearDrill from depending on the layout still being the drill layout. A blank model has no measures, so wide format alone would leave nowhere to type a number and break the onboarding path. A trailing "+" column creates a measure by name; being a schema change, it applies immediately rather than staging inside a drill. Formula targets are not columns — they are computed, so there is nothing to type into them. prune_empty's records branch went with it: the function early-returns in records mode, so that tail could never run. Filed as improvise-2yk in case the early return is itself the bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
7955027473 |
refactor: remove the last command-name literals from the registry (improvise-9cn)
Modelled the registry in Prolog — 85 registration sites against 80 Cmd::name
impls, facts extracted from source — and let it classify the drift risk.
70 sites were already safe: register/register_pure/register_nullary read the
name off a prototype instance, not a string. Only 8 used NamedCmd("literal")
as a stand-in prototype, and Prolog confirmed all 8 currently agree with what
their closures build, so there was no live bug.
Those 8 now take a real prototype, which removes the literal rather than
wrapping it in a macro; NamedCmd is deleted and registry.rs holds no command
name literal at all.
Prolog also surfaced a class the ticket never mentions: ViewNavigate,
TileAxisOp, ToggleGroupAtCursor and CommitAndAdvance name themselves off a
field, so a prototype can name the wrong variant and still typecheck. No
prototype scheme reaches that, which is what improvise-61f is for.
61f as specified is insufficient, and injecting the fault proved it: flipping
the ViewNavigate prototype to forward: false while its closure still builds
true does NOT produce a name mismatch. The entry collides with its sibling
under "view-back", every surviving entry still agrees with its own name, and
"view-forward" simply disappears. Uniqueness is what catches it.
Four guards, each verified by injecting the fault it targets: name roundtrip
over both construction paths (asserting no entry escapes both), name
uniqueness, every keymap-bound name resolving in the registry, and aliases
pointing at real commands.
Closes improvise-61f.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn
|