Merge pull request #34 from mvanhorn/fix/dedupe-stale-file-errors

fix(staleness): collapse day + commit thresholds into a single issue

Author: theDakshJaitly

src/drift/checkers/staleness.ts

@@ -1,57 +1,92 @@
 import { daysSinceLastChange, commitsSinceLastChange } from "../../git.js";
-import type { DriftIssue } from "../../types.js";
+import type { DriftIssue, Severity } from "../../types.js";
 
 const WARN_DAYS = 30;
 const ERROR_DAYS = 90;
 const WARN_COMMITS = 50;
 const ERROR_COMMITS = 200;
 
-/** Check how stale a scaffold file is based on git history */
-export async function checkStaleness(
-  filePath: string,
-  source: string,
-  cwd: string
-): Promise<DriftIssue[]> {
-  const issues: DriftIssue[] = [];
+type StaleSignal = { severity: Severity; message: string };
 
-  const days = await daysSinceLastChange(filePath, cwd);
-  const commits = await commitsSinceLastChange(filePath, cwd);
-
-  if (days !== null && days >= ERROR_DAYS) {
-    issues.push({
-      code: "STALE_FILE",
+function daysSignal(days: number): StaleSignal | null {
+  if (days >= ERROR_DAYS) {
+    return {
       severity: "error",
-      file: source,
-      line: null,
       message: `File hasn't been updated in ${days} days (threshold: ${ERROR_DAYS}d)`,
-    });
-  } else if (days !== null && days >= WARN_DAYS) {
-    issues.push({
-      code: "STALE_FILE",
+    };
+  }
+  if (days >= WARN_DAYS) {
+    return {
       severity: "warning",
-      file: source,
-      line: null,
       message: `File hasn't been updated in ${days} days (threshold: ${WARN_DAYS}d)`,
-    });
+    };
   }
+  return null;
+}
 
-  if (commits !== null && commits >= ERROR_COMMITS) {
-    issues.push({
-      code: "STALE_FILE",
+function commitsSignal(commits: number): StaleSignal | null {
+  if (commits >= ERROR_COMMITS) {
+    return {
       severity: "error",
-      file: source,
-      line: null,
       message: `${commits} commits since file was last updated (threshold: ${ERROR_COMMITS})`,
-    });
-  } else if (commits !== null && commits >= WARN_COMMITS) {
-    issues.push({
-      code: "STALE_FILE",
+    };
+  }
+  if (commits >= WARN_COMMITS) {
+    return {
       severity: "warning",
-      file: source,
-      line: null,
       message: `${commits} commits since file was last updated (threshold: ${WARN_COMMITS})`,
-    });
+    };
+  }
+  return null;
+}
+
+const SEVERITY_RANK: Record<Severity, number> = {
+  info: 0,
+  warning: 1,
+  error: 2,
+};
+
+/**
+ * Check how stale a scaffold file is based on git history.
+ *
+ * When both the day threshold and the commit threshold are exceeded, this
+ * returns a single combined issue at the higher of the two severities —
+ * two STALE_FILE issues on the same file are the same underlying condition
+ * and should cost the score once, not twice.
+ */
+export async function checkStaleness(
+  filePath: string,
+  source: string,
+  cwd: string
+): Promise<DriftIssue[]> {
+  const days = await daysSinceLastChange(filePath, cwd);
+  const commits = await commitsSinceLastChange(filePath, cwd);
+
+  const signals: StaleSignal[] = [];
+  if (days !== null) {
+    const s = daysSignal(days);
+    if (s) signals.push(s);
+  }
+  if (commits !== null) {
+    const s = commitsSignal(commits);
+    if (s) signals.push(s);
   }
 
-  return issues;
+  if (signals.length === 0) return [];
+
+  const severity = signals.reduce<Severity>(
+    (acc, s) => (SEVERITY_RANK[s.severity] > SEVERITY_RANK[acc] ? s.severity : acc),
+    signals[0].severity
+  );
+  const message = signals.map((s) => s.message).join("; ");
+
+  return [
+    {
+      code: "STALE_FILE",
+      severity,
+      file: source,
+      line: null,
+      message,
+    },
+  ];
 }

test/checkers.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
 import {
   mkdtempSync,
   writeFileSync,
@@ -15,6 +15,13 @@ import { checkCrossFile } from "../src/drift/checkers/cross-file.js";
 import { checkIndexSync } from "../src/drift/checkers/index-sync.js";
 import type { Claim, ScaffoldFrontmatter } from "../src/types.js";
 
+vi.mock("../src/git.js", () => ({
+  daysSinceLastChange: vi.fn(),
+  commitsSinceLastChange: vi.fn(),
+}));
+const gitMock = await import("../src/git.js");
+const { checkStaleness } = await import("../src/drift/checkers/staleness.js");
+
 let tmpDir: string;
 
 beforeEach(() => {
@@ -299,3 +306,65 @@ describe("checkIndexSync", () => {
     expect(issues).toHaveLength(0);
   });
 });
+
+// ── Staleness Checker ──
+
+describe("checkStaleness", () => {
+  const daysFn = gitMock.daysSinceLastChange as unknown as ReturnType<typeof vi.fn>;
+  const commitsFn = gitMock.commitsSinceLastChange as unknown as ReturnType<typeof vi.fn>;
+
+  beforeEach(() => {
+    daysFn.mockReset();
+    commitsFn.mockReset();
+  });
+
+  it("returns no issues when both thresholds are clean", async () => {
+    daysFn.mockResolvedValue(10);
+    commitsFn.mockResolvedValue(5);
+    const issues = await checkStaleness("file.md", "source.md", ".");
+    expect(issues).toHaveLength(0);
+  });
+
+  it("returns a single issue when only the day threshold is exceeded", async () => {
+    daysFn.mockResolvedValue(100);
+    commitsFn.mockResolvedValue(5);
+    const issues = await checkStaleness("file.md", "source.md", ".");
+    expect(issues).toHaveLength(1);
+    expect(issues[0].severity).toBe("error");
+    expect(issues[0].message).toContain("100 days");
+  });
+
+  it("collapses day + commit thresholds into a single compound issue", async () => {
+    daysFn.mockResolvedValue(100);
+    commitsFn.mockResolvedValue(250);
+    const issues = await checkStaleness("file.md", "source.md", ".");
+    expect(issues).toHaveLength(1);
+    expect(issues[0].code).toBe("STALE_FILE");
+    expect(issues[0].severity).toBe("error");
+    expect(issues[0].message).toContain("100 days");
+    expect(issues[0].message).toContain("250 commits");
+  });
+
+  it("uses the higher severity when one threshold is warning and the other error", async () => {
+    daysFn.mockResolvedValue(40);
+    commitsFn.mockResolvedValue(250);
+    const issues = await checkStaleness("file.md", "source.md", ".");
+    expect(issues).toHaveLength(1);
+    expect(issues[0].severity).toBe("error");
+  });
+
+  it("keeps warning severity when neither threshold reaches error", async () => {
+    daysFn.mockResolvedValue(40);
+    commitsFn.mockResolvedValue(60);
+    const issues = await checkStaleness("file.md", "source.md", ".");
+    expect(issues).toHaveLength(1);
+    expect(issues[0].severity).toBe("warning");
+  });
+
+  it("returns empty when git history is unavailable", async () => {
+    daysFn.mockResolvedValue(null);
+    commitsFn.mockResolvedValue(null);
+    const issues = await checkStaleness("file.md", "source.md", ".");
+    expect(issues).toHaveLength(0);
+  });
+});