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

Merge pull request #83 from fiddlerwoaroof/wave/docs-evidence-gates

Browse Source 
Ten gates that could not fail, and the prose that said they could (wave E)
This commit is contained in:
Ed Langley
2026-09-20 21:49:02 -07:00
committed by GitHub
parent b2320ff080 3542d6f717
commit 8bb1d539d8
18 changed files with 1087 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/ci.yml
+19 -1
View File
@@ -38,6 +38,12 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v7
# First, because it needs nothing and takes a second: the gate that stops
# an unstamped bundle shipping was deleted by injection on PR #69 and the
# assembly still exited 0 (improvise-w8ih). It guards the assemble step
# below, so it fails under this job's name rather than another's.
- name: The stamp gate refuses what it claims to
run: ./scripts/assemble-stamp-test.sh
- uses: dtolnay/rust-toolchain@stable
with:
targets: wasm32-unknown-unknown
@@ -140,6 +146,12 @@ jobs:
run: python3 scripts/check-comment-runs.py
- name: Test-evidence headings name real tests
run: python3 scripts/check-comment-runs.py --check-evidence
# `pages-deploy` needs every other job, and saying so was not enough: a
# job added during review of PR #83 was ungated and nothing noticed.
- name: The deploy gate refuses what it claims to
run: python3 scripts/check-ci-graph.py --self-test
- name: Every check job gates the deploy
run: python3 scripts/check-ci-graph.py
# Too slow on GitHub Actions
#
@@ -391,9 +403,15 @@ jobs:
# between upload and deploy — 182s before, ~115s after, on the run 35535225637
# shape — so the window in which a superseding run can expire this run's
# artifact is smaller. It does not close the race; it does not widen it.
# `needs` is EVERY check job, which is the only rule that does not go stale.
# `kb` was the one absent, on the unstated ground that kb.pl is not shipped by
# the site — but neither is `harness-lint`'s crate, and it was in the list, so
# the ground proved too much (improvise-x7cq). A failing consistency check
# means main is broken, and publishing from a broken main is what this list
# exists to stop.
pages-deploy:
name: Deploy to GitHub Pages
needs: [test, wasm, js, browser, harness-lint, build]
needs: [test, wasm, js, browser, harness-lint, kb, build]
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
runs-on: ubuntu-latest
permissions:
README.md
+4 -2
View File
@@ -181,8 +181,10 @@ improvise script setup.txt -f model.improv
The browser page is at **<https://fiddlerwoaroof.github.io/improvise/app/>**,
built and published from `main` by the `build` / `pages-deploy` jobs in
`.github/workflows/ci.yml`. A deploy only happens after `cargo test`, the
wasm32 check and the Playwright browser harness have passed.
`.github/workflows/ci.yml`. A deploy only happens after **every** check job in
that workflow has passed. The list is deliberately not repeated here — naming a
subset is how this sentence went stale before, and `scripts/check-ci-graph.py`
now holds `pages-deploy`'s `needs` to the workflow's own job keys.
The site **root** is the landing page — `docs/index.html`, with the recorded
terminal casts — and it links to the app. `web/assemble.sh` puts the two
context/code/clients.md
+7
View File
@@ -1820,3 +1820,10 @@ rather than an accident: the moment a `web/tests/*.test.js` file needs
`web/pkg/`, it inherits the four-minute build and the argument for it
evaporates. That is why the one test that does need the bundle lives in
`web/tests/bundle/` and runs in the browser job instead.
Until improvise-4ktt the mechanism was a directory convention and a glob, and
nothing checked it. `bundle_freedom.test.js` does now: it reads its own sibling
files and refuses one that names the bundle directory, and its companion
asserts `web/tests/bundle/` still holds a test that *does* — a scoping rule is
uninteresting if nothing falls outside it. Evidence in
`context/test-evidence/tooling.md`.
context/code/doc-site.md
+22
View File
@@ -40,6 +40,21 @@ Neither is referenced by any published document except as a row in
`repo-map.md`'s file inventory, which is inside a code fence, so leaving them
out creates no dangling link. That was checked rather than assumed.
**And "not published" is now derived rather than assumed** (improvise-res9).
The generator walks `context/**/*.md` plus `docs/design-notes.md`, subtracts
the manifest, and fails unless the remainder is exactly the `OMITTED` map in
the script — which carries the reason beside the name, so this section and the
code cannot drift into disagreeing. An allowlist and not a count, because an
omission is a decision that should have to be argued in code; and the map is
checked in *both* directions, because an entry naming a file nobody can find
asserts a reason for nothing, which is the failure a dead ticket id is.
The check was latent when it was filed and **live by the time it was written**:
`context/code/kb-tool.md` had been added after this table was drafted and was
publishing nothing, with no argument for its absence anywhere. The table above
says `context/code/*.md` is published; the manifest disagreed and nothing could
see it. That is the whole failure mode in one file.
**Test evidence is the one judgement call worth stating.** The argument for
leaving it out is that 501 entries about individual tests are noise to a reader
who wanted to know what a pivot table is. The argument for keeping it is
@@ -136,6 +151,13 @@ filesystem instead. That is the whole reason `assemble.sh` runs the generator
version skipped those 48 links while still counting them as checked, behind a
comment promising they were "checked below" when there was no below.
That ordering is itself checked now (improvise-res9). The generator reads
`web/assemble.sh` and refuses to run unless its own invocation is that script's
last command. PR #63 appended a loop and it happened to land above the
generator; one landing below would have made the outward-link check pass
against a half-built tree, silently and in the one direction this section says
must not go unnoticed.
**And the link check is not sufficient, which is the more important lesson.**
It answers one question — does every href resolve — and a defect outside that
question is invisible to it however carefully it is run. The worked example is
context/code/tooling.md
+175
View File
@@ -0,0 +1,175 @@
# The repository's own checkers
`scripts/` holds the gates this repo runs on itself. They are code, and they are
held to the rule they exist to enforce: a check nobody has seen fail is
indistinguishable from one that always passes, so each of them carries a
self-test whose cases are injected faults
(`doctrine(check_the_checker)`).
Four docstrings in `scripts/check-comment-runs.py` point here. This is the file
they mean.
---
## 1. The comment cap
`check-comment-runs.py` fails a `.rs` file carrying more than two consecutive
comment lines. The rule is `context/design-principles.md` §6: reasoning lives in
`context/` prose, where it can be read, corrected and audited, not in a comment
that nothing checks against the code.
The whole difficulty is deciding what *is* a comment line. Ten silent successes
have been found in this one function, every one of them failing in the direction
that looks like passing — a file that should have been flagged and was not:
- `"test" in line` matched `#[cfg(test)]`, so every cfg-gated fixture cleared a
gate written to reject exactly those. **Anchor the match**; an unanchored
substring is a filter that over-matches, and a filter that over-matches fails
in the direction that looks like success. (The same class as the bd ID audit's
exclusion list, and as `_JS_TEST`'s exclusion of `.skip`/`.only`.)
- A `proptest!` flag was set and never cleared, so the first `proptest!` in a
file made every later `fn` — helpers included — count as a test.
- An attribute holding a bracket inside a string (`#[cfg(feature = "[")]`)
left the bracket depth unbalanced and exempted the rest of the file. Every
question is therefore asked of the **scanner**, never of the raw line: the
scanner knows Rust strings, raw strings, char literals and nested block
comments, and the attribute depth is counted over code text only.
- A fence that opens and never closes exempted everything after it.
Two rules that are decisions rather than accidents. A ``` fence **suspends** the
run rather than ending it, so two lines above a fence and two below is a run of
four — a reader meets it as one uninterrupted block. And only a *runnable* fence
earns the exemption: rustdoc executes a bare ``` or ```rust and does not execute
```text, ```ignore or ```no_run, so the rest is prose wearing a fence.
## 2. The evidence check
`--check-evidence` holds `context/test-evidence/` and the tests it describes to
each other, in two directions:
- **Orphans.** Every heading shaped like a test name must name a test that runs.
- **Unstated.** Every test whose doc comment cites `context/test-evidence/` must
have an entry there.
Neither direction is "every test has evidence". Most tests have none and should
not be forced to have one; what is checkable is a *claim*, and both directions
are claims. The failure they close is the same one a dead ticket ID is: a
pointer that reads as a promise the thing exists.
### A leaf is not an identity
The names cargo lists are **module paths**, and the checker keeps them whole.
Six leaf names collide in this workspace — two of them cited in evidence — so
reducing a path to its last segment made a heading satisfiable by a test in
another module answering another question. Renaming
`ui::project::tests::a_load_that_fails_is_not_a_rebase`, the one its entry
actually describes, left the checker at *579 entries, all name a test that
runs*, exit 0: the surviving `ui::host::tests` namesake answered for it.
So a bare leaf resolves only when it is **unique**, and an ambiguous one is a
**refusal** naming the candidates — not a warning, because a warning re-creates
the problem for anyone who does not read it. The heading is then written with
its module path, which is also what makes the entry say which test it is about.
Note the two halves are both needed and neither is sufficient. The refusal
forces qualification but cannot survive a rename on its own: rename one of a
colliding pair and the leaf becomes *unique*, silently resolving to the
survivor. Qualification is what closes that, and the refusal is what makes
somebody write it.
### The names come from cargo, not from the source
A heading is diffed against `cargo test --workspace -- --list`, minus the names
that `--list --ignored` reports. It used to be scraped out of `.rs` text, and
four shapes defeated that, all of them "evidence for a check that never runs"
(improvise-rm4s):
| shape | why the scrape accepted it |
|---|---|
| `#[test] fn x` inside a raw string literal | the scanner reads inside strings |
| `#[cfg(feature = "…")] #[test]` | the feature is never enabled |
| `#[cfg(target_arch = "wasm32")] #[test]` | `cargo test --workspace` is native |
| `#[test] #[ignore]` | it compiles, and never runs |
The third is the realistic one here: this repo has real wasm32-only code and a
`wasm` CI job that only runs `cargo check`. Adopting the stricter source cost
nothing — all 539 headings resolved against a real listing on the commit that
made the change — which is the moment to tighten a gate rather than the moment
to argue about it.
Demonstrated before the fix, by injecting all four shapes into `src/draw.rs` and
`context/test-evidence/tui.md` at once: the previous checker printed *543
entries, all name a test that exists*, exit 0. The current one names all four.
What this costs: `--check-evidence` now needs a toolchain and a compiled
workspace, about three seconds warm. A missing `cargo` says so rather than
raising. The comment-cap check is unaffected and needs neither.
`--rust-tests FILE` takes a saved listing for a caller that has one, and it
**requires `--rust-tests-ignored FILE`**: a first version took one file, called
`cargo_test_names` with one argument, and so skipped the ignored rule entirely
while printing the same success line as the honest path. A flag that is
strictly more permissive and indistinguishable in the log is a bypass, not a
convenience — so the pair is mandatory and the success line **names its
source**. CI passes neither and runs cargo itself.
A **missing** `context/test-evidence/` is not an early return either. It used to
be, and it exited 0 while 170 tests cited a directory that was not there; the
walk now yields nothing and every citation is reported unstated, which is loud.
Note what the self-test can and cannot carry here. The listing *reader* is
testable from fixtures — a leaf name taken off a module path, an `--ignored`
name subtracted, cargo's own prose not mistaken for a test — and those cases
are in `SELF_TEST_LISTING`. The four *source shapes* are not: once `defined`
comes from cargo, a fixture asserting "a heading with an empty listing is an
orphan" is the same case however the Rust beside it is spelled, and writing
four of them would be four copies of one assertion dressed as coverage. The
evidence for those is the injection above, against the real tree.
What it does **not** close: `citing` is still scraped, so a citation inside a
string literal would be reported as a test owing an entry. That is a false
positive in the safe direction — loud, not silent — and it has no instance.
### A demotion is a reviewer's job, not a gate's
A heading is classified by **shape**: a bare identifier is a Rust test, a quoted
string a JS test, anything else is prose about the section (`Helper — x`,
`Note — y`, a file's own scope). So any orphaned heading can be retired by
rewriting it with a space and a dash, and nothing can check whether the prose is
honest — no mechanism could, because the question is whether a sentence is true
(improvise-9za9).
Two things make that tolerable. The unsafe direction is the one that fails
loudly: prose that *looks* like an identifier is classified as a Rust claim and
reported as an orphan, so the error you can make by accident is the noisy one.
And **the mechanism is `git diff context/test-evidence/`** — a demotion shows
up there as a `### identifier` line becoming prose, in the diff every reviewer
already reads. The live instance, PR #69 demoting a heading while PR #72
restored its coverage under another name, is what says a reviewer rather than a
gate has to catch it.
The printed count of prose headings is a **convenience, not a guard**: nothing
stores the previous count and nobody diffs two CI logs, so a retirement moves a
number no one is watching. Saying otherwise would claim a detectability it
cannot support, which is the class of defect §7's adjacent-check section is
about.
## 3. The others
`scripts/kb` is the only writer of `prolog-sessions/kb.pl`; its own reasoning is
in `context/code/kb-tool.md`. `scripts/node-test.sh`, `scripts/kb-test.sh`,
`scripts/pr-watch-test.sh` and `scripts/assemble-stamp-test.sh` are the same
shape as this file's subject: a guard, plus a suite that makes the guard refuse
what it claims to refuse. Each is wired into `.github/workflows/ci.yml` in the
same job as the thing it guards, because a check that only runs locally is a
convention.
`scripts/check-ci-graph.py` is the workflow's own gate: `pages-deploy` must
`needs` every other job in `ci.yml`. The rule "every check job" was written down
(improvise-x7cq) and a job added during review of PR #83 was still ungated with
nothing noticing, which is what moved it from a sentence to a check. It reads
the job keys and the `needs:` list off indentation, tracking which job a
`needs:` belongs to — taking the first one seen would read another job's list as
the deploy's and pass. `README.md` therefore states the rule and deliberately
**does not list the jobs**: a second copy with nothing holding it to the first
is what went stale, twice.
context/design-principles.md
+75 -8
View File
@@ -331,11 +331,28 @@ testing trivial getters or chasing 100% on rendering widgets. The test suite
should remain fast, and the rule is **two numbers** because the ambiguity
between them is what let the old single "under 2 seconds" drift: **no single
test binary may exceed 2 seconds**, and the workspace wall clock is the sum of
them. Measured at `22d43b2`: 1309 tests over 33 binaries, ~2.2s wall, longest
single binary 1.42s — so the workspace is already past 2s and the rule that
bites is the per-binary one. Excludes compilation, and excludes `harness/`,
which is Playwright and takes minutes in its own CI job. Slow tests erode the
habit of running them.
them. Excludes compilation, and excludes `harness/`, which is Playwright and
takes minutes in its own CI job. Slow tests erode the habit of running them.
**The threshold is a tripwire, and the seconds are not portable.** The same
commit measured `improvise-io`'s lib binary at 1.4s on one machine and
3.7–5.2s on another (improvise-8ec3), so a PR body citing "longest single
binary 1.47s" is checkable and *false* for the reader who checks it, which is
worse than no claim at all. Three consequences:
- **Re-measure; never re-assert.** `cargo test --workspace` prints a
`finished in` per binary. Read your own run.
- **If you cite a figure, name the machine you took it on.** A number with no
machine is a property of the suite, which it is not.
- **The portable signal is the ratio, not the seconds.** One binary an order of
magnitude above the run's **median** is the thing to look at — not above
second place, which it need not be: on one machine `improvise-io` is 2.2×
the runner-up and ~15× the median. It is the same binary on all three
machines it has been measured on: `improvise-io`'s lib, which
is proptest-dominated by the grammar-walking `.improv` generator and the
round-trip properties over it. Constraining that generator's strategy is the
open work (improvise-8shu); lowering the case count below 256 is not (see
below).
### Demonstrate bugs before fixing them
@@ -348,9 +365,59 @@ what fault was injected, and why the test can fail. It does not go in the test's
doc-comment: comment runs are capped at two lines (see below), and the evidence
is longer than that in every case worth having. The cost of the split is that a
renamed test orphans its entry. `scripts/check-comment-runs.py --check-evidence`
catches that: every `### name` heading must name a test that exists. It checks
one direction only, deliberately — a test may have no evidence (most do not),
but evidence naming a test nobody can find is a dangling explanation.
catches that, in two directions. Every `###`/`####` heading shaped like a test
name must name a test that **runs** — the names are diffed against
`cargo test --workspace -- --list` minus its `--ignored` twin, never scraped out
of the source, because text that looks like a test and a test are
indistinguishable to a scanner (improvise-rm4s). And a test whose doc comment
*cites* `context/test-evidence/` must have an entry there, or the pointer reads
as a promise the explanation exists.
Neither direction is "every test has evidence", deliberately: most tests have
none and should not be forced to have one. The reasoning and the residual holes
are in `context/code/tooling.md`.
### A check that looks like the answer, answering a neighbouring question
The 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:
- **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*.
- **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`.
- **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.
Each has the same remedy shape: **name the question you actually mean, then
find an input that separates it from its neighbour.**
- For an assertion: **assert the direction the default cannot supply.** Closing
the panel discriminates; opening it proves nothing.
- 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.
- For a merge: **materialise it and run the gates**, and check that a count
which should MOVE actually moved. Playwright going 16 → 17 is what showed the
other side survived; "no conflict" and an unchanged total would both have
been consistent with losing it.
This generalises `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.
### E2E pins the boundaries; unit tests work between them
context/test-evidence/command.md
+4 -1
View File
@@ -1711,7 +1711,10 @@ command that mutates the document in place, and a view-only one, are both diffs
the question `apply_model` asks is whether the generation moved, not which variant
arrived.
### a_load_that_fails_is_not_a_rebase
### ui::project::tests::a_load_that_fails_is_not_a_rebase
Qualified because `ui::host::tests` has a test of the same name: this is the
`apply_model` one, and a bare leaf was satisfiable by either.
A `LoadModel` naming a document the storage does not hold leaves the document
alone, so it is a diff too. This is why `apply_model` asks the generation rather
context/test-evidence/tooling.md
+151
View File
@@ -0,0 +1,151 @@
# Test evidence — the repository's own gates
Evidence for the checks in `scripts/` and for the tests that hold CI's shape.
The arguments behind the checkers themselves are in `context/code/tooling.md`;
this file records what fault each test was watched to catch.
Note what is deliberately absent. `scripts/check-comment-runs.py --self-test`,
`scripts/kb-test.sh` and `scripts/assemble-stamp-test.sh` are check-the-checker
suites whose cases are already injected faults, and none of their case names is
a Rust `fn` or a JS `test(` the evidence checker could resolve — so their
evidence is the case list itself plus `context/code/tooling.md`, and inventing
headings for them here would be headings naming nothing.
### "no test in web/tests/ reaches for a built bundle"
The `js` CI job runs `node --test web/tests/*.test.js` and installs nothing; the
`browser` job runs `web/tests/bundle/*.test.js` after three wasm-pack builds.
The moment a flat `web/tests/` file reads `web/pkg/`, the fast job inherits the
slow one's dependency — and the likely repair is to add a build step to it,
silently spending the ~4m30s the split exists to avoid. Until improvise-4ktt
that constraint was a directory convention and a glob, checked by nothing, and
it failed in the direction that looks like ordinary breakage: a confusing ENOENT
in CI on a machine where the bundle happens not to be built.
Fault injected, 2026-09-21: appended
`const injected = readFileSync("../pkg/improvise_wasm_client.js");` to
`web/tests/grid_plan.test.js`. The test failed naming that file and pointing at
`web/tests/bundle/`. Reverted; green again.
Two things the test does that a simpler one would not. It asserts its own
filename is among the files it scans, so it cannot become the one place a
violation may hide. And it strips comments before matching, because this file's
own header and the test's header both name the directory in prose — a lexical
grep over raw text would have had to exempt them, and an exemption is where the
next violation goes.
**What it cannot see**, stated because the check itself uses the evasion: a path
assembled at runtime (`"../" + "p" + "kg"`) is invisible to a lexical match, and
the test's own `FORBIDDEN` constant is built exactly that way to stay out of its
own scan. That is a deliberate, disclosed hole — closing it needs a parser, and
the failure it would catch is one somebody has to write on purpose, where this
gate exists for the one written by accident.
### "the subdirectory that MAY use a bundle is not the one being checked"
The companion, and the one that stops the first from passing vacuously. If
`web/tests/bundle/` were emptied or its files stopped touching the bundle, the
first test would still be green while having nothing to be scoped *against* —
"no flat test uses the bundle" is uninteresting if no test anywhere does. This
asserts the other half of the split exists and really does what the flat half
is forbidden to do.
Fault injected, 2026-09-21: moved `web/tests/bundle/wasm_surface.test.js` out of
the tree. The test failed with "web/tests/bundle/ is empty — the split lost its
other half" rather than passing. Restored; green again.
### "no test in web/tests/ imports a module that reaches the bundle"
The companion to the lexical check above, and the reason that one is not
enough: `import "../app.js"` needs the bundle *transitively* and names nothing
forbidden, so the `js` job breaks loudly but unattributed. `app.js` and
`worker.js` are the two entry points that load the bundle; `shell.js` and
`serial.js` do not, and `serial.test.js` imports one legitimately.
Fault injected, 2026-09-21: appended `import "../app.js";` to
`web/tests/grid_plan.test.js`. The lexical test stayed green; this one failed
naming the file. Reverted.
### Note — the checker's own blockers, found by review of PR #83
Two gates in `scripts/check-comment-runs.py` produced a pass while not doing
what their prose said. Both were in the mode the same PR rewrote, which is the
part worth keeping: writing the doctrine down does not exempt the writer.
**A leaf name is not an identity.** Six leaf names collide in this workspace and
two were cited in evidence. Renaming
`ui::project::tests::a_load_that_fails_is_not_a_rebase` — the one its entry
describes — left the checker printing *all name a test that runs*, exit 0, with
the heading answered by the `ui::host::tests` namesake. Now an ambiguous leaf is
refused with both candidates named, and the two entries are qualified.
Re-verified both ways, 2026-09-21: the refusal fires on the unqualified
headings and names exactly those two; with the heading qualified, the same
rename is reported as an orphan rather than absorbed.
**`--rust-tests` was strictly more permissive and said the same words.** It
called `cargo_test_names` with one argument, so the `--ignored` subtraction
never ran, and the success line was byte-identical to the honest path's.
Injecting `### eyeball_the_rendered_hints_and_pages` orphaned on the default
path and passed on the flag's. Now the flag requires
`--rust-tests-ignored` (one alone exits 2) and the success line names its
source.
**A missing evidence directory exited 0.** The early return preceded the
citation scan, so deleting `context/test-evidence/` reported nothing while 170
citations pointed at a directory that was not there. Verified 2026-09-21 by
moving the directory aside: exit 1, *170 tests cite context/test-evidence and
have no entry there*.
### Note — the stamp gate's discrimination, measured
`scripts/assemble-stamp-test.sh` separates three outcomes rather than two, which
is what makes it evidence rather than a smoke test. Measured 2026-09-21 over its
nine cases: the real script passes 9; deleting the final `test -z "$missing"`
(the PR #69 injection, which the inline version survived at exit 0) passes 4 and
fails 5; a script that does nothing passes 2; one that always fails passes 0.
The PR body first cited "4 of 9" for the middle row, from a run taken when the
suite had eight cases. A figure is what the next reader checks, so it is
recorded here where it can be re-measured rather than only in a message nothing
can audit (design-principles §6).
### Note — the stamp gate matches literally
`scripts/assemble-stamp.sh` greps with `-F`. As a regex, the `.` in
`improvise_wasm_client.js?v=abc` matches any character, so a stamp corrupted in
exactly that position reads as present — a pass in the one direction a guard
must not fail in. Inherited from the inline version this was extracted from,
and found by reading the extraction rather than by anything failing.
Fault injected, 2026-09-21: dropping the `-F` makes
`improvise_wasm_clientXjs?v=abc` satisfy the pattern, and
`a pattern is matched literally, not as a regex` goes from pass to fail.
### Note — the doc site's derived checks
`scripts/build-doc-site.mjs` carries two gates that no test names, because the
generator *is* the test: it runs on every assembly and exits non-zero.
Faults injected, 2026-09-21, each observed:
| injected | reported |
|---|---|
| a new `context/code/newarea.md` | `context/code/newarea.md is neither published nor a named omission` |
| `context/plan.md` moved out of the tree | `context/plan.md is listed as omitted but does not exist` |
| a line appended below the generator in `web/assemble.sh` | `web/assemble.sh must invoke this generator LAST`, naming the offending command |
| a trailing `echo "see scripts/build-doc-site.mjs"` | the same, once the check asks for an INVOCATION rather than a mention — `.includes` had accepted it (review, PR #83) |
`scripts/check-ci-graph.py` is the third, and it is a script rather than a
stanza because its own refusals need checking: adding a `zz-new-check:` job to
`ci.yml` reports *pages-deploy does not need `zz-new-check`* and exits 1, where
before the rule was prose and nothing noticed. Its five self-test graphs include
the one a naive parse gets wrong — another job's `needs:` read as the deploy's.
The second arm is the one that would not get written first, and it is the point:
without it, deleting or renaming an omitted file leaves an entry asserting a
reason for a file nobody can find — the same failure as a dead ticket id, which
`AGENTS.md` already calls worse than no id.
The first arm was filed as latent and was live when it was written:
`context/code/kb-tool.md` had been unpublished, unargued and unnoticed since it
was added.
context/test-evidence/tui.md
+6 -3
View File
@@ -384,10 +384,13 @@ the move onto a projected cache, one pinning the *drawing* of them. The pair is
the shape — a cache equivalence alone would not notice a changed renderer, and a
rendered fixture alone would not notice the cache carrying different data.
### the_cache_carries_what_the_widget_used_to_read_off_the_model
### ui::tile_bar::tests::the_cache_carries_what_the_widget_used_to_read_off_the_model
Two tests share this name, in `src/ui/tile_bar.rs` and
`src/ui/formula_panel.rs`. The tile bar used to read
### ui::formula_panel::tests::the_cache_carries_what_the_widget_used_to_read_off_the_model
Two tests share this leaf name, which is why both headings are qualified: one
entry describes the pair, and an unqualified heading would have been satisfied
by whichever of them the checker happened to see. The tile bar used to read
`view.tileable_cats(&model.category_names())` and `view.axis_of(cat)` off the
model directly; the formula panel used to read `model.formulas()[i].raw`. Each
now reads its cache, and a divergence is a rendering change nobody asked for.
scripts/assemble-stamp-test.sh
Executable
+80
View File
@@ -0,0 +1,80 @@
#!/usr/bin/env bash
# check_the_checker for scripts/assemble-stamp.sh. The gate it holds was
# deleted on PR #69 by injection and the assembly still exited 0 with both
# failures printed -- a guard nobody had watched fail (improvise-w8ih).
set -uo pipefail
here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
RUN="$here/assemble-stamp.sh"
work="$(mktemp -d)"
trap 'rm -rf "$work"' EXIT
pass=0; fail=0
ok() { printf ' ok %s\n' "$1"; pass=$((pass+1)); }
bad() { printf ' FAIL %s (exit %s, wanted %s)\n' "$1" "$2" "$3"; fail=$((fail+1)); }
# $1 name, $2 expected exit, $3 expected stderr substring ("" for none), rest: args
run_case() {
local name="$1" want="$2" says="$3"; shift 3
local err
err="$(sh "$RUN" "$@" 2>&1 >/dev/null)"
local got=$?
if [ "$got" != "$want" ]; then
bad "$name" "$got" "$want"
elif [ -n "$says" ] && [[ "$err" != *"$says"* ]]; then
bad "$name: stderr did not name it" "$err" "$says"
else
ok "$name"
fi
}
printf 'import "./pkg/improvise_wasm_client.js?v=abc";\n' > "$work/stamped.js"
printf 'import "./worker-pkg/improvise_worker_server.js?v=abc";\n' > "$work/stamped2.js"
printf 'import "./pkg/improvise_wasm_client.js";\n' > "$work/bare.js"
printf 'import "./worker-pkg/improvise_worker_server.js";\n' > "$work/bare2.js"
run_case "a stamped file passes" 0 "" \
"$work/stamped.js" "pkg/improvise_wasm_client.js?v=abc"
run_case "an unstamped file is refused and named" 1 "bare.js" \
"$work/bare.js" "pkg/improvise_wasm_client.js?v=abc"
# The bug the gate exists for: without the final `test -z`, the misses print
# and the script exits 0. Both must be NAMED, which is what exiting on the
# first one cost.
run_case "two misses are both named, not just the first" 1 "bare2.js" \
"$work/bare.js" "pkg/improvise_wasm_client.js?v=abc" \
"$work/bare2.js" "worker-pkg/improvise_worker_server.js?v=abc"
run_case "a miss among hits is still refused" 1 "bare2.js" \
"$work/stamped.js" "pkg/improvise_wasm_client.js?v=abc" \
"$work/bare2.js" "worker-pkg/improvise_worker_server.js?v=abc"
run_case "all four stamped is the shipping case" 0 "" \
"$work/stamped.js" "pkg/improvise_wasm_client.js?v=abc" \
"$work/stamped2.js" "worker-pkg/improvise_worker_server.js?v=abc"
# A file the assembly never copied is indistinguishable from an unstamped one
# to `grep`, which errors rather than failing quietly -- but only because the
# -f test runs first. Without it `set -e` would abort mid-list and the later
# files would go unreported, which is the failure this gate was built to end.
run_case "a file that does not exist is a miss, not an abort" 1 "vanished.js" \
"$work/vanished.js" "anything" \
"$work/bare2.js" "worker-pkg/improvise_worker_server.js?v=abc"
# A caller that drops one argument would otherwise have the last FILE read as
# a PATTERN over the file before it -- a check that passes by accident.
# The pattern is a literal. As a regex, `.` matches any character, so a stamp
# corrupted in exactly that position would read as present -- the one direction
# a guard must not fail in.
printf 'import "./pkg/improvise_wasm_clientXjs?v=abc";\n' > "$work/corrupt.js"
run_case "a pattern is matched literally, not as a regex" 1 "corrupt.js" \
"$work/corrupt.js" "pkg/improvise_wasm_client.js?v=abc"
run_case "an odd number of arguments is a usage error" 2 "pairs" \
"$work/stamped.js" "pkg/improvise_wasm_client.js?v=abc" "$work/bare.js"
run_case "no arguments at all is a usage error, not a pass" 2 "usage"
printf '\nassemble-stamp-test: %d passed, %d failed\n' "$pass" "$fail"
[ "$fail" -eq 0 ]
scripts/assemble-stamp.sh
Executable
+40
View File
@@ -0,0 +1,40 @@
#!/bin/sh
# Every cache-bust stamp web/assemble.sh writes must be readable back.
#
# Usage: scripts/assemble-stamp.sh FILE PATTERN [FILE PATTERN ...]
#
# It RECORDS every miss and fails once at the end, naming all of them. Exiting
# on the first named one file and never reached the rest, so a shape change
# that broke two stamps cost two wasm-pack rebuilds to diagnose -- the slowest
# loop in this repo (improvise-x4wv).
#
# It lives here rather than inside assemble.sh because the gate is the whole
# guard and deleting it was invisible: both misses still printed and the
# assembly exited 0 (improvise-w8ih). Exercising it in assemble.sh would need a
# fabricated web/pkg, which is a second answer to "what does the site assemble
# from"; exercising it here needs nothing. scripts/assemble-stamp-test.sh is
# the check_the_checker, and CI runs it.
set -eu
test $# -ge 2 || { echo "usage: assemble-stamp.sh FILE PATTERN [FILE PATTERN ...]" >&2; exit 2; }
test $((($# % 2))) -eq 0 || { echo "assemble-stamp: FILE and PATTERN come in pairs" >&2; exit 2; }
missing=""
while [ $# -ge 2 ]; do
file=$1
pattern=$2
shift 2
# -F, because every pattern is a literal and `.` as "any character" would
# let a corrupted stamp match -- a pass in the direction that looks like one.
if [ -f "$file" ] && grep -qF "$pattern" "$file"; then
grep -nF "$pattern" "$file"
else
echo "assemble: $file is missing the cache-bust stamp: $pattern" >&2
missing="${missing} $file"
fi
done
test -z "$missing" || {
echo "assemble: refusing to ship; unstamped:${missing}" >&2
exit 1
}
scripts/build-doc-site.mjs
+56 -1
View File
@@ -11,7 +11,7 @@
// The reorder also moved WHERE a failure shows: a dead link now leaves a site/
// that looks complete with a partial docs/, where before it left one with no
// app/. Contained only because assemble.sh is `set -eu` and CI gates on it.
import { readFileSync, writeFileSync, mkdirSync, rmSync, existsSync, statSync } from "node:fs";
import { readFileSync, writeFileSync, mkdirSync, rmSync, readdirSync, existsSync, statSync } from "node:fs";
import { dirname, join, relative } from "node:path";
import { fileURLToPath } from "node:url";
@@ -59,6 +59,8 @@ const SECTIONS = [
["context/code/clients.md", "code/clients", "The clients"],
["context/code/tui.md", "code/tui", "The TUI"],
["context/code/gp.md", "code/gp", "gp-tree and improvise-gp"],
["context/code/kb-tool.md", "code/kb-tool", "The kb tool"],
["context/code/tooling.md", "code/tooling", "The repository's own checkers"],
],
},
{
@@ -74,6 +76,7 @@ const SECTIONS = [
["context/test-evidence/clients.md", "test-evidence/clients", "The clients"],
["context/test-evidence/tui.md", "test-evidence/tui", "The TUI"],
["context/test-evidence/gp.md", "test-evidence/gp", "gp-tree and improvise-gp"],
["context/test-evidence/tooling.md", "test-evidence/tooling", "The repository's own gates"],
],
},
{
@@ -87,6 +90,58 @@ const SECTIONS = [
const PAGE_BY_SOURCE = new Map();
for (const s of SECTIONS) for (const [src, slug] of s.docs) PAGE_BY_SOURCE.set(src, `${slug}.html`);
// An omission is a DECISION, so it is restated here beside the manifest rather
// than only in doc-site.md section 2. An allowlist and not a count: a new
// omission has to be argued in code, and a vanished one has to be removed --
// a stale entry asserting a reason for a file nobody can find is the failure a
// dead ticket id is (improvise-res9).
const OMITTED = new Map([
["context/plan.md", "a prompt, not a plan; cites a context/SPEC.md that no longer exists"],
["context/cloud-agents.md", "operational notes for agents, not about the program"],
]);
// Derived, because the manifest only ever grows and a doc added without a line
// here publishes nothing and says nothing: build green, link check green, doc
// simply absent. This runs at module scope, before any page is generated, so
// it answers the manifest question before the outward-link check answers a
// different one -- a missing doc is actionable wherever you ran the generator.
{
const onDisk = [];
(function walk(d) {
for (const e of readdirSync(join(root, d), { withFileTypes: true })) {
const rel = `${d}/${e.name}`;
if (e.isDirectory()) walk(rel);
else if (e.name.endsWith(".md")) onDisk.push(rel);
}
})("context");
onDisk.push("docs/design-notes.md");
const unaccounted = onDisk.filter((f) => !PAGE_BY_SOURCE.has(f) && !OMITTED.has(f));
const vanished = [...OMITTED.keys()].filter((f) => !existsSync(join(root, f)));
if (unaccounted.length || vanished.length) {
for (const f of unaccounted) console.error(`doc-site: ${f} is neither published nor a named omission`);
for (const f of vanished) console.error(`doc-site: ${f} is listed as omitted but does not exist`);
process.exit(1);
}
}
// The generator's link check resolves ../index.html and ../app/ against the
// filesystem, so it must run once the rest of the tree is there. PR #63's
// appended loop happened to land above it; one landing below would make the
// outward-link check pass against a half-built tree, silently.
{
const cmds = readFileSync(join(root, "web", "assemble.sh"), "utf8")
.split("\n")
.map((l) => l.trim())
.filter((l) => l && !l.startsWith("#"));
// An INVOCATION, not a mention: `echo "see scripts/build-doc-site.mjs"` after
// a real command satisfied `.includes` while running nothing (review, PR #83).
if (!/^(node|sh)\s.*build-doc-site\.mjs/.test(cmds[cmds.length - 1])) {
console.error("doc-site: web/assemble.sh must invoke this generator LAST;");
console.error(`doc-site: its last command is instead: ${cmds[cmds.length - 1]}`);
process.exit(1);
}
}
// ---------------------------------------------------------------------------
// The ticket index. `.beads/issues.jsonl` is a passive export and lags the Dolt
// database, so an id it does not carry is rendered as plain text rather than a
scripts/check-ci-graph.py
Executable
+124
View File
@@ -0,0 +1,124 @@
#!/usr/bin/env python3
"""pages-deploy must `needs` every other job in the workflow.
The rule that does not go stale is "every check job"; a subset is what let
README name three of seven while the rest read as ungated (improvise-x7cq).
Stating it was not enough -- a new job was added during review of PR #83 and
nothing noticed, so the rule is a gate. See context/code/tooling.md.
`--self-test` checks the checker: each case is a graph this must refuse.
"""
import re
import sys
WORKFLOW = ".github/workflows/ci.yml"
DEPLOY = "pages-deploy"
_JOB = re.compile(r"^ ([A-Za-z0-9_-]+):$")
_NEEDS = re.compile(r"^ needs:\s*\[(.*)\]\s*$")
def read_graph(text):
"""(all job names in order, the names `pages-deploy` needs).
Both come off indentation, which is what a workflow's own shape is. A job
key is two spaces; `needs:` is four INSIDE the job it belongs to, so the
current job is tracked rather than the first `needs:` taken.
"""
jobs, needs, current = [], None, None
in_jobs = False
for raw in text.split("\n"):
if raw.startswith("jobs:"):
in_jobs = True
continue
if not in_jobs:
continue
m = _JOB.match(raw)
if m:
current = m.group(1)
jobs.append(current)
continue
m = _NEEDS.match(raw)
if m and current == DEPLOY:
needs = [n.strip() for n in m.group(1).split(",") if n.strip()]
return jobs, needs
def verdict(jobs, needs):
"""The complaints, as strings. Empty means the graph is sound."""
if DEPLOY not in jobs:
return [f"{DEPLOY} is not a job in this workflow"]
if needs is None:
return [f"{DEPLOY} has no `needs:` list, so nothing gates the deploy"]
expected = {j for j in jobs if j != DEPLOY}
missing = sorted(expected - set(needs))
unknown = sorted(set(needs) - expected)
out = [f"{DEPLOY} does not need `{j}`, so a failing {j} cannot stop a deploy"
for j in missing]
out += [f"{DEPLOY} needs `{j}`, which is not a job in this workflow" for j in unknown]
return out
# (name, workflow text, the complaints it must draw).
SELF_TEST = [
("a job nobody gates is named",
"jobs:\n test:\n x: 1\n zz-new-check:\n x: 1\n"
" pages-deploy:\n needs: [test]\n",
[f"{DEPLOY} does not need `zz-new-check`, so a failing zz-new-check cannot stop a deploy"]),
("the complete graph is accepted",
"jobs:\n test:\n x: 1\n wasm:\n x: 1\n"
" pages-deploy:\n needs: [test, wasm]\n", []),
("a needs naming a job that does not exist is caught",
"jobs:\n test:\n x: 1\n pages-deploy:\n needs: [test, ghost]\n",
[f"{DEPLOY} needs `ghost`, which is not a job in this workflow"]),
("a deploy with no needs at all is caught",
"jobs:\n test:\n x: 1\n pages-deploy:\n x: 1\n",
[f"{DEPLOY} has no `needs:` list, so nothing gates the deploy"]),
# `needs:` belongs to the job it is indented under. Taking the first one
# seen would read ANOTHER job's list as the deploy's and pass.
("another job's needs is not mistaken for the deploy's",
"jobs:\n test:\n x: 1\n browser:\n needs: [test]\n"
" pages-deploy:\n x: 1\n",
[f"{DEPLOY} has no `needs:` list, so nothing gates the deploy"]),
]
def self_test():
failures = []
for name, text, want in SELF_TEST:
jobs, needs = read_graph(text)
got = verdict(jobs, needs)
if got != want:
failures.append(f" {name}: expected {want}, saw {got}")
if failures:
print("ci-graph self-test FAILED:", file=sys.stderr)
print("\n".join(failures), file=sys.stderr)
return 1
print(f"ci-graph self-test: {len(SELF_TEST)} graphs judged")
return 0
def main():
if "--self-test" in sys.argv[1:]:
return self_test()
jobs, needs = read_graph(open(WORKFLOW, encoding="utf8").read())
if len(jobs) < 2:
print(f"error: {WORKFLOW}: found {len(jobs)} jobs; the parse is wrong",
file=sys.stderr)
return 2
complaints = verdict(jobs, needs)
if complaints:
for c in complaints:
print(f"ci-graph: {c}", file=sys.stderr)
return 1
print(f"ci-graph: {DEPLOY} needs all {len(jobs) - 1} other jobs")
return 0
if __name__ == "__main__":
sys.exit(main())
scripts/check-comment-runs.py
+250 -58
View File
@@ -22,6 +22,7 @@ the rest is prose wearing a fence and the cap applies to it.
import argparse
import os
import re
import subprocess
import sys
LIMIT = 2
@@ -252,7 +253,7 @@ EVIDENCE_DIR = os.path.join("context", "test-evidence")
EVIDENCE_CITE = "test-evidence"
JS_ROOTS = [os.path.join("harness", "tests"), os.path.join("web", "tests")]
_RUST_NAME = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
_RUST_NAME = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*(?:::[A-Za-z_][A-Za-z0-9_]*)*$")
_QUOTED_NAME = re.compile(r'^"(.+)"$')
# Only a RUNNABLE LEAF test. `.skip`, `.todo`, `.describe` and `.only` are all
# excluded: the first three name no executed assertion, and `.only` disables
@@ -264,15 +265,17 @@ _JS_TEST = re.compile(
def evidence_claims(text):
"""Yield (kind, name, lineno) for every heading in an evidence file.
"""Yield ("rust"|"js"|"prose", name, lineno) for every heading in a file.
`### ` and `#### ` alike, because the browser entries have to use `####`
and were invisible to a check that collected only `### ` (improvise-5ray).
A heading is a CLAIM when it is shaped like a test name -- a bare
identifier (Rust) or a quoted string (JS). Anything else is prose about
the section: `Helper - x`, `Note - y`, a file's own scope. That shape rule
is the whole discrimination, so see context/code/tooling.md before loosening
it: a heading that is neither shape names nothing and is checked by nobody.
is the whole discrimination, and rewriting a claim as prose retires it with
nothing checking the prose is honest (improvise-9za9). The mechanism is the
diff a reviewer reads, not the printed prose count, which nothing stores.
See context/code/tooling.md.
"""
for n, raw in enumerate(text.split("\n"), 1):
s = raw.strip()
@@ -284,16 +287,54 @@ def evidence_claims(text):
yield "js", m.group(1), n
elif _RUST_NAME.match(body):
yield "rust", body, n
else:
yield "prose", body, n
def rust_tests(text):
"""(tests defined here, tests whose doc comment cites the evidence dir).
_CARGO_LISTED = re.compile(r"^(\S+): test$")
A heading must name a TEST, not merely a function. Accepting any `fn` let a
`###` naming a fixture or a helper pass, so an evidence file could report
itself satisfied while naming nothing that runs (found on PR #39).
def cargo_test_names(listing, ignored_listing=""):
"""The FULL PATHS cargo says it will run, from `cargo test -- --list`.
Paths and not leaves: six leaf names collide in this workspace, so a leaf
is not an identity. See context/code/tooling.md and improvise-rm4s.
"""
defined, citing = set(), set()
def paths(text):
return {
m.group(1)
for m in (_CARGO_LISTED.match(line.strip()) for line in text.split("\n"))
if m
}
return paths(listing) - paths(ignored_listing)
def _leaf(path):
return path.split("::")[-1]
def _cargo_listing(extra):
cmd = ["cargo", "test", "--workspace", "--", "--list", *extra]
try:
run = subprocess.run(cmd, capture_output=True, text=True)
except OSError as e:
raise SystemExit(f"error: cannot run cargo ({e}).\n"
"--check-evidence needs a toolchain, or --rust-tests FILE "
"holding a saved `cargo test --workspace -- --list`.")
if run.returncode != 0:
raise SystemExit(f"error: {' '.join(cmd)} failed:\n{run.stderr}")
return run.stdout
def rust_citations(text):
"""The tests here whose doc comment cites the evidence dir.
A citation obliges only a TEST. Accepting any `fn` let a `###` naming a
fixture or a helper pass, so an evidence file could report itself satisfied
while naming nothing that runs (found on PR #39).
"""
citing = set()
annotated = False
doc = []
# A `proptest!` block accepts its bodies as tests -- but only INSIDE the
@@ -326,7 +367,6 @@ def rust_tests(text):
if s.startswith("fn ") or s.startswith("pub fn ") or s.startswith("async fn "):
if annotated or in_proptest:
name = s.split("fn ", 1)[1].split("(")[0].split("<")[0].strip()
defined.add(name)
if any(EVIDENCE_CITE in d for d in doc):
citing.add(name)
annotated = False
@@ -334,7 +374,7 @@ def rust_tests(text):
elif s and not s.startswith("#["):
annotated = False
doc = []
return defined, citing
return citing
def js_tests(text):
@@ -373,32 +413,54 @@ def js_files(roots):
def evidence_verdict(claims, defined, citing):
"""(orphans, unstated) -- the two directions, keyed by ("rust"|"js", name).
"""(orphans, unstated, ambiguous), keyed by ("rust"|"js", name).
`orphans` is evidence naming a test nobody can find. `unstated` is the
reverse obligation (improvise-lyyc): a test that POINTS AT the evidence
file must appear in it, or a reader following the pointer arrives nowhere.
Only that narrower obligation, because most tests have no evidence and
should not be forced to. Note what neither direction can see: an entry can
exist and still describe its subject backwards, which is discipline rather
than a gate.
`ambiguous` is a bare leaf that two tests answer to -- a refusal and not a
warning, because a heading satisfied by a test in another module answering
another question is a pass while the gate does nothing.
Only that narrower reverse obligation, because most tests have no evidence
and should not be forced to. What none of the three can see: an entry can
exist and still describe its subject backwards.
"""
orphans = {k: v for k, v in claims.items() if k[1] not in defined[k[0]]}
by_leaf = {}
for path in defined["rust"]:
by_leaf.setdefault(_leaf(path), set()).add(path)
orphans, ambiguous = {}, {}
for key, where in claims.items():
kind, name = key
if kind == "js":
if name not in defined["js"]:
orphans[key] = where
continue
if name in defined["rust"]:
continue
matches = by_leaf.get(name, set())
if len(matches) == 1:
continue
(ambiguous if matches else orphans)[key] = where
# A citation is a bare `fn` name, so it is satisfied by a QUALIFIED heading
# naming that leaf -- qualifying a heading must not orphan its own test.
claimed = {(kind, _leaf(name)) for kind, name in claims}
unstated = {}
for kind, names in citing.items():
for name in names:
if (kind, name) not in claims:
if (kind, name) not in claimed:
unstated.setdefault((kind, name), []).extend(sorted(names[name]))
return orphans, unstated
return orphans, unstated, ambiguous
def check_evidence(roots):
"""Both directions over context/test-evidence/. See evidence_verdict."""
if not os.path.isdir(EVIDENCE_DIR):
print("no context/test-evidence/ yet: nothing to check")
return 0
def check_evidence(roots, rust_listing=None, rust_ignored=None):
"""Both directions over context/test-evidence/. See evidence_verdict.
claims = {}
A MISSING evidence directory is not an early return: it used to be, and it
exited 0 while 170 citations pointed at nothing (review of PR #83).
"""
claims, prose = {}, 0
for dirpath, _, filenames in os.walk(EVIDENCE_DIR):
for f in sorted(filenames):
if not f.endswith(".md"):
@@ -406,14 +468,23 @@ def check_evidence(roots):
path = os.path.join(dirpath, f)
text = open(path, encoding="utf8", errors="replace").read()
for kind, name, n in evidence_claims(text):
claims.setdefault((kind, name), []).append(f"{path}:{n}")
if kind == "prose":
prose += 1
else:
claims.setdefault((kind, name), []).append(f"{path}:{n}")
defined = {"rust": set(), "js": set()}
if rust_listing is None:
source = "cargo"
rust_names = cargo_test_names(_cargo_listing([]), _cargo_listing(["--ignored"]))
else:
source = f"{rust_listing} + {rust_ignored}"
rust_names = cargo_test_names(open(rust_listing, encoding="utf8").read(),
open(rust_ignored, encoding="utf8").read())
defined = {"rust": rust_names, "js": set()}
citing = {"rust": {}, "js": {}}
for path in rust_files(roots):
d, c = rust_tests(open(path, encoding="utf8", errors="replace").read())
defined["rust"] |= d
for name in c:
for name in rust_citations(open(path, encoding="utf8", errors="replace").read()):
citing["rust"].setdefault(name, []).append(path)
for path in js_files(JS_ROOTS):
d, c = js_tests(open(path, encoding="utf8", errors="replace").read())
@@ -421,15 +492,27 @@ def check_evidence(roots):
for name in c:
citing["js"].setdefault(name, []).append(path)
orphans, unstated = evidence_verdict(claims, defined, citing)
if not orphans and not unstated:
print(f"test-evidence: {len(claims)} entries, all name a test that exists; "
f"{sum(len(v) for v in citing.values())} tests cite the file and all have one")
orphans, unstated, ambiguous = evidence_verdict(claims, defined, citing)
if not orphans and not unstated and not ambiguous:
print(f"test-evidence: {len(claims)} entries, all name a test that runs "
f"(names from {source}); "
f"{sum(len(v) for v in citing.values())} tests cite the file and all have one; "
f"{prose} prose headings claim nothing")
return 0
if orphans:
print(f"{len(orphans)} test-evidence entries name no test:\n", file=sys.stderr)
for (kind, name), where in sorted(orphans.items()):
print(f" [{kind}] {name} ({', '.join(where)})", file=sys.stderr)
if ambiguous:
by_leaf = {}
for path in defined["rust"]:
by_leaf.setdefault(_leaf(path), set()).add(path)
print(f"\n{len(ambiguous)} test-evidence entries name a leaf that is not unique; "
"write the module path:\n", file=sys.stderr)
for (kind, name), where in sorted(ambiguous.items()):
print(f" [{kind}] {name} ({', '.join(where)})", file=sys.stderr)
for path in sorted(by_leaf[name]):
print(f" -> {path}", file=sys.stderr)
if unstated:
print(f"\n{len(unstated)} tests cite {EVIDENCE_DIR} and have no entry there:\n",
file=sys.stderr)
@@ -669,7 +752,25 @@ fn g() {}
]
# (name, evidence markdown, rust source, js source, orphans, unstated).
# (name, listing, ignored listing, the paths that RUN).
#
# Three source shapes defeated the old scrape -- a test in a string literal, one
# behind a cfg feature, one behind cfg(target_arch = "wasm32") -- and an ignored
# test defeats a listing read without its `--ignored` twin (improvise-rm4s).
SELF_TEST_LISTING = [
("the module path is kept, because a leaf is not an identity",
"help::tests::a_real_test: test\n", "", ["help::tests::a_real_test"]),
("an ignored test is one that does not run",
"a::runs: test\na::skipped: test\n", "a::skipped: test\n", ["a::runs"]),
("cargo's prose is not a test name",
" Running unittests src/lib.rs (target/debug/deps/x-1)\n"
"2 tests, 0 benchmarks\n", "", []),
]
# (name, evidence markdown, rust source, js source, cargo listing, orphans, unstated).
#
# The evidence checker had no self-test at all, which is how both halves of
# improvise-5ray and improvise-lyyc survived: the shape that silenced the
@@ -677,87 +778,166 @@ fn g() {}
SELF_TEST_EVIDENCE = [
("a #### heading naming no test is an orphan",
'#### "a test that does not exist anywhere"\n',
"", "", ["a test that does not exist anywhere"], []),
"", "", "", ["a test that does not exist anywhere"], []),
("a #### heading naming a JS test that exists is satisfied",
'#### "a peer that cannot be reached is named"\n',
"", 'test("a peer that cannot be reached is named", async () => {});\n', [], []),
"", 'test("a peer that cannot be reached is named", async () => {});\n',
"", [], []),
("a quoted heading can never be satisfied by a Rust fn of that name",
'#### "a_rust_name"\n',
"#[test]\nfn a_rust_name() {}\n", "", ["a_rust_name"], []),
"#[test]\nfn a_rust_name() {}\n", "", "m::a_rust_name: test\n",
["a_rust_name"], []),
("a prose #### heading claims nothing",
"#### Helper — `addressed`\n#### Note — the basis_check module\n"
"#### harness/tests/shell.spec.js — the file's own scope\n",
"", "", [], []),
"", "", "", [], []),
("a ### naming a helper rather than a test is still an orphan",
"### a_helper\n", "fn a_helper() {}\n", "", ["a_helper"], []),
"### a_helper\n", "fn a_helper() {}\n", "", "", ["a_helper"], []),
("a test that cites the evidence file and has no entry is caught",
"", "/// See `context/test-evidence/clients.md`.\n#[test]\nfn a_cited_test() {}\n",
"", [], ["a_cited_test"]),
"", "m::a_cited_test: test\n", [], ["a_cited_test"]),
("the same test with an entry is satisfied",
"### a_cited_test\n",
"/// See `context/test-evidence/clients.md`.\n#[test]\nfn a_cited_test() {}\n",
"", [], []),
"", "m::a_cited_test: test\n", [], []),
("a NON-test fn citing the file is under no obligation",
"", "/// See `context/test-evidence/clients.md`.\nfn a_helper() {}\n", "", [], []),
"", "/// See `context/test-evidence/clients.md`.\nfn a_helper() {}\n", "",
"", [], []),
("a citation belonging to the fn ABOVE does not attach to the next one",
"### cited\n",
"/// See `context/test-evidence/clients.md`.\n#[test]\nfn cited() {}\n"
"\n#[test]\nfn uncited() {}\n",
"", [], []),
"", "m::cited: test\nm::uncited: test\n", [], []),
("a heading cannot be satisfied by a skipped or todo test",
'#### "a skipped one"\n#### "a todo one"\n', "",
'test.skip("a skipped one", () => {});\ntest.todo("a todo one");\n',
["a skipped one", "a todo one"], []),
"", ["a skipped one", "a todo one"], []),
("a heading cannot be satisfied by a test.only, which disables its file",
'#### "the only one left running"\n', "",
'test.only("the only one left running", () => {});\n',
["the only one left running"], []),
"", ["the only one left running"], []),
("a heading cannot be satisfied by an empty suite's name",
'#### "a suite with nothing in it"\n', "",
'test.describe("a suite with nothing in it", () => {});\n',
["a suite with nothing in it"], []),
"", ["a suite with nothing in it"], []),
("a test nested in a suite still satisfies its heading",
'#### "a nested test"\n', "",
'test.describe("a group", () => {\n test("a nested test", () => {});\n});\n',
[], []),
"", [], []),
("a JS test that cites the evidence file and has no entry is caught",
"", "",
'// See context/test-evidence/clients.md.\ntest("a cited spec", async () => {});\n',
[], ["a cited spec"]),
"", [], ["a cited spec"]),
]
def _evidence_self_test():
# (name, evidence markdown, the kind of each heading in order).
#
# Prose is a third KIND rather than a heading the collector drops, so retiring a
# claim by rewriting it as prose moves the reported count (improvise-9za9).
SELF_TEST_HEADINGS = [
("a heading is classified three ways, prose included",
'#### Helper — `addressed`\n### a_rust_name\n#### "a js name"\n',
["prose", "rust", "js"]),
("a module-qualified heading is a Rust claim, not prose",
"### ui::project::tests::a_load_that_fails_is_not_a_rebase\n", ["rust"]),
]
# (name, evidence markdown, cargo listing, orphans, ambiguous).
#
# Six leaf names collide in this workspace and two were cited, so a heading was
# satisfiable by a test in another module answering another question -- renaming
# the one an entry describes left the checker at exit 0 (review of PR #83).
SELF_TEST_AMBIGUITY = [
("a leaf two tests answer to is refused, not silently resolved",
"### a_load_that_fails_is_not_a_rebase\n",
"ui::host::tests::a_load_that_fails_is_not_a_rebase: test\n"
"ui::project::tests::a_load_that_fails_is_not_a_rebase: test\n",
[], ["a_load_that_fails_is_not_a_rebase"]),
("qualifying it resolves it, and to the one it names",
"### ui::project::tests::a_load_that_fails_is_not_a_rebase\n",
"ui::host::tests::a_load_that_fails_is_not_a_rebase: test\n"
"ui::project::tests::a_load_that_fails_is_not_a_rebase: test\n",
[], []),
("a qualified heading naming a module that does not have it is an orphan",
"### ui::nowhere::tests::a_load_that_fails_is_not_a_rebase\n",
"ui::host::tests::a_load_that_fails_is_not_a_rebase: test\n",
["ui::nowhere::tests::a_load_that_fails_is_not_a_rebase"], []),
("a unique leaf still resolves unqualified",
"### a_lonely_test\n", "m::tests::a_lonely_test: test\n", [], []),
]
def _ambiguity_self_test():
failures = []
for name, md, rs, js, want_orphans, want_unstated in SELF_TEST_EVIDENCE:
for name, md, listing, want_o, want_a in SELF_TEST_AMBIGUITY:
claims = {}
for kind, cname, n in evidence_claims(md):
claims.setdefault((kind, cname), []).append(f"fixture:{n}")
rd, rc = rust_tests(rs)
if kind != "prose":
claims.setdefault((kind, cname), []).append(f"fixture:{n}")
defined = {"rust": cargo_test_names(listing), "js": set()}
orphans, _, ambiguous = evidence_verdict(claims, defined, {"rust": {}, "js": {}})
got_o = sorted(k[1] for k in orphans)
got_a = sorted(k[1] for k in ambiguous)
if got_o != sorted(want_o):
failures.append(f" ORPHANS: {name}: expected {sorted(want_o)}, saw {got_o}")
if got_a != sorted(want_a):
failures.append(f" AMBIGUOUS: {name}: expected {sorted(want_a)}, saw {got_a}")
return failures
def _listing_self_test():
failures = []
for name, listing, ignored, want in SELF_TEST_LISTING:
got = sorted(cargo_test_names(listing, ignored))
if got != sorted(want):
failures.append(f" LISTING: {name}: expected {sorted(want)}, saw {got}")
for name, md, want in SELF_TEST_HEADINGS:
got = [kind for kind, _, _ in evidence_claims(md)]
if got != want:
failures.append(f" HEADINGS: {name}: expected {want}, saw {got}")
return failures
def _evidence_self_test():
failures = _listing_self_test() + _ambiguity_self_test()
for name, md, rs, js, listing, want_orphans, want_unstated in SELF_TEST_EVIDENCE:
claims = {}
for kind, cname, n in evidence_claims(md):
if kind != "prose":
claims.setdefault((kind, cname), []).append(f"fixture:{n}")
rc = rust_citations(rs)
jd, jc = js_tests(js)
defined = {"rust": rd, "js": jd}
defined = {"rust": cargo_test_names(listing), "js": jd}
citing = {"rust": {k: ["fixture.rs"] for k in rc},
"js": {k: ["fixture.js"] for k in jc}}
orphans, unstated = evidence_verdict(claims, defined, citing)
orphans, unstated, ambiguous = evidence_verdict(claims, defined, citing)
got_o = sorted(k[1] for k in orphans)
got_u = sorted(k[1] for k in unstated)
if got_o != sorted(want_orphans):
failures.append(f" ORPHANS: {name}: expected {sorted(want_orphans)}, saw {got_o}")
if got_u != sorted(want_unstated):
failures.append(f" UNSTATED: {name}: expected {sorted(want_unstated)}, saw {got_u}")
if ambiguous:
failures.append(f" AMBIGUOUS: {name}: unexpected {sorted(k[1] for k in ambiguous)}")
return failures
@@ -784,7 +964,9 @@ def self_test():
return 1
print(f"checker self-test: {len(SELF_TEST_CASES)} violations caught, "
f"{len(SELF_TEST_CLEAN)} legitimate shapes allowed, "
f"{len(SELF_TEST_EVIDENCE)} evidence verdicts")
f"{len(SELF_TEST_EVIDENCE)} evidence verdicts, "
f"{len(SELF_TEST_LISTING) + len(SELF_TEST_HEADINGS)} listing readings, "
f"{len(SELF_TEST_AMBIGUITY)} name resolutions")
return 0
@@ -797,6 +979,12 @@ def main():
help="list doc comments containing a ``` fence and exit")
ap.add_argument("--check-evidence", action="store_true",
help="every context/test-evidence heading must name a test that exists")
ap.add_argument("--rust-tests", metavar="FILE",
help="a saved `cargo test --workspace -- --list`; the default runs it. "
"Requires --rust-tests-ignored: one file alone would skip the "
"ignored-test rule entirely")
ap.add_argument("--rust-tests-ignored", metavar="FILE",
help="the matching `cargo test --workspace -- --list --ignored`")
ap.add_argument("--self-test", action="store_true",
help="check the checker against the bypasses it has been fixed for")
args = ap.parse_args()
@@ -820,7 +1008,11 @@ def main():
return 2
if args.check_evidence:
return check_evidence(roots)
if bool(args.rust_tests) != bool(args.rust_tests_ignored):
print("error: --rust-tests and --rust-tests-ignored come together; one alone "
"silently drops the ignored-test rule", file=sys.stderr)
return 2
return check_evidence(roots, args.rust_tests, args.rust_tests_ignored)
if args.list_doctests:
for path in sorted(rust_files(roots)):
scripts/kb-merge.pl
+1 -1
View File
@@ -212,7 +212,7 @@ check_rule_replace(Funcs, replace, Old, New) :-
Fresh),
( Fresh == []
-> true
; format("kb-merge: REFUSED -- replace cannot find these rules, so it would ADD them beside the ones already there:~n", []),
; format("kb-merge: REFUSED -- replace cannot find these rules, so it would ADD them rather than replace anything:~n", []),
forall(member(T, Fresh), format(" ~q~n", [T])),
die('a rule is keyed on its whole clause, so an edited body is a NEW clause: kb rm the old one, then kb add')
).
scripts/kb-test.sh
+5 -3
View File
@@ -319,9 +319,11 @@ printf 'probe_fact(a).\nprobe_fact(b).\n' > "$work/facts.pl"
kb add "$work/facts.pl" >/dev/null 2>&1
printf 'probe_fact(X) :- doctrine(X,_).\n' > "$work/rule.pl"
# `add`, not `replace`: the rule is new, and a rule that is not already there
# is what case 15 makes replace refuse. This is also the discriminating door --
# under unification `conflicts` sees the non-ground key match both facts and
# refuses the add outright, so the reverted fix fails HERE.
# is what case 15 makes replace refuse. This case is NOT where same_key is
# discriminated -- a rule keys on its whole clause, so the `:-` term cannot
# unify with probe_fact(a) and `conflicts` never fires here. Reverting same_key
# to unification leaves this case passing and fails the wildcard rm and case 17
# instead; the site is guarded, by those (improvise-9d5k).
if kb add "$work/rule.pl" >/dev/null 2>&1; then
kept=0
grep -q '^probe_fact(a)\.$' "$work/kb.pl" && grep -q '^probe_fact(b)\.$' "$work/kb.pl" && kept=1
web/assemble.sh
+9 -27
View File
@@ -71,33 +71,15 @@ sedi "s|'improvise_worker_server_bg\.wasm'|'improvise_worker_server_bg.wasm?v=${
# One `grep` over all four files succeeds when ANY one of them matches, so
# three failed substitutions could still produce a "successful" assembly under
# `set -e` (found by Copilot on PR #24). These checks are the only guard
# against the generated glue changing shape, so each stamped reference is
# verified on its own and names itself when it is missing.
#
# `stamped` RECORDS rather than exits, and the gate below is what fails the
# build. Exiting on the first failure named one file and never reached the
# rest, so a shape change that broke two stamps cost two wasm-pack rebuilds to
# diagnose -- the slowest loop in this repo (improvise-x4wv). The gate is the
# whole guard: without it every failure prints and the assembly succeeds, which
# is the check-that-does-not-check this whole file is full of fixes for.
missing=""
stamped() {
if grep -q "$2" "$1"; then
grep -n "$2" "$1"
else
echo "assemble: $1 is missing the cache-bust stamp: $2" >&2
missing="${missing} $1"
fi
}
stamped "$app/app.js" "pkg/improvise_wasm_client.js?v=${token}"
stamped "$app/worker.js" "worker-pkg/improvise_worker_server.js?v=${token}"
stamped "$app/pkg/improvise_wasm_client.js" "improvise_wasm_client_bg.wasm?v=${token}"
stamped "$app/worker-pkg/improvise_worker_server.js" "improvise_worker_server_bg.wasm?v=${token}"
test -z "$missing" || {
echo "assemble: refusing to ship; unstamped:${missing}" >&2
exit 1
}
# `set -e` (found by Copilot on PR #24). Each stamped reference is therefore
# verified on its own, by scripts/assemble-stamp.sh -- which is a separate
# script so its refusals can be exercised without fabricating a web/pkg
# (improvise-w8ih). Its argument is FILE PATTERN, four times.
sh "$root/scripts/assemble-stamp.sh" \
"$app/app.js" "pkg/improvise_wasm_client.js?v=${token}" \
"$app/worker.js" "worker-pkg/improvise_worker_server.js?v=${token}" \
"$app/pkg/improvise_wasm_client.js" "improvise_wasm_client_bg.wasm?v=${token}" \
"$app/worker-pkg/improvise_worker_server.js" "improvise_worker_server_bg.wasm?v=${token}"
# Every relative import the two entry points make must have been copied. The
# cp list above is a second answer to "what ships", and a missing sibling is a
web/tests/bundle_freedom.test.js
+59
View File
@@ -0,0 +1,59 @@
// The `js` CI job must never need a built wasm bundle. See
// context/code/clients.md; the split is what keeps it seconds, not ~4m30s.
import { test } from "node:test";
import assert from "node:assert/strict";
import { readFileSync, readdirSync } from "node:fs";
import { fileURLToPath } from "node:url";
const dir = fileURLToPath(new URL(".", import.meta.url));
/// Built by concatenation so this file carries no literal of the thing it
/// forbids, and is therefore checked by itself rather than exempt from itself.
const FORBIDDEN = "p" + "kg";
/// Comments may name the directory — that is what the header above does. Only
/// what the code reaches for is a dependency, so strip comments first, exactly
/// as render_surface.test.js does.
const code = (name) =>
readFileSync(dir + name, "utf8")
.replace(/\/\*[\s\S]*?\*\//g, " ")
.replace(/(^|[^:"'`])\/\/.*$/gm, "$1");
test("no test in web/tests/ reaches for a built bundle", () => {
const siblings = readdirSync(dir).filter((f) => f.endsWith(".test.js"));
assert.ok(siblings.length > 1, `found ${siblings.length} test files — the glob moved`);
assert.ok(siblings.includes("bundle_freedom.test.js"), "this file must check itself");
const offenders = siblings.filter((f) => code(f).includes(FORBIDDEN));
assert.deepEqual(
offenders,
[],
`these belong in web/tests/bundle/, which the browser job runs: ${offenders}`,
);
});
test("no test in web/tests/ imports a module that reaches the bundle", () => {
// The check above is lexical and DIRECT-only: `import "../app.js"` needs the
// bundle transitively and names nothing forbidden. app.js and worker.js are
// the two entry points that load it; shell.js and serial.js do not.
const siblings = readdirSync(dir).filter((f) => f.endsWith(".test.js"));
const reaches = /^\s*import\b[^;]*["']\.\.\/(app|worker)\.js["']/m;
const offenders = siblings.filter((f) => reaches.test(code(f)));
assert.deepEqual(
offenders,
[],
`importing ../app.js or ../worker.js executes the bundle loader: ${offenders}`,
);
});
test("the subdirectory that MAY use a bundle is not the one being checked", () => {
// Reading it proves the exemption is a real directory rather than a
// convention nobody kept, and that this check is scoped to the flat listing.
const nested = readdirSync(dir + "bundle").filter((f) => f.endsWith(".test.js"));
assert.ok(nested.length > 0, "web/tests/bundle/ is empty — the split lost its other half");
assert.ok(
nested.some((f) => readFileSync(dir + "bundle/" + f, "utf8").includes(FORBIDDEN)),
"no bundle test reaches for a bundle, so this check has nothing to be scoped against",
);
});