From 4a9c93f8665d59246828edb49d073f028eb0c9bb Mon Sep 17 00:00:00 2001 From: SkyFi Geek <45924209+mobileskyfi@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:53:46 -0700 Subject: [PATCH 1/3] explain: gate the value census, don't just make it re-derivable (#260) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The census figures live in three places — the census itself, the `corpus` block of `test/fixtures/explain/values.json`, and prose in `commands/explain/README.md`. Committing the census script (#242) made them re-derivable; nothing checked that they still matched. #256 changed the value emission, regenerated the fixture and the unit assertions, and shipped with the README prose still quoting the pre-change numbers — three review comments found that, not CI. Each link in corpus -> fixture -> README now has its own gate, separated by what data it needs: - `explain:value-census:check` re-runs the census and asserts it figure by figure against the committed fixture. It needs `corpus.sqlite`, so it runs in ci.yaml's `corpus` job, which #186 gave `corpus:fetch` reachability for precisely so this could exist. It runs after the summary step, so a drifted run still publishes its figures — those are what say whether the drift is an intended emission change or a regression. - `explain:value-census:readme:check` asserts the README paragraph against that same fixture block. The paragraph is now a generated projection of it (`explain:value-census:readme`), so the drift that actually happened is caught with no corpus at all: in `lint:ci`, and in `bun test`. The fixture is the hinge — the one copy a human writes — and both gates point at it rather than at each other. The generated prose reports the invariants as counters rather than asserting they hold ("read 0, 0 and 0, and each must stay 0"), so every sentence stays true at any value and regenerating after a regression states it instead of publishing a reassuring lie. A unit test proves that by rendering a synthetic regressed census. Verified end to end against the real corpus: `explain:value-census --json` still reproduces the committed fixture byte for byte, and both gates were confirmed to FAIL on an injected one-figure drift before being wired in. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yaml | 18 ++- CHANGELOG.md | 12 ++ commands/explain/README.md | 28 ++-- package.json | 5 +- scripts/explain-value-census.ts | 213 ++++++++++++++++++++++++- test/unit/explain-value-census.test.ts | 115 +++++++++++++ 6 files changed, 374 insertions(+), 17 deletions(-) create mode 100644 test/unit/explain-value-census.test.ts diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7570426..5caed81 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -158,10 +158,15 @@ jobs: # which is what #260 hit. `corpus:fetch` pulls the snapshot pinned by # scripts/corpus-pin.json and verifies its sha256. # - # What is a gate here and what is not: the FETCH gates — a hash mismatch is a - # hard failure, or the pin is decorative. The census figures do NOT gate; they - # go to the run summary as a trend. Asserting them against the committed - # fixture is #260's job, and it needs the reachability this job provides. + # What gates here (#260): the FETCH — a hash mismatch is a hard failure, or + # the pin is decorative — and the VALUE census against the committed + # `corpus` block of test/fixtures/explain/values.json, which is the + # reachability this job was built to provide. The genre census still only + # reports: it has no committed fixture to be asserted against. + # + # The other half of #260 needs no corpus and is not here: the README figures + # are a generated projection of that same fixture, gated by + # `explain:value-census:readme:check` inside `lint:ci`. corpus: name: Corpus census needs: [checks] @@ -209,6 +214,11 @@ jobs: bun run explain:value-census 2>&1 echo '```' } >> "$GITHUB_STEP_SUMMARY" + # After the summary, so a drifted run still publishes its figures — the + # numbers are what tells you whether the drift is an intended emission + # change or a regression. + - name: Value census matches the committed fixture + run: bun run explain:value-census:check chr-smoke: name: CHR smoke (stable) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49fc0ed..5c1f021 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,18 @@ documenting cross-cutting shifts that affect contributors and consumers. and never on a PR; both now fall back to the pinned snapshot and announce the source and hash they used. A new CI job gates the fetch and posts both censuses to the run summary (#186). +- **The `explain` value census is drift-gated, not just re-derivable.** Its + figures live in three places — the census itself, the `corpus` block of + `test/fixtures/explain/values.json`, and prose in `commands/explain/README.md` + — and #256 shipped with the README still quoting pre-change numbers, caught by + review rather than CI. Each link now has a gate keyed to the data it needs: + `bun run explain:value-census:check` re-runs the census against the committed + fixture in the CI corpus job, while the README paragraph became a generated + projection of that fixture (`bun run explain:value-census:readme`) checked by + `bun run explain:value-census:readme:check` in `lint:ci` and in `bun test`, + needing no corpus at all. The generated prose reports the invariants as + counters rather than asserting they hold, so regenerating after a regression + states it instead of reading as reassurance (#260). ## 0.1.3 — 2026-07-06 diff --git a/commands/explain/README.md b/commands/explain/README.md index 2a79d5e..324f1cf 100644 --- a/commands/explain/README.md +++ b/commands/explain/README.md @@ -779,16 +779,26 @@ and its phase is named below. on the seconds fallback rather than encoding a chain that depends on a failed address attempt. + The corpus census is re-derivable with `bun run explain:value-census` and - covers 948 source scripts. Every one of the 13,168 emitted values in a - statement the strict argument lexer ALSO read has identical half-open byte - spans and decoded text (0 contradictions), while the prefix-safe scan retains - 6,540 values across 569 statements whose strict REST reading abstains. Of - 19,708 emitted occurrences, 5,636 are array members (529 keyed, 1,147 nested - inside another member) and 813 are arrays; the corpus contains no - source-literal `id` example. All spans were in bounds, every member named a - container that exists, and every member sat strictly inside it. The strict - lexer remains all-or-nothing; only non-authoritative hints use the wider view. + covers 948 source scripts. The figures below are generated from + `test/fixtures/explain/values.json` → `corpus` by + `bun run explain:value-census:readme` and gated against it by + `bun run explain:value-census:readme:check`; the fixture itself is gated + against a fresh corpus run by `bun run explain:value-census:check`. Of the + 13,168 emitted values in a statement the strict argument lexer ALSO read, 0 + disagree with it on half-open byte span or decoded text, while the + prefix-safe scan retains a further 6,540 values across 569 statements whose + strict REST reading abstains. Of 19,708 emitted occurrences, 5,636 are array + members (529 keyed, 1,147 nested inside another member) and 813 are arrays; + the corpus holds no source-literal `id` example. The three structural + counters — spans addressing bytes outside their own source, members naming a + container that does not exist, members escaping the container they name — + read 0, 0 and 0, and each must stay 0. + + + The strict lexer remains all-or-nothing; only non-authoritative hints use the + wider view. ### Offline comment placement (#245) diff --git a/package.json b/package.json index ab58525..74b4240 100644 --- a/package.json +++ b/package.json @@ -64,7 +64,7 @@ "lint": "bun run lint:biome && bun run lint:typecheck", "lint:git:commit": "bun run lint", "lint:git:push": "bun run lint:ci && bun run test && bun run build", - "lint:ci": "bun run lint && bun run docs:cli:check && bun run lint:markdownlint && bun run lint:cspell && bun run lint:secretlint", + "lint:ci": "bun run lint && bun run docs:cli:check && bun run explain:value-census:readme:check && bun run lint:markdownlint && bun run lint:cspell && bun run lint:secretlint", "lint:cspell": "cspell lint . --gitignore", "lint:cspell:all": "cspell lint . --no-gitignore", "lint:markdownlint": "markdownlint-cli2 \"**/*.md\" \"!.scratch/**\"", @@ -98,6 +98,9 @@ "explain:menus": "bun run scripts/gen-explain-menus.ts", "explain:menus:check": "bun run scripts/gen-explain-menus.ts --check", "explain:value-census": "bun run scripts/explain-value-census.ts", + "explain:value-census:check": "bun run scripts/explain-value-census.ts --check", + "explain:value-census:readme": "bun run scripts/explain-value-census.ts --readme", + "explain:value-census:readme:check": "bun run scripts/explain-value-census.ts --readme --check", "security": "bun run lint:secretlint && bun run lint:markdownlint" }, "type": "module", diff --git a/scripts/explain-value-census.ts b/scripts/explain-value-census.ts index c6b12cd..44faf46 100644 --- a/scripts/explain-value-census.ts +++ b/scripts/explain-value-census.ts @@ -11,15 +11,40 @@ * census is (`scripts/explain-corpus-census.ts`). * * ``` - * bun run explain:value-census # markdown - * bun run explain:value-census --json # the fixture's `corpus` block - * bun run explain:value-census --db PATH # override the corpus.sqlite location + * bun run explain:value-census # markdown + * bun run explain:value-census --json # the fixture's `corpus` block + * bun run explain:value-census --db PATH # override the corpus.sqlite location + * bun run explain:value-census:check # gate: fresh census vs the fixture + * bun run explain:value-census:readme # rewrite the README block from the fixture + * bun run explain:value-census:readme:check # gate: README block vs the fixture * ``` * * The corpus is not in this repo. A sibling `lsp-routeros-ts` checkout is used * when present, otherwise the snapshot pinned by `bun run corpus:fetch`; the * source and its sha256 are announced on stderr. See `corpus-fetch.ts` (#186). * + * ## Two gates, because the figures live in three places (#260) + * + * Committing this script made the figures **re-derivable**; it did not make + * them **checked**. #256 changed the emission, regenerated the fixture and the + * unit assertions, and shipped with the README prose still quoting the + * pre-change numbers — three review comments found that, not CI. + * + * The chain is corpus → fixture → README, and each link now has its own gate, + * separated by what data it needs: + * + * - `--check` re-runs the census and asserts it against + * `test/fixtures/explain/values.json` → `corpus`. It needs the corpus, so it + * runs in ci.yaml's `corpus` job (the one #186 gave `corpus:fetch` + * reachability for) and never in the offline `lint:ci`. + * - `--readme --check` asserts the README paragraph against that same fixture + * block. The README is now a GENERATED projection of it, so the drift that + * actually happened is caught with no corpus at all — that one is in + * `lint:ci` and in `bun test`. + * + * The fixture is the hinge: it is the one copy a human writes, and both gates + * point at it rather than at each other. + * * ## What each figure means * * - **strictComparableAnchors** — emitted values in statements the STRICT REST @@ -43,6 +68,8 @@ */ import { Database } from "bun:sqlite"; +import { readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; import { lexArguments, lexValueAnchors } from "../src/explain/args.ts"; import { valueShapeHints } from "../src/explain/values.ts"; import { resolveVerbs } from "../src/explain/verbsplit.ts"; @@ -216,7 +243,171 @@ function flag(args: readonly string[], name: string): string | undefined { return at < 0 ? undefined : args[at + 1]; } +const README_PATH = join( + import.meta.dir, + "..", + "commands", + "explain", + "README.md", +); +const FIXTURE_PATH = join( + import.meta.dir, + "..", + "test", + "fixtures", + "explain", + "values.json", +); + +/** + * The generated paragraph sits inside a list item, so both markers and every + * rendered line carry the item's two-space continuation indent. + */ +const BLOCK_INDENT = " "; +const BLOCK_BEGIN = `${BLOCK_INDENT}`; +const BLOCK_END = `${BLOCK_INDENT}`; +const WRAP_COLUMNS = 78; + +/** + * Greedy word wrap that never breaks inside a `code span`. + * + * A token is a maximal run of non-space characters in which a whole + * backtick-delimited span counts as one character, so `` `bun run x` `` and any + * punctuation glued to it stay on one line. CommonMark would in fact fold a + * newline inside a code span into a space, but a command the reader may want to + * copy should not be split across lines in the source either. + */ +function wrap(text: string): string[] { + const lines: string[] = []; + let line = BLOCK_INDENT; + for (const word of text.match(/(?:`[^`]*`|\S)+/g) ?? []) { + if (line !== BLOCK_INDENT && line.length + 1 + word.length > WRAP_COLUMNS) { + lines.push(line); + line = BLOCK_INDENT; + } + line += line === BLOCK_INDENT ? word : ` ${word}`; + } + if (line !== BLOCK_INDENT) lines.push(line); + return lines; +} + +const count = (value: number): string => value.toLocaleString("en-US"); + +/** + * Render the README's census paragraph from a census result. + * + * Every sentence stays true at any value: the invariants are reported as + * counters rather than asserted as held, so a regenerated block states a + * regression instead of quietly contradicting itself. + */ +export function renderReadmeBlock(result: ValueCensus): string[] { + const idCount = result.shapeCounts["id"] ?? 0; + const idClause = + idCount === 0 + ? "the corpus holds no source-literal `id` example" + : `${count(idCount)} are \`id\``; + return wrap( + `The corpus census is re-derivable with \`bun run explain:value-census\` and ` + + `covers ${count(result.sourceScripts)} source scripts. The figures below are generated from ` + + "`test/fixtures/explain/values.json` → `corpus` by " + + "`bun run explain:value-census:readme` and gated against it by " + + "`bun run explain:value-census:readme:check`; the fixture itself is gated " + + "against a fresh corpus run by `bun run explain:value-census:check`. Of the " + + `${count(result.strictComparableAnchors)} emitted values in a statement the strict argument lexer ` + + `ALSO read, ${count(result.boundaryContradictions)} disagree with it on half-open byte span or ` + + `decoded text, while the prefix-safe scan retains a further ${count(result.recoveredPrefixAnchors)} ` + + `values across ${count(result.unreadStatementsWithAnchors)} statements whose strict REST reading ` + + `abstains. Of ${count(result.valueOccurrences)} emitted occurrences, ` + + `${count(result.elementOccurrences)} are array members (${count(result.keyedElements)} keyed, ` + + `${count(result.nestedElements)} nested inside another member) and ` + + `${count(result.shapeCounts["array"] ?? 0)} are arrays; ${idClause}. The three structural ` + + "counters — spans addressing bytes outside their own source, members " + + "naming a container that does not exist, members escaping the container " + + `they name — read ${count(result.invalidSpans)}, ${count(result.danglingParents)} and ` + + `${count(result.containmentBreaks)}, and each must stay 0.`, + ); +} + +/** The `corpus` block of the values fixture, which the README block projects. */ +function readFixtureCensus(): ValueCensus { + const fixture = JSON.parse(readFileSync(FIXTURE_PATH, "utf8")) as { + corpus?: ValueCensus; + }; + if (fixture.corpus === undefined) + throw new Error(`${FIXTURE_PATH} has no \`corpus\` block`); + return fixture.corpus; +} + +/** + * Compare a fresh census against the committed fixture, figure by figure. + * + * `censusCommand` is the fixture's own provenance note, not a measurement, so + * it is excluded here; `explain-values.test.ts` pins it instead. + */ +export function diffAgainstFixture( + fresh: ValueCensus, + pinned: ValueCensus, +): string[] { + const measured = fresh as unknown as Record; + const committed = pinned as unknown as Record; + const render = (value: unknown): string => + typeof value === "number" ? String(value) : JSON.stringify(value); + const drift: string[] = []; + for (const key of Object.keys(measured)) { + const a = render(measured[key]); + const b = render(committed[key]); + if (a !== b) drift.push(`${key}: fixture ${b}, measured ${a}`); + } + // A figure the fixture carries and the census no longer emits is drift too: + // silently keeping it would leave the README quoting a retired definition. + for (const key of Object.keys(committed)) { + if (key in measured || key === "censusCommand") continue; + drift.push(`${key}: in the fixture, not measured`); + } + return drift; +} + +/** + * Rewrite or verify the README's generated block. Returns a process exit code. + */ +export function runReadme(check: boolean): number { + const rendered = renderReadmeBlock(readFixtureCensus()); + const readme = readFileSync(README_PATH, "utf8"); + const lines = readme.split("\n"); + const begin = lines.indexOf(BLOCK_BEGIN); + const end = lines.indexOf(BLOCK_END); + if (begin < 0 || end < begin) { + console.error( + `::error title=explain value census::commands/explain/README.md is missing the generated value-census block markers`, + ); + return 1; + } + const current = lines.slice(begin + 1, end); + if (current.join("\n") === rendered.join("\n")) { + if (!check) console.error("value-census README block already current"); + return 0; + } + if (check) { + console.error( + "::error title=explain value census::commands/explain/README.md no longer matches " + + "test/fixtures/explain/values.json → corpus. Run `bun run explain:value-census:readme`.", + ); + console.error(`--- README\n${current.join("\n")}`); + console.error(`+++ fixture\n${rendered.join("\n")}`); + return 1; + } + writeFileSync( + README_PATH, + [...lines.slice(0, begin + 1), ...rendered, ...lines.slice(end)].join("\n"), + ); + console.error("rewrote the value-census block in commands/explain/README.md"); + return 0; +} + export async function main(args: readonly string[]): Promise { + // Before any corpus resolution: the doc gate reads the fixture, so it must + // run in CI and from a bare clone, where `corpus.sqlite` does not exist. + if (args.includes("--readme")) return runReadme(args.includes("--check")); const resolution = resolveCorpusDb(flag(args, "--db")); // All of this goes to stderr, not stdout: `--json` is piped into the fixture. // The warning is emitted before the reachability check because a corrupt @@ -242,6 +433,22 @@ export async function main(args: readonly string[]): Promise { db.close(); } const result = census(scripts); + if (args.includes("--check")) { + const drift = diffAgainstFixture(result, readFixtureCensus()); + if (drift.length > 0) { + console.error( + "::error title=explain value census::the census no longer matches " + + "test/fixtures/explain/values.json → corpus. Repin with " + + "`bun run explain:value-census --json`, then " + + "`bun run explain:value-census:readme`, and update the assertions in " + + "test/unit/explain-values.test.ts.", + ); + for (const line of drift) console.error(` ${line}`); + return 1; + } + console.error("value census matches the committed fixture"); + return 0; + } // Written rather than logged: `console.log` followed by `process.exit` can // truncate a piped stdout in Bun, and this output is normally piped into a // file or `jq`. diff --git a/test/unit/explain-value-census.test.ts b/test/unit/explain-value-census.test.ts new file mode 100644 index 0000000..a8aa999 --- /dev/null +++ b/test/unit/explain-value-census.test.ts @@ -0,0 +1,115 @@ +/** + * #260 — the value census is re-derivable; these tests make it drift-gated. + * + * The census figures live in three places: `test/fixtures/explain/values.json` + * → `corpus` (the measurement), the assertions in `explain-values.test.ts` + * (which gate the fixture), and the prose in `commands/explain/README.md`. The + * third copy was ungated and drifted in #256. It is now a generated projection + * of the fixture, checked here as well as in `bun run lint:ci`, because a + * `bun test` failure is the one most people see first. + * + * Nothing here needs `corpus.sqlite`. The corpus → fixture link is gated by + * `bun run explain:value-census:check` in ci.yaml's `corpus` job; what this + * file can still do offline is prove the COMPARATOR that gate depends on + * actually reports drift. + */ + +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { + diffAgainstFixture, + renderReadmeBlock, + type ValueCensus, +} from "../../scripts/explain-value-census.ts"; + +const README = readFileSync( + new URL("../../commands/explain/README.md", import.meta.url), + "utf8", +); +const fixture = JSON.parse( + readFileSync( + new URL("../fixtures/explain/values.json", import.meta.url), + "utf8", + ), +) as { corpus: ValueCensus }; + +const BEGIN = + " "; +const END = " "; + +function readmeBlock(): string[] { + const lines = README.split("\n"); + const begin = lines.indexOf(BEGIN); + const end = lines.indexOf(END); + expect(begin).toBeGreaterThanOrEqual(0); + expect(end).toBeGreaterThan(begin); + return lines.slice(begin + 1, end); +} + +describe("#260 value-census drift gate", () => { + test("the README block is the fixture's `corpus` block, rendered", () => { + expect(readmeBlock().join("\n")).toBe( + renderReadmeBlock(fixture.corpus).join("\n"), + ); + }); + + test("the README quotes no census figure outside the generated block", () => { + // The drift that happened in #256 was a number left behind in prose. Any + // figure large enough to need a thousands separator is census-shaped, so + // none may appear outside the block that regenerates. + const block = readmeBlock().join("\n"); + const outside = README.split("\n") + .filter((line) => !block.includes(line)) + .join("\n"); + for (const figure of [ + fixture.corpus.valueOccurrences, + fixture.corpus.strictComparableAnchors, + fixture.corpus.elementOccurrences, + fixture.corpus.recoveredPrefixAnchors, + fixture.corpus.nestedElements, + ]) { + expect(outside).not.toContain(figure.toLocaleString("en-US")); + } + }); + + test("a rendered line never breaks a code span", () => { + for (const line of readmeBlock()) { + expect(line.split("`").length % 2).toBe(1); + } + }); + + test("the rendering states a regression rather than contradicting itself", () => { + // The block's sentences have to stay true at any value, or regenerating + // after a real regression would publish a reassuring lie. + const regressed = renderReadmeBlock({ + ...fixture.corpus, + boundaryContradictions: 3, + invalidSpans: 1, + danglingParents: 2, + containmentBreaks: 4, + shapeCounts: { ...fixture.corpus.shapeCounts, id: 7 }, + }) + .join(" ") + .replace(/\s+/g, " "); + expect(regressed).toContain("3 disagree with it"); + expect(regressed).toContain("read 1, 2 and 4, and each must stay 0"); + expect(regressed).toContain("7 are `id`"); + expect(regressed).not.toContain("holds no source-literal"); + }); + + test("the fixture comparator finds drift in a scalar and inside a shape map", () => { + expect(diffAgainstFixture(fixture.corpus, fixture.corpus)).toEqual([]); + // A nested `shapeCounts` change is the one a shallow compare would miss, + // and it is exactly how a changed emission shows up first. + expect( + diffAgainstFixture( + { + ...fixture.corpus, + keyedElements: fixture.corpus.keyedElements + 1, + shapeCounts: { ...fixture.corpus.shapeCounts, array: 0 }, + }, + fixture.corpus, + ).map((line) => line.split(":")[0]), + ).toEqual(["keyedElements", "shapeCounts"]); + }); +}); From 7612f1eb0279cfb55f2655781bf4f9f783132b35 Mon Sep 17 00:00:00 2001 From: SkyFi Geek <45924209+mobileskyfi@users.noreply.github.com> Date: Mon, 10 Aug 2026 22:04:12 -0700 Subject: [PATCH 2/3] =?UTF-8?q?explain:=20address=20review=20=E2=80=94=20b?= =?UTF-8?q?lock=20location,=20CRLF,=20and=20two=20doc/contract=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit's finding on the "no census figure outside the block" test was right, and worse than reported. Filtering out README lines whose text appears in the generated block used a SUBSTRING test, so it dropped every blank line (the empty string is "in" any block) plus any short line duplicating generated text: measured, 112 of 1343 lines vanished from the region the test claimed to scan. Now sliced by marker index, like the gate itself does. The test still passes over the full region, so it was not masking real drift. Copilot's CRLF finding is real here, not hypothetical: the repo ships no .gitattributes, so a Windows clone with the Git-for-Windows default core.autocrlf=true gets CRLF, every line keeps a trailing \r, and neither marker is found — the gate would fail for a line ending rather than for drift, in the unit tier #142 wants running on Windows. Both the gate and the test now split on /\r?\n/. The write path additionally rejoins with the ending the file already used: splitLines discards the \r, so writing back a bare "\n" would have silently converted a CRLF checkout to LF and put the whole README in the diff. Verified by CRLF-ifying the real README — check passes, and a forced rewrite preserves all 1342 CRLFs with zero bare LFs introduced. The remaining two were accurate reads of imprecise prose, not bugs: - wrap()'s "counts as one character" described the TOKENIZATION regex, where a backtick span is one atom in the alternation, but read as a claim about length accounting. Reworded to say which it is, and that a span longer than the wrap column overflows rather than splitting. No behavior change. - diffAgainstFixture excluded `censusCommand` only in its second loop. It cannot fire today, since a fresh ValueCensus never carries that key, but the contract was true by accident rather than by rule. Now a PROVENANCE_KEYS set skipped in both directions. Adds a CRLF regression test that asserts the fix is load-bearing (a bare "\n" split finds neither marker). Co-Authored-By: Claude Opus 5 --- scripts/explain-value-census.ts | 53 ++++++++++++++++++++------ test/unit/explain-value-census.test.ts | 38 +++++++++++++++--- 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/scripts/explain-value-census.ts b/scripts/explain-value-census.ts index 44faf46..1eb2878 100644 --- a/scripts/explain-value-census.ts +++ b/scripts/explain-value-census.ts @@ -268,14 +268,34 @@ const BLOCK_BEGIN = `${BLOCK_INDENT}`; const WRAP_COLUMNS = 78; +/** + * Split on either line ending, so a checkout with `core.autocrlf=true` — the + * Git-for-Windows default, and this repo ships no `.gitattributes` to override + * it — still finds the markers instead of failing the gate for a `\r` (#142 + * wants the unit tier on Windows). + */ +export function splitLines(text: string): string[] { + return text.split(/\r?\n/); +} + +/** The line ending a file already uses, so rewriting it does not convert one. */ +function lineEndingOf(text: string): string { + return text.includes("\r\n") ? "\r\n" : "\n"; +} + /** * Greedy word wrap that never breaks inside a `code span`. * - * A token is a maximal run of non-space characters in which a whole - * backtick-delimited span counts as one character, so `` `bun run x` `` and any - * punctuation glued to it stay on one line. CommonMark would in fact fold a - * newline inside a code span into a space, but a command the reader may want to - * copy should not be split across lines in the source either. + * Tokenization, not measurement, is what protects the span: a token is a + * maximal run in which a whole backtick-delimited span counts as ONE atom + * alongside single non-space characters, so `` `bun run x` `` and any + * punctuation glued to it arrive here as one indivisible word. Its full text + * still counts toward the line budget, so a span longer than the wrap column + * simply overflows its line rather than being split. + * + * CommonMark would in fact fold a newline inside a code span into a space, so + * splitting one renders correctly; a command the reader may want to copy should + * still not be broken in the source. */ function wrap(text: string): string[] { const lines: string[] = []; @@ -339,11 +359,14 @@ function readFixtureCensus(): ValueCensus { } /** - * Compare a fresh census against the committed fixture, figure by figure. - * - * `censusCommand` is the fixture's own provenance note, not a measurement, so - * it is excluded here; `explain-values.test.ts` pins it instead. + * Fixture keys that record provenance rather than measure anything, skipped by + * BOTH directions of the comparison so the exclusion is a stated rule and not + * an accident of `censusCommand` happening never to appear in a fresh result. + * `explain-values.test.ts` pins it instead. */ +const PROVENANCE_KEYS: ReadonlySet = new Set(["censusCommand"]); + +/** Compare a fresh census against the committed fixture, figure by figure. */ export function diffAgainstFixture( fresh: ValueCensus, pinned: ValueCensus, @@ -354,6 +377,7 @@ export function diffAgainstFixture( typeof value === "number" ? String(value) : JSON.stringify(value); const drift: string[] = []; for (const key of Object.keys(measured)) { + if (PROVENANCE_KEYS.has(key)) continue; const a = render(measured[key]); const b = render(committed[key]); if (a !== b) drift.push(`${key}: fixture ${b}, measured ${a}`); @@ -361,7 +385,7 @@ export function diffAgainstFixture( // A figure the fixture carries and the census no longer emits is drift too: // silently keeping it would leave the README quoting a retired definition. for (const key of Object.keys(committed)) { - if (key in measured || key === "censusCommand") continue; + if (key in measured || PROVENANCE_KEYS.has(key)) continue; drift.push(`${key}: in the fixture, not measured`); } return drift; @@ -373,7 +397,7 @@ export function diffAgainstFixture( export function runReadme(check: boolean): number { const rendered = renderReadmeBlock(readFixtureCensus()); const readme = readFileSync(README_PATH, "utf8"); - const lines = readme.split("\n"); + const lines = splitLines(readme); const begin = lines.indexOf(BLOCK_BEGIN); const end = lines.indexOf(BLOCK_END); if (begin < 0 || end < begin) { @@ -396,9 +420,14 @@ export function runReadme(check: boolean): number { console.error(`+++ fixture\n${rendered.join("\n")}`); return 1; } + // Rejoined with the ending the file already used: `splitLines` discards a + // `\r`, so writing back with a bare "\n" would silently convert a CRLF + // checkout to LF and put the whole README in the diff. writeFileSync( README_PATH, - [...lines.slice(0, begin + 1), ...rendered, ...lines.slice(end)].join("\n"), + [...lines.slice(0, begin + 1), ...rendered, ...lines.slice(end)].join( + lineEndingOf(readme), + ), ); console.error("rewrote the value-census block in commands/explain/README.md"); return 0; diff --git a/test/unit/explain-value-census.test.ts b/test/unit/explain-value-census.test.ts index a8aa999..67562ac 100644 --- a/test/unit/explain-value-census.test.ts +++ b/test/unit/explain-value-census.test.ts @@ -19,6 +19,7 @@ import { readFileSync } from "node:fs"; import { diffAgainstFixture, renderReadmeBlock, + splitLines, type ValueCensus, } from "../../scripts/explain-value-census.ts"; @@ -37,12 +38,18 @@ const BEGIN = " "; const END = " "; -function readmeBlock(): string[] { - const lines = README.split("\n"); +/** Marker line indices, located exactly as the gate itself locates them. */ +function markers(): { lines: string[]; begin: number; end: number } { + const lines = splitLines(README); const begin = lines.indexOf(BEGIN); const end = lines.indexOf(END); expect(begin).toBeGreaterThanOrEqual(0); expect(end).toBeGreaterThan(begin); + return { lines, begin, end }; +} + +function readmeBlock(): string[] { + const { lines, begin, end } = markers(); return lines.slice(begin + 1, end); } @@ -57,10 +64,15 @@ describe("#260 value-census drift gate", () => { // The drift that happened in #256 was a number left behind in prose. Any // figure large enough to need a thousands separator is census-shaped, so // none may appear outside the block that regenerates. - const block = readmeBlock().join("\n"); - const outside = README.split("\n") - .filter((line) => !block.includes(line)) - .join("\n"); + // + // Sliced by marker index rather than by filtering out lines whose text + // appears in the block: a substring test drops every blank line (the empty + // string is "in" any block) and any line duplicating generated text, which + // silently shrank the region this test actually scanned. + const { lines, begin, end } = markers(); + const outside = [...lines.slice(0, begin), ...lines.slice(end + 1)].join( + "\n", + ); for (const figure of [ fixture.corpus.valueOccurrences, fixture.corpus.strictComparableAnchors, @@ -72,6 +84,20 @@ describe("#260 value-census drift gate", () => { } }); + test("the block locator survives a CRLF checkout", () => { + // This repo ships no `.gitattributes`, so a Windows clone with the + // Git-for-Windows default `core.autocrlf=true` gets CRLF here — and #142 + // wants the unit tier running there. + const crlf = README.replace(/\r?\n/g, "\r\n"); + const { begin, end } = markers(); + expect(splitLines(crlf).indexOf(BEGIN)).toBe(begin); + expect(splitLines(crlf).indexOf(END)).toBe(end); + // The fix is load-bearing, not belt-and-braces: a bare "\n" split leaves a + // trailing `\r` on every line and finds neither marker, which would fail + // the gate for a line ending rather than for drift. + expect(crlf.split("\n").indexOf(BEGIN)).toBe(-1); + }); + test("a rendered line never breaks a code span", () => { for (const line of readmeBlock()) { expect(line.split("`").length % 2).toBe(1); From 1e70c7dd478e3b58a01fc72fa81bd532fbe64f21 Mon Sep 17 00:00:00 2001 From: SkyFi Geek <45924209+mobileskyfi@users.noreply.github.com> Date: Mon, 10 Aug 2026 22:16:59 -0700 Subject: [PATCH 3/3] explain: compare census tallies entry by entry, not as serialized text (#260) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex's exact-head review of 7612f1e found diffAgainstFixture treating JSON object insertion order as census data. Reproduced: reversing only the KEYS of `shapeCounts`, with every key/value pair preserved, reported drift. It is a live hazard, not a theoretical one. `shapeCounts` is built by `bump()` in first-seen order across the corpus walk, so a corpus whose scripts are visited in a different order — or a hand-tidied fixture — reorders the keys without moving a single count. And the failure mode was the worst kind: the message dumped both tallies, so a reader would have compared two 200-character blobs containing identical data and seen no difference. Tallies are now compared entry by entry over the union of their keys, which fixes the false positive and sharpens the diagnostic at the same time: before: shapeCounts: fixture {"time":128,"str":14845,…}, measured {…} after: shapeCounts.array: fixture 813, measured 814 Sensitivity is unchanged in the direction that matters — added, removed and changed entries all still report, each naming the shape that moved, and an absent side prints "absent" rather than a misleading 0. Regression test covers both halves as asked: reordered-equal maps produce no drift, while add/remove/change still do. Verified end to end that the CI gate still fails on a real injected `shapeCounts.array` change, now with the precise message. Co-Authored-By: Claude Opus 5 --- scripts/explain-value-census.ts | 37 +++++++++++++++++++++---- test/unit/explain-value-census.test.ts | 38 +++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/scripts/explain-value-census.ts b/scripts/explain-value-census.ts index 1eb2878..9081f21 100644 --- a/scripts/explain-value-census.ts +++ b/scripts/explain-value-census.ts @@ -366,20 +366,47 @@ function readFixtureCensus(): ValueCensus { */ const PROVENANCE_KEYS: ReadonlySet = new Set(["censusCommand"]); -/** Compare a fresh census against the committed fixture, figure by figure. */ +/** A per-shape tally such as `shapeCounts`, as opposed to a scalar figure. */ +function isCountMap(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** + * Compare a fresh census against the committed fixture, figure by figure. + * + * A tally is compared **entry by entry**, never as serialized text. JSON object + * order is insertion order, and `shapeCounts` is built in first-seen order + * across the corpus walk — so a corpus whose scripts are visited in a different + * order, or a hand-tidied fixture, reorders the keys without moving a single + * count. Comparing `JSON.stringify` output would call that drift and print two + * blobs a reader cannot tell apart. Entry-wise comparison also makes the + * message say which shape moved instead of dumping both tallies. + */ export function diffAgainstFixture( fresh: ValueCensus, pinned: ValueCensus, ): string[] { const measured = fresh as unknown as Record; const committed = pinned as unknown as Record; - const render = (value: unknown): string => - typeof value === "number" ? String(value) : JSON.stringify(value); const drift: string[] = []; for (const key of Object.keys(measured)) { if (PROVENANCE_KEYS.has(key)) continue; - const a = render(measured[key]); - const b = render(committed[key]); + const a = measured[key]; + const b = committed[key]; + if (isCountMap(a) || isCountMap(b)) { + const am = isCountMap(a) ? a : {}; + const bm = isCountMap(b) ? b : {}; + const names = [ + ...new Set([...Object.keys(am), ...Object.keys(bm)]), + ].sort(); + for (const name of names) { + if (am[name] === bm[name]) continue; + drift.push( + `${key}.${name}: fixture ${bm[name] ?? "absent"}, measured ${am[name] ?? "absent"}`, + ); + } + continue; + } if (a !== b) drift.push(`${key}: fixture ${b}, measured ${a}`); } // A figure the fixture carries and the census no longer emits is drift too: diff --git a/test/unit/explain-value-census.test.ts b/test/unit/explain-value-census.test.ts index 67562ac..5c879ac 100644 --- a/test/unit/explain-value-census.test.ts +++ b/test/unit/explain-value-census.test.ts @@ -136,6 +136,42 @@ describe("#260 value-census drift gate", () => { }, fixture.corpus, ).map((line) => line.split(":")[0]), - ).toEqual(["keyedElements", "shapeCounts"]); + ).toEqual(["keyedElements", "shapeCounts.array"]); + }); + + test("a tally is compared by entry, so key ORDER is not census data", () => { + // `shapeCounts` is built in first-seen order across the corpus walk, so a + // different visit order — or a hand-tidied fixture — reorders the keys + // without moving a count. Serialized comparison called that drift and + // printed two blobs a reader cannot tell apart. + const reversed = Object.fromEntries( + Object.entries(fixture.corpus.shapeCounts).reverse(), + ); + expect(Object.keys(reversed)).not.toEqual( + Object.keys(fixture.corpus.shapeCounts), + ); + expect( + diffAgainstFixture( + { ...fixture.corpus, shapeCounts: reversed }, + fixture.corpus, + ), + ).toEqual([]); + + // Order-independence must not cost sensitivity: added, removed and + // changed entries all still report, and each names the shape that moved. + const { mac: _dropped, ...withoutMac } = fixture.corpus.shapeCounts; + expect( + diffAgainstFixture( + { + ...fixture.corpus, + shapeCounts: { ...withoutMac, id: 4, num: 0 }, + }, + fixture.corpus, + ).sort(), + ).toEqual([ + "shapeCounts.id: fixture absent, measured 4", + `shapeCounts.mac: fixture ${fixture.corpus.shapeCounts["mac"]}, measured absent`, + `shapeCounts.num: fixture ${fixture.corpus.shapeCounts["num"]}, measured 0`, + ]); }); });