feat(review): enrich Gerrit review agent context

src/connectors/gerritSshReviewProvider.ts

@@ -7,7 +7,7 @@
  */
 import { execFile } from "node:child_process";
 import { promisify } from "node:util";
-import { mkdtemp, rm } from "node:fs/promises";
+import { mkdir, mkdtemp, rm } from "node:fs/promises";
 import { join } from "node:path";
 import { z } from "zod";
 import {
@@ -105,7 +105,7 @@ export interface GerritSshReviewProviderConfig {
   reviewerAccountId?: string | undefined;
   /**
    * Base directory for temporary git checkouts (used to compute diffs).
-   * Defaults to /tmp/virtual-engineer/review-diffs.
+   * Defaults to /tmp/ve-review-diffs.
    */
   workspaceBaseDir?: string | undefined;
 }
@@ -250,6 +250,7 @@ export class GerritSshReviewProvider implements ReviewProvider {
     const changeRef = `refs/changes/${nn}/${details.changeNumber}/${ps}`;
 
     const baseDir = this.config.workspaceBaseDir ?? "/tmp/ve-review-diffs";
+    await mkdir(baseDir, { recursive: true });
     const dir = await mkdtemp(join(baseDir, `diff-${details.changeNumber}-`));
 
     try {
@@ -295,7 +296,76 @@ export class GerritSshReviewProvider implements ReviewProvider {
     }
   }
 
-  /** Post inline comments together with a Code-Review vote via SSH `gerrit review --json`. */
+  /**
+   * Diff two patchsets of the same change (`fromPatchset` → `toPatchset`) by
+   * fetching both refs and comparing their tips. Used on a re-review to surface
+   * what changed since VE last reviewed.
+   */
+  async getInterPatchsetDiff(
+    details: ReviewChangeDetails,
+    fromPatchset: number,
+    toPatchset: number
+  ): Promise<ReviewChangeDiff> {
+    const baseDir = this.config.workspaceBaseDir ?? "/tmp/ve-review-diffs";
+    await mkdir(baseDir, { recursive: true });
+    const dir = await mkdtemp(join(baseDir, `delta-${details.changeNumber}-`));
+
+    try {
+      const sshUrl = `ssh://${this.config.sshUser}@${this.config.sshHost}:${this.config.sshPort}/${details.project}`;
+      const hostKeyOpts = buildSshHostKeyOptions(this.config.sshKnownHostsPath).join(" ");
+      const sshCmd = `ssh -p ${this.config.sshPort} -i ${this.config.sshKeyPath} ${hostKeyOpts}`;
+      const gitEnv = {
+        ...process.env,
+        GIT_TERMINAL_PROMPT: "0",
+        GIT_SSH_COMMAND: sshCmd,
+      };
+
+      await execFileAsync("git", ["init"], { cwd: dir, timeout: GIT_TIMEOUT_MS });
+      await execFileAsync("git", ["remote", "add", "origin", sshUrl], { cwd: dir, timeout: GIT_TIMEOUT_MS });
+
+      const fromSha = await this.fetchPatchsetTip(dir, gitEnv, details.changeNumber, fromPatchset);
+      const toSha = await this.fetchPatchsetTip(dir, gitEnv, details.changeNumber, toPatchset);
+
+      // Tree-level comparison between the two patchset tips. This does not need
+      // shared history, so shallow (depth=1) fetches of each ref are sufficient.
+      const { stdout: diffOutput } = await execFileAsync(
+        "git",
+        ["diff", `${fromSha}..${toSha}`, "--no-color", "--unified=5"],
+        { cwd: dir, timeout: GIT_TIMEOUT_MS, maxBuffer: 10 * 1024 * 1024 }
+      );
+
+      const files = parseDiffOutput(diffOutput);
+      log.info(
+        { changeId: details.changeId, fromPatchset, toPatchset, fileCount: files.length },
+        "computed inter-patchset diff from SSH fetch"
+      );
+      return { changeId: details.changeId, patchset: toPatchset, files };
+    } finally {
+      await rm(dir, { recursive: true, force: true }).catch(() => undefined);
+    }
+  }
+
+  /** Shallow-fetch a single patchset ref and return its tip SHA. */
+  private async fetchPatchsetTip(
+    dir: string,
+    gitEnv: NodeJS.ProcessEnv,
+    changeNumber: number,
+    patchset: number
+  ): Promise<string> {
+    const nn = String(changeNumber % 100).padStart(2, "0");
+    const ref = `refs/changes/${nn}/${changeNumber}/${patchset}`;
+    await execFileAsync("git", ["fetch", "--depth=1", "origin", ref], {
+      cwd: dir,
+      timeout: GIT_TIMEOUT_MS,
+      env: gitEnv,
+    });
+    const { stdout } = await execFileAsync("git", ["rev-parse", "FETCH_HEAD"], {
+      cwd: dir,
+      timeout: GIT_TIMEOUT_MS,
+    });
+    return stdout.trim();
+  }
+
   async postReviewWithComments(
     changeId: ExternalChangeId,
     revision: number,
@@ -533,4 +603,3 @@ function groupCommentsByFile(
   }
   return grouped;
 }
-

src/interfaces.ts

@@ -842,6 +842,18 @@ export interface ReviewProvider {
   /** Fetch the diff for a specific patchset (defaults to current). */
   getChangeDiff(changeId: ExternalChangeId, patchset?: number): Promise<ReviewChangeDiff>;
 
+  /**
+   * Fetch the diff between two patchsets of the same change (`fromPatchset` →
+   * `toPatchset`). Used on a re-review to surface "what changed since my last
+   * review". Optional — providers that cannot compute an inter-patchset diff
+   * omit it and the orchestrator falls back to the full diff only.
+   */
+  getInterPatchsetDiff?(
+    details: ReviewChangeDetails,
+    fromPatchset: number,
+    toPatchset: number
+  ): Promise<ReviewChangeDiff>;
+
   /**
    * Post inline comments and a summary on the given revision.
    *

src/review/reviewOrchestrator.ts

@@ -20,7 +20,7 @@ import {
   makeTaskId,
   makeTicketId,
 } from "../interfaces.js";
-import { buildReviewPrompt } from "./reviewPromptBuilder.js";
+import { buildReviewPrompt, type SinceLastReviewDelta } from "./reviewPromptBuilder.js";
 import { computeVote, parseReviewResult } from "./reviewResultParser.js";
 import { filterCommentsByAllowedFiles } from "./commentFilter.js";
 import { computeCommentHash, computeThreadReplyHash } from "./commentHash.js";
@@ -453,6 +453,36 @@ export class ReviewOrchestrator {
         }
       }
 
+      // On a re-review, surface what changed since the patchset VE last
+      // reviewed so the agent focuses new findings on the delta. Guarded:
+      // providers without inter-patchset diff support skip this entirely, and
+      // empty deltas are dropped by the prompt builder.
+      let sinceLastReview: SinceLastReviewDelta | undefined;
+      const previousReviewed = task.reviewedPatchset;
+      if (
+        previousReviewed !== null &&
+        previousReviewed < details.currentPatchset &&
+        typeof this.deps.reviewProvider.getInterPatchsetDiff === "function"
+      ) {
+        try {
+          const deltaDiff = await this.deps.reviewProvider.getInterPatchsetDiff(
+            details,
+            previousReviewed,
+            details.currentPatchset
+          );
+          sinceLastReview = {
+            fromPatchset: previousReviewed,
+            toPatchset: details.currentPatchset,
+            diff: deltaDiff,
+          };
+        } catch (err) {
+          log.warn(
+            { err, taskId, fromPatchset: previousReviewed, toPatchset: details.currentPatchset },
+            "failed to compute inter-patchset delta; continuing with full diff only"
+          );
+        }
+      }
+
       const prompt = buildReviewPrompt({
         details,
         diff,
@@ -468,6 +498,7 @@ export class ReviewOrchestrator {
           : {}),
         ...(eligibleThreads.length > 0 ? { discussionThreads: eligibleThreads } : {}),
         ...(this.deps.maxDiffChars !== undefined ? { maxDiffChars: this.deps.maxDiffChars } : {}),
+        ...(sinceLastReview !== undefined ? { sinceLastReview } : {}),
       });
 
       emitReviewEvent("review.prompt_built", { promptLength: prompt.length, patchset: details.currentPatchset });
@@ -601,19 +632,20 @@ export class ReviewOrchestrator {
         repliesToPost.push({ threadId: reply.threadId, message, handledHash: entry.handledHash });
       }
 
-      // Avoid re-posting an identical verdict on re-reviews. When a pass
-      // finds nothing new to say (no inline comments, no folded notes, no
-      // replies) and the overall vote matches the last review cycle, stay
-      // silent instead of spamming another summary + vote notification.
+      // Avoid re-posting an identical verdict on re-reviews. When a pass finds
+      // nothing new to say (no inline comments, no folded notes) and the overall
+      // vote matches the last review cycle, stay silent instead of spamming
+      // another summary + vote notification. This gate is decoupled from
+      // discussion replies: a pending reply is always delivered through its own
+      // path below and never forces the verdict to be re-posted.
       const hasNothingNew = commentsToPost.length === 0 && folded.length === 0;
       const previousVote = await this.getLastReviewVote(taskId);
       const skipPosting =
         options?.force !== true &&
         cycleNumber > 1 &&
         hasNothingNew &&
         previousVote !== null &&
-        previousVote === vote &&
-        repliesToPost.length === 0;
+        previousVote === vote;
 
       emitReviewEvent("review.posting_comments", {
         commentCount: commentsToPost.length,

src/review/reviewPromptBuilder.ts

@@ -8,8 +8,11 @@ import type { ReviewChangeDetails, ReviewChangeDiff, ReviewDiscussionThread } fr
 
 /** Default upper bound on diff size injected into the prompt to avoid token blow-ups. */
 const DEFAULT_MAX_DIFF_CHARS = 60_000;
+const DEFAULT_MAX_COMMIT_MESSAGE_CHARS = 8_000;
 const TRUNCATION_NOTE =
   "\n\n[... diff truncated to fit in the model context — review the rest from the repository if needed ...]";
+const COMMIT_MESSAGE_TRUNCATION_NOTE =
+  "\n\n[... commit message truncated to fit in the model context ...]";
 
 export interface ReviewPromptInput {
   details: ReviewChangeDetails;
@@ -28,6 +31,23 @@ export interface ReviewPromptInput {
    * `threadId` the agent must echo back in its `replies[]` output to address it.
    */
   discussionThreads?: ReviewDiscussionThread[] | undefined;
+  /**
+   * On a re-review, the diff between the last patchset VE reviewed and the
+   * current one. Surfaced as a focused "what changed since my last review"
+   * section so the agent concentrates new findings on the delta while still
+   * seeing the full change. Omitted on the first review pass.
+   */
+  sinceLastReview?: SinceLastReviewDelta | undefined;
+}
+
+/** Inter-patchset delta surfaced to the agent on a re-review. */
+export interface SinceLastReviewDelta {
+  /** Patchset VE last reviewed. */
+  fromPatchset: number;
+  /** Current patchset under review. */
+  toPatchset: number;
+  /** Diff between `fromPatchset` and `toPatchset`. */
+  diff: ReviewChangeDiff;
 }
 
 /** A previously-posted review comment surfaced back to the agent as memory. */
@@ -39,7 +59,15 @@ export interface PriorReviewComment {
 
 /** Build the full prompt sent to the review agent for a single change. */
 export function buildReviewPrompt(input: ReviewPromptInput): string {
-  const { details, diff, userPrompt, maxDiffChars, priorComments, discussionThreads } = input;
+  const { details, diff, userPrompt, maxDiffChars, priorComments, discussionThreads, sinceLastReview } =
+    input;
+
+  const effectiveMax = maxDiffChars ?? DEFAULT_MAX_DIFF_CHARS;
+  const hasDelta = sinceLastReview !== undefined && sinceLastReview.diff.files.length > 0;
+  // When both a delta section and a full diff are present, split the budget
+  // equally so the combined diff content stays within effectiveMax chars.
+  const deltaBudget = hasDelta ? Math.floor(effectiveMax / 2) : effectiveMax;
+  const fullDiffBudget = hasDelta ? effectiveMax - deltaBudget : effectiveMax;
 
   const header = [
     `# Code Review Task`,
@@ -55,14 +83,24 @@ export function buildReviewPrompt(input: ReviewPromptInput): string {
     .map((f) => `- ${f.status.toUpperCase().padEnd(8)} ${f.path}`)
     .join("\n");
 
-  const diffSections = renderDiffSections(diff, maxDiffChars ?? DEFAULT_MAX_DIFF_CHARS);
+  const diffSections = renderDiffSections(diff, fullDiffBudget);
 
   const sections = [
     header,
+  ];
+
+  // Commit message body ("the why"). Omitted when the change has no description
+  // beyond its subject so we never inject an empty section.
+  const description = details.description.trim();
+  if (description.length > 0) {
+    sections.push(``, `## Commit message`, truncateCommitMessage(description));
+  }
+
+  sections.push(
     ``,
     `## User Instructions`,
     userPrompt,
-  ];
+  );
 
   if (priorComments && priorComments.length > 0) {
     sections.push(``, `## Already reported (do not repeat)`, renderPriorComments(priorComments));
@@ -76,6 +114,14 @@ export function buildReviewPrompt(input: ReviewPromptInput): string {
     );
   }
 
+  if (sinceLastReview && sinceLastReview.diff.files.length > 0) {
+    sections.push(
+      ``,
+      `## Changes since last reviewed patchset (PS ${sinceLastReview.fromPatchset} \u2192 ${sinceLastReview.toPatchset})`,
+      renderSinceLastReview(sinceLastReview, deltaBudget),
+    );
+  }
+
   sections.push(
     ``,
     `## Files in this patchset`,
@@ -152,3 +198,26 @@ function renderDiffSections(diff: ReviewChangeDiff, maxDiffChars: number): strin
   }
   return parts.join("\n\n");
 }
+
+function truncateCommitMessage(description: string): string {
+  if (description.length <= DEFAULT_MAX_COMMIT_MESSAGE_CHARS) return description;
+  const bodyLimit = DEFAULT_MAX_COMMIT_MESSAGE_CHARS - COMMIT_MESSAGE_TRUNCATION_NOTE.length;
+  return description.slice(0, bodyLimit) + COMMIT_MESSAGE_TRUNCATION_NOTE;
+}
+
+/**
+ * Render the inter-patchset delta as a guidance note followed by the delta diff.
+ * Caps the diff with the same per-section budget as the full diff. Rebase noise
+ * (upstream changes pulled in between patchsets) may appear here; the note tells
+ * the agent to treat it as such.
+ */
+function renderSinceLastReview(delta: SinceLastReviewDelta, maxDiffChars: number): string {
+  return [
+    "These are the changes between the patchset you last reviewed and the current",
+    "one. Focus genuinely new findings on this delta. If the change was rebased,",
+    "some hunks here may be upstream churn rather than author edits — judge",
+    "accordingly. The full change is still provided below for context.",
+    "",
+    renderDiffSections(delta.diff, maxDiffChars),
+  ].join("\n");
+}