fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore: more updates

Browse Source
This commit is contained in:
Edward Langley
2026-09-25 00:28:55 -07:00
parent b155ce9f0e
commit f155d12afe
2 changed files with 4 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.beads/issues.jsonl
+1 -1
View File
@@ -357,7 +357,7 @@
{"_type":"issue","id":"improvise-5ubs","title":"A cyclic-successor all() can silently omit a variant while its doc says it cannot","description":"Found by the adversarial review of PR #78, which probed it rather than reading the doc comment.\n\nAppMode::all() and Rejection::all() walk a total, cyclic after(). The claim in both doc comments is that a new variant cannot be omitted, because after() has no None arm so the only way to answer for a new variant is to NAME ANOTHER ONE.\n\nThat is half true and the half it gets wrong is the half that matters. ANSWERING after() FOR a new variant is forced by the compiler. POINTING AN EXISTING ARM AT IT IS NOT. A probe variant whose after() arm returns some existing variant, with nothing in the cycle pointing back to it, is simply never walked: all().len() came back 17, not 18, with the suite green.\n\nSo the consolidation replaced five grep-visible lists with one derivation whose totality is asserted in prose and checked by nothing. THE REVIEWER'S META-POINT IS THE REAL TICKET: five disagreeing AppMode lists were visible to grep; all() under-enumerating is visible to nothing. A consolidation owes a test that the surviving derivation is TOTAL, not merely that it agrees with what it replaced.\n\nWHAT IS STILL BETTER THAN BEFORE, so this is not a revert: the compiler now forces a human to look, and two of the five lists it replaced were ALREADY WRONG (both missing SavePrompt). This is a gap in a guarantee, not a regression.\n\nOPTIONS, roughly in order of cost:\n- A test that walks the cycle and asserts every variant is visited, driven by something independent of after() -- which needs an independent enumeration, and that is the problem restated.\n- A derive macro or a dependency (strum). Rejected during PR #78 only because adding a workspace dep mid-wave collides in Cargo.toml; on its own it is the honest answer and cheap.\n- Accept and document the residual precisely, replacing both doc comments, which currently overclaim.\n\nWHATEVER IS CHOSEN, FIX THE DOC COMMENTS FIRST: they presently say the omission is impossible, and a reader checking this property will believe them. An overclaiming comment is worse than none, because it stops the next person looking.","notes":"A FOURTH INSTANCE, inside the PR that found this ticket. The adversarial review of PR #78 added a fourteenth `pub struct` declaring `EffectKind::Model(ModelChange::LayoutGeometry)`, pointed at by nothing, and the whole suite stayed green.\n\nThe site is layout_geometry_effects_actually_change_document_config in ui/effect.rs. Its comment said \"a fourteenth declaring LayoutGeometry must come here and say so\". That is false for exactly the reason this ticket names: adding is compiler-forced only where an EXISTING arm must point at the new thing, and here nothing must. `assert_eq!(cases.len(), 13)` is a count, not a totality check -- it catches a case being deleted from the list, never one being absent from it.\n\nCorrected in #78 to say what it is rather than what it is not; the mechanism is unchanged and the cursor consolidation does not depend on it. Filed here rather than fixed because the honest fix is whatever 5ubs settles on, and a fourth hand-rolled attempt in a fourth place is the disease.\n\nWHAT MAKES THIS ONE WORTH RECORDING: it is the same defect as AppMode::all(), in the PR whose own consolidation work surfaced AppMode::all(). A list that is checked for LENGTH reads exactly like a list that is checked for COMPLETENESS, and both are written as an assert_eq over a number.\n","status":"closed","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T04:01:27Z","created_by":"claude-agent-protocol","updated_at":"2026-09-21T12:39:36Z","closed_at":"2026-09-21T12:39:36Z","dependency_count":0,"dependent_count":0,"comment_count":0}
{"_type":"issue","id":"improvise-349m","title":"format_md/parse_md is not idempotent when a .improv file has two Category sections with the same name","description":"format -\u003e parse -\u003e format differs, the two sides differing only in the order of '\u003e' group lines. DETERMINISTIC -- 8 failures in 8 runs on unmodified main (00f7678) -- so it is NOT gotcha 18's HashSet shape.\n\nCause: the file carries two '## Category: ||' sections with the SAME name. The second merges into the first and its group lines are appended in a position the second parse does not reproduce.\n\nReproducer, from the proptest's own minimal input:\n\n let file = \"v2025-04-09\\n# |\\\\a|\\n## View: a\\n_-: row\\ncollapsed: ||/A--\\n\\\n ## Category: ||\\n- |\\\\a\\\\a\\\\a|,|\\\\a\\\\a|,|\\\\a|,|\\\\a|\\n\u003e _---[|\\\\a|]\\n\u003e _[|\\\\a|]\\n\\\n ## Category: ||\\n- ||[||]\\n\u003e ||[||]\\n\";\n let m1 = parse_md(file).unwrap();\n let printed = format_md(\u0026m1);\n assert_eq!(printed, format_md(\u0026parse_md(\u0026printed).unwrap())); // fails\n\nWHY IT LOOKS FLAKY AND IS NOT: persistence::parse::grammar_prop_tests::generated_file_roundtrips goes red only when the grammar-walking generator happens to emit a duplicate section header, which is rare. It was observed once during PR #70's CI, the rerun passed, and the honest move was to take proptest's minimal input and try it deterministically rather than rerun and move on. That is what turned a 'flake' into a filed bug.\n\nFound by the improvise-pdvm/75ld agent on PR #70; not caused by that PR. bd search for idempotent, roundtrip, duplicate category, format_md and group order returned nothing, so it was not already filed.\n\nWorth deciding as part of the fix: is a duplicate '## Category:' header legal input at all? If it is not, the parser should refuse it and the generator should stop emitting it; if it is, the merge must produce a canonical group order. The round-trip property cannot hold without one of those answers.","status":"closed","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-20T23:42:59Z","created_by":"orchestrator","updated_at":"2026-09-21T04:42:54Z","closed_at":"2026-09-21T04:42:54Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
{"_type":"issue","id":"improvise-v5pc","title":"restore the browser assertion that ? opens help","description":"36ade35 moved the shell suite to node and deleted harness/tests/widgets.spec.js, taking the Playwright test '? opens help, and its rows are the shared ones' with it. Nothing replaced it: no node test and no spec asserts that the help overlay is drawn, that its active tab is the shared module's page title, or that l pages it. The evidence entry in context/test-evidence/clients.md survived the deletion and went on promising the coverage for a whole release -- the improvise-5ray cost, now caught by --check-evidence, and the heading is demoted to prose rather than deleted so its two injections (dropping the paintHelp call; removing the mode != Help guard in help_rows) survive for whoever restores it. improvise-ktv1's acceptance -- a browser shows help -- is currently unasserted end to end.","status":"closed","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-20T23:25:28Z","created_by":"claude-agent-scripts","updated_at":"2026-09-21T00:06:02Z","closed_at":"2026-09-21T00:06:02Z","close_reason":"Duplicate of improvise-78v6, already fixed and merged in PR #72. harness/tests/help.spec.js on main carries test(\"? opens help, and its rows carry the keys and the words, not empty frames\") plus a second test that reads an advertised key out of the DOM, presses it, and asserts the selection moved.\n\nWorth noting HOW this duplicate happened, because the filing agent did nothing wrong: it searched bd before filing, as AGENTS.md requires, and found nothing -- because the fix was in a sibling agent's UNMERGED PR at the time. A parallel wave makes in-flight work invisible to bd search. The cheap addition to the wave brief is to check open PRs, not only the tracker, before filing something a sibling unit might own.","dependency_count":0,"dependent_count":0,"comment_count":0}
{"_type":"issue","id":"improvise-xfsb","title":"Agents should post to GitHub under their own identity, not the owner's","description":"Today every agent comment, review and PR goes out as @fiddlerwoaroof, because gh uses his credentials. The interim is a convention -- open each comment with a line saying a model wrote it (doctrine(an_agent_posting_to_github_says_so), AGENTS.md) -- and a convention is something an agent can forget. A separate identity makes attribution structural instead.\n\nTwo routes:\n\n1. GitHub App installed on the repo. Agents mint an installation token and export GH_TOKEN. Comments render as \u003cname\u003e[bot], the same shape as copilot-pull-request-reviewer[bot]. Scoped permissions, no seat consumed, revocable independently of the owner's account, and the bot suffix is not spoofable by a human account.\n\n2. Machine user with its own PAT as GH_TOKEN. Simpler to set up, consumes a seat on some plans, and is indistinguishable from a person at a glance -- which is most of what we are trying to fix.\n\nPrefer 1. Needs the user: create the App, grant contents+pull_requests+issues write, install it, and put the app id and private key where agents can reach them.\n\nWorth checking while doing it: whether the bot can be requested as a PR reviewer, which would let review agents post as reviews rather than as comments.\n\nOnce it lands, the attribution line becomes belt-and-braces rather than the mechanism, and the doctrine should be amended to say so rather than deleted -- a reader seeing an unattributed comment from the owner's account still needs to know it might be an agent.","notes":"DEFERRED by the user 2026-09-22. Recording the reasoning, because the status was\nset with none and the INTERIM IS LOAD-BEARING WHILE THIS SITS.\n\nWHY IT IS A FINE THING TO DEFER: route 1 needs the user to create a GitHub App,\ngrant it contents+pull_requests+issues write, install it, and put the app id and\nprivate key where agents can reach them. That is credential work only he can do,\nand nothing in the codebase is blocked on it.\n\nWHAT MUST STAY IN FORCE MEANWHILE, and this is the part that would be quietly\nlost: EVERY agent comment, review, PR body and issue still opens with a line\nnaming the model and that it is an agent posting in his session\n(doctrine(an_agent_posting_to_github_says_so), AGENTS.md). While this ticket is\ndeferred that convention is THE MECHANISM, not belt-and-braces -- an\nunattributed comment reads as the owner's own words to everyone who sees it.\nDeferring the structural fix therefore RAISES the cost of forgetting the\nconvention rather than lowering it.\n\nMeasured this session: the convention held across four merged PRs and every\nthread comment. It is not currently failing; it is simply the kind of thing that\nfails silently and only once someone notices.\n\nWHAT REOPENS IT:\n 1. The user creating the App -- the only real blocker.\n 2. Any instance of the convention actually being forgotten on a public\n artefact. One is enough: the failure mode is a human reading a model's\n words as his own, and it is not detectable after the fact by anything\n automated.\n 3. A second person gaining write access to the repo, which makes \"who said\n this\" ambiguous in a way the attribution line alone does not resolve.\n\nNOT A REASON TO REOPEN: more agents, or more PRs. Volume does not change the\nargument either way.\n\nNote the ticket's own closing paragraph still applies and should not be lost: when\nthis lands, the attribution doctrine is AMENDED rather than deleted, because a\nreader seeing an unattributed comment from the owner's account still needs to know\nit might be an agent.","status":"deferred","priority":2,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-20T23:04:19Z","created_by":"orchestrator","updated_at":"2026-09-22T06:22:50Z","dependencies":[{"issue_id":"improvise-xfsb","depends_on_id":"improvise-1xfb","type":"parent-child","created_at":"2026-09-25T01:05:51Z","created_by":"Edward Langley","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0}
{"_type":"issue","id":"improvise-xfsb","title":"Agents should post to GitHub under their own identity, not the owner's","description":"Today every agent comment, review and PR goes out as @fiddlerwoaroof, because gh uses his credentials. The interim is a convention -- open each comment with a line saying a model wrote it (doctrine(an_agent_posting_to_github_says_so), AGENTS.md) -- and a convention is something an agent can forget. A separate identity makes attribution structural instead.\n\nTwo routes:\n\n1. GitHub App installed on the repo. Agents mint an installation token and export GH_TOKEN. Comments render as \u003cname\u003e[bot], the same shape as copilot-pull-request-reviewer[bot]. Scoped permissions, no seat consumed, revocable independently of the owner's account, and the bot suffix is not spoofable by a human account.\n\n2. Machine user with its own PAT as GH_TOKEN. Simpler to set up, consumes a seat on some plans, and is indistinguishable from a person at a glance -- which is most of what we are trying to fix.\n\nPrefer 1. Needs the user: create the App, grant contents+pull_requests+issues write, install it, and put the app id and private key where agents can reach them.\n\nWorth checking while doing it: whether the bot can be requested as a PR reviewer, which would let review agents post as reviews rather than as comments.\n\nOnce it lands, the attribution line becomes belt-and-braces rather than the mechanism, and the doctrine should be amended to say so rather than deleted -- a reader seeing an unattributed comment from the owner's account still needs to know it might be an agent.","notes":"DEFERRED by the user 2026-09-22. Recording the reasoning, because the status was\nset with none and the INTERIM IS LOAD-BEARING WHILE THIS SITS.\n\nWHY IT IS A FINE THING TO DEFER: route 1 needs the user to create a GitHub App,\ngrant it contents+pull_requests+issues write, install it, and put the app id and\nprivate key where agents can reach them. That is credential work only he can do,\nand nothing in the codebase is blocked on it.\n\nWHAT MUST STAY IN FORCE MEANWHILE, and this is the part that would be quietly\nlost: EVERY agent comment, review, PR body and issue still opens with a line\nnaming the model and that it is an agent posting in his session\n(doctrine(an_agent_posting_to_github_says_so), AGENTS.md). While this ticket is\ndeferred that convention is THE MECHANISM, not belt-and-braces -- an\nunattributed comment reads as the owner's own words to everyone who sees it.\nDeferring the structural fix therefore RAISES the cost of forgetting the\nconvention rather than lowering it.\n\nMeasured this session: the convention held across four merged PRs and every\nthread comment. It is not currently failing; it is simply the kind of thing that\nfails silently and only once someone notices.\n\nWHAT REOPENS IT:\n 1. The user creating the App -- the only real blocker.\n 2. Any instance of the convention actually being forgotten on a public\n artefact. One is enough: the failure mode is a human reading a model's\n words as his own, and it is not detectable after the fact by anything\n automated.\n 3. A second person gaining write access to the repo, which makes \"who said\n this\" ambiguous in a way the attribution line alone does not resolve.\n\nNOT A REASON TO REOPEN: more agents, or more PRs. Volume does not change the\nargument either way.\n\nNote the ticket's own closing paragraph still applies and should not be lost: when\nthis lands, the attribution doctrine is AMENDED rather than deleted, because a\nreader seeing an unattributed comment from the owner's account still needs to know\nit might be an agent.","status":"in_progress","priority":2,"issue_type":"task","assignee":"Ed","owner":"el-github@elangley.org","created_at":"2026-09-20T23:04:19Z","created_by":"orchestrator","updated_at":"2026-09-25T07:19:43Z","started_at":"2026-09-25T07:19:43Z","lease_expires_at":"2026-09-25T07:24:43Z","heartbeat_at":"2026-09-25T07:19:43Z","dependencies":[{"issue_id":"improvise-xfsb","depends_on_id":"improvise-1xfb","type":"parent-child","created_at":"2026-09-25T01:05:51Z","created_by":"Edward Langley","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0}
{"_type":"issue","id":"improvise-84wp","title":"Doctrine: a check that looks like the answer, answering a neighbouring question","description":"Ready-to-apply prose for `context/design-principles.md` §7. Filed rather than committed because PR #60 head `ec03f95` is byte-identical (tree `ec5d531...`) to a tree the reviewer fully tested, and a further commit would destroy that property and require a fresh harness run while `main` was moving several times an hour. The artefact is worth more preserved than rushed.\n\n---\n\n### A check that looks like the answer, answering a neighbouring question\n\nThe characteristic defect in this repo is not a wrong line; it is a check that is *adjacent* to the thing it names. Three instances landed in one campaign (PRs #59/#60), in three different layers, which is why it is worth stating as a shape rather than three gotchas:\n\n- **A vacuous assertion.** `set_panel_open_and_cursor` asserted `category_panel_open` was true after an effect set it true -- once the launch default supplied `true`, the effect could be replaced by a **no-op** and 1391 tests passed. It answered *is the flag true* where it meant *did the effect set it*.\n- **A vacuous repair of it.** The first fix folded `[open:false, open:true]` and asserted the final state -- which a reducer that folds nothing also satisfies. It answered *does the text mention the other direction* where it meant *does the writer move the flag both ways*. **Reading the diff is what failed**, and it failed because the vacuous version contained `open: false`.\n- **A clean auto-merge.** `git merge-tree` reporting no conflict answers *can git combine these texts*, where the question is *does the combination do what both sides meant*. Two PRs edited the same four files with no conflict and neither side CI-tested together.\n\nEach has the same remedy shape: **name the question you actually mean, then find an input that separates it from its neighbour.**\n\n- For an assertion: **assert the direction the default cannot supply.** Closing the panel discriminates; opening it proves nothing.\n- For a repair: inject a **one-directional** fault (`if open { set true }` -- can raise, never lower). A vacuous repair cannot see it, because a test that only asserts the `true` direction is satisfied by a writer that only ever writes `true`.\n- For a merge: **materialise it and run the gates**, and check that a count which should MOVE actually moved. Playwright going 16 -\u003e 17 is what showed the other side survived; \"no conflict\" and an unchanged total would both have been consistent with losing it.\n\nThis generalises the existing `doctrine(fold_tests_must_test_order_not_the_axiom)` (asserting `fold(prefix)+fold(rest) == fold(whole)` is the definition of a left fold, so it answers a neighbouring question too). What the campaign adds is that the pattern recurs in **test repairs** and in **merge hygiene**, not only in production code and not only in fold tests.\n\n---\n\nAcceptance: the section is in §7; `doctrine(fold_tests_must_test_order_not_the_axiom)` cross-references it or is subsumed by it. Worth considering a `kb.pl` doctrine clause, which is orchestrator-only.\n","status":"closed","priority":2,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-20T21:38:23Z","created_by":"wave3-webui","updated_at":"2026-09-21T04:49:32Z","closed_at":"2026-09-21T04:49:32Z","dependency_count":0,"dependent_count":0,"comment_count":0}
{"_type":"issue","id":"improvise-qsmp","title":"App::cmd_context's panel-open propagator is untested: C cannot open a closed panel and nothing fails","description":"`App::cmd_context` (crates/improvise-command/src/ui/app.rs) propagates `self.view_state.category_panel_open` into the `CmdContext`. **Hardcode it to `true` and the entire workspace passes** — 1394 tests, nothing fails.\n\nThat matters because `registry.rs` reads it in three places for `toggle-panel-and-focus`s open-vs-close decision and the panel focus/cycle commands. With it stuck true, pressing `C` on a CLOSED category panel decides to *close* it rather than open it — the panel becomes unopenable by its own key — and no test notices.\n\n**Why it is invisible:** `cmd/mod.rs`s `test_helpers` sets `category_panel_open` explicitly on the context it builds, so no command test ever exercises the real propagator. The tests construct the value they then assert on.\n\n**This is PRE-EXISTING and was not caused by the launch-default ruling (PR #60).** That distinction was established by control rather than assumed: with the default inverted, this mutation fails *exactly the same two tests as inverting the default alone* and nothing more. So it was unguarded before the ruling too. The remedy is therefore \"a test at all\", not the precondition-stating fixture that fixed the two genuinely-disarmed tests (`set_panel_open_and_cursor`, `panel_commands_address_the_named_panel_only`).\n\n**The method that found it is worth more than the finding** and is written up in `context/test-evidence/clients.md` under \"How to know a launch default has stopped disarming things\": mutate every site that writes, propagates or reads the value; sites where nothing fails are suspects; then re-run each suspect with the default inverted to separate *vacuous* (fix the fixture) from *unguarded* (file it). Grep cannot do this — it finds sites that NAME the flag, not tests that merely start from a default-built state.\n\nAcceptance: a command-layer test that drives the real `App::cmd_context` rather than a hand-built context, such that `toggle-panel-and-focus` on a closed panel opens it; proved by hardcoding the propagator and watching it fail.\n\nSibling note: `improvise-wasm-client/src/render.rs` has the same propagator shape for `RenderModel`. Hardcoding THAT one true is improvise-wsaw reintroduced one layer up, and Rust sees nothing — only the Chromium harness catches it, at `contract.spec.js`s `toBeHidden`. Consequence worth keeping: a Rust-only run of the `test` CI job cannot see a reintroduced wsaw, which is the sharpest argument on file for the browser job being able to fail the build.\n","status":"closed","priority":2,"issue_type":"bug","assignee":"claude-agent-cmd","owner":"el-github@elangley.org","created_at":"2026-09-20T21:23:53Z","created_by":"wave3-webui","updated_at":"2026-09-21T04:43:50Z","started_at":"2026-09-21T03:27:55Z","closed_at":"2026-09-21T04:43:50Z","dependency_count":0,"dependent_count":0,"comment_count":0}
{"_type":"issue","id":"improvise-5ray","title":"check-evidence ignores #### headings and every browser-test entry uses them","description":"Found by review of PR #65, 2026-09-20 (finding 4). Sibling of improvise-lyyc\nand improvise-r0qa, and the sharpest of the three because this one has already\nlet a false claim stand for a release.\n\nscripts/check-comment-runs.py --check-evidence collects headings with\n`s.startswith(\"### \")` and builds its `defined` set from rust_files(roots) only.\nTwo consequences compound:\n\n 1. A `####` heading is invisible to it — `\"### \"` does not match `\"#### \"`.\n 2. Even if it matched, `defined` holds only Rust `fn` names, so a JavaScript\n test name could never resolve.\n\ncontext/test-evidence/clients.md uses `#### \"quoted name\"` for every browser\ntest precisely BECAUSE of (2) — a `###` heading naming a Playwright test would\nbe reported as an orphan. So the convention that keeps the checker quiet is the\nsame convention that makes it blind.\n\nDEMONSTRATED: appending\n\n #### \"a test that does not exist anywhere\"\n\nto context/test-evidence/clients.md leaves `--check-evidence` exiting 0.\n\nWHAT IT COST: PR #61 deleted `a socket that opened and then dropped reads as\ndisconnected, and stays that way`. Its `####` heading stayed. The gate passed\nfor the whole release while the file promised a guard that was not there —\nexactly the \"dangling explanation\" design-principles §7 says the checker exists\nto prevent.\n\nFIX SHAPE: collect `###`/`####` alike, and extend `defined` with JS test names\n(`test(\"…\"` in harness/tests/*.spec.js and web/tests/*.test.js). Note the\nchecker's own --self-test must grow a case, or this fix is unguarded in exactly\nthe way the thing it fixes was.\n","status":"closed","priority":2,"issue_type":"bug","assignee":"claude-agent-scripts","owner":"el-github@elangley.org","created_at":"2026-09-20T21:22:55Z","created_by":"wave3-jstests","updated_at":"2026-09-20T23:35:11Z","started_at":"2026-09-20T23:22:05Z","closed_at":"2026-09-20T23:35:11Z","close_reason":"fixed in PR #69, each proved by injection","dependency_count":0,"dependent_count":0,"comment_count":0}
flake.nix
+3 -3
View File
@@ -47,13 +47,13 @@
wasmBindgenCli = pkgs.buildWasmBindgenCli rec {
src = pkgs.fetchCrate {
pname = "wasm-bindgen-cli";
version = "0.2.128";
hash = "sha256-a7lcXJnnZkYReja+iUO7NqqrWyv3toxnUgQb8s4IS5s=";
version = "0.2.129";
hash = "sha256-pcecKQd7E8Opw6bkFoE569epUi7gh5qpQF1e5PJY6V8=";
};
cargoDeps = pkgs.rustPlatform.fetchCargoVendor {
inherit src;
inherit (src) pname version;
hash = "sha256-R1Tas33Ursy8kqsxguAkG0ZhNed2n5uFTAhw1l2qlLY=";
hash = "sha256-vmUrWVU7kPJJxO5qIVeAkwQyWDELO1Z4Z5gitz2kco8=";
};
};