Stop reporting "no matching diff" for code the model was never shown

src/ai/prompt.ts

@@ -283,16 +283,28 @@ Generate a walkthrough of this PR.${poemNote}`;
 
 const CHAT_SYSTEM = `You are DiffSentry, an AI code review assistant. You are responding to a question or comment about a pull request.
 
-You have full context of the PR including the title, description, changed files, and diffs. Answer the user's question helpfully and concisely. Use markdown formatting. If they ask about specific code, reference the relevant files and lines.
+You have the PR's title, description, changed files, and diffs. Answer the user's question helpfully and concisely. Use markdown formatting. If they ask about specific code, reference the relevant files and lines.
+
+The diffs shown may be incomplete: large patches can be truncated and whole files can be omitted to fit a size budget, and both are labelled where that happens. Treat what you see as a partial view — never infer from a file's or a line's absence that the change does not exist. If answering would require code you weren't shown, say so instead of concluding it is missing.
 
 If you don't know the answer or the question is outside the scope of the PR, say so honestly.`;
 
 export function buildChatPrompt(
   context: PRContext,
   userMessage: string
 ): { system: string; user: string } {
+  // Honor the diff budget like the review and walkthrough builders do. Chat
+  // callers that set no budget are unaffected; drift detection sends the full
+  // PR file set and relies on this to stay inside the model window.
+  const budget = context.diffBudget;
   const filesSection = context.files
-    .map((f) => `### ${f.filename} (${f.status}, +${f.additions} -${f.deletions})\n\`\`\`diff\n${f.patch}\n\`\`\``)
+    .filter((f) => !budget?.byFile[f.filename]?.omitted)
+    .map((f) => {
+      const budgeted = budget?.byFile[f.filename];
+      const patch = budgeted ? budgeted.patch : f.patch;
+      const truncNote = budgeted?.truncated ? " _(patch truncated to fit the size budget)_" : "";
+      return `### ${f.filename} (${f.status}, +${f.additions} -${f.deletions})${truncNote}\n\`\`\`diff\n${patch}\n\`\`\``;
+    })
     .join("\n\n");
 
   const user = `## PR Context: ${context.title}

src/drift.ts

@@ -21,6 +21,12 @@ export type DriftFinding = {
 export async function detectDescriptionDrift(opts: {
   ai: AIProvider;
   context: PRContext;
+  /** Filenames that are part of the PR but whose diffs are absent from the
+   *  prompt (ignored by config, past the file cap, or dropped by the size
+   *  budget). Absence of a diff is not evidence of absence of a change, so the
+   *  model is told to stay silent about these rather than reporting them as
+   *  unsupported description claims. */
+  unavailableFiles?: string[];
 }): Promise<DriftFinding[]> {
   const desc = opts.context.description.trim();
   if (desc.length < 30) {
@@ -55,7 +61,19 @@ Use "warning" only when the drift is meaningful: missing critical changes, contr
 
 Be specific — name files and identifiers, don't say "various changes".`;
 
-  const raw = await opts.ai.chat(opts.context, ask);
+  // Scope guard. The diff shown may be incomplete; claiming "no matching diff"
+  // for code you simply weren't shown is the single most common false positive
+  // this check produces, and it reads as confident because the reasoning is
+  // sound given the (wrong) input.
+  const unavailable = [...new Set(opts.unavailableFiles ?? [])];
+  const scopeNote =
+    unavailable.length > 0
+      ? `\n\nIMPORTANT: these files are part of this PR but their diffs are NOT shown to you: ${unavailable
+          .map((f) => `\`${f}\``)
+          .join(", ")}. Do NOT report drift about them — you cannot see their contents, so you have no evidence either way. Report a claim as unsupported only when the file it concerns IS shown above and plainly lacks the change.`
+      : "";
+
+  const raw = await opts.ai.chat(opts.context, ask + scopeNote);
   const cleaned = raw.trim().replace(/^```(?:json)?\s*/, "").replace(/\s*```$/, "");
   try {
     const parsed = JSON.parse(cleaned);

src/github.ts

@@ -46,6 +46,11 @@ function renderFileLevelFallbackSection(comments: ReviewComment[]): string {
 //   - A live AbortSignal short-circuits both the wait and any further retry,
 //     so a cancelled review stops hammering GitHub immediately.
 
+/** GitHub's own hard ceiling on pulls.listFiles — it will not return more than
+ *  3000 files for a PR regardless of pagination, so walking past this is pure
+ *  wasted API calls. */
+const LISTFILES_MAX = 3000;
+
 const MAX_RETRIES = 3;
 const MAX_RETRY_WAIT_MS = 60_000;
 const BASE_BACKOFF_MS = 1_000;
@@ -442,10 +447,29 @@ export class GitHubClient {
   ): Promise<PRContext> {
     const octokit = await this.getInstallationOctokit(installationId, signal);
 
-    const [pr, filesResponse] = await Promise.all([
+    // Paginate. A single page caps at 100 files, and silently losing the
+    // remainder is worse than it looks: the dropped files are invisible to
+    // BOTH the review and the description-drift check, and they never reach
+    // ignoredFiles/cappedFiles, so nothing downstream can even report that
+    // they were missed. Drift then reads their absence as the description
+    // claiming changes that aren't there.
+    //
+    // Bounded at GitHub's own listFiles ceiling (3000 files) so a runaway PR
+    // costs a known number of API calls rather than an unbounded walk.
+    let fetchedCount = 0;
+    const [pr, allFiles] = await Promise.all([
       octokit.pulls.get({ owner, repo, pull_number: pullNumber }),
-      octokit.pulls.listFiles({ owner, repo, pull_number: pullNumber, per_page: 100 }),
+      octokit.paginate(
+        octokit.pulls.listFiles,
+        { owner, repo, pull_number: pullNumber, per_page: 100 },
+        (response, done) => {
+          fetchedCount += response.data.length;
+          if (fetchedCount >= LISTFILES_MAX) done();
+          return response.data;
+        },
+      ),
     ]);
+    const filesResponse = { data: allFiles.slice(0, LISTFILES_MAX) };
 
     const fileCap = maxFiles != null && maxFiles > 0 ? maxFiles : this.config.maxFilesPerReview;
     const ignoredFiles: string[] = [];

src/reviewer.ts

@@ -1,5 +1,5 @@
 import { randomUUID } from "node:crypto";
-import { Config, AIProvider, PRContext, RepoConfig, ReviewComment } from "./types.js";
+import { Config, AIProvider, FileChange, PRContext, RepoConfig, ReviewComment } from "./types.js";
 import { buildProvider, ProviderSpec } from "./ai/provider-factory.js";
 import { FailoverProvider } from "./ai/failover.js";
 import { isAiTimeoutError } from "./ai/timeout.js";
@@ -25,7 +25,7 @@
 import { encodeState, encodeStateRef, extractState, isTrivialPatch, WalkthroughState } from "./walkthrough-state.js";
 import { assessRisk, renderRiskBlock, assessCoverage, renderCoverageBlock, shouldSuggestSplit, renderSplitSuggestion, renderConfidenceAggregate, computeReviewerDeltas, renderReviewerDeltaBlock, calibrateSeverities, resolveSeverityCalibration, renderSeverityCalibrationBlock, type CalibrationResult } from "./insights.js";
 import { suggestReviewersFromBlame, renderSuggestedReviewers, combineReviewers, renderCombinedReviewers } from "./blame-reviewers.js";
 import { loadCodeowners, ownersForFiles, renderCodeownersBlock } from "./codeowners.js";
 import { findPriorBotThreadsForPaths, renderPriorDiscussionsBlock, diffWithOtherPR, renderDiffPRReply } from "./cross-pr.js";
 import { renderStickyStatus, STICKY_MARKER } from "./sticky-status.js";
 import { recordRepo, recordPR, recordReview, recordFindings, recordPatternHits, recordIssue, recordIssueAction, getSuppressedFingerprints, listCustomRulesForRepo, deleteReviewJob, getWalkthroughState, saveWalkthroughState } from "./storage/dao.js";
@@ -85,6 +85,50 @@
   return createHash("sha256").update(normalizePatchForHash(filename, patch)).digest("hex").slice(0, 16);
 }
 
+/**
+ * Splits the PR's files into the set worth reviewing line-by-line and the sets
+ * deliberately skipped (trivial diffs, and — in incremental mode — files whose
+ * patch is byte-identical to the last review).
+ *
+ * `allFiles` is returned alongside deliberately: callers that reason about the
+ * PR as a whole rather than about individual lines (description drift, most
+ * notably) MUST use it rather than `filesToReview`. Judging a whole-PR
+ * description against the incremental slice reports every claim whose code
+ * landed in an earlier commit as unsupported.
+ */
+export function partitionFilesForReview(
+  files: FileChange[],
+  mode: "full" | "incremental",
+  priorFileShas: Record<string, string> | undefined,
+): {
+  allFiles: FileChange[];
+  filesToReview: FileChange[];
+  currentFileShas: Record<string, string>;
+  filesSkippedSimilar: string[];
+  filesSkippedTrivial: string[];
+} {
+  const currentFileShas: Record<string, string> = {};
+  const filesSkippedSimilar: string[] = [];
+  const filesSkippedTrivial: string[] = [];
+  const filesToReview: FileChange[] = [];
+
+  for (const f of files) {
+    const ph = patchHash(f.filename, f.patch);
+    currentFileShas[f.filename] = ph;
+    if (isTrivialPatch(f.patch)) {
+      filesSkippedTrivial.push(f.filename);
+      continue;
+    }
+    if (mode === "incremental" && priorFileShas?.[f.filename] === ph) {
+      filesSkippedSimilar.push(f.filename);
+      continue;
+    }
+    filesToReview.push(f);
+  }
+
+  return { allFiles: files, filesToReview, currentFileShas, filesSkippedSimilar, filesSkippedTrivial };
+}
+
 const WALKTHROUGH_MARKER = "<!-- DiffSentry Walkthrough -->";
 const WALKTHROUGH_START = "<!-- walkthrough_start -->";
 const WALKTHROUGH_END = "<!-- walkthrough_end -->";
@@ -598,24 +642,12 @@
       const priorFingerprints = new Set(priorState?.postedFingerprints ?? []);
       const priorPrLevelKeys = priorState?.postedPrLevelKeys ?? [];
 
-      // Classify each file against prior state
-      const currentFileShas: Record<string, string> = {};
-      const filesSkippedSimilar: string[] = [];
-      const filesSkippedTrivial: string[] = [];
-      const filesToReview: typeof context.files = [];
-      for (const f of context.files) {
-        const ph = patchHash(f.filename, f.patch);
-        currentFileShas[f.filename] = ph;
-        if (isTrivialPatch(f.patch)) {
-          filesSkippedTrivial.push(f.filename);
-          continue;
-        }
-        if (mode === "incremental" && priorState?.fileShas?.[f.filename] === ph) {
-          filesSkippedSimilar.push(f.filename);
-          continue;
-        }
-        filesToReview.push(f);
-      }
+      // Classify each file against prior state. allReviewableFiles keeps the
+      // complete set alive past the trim below — line-level review is rightly
+      // incremental, but whole-PR reasoning (description drift) is not.
+      const { allFiles: allReviewableFiles, filesToReview, currentFileShas, filesSkippedSimilar, filesSkippedTrivial } =
+        partitionFilesForReview(context.files, mode, priorState?.fileShas);
+
       // Replace context.files with the trimmed set so the AI prompt + review
       // submission only operate on what actually changed.
       context.files = filesToReview;
@@ -759,7 +791,7 @@
       // (Guidelines and issues are injected via the prompt builder's learnings param)
       const knowledgeLearnings = [...relevantLearnings];
       // Add guideline content as synthetic learnings
       const guidelinesPrompt = formatGuidelinesForPrompt(relevantGuidelines);
       const issuesPrompt = formatIssuesForPrompt(linkedIssues);
 
       // Inject language preference
@@ -1018,10 +1050,38 @@
         log.debug({ err }, "listPRCommits failed");
       }
 
-      // Description drift detection (one extra AI call, best-effort)
+      // Description drift detection (one extra AI call, best-effort).
+      //
+      // Drift MUST see the full PR diff, not the incremental slice. The
+      // description describes every commit on the branch; comparing it against
+      // only the files that changed since the last review manufactures
+      // "description claims X with no matching diff" for every claim whose
+      // supporting code landed in an earlier commit. That failure mode is
+      // silent and worsens with each push, since more files fall into
+      // filesSkippedSimilar.
+      //
+      // The full set needs its own budget: context.diffBudget was computed
+      // above against the trimmed set, so reusing it would leave the restored
+      // files unbounded and risk overflowing the window (which throws, and the
+      // catch below would swallow drift entirely).
       let driftFindings: Awaited<ReturnType<typeof detectDescriptionDrift>> = [];
       try {
-        driftFindings = await detectDescriptionDrift({ ai: this.ai, context });
+        const driftBudget = applyDiffBudget(
+          allReviewableFiles.map((f) => ({ filename: f.filename, patch: f.patch })),
+          repoConfig.reviews?.diff_budget,
+        );
+        driftFindings = await detectDescriptionDrift({
+          ai: this.ai,
+          context: { ...context, files: allReviewableFiles, diffBudget: driftBudget },
+          // Files that are part of the PR but whose diffs drift cannot see.
+          // Without this the check reports the same false positive for large
+          // or ignored files even on a full review.
+          unavailableFiles: [
+            ...(context.ignoredFiles ?? []),
+            ...(context.cappedFiles ?? []),
+            ...driftBudget.filesOmitted,
+          ],
+        });
       } catch (err) {
         log.debug({ err }, "Drift detection failed");
       }

tests/unit/drift-scope.test.ts

@@ -0,0 +1,127 @@
+import { describe, it, expect } from "vitest";
+import { detectDescriptionDrift } from "../../src/drift.js";
+import { partitionFilesForReview } from "../../src/reviewer.js";
+import type { AIProvider, FileChange, PRContext } from "../../src/types.js";
+
+// Regression coverage for the "Description claims X with no matching diff"
+// false-positive class. The check compares prose describing the WHOLE PR
+// against whatever diff it is handed, so any file missing from the prompt
+// reads as an unsupported claim. Two ways a file goes missing:
+//   1. incremental review trims files unchanged since the last review
+//   2. config ignores / the file cap / the size budget drop them
+// Both produced confident, wrong Major findings. See mk7luke/agent-tui#1,
+// where 6 of 6 PR-level findings named files that were in the PR all along.
+
+function file(filename: string): FileChange {
+  return {
+    filename,
+    status: "modified",
+    patch: `@@ -1,2 +1,3 @@\n context\n+changed ${filename}\n`,
+    additions: 1,
+    deletions: 0,
+  };
+}
+
+function ctx(files: FileChange[]): PRContext {
+  return {
+    owner: "o",
+    repo: "r",
+    pullNumber: 1,
+    title: "Make it build on Windows",
+    description:
+      "Adds a windows-x86_64 DotSlash entry, appends -C symbol-mangling-version=v0 to " +
+      ".cargo/config.toml, and raises the main-thread stack via /STACK:8388608 in build.rs.",
+    baseBranch: "main",
+    headBranch: "feat",
+    headSha: "deadbee",
+    files,
+  };
+}
+
+/** Captures the prompt drift sends so we can assert on what the model sees. */
+function recordingAI(): { ai: AIProvider; prompts: string[]; contexts: PRContext[] } {
+  const prompts: string[] = [];
+  const contexts: PRContext[] = [];
+  const ai = {
+    chat: async (context: PRContext, message: string) => {
+      contexts.push(context);
+      prompts.push(message);
+      return "[]";
+    },
+  } as unknown as AIProvider;
+  return { ai, prompts, contexts };
+}
+
+describe("description drift — diff scope", () => {
+  it("sees every file in the PR, not just the incrementally-changed one", async () => {
+    // What reviewer.ts now passes: the full reviewable set, even though only
+    // lib.rs changed since the last review.
+    const all = [file(".cargo/config.toml"), file("build.rs"), file("lib.rs")];
+    const { ai, contexts } = recordingAI();
+
+    await detectDescriptionDrift({ ai, context: ctx(all) });
+
+    const seen = contexts[0].files.map((f) => f.filename);
+    expect(seen).toEqual([".cargo/config.toml", "build.rs", "lib.rs"]);
+  });
+
+  it("tells the model which PR files it cannot see, so absence isn't read as evidence", async () => {
+    const { ai, prompts } = recordingAI();
+
+    await detectDescriptionDrift({
+      ai,
+      context: ctx([file("lib.rs")]),
+      unavailableFiles: ["bin/protoc", ".cargo/config.toml"],
+    });
+
+    const prompt = prompts[0];
+    expect(prompt).toContain("bin/protoc");
+    expect(prompt).toContain(".cargo/config.toml");
+    expect(prompt).toContain("Do NOT report drift about them");
+  });
+
+  it("adds no scope note when the full diff is present", async () => {
+    const { ai, prompts } = recordingAI();
+
+    await detectDescriptionDrift({ ai, context: ctx([file("lib.rs")]) });
+
+    expect(prompts[0]).not.toContain("Do NOT report drift about them");
+  });
+
+  it("preserves the full file set across the incremental trim", () => {
+    // The bug: on a synchronize push only the newest file differs from prior
+    // state, so filesToReview collapses to it. allFiles must still carry the
+    // whole PR, which is what the drift call site consumes.
+    const files = [file(".cargo/config.toml"), file("build.rs"), file("lib.rs")];
+    const priorShas: Record<string, string> = {};
+    // Simulate "already reviewed" for the two older files by round-tripping
+    // their hashes through a full-mode partition.
+    const first = partitionFilesForReview(files, "full", undefined);
+    priorShas[".cargo/config.toml"] = first.currentFileShas[".cargo/config.toml"];
+    priorShas["build.rs"] = first.currentFileShas["build.rs"];
+
+    const result = partitionFilesForReview(files, "incremental", priorShas);
+
+    expect(result.filesToReview.map((f) => f.filename)).toEqual(["lib.rs"]);
+    expect(result.filesSkippedSimilar).toEqual([".cargo/config.toml", "build.rs"]);
+    // The invariant that keeps drift honest.
+    expect(result.allFiles.map((f) => f.filename)).toEqual([
+      ".cargo/config.toml",
+      "build.rs",
+      "lib.rs",
+    ]);
+  });
+
+  it("deduplicates unavailable filenames", async () => {
+    const { ai, prompts } = recordingAI();
+
+    // ignoredFiles and the budget's omitted list can name the same file.
+    await detectDescriptionDrift({
+      ai,
+      context: ctx([file("lib.rs")]),
+      unavailableFiles: ["vendor/big.rs", "vendor/big.rs"],
+    });
+
+    expect(prompts[0].match(/vendor\/big\.rs/g)).toHaveLength(1);
+  });
+});