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..9081f21 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,227 @@ 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; + +/** + * 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`. + * + * 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[] = []; + 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; +} + +/** + * 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"]); + +/** 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 drift: string[] = []; + for (const key of Object.keys(measured)) { + if (PROVENANCE_KEYS.has(key)) continue; + 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: + // silently keeping it would leave the README quoting a retired definition. + for (const key of Object.keys(committed)) { + if (key in measured || PROVENANCE_KEYS.has(key)) 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 = splitLines(readme); + 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; + } + // 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( + lineEndingOf(readme), + ), + ); + 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 +489,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..5c879ac --- /dev/null +++ b/test/unit/explain-value-census.test.ts @@ -0,0 +1,177 @@ +/** + * #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, + splitLines, + 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 = " "; + +/** 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); +} + +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. + // + // 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, + fixture.corpus.elementOccurrences, + fixture.corpus.recoveredPrefixAnchors, + fixture.corpus.nestedElements, + ]) { + expect(outside).not.toContain(figure.toLocaleString("en-US")); + } + }); + + 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); + } + }); + + 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.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`, + ]); + }); +});