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

Merge pull request #69 from fiddlerwoaroof/wave/tooling-gates

Browse Source 
fix(tooling): six checks that did not check (vz54, 5ray, lyyc, x4wv, 2f97, dj1l)
This commit is contained in:
Ed Langley
2026-09-20 17:11:04 -07:00
committed by GitHub
parent 4e48a6ecb4 cd094b6dfb
commit fe25952aed
10 changed files with 890 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/ci.yml
+10 -3
View File
@@ -218,10 +218,17 @@ jobs:
- uses: actions/setup-node@v4
with:
node-version: 22
# The glob is the SHELL's, not node's: `web/tests/bundle/` needs a built
# The glob is the SCRIPT's, not the shell's and not node's. Under bash an
# unmatched glob survives as a literal, node globs it, matches nothing and
# exits 0 having run no test -- so this step gated nothing the moment
# `web/tests/` moved (improvise-vz54). `web/tests/bundle/` needs a built
# wasm bundle and runs in the browser job instead.
- name: What the page decides between its boundaries
run: node --test web/tests/*.test.js
run: ./scripts/node-test.sh 'web/tests/*.test.js'
# The guard above is only worth its line if its refusals are themselves
# checked -- the step it replaces was green while asserting nothing.
- name: The node test runner refuses what it claims to
run: ./scripts/node-test-test.sh
kb:
name: Knowledge base consistency
@@ -349,7 +356,7 @@ jobs:
# runs here rather than in the `js` job, and before Playwright — a
# renamed export is cheaper to read here than as a blank page.
- name: The page's calls match the bundle it calls
run: node --test web/tests/bundle/*.test.js
run: ./scripts/node-test.sh 'web/tests/bundle/*.test.js'
- name: Run the harness
working-directory: harness
run: npx playwright test
context/test-evidence/clients.md
+25 -1
View File
@@ -76,6 +76,24 @@ client catch up.
---
## `improvise-worker-server/src/vfs.rs`
### an_entry_that_holds_no_document_is_seeded_over
The first browser run of the deployed page failed here, and no host test could
see it: `attach(create: true)` leaves a **zero-byte** entry behind, `documents`
listed it, and `open` therefore took the seed as already applied and read an
empty file. The precondition is asserted before the act, because a test that
only opens cannot tell "the seed landed" from "there was nothing in the way".
The rule it pins is the one gotcha 45 states for the index: what is indexed is
a document, and an entry nobody has written bytes to is not one.
This entry exists because the test cites this file and `--check-evidence` now
requires that (improvise-lyyc); it was written from the test, so read the test
first if the two ever disagree.
---
## `improvise-ws-server`
### distinct_stems_both_open
@@ -839,7 +857,7 @@ command. It failed on the first press past the end — client 0, host 4 — and
wire was changed to match the effect. Whether both should wrap instead is
improvise-trd3.
#### "? opens help, and its rows are the shared ones"
#### Retired — the help spec, `? opens help, and its rows are the shared ones`
improvise-ktv1's acceptance in one place: **a browser shows help.** `?` had
entered `AppMode::Help` since the keymap was shared, and nothing appeared —
@@ -858,6 +876,12 @@ always open passes any check that only ever opens it.
It runs against the **assembled** site like every other test here, so it
covers `render.rs`, the serialized model, `web/app.js` and the CSS together.
**The test no longer exists.** `36ade35` moved the shell suite to node and
deleted it with the rest of `harness/tests/widgets.spec.js`; nothing replaced
the assertion, and the heading went on promising it for a whole release --
which is the cost improvise-5ray names. The heading is demoted rather than
deleted so the injections above survive for whoever restores the coverage.
### the_shipped_demo_seeds_a_worker_whose_raw_cells_are_settable
improvise-ir7l renamed this from `..._whose_every_cell_is_settable` and
prolog-sessions/kb.pl
+34
View File
@@ -116,6 +116,7 @@ invariant(dataset_columns_are_not_header_aligned,'gp_tree::data::preprocess cont
invariant(no_input_line_is_dropped,'The Markdown renderer covers exactly the subset the corpus uses; anything it does not recognise falls through to paragraph text. The failure mode is an ugly paragraph, never a missing one.').
invariant(a_table_cell_boundary_is_an_unescaped_pipe_outside_a_code_span,'splitRow walks the row rather than split(\"|\"), because this corpus documents a file format whose own quoting syntax is |Time Period| and a cell containing one would be shredded into extra columns -- a row that still parses, so no column count could detect it. Latent when fixed, which is the point: the trigger is one ordinary sentence away in a document whose subject is pipe quoting.').
invariant(code_spans_stand_aside_for_emphasis,'Inline rendering replaces code spans with placeholders before matching emphasis, because this corpus routinely opens ** on one side of a code span and closes it on the other; matching per segment silently drops that bold with no visible symptom. Strong is matched before emphasis with a body that may contain a single * pair, because [^*]+ cannot match **a *b* c** and the leftover * then pairs across the bold -- which turned one sentence of gotcha 33 into three italic paragraphs.').
invariant(key_matching_is_variant_equality_never_unification,'scripts/kb-merge.pl compares clause keys with same_key/2 (copy_term + numbervars), never by unifying them. Unification matched too much -- a non-ground key probe(X) matched probe(a) and deleted it -- and MUTATED the incoming clause, binding X to a, so the clause written was not the clause supplied; the round-trip verify cannot see that, since it compares the file against an intended list already corrupted by the same unification (improvise-dj1l). A RULE keys on the whole clause, because one head shape with different bodies is ordinary Prolog. check_total/2 asserts the arithmetic the tool already PRINTED beside the delta size.').
doctrine(bug_first,'write a failing test demonstrating the bug before fixing it').
doctrine(unit_over_e2e,'prefer small unit tests over integration/e2e').
doctrine(suite_speed,'Two numbers, because the ambiguity between them is what let this drift: NO SINGLE TEST BINARY may exceed 2 seconds, and the WORKSPACE WALL CLOCK is the sum of them. Measured at 22d43b2: 1309 tests over 33 binaries, ~2.2s wall, longest single binary 1.42s -- so the workspace is already past 2s and the rule that bites is the per-binary one. Re-MEASURE rather than re-assert: cargo test --workspace prints a \"finished in\" per binary. Excludes compilation, and excludes harness/ (Playwright, minutes, its own CI job).').
@@ -218,6 +219,9 @@ doctrine(a_write_must_not_be_able_to_disarm_its_own_gate,'scripts/kb validates a
doctrine(a_read_validate_write_sequence_is_a_lost_update,'scripts/kb read the knowledge base, merged, validated and moved the result into place, with nothing between the read and the move. Two agents doing that concurrently -- which is ordinary here, waves run agents in parallel -- both read the same file, both validate, and the later move silently discards the earlier addition. Exactly the class this tool was built to remove, reintroduced one level up in the tool itself. Closed by stamping the file content at read time and refusing to commit if it changed, plus an mkdir lock (flock is absent on macOS) so the collision is rare rather than merely loud. The stamp is the part that matters: a lock prevents the common case, a compare-and-swap is what makes an uncommon one impossible to lose silently.').
doctrine(design_against_the_model_before_implementing,'AGENTS.md has always said Prolog is a design tool for reasoning about tickets, and it was the least-used line in the file: kb.pl was written retrospectively. \`scripts/kb check --with <design.pl>\` closes that -- proposed facts are merged into a throwaway candidate and kb_consistent runs over the union, so a design can be refused before code exists. The model already DERIVES rather than merely records: illegal_dependency, may_use, depends_transitively, portable, snapshot_safe, view_state_admits, record_obligation, needs_runtime_check, and eleven kb_error classes including layering_inverted and field_misplaced_in_view_state. Demonstrated on three proposals in one sitting: one added edge (improvise_core depending on improvise_command) produced SIX derived layering violations through the transitive closure, which no author would have enumerated; a proposal to declare decision/3 functional was refused with conflicting_values naming improvise-907, correctly, because one ticket carries several decisions; and a mis-ordered layer/2 argument was named as layer_names_unknown_crate rather than silently accepted. What makes this worth doing rather than decorative is the same rule as everywhere else -- the model must be able to REFUSE, and you must watch it refuse. A design language nothing can reject is documentation with parentheses.').
doctrine(a_key_rule_must_ask_whether_the_relation_is_multi_valued,'scripts/kb keyed every clause on the functor plus all arguments but the last, which is right for doctrine(Name,Text) and wrong for any RELATION: crate_depends_on(A,B) then keyed on A, so a crate depending on two crates read as one key twice and was unaddable. Found by trying to model a proposed crate, not by any test -- the tool had only ever been pointed at single-valued facts. The fix cannot read the KB own functional/1, because that check keys an arity-3 predicate on its FIRST argument, which is right for crate/3 and wrong for decision/3 where one ticket holds several decisions under different names. So key_arity/2 is a hand-maintained table and anything unlisted keys on the WHOLE head, which is the safe direction: an unlisted relation accepts many clauses and only an exact repeat is a duplicate.').
doctrine(a_local_shell_is_not_the_ci_shell,'zsh ABORTS on an unmatched glob and bash leaves it as a literal, so a CI step written and probed in zsh can fail in a way no developer shell can reproduce. node --test web/tests/*.test.js under bash therefore globbed the literal itself, matched nothing, reported 0 tests and exited 0 -- both node steps gated nothing the moment their directory moved (improvise-vz54). Local shell vs CI shell is a fault-injection axis in its own right: probe a CI line under the shell the workflow names, not the one you are typing in. The fix moves the glob into scripts/node-test.sh so the expansion is the script\'s and can be refused.').
doctrine(a_convention_that_keeps_a_checker_quiet_is_the_one_that_blinds_it,'context/test-evidence used #### for browser tests precisely BECAUSE --check-evidence resolved names against Rust fn names only, so a ### heading naming a Playwright test would be reported as an orphan. The workaround made every browser entry invisible, and a deleted test left its heading promising coverage for a whole release (improvise-5ray). Cure: collect ### and #### alike and decide by SHAPE -- a bare identifier is a Rust test, a quoted string is a JS test, anything else is prose naming nothing. When a checker forces a convention on the thing it checks, ask what that convention hides.').
doctrine(a_refusal_is_evidence_only_when_it_is_the_one_you_named,'Two kb-test cases passed against a tree with the fix they guarded reverted, because a DIFFERENT guard refused the same delta and the assertion was only \"fails, and the kb is unchanged\" (review of PR #69). A tool with several guards will usually refuse a broken delta somehow, so \"it was refused\" is nearly free and says nothing about the fix under test. Assert the REFUSAL MESSAGE, or construct a case only the fix can catch -- and prove it by reverting that one fix and watching the case fail. The same trap in reverse: a case asserting an OUTCOME (the add lands and every sibling survives) can discriminate where a message cannot, because what a wrong key destroys is the outcome.').
work_done('improvise-8tk',bug_fix,'ZZ/:wq quit even when the save failed, silently discarding the model. Save now aborts the effect batch on failure; save_to_current_path/1 returns Result<String,String>; three tests in ui/effect.rs.').
work_done('improvise-907',bug_fix,'Import wizard step machine redesign: validation_error() gate at ReviewProposals (no-category and over-limit), history-stack previous() bound to Left on every step, and Done made a real confirmation step holding the built workbook. Superseded improvise-gfq, improvise-o1c, improvise-c6j.').
work_done('improvise-6sq',wont_do,'Merging EditOrDrill and EnterEditAtCursorCmd into one EnterEdit{mode} is not safe: they differ in WHEN the cell value is read, not in a mode parameter. Added ui/app.rs::tests::i_and_o_diverge_on_an_empty_records_grid as executable evidence.').
@@ -241,6 +245,12 @@ work_done('backlog-groom-2026-09-19',groom,'ran the five 2026-08-30 checks over
work_done('improvise-0yq8',refactor,'ClientState -> improvise-protocol/src/client.rs and keys.rs::{resolve,step,KeyOutcome} -> improvise-protocol/src/resolve.rs, with socket.rs::{typed,typed_cell_command} following step. BrowserKey stays in the wasm client: it is the browser half of the two things a front end owns. render_model became a free function, ClientState being foreign. 1311 tests before and after -- no behaviour change.').
work_done('improvise-e24v',feature,'Fifteen command names resolve client-side: cycle-panel-focus, the category panel five, the view panel three, both tile-axis names, both help-page steps, commit-buffer in three kinds, clear-cell and paste. New lockstep harness applies what the client composed to a SECOND authoritative host and compares documents as .improv text (Debug prints the symbol table HashMaps, so it differs per process). Four injected faults, each failing a named test; one needed the fixture widened first. Found and fixed a wire/effect divergence in Command::HelpPage.').
work_done('improvise-x7sw',refactor,'Closed with improvise-ktv1 and improvise-k099 as one question. improvise-protocol/src/help.rs is the one home for a UI string; AppMode::label/hint_text and Keymap::binding_hints deleted, along with every_hint_key_resolves_in_its_mode and its 60-line tokenizer. The TUI help overlay kept its ratatui styles and lost ~310 lines of content; the browser gained one (RenderModel::help, empty means closed), asserted by a new Chromium test in harness/tests/shell.spec.js proved discriminating by two injected faults.').
work_done('improvise-vz54',bug_fix,'node --test with an unmatched glob ran zero tests and exited 0 on both CI node steps. scripts/node-test.sh now owns the expansion and refuses an empty match; it also refuses a matched file that declares no test, checked LEXICALLY because node reports such a file as one passing test named after the file, so its own count reads as the FILE count. scripts/node-test-test.sh proves each refusal by injection and runs in the js job.').
work_done('improvise-5ray',bug_fix,'--check-evidence collected only ### headings and resolved names against Rust fn names, so every browser entry was invisible. Now collects ###/#### alike, classifies by shape, and resolves JS names out of harness/tests and web/tests. It immediately found a real orphan: 36ade35 deleted the Playwright test that asserted the browser draws help and left its heading behind (improvise-v5pc).').
work_done('improvise-lyyc',bug_fix,'--check-evidence gained its second direction: a test whose doc comment names context/test-evidence must have an entry there. It immediately found one, an_entry_that_holds_no_document_is_seeded_over, which had cited the file with no entry since it was written. Ten evidence cases were added to the checker\'s own --self-test, which had none.').
work_done('improvise-x4wv',bug_fix,'web/assemble.sh\'s stamped guard exited on the FIRST missing cache-bust stamp, so breaking two substitutions named one and cost a wasm-pack rebuild to find the next. stamped now records and a gate after the four calls fails the build. Both the fix and the gate\'s own absence were proved by injection; the comment says the gate is the whole guard.').
work_done('improvise-2f97',feature,'kb_error(dangling_citation/3): a name cited inside another clause\'s text must resolve as a doctrine/2, invariant/2 or decision/3. The token rule is narrow on purpose -- only a name-shaped token (lowercase, digits, underscores, at least one underscore) directly inside doctrine(, invariant( or decision( -- so prose cannot false-positive. Retiring doctrine(an_advisory_gate_is_not_a_gate) is now refused by name rather than silently dangling its citers. kb_text/2 is hand-maintained, like gate_predicate/1.').
work_done('improvise-dj1l',bug_fix,'scripts/kb-merge.pl matched clause keys by unification, which lost and corrupted clauses: two variant-headed rules in one delta went two in, one out (536 before, 2 in delta, 537 after), and a non-ground rule over a predicate holding facts deleted one AND was written with its variable bound. See invariant(key_matching_is_variant_equality_never_unification). Five regression cases added to scripts/kb-test.sh, including one that injects a dropping fold to prove check_total fires; scripts/kb gained a KB_MERGE seam so a broken merge can be injected.').
work_followup('improvise-8tk','improvise-lae','no-file-path UX: prompt for a path rather than only erroring. ExportPrompt is CSV-only, so a new SavePrompt TextEntry mode committing to SaveAs is the likely shape.').
work_followup('improvise-6sq','improvise-ct5','i on an empty records grid reports \'cannot drill\', leaking a pivot-mode concept into records mode').
work_followup('improvise-h0b','improvise-a2x','ViewState field types (AppMode, MinibufferConfig, ViewFrame, KeyCode, KeyModifiers) still derive no Serialize, and improvise-command depends only on serde_json. Deferred: nothing serializes ViewState until improvise-cqi builds a Snapshot, so the derives would have no consumer.').
@@ -473,6 +483,7 @@ kb_error(followup_from_unrecorded_work(A)):-work_followup(A,B,C), \+work_done(A,
kb_error(unblocked_by_unrecorded_work(A)):-work_unblocked(A,B,C), \+work_done(A,D,E).
kb_error(field_misplaced_in_view_state(A)):-session_field(A,view_state,B), \+view_state_admits(A).
kb_error(field_evicted_without_cause(A)):-session_field(A,app,B),view_state_admits(A).
kb_error(dangling_citation(A,B,C)):-kb_text(A,D),cited_name(D,B,C), \+citation_resolves(B,C).
kb_consistent:- \+kb_error(A).
kb_report:-setof(A,kb_error(A),B)->length(B,C),format([k,b,:,' ',~,w,' ',p,r,o,b,l,e,m,'(',s,')',~,n],[C]),forall(member(A,B),format([' ',' ',-,' ',~,w,~,n],[A]));write('kb: consistent'),nl.
kb_external_check('grep -oE \"^location\\([a-z_]+,\'[^\']+\',\'[^\']+\'\\)\" prolog-sessions/kb.pl # then confirm each file/symbol still exists').
@@ -534,3 +545,26 @@ opfs_sync(flush).
opfs_sync(close).
browser_fact(sync_access_handle_safari,'Safari 16.4+ supports createSyncAccessHandle, and all current browsers implement the handle methods synchronously despite an early spec that wrongly called them async. Checked 2026-08-31, so the IndexedDB fallback improvise-i34 called for is not needed.').
browser_fact(sync_access_handle_worker_only,'available only in dedicated Web Workers, and exclusive per file entry: a second handle on the same file fails until the first is closed. Hence attach closing what it replaces, and detach existing at all.').
citation_marker('doctrine(',doctrine).
citation_marker('invariant(',invariant).
citation_marker('decision(',decision).
kb_text(doctrine(A),B):-doctrine(A,B).
kb_text(invariant(A),B):-invariant(A,B).
kb_text(decision(A,B),C):-decision(A,B,C).
kb_text(work_done(A,B),C):-work_done(A,B,C).
kb_text(work_followup(A,B),C):-work_followup(A,B,C).
kb_text(kb_issue_found(A),B):-kb_issue_found(A,B).
lower_or_digit(A):-char_code(A,B),B>=97,B=<122.
lower_or_digit(A):-char_code(A,B),B>=48,B=<57.
name_char(A):-lower_or_digit(A),!.
name_char('_').
name_shaped(A):-atom_chars(A,B),B=[C|D],lower_or_digit(C),forall(member(E,B),name_char(E)),memberchk('_',D).
citation_token(A,B):-(sub_atom(A,C,1,D,')')->true;C=999),(sub_atom(A,E,1,F,',')->true;E=999),G is min(C,E),G>0,G<120,sub_atom(A,0,G,H,B).
cited_name(A,B,C):-citation_marker(D,B),sub_atom(A,E,F,G,D),sub_atom(A,H,G,0,I),citation_token(I,C),name_shaped(C).
cited_name(A,decision,B):-sub_atom(A,C,D,E,'decision('),sub_atom(A,F,E,0,G),decision_name(G,B),name_shaped(B).
citation_resolves(doctrine,A):-doctrine(A,B).
citation_resolves(invariant,A):-invariant(A,B).
citation_resolves(decision,A):-decision(B,A,C).
citation_body(A,B):-sub_atom(A,C,1,D,')'),!,C>0,C<200,sub_atom(A,0,C,E,B).
trim_left(A,B):-sub_atom(A,0,1,C,' ')->sub_atom(A,1,D,0,E),trim_left(E,B);B=A.
decision_name(A,B):-citation_body(A,C),sub_atom(C,D,1,E,','),!,F is D+1,sub_atom(C,F,G,0,H),trim_left(H,B).
scripts/check-comment-runs.py
+265 -55
View File
@@ -249,76 +249,192 @@ def _is_test_attribute(line):
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_]*$")
_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
# every OTHER test in its file, so a heading satisfied by one is a heading
# satisfied by a file that stopped running (review, PR #69).
_JS_TEST = re.compile(
r"""(?:^|[^\w.])(?:test|it)\s*\(\s*(['"`])(.+?)\1"""
)
def evidence_claims(text):
"""Yield (kind, name, lineno) for every heading in an evidence 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.
"""
for n, raw in enumerate(text.split("\n"), 1):
s = raw.strip()
if not (s.startswith("### ") or s.startswith("#### ")):
continue
body = s.lstrip("#").strip().strip("`").strip()
m = _QUOTED_NAME.match(body)
if m:
yield "js", m.group(1), n
elif _RUST_NAME.match(body):
yield "rust", body, n
def rust_tests(text):
"""(tests defined here, tests whose doc comment cites the evidence dir).
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).
"""
defined, citing = set(), set()
annotated = False
doc = []
# A `proptest!` block accepts its bodies as tests -- but only INSIDE the
# block. A bare flag was never cleared, so the first proptest! in a file
# made every later fn, helpers included, count as a test: a silent
# success in the gate itself (found by review on PR #40). Track the
# block's braces and leave when they balance.
proptest_depth = None
for raw in text.split("\n"):
s = raw.strip()
if proptest_depth is not None:
proptest_depth += raw.count("{") - raw.count("}")
if proptest_depth <= 0:
proptest_depth = None
elif s.startswith("proptest!"):
proptest_depth = raw.count("{") - raw.count("}")
if proptest_depth <= 0:
proptest_depth = None if "{" in raw else 0
in_proptest = proptest_depth is not None
if s.startswith("//"):
doc.append(s)
continue
# ANCHOR the attribute. `"test" in s` also matched `#[cfg(test)]`,
# so every cfg-gated fixture cleared a gate written to reject
# exactly those -- the third instance of the unanchored-substring
# class in this campaign, after the bd audit's exclusion list.
if s.startswith("#[") and _is_test_attribute(s):
annotated = True
continue
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
doc = []
elif s and not s.startswith("#["):
annotated = False
doc = []
return defined, citing
def js_tests(text):
"""(tests defined here, tests whose preceding comment cites the evidence dir).
The JS half exists because `defined` held only Rust `fn` names, so a
`####` naming a Playwright or node test could never have resolved even if
the heading had been collected -- the convention that kept the checker
quiet was the same one that made it blind (improvise-5ray).
"""
defined, citing = set(), set()
doc = []
for raw in text.split("\n"):
s = raw.strip()
m = _JS_TEST.search(raw)
if m:
defined.add(m.group(2))
if any(EVIDENCE_CITE in d for d in doc):
citing.add(m.group(2))
doc = []
continue
if s.startswith("//") or s.startswith("*") or s.startswith("/*"):
doc.append(s)
elif s:
doc = []
return defined, citing
def js_files(roots):
for root in roots:
for dirpath, dirnames, filenames in os.walk(root):
dirnames[:] = [d for d in dirnames if d != "node_modules"]
for f in filenames:
if f.endswith(".spec.js") or f.endswith(".test.js"):
yield os.path.join(dirpath, f)
def evidence_verdict(claims, defined, citing):
"""(orphans, unstated) -- the two directions, 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.
"""
orphans = {k: v for k, v in claims.items() if k[1] not in defined[k[0]]}
unstated = {}
for kind, names in citing.items():
for name in names:
if (kind, name) not in claims:
unstated.setdefault((kind, name), []).extend(sorted(names[name]))
return orphans, unstated
def check_evidence(roots):
"""Every `### name` in context/test-evidence/ must name a test that exists.
One direction only, deliberately: a test may have no evidence (most do
not), but evidence naming a test nobody can find is an orphan -- which is
the cost design-principles names for splitting the narrative from the test.
"""
"""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
claimed = {}
claims = {}
for dirpath, _, filenames in os.walk(EVIDENCE_DIR):
for f in sorted(filenames):
if not f.endswith(".md"):
continue
path = os.path.join(dirpath, f)
for n, raw in enumerate(open(path, encoding="utf8", errors="replace"), 1):
s = raw.strip()
if s.startswith("### "):
claimed.setdefault(s[4:].strip().strip("`"), []).append(f"{path}:{n}")
text = open(path, encoding="utf8", errors="replace").read()
for kind, name, n in evidence_claims(text):
claims.setdefault((kind, name), []).append(f"{path}:{n}")
# 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).
defined = set()
defined = {"rust": set(), "js": set()}
citing = {"rust": {}, "js": {}}
for path in rust_files(roots):
lines = open(path, encoding="utf8", errors="replace").read().split("\n")
annotated = False
# A `proptest!` block accepts its bodies as tests -- but only INSIDE the
# block. A bare flag was never cleared, so the first proptest! in a file
# made every later fn, helpers included, count as a test: a silent
# success in the gate itself (found by review on PR #40). Track the
# block's braces and leave when they balance.
proptest_depth = None
for raw in lines:
s = raw.strip()
if proptest_depth is not None:
proptest_depth += raw.count("{") - raw.count("}")
if proptest_depth <= 0:
proptest_depth = None
elif s.startswith("proptest!"):
proptest_depth = raw.count("{") - raw.count("}")
if proptest_depth <= 0:
proptest_depth = None if "{" in raw else 0
in_proptest = proptest_depth is not None
# ANCHOR the attribute. `"test" in s` also matched `#[cfg(test)]`,
# so every cfg-gated fixture cleared a gate written to reject
# exactly those -- the third instance of the unanchored-substring
# class in this campaign, after the bd audit's exclusion list.
if s.startswith("#[") and _is_test_attribute(s):
annotated = True
continue
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)
annotated = False
elif s and not s.startswith("#[") and not s.startswith("//"):
annotated = False
d, c = rust_tests(open(path, encoding="utf8", errors="replace").read())
defined["rust"] |= d
for name in c:
citing["rust"].setdefault(name, []).append(path)
for path in js_files(JS_ROOTS):
d, c = js_tests(open(path, encoding="utf8", errors="replace").read())
defined["js"] |= d
for name in c:
citing["js"].setdefault(name, []).append(path)
orphans = {k: v for k, v in claimed.items() if k not in defined}
if not orphans:
print(f"test-evidence: {len(claimed)} entries, all name a test that exists")
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")
return 0
print(f"{len(orphans)} test-evidence entries name no test:\n", file=sys.stderr)
for name, where in sorted(orphans.items()):
print(f" {name} ({', '.join(where)})", file=sys.stderr)
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 unstated:
print(f"\n{len(unstated)} tests cite {EVIDENCE_DIR} and have no entry there:\n",
file=sys.stderr)
for (kind, name), where in sorted(unstated.items()):
print(f" [{kind}] {name} ({', '.join(where)})", file=sys.stderr)
return 1
@@ -553,6 +669,98 @@ fn g() {}
]
# (name, evidence markdown, rust source, js source, 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
# checker was the shape it could not see. Each case below is an injection.
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 #### 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', [], []),
("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"], []),
("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 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"]),
("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",
"", [], []),
("a NON-test fn citing the file is under no obligation",
"", "/// 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",
"", [], []),
("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 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"], []),
("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 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"]),
]
def _evidence_self_test():
failures = []
for name, md, rs, js, want_orphans, want_unstated in SELF_TEST_EVIDENCE:
claims = {}
for kind, cname, n in evidence_claims(md):
claims.setdefault((kind, cname), []).append(f"fixture:{n}")
rd, rc = rust_tests(rs)
jd, jc = js_tests(js)
defined = {"rust": rd, "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)
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}")
return failures
def self_test():
"""Check the checker against every bypass it has already been fixed for.
@@ -569,12 +777,14 @@ def self_test():
runs = [n for n in comment_runs(src.split("\n")) if n[1] > LIMIT]
if runs:
failures.append(f" FALSE POSITIVE: {name}: flagged {runs}")
failures += _evidence_self_test()
if failures:
print("checker self-test FAILED:", file=sys.stderr)
print("\n".join(failures), file=sys.stderr)
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_CLEAN)} legitimate shapes allowed, "
f"{len(SELF_TEST_EVIDENCE)} evidence verdicts")
return 0
scripts/kb
+3 -1
View File
@@ -6,7 +6,9 @@ set -euo pipefail
here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
repo="$(cd "$here/.." && pwd)"
KB="${KB_FILE:-$repo/prolog-sessions/kb.pl}"
MERGE="$here/kb-merge.pl"
# Test seam: kb-test.sh injects a pre-fix kb-merge.pl to prove the arithmetic
# check catches a merge that drops a clause.
MERGE="${KB_MERGE:-$here/kb-merge.pl}"
# trealla comes from the dev shell, which direnv puts on PATH. Reaching it
# through `nix shell` per invocation cost more than the work it did: this
scripts/kb-merge.pl
+70 -5
View File
@@ -33,6 +33,9 @@ run(remove, KbFile, DeltaFile, OutFile) :-
halt(2)
),
foldl_remove(Funcs, Doomed, Old, Kept),
length(Old, NOld), length(Doomed, NDoomed),
ExpectK is NOld - NDoomed,
check_total(Kept, ExpectK),
write_terms(OutFile, Kept),
verify(OutFile, Kept),
length(Old, NO), length(Kept, NK),
@@ -46,7 +49,10 @@ run(Mode, KbFile, DeltaFile, OutFile) :-
check_self_consistent(Funcs, New),
conflicts(Funcs, Old, New, Clashes),
gate(Mode, Clashes),
check_rule_replace(Funcs, Mode, Old, New),
foldl_merge(Funcs, New, Mode, Old, Merged),
expected_total(Funcs, Mode, Old, New, Expect),
check_total(Merged, Expect),
write_terms(OutFile, Merged),
verify(OutFile, Merged),
length(Old, NO), length(New, NN), length(Merged, NM),
@@ -106,7 +112,15 @@ clause_head(H, H).
% predicate on its FIRST argument, which is right for crate/3 and wrong for
% decision/3, where one ticket carries several decisions under different names.
% Hand-maintained, like gate_predicate/1, but the default is the safe direction.
clause_key(Funcs, (H :- _), K) :- !, head_key(Funcs, H, K).
% A RULE keys on the whole clause, because two clauses with the same head shape
% and different bodies is ordinary Prolog -- keying a rule on its head alone
% made the second refuse as a duplicate, or worse, silently replace the first.
clause_key(Funcs, (H :- B), K) :-
!,
( clause_pred((H :- B), P), key_arity(P, _)
-> head_key(Funcs, H, K)
; K = (H :- B)
).
clause_key(Funcs, H, K) :- head_key(Funcs, H, K).
key_arity(doctrine/2, 1).
@@ -137,7 +151,7 @@ clause_pred(H, F/A) :- functor(H, F, A).
% first -- add discarding part of its own payload, visible only as a delta count
% that did not match the clauses added.
check_self_consistent(Funcs, New) :-
findall(K, ( member(T, New), clause_key(Funcs, T, K) ), Ks),
findall(C, ( member(T, New), clause_key(Funcs, T, K), canon_key(K, C) ), Ks),
msort(Ks, Sorted),
adjacent_dups(Sorted, Dups0),
sort(Dups0, Dups),
@@ -157,12 +171,53 @@ adjacent_dups([A, B|T], Out) :-
),
adjacent_dups([B|T], Rest).
% The tool printed "N in delta" beside a total that did not account for N,
% twice, and nothing compared them. This checks the arithmetic the report
% already shows, which catches a dropped clause without knowing about keys.
check_total(Result, Expect) :-
length(Result, Got),
( Got =:= Expect
-> true
; format("kb-merge: REFUSED -- would write ~w clauses, expected ~w; a clause was dropped~n",
[Got, Expect]),
halt(2)
).
expected_total(_, add, Old, New, Expect) :-
length(Old, NO), length(New, NN), Expect is NO + NN.
expected_total(Funcs, replace, Old, New, Expect) :-
length(Old, NO),
findall(x, ( member(T, New), clause_key(Funcs, T, K),
\+ ( member(O, Old), clause_key(Funcs, O, K2), same_key(K, K2) ) ), Fresh),
length(Fresh, NF),
Expect is NO + NF.
conflicts(Funcs, Old, New, Clashes) :-
findall(K,
( member(T, New), clause_key(Funcs, T, K),
member(O, Old), clause_key(Funcs, O, K) ),
member(O, Old), clause_key(Funcs, O, K2), same_key(K, K2) ),
Clashes).
% A rule keys on its whole clause, so an EDITED rule matches nothing and
% `replace` silently becomes `add` -- both arms then live, and the stale one
% goes on firing beside the new one. check_total cannot see it: NO + NF is
% correct arithmetic for an add. So replace refuses a rule it cannot find and
% names the route, which is rm then add (improvise-dj1l, review of PR #69).
check_rule_replace(Funcs, replace, Old, New) :-
!,
findall(T,
( member(T, New), T = (_ :- _),
clause_key(Funcs, T, K),
\+ ( member(O, Old), clause_key(Funcs, O, K2), same_key(K, K2) ) ),
Fresh),
( Fresh == []
-> true
; format("kb-merge: REFUSED -- replace cannot find these rules, so it would ADD them beside the ones already there:~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')
).
check_rule_replace(_, _, _, _).
gate(add, []) :- !.
gate(add, Clashes) :-
!,
@@ -180,14 +235,24 @@ foldl_merge(Funcs, [T|Ts], Mode, Acc0, Acc) :-
insert_after_last(Acc1, P, T, Acc2),
foldl_merge(Funcs, Ts, Mode, Acc2, Acc).
has_key(Funcs, K, T) :- clause_key(Funcs, T, K).
% Key matching is VARIANT equality, never unification. Unifying a key against a
% clause both matched too much -- a non-ground key probe(X) matched probe(a),
% destroying it -- and MUTATED the incoming clause, because the unification
% bound X to a, so the rule written was not the rule supplied. The round-trip
% check cannot see that: it compares the file against an intended list already
% corrupted by the same unification.
has_key(Funcs, K, T) :- clause_key(Funcs, T, K2), same_key(K, K2).
same_key(K1, K2) :- canon_key(K1, C1), canon_key(K2, C2), C1 == C2.
canon_key(K, C) :- copy_term(K, C), numbervars(C, 0, _).
% A remove that matches nothing is refused rather than silently doing nothing:
% a typo'd key would otherwise report success and leave the clause in place.
absent(Funcs, Old, Doomed, Missing) :-
findall(K,
( member(T, Doomed), clause_key(Funcs, T, K),
\+ ( member(O, Old), clause_key(Funcs, O, K) ) ),
\+ ( member(O, Old), clause_key(Funcs, O, K2), same_key(K, K2) ) ),
Missing).
foldl_remove(_, [], Acc, Acc).
scripts/kb-test.sh
+222 -1
View File
@@ -18,6 +18,11 @@ bad() { printf ' FAIL %s\n' "$1"; fail=$((fail+1)); }
fresh() { cp "$KB" "$work/kb.pl"; }
kb() { KB_FILE="$work/kb.pl" "$here/kb" "$@"; }
unchanged() { diff -q "$KB" "$work/kb.pl" >/dev/null; }
# A refusal is only evidence when it is the refusal you NAMED: two of these
# cases passed against a tree with the fix reverted, because a DIFFERENT guard
# caught the same delta and the assertion was only "fails and kb unchanged"
# (review of PR #69). said() asserts which one fired.
said() { grep -qF "$2" "$1"; }
echo "kb-test: the refusals"
@@ -88,8 +93,13 @@ fi
# 4c. rm removes by key, and a key that is not there is REFUSED rather than
# silently doing nothing -- a typo would otherwise report success.
# The clause removed is one this case ADDS. It used to name bug_first, which
# the citation check (improvise-2f97) now refuses to retire -- correctly, since
# before_fixing/2 calls it and kb_issue_found names it in prose.
fresh
printf "doctrine(bug_first,_).\n" > "$work/drop.pl"
printf "doctrine(a_probe_doctrine_to_remove,'a clause nothing else names').\n" > "$work/probe.pl"
kb add "$work/probe.pl" >/dev/null 2>&1
printf "doctrine(a_probe_doctrine_to_remove,_).\n" > "$work/drop.pl"
before="$(kb count | grep '^doctrine/2=' | cut -d= -f2)"
if kb rm "$work/drop.pl" >/dev/null 2>&1; then
after="$(kb count | grep '^doctrine/2=' | cut -d= -f2)"
@@ -277,5 +287,216 @@ else
bad "fmt changed the counts"
fi
# 9. A delta holding two clauses whose heads are VARIANTS -- same shape, fresh
# variables -- is two clauses. check_self_consistent compared keys with ==
# over findall results, which never holds for fresh variables, so the delta
# passed; the merge then matched by UNIFICATION and the second removed the
# first. Two in, one out, reported as a success (improvise-dj1l).
cat > "$work/rules.pl" <<'EOF'
probe_rule(X) :- doctrine(X,_).
probe_rule(Y) :- invariant(Y,_).
EOF
fresh
b="$(kb count | grep '^total=' | cut -d= -f2)"
if kb add "$work/rules.pl" >/dev/null 2>&1; then
a="$(kb count | grep '^total=' | cut -d= -f2)"
if [ "$((a - b))" -eq 2 ]; then
ok "two variant-headed rules in one delta both land ($b -> $a)"
else
bad "two clauses in the delta, $((a - b)) landed"
fi
else
bad "add refused two variant-headed rules"
fi
# 10. A non-ground key must not match a ground clause. `probe_fact(X) :- ...`
# unified with `probe_fact(a)`, which DELETED the fact and, worse, bound X
# to a, so the clause written was not the clause supplied. The round-trip
# check cannot see that: it compares the file against an intended list
# already corrupted by the same unification.
fresh
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.
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
if [ "$kept" = 1 ]; then
ok "a non-ground rule does not delete the ground clauses it would unify with"
else
bad "a non-ground rule deleted a ground clause"
fi
if grep -q '^probe_fact(a):-' "$work/kb.pl"; then
bad "the clause written is not the clause supplied: its variable was bound"
elif grep -qE '^probe_fact\([A-Z][A-Za-z0-9_]*\):-doctrine\(' "$work/kb.pl"; then
ok "the rule is written as supplied, with its variable still a variable"
else
bad "the rule was not written at all"
fi
else
bad "add refused a rule over a predicate holding facts: a variable key matched them"
fi
# 11. The duplicate gate must still fire once keys are compared as variants:
# two clauses that really are the same key are still two clauses for one key.
fresh
printf 'probe_rule(X) :- doctrine(X,_).\nprobe_rule(Z) :- doctrine(Z,_).\n' > "$work/same.pl"
if kb add "$work/same.pl" >"$work/out" 2>&1; then
bad "add accepted the same clause twice in one delta"
elif ! unchanged; then
bad "add refused a duplicate but MODIFIED the kb"
elif said "$work/out" "names one key twice"; then
ok "add still refuses one key named twice, compared as variants"
else
bad "the delta was refused, but not BY THE DUPLICATE GATE (canon_key is unreached)"
fi
# 12. The arithmetic check, proved by injecting the fault it exists for: a
# merge that silently drops the last clause of its delta. The count was
# PRINTED beside the delta size all along and nothing compared them, which
# is how the real loss reported itself as "2 in delta, 537 after". The
# injection is aimed at the fold rather than at the keys, because the check
# must catch a dropped clause whatever dropped it.
sed 's/^foldl_merge(_, \[\], _, Acc, Acc)\./foldl_merge(_, [_], _, Acc, Acc)./' \
"$here/kb-merge.pl" > "$work/merge-dropping.pl"
if ! grep -q '^foldl_merge(_, \[_\], _, Acc, Acc)\.$' "$work/merge-dropping.pl"; then
bad "the injection did not apply -- case 12 proves nothing"
else
fresh
if KB_MERGE="$work/merge-dropping.pl" kb add "$work/rules.pl" >/dev/null 2>&1; then
bad "a merge that dropped a clause was accepted: the total is unchecked"
elif unchanged; then
ok "a merge that drops a clause is refused by the total it already printed"
else
bad "the dropped clause was caught but the kb was MODIFIED"
fi
fi
# 13. Clauses cite each other by NAME inside their text, and nothing checked
# that the name resolved -- so retiring a clause silently dangled every
# clause referring to it, which is the fact-stated-twice hazard the KB's
# own doctrine warns about in prose (improvise-2f97). The token rule is
# deliberately narrow: only a name-shaped token inside doctrine(...),
# invariant(...) or decision(...) brackets, so ordinary prose cannot
# false-positive.
fresh
echo 'doctrine(an_advisory_gate_is_not_a_gate,_).' > "$work/cited.pl"
if kb rm "$work/cited.pl" >/dev/null 2>&1; then
bad "rm retired a clause that another clause cites by name"
elif unchanged; then
ok "retiring a cited clause is refused, and the kb is unchanged"
else
bad "the dangling citation was caught but the kb was MODIFIED"
fi
# The other direction: the arm must FIRE on a citation that resolves to
# nothing, and must not fire on one that resolves. A check that only ever
# reports success is untested (kb_issue_found(checker_never_failed)).
cat > "$work/dangling.pl" <<'EOF'
doctrine(a_probe_that_cites_nothing,'cites doctrine(a_name_that_was_retired), which does not exist').
EOF
cat > "$work/resolving.pl" <<'EOF'
doctrine(a_probe_that_cites_a_real_one,'cites doctrine(an_advisory_gate_is_not_a_gate), which exists').
EOF
fresh
if kb check --with "$work/dangling.pl" >/dev/null 2>&1; then
bad "the model accepted a design citing a clause that does not exist"
elif kb check --with "$work/resolving.pl" >/dev/null 2>&1; then
ok "a citation is checked, and one that resolves is still accepted"
else
bad "the citation check refuses a name that DOES resolve"
fi
# 14. The case only same_key/2 can catch. A rm key with a variable in a
# NON-final position -- work_done keys on two arguments -- matches nothing
# as a variant and everything by unification. Under unification it removed
# an arbitrary bug_fix entry and reported success; here it must be refused
# as ABSENT, by that message and no other.
fresh
printf 'work_done(_,bug_fix,_).\n' > "$work/wild.pl"
if kb rm "$work/wild.pl" >"$work/out" 2>&1; then
bad "rm removed clauses by unifying a variable key against them"
elif ! unchanged; then
bad "rm refused a wildcard key but MODIFIED the kb"
elif said "$work/out" "no such clause to remove"; then
ok "a key with a variable in it matches nothing, rather than everything"
else
bad "the wildcard rm was refused, but not as ABSENT (same_key is unreached)"
fi
# 15. A rule keys on its whole clause, so an EDITED rule matches nothing and
# replace would silently become add -- leaving the stale arm firing beside
# the new one, with correct arithmetic (found by review on PR #69).
fresh
printf 'probe_rule(X) :- doctrine(X,_).\n' > "$work/rule1.pl"
printf 'probe_rule(X) :- doctrine(X,_), X \\== b.\n' > "$work/rule2.pl"
kb add "$work/rule1.pl" >/dev/null 2>&1
before="$(kb count | grep '^total=' | cut -d= -f2)"
if kb replace "$work/rule2.pl" >"$work/out" 2>&1; then
after="$(kb count | grep '^total=' | cut -d= -f2)"
bad "replace ADDED an edited rule beside the old one ($before -> $after)"
elif said "$work/out" "kb rm the old one"; then
ok "replace refuses a rule it cannot find, and names the route"
else
bad "the edited rule was refused, but without naming rm/add"
fi
# The negative arm: replace of a rule that IS there must still work, or the
# refusal above would just be "replace never works on rules".
if kb replace "$work/rule1.pl" >/dev/null 2>&1; then
ok "replace of an unchanged rule is still accepted"
else
bad "replace refuses a rule that is present verbatim"
fi
# 16. All three citation markers must FIRE. doctrine( and invariant( take the
# name as the first field; decision( takes the SECOND, because the repo
# spells decision(<ticket>, <name>) and a ticket id is not name-shaped --
# so a single-field rule left that marker unreachable (review, PR #69).
cat > "$work/cite-d.pl" <<'EOF'
doctrine(a_probe_citing_a_doctrine,'cites doctrine(a_name_that_was_retired), which does not exist').
EOF
cat > "$work/cite-i.pl" <<'EOF'
doctrine(a_probe_citing_an_invariant,'cites invariant(a_rule_that_was_never_written), which does not exist').
EOF
cat > "$work/cite-x.pl" <<'EOF'
doctrine(a_probe_citing_a_decision,'cites decision(improvise-nope, a_name_that_was_never_decided), which does not exist').
EOF
fresh
markers=0
for m in d i x; do
kb check --with "$work/cite-$m.pl" >/dev/null 2>&1 || markers=$((markers+1))
done
if [ "$markers" -eq 3 ]; then
ok "all three citation markers are reachable, not just the first"
else
bad "only $markers of 3 citation markers fire"
fi
# 17. The case aimed at has_key/2 itself. A keyed FACT with a variable in a
# key position -- work_done keys on (id, kind) -- is a key no existing
# clause is a variant of, so the add must land and every sibling survive.
# Under unification the fold excludes the clauses the key happens to match
# and the add is refused instead (by the total, which is the backstop, not
# the fix). Asserted as the OUTCOME, because that is what a wrong has_key
# destroys.
fresh
before="$(kb count | grep '^work_done/3=' | cut -d= -f2)"
printf "work_done(_,maintenance,'a probe clause with a variable id').\n" > "$work/wildfact.pl"
if kb add "$work/wildfact.pl" >"$work/out" 2>&1; then
after="$(kb count | grep '^work_done/3=' | cut -d= -f2)"
if [ "$((before + 1))" = "$after" ]; then
ok "a fact whose key holds a variable adds without excluding its siblings"
else
bad "adding one clause moved work_done/3 from $before to $after"
fi
else
bad "add refused a key holding a variable: the fold matched by unification"
fi
printf '\nkb-test: %d passed, %d failed\n' "$pass" "$fail"
[ "$fail" -eq 0 ]
scripts/node-test-test.sh
Executable
+131
View File
@@ -0,0 +1,131 @@
#!/usr/bin/env bash
# check_the_checker for scripts/node-test.sh. Every case is a way a node test
# step can exit 0 having asserted nothing -- which is what the step it replaces
# did, on both CI jobs, for the whole life of web/tests/ (improvise-vz54).
set -uo pipefail
here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
RUN="$here/node-test.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, rest: patterns
run_case() {
local name="$1" want="$2"; shift 2
"$RUN" "$@" >/dev/null 2>&1
local got=$?
if [ "$want" = "nonzero" ]; then
[ "$got" -ne 0 ] && ok "$name" || bad "$name" "$got" "$want"
else
[ "$got" = "$want" ] && ok "$name" || bad "$name" "$got" "$want"
fi
}
mkdir -p "$work/tests" "$work/empty"
cat > "$work/tests/green.test.js" <<'EOF'
const { test } = require("node:test");
test("a test that passes", () => {});
EOF
cat > "$work/tests/red.test.js" <<'EOF'
const { test } = require("node:test");
const assert = require("node:assert");
test("a test that fails", () => { assert.equal(1, 2); });
EOF
# A file that matches the glob and registers no test -- a helper left in the
# directory, or a suite emptied by a refactor. node reports it as ONE PASSING
# TEST NAMED AFTER THE FILE, so `# tests` reads as the file count and a
# minimum-count assertion over it is satisfied by the offender. This case also
# PINS that naming convention: if node stops doing it, this fails rather than
# the guard going quiet.
cat > "$work/empty/nothing.test.js" <<'EOF'
module.exports = { helper: () => 1 };
EOF
# The four shapes a lexical `grep` for `test(` cannot tell from a test, each
# of which executes no assertion (found by review on PR #69).
mkdir -p "$work/commented" "$work/skipped" "$work/todo" "$work/suite" "$work/shapes"
cat > "$work/commented/commented.test.js" <<'EOF'
const { test } = require("node:test");
// test("this one was removed", () => {});
const note = 'the word test( appears in this string';
EOF
cat > "$work/skipped/skipped.test.js" <<'EOF'
const { test } = require("node:test");
test.skip("skipped, so nothing ran", () => {});
EOF
cat > "$work/todo/todo.test.js" <<'EOF'
const { test } = require("node:test");
test.todo("not written yet");
EOF
cat > "$work/suite/suite.test.js" <<'EOF'
const { test } = require("node:test");
test.describe("a suite with nothing in it", () => {});
EOF
# The guard must not call a real suite barren: a describe holding a test is
# coverage, and its tests are nested rather than top-level.
cat > "$work/shapes/shapes.test.js" <<'EOF'
const { test } = require("node:test");
test.describe("a group", () => {
test("a nested test", () => {});
});
EOF
# The bug itself. Under bash the literal survives, node globs it, matches
# nothing, and the step goes green. Demonstrated at improvise-vz54.
run_case "a pattern that matches nothing is refused" 1 "$work/gone/*.test.js"
run_case "a matching file with a passing test is accepted" 0 "$work/tests/green.test.js"
# The refusal must not be bought by swallowing real failures.
run_case "a failing test still fails" nonzero "$work/tests/red.test.js"
run_case "a file that registers no test is refused" 1 "$work/empty/*.test.js"
run_case "a commented-out test, and the words test( in a string, are not a test" 1 \
"$work/commented/*.test.js"
run_case "a file whose only test is skipped is refused" 1 "$work/skipped/*.test.js"
run_case "a file whose only test is a todo is refused" 1 "$work/todo/*.test.js"
run_case "an empty suite is refused" 1 "$work/suite/*.test.js"
run_case "a describe holding a real test is not called barren" 0 \
"$work/shapes/*.test.js"
# A degenerate file hiding among real ones is the shape a refactor leaves: the
# run passes, the count is healthy, and one file asserts nothing. node's TAP
# does not attribute a passing test to its file, so a whole-run count cannot
# see either of these -- which is why the guard runs one file at a time.
run_case "a barren file among passing ones is still named" 1 \
"$work/tests/green.test.js" "$work/empty/nothing.test.js"
run_case "a skip-only file among passing ones is still named" 1 \
"$work/tests/green.test.js" "$work/skipped/skipped.test.js"
run_case "a todo-only file among passing ones is still named" 1 \
"$work/tests/green.test.js" "$work/todo/todo.test.js"
# Two globs where the SECOND dies is the shape a directory move makes: the
# first still matches, so a check over the union would pass.
run_case "one dead pattern among several is refused" 1 \
"$work/tests/green.test.js" "$work/gone/*.test.js"
run_case "no argument at all is a usage error, not a pass" 2
# The literal the caller would pass if they forgot to quote AND the glob
# matched: ordinary filenames, which expand to themselves -- several of them,
# since one is what the accepted case above already passes.
run_case "already-expanded filenames still run" 0 \
"$work/tests/green.test.js" "$work/shapes/shapes.test.js"
printf '\nnode-test-test: %d passed, %d failed\n' "$pass" "$fail"
[ "$fail" -eq 0 ]
scripts/node-test.sh
Executable
+112
View File
@@ -0,0 +1,112 @@
#!/usr/bin/env bash
# `node --test web/tests/*.test.js` is a gate that stops gating the moment the
# directory moves: bash leaves an unmatched glob as a literal, node 22+ treats
# the literal as a glob of its own, matches nothing, reports 0 tests and exits
# 0 (improvise-vz54). zsh aborts on an unmatched glob, so the failure is
# invisible from a developer's shell and visible only under CI's.
#
# So the glob is the SCRIPT's, not the shell's -- quote it at the call site.
# Three things are refused, and all three are "the gate asserted nothing"
# wearing an exit 0: a pattern that matches no file, a run in which nothing
# passed, and a matched file that registered no test.
#
# The last two are read off node's OWN report rather than scanned for lexically.
# A `grep` for `test(` is satisfied by a commented-out call, by a string
# mentioning one, by `test.skip`/`test.todo` and by an empty `test.describe` --
# every one of which executes no assertion (found by review on PR #69).
#
# Two measurements make the report usable, both on node 24.20, and both are
# taken PER FILE. `# pass` is 0 for a skip-only file, a todo-only file and an
# empty suite -- read over a whole run it would be masked by a passing
# neighbour. And a file that registered NO test is reported as one passing test
# WHOSE NAME IS THE FILE PATH, which is why `# tests` alone reads as the file
# count and a minimum-count assertion over it is satisfied by the very file it
# should reject. That naming convention is an assumption about node, so
# scripts/node-test-test.sh pins it: if a future node stops doing it, the
# self-test fails rather than the guard silently going quiet.
set -uo pipefail
usage() {
echo "usage: ${0##*/} '<glob>' ['<glob>'...]" >&2
echo " quote each glob: this script expands it, not the calling shell" >&2
exit 2
}
[ "$#" -gt 0 ] || usage
shopt -s nullglob
files=()
for pattern in "$@"; do
matched=($pattern)
if [ "${#matched[@]}" -eq 0 ]; then
echo "node-test: no file matches '$pattern'" >&2
echo "node-test: refusing to run, because node would report success here" >&2
exit 1
fi
files+=("${matched[@]}")
done
tap="$(mktemp)"
trap 'rm -f "$tap"' EXIT
# ONE FILE PER RUN. node's TAP does not attribute a passing test to the file it
# came from -- three files produce three flat `ok` lines named after the tests
# -- so a whole-run `# pass` count cannot tell a fully-skipped file from its
# passing neighbours, and a skip-only file rode in on a sibling (found by
# review on PR #69). node already forks a child per file, so this changes what
# is OBSERVABLE, not how the tests run.
status=0
total=0
silent=()
barren=()
for f in "${files[@]}"; do
node --test \
--test-reporter=spec --test-reporter-destination=stdout \
--test-reporter=tap --test-reporter-destination="$tap" \
"$f"
ran=$?
if [ "$ran" -ne 0 ]; then
status="$ran"
continue
fi
passed="$(sed -n 's/^# pass \([0-9][0-9]*\)$/\1/p' "$tap" | tail -1)"
if [ -z "$passed" ]; then
echo "node-test: could not read a pass count out of node's TAP output for $f" >&2
status=1
continue
fi
total=$((total + passed))
if [ "$passed" -eq 0 ]; then
silent+=("$f")
continue
fi
# A file that registered NO test is reported as one passing test whose name
# is the FILE PATH, which is why `# tests` reads as the file count.
awk -v want="$f" '
/^(ok|not ok) [0-9]+ - / {
desc = $0
sub(/^(ok|not ok) [0-9]+ - /, "", desc)
sub(/ # (SKIP|TODO).*$/, "", desc)
if (desc == want) { found = 1 }
}
END { exit found ? 0 : 1 }
' "$tap" && barren+=("$f")
done
if [ "${#silent[@]}" -gt 0 ]; then
echo "node-test: these files passed nothing (skipped, todo or an empty suite):" >&2
printf ' %s\n' "${silent[@]}" >&2
fi
if [ "${#barren[@]}" -gt 0 ]; then
echo "node-test: these files registered no test, so running them asserted nothing:" >&2
printf ' %s\n' "${barren[@]}" >&2
fi
if [ "$status" -ne 0 ]; then
exit "$status"
fi
if [ "${#silent[@]}" -gt 0 ] || [ "${#barren[@]}" -gt 0 ]; then
exit 1
fi
echo "node-test: $total test(s) passed over ${#files[@]} file(s)"
web/assemble.sh
+18 -5
View File
@@ -73,18 +73,31 @@ sedi "s|'improvise_worker_server_bg\.wasm'|'improvise_worker_server_bg.wasm?v=${
# 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 the one missing.
# 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() {
grep -q "$2" "$1" || {
if grep -q "$2" "$1"; then
grep -n "$2" "$1"
else
echo "assemble: $1 is missing the cache-bust stamp: $2" >&2
exit 1
}
grep -n "$2" "$1"
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
}
# 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