check-figures: the guard's body belongs behind the entry-point check
The fold exported scanDocument for the string tests, and importing it also ran the guard. check-behaviour imports that module, so a figure defect called process.exit(1) inside check-behaviour: it reported check-figures' failure under its own name having run zero of its 108 tests. The build went red, which is why this was survivable, but it went red in the wrong place and every behavioural test was silently not running while appearing to. A guard that stops another guard from running, and cannot say so, is the worst version of the fault this file exists to catch. Body now sits behind import.meta.main, the idiom step5-split.mjs already uses. Importing yields the four readers and nothing else. Tested by spawning a fresh process, because the property is "importing has no effect" and a source assertion would pass on a file that grew a second side effect elsewhere. Reverting the check fails exactly one test. With a defect planted, check-behaviour runs 108 green and check-figures fails in its own slot. Mine, introduced by R-307. R-309. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
slaguru666
co-authored by
Claude Opus 5
Claude Opus 5
parent
bd9b3bdd27
commit
d0db7954bc
@@ -7791,3 +7791,47 @@ rounds"* fails, the ratchet trips, **the cited-figure-with-duplicate case is now
|
||||
the clean tree is silent.
|
||||
|
||||
**State:** 21 guards, one merged reader, CLEAN GROUND v0.24.
|
||||
|
||||
---
|
||||
|
||||
## R-309 — a guard that stopped another guard from running
|
||||
|
||||
**The fold exported `scanDocument` so the readers could be tested on strings, and for two
|
||||
commits importing it ran the whole guard.** `check-behaviour` imports that module. So:
|
||||
|
||||
```
|
||||
$ # with the historical 24 restored
|
||||
$ node tools/check-behaviour.mjs
|
||||
check-figures: FAILED — measured figures are printed with nothing holding them
|
||||
docs/scenarios/CLEAN_GROUND.md:1277 — "**24**" carries no citation marker …
|
||||
exit=1
|
||||
```
|
||||
|
||||
**A figure defect called `process.exit(1)` inside check-behaviour, which reported another
|
||||
guard's failure under its own name having run zero of its 108 tests.** The build still went
|
||||
red, which is the only reason this was survivable — but it went red in the wrong place, and
|
||||
every behavioural test in the repo was silently not running while appearing to.
|
||||
|
||||
**This is the sharpest form yet of the fault this log keeps recording.** R-300 found a check
|
||||
that passed for a reason unrelated to what it claimed. This is a check that *did not run at
|
||||
all* and could not say so, because the thing that stopped it was a different guard's verdict
|
||||
arriving through an import. A guard is a claim we are still checking; this one had quietly
|
||||
stopped being checked, and the evidence that it had was indistinguishable from a failure.
|
||||
|
||||
**Fixed with the idiom already in `step5-split.mjs`** — the executable body sits behind
|
||||
`import.meta.main || process.argv[1]?.endsWith(...)`, so importing the module yields the four
|
||||
readers and nothing else.
|
||||
|
||||
**The test is a spawn, not a source assertion.** The property is *importing has no effect*,
|
||||
and only a fresh process can tell you that; asserting the source contains the idiom would
|
||||
pass on a file that had grown a second side effect elsewhere. Importing must print nothing
|
||||
and exit 0.
|
||||
|
||||
**Proved both ways.** Reverting the guard to `if (true)` fails exactly one test — the new one.
|
||||
With a figure defect planted, `check-behaviour` now runs all 108 tests green and
|
||||
`check-figures` fails in its own slot with its own message.
|
||||
|
||||
**It arrived with the fold and it is mine.** It exists because R-307 needed an export for the
|
||||
string tests, and it was found because those same tests print their own name — the second
|
||||
time in one evening that the string tests paid for themselves in a way nobody designed. The
|
||||
general rule for this repo: **a module a guard imports must not be a guard that runs.**
|
||||
|
||||
@@ -33,6 +33,7 @@ import { STARTER_AUTHORITY, STARTER_GROUPS, STARTER_CASE, STARTER_TEAM }
|
||||
from "./scenario-starter.mjs";
|
||||
import { castMarkersIn, castLikeIn } from "./declared-cast.mjs";
|
||||
import { scanDocument } from "./check-figures.mjs";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { tagsIn, classifiedFiles, playableFiles } from "./check-scenarios.mjs";
|
||||
import { beatsIn, beatLikeIn } from "./outcome-coverage.mjs";
|
||||
import { step5Split, split as step5Of, THING_RATING } from "./step5-split.mjs";
|
||||
@@ -999,6 +1000,22 @@ test("a document that cites nothing is held by the ratchet, not the strict regim
|
||||
assert.ok(r.unmarked >= 1, "but its bare figures are still counted for the ratchet");
|
||||
});
|
||||
|
||||
test("importing check-figures runs the readers and NOT the guard", () => {
|
||||
/* THE REGRESSION THIS EXISTS FOR: for one commit the guard's body ran at import, so a
|
||||
figure defect called process.exit(1) inside THIS file — check-behaviour reporting
|
||||
check-figures' failure under its own name, having run none of its tests. Spawned rather
|
||||
than asserted on the source, because the property is "importing has no effect", and only
|
||||
a fresh process can tell you that. */
|
||||
const mod = new URL("./check-figures.mjs", import.meta.url).href;
|
||||
const r = spawnSync(process.execPath,
|
||||
["--input-type=module", "-e", `await import(${JSON.stringify(mod)});`],
|
||||
{ encoding: "utf8" });
|
||||
assert.equal(r.status, 0, "importing check-figures must not exit non-zero");
|
||||
assert.equal((r.stdout + r.stderr).trim(), "",
|
||||
"importing check-figures must print nothing — the guard's body belongs behind the "
|
||||
+ "import.meta.main check, or a figure defect stops this file mid-run");
|
||||
});
|
||||
|
||||
test("the reader reports the line the figure is actually on", () =>
|
||||
assert.match(firstFault(doc("one\ntwo\nwipes **8.3%** here")), /x\.md:3/));
|
||||
|
||||
|
||||
+77
-67
@@ -370,82 +370,92 @@ export function scanDocument(label, src) {
|
||||
return { rows, strict, waived, unmarked, optedIn };
|
||||
}
|
||||
|
||||
const strict = [];
|
||||
const loose = {};
|
||||
const rows = [];
|
||||
const waived = [];
|
||||
/* ── running the guard ─────────────────────────────────────────────────────────────────
|
||||
EVERYTHING BELOW RUNS ONLY WHEN THIS FILE IS THE ENTRY POINT, and the reason is not tidiness.
|
||||
`scanDocument` is exported so check-behaviour can test the readers on strings (R-292), and
|
||||
for one commit importing it ALSO ran the whole guard: a figure defect called `process.exit(1)`
|
||||
inside check-behaviour, which then reported check-figures' failure under its own name having
|
||||
run ZERO of its 107 tests. A guard that stops another guard from running while appearing to
|
||||
participate is the worst version of the fault this file exists to catch. Same idiom as
|
||||
step5-split.mjs. */
|
||||
if (import.meta.main || process.argv[1]?.endsWith("check-figures.mjs")) {
|
||||
const strict = [];
|
||||
const loose = {};
|
||||
const rows = [];
|
||||
const waived = [];
|
||||
|
||||
for (const [label, abs] of playableFiles()) {
|
||||
const r = scanDocument(label, readFileSync(abs, "utf8"));
|
||||
rows.push(...r.rows); strict.push(...r.strict); waived.push(...r.waived);
|
||||
if (!r.optedIn) loose[label] = r.unmarked;
|
||||
}
|
||||
for (const [label, abs] of playableFiles()) {
|
||||
const r = scanDocument(label, readFileSync(abs, "utf8"));
|
||||
rows.push(...r.rows); strict.push(...r.strict); waived.push(...r.waived);
|
||||
if (!r.optedIn) loose[label] = r.unmarked;
|
||||
}
|
||||
|
||||
/* ── the ratchet, for documents outside the citation regime ────────────────────────────── */
|
||||
/* ── the ratchet, for documents outside the citation regime ────────────────────────────── */
|
||||
|
||||
const base = existsSync(BASELINE) ? JSON.parse(readFileSync(BASELINE, "utf8")) : null;
|
||||
const base = existsSync(BASELINE) ? JSON.parse(readFileSync(BASELINE, "utf8")) : null;
|
||||
|
||||
if (UPDATE) {
|
||||
writeFileSync(BASELINE, JSON.stringify({
|
||||
note: "Unmarked measurement figures in scenarios that use no citations. May fall, never rise. "
|
||||
+ "Generated by tools/check-figures.mjs --update.",
|
||||
uncited: loose
|
||||
}, null, 1) + "\n");
|
||||
console.log(`check-figures: recorded ${Object.keys(loose).length} document(s) outside the citation regime`);
|
||||
process.exit(0);
|
||||
}
|
||||
if (UPDATE) {
|
||||
writeFileSync(BASELINE, JSON.stringify({
|
||||
note: "Unmarked measurement figures in scenarios that use no citations. May fall, never rise. "
|
||||
+ "Generated by tools/check-figures.mjs --update.",
|
||||
uncited: loose
|
||||
}, null, 1) + "\n");
|
||||
console.log(`check-figures: recorded ${Object.keys(loose).length} document(s) outside the citation regime`);
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
const problems = [...strict];
|
||||
const problems = [...strict];
|
||||
|
||||
if (!base) {
|
||||
problems.push(` no ratchet at tools/figures-baseline.json — run: node tools/check-figures.mjs --update`);
|
||||
} else {
|
||||
for (const [label, n] of Object.entries(loose)) {
|
||||
const was = base.uncited?.[label];
|
||||
if (was === undefined) {
|
||||
problems.push(` ${label} uses no citations and is not in the ratchet. Re-record with --update, `
|
||||
+ `or give it its first citation marker and it joins the strict regime.`);
|
||||
} else if (n > was) {
|
||||
problems.push(` ${label}: unmarked measurement figures went ${was} -> ${n}. This document cites `
|
||||
+ `nothing, so the ratchet is all that holds it: the count may fall, never rise.`);
|
||||
if (!base) {
|
||||
problems.push(` no ratchet at tools/figures-baseline.json — run: node tools/check-figures.mjs --update`);
|
||||
} else {
|
||||
for (const [label, n] of Object.entries(loose)) {
|
||||
const was = base.uncited?.[label];
|
||||
if (was === undefined) {
|
||||
problems.push(` ${label} uses no citations and is not in the ratchet. Re-record with --update, `
|
||||
+ `or give it its first citation marker and it joins the strict regime.`);
|
||||
} else if (n > was) {
|
||||
problems.push(` ${label}: unmarked measurement figures went ${was} -> ${n}. This document cites `
|
||||
+ `nothing, so the ratchet is all that holds it: the count may fall, never rise.`);
|
||||
}
|
||||
}
|
||||
/* A document that LEAVES the loose set has gained its first citation, which is good news
|
||||
and must not be reported as a missing row. A document that vanishes entirely has been
|
||||
deleted or reclassified, and that is worth saying out loud. */
|
||||
for (const label of Object.keys(base.uncited ?? {})) {
|
||||
if (!(label in loose) && !rows.some(r => r.label === label)) {
|
||||
problems.push(` ${label} is in the ratchet but no longer in the corpus. If it was renamed or `
|
||||
+ `deleted, re-record with --update.`);
|
||||
}
|
||||
}
|
||||
}
|
||||
/* A document that LEAVES the loose set has gained its first citation, which is good news
|
||||
and must not be reported as a missing row. A document that vanishes entirely has been
|
||||
deleted or reclassified, and that is worth saying out loud. */
|
||||
for (const label of Object.keys(base.uncited ?? {})) {
|
||||
if (!(label in loose) && !rows.some(r => r.label === label)) {
|
||||
problems.push(` ${label} is in the ratchet but no longer in the corpus. If it was renamed or `
|
||||
+ `deleted, re-record with --update.`);
|
||||
|
||||
if (LIST) {
|
||||
for (const r of rows) {
|
||||
console.log(` ${r.marked ? "cited " : "BARE "} ${r.label}:${r.line} [${r.kind}] ${r.text}`);
|
||||
}
|
||||
problems.forEach(p => console.log(p));
|
||||
process.exit(0);
|
||||
}
|
||||
}
|
||||
|
||||
if (LIST) {
|
||||
for (const r of rows) {
|
||||
console.log(` ${r.marked ? "cited " : "BARE "} ${r.label}:${r.line} [${r.kind}] ${r.text}`);
|
||||
if (problems.length) {
|
||||
console.error("check-figures: FAILED — measured figures are printed with nothing holding them");
|
||||
problems.forEach(p => console.error(p));
|
||||
console.error(` check-cited resolves the markers that exist; it cannot see a figure that has none. `
|
||||
+ `That is how "a median 24 rounds" survived ten passes against a baseline holding 21.`);
|
||||
process.exit(1);
|
||||
}
|
||||
/* WAIVERS ARE PRINTED ON EVERY GREEN BUILD. A figure nobody checks is the defect this guard
|
||||
exists for, and a waived figure is one we have decided to stop checking — so the decision
|
||||
stays on screen rather than becoming the default. Six of them is a short list; sixty would
|
||||
be a finding. */
|
||||
if (waived.length) {
|
||||
console.log(`check-figures: ${waived.length} figure(s) waived, each on the record:`);
|
||||
for (const w of waived) console.log(` ${w.label}:${w.line} ${w.text} — ${w.reason}`);
|
||||
}
|
||||
problems.forEach(p => console.log(p));
|
||||
process.exit(0);
|
||||
}
|
||||
if (problems.length) {
|
||||
console.error("check-figures: FAILED — measured figures are printed with nothing holding them");
|
||||
problems.forEach(p => console.error(p));
|
||||
console.error(` check-cited resolves the markers that exist; it cannot see a figure that has none. `
|
||||
+ `That is how "a median 24 rounds" survived ten passes against a baseline holding 21.`);
|
||||
process.exit(1);
|
||||
}
|
||||
/* WAIVERS ARE PRINTED ON EVERY GREEN BUILD. A figure nobody checks is the defect this guard
|
||||
exists for, and a waived figure is one we have decided to stop checking — so the decision
|
||||
stays on screen rather than becoming the default. Six of them is a short list; sixty would
|
||||
be a finding. */
|
||||
if (waived.length) {
|
||||
console.log(`check-figures: ${waived.length} figure(s) waived, each on the record:`);
|
||||
for (const w of waived) console.log(` ${w.label}:${w.line} ${w.text} — ${w.reason}`);
|
||||
}
|
||||
|
||||
const marked = rows.filter(r => r.marked).length;
|
||||
const looseTotal = Object.values(loose).reduce((a, b) => a + b, 0);
|
||||
console.log(`check-figures: OK — ${rows.length} measurement-shaped figures across ${new Set(rows.map(r => r.label)).size} `
|
||||
+ `scenarios, ${marked} marked and ${waived.length} waived; ${looseTotal} bare in `
|
||||
+ `${Object.keys(loose).length} document(s) that cite nothing, held at the ratchet`);
|
||||
const marked = rows.filter(r => r.marked).length;
|
||||
const looseTotal = Object.values(loose).reduce((a, b) => a + b, 0);
|
||||
console.log(`check-figures: OK — ${rows.length} measurement-shaped figures across ${new Set(rows.map(r => r.label)).size} `
|
||||
+ `scenarios, ${marked} marked and ${waived.length} waived; ${looseTotal} bare in `
|
||||
+ `${Object.keys(loose).length} document(s) that cite nothing, held at the ratchet`);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user