fix(findings): stabilize evidence signatures

src/findings.test.ts

@@ -0,0 +1,127 @@
+import { describe, expect, it } from "vitest";
+import { findingFromOutput } from "./findings.js";
+import type { ReviewOutput } from "./types.js";
+
+describe("findingFromOutput", () => {
+  it("keeps signatures stable for equivalent evidence with different key insertion order", () => {
+    const orderedEvidence = {
+      path: "src/app.ts",
+      startLine: 10,
+      endLine: 12,
+      symbol: "runWorkflow",
+      quote: "await runWorkflow();",
+    };
+    const reorderedEvidence = {} as Record<string, unknown>;
+    reorderedEvidence["quote"] = "await runWorkflow();";
+    reorderedEvidence["symbol"] = "runWorkflow";
+    reorderedEvidence["endLine"] = 12;
+    reorderedEvidence["startLine"] = 10;
+    reorderedEvidence["path"] = "src/app.ts";
+
+    const first = findingFromOutput(finding([orderedEvidence]), "feature_1", "run_1");
+    const second = findingFromOutput(
+      finding([reorderedEvidence as ReviewOutput["findings"][number]["evidence"][number]]),
+      "feature_1",
+      "run_1",
+    );
+
+    expect(second.signature).toBe(first.signature);
+    expect(second.findingId).toBe(first.findingId);
+  });
+
+  it("keeps signatures stable for equivalent evidence with different reference order", () => {
+    const first = findingFromOutput(
+      finding([
+        {
+          path: "src/app.ts",
+          startLine: 10,
+          endLine: 12,
+          symbol: "runWorkflow",
+          quote: "await runWorkflow();",
+        },
+        {
+          path: "src/provider.ts",
+          startLine: 20,
+          endLine: 22,
+          symbol: null,
+          quote: null,
+        },
+      ]),
+      "feature_1",
+      "run_1",
+    );
+    const second = findingFromOutput(
+      finding([
+        {
+          path: "src/provider.ts",
+          startLine: 20,
+          endLine: 22,
+          symbol: null,
+          quote: null,
+        },
+        {
+          path: "src/app.ts",
+          startLine: 10,
+          endLine: 12,
+          symbol: "runWorkflow",
+          quote: "await runWorkflow();",
+        },
+      ]),
+      "feature_1",
+      "run_1",
+    );
+
+    expect(second.signature).toBe(first.signature);
+    expect(second.findingId).toBe(first.findingId);
+  });
+
+  it("keeps signatures distinct when evidence content changes", () => {
+    const first = findingFromOutput(
+      finding([
+        {
+          path: "src/app.ts",
+          startLine: 10,
+          endLine: 12,
+          symbol: "runWorkflow",
+          quote: "await runWorkflow();",
+        },
+      ]),
+      "feature_1",
+      "run_1",
+    );
+    const second = findingFromOutput(
+      finding([
+        {
+          path: "src/app.ts",
+          startLine: 10,
+          endLine: 12,
+          symbol: "runWorkflow",
+          quote: "await runWorkflowSafely();",
+        },
+      ]),
+      "feature_1",
+      "run_1",
+    );
+
+    expect(second.signature).not.toBe(first.signature);
+    expect(second.findingId).not.toBe(first.findingId);
+  });
+});
+
+function finding(
+  evidence: ReviewOutput["findings"][number]["evidence"],
+): ReviewOutput["findings"][number] {
+  return {
+    title: "Finding title",
+    category: "bug",
+    severity: "medium",
+    confidence: "high",
+    evidence,
+    reasoning: "Reasoning.",
+    reproduction: null,
+    recommendation: "Recommendation.",
+    whyTestsDoNotAlreadyCoverThis: "No regression coverage exists.",
+    suggestedRegressionTest: null,
+    minimumFixScope: "Keep the fix narrow.",
+  };
+}

src/findings.ts

@@ -50,7 +50,7 @@ export function findingFromOutput(
     featureId,
     finding.category,
     finding.title,
-    JSON.stringify(finding.evidence),
+    canonicalEvidence(finding.evidence),
   ]);
   const now = nowIso();
   return {
@@ -78,3 +78,67 @@ export function findingFromOutput(
     updatedAt: now,
   };
 }
+
+type CanonicalEvidenceRef = {
+  path: string;
+  startLine: number | null;
+  endLine: number | null;
+  symbol: string | null;
+  quote: string | null;
+};
+
+function canonicalEvidence(finding: ReviewOutput["findings"][number]["evidence"]): string {
+  return JSON.stringify(
+    finding
+      .map(
+        (evidence): CanonicalEvidenceRef => ({
+          path: evidence.path,
+          startLine: evidence.startLine,
+          endLine: evidence.endLine,
+          symbol: evidence.symbol,
+          quote: evidence.quote,
+        }),
+      )
+      .toSorted(compareCanonicalEvidence),
+  );
+}
+
+function compareCanonicalEvidence(left: CanonicalEvidenceRef, right: CanonicalEvidenceRef): number {
+  return (
+    compareStrings(left.path, right.path) ||
+    compareNullableNumbers(left.startLine, right.startLine) ||
+    compareNullableNumbers(left.endLine, right.endLine) ||
+    compareNullableStrings(left.symbol, right.symbol) ||
+    compareNullableStrings(left.quote, right.quote)
+  );
+}
+
+function compareNullableNumbers(left: number | null, right: number | null): number {
+  if (left === right) {
+    return 0;
+  }
+  if (left === null) {
+    return -1;
+  }
+  if (right === null) {
+    return 1;
+  }
+  return left - right;
+}
+
+function compareNullableStrings(left: string | null, right: string | null): number {
+  if (left === right) {
+    return 0;
+  }
+  if (left === null) {
+    return -1;
+  }
+  if (right === null) {
+    return 1;
+  }
+  return compareStrings(left, right);
+}
+
+function compareStrings(left: string, right: string): number {
+  return left < right ? -1 : left > right ? 1 : 0;
+}