Harden Grok review output handling

apps/server/src/review/GrokReviewModel.ts

@@ -22,6 +22,51 @@ export const MAX_REVIEW_LIST_ENTRY_CHARS = 300;
 export const MAX_REVIEW_DELEGATION_CHARS = 800;
 export const MAX_REVIEW_DELEGATION_PATHS = 8;
 
+/**
+ * Grok's constrained output is intentionally decoded through a permissive wire
+ * shape first. Real models still occasionally emit null for optional fields or
+ * exceed a presentation bound by a few characters. Rejecting the whole agent
+ * result for those repairable differences made otherwise useful reviews
+ * disappear, so normalization below restores the strict canonical shape.
+ */
+const GrokReviewCandidateFindingWire = Schema.Struct({
+  severity: Schema.Literals(["blocker", "high", "medium", "low"]),
+  confidence: Schema.Number,
+  category: Schema.String,
+  title: Schema.String,
+  path: Schema.String,
+  startLine: Schema.optionalKey(Schema.NullOr(Schema.Number)),
+  endLine: Schema.optionalKey(Schema.NullOr(Schema.Number)),
+  evidence: Schema.String,
+  impact: Schema.String,
+  suggestion: Schema.String,
+  verification: Schema.optionalKey(Schema.NullOr(Schema.String)),
+});
+
+const GrokReviewDelegationWire = Schema.Struct({
+  objective: Schema.String,
+  paths: Schema.Array(Schema.String),
+});
+
+export const GrokReviewCandidateWire = Schema.Struct({
+  summary: Schema.String,
+  findings: Schema.Array(GrokReviewCandidateFindingWire),
+  coverage: Schema.Array(Schema.String),
+  limitations: Schema.Array(Schema.String),
+  delegation: Schema.optionalKey(Schema.NullOr(GrokReviewDelegationWire)),
+});
+export type GrokReviewCandidateWire = typeof GrokReviewCandidateWire.Type;
+
+export const GrokReviewVerificationWire = Schema.Struct({
+  summary: Schema.String,
+  findings: Schema.Array(GrokReviewCandidateFindingWire),
+  coverage: Schema.Array(Schema.String),
+  limitations: Schema.Array(Schema.String),
+  needsHighEffortReview: Schema.optionalKey(Schema.Boolean),
+  escalationReason: Schema.optionalKey(Schema.NullOr(Schema.String)),
+});
+export type GrokReviewVerificationWire = typeof GrokReviewVerificationWire.Type;
+
 const boundedNonEmpty = (maxLength: number) =>
   TrimmedNonEmptyString.check(Schema.isMaxLength(maxLength));
 const PositiveLine = Schema.Int.check(Schema.isGreaterThan(0));
@@ -84,6 +129,111 @@ export const GrokReviewVerification = Schema.Struct({
 });
 export type GrokReviewVerification = typeof GrokReviewVerification.Type;
 
+function boundedText(value: string, maxLength: number): string {
+  const trimmed = value.trim();
+  return trimmed.length <= maxLength ? trimmed : trimmed.slice(0, maxLength).trimEnd();
+}
+
+function boundedStrings(values: ReadonlyArray<string>): Array<string> {
+  return [
+    ...new Set(
+      values.map((value) => boundedText(value, MAX_REVIEW_LIST_ENTRY_CHARS)).filter(Boolean),
+    ),
+  ].slice(0, MAX_REVIEW_LIST_ENTRIES);
+}
+
+function positiveLine(value: number | null | undefined): number | undefined {
+  return typeof value === "number" && Number.isInteger(value) && value > 0 ? value : undefined;
+}
+
+function normalizeCandidateFinding(
+  finding: typeof GrokReviewCandidateFindingWire.Type,
+): GrokReviewCandidateFinding | null {
+  const category = boundedText(finding.category, MAX_REVIEW_CATEGORY_CHARS);
+  const title = boundedText(finding.title, MAX_REVIEW_TITLE_CHARS);
+  const path = boundedText(finding.path, MAX_REVIEW_PATH_CHARS);
+  const evidence = boundedText(finding.evidence, MAX_REVIEW_EVIDENCE_CHARS);
+  const impact = boundedText(finding.impact, MAX_REVIEW_IMPACT_CHARS);
+  const suggestion = boundedText(finding.suggestion, MAX_REVIEW_SUGGESTION_CHARS);
+  if (!category || !title || !path || !evidence || !impact || !suggestion) return null;
+
+  const startLine = positiveLine(finding.startLine);
+  const candidateEndLine = positiveLine(finding.endLine);
+  const endLine =
+    startLine !== undefined && candidateEndLine !== undefined && candidateEndLine >= startLine
+      ? candidateEndLine
+      : undefined;
+  const verification = finding.verification
+    ? boundedText(finding.verification, MAX_REVIEW_VERIFICATION_CHARS)
+    : undefined;
+
+  return {
+    severity: finding.severity,
+    confidence: Math.max(0, Math.min(1, finding.confidence)),
+    category,
+    title,
+    path,
+    ...(startLine === undefined ? {} : { startLine }),
+    ...(endLine === undefined ? {} : { endLine }),
+    evidence,
+    impact,
+    suggestion,
+    ...(verification ? { verification } : {}),
+  };
+}
+
+function normalizedFindings(
+  findings: ReadonlyArray<typeof GrokReviewCandidateFindingWire.Type>,
+): Array<GrokReviewCandidateFinding> {
+  return findings
+    .flatMap((finding) => {
+      const normalized = normalizeCandidateFinding(finding);
+      return normalized ? [normalized] : [];
+    })
+    .slice(0, MAX_REVIEW_FINDINGS);
+}
+
+export function normalizeGrokReviewCandidate(
+  candidate: GrokReviewCandidateWire,
+): GrokReviewCandidate {
+  const objective = candidate.delegation
+    ? boundedText(candidate.delegation.objective, MAX_REVIEW_DELEGATION_CHARS)
+    : "";
+  const delegationPaths = candidate.delegation
+    ? candidate.delegation.paths
+        .map((value) => boundedText(value, MAX_REVIEW_PATH_CHARS))
+        .filter(Boolean)
+        .slice(0, MAX_REVIEW_DELEGATION_PATHS)
+    : [];
+  return {
+    summary:
+      boundedText(candidate.summary, MAX_REVIEW_SUMMARY_CHARS) ||
+      "The reviewer returned no summary.",
+    findings: normalizedFindings(candidate.findings),
+    coverage: boundedStrings(candidate.coverage),
+    limitations: boundedStrings(candidate.limitations),
+    delegation: objective ? { objective, paths: delegationPaths } : null,
+  };
+}
+
+export function normalizeGrokReviewVerification(
+  verification: GrokReviewVerificationWire,
+): GrokReviewVerification {
+  const escalationReason = verification.escalationReason
+    ? boundedText(verification.escalationReason, MAX_REVIEW_DELEGATION_CHARS)
+    : null;
+  return {
+    summary:
+      boundedText(verification.summary, MAX_REVIEW_SUMMARY_CHARS) ||
+      "The verifier returned no summary.",
+    findings: normalizedFindings(verification.findings),
+    coverage: boundedStrings(verification.coverage),
+    limitations: boundedStrings(verification.limitations),
+    needsHighEffortReview: verification.needsHighEffortReview ?? false,
+    escalationReason,
+  };
+}
+
 const severityRank: Readonly<Record<GrokReviewSeverity, number>> = {
   blocker: 0,
   high: 1,

apps/server/src/review/GrokReviewSwarm.test.ts

@@ -1,6 +1,6 @@
 import * as NodeServices from "@effect/platform-node/NodeServices";
 import { describe, expect, it } from "@effect/vitest";
-import { GrokReviewInput, type ReviewDiffPreviewSource } from "@t3tools/contracts";
+import { GrokReviewError, GrokReviewInput, type ReviewDiffPreviewSource } from "@t3tools/contracts";
 import * as Effect from "effect/Effect";
 import * as Schema from "effect/Schema";
 
@@ -339,6 +339,107 @@ describe("buildVerificationPrompt", () => {
 });
 
 describe("runGrokReviewSwarm", () => {
+  it.effect("normalizes repairable Grok output before building the canonical report", () => {
+    const wireVerification = {
+      summary: `  ${"summary ".repeat(400)}  `,
+      findings: [
+        {
+          ...verifiedFinding,
+          confidence: 1.4,
+          title: "t".repeat(500),
+          startLine: null,
+          endLine: null,
+          verification: null,
+        },
+      ],
+      coverage: Array.from({ length: 12 }, (_, index) => `${index}-${"c".repeat(400)}`),
+      limitations: [],
+      needsHighEffortReview: false,
+      escalationReason: null,
+    };
+    const agent: GrokReviewAgent = {
+      resolvedModel: "grok-4.5",
+      grokBuildVersion: "0.2.112",
+      run: (request) =>
+        Effect.succeed(
+          (request.prompt.includes("medium-effort verifier")
+            ? wireVerification
+            : {
+                ...emptyCandidate(),
+                delegation: undefined,
+              }) as never,
+        ),
+    };
+
+    return Effect.gen(function* () {
+      const report = yield* runGrokReviewSwarm({
+        request: { cwd: process.cwd(), target: "working-tree" },
+        source,
+        agent,
+      });
+
+      expect(report.summary.length).toBeLessThanOrEqual(2_000);
+      expect(report.coverage).toHaveLength(8);
+      expect(report.findings[0]).toMatchObject({
+        confidence: 1,
+        inlineEligible: false,
+      });
+      expect(report.findings[0]!.title).toHaveLength(240);
+      expect(report.findings[0]).not.toHaveProperty("startLine");
+      expect(report.findings[0]).not.toHaveProperty("verification");
+    }).pipe(Effect.provide(NodeServices.layer));
+  });
+
+  it.effect("retries failed medium verification at high effort", () => {
+    const calls: Array<{ readonly effort: string; readonly prompt: string }> = [];
+    const highVerification: GrokReviewVerification = {
+      summary: "The high-effort fallback completed the verification.",
+      findings: [],
+      coverage: ["Correctness", "Security"],
+      limitations: [],
+      needsHighEffortReview: false,
+      escalationReason: null,
+    };
+    const agent: GrokReviewAgent = {
+      resolvedModel: "grok-4.5",
+      grokBuildVersion: "0.2.112",
+      run: (request) => {
+        calls.push({ effort: request.effort, prompt: request.prompt });
+        if (request.prompt.includes("medium-effort verifier")) {
+          return Effect.fail(
+            new GrokReviewError({
+              operation: "GrokReviewAgent.decode",
+              detail: "The medium verifier returned malformed output.",
+            }),
+          );
+        }
+        if (request.prompt.includes("high-effort final")) {
+          return Effect.succeed(highVerification as never);
+        }
+        return Effect.succeed(emptyCandidate() as never);
+      },
+    };
+
+    return Effect.gen(function* () {
+      const report = yield* runGrokReviewSwarm({
+        request: { cwd: process.cwd(), target: "working-tree" },
+        source,
+        agent,
+      });
+
+      expect(report.escalatedToHigh).toBe(true);
+      expect(report.limitations).toContain(
+        "Medium-effort verification failed; the high-effort fallback completed.",
+      );
+      expect(report.usage).toEqual({
+        agentRuns: 6,
+        mediumEffortRuns: 5,
+        highEffortRuns: 1,
+      });
+      expect(calls.filter(({ effort }) => effort === "high")).toHaveLength(1);
+    }).pipe(Effect.provide(NodeServices.layer));
+  });
+
   it.effect("uses medium by default and escalates an ambiguous high finding", () => {
     const calls: Array<{ readonly effort: string; readonly prompt: string }> = [];
     const mediumVerification: GrokReviewVerification = {