From e652f4409ea9de9ae596a653dcda5b53824911fd Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 09:42:02 +0800 Subject: [PATCH] fix(verify): rewrite counted baselines only when a count was lowered (#10669) * fix(verify): rewrite counted baselines only when a count was lowered * test(verify): own the baseline write decision in a tested helper --- scripts/verify/architecture-guards.mjs | 10 ++--- scripts/verify/counted-baseline.d.mts | 12 +++++ scripts/verify/counted-baseline.mjs | 26 ++++++++--- .../filesystem/countedBaseline.test.ts | 45 ++++++++++++++++++- 4 files changed, 81 insertions(+), 12 deletions(-) diff --git a/scripts/verify/architecture-guards.mjs b/scripts/verify/architecture-guards.mjs index a6e509028e..b3a8c20673 100644 --- a/scripts/verify/architecture-guards.mjs +++ b/scripts/verify/architecture-guards.mjs @@ -3,7 +3,10 @@ import { existsSync } from "node:fs"; import { readFile, readdir, writeFile } from "node:fs/promises"; import { join, relative, sep } from "node:path"; import { fileURLToPath } from "node:url"; -import { compareCountedBaseline } from "./counted-baseline.mjs"; +import { + compareCountedBaseline, + recordLoweredBaseline, +} from "./counted-baseline.mjs"; // Architecture guards for the strict-identity / best-effort-inference split. // - Composite dedup keys must not NUL-join variable-length or free-form @@ -205,10 +208,7 @@ const checkCounted = (label, key, selected, pattern, owners) => { failures.push( `${label}; new copies are not allowed:\n${lines(compared.grown)}`, ); - if (compared.baseline !== (baselines[key] ?? {})) { - baselines[key] = compared.baseline; - baselinesShrunk = true; - } + if (recordLoweredBaseline(baselines, key, compared)) baselinesShrunk = true; if (compared.stale.length > 0) failures.push( `${label}; copies were removed, so lower the baseline with \`node scripts/verify/architecture-guards.mjs --shrink-baselines\`:\n${lines(compared.stale)}`, diff --git a/scripts/verify/counted-baseline.d.mts b/scripts/verify/counted-baseline.d.mts index c44a7a97af..4a10c97d7b 100644 --- a/scripts/verify/counted-baseline.d.mts +++ b/scripts/verify/counted-baseline.d.mts @@ -14,4 +14,16 @@ export function compareCountedBaseline( readonly grown: CountedBaselineDifference[]; readonly stale: CountedBaselineDifference[]; readonly baseline: Record; + /** True only when `shrink` lowered or removed a recorded count. */ + readonly lowered: boolean; }; + +/** Record a lowered baseline under `key`; true when the map changed. */ +export function recordLoweredBaseline( + baselines: Record>, + key: string, + compared: { + readonly baseline: Record; + readonly lowered: boolean; + }, +): boolean; diff --git a/scripts/verify/counted-baseline.mjs b/scripts/verify/counted-baseline.mjs index 9098e97e66..a090377c22 100644 --- a/scripts/verify/counted-baseline.mjs +++ b/scripts/verify/counted-baseline.mjs @@ -2,7 +2,8 @@ * Compare observed per-file counts with a counted baseline. A file may not * exceed its recorded count. A count below the baseline is stale; with * `shrink`, the returned baseline lowers it (and drops zero entries) but never - * raises any count. + * raises any count; `lowered` says whether it did, so callers rewrite a + * baseline only when it changed. */ export const compareCountedBaseline = (counts, baseline, shrink) => { const grown = []; @@ -15,10 +16,23 @@ export const compareCountedBaseline = (counts, baseline, shrink) => { const count = counts.get(path) ?? 0; if (count < allowed) stale.push({ path, count, allowed }); } - if (!shrink || stale.length === 0) return { grown, stale, baseline }; - const lowered = { ...baseline }; + if (!shrink || stale.length === 0) + return { grown, stale, baseline, lowered: false }; + const shrunk = { ...baseline }; for (const { path, count } of stale) - if (count === 0) delete lowered[path]; - else lowered[path] = count; - return { grown, stale: [], baseline: lowered }; + if (count === 0) delete shrunk[path]; + else shrunk[path] = count; + return { grown, stale: [], baseline: shrunk, lowered: true }; +}; + +/** + * Record a comparison's lowered baseline under `key` in the full baseline + * map. Returns whether the map changed, so the caller rewrites the file only + * then. A key whose last count was removed is deleted rather than kept empty. + */ +export const recordLoweredBaseline = (baselines, key, compared) => { + if (!compared.lowered) return false; + if (Object.keys(compared.baseline).length === 0) delete baselines[key]; + else baselines[key] = compared.baseline; + return true; }; diff --git a/tests/boundary/filesystem/countedBaseline.test.ts b/tests/boundary/filesystem/countedBaseline.test.ts index 855027cfd3..9617ff763f 100644 --- a/tests/boundary/filesystem/countedBaseline.test.ts +++ b/tests/boundary/filesystem/countedBaseline.test.ts @@ -1,6 +1,9 @@ import { expect, it } from "vitest"; -import { compareCountedBaseline } from "../../../scripts/verify/counted-baseline.mjs"; +import { + compareCountedBaseline, + recordLoweredBaseline, +} from "../../../scripts/verify/counted-baseline.mjs"; it("rejects a file that gains copies or is missing from the baseline", () => { const compared = compareCountedBaseline( @@ -30,6 +33,19 @@ it("reports removed copies as stale until the baseline is lowered", () => { { path: "src/gone.ts", count: 0, allowed: 1 }, ]); expect(compared.baseline).toBe(baseline); + expect(compared.lowered).toBe(false); +}); + +it("leaves an absent baseline unchanged so the guard writes no empty entry", () => { + for (const shrink of [false, true]) { + const compared = compareCountedBaseline(new Map(), {}, shrink); + expect(compared).toEqual({ + grown: [], + stale: [], + baseline: {}, + lowered: false, + }); + } }); it("only lowers counts when shrinking, never raising one that grew", () => { @@ -45,5 +61,32 @@ it("only lowers counts when shrinking, never raising one that grew", () => { expect(compared.baseline).toEqual({ "src/a.ts": 1, "src/b.ts": 1 }); expect(compared.grown).toEqual([{ path: "src/b.ts", count: 5, allowed: 1 }]); expect(compared.stale).toEqual([]); + expect(compared.lowered).toBe(true); expect(baseline).toEqual({ "src/a.ts": 2, "src/b.ts": 1, "src/gone.ts": 4 }); }); + +it("leaves the baseline map unchanged for a key with no baseline", () => { + for (const shrink of [false, true]) { + const baselines: Record> = { + other: { "src/a.ts": 1 }, + }; + const compared = compareCountedBaseline( + new Map(), + baselines["new-key"] ?? {}, + shrink, + ); + expect(recordLoweredBaseline(baselines, "new-key", compared)).toBe(false); + expect(baselines).toEqual({ other: { "src/a.ts": 1 } }); + } +}); + +it("removes a key when shrinking removes its last recorded count", () => { + const gone = { "src/a.ts": 1 }; + const baselines: Record> = { + gone, + kept: { "src/b.ts": 2 }, + }; + const compared = compareCountedBaseline(new Map(), gone, true); + expect(recordLoweredBaseline(baselines, "gone", compared)).toBe(true); + expect(baselines).toEqual({ kept: { "src/b.ts": 2 } }); +});