From aa00f97b085e236bd00d23df258f62676a3fee7b Mon Sep 17 00:00:00 2001
From: Nick M <274344962+nickmopen@users.noreply.github.com>
Date: Sun, 5 Jul 2026 11:39:03 -0500
Subject: [PATCH] feat(review): deterministic changed-file classifier (#2143)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add a pure `classifyChangedFile(path)` that maps a changed file to one of five
review buckets — source / test / docs / config / generated — so the changed-files
summary (and future analytics) group deterministically. Pure + path-only (no diff
content, no IO); composes the existing hardened path-matchers.

- src/review/changed-files-classify.ts: fixed precedence generated > test > docs >
  config > source (documented), so a file matching several buckets — a generated test
  file, a lockfile, a vendored fixture — always resolves to the higher-precedence
  class. Reuses isGeneratedFile/isVendoredFile/isLockfile/isMinifiedFile,
  isTestFile/isTestPath, isDocsFile, isConfigFile.
- Distinct from path-matchers' 10-way `classifyChangedFile` (different category set
  AND precedence: that one ranks config above test), so it can't just fold it.
- test/unit/changed-files-classify.test.ts: one file per class, precedence conflict
  (vendored test file → generated), lockfile/vendored/minified/generated → generated,
  unknown → source; asserts the underlying matcher for each path.

Part of #1957. Pure function + tests only — no rendering.
---
 src/review/changed-files-classify.ts     | 32 +++++++++++++++++
 test/unit/changed-files-classify.test.ts | 46 ++++++++++++++++++++++++
 2 files changed, 78 insertions(+)
 create mode 100644 src/review/changed-files-classify.ts
 create mode 100644 test/unit/changed-files-classify.test.ts

diff --git a/src/review/changed-files-classify.ts b/src/review/changed-files-classify.ts
new file mode 100644
index 0000000000..94875d5e65
--- /dev/null
+++ b/src/review/changed-files-classify.ts
@@ -0,0 +1,32 @@
+import { isConfigFile, isDocsFile, isGeneratedFile, isLockfile, isMinifiedFile, isVendoredFile } from "../signals/path-matchers";
+import { isTestFile } from "../signals/local-branch";
+import { isTestPath } from "../signals/test-evidence";
+
+// Deterministic changed-file classifier for the review changed-files summary (#2143, part of #1957). Maps a changed
+// file PATH to exactly one of five review-oriented buckets so the summary table (and future analytics) group
+// deterministically. Pure + path-only — no diff content, no IO — composing the existing hardened path-matchers.
+//
+// NOTE: this is DISTINCT from `classifyChangedFile` in src/signals/path-matchers.ts, which returns the finer-grained
+// 10-way slop category with a DIFFERENT precedence (it ranks config above test). This review classifier deliberately
+// uses its own precedence below, so it can't just fold that one.
+
+/** The five review-summary buckets a changed file maps to. */
+export type ReviewFileClass = "source" | "test" | "docs" | "config" | "generated";
+
+/**
+ * Classify a changed file path into one review bucket. FIXED precedence — `generated > test > docs > config > source`
+ * — so a file matching several buckets (a generated test file, a lockfile, a vendored fixture) always resolves to the
+ * higher-precedence class deterministically:
+ *  - `generated`: machine-produced/imported output (generated markers, vendored trees, lockfiles, minified bundles) —
+ *    never real hand-authored effort, so it outranks everything.
+ *  - `test`, `docs`, `config`: the remaining recognized categories, in that order.
+ *  - `source`: anything unrecognized (including plain code) falls through here.
+ * Pure.
+ */
+export function classifyChangedFile(path: string): ReviewFileClass {
+  if (isGeneratedFile(path) || isVendoredFile(path) || isLockfile(path) || isMinifiedFile(path)) return "generated";
+  if (isTestFile(path) || isTestPath(path)) return "test";
+  if (isDocsFile(path)) return "docs";
+  if (isConfigFile(path)) return "config";
+  return "source";
+}
diff --git a/test/unit/changed-files-classify.test.ts b/test/unit/changed-files-classify.test.ts
new file mode 100644
index 0000000000..fedaf30607
--- /dev/null
+++ b/test/unit/changed-files-classify.test.ts
@@ -0,0 +1,46 @@
+import { describe, expect, it } from "vitest";
+import { classifyChangedFile } from "../../src/review/changed-files-classify";
+import { isConfigFile, isDocsFile, isGeneratedFile, isLockfile, isMinifiedFile, isVendoredFile } from "../../src/signals/path-matchers";
+import { isTestFile } from "../../src/signals/local-branch";
+
+describe("classifyChangedFile (#2143)", () => {
+  it("source: plain hand-authored code that matches no other bucket", () => {
+    expect(classifyChangedFile("src/app.ts")).toBe("source");
+  });
+
+  it("test: a test file", () => {
+    expect(isTestFile("src/app.test.ts")).toBe(true);
+    expect(classifyChangedFile("src/app.test.ts")).toBe("test");
+  });
+
+  it("docs: a markdown doc", () => {
+    expect(isDocsFile("docs/guide.md")).toBe(true);
+    expect(classifyChangedFile("docs/guide.md")).toBe("docs");
+  });
+
+  it("config: a config file", () => {
+    expect(isConfigFile(".eslintrc.json")).toBe(true);
+    expect(classifyChangedFile(".eslintrc.json")).toBe("config");
+  });
+
+  it("generated: generated / vendored / lockfile / minified all fold to generated", () => {
+    expect(isGeneratedFile("src/api.generated.ts")).toBe(true);
+    expect(classifyChangedFile("src/api.generated.ts")).toBe("generated");
+    expect(isLockfile("package-lock.json")).toBe(true);
+    expect(classifyChangedFile("package-lock.json")).toBe("generated");
+    expect(isVendoredFile("vendor/jquery.js")).toBe(true);
+    expect(classifyChangedFile("vendor/jquery.js")).toBe("generated");
+    expect(isMinifiedFile("dist/app.min.js")).toBe(true);
+    expect(classifyChangedFile("dist/app.min.js")).toBe("generated");
+  });
+
+  it("precedence: a vendored file that is ALSO a test → generated (generated > test)", () => {
+    expect(isTestFile("vendor/foo.test.js")).toBe(true); // it is a test file
+    expect(isVendoredFile("vendor/foo.test.js")).toBe(true); // and vendored
+    expect(classifyChangedFile("vendor/foo.test.js")).toBe("generated"); // generated outranks test
+  });
+
+  it("unknown → source (the 5-bucket set has no 'other')", () => {
+    expect(classifyChangedFile("assets/logo.bin")).toBe("source");
+  });
+});