diff --git a/docs/REVIEW_LOG.md b/docs/REVIEW_LOG.md index 5bdb7a1..02c109e 100644 --- a/docs/REVIEW_LOG.md +++ b/docs/REVIEW_LOG.md @@ -6690,3 +6690,65 @@ nothing to check. Five cases in a worktree: two decoys (fail here, pass at R-284, the pass confirmed by running HEAD's copy against the same tree), a reword inside the section, the renamed heading, and the clean control. + +## R-286 — a declaration and a mention of one look identical to a regex + +Swept the other seventeen guards for the page-wide match R-285 removed from check-powers. +Most are already scoped or have nothing to scope. Two are not, and the first is live. + +`CLEAN_GROUND.md` contains **two** things matching ``: the declaration +under `## Casting`, and — ten lines below it — the warning that explains it, which quotes +`` `` `` inside backticks and matches the same pattern with an empty capture. +`check-rollable` and `declared-cast.mjs` both used `.match()`, first hit wins. **The whole +arrangement has been correct because the declaration happens to come first.** + +Move that warning above the list and `check-rollable` reads a cast of nobody, falls back to +`ROSTER_BEST`, and goes green having held every skill in the document to the full duty roster +instead of the declared six — the identical OK line, the scope silently widened. Instrumented +and read off, not reasoned about: + + [scope] docs/scenarios/CLEAN_GROUND.md -> the duty roster (warning moved above) + [scope] docs/scenarios/CLEAN_GROUND.md -> its declared cast of 6 (as the file stands) + +`check-firstblood` fails on the same page, but says *"CLEAN GROUND now casts 0"* — describing +a scenario that casts six, which sends a reader to the cast list rather than to the two +markers. R-268 had this bug from the other side, when a tools file's own example was read as +a real declaration; this is the same defect facing inward, and the marker that triggers it was +added to the document by the warning telling everyone how load-bearing the marker is. + +`castMarkersIn` now blanks code spans and fenced blocks before scanning — padded, so line +numbers still point at the source — and more than one surviving marker is fatal with every +line named. `check-rollable` imports it rather than carrying a second regex, so the guard that +checks the cast and the guards that measure the fight cannot disagree about who is in it. + +**Disclosed to c0 the same minute.** The comparison that found this was meant to run in a +throwaway worktree; `git worktree add` failed on a stale registration, the heredoc ran on in +the shared tree, and it moved one line of c0's uncommitted `CLEAN_GROUND.md`. Reverted by +hand, `## Casting` block diffed byte-identical against HEAD, c0 told what to check and why +before anything else was done. A `test -d` on the worktree now guards the pattern. + +## R-287 — a scope anchored on a name that has moved is the whole file + +`check-behaviour` scopes three source assertions with `indexOf`. For a name that is gone +`indexOf` returns -1 and `slice(0, -1)` is everything-but-one-character, so the scope does not +shrink or fail — it becomes the file. Renaming the end anchor took one test's body from +**3,184 characters to 30,825** with the assertion still passing: a test that reads like a +statement about `rollWeaponDamage` making a statement about all of `ringbrp.mjs`. + +The other site windowed `burstAttack` at 4,000 characters. The function runs 4,305, so the +window was already short of its own subject, and one edit the other way would have had it +testing whatever came next for a `locate: false` that belongs to a different call. + +`bodyOf` now asserts the anchor and ends structurally, at the next top-level function. A +missing anchor says so — *check-behaviour scopes a test to "export async function burstAttack" +and ringbrp.mjs no longer contains it. Fix the anchor: unfixed, the test would read the whole +file and pass* — where the old code reported *"burstAttack still calls rollWeaponDamage"*, +which is a claim about a call when the truth is a claim about a name. + +**The rest of the sweep, so the negative result is on the record.** check-cited resolves each +citation positionally and already refuses markers it cannot read. check-handouts matches each +almanac row's note against the whole scenario (`md.includes(note)`); the notes are distinctive +sentences and there is no smaller slice that owns them, so it is left alone deliberately +rather than overlooked. check-rules builds its covered-set from labels it generates, not from +searching a document. check-lang, check-creatures, check-templates, check-kits and the +baseline guards do no document matching at all. diff --git a/tools/check-behaviour.mjs b/tools/check-behaviour.mjs index 958df6f..c0e51d9 100644 --- a/tools/check-behaviour.mjs +++ b/tools/check-behaviour.mjs @@ -626,10 +626,29 @@ test("the marking outcome and the improving outcome are the same idea", () => { did not. These tests are about the JOIN, because every individual part was already correct and the table still never saw a located hit. */ +/* A SCOPE ANCHORED ON A NAME THAT HAS MOVED IS NOT A SMALLER SCOPE, IT IS THE WHOLE FILE. + `indexOf` returns -1 for a name that is gone, `slice(0, -1)` is everything-but-one-character, + and the assertion below then passes against 30,825 characters of unrelated code while + reading like a test about one function — measured, not supposed: renaming the end anchor + took this body from 3,184 characters to 30,825 and the test stayed green. The other site + used a 4,000-character window on a function that runs 4,305, so it was both short of its + own subject and one edit away from covering the next one. + + Same defect R-285 removed from check-powers, in source rather than prose. So the anchor is + asserted and the end is structural: the next top-level function, whatever it is called. */ +const NEXT_FN = /\n(?:export\s+)?(?:async\s+)?function\s/; +function bodyOf(src, decl) { + const at = src.indexOf(decl); + assert.notEqual(at, -1, `check-behaviour scopes a test to "${decl}" and ringbrp.mjs no longer ` + + `contains it. Fix the anchor: unfixed, the test would read the whole file and pass.`); + const rest = src.slice(at); + const end = rest.slice(1).search(NEXT_FN); + return end < 0 ? rest : rest.slice(0, end + 1); +} + test("an ordinary weapon damage roll asks for a location", () => { const src = readFileSync(new URL("../ringbrp.mjs", import.meta.url), "utf8"); - const fn = src.slice(src.indexOf("export async function rollWeaponDamage")); - const body = fn.slice(0, fn.indexOf("\nasync function promptDamageBand")); + const body = bodyOf(src, "export async function rollWeaponDamage"); assert.ok(/hitLocationRoll\(/.test(body), "rollWeaponDamage must reach hitLocationRoll — this is the join that was missing"); }); @@ -649,8 +668,7 @@ test("a weapon's class picks the column of the table it reads", () => { test("the one caller that locates for itself opts out, so nothing applies twice", () => { const src = readFileSync(new URL("../ringbrp.mjs", import.meta.url), "utf8"); - const burst = src.slice(src.indexOf("export async function burstAttack")); - const body = burst.slice(0, 4000); + const body = bodyOf(src, "export async function burstAttack"); const call = body.match(/rollWeaponDamage\(\{[^}]*\}/); assert.ok(call, "burstAttack still calls rollWeaponDamage"); assert.ok(/locate:\s*false/.test(call[0]),