ci: tolerate inherited TypeScript LOC drift (#106042)

* ci: reconcile LOC baseline drift safely

* ci: tolerate pre-existing LOC drift

scripts/check-ts-max-loc.ts

@@ -97,6 +97,10 @@ export function countPhysicalLines(content: string): number {
   return content.endsWith("\n") ? splitCount - 1 : splitCount;
 }
 
+export function splitNullDelimitedPaths(output: string): string[] {
+  return output.split("\0").filter(Boolean);
+}
+
 async function countLines(filePath: string): Promise<number> {
   const content = await readFile(filePath, "utf8");
   return countPhysicalLines(content);
@@ -151,21 +155,23 @@ export function findLocRatchetViolations(params: {
   );
 }
 
-export function findLocBaselineUpdateViolations(params: {
+export function findVersionedBaselineViolations(params: {
   baseline: LocBaseline;
-  maxLines: number;
-  results: LocResult[];
+  baseBaseline: LocBaseline;
+  baseLinesByChangedPath: ReadonlyMap<string, number | undefined>;
 }): LocRatchetViolation[] {
   const violations: LocRatchetViolation[] = [];
-  for (const result of params.results) {
-    if (result.lines <= params.maxLines) {
+  for (const [filePath, lines] of Object.entries(params.baseline)) {
+    if (!params.baseLinesByChangedPath.has(filePath)) {
+      // Unchanged source may reconcile base drift; the current-tree check below still requires
+      // this baseline to equal the file's actual LOC, so arbitrary inflation remains stale.
       continue;
     }
-    const baselineLines = params.baseline[result.filePath];
-    if (baselineLines === undefined) {
-      violations.push({ ...result, reason: "baseline-missing" });
-    } else if (result.lines > baselineLines) {
-      violations.push({ ...result, baselineLines, reason: "grew" });
+    const baseLines = params.baseLinesByChangedPath.get(filePath);
+    if (baseLines === undefined && params.baseBaseline[filePath] === undefined) {
+      violations.push({ filePath, lines, reason: "baseline-missing" });
+    } else if (baseLines === undefined || lines > baseLines) {
+      violations.push({ filePath, lines, baselineLines: baseLines ?? 0, reason: "grew" });
     }
   }
   return violations.toSorted(
@@ -173,21 +179,16 @@ export function findLocBaselineUpdateViolations(params: {
   );
 }
 
-export function findVersionedBaselineViolations(params: {
+export function filterPreexistingBaseDriftViolations(params: {
   baseline: LocBaseline;
   baseBaseline: LocBaseline;
+  changedPaths: ReadonlySet<string>;
+  violations: LocRatchetViolation[];
 }): LocRatchetViolation[] {
-  const violations: LocRatchetViolation[] = [];
-  for (const [filePath, lines] of Object.entries(params.baseline)) {
-    const baselineLines = params.baseBaseline[filePath];
-    if (baselineLines === undefined) {
-      violations.push({ filePath, lines, reason: "baseline-missing" });
-    } else if (lines > baselineLines) {
-      violations.push({ filePath, lines, baselineLines, reason: "grew" });
-    }
-  }
-  return violations.toSorted(
-    (left, right) => right.lines - left.lines || left.filePath.localeCompare(right.filePath),
+  return params.violations.filter(
+    (violation) =>
+      params.changedPaths.has(violation.filePath) ||
+      params.baseline[violation.filePath] !== params.baseBaseline[violation.filePath],
   );
 }
 
@@ -254,6 +255,54 @@ function readBaselineAtRef(
   return content === undefined ? undefined : parseBaseline(content, `${baseRef}:${baselinePath}`);
 }
 
+function readFileAtRef(baseRef: string, filePath: string): string | undefined {
+  try {
+    return execFileSync("git", ["show", `${baseRef}:${filePath}`], {
+      encoding: "utf8",
+      stdio: ["ignore", "pipe", "ignore"],
+    });
+  } catch {
+    return undefined;
+  }
+}
+
+function readChangedPaths(baseRef: string): ReadonlySet<string> {
+  const changedPaths = new Set(
+    splitNullDelimitedPaths(
+      execFileSync("git", ["diff", "--name-only", "-z", baseRef, "--"], {
+        encoding: "utf8",
+      }),
+    ),
+  );
+  for (const filePath of splitNullDelimitedPaths(
+    execFileSync("git", ["ls-files", "--others", "--exclude-standard", "-z"], {
+      encoding: "utf8",
+    }),
+  )) {
+    changedPaths.add(filePath);
+  }
+  return changedPaths;
+}
+
+function readBaseLinesForChangedBaselinePaths(
+  baseRef: string,
+  baseline: LocBaseline,
+  changedPaths: ReadonlySet<string>,
+): ReadonlyMap<string, number | undefined> {
+  const baseLinesByChangedPath = new Map<string, number | undefined>();
+  for (const filePath of Object.keys(baseline)) {
+    if (!changedPaths.has(filePath)) {
+      continue;
+    }
+    const baseContent = readFileAtRef(baseRef, filePath);
+    baseLinesByChangedPath.set(
+      filePath,
+      baseContent === undefined ? undefined : countPhysicalLines(baseContent),
+    );
+  }
+  return baseLinesByChangedPath;
+}
+
 function buildBaseline(results: LocResult[], maxLines: number): LocBaseline {
   return Object.fromEntries(
     results
@@ -288,35 +337,59 @@ export async function main(argv = process.argv.slice(2)): Promise<number> {
   );
 
   if (writeBaseline) {
-    const baseline = await readBaseline(baselinePath);
     const comparisonBaseRef = resolveComparisonBaseRef(baselinePath, baseRef);
     if (!comparisonBaseRef) {
       throw new Error("Unable to resolve a comparison ref for the TypeScript LOC baseline update");
     }
     const baseBaseline = readBaselineAtRef(comparisonBaseRef, baselinePath);
+    const updatedBaseline = buildBaseline(results, maxLines);
+    const changedPaths = readChangedPaths(comparisonBaseRef);
     // A missing baseline at a valid base ref is the one-time initialization path.
-    const violations = [
-      ...(baseBaseline ? findVersionedBaselineViolations({ baseline, baseBaseline }) : []),
-      ...findLocBaselineUpdateViolations({ baseline, maxLines, results }),
-    ];
+    const violations = baseBaseline
+      ? findVersionedBaselineViolations({
+          baseline: updatedBaseline,
+          baseBaseline,
+          baseLinesByChangedPath: readBaseLinesForChangedBaselinePaths(
+            comparisonBaseRef,
+            updatedBaseline,
+            changedPaths,
+          ),
+        })
+      : [];
     reportViolations(violations);
     if (violations.length > 0) {
       return 1;
     }
-    const updatedBaseline = buildBaseline(results, maxLines);
     await writeFile(baselinePath, `${JSON.stringify(updatedBaseline, null, 2)}\n`, "utf8");
     writeStdoutLine(`updated ${baselinePath} (${Object.keys(updatedBaseline).length} files)`);
     return 0;
   }
 
   const baseline = await readBaseline(baselinePath);
-  const baseBaseline = readBaselineAtRef(
-    resolveComparisonBaseRef(baselinePath, baseRef),
-    baselinePath,
-  );
+  const comparisonBaseRef = resolveComparisonBaseRef(baselinePath, baseRef);
+  const baseBaseline = readBaselineAtRef(comparisonBaseRef, baselinePath);
+  const changedPaths = comparisonBaseRef ? readChangedPaths(comparisonBaseRef) : new Set<string>();
+  const currentViolations = findLocRatchetViolations({ baseline, maxLines, results });
   const violations = [
-    ...(baseBaseline ? findVersionedBaselineViolations({ baseline, baseBaseline }) : []),
-    ...findLocRatchetViolations({ baseline, maxLines, results }),
+    ...(baseBaseline && comparisonBaseRef
+      ? findVersionedBaselineViolations({
+          baseline,
+          baseBaseline,
+          baseLinesByChangedPath: readBaseLinesForChangedBaselinePaths(
+            comparisonBaseRef,
+            baseline,
+            changedPaths,
+          ),
+        })
+      : []),
+    ...(baseBaseline
+      ? filterPreexistingBaseDriftViolations({
+          baseline,
+          baseBaseline,
+          changedPaths,
+          violations: currentViolations,
+        })
+      : currentViolations),
   ];
   reportViolations(violations);
   return violations.length === 0 ? 0 : 1;

test/scripts/check-ts-max-loc.test.ts

@@ -3,11 +3,12 @@ import { spawnSync } from "node:child_process";
 import { describe, expect, it } from "vitest";
 import {
   countPhysicalLines,
-  findLocBaselineUpdateViolations,
+  filterPreexistingBaseDriftViolations,
   findLocRatchetViolations,
   findVersionedBaselineViolations,
   isProductionTypeScriptFile,
   parseArgs,
+  splitNullDelimitedPaths,
 } from "../../scripts/check-ts-max-loc.js";
 
 function runCheckTsMaxLoc(args: string[]) {
@@ -93,6 +94,14 @@ describe("scripts/check-ts-max-loc", () => {
     expect(countPhysicalLines("one\ntwo\n")).toBe(2);
   });
 
+  it("parses git paths without quoting or newline ambiguity", () => {
+    expect(splitNullDelimitedPaths("src/normal.ts\0src/café.ts\0src/line\nbreak.ts\0")).toEqual([
+      "src/normal.ts",
+      "src/café.ts",
+      "src/line\nbreak.ts",
+    ]);
+  });
+
   it("excludes repository test and test-support naming conventions", () => {
     expect(isProductionTypeScriptFile("src/runtime.ts")).toBe(true);
     expect(isProductionTypeScriptFile("src/runtime.mts")).toBe(true);
@@ -117,44 +126,86 @@ describe("scripts/check-ts-max-loc", () => {
     expect(isProductionTypeScriptFile("ui/src/i18n/lib/translate.ts")).toBe(true);
   });
 
-  it("allows baseline updates only for decreases and removals", () => {
-    const violations = findLocBaselineUpdateViolations({
-      maxLines: 500,
-      baseline: {
+  it("allows base drift reconciliation but rejects changed-source growth", () => {
+    const violations = findVersionedBaselineViolations({
+      baseBaseline: {
         "src/grew.ts": 700,
+        "src/readded.ts": 700,
         "src/shrank.ts": 700,
-        "src/removed.ts": 700,
+        "src/preexisting-drift.ts": 700,
       },
-      results: [
-        { filePath: "src/grew.ts", lines: 701 },
-        { filePath: "src/shrank.ts", lines: 650 },
-        { filePath: "src/new.ts", lines: 501 },
+      baseline: {
+        "src/grew.ts": 701,
+        "src/readded.ts": 650,
+        "src/shrank.ts": 650,
+        "src/new.ts": 501,
+        "src/preexisting-drift.ts": 710,
+        "src/preexisting-missing.ts": 510,
+      },
+      baseLinesByChangedPath: new Map([
+        ["src/grew.ts", 700],
+        ["src/readded.ts", undefined],
+        ["src/shrank.ts", 700],
+        ["src/new.ts", undefined],
+      ]),
+    });
+
+    expect(violations).toEqual([
+      { filePath: "src/grew.ts", lines: 701, baselineLines: 700, reason: "grew" },
+      { filePath: "src/readded.ts", lines: 650, baselineLines: 0, reason: "grew" },
+      { filePath: "src/new.ts", lines: 501, reason: "baseline-missing" },
+    ]);
+  });
+
+  it("ignores only drift already present on the comparison base", () => {
+    const violations = filterPreexistingBaseDriftViolations({
+      baseBaseline: {
+        "src/changed.ts": 700,
+        "src/preexisting-growth.ts": 700,
+        "src/preexisting-shrink.ts": 700,
+        "src/removed-entry.ts": 700,
+      },
+      baseline: {
+        "src/changed.ts": 700,
+        "src/preexisting-growth.ts": 700,
+        "src/preexisting-shrink.ts": 700,
+        "src/tampered.ts": 900,
+      },
+      changedPaths: new Set(["src/changed.ts"]),
+      violations: [
+        { filePath: "src/changed.ts", lines: 701, baselineLines: 700, reason: "grew" },
+        {
+          filePath: "src/preexisting-growth.ts",
+          lines: 710,
+          baselineLines: 700,
+          reason: "grew",
+        },
+        {
+          filePath: "src/preexisting-shrink.ts",
+          lines: 690,
+          baselineLines: 700,
+          reason: "baseline-stale",
+        },
+        { filePath: "src/preexisting-new.ts", lines: 510, reason: "baseline-missing" },
+        { filePath: "src/removed-entry.ts", lines: 700, reason: "baseline-missing" },
+        {
+          filePath: "src/tampered.ts",
+          lines: 700,
+          baselineLines: 900,
+          reason: "baseline-stale",
+        },
       ],
     });
 
     expect(violations).toEqual([
-      { filePath: "src/grew.ts", lines: 701, baselineLines: 700, reason: "grew" },
-      { filePath: "src/new.ts", lines: 501, reason: "baseline-missing" },
-    ]);
-  });
-
-  it("rejects versioned baseline additions and increases", () => {
-    const violations = findVersionedBaselineViolations({
-      baseBaseline: {
-        "src/grew.ts": 700,
-        "src/shrank.ts": 700,
-        "src/removed.ts": 700,
+      { filePath: "src/changed.ts", lines: 701, baselineLines: 700, reason: "grew" },
+      { filePath: "src/removed-entry.ts", lines: 700, reason: "baseline-missing" },
+      {
+        filePath: "src/tampered.ts",
+        lines: 700,
+        baselineLines: 900,
+        reason: "baseline-stale",
       },
-      baseline: {
-        "src/grew.ts": 701,
-        "src/shrank.ts": 650,
-        "src/new.ts": 501,
-      },
-    });
-
-    expect(violations).toEqual([
-      { filePath: "src/grew.ts", lines: 701, baselineLines: 700, reason: "grew" },
-      { filePath: "src/new.ts", lines: 501, reason: "baseline-missing" },
     ]);
   });
 });