fix(review): persist AI review diagnostics into cache metadata on degraded runs (#9447)

* fix(review): persist AI review diagnostics into cache metadata on degraded runs

`ai_review_cache.metadata_json` records `inconclusive` — that a review degraded, but never why.
The per-attempt `AiReviewDiagnostic` values that carry the answer were only ever sent to PostHog
as a capture property, so the 2026-07-27 investigation into missing review summaries could not
distinguish the failure classes from the database or the container logs, and reaching PostHog
needs an interactive OAuth an incident responder may not have.

Persist the compact `model#attempt:status[:error]` strings alongside `inconclusive`, so
`missing_assessment` (the model returned no narrative), `unparseable_output`/`empty_output`
(it returned something parseModelReview could not read) and `provider_error` are separable at
read time — three different bugs with three different fixes.

Written on the degraded paths only, never on the healthy one: every byte lands in a D1 row and
this database hit its 10 GB ceiling on 2026-07-26, so this buys diagnosability for the reviews
that actually failed rather than growing all of them. The strings come from
formatReviewDiagnosticsForCapture, whose `error` field is errorMessage() output and never raw
provider text, so the public/private boundary is unchanged.

Refs #9432

* fix(review): move the v8 ignore hint onto its own line so the type-level nullish arm is excluded

codecov/patch flagged the `?? []` on the diagnostics spread as a partial: the ignore comment
shared a line with the ternary's `?`, where the v8 provider does not register it. Match the
working precedent at the PostHog capture site -- comment on its own line directly above the
property -- and the branch drops out of the report. No behavior change.

Author: JSONbored

src/queue/ai-review-orchestration.ts

@@ -987,23 +987,44 @@ export async function runAiReviewForAdvisory(
       }, "ai_review_inconclusive");
     }
     args.advisory.findings.push(...findings);
+    // #9432: `degraded` persists the per-attempt review diagnostics into `ai_review_cache.metadata_json`.
+    // Until now these existed ONLY as a PostHog capture property, which made the "why did this review produce
+    // no summary?" question unanswerable from the database or the container logs -- the 2026-07-27
+    // investigation had to stop there, because reaching PostHog needs an interactive OAuth the incident
+    // responder may not have. The diagnostics are the discriminator that matters: `missing_assessment` (the
+    // model genuinely returned no narrative) vs `unparseable_output`/`empty_output` (it returned something
+    // parseModelReview could not read, i.e. no balanced JSON object) vs `provider_error` are three different
+    // bugs with three different fixes, and `metadata_json.inconclusive` alone cannot tell them apart.
+    //
+    // Written on the DEGRADED paths only, never on the healthy one. Every byte here lands in a D1 row, and
+    // the 2026-07-26 outage was this database hitting its 10 GB ceiling -- so this buys diagnosability for
+    // the small minority of reviews that actually failed, rather than growing all of them. The compact
+    // `model#attempt:status[:error]` strings come from formatReviewDiagnosticsForCapture, whose `error` field
+    // is errorMessage() output and never raw provider text, so the public/private boundary is unchanged.
     const metadataFor = (
       notes: string | null | undefined,
       inlineFindings: InlineFinding[],
+      degraded?: boolean,
     ): Record<string, unknown> => ({
       rag: attributeReviewRagTelemetry(ragTelemetry, {
         notes,
         findings,
         inlineFindings,
       }),
+      ...(degraded
+        ? {
+            /* v8 ignore next -- current review runner always supplies diagnostics for completed AI attempts; the `?? []` is a type-level fallback for the optional field. */
+            reviewDiagnostics: formatReviewDiagnosticsForCapture(result.reviewDiagnostics ?? []),
+          }
+        : {}),
     });
     if (result.inconclusive && hasPublicReviewAssessment(result.advisoryNotes)) {
       return {
         notes: result.advisoryNotes!,
         reviewerCount: result.reviewerCount,
         inlineFindings: [],
         findings,
-        metadata: metadataFor(result.advisoryNotes, []),
+        metadata: metadataFor(result.advisoryNotes, [], true),
         cacheable: false,
         valueAssessment: result.valueAssessment ?? undefined,
       };
@@ -1026,7 +1047,7 @@ export async function runAiReviewForAdvisory(
         reviewerCount: result.reviewerCount,
         inlineFindings: [],
         findings,
-        metadata: metadataFor(null, []),
+        metadata: metadataFor(null, [], true),
         cacheable: false,
       };
     }
@@ -1068,7 +1089,7 @@ export async function runAiReviewForAdvisory(
       reviewerCount: result.reviewerCount,
       inlineFindings: [],
       findings,
-      metadata: metadataFor(null, []),
+      metadata: metadataFor(null, [], true),
       cacheable: false,
     };
   } catch (error) {

test/unit/ai-review-advisory.test.ts

@@ -814,6 +814,46 @@ describe("runAiReviewForAdvisory", () => {
     captureSpy.mockRestore();
   });
 
+  it("#9432: persists the review diagnostics into metadata when no public notes were produced", async () => {
+    // The whole point: `metadata_json.inconclusive` says a review degraded but not WHY, and the diagnostics
+    // used to reach PostHog only -- unreadable during an incident without an interactive OAuth. An empty
+    // provider response must now be recoverable from the row itself as `empty_output`, which is what
+    // distinguishes "the model returned nothing" from "the model returned a narrative we then dropped".
+    const result = await runAiReviewForAdvisory(aiEnv(async () => ({ response: "" })), {
+      mode: "live",
+      settings: { aiReviewMode: "advisory" } as RepositorySettings,
+      advisory: advisory(),
+      repoFullName: "acme/widgets",
+      pr,
+      author: "alice",
+      confirmedContributor: true,
+    });
+    const diagnostics = result?.metadata?.reviewDiagnostics as string[] | undefined;
+    expect(Array.isArray(diagnostics)).toBe(true);
+    expect(diagnostics?.length).toBeGreaterThan(0);
+    // Compact `model#attempt:status` form from formatReviewDiagnosticsForCapture -- the status is the payload.
+    expect(diagnostics?.every((entry) => /^[^#]+#\d+:/.test(entry))).toBe(true);
+    expect(diagnostics?.join("\n")).toContain("empty_output");
+  });
+
+  it("#9432: omits the review diagnostics from metadata on a healthy review", async () => {
+    // The other side of the `degraded` branch. Diagnostics are deliberately NOT written on the success path:
+    // every byte lands in a D1 row, and this database hit its 10 GB ceiling on 2026-07-26. Diagnosability is
+    // bought only for the reviews that actually failed.
+    const result = await runAiReviewForAdvisory(aiEnv(async () => ({ response: defectJson() })), {
+      mode: "live",
+      settings: { aiReviewMode: "advisory" } as RepositorySettings,
+      advisory: advisory(),
+      repoFullName: "acme/widgets",
+      pr,
+      author: "alice",
+      confirmedContributor: true,
+    });
+    expect(result?.notes).toBeTruthy();
+    expect(result?.metadata).toBeDefined();
+    expect(result?.metadata?.reviewDiagnostics).toBeUndefined();
+  });
+
   it("#8790: stops retrying a model after a byte-identical repeat of a failed attempt (deterministic at temperature 0)", async () => {
     const adv = advisory();
     let aiCalls = 0;