fix(observability): fix Sentry fingerprinting for 3 fragmented issue groups

review-enrichment/src/sentry.ts

@@ -309,7 +309,14 @@ export function captureAnalyzerDegradation(error: unknown, context: AnalyzerDegr
     release: activeRelease,
     environment: activeEnvironment,
     },
-    fingerprint: ["rees-analyzer-degraded", context.analyzer],
+    // Group by WHY (partialReason, e.g. "analyzer_timeout"), not WHICH analyzer hit it (#5010): the generic
+    // reasons genuinely share one root cause (the shared, dynamically-shrinking per-analyzer time budget)
+    // regardless of which analyzer's turn it was, so grouping by analyzer name fragmented one condition into
+    // N issues (one per analyzer) that each individually looked small. A reason that IS inherently
+    // analyzer-specific (e.g. "bundlephobia-size_http_error") stays its own issue either way, since the
+    // reason string itself already encodes that specificity -- falls back to analyzer name only on the
+    // defensive case where partialReason is somehow absent.
+    fingerprint: ["rees-analyzer-degraded", context.partialReason ?? context.analyzer],
     tags: {
       event: "rees_analyzer_degraded",
       analyzer: context.analyzer,

review-enrichment/test/sentry-degradation.test.ts

@@ -100,6 +100,49 @@ test("captureAnalyzerDegradation tags and fingerprints sanitized analyzer failur
   assert.equal(serializedContext.includes("Bearer should_never_be_attached"), false);
 });
 
+test("captureAnalyzerDegradation groups by partialReason (WHY), not analyzer name (WHICH), so the same reason from different analyzers is one issue (#5010)", () => {
+  const sentry = sentryHarness();
+
+  captureAnalyzerDegradation(new Error("analyzer_timeout"), {
+    analyzer: "installScript",
+    repoFullName: "JSONbored/gittensory",
+    prNumber: 7,
+    headSha: "abc123",
+    timeoutMs: 1400,
+    partialReason: "analyzer_timeout",
+  } as never);
+  captureAnalyzerDegradation(new Error("analyzer_timeout"), {
+    analyzer: "nativeBuild",
+    repoFullName: "JSONbored/gittensory",
+    prNumber: 8,
+    headSha: "def456",
+    timeoutMs: 1400,
+    partialReason: "analyzer_timeout",
+  } as never);
+
+  // Same fingerprint from two DIFFERENT analyzers: both group into one Sentry issue.
+  assert.deepEqual(sentry.fingerprints, [
+    ["rees-analyzer-degraded", "analyzer_timeout"],
+    ["rees-analyzer-degraded", "analyzer_timeout"],
+  ]);
+  // The specific analyzer is still fully visible via the tag -- only the GROUPING changed.
+  assert.equal(sentry.tags.analyzer, "nativeBuild");
+});
+
+test("captureAnalyzerDegradation falls back to analyzer name when partialReason is absent", () => {
+  const sentry = sentryHarness();
+
+  captureAnalyzerDegradation(new Error("boom"), {
+    analyzer: "dependency",
+    repoFullName: "JSONbored/gittensory",
+    prNumber: 7,
+    headSha: "abc123",
+    timeoutMs: 8000,
+  });
+
+  assert.deepEqual(sentry.fingerprints, [["rees-analyzer-degraded", "dependency"]]);
+});
+
 test("captureAnalyzerDegradation filters tag values before sending them", () => {
   const sentry = sentryHarness();
   const secretLikeValue = ["ghp", "abcdefghijklmnopqrstuvwxyz1234567890"].join("_");

src/selfhost/sentry.ts

@@ -444,7 +444,11 @@ function namedCaptureError(error: unknown, eventName?: string): Error {
 }
 
 /** Capture an error with optional structured context. No-op when Sentry is off. `eventName`, when given, becomes
- *  the Sentry issue title's prefix (see {@link namedCaptureError}) instead of the generic "Error". */
+ *  the Sentry issue title's prefix (see {@link namedCaptureError}) AND the grouping fingerprint (#5010) --
+ *  Sentry's default stack-trace-based grouping fragments the SAME logical failure into separate issues whenever
+ *  it is captured from more than one call site (e.g. two different functions each constructing the identical
+ *  `new Error("...")` message), which is exactly what happened to GITTENSORY-5/10 and GITTENSORY-C/W before this.
+ *  Mirrors forwardStructuredLogToSentry's identical `scope.setFingerprint(["gittensory-log", event])` discipline. */
 export function captureError(
   error: unknown,
   context?: Record<string, unknown>,
@@ -454,13 +458,15 @@ export function captureError(
   Sentry.withScope((scope) => {
     setOtelTraceScope(scope);
     if (context) { const safeContext = hashedInstallationContext(context); scope.setContext("gittensory", safeContext); applyOperationalTags(scope, safeContext); }
+    if (eventName) scope.setFingerprint(["gittensory-error", eventName]);
     Sentry!.captureException(namedCaptureError(error, eventName));
   });
 }
 
 /** Capture a failed review at ERROR level, tagged by repo/PR/SHA for triage. A review that cannot be produced is a
  *  real failure the maintainer must SEE — not a warning that hides in the noise. No-op when off. `eventName`, when
- *  given, becomes the Sentry issue title's prefix (see {@link namedCaptureError}) instead of the generic "Error". */
+ *  given, becomes the Sentry issue title's prefix AND the grouping fingerprint -- see {@link captureError}'s
+ *  identical discipline and #5010. */
 export function captureReviewFailure(
   error: unknown,
   context?: Record<string, unknown>,
@@ -475,6 +481,7 @@ export function captureReviewFailure(
       scope.setContext("review", safeContext);
       applyOperationalTags(scope, safeContext);
     }
+    if (eventName) scope.setFingerprint(["gittensory-review-failure", eventName]);
     Sentry!.captureException(namedCaptureError(error, eventName));
   });
 }

test/unit/selfhost-sentry.test.ts

@@ -578,14 +578,15 @@ describe("enabled when SENTRY_DSN is set", () => {
     expect(mocks.captureException).toHaveBeenCalledTimes(2);
   });
 
-  it("captureError with an eventName renames the captured Error so the Sentry title isn't the generic 'Error'", async () => {
+  it("captureError with an eventName renames the captured Error so the Sentry title isn't the generic 'Error', and groups it by that same name (#5010)", async () => {
     await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv);
     captureError(new Error("self-host queue processing lease expired"), { kind: "job_dead" }, "processing_timeout");
     expect(lastCapturedError().name).toBe("processing_timeout");
     expect(lastCapturedError().message).toBe("self-host queue processing lease expired");
+    expect(mocks.scope.setFingerprint).toHaveBeenCalledWith(["gittensory-error", "processing_timeout"]);
   });
 
-  it("captureError without an eventName leaves a caught exception's own name untouched", async () => {
+  it("captureError without an eventName leaves a caught exception's own name untouched, and never overrides Sentry's default grouping", async () => {
     await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv);
     class HttpError extends Error {
       constructor(message: string) {
@@ -595,12 +596,20 @@ describe("enabled when SENTRY_DSN is set", () => {
     }
     captureError(new HttpError("merge already in progress"), { kind: "agent_merge_blocked" });
     expect(lastCapturedError().name).toBe("HttpError");
+    expect(mocks.scope.setFingerprint).not.toHaveBeenCalled();
   });
 
-  it("captureReviewFailure with an eventName renames the captured Error the same way captureError does", async () => {
+  it("captureReviewFailure with an eventName renames the captured Error and groups it the same way captureError does (#5010) -- this is what consolidates the same failure captured from two different call sites (GITTENSORY-5/10, GITTENSORY-C/W) into one issue", async () => {
     await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv);
     captureReviewFailure(new Error("AI review inconclusive — no usable verdict for the PR head"), { repo: "o/r" }, "ai_review_inconclusive");
     expect(lastCapturedError().name).toBe("ai_review_inconclusive");
+    expect(mocks.scope.setFingerprint).toHaveBeenCalledWith(["gittensory-review-failure", "ai_review_inconclusive"]);
+  });
+
+  it("captureReviewFailure without an eventName never overrides Sentry's default grouping", async () => {
+    await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv);
+    captureReviewFailure(new Error("rev"), { repo: "o/r" });
+    expect(mocks.scope.setFingerprint).not.toHaveBeenCalled();
   });
 
   it("captureError/captureReviewFailure with an eventName still names a non-Error value's synthesized Error", async () => {