engineeringaicode-review

Why diff-only AI code review produces false positives — and how repo context fixes it

Pankaj Kumar··9 min read

One of the first people I gave MicroReview access to ran it on a handful of his own pull requests, then sent me a message. He liked the PR summaries and the walkthroughs. Then the part that mattered: “I noticed a few false positives in the AI bug detection.” He attached a screenshot, and when I asked for specifics, he sent the exact PR and a three-line breakdown. On a single React Native change, MicroReview had reported:

  • useStatewas “missing” — it was imported at the top of the file.
  • An interface, SectionHeaderProps, was “missing” its isDark property — isDark was defined right there in the interface.
  • A SafeAreaView was flagged for its edges prop — but the component came from react-native-safe-area-context, where edges is perfectly valid, not from react-native.

He was right on all three. And all three were the same bug — not in his code, in mine. This is a write-up of what caused it and the architecture that fixed it. Spoiler: the fix was not a better prompt, and it was not embeddings.

The root cause: the model never saw the file

MicroReview's review pipeline is straightforward. A GitHub webhook lands, the job goes onto a queue, and a worker pulls the pull request's changed files. Each changed file arrives as a unified diff — just the hunks that changed, with a few lines of surrounding context. That diff gets wrapped into a prompt and sent to the model for bug detection.

The problem is in that word “context.” A diff's context is three lines above and below each hunk. If a component uses useState on line 44, but the change starts at line 40, the diff the model sees begins somewhere around line 37 — and the import { useState } from "react" on line 1 is nowhere in the payload. The model is asked to review code it can only partially see, and it does the reasonable thing: it reports what looks, from inside the diff, like an undeclared symbol.

Every one of his three false positives is a flavor of this. The declaration exists; it just lives outside the diff window.

A taxonomy of diff-only false positives

Once I started looking for the pattern, it split into three recognizable shapes — the three he hit, in fact.

1. Undefined-symbol flags for things defined elsewhere in the file

This is the useState case. The symbol is imported or declared at the top of the file, dozens of lines above the change. From inside the diff it reads as undefined. The same shape covers the isDark report: SectionHeaderProps was defined in the file, the diff used isDark, and the model — never having seen the interface — assumed the property was missing.

2. Invalid-prop / wrong-API flags because the import source is out of view

The SafeAreaView case is subtler. There are two SafeAreaViews in the React Native world: the one from react-native (no edges prop) and the one from react-native-safe-area-context (which supports edges). The import line that disambiguates them sits at the top of the file. Without it, the model guesses the wrong provenance and flags a valid prop as invalid.

3. “Missing” handling a caller already does

The same root cause produces a fourth flavor I've seen since: a function flagged for “missing error handling” when the wrapper that calls it, two files away, already wraps it in a try/catch. The diff shows the callee; the safety lives at the call site, off screen. I'm calling this one out because it's the same disease even though it looks different — and it's the one plain full-file context doesn't fully cure, which I'll come back to.

Why the obvious fix is the wrong one

The tempting fix is to stop sending a diff and start sending the whole repository. Give the model everything and it can never miss a declaration. In practice this fails on three fronts.

Cost. You pay per input token, per pull request, on every push. A mid-sized repository is millions of tokens. Sending it — or even a large fraction of it — on every commit turns a fraction-of-a-cent review into a real line item, for information that is 99% irrelevant to a ten-line change.

Latency. Reviews are supposed to land in seconds, before the author has context-switched away. Stuffing a huge context window pushes that into tens of seconds and risks hitting model context limits outright.

Recall.This is the one people underestimate. More context is not more accuracy. Bury the four lines that changed inside 200KB of unrelated source and the model's attention smears across all of it. You trade false positives from too little context for missed bugs from too much. The goal isn't maximum context — it's the right context.

So the design constraint is: give the model exactly the definitions the diff touches, and nothing else. Bounded, cheap, fast.

What actually works: retrieve only what the diff references

The fix is two deterministic passes that run before the model call. Neither uses a database or an embedding. That's a deliberate choice I'll defend in a minute.

Pass one: the full changed file, budget-trimmed

For each modified file, fetch its complete current contents from the GitHub Contents API at the PR's head SHA. That single move kills cases 1 and 2 outright — the imports and the interface are right there. Large files would blow the budget, so the content is trimmed with a simple rule that keeps the parts that carry declarations:

// src/llm/fullFileContext.ts
export function truncateForBudget(content: string, patch: string | null,
                                  maxChars = 32000): string {
  if (content.length <= maxChars) return content;
  // Always keep: the first ~100 lines (imports, types, constants),
  // the last ~20 (exports), and a ±30-line window around each changed
  // hunk parsed from the patch. Collapse everything else into:
  //   // ... [lines 210-540 omitted for brevity] ...
}

Imports and type definitions cluster at the top; exports at the bottom; the change itself is surrounded by its own window. Everything in between — the parts a ten-line change doesn't touch — collapses into an omission marker so the model knows content was elided rather than absent.

Pass two: the local files the diff imports

Case 2 and cross-file bugs need more than the file itself — they need the other files the change depends on. Rather than embed and search the repo, I resolve imports the way the compiler would: parse the import statements, turn each specifier into candidate paths, and fetch the local ones.

// src/llm/contextBuilder.ts

// import ... from "x"  |  require("x")  |  import("x")
const JS_IMPORT_RE =
  /(?:from\s+|import\s*\(\s*|require\s*\(\s*)["']([^"']+)["']/g;

function resolveCandidates(fromFile: string, spec: string): string[] {
  if (spec.startsWith("./") || spec.startsWith("../")) {
    // relative import — resolve against the changed file's directory
  } else if (spec.startsWith("@/")) {
    // common alias: @/x -> src/x
  } else {
    return []; // bare package import (node_modules) — not fetched
  }
  // try base + .ts/.tsx/.js/.jsx, base/index.*, base.py, ...
}

Bare package imports return an empty list — I don't fetch node_modules, because the model already knows the public API of react or react-native-safe-area-context from training. What it can't know is your ./SectionHeader. The whole thing is bounded so a large PR can't fan out into hundreds of fetches:

const MAX_RELATED_FILES = 6;    // cap on cross-file fetches per PR
const MAX_FILE_CHARS   = 2500;  // trim each related file
const MAX_FULL_FILE_CHARS = 32000; // ~8k tokens per changed file

Framing it for the model

The two contexts are assembled once per review and handed to the bug detector, which presents each file as an explicit <full_file> block (context only) followed by the <diff> block (the thing to actually review):

### FILE: components/SectionHeader.tsx
<full_file path="components/SectionHeader.tsx">
import { useState } from "react";
// ...40 lines of the actual file...
interface SectionHeaderProps { title: string; isDark: boolean }
// ...
</full_file>
<diff>
+  const [open, setOpen] = useState(false);
+  return <Header isDark={isDark} ... />
</diff>

The system prompt then makes the boundary explicit:

Use the <full_file> section to verify imports, type definitions, variable declarations, and component sources BEFORE flagging any issue. Do NOT flag missing imports, missing type properties, undefined variables, or invalid props if they exist in the full file but are outside the diff. Report issues only for the changed (diff) lines.

Crucially, the secret scanner still runs on the diff only — full-file context is for the AI reasoning pass, not for what we grep for leaked keys. Widening that would be a way to leak more, not less.

Why deterministic resolution and not embeddings

The fashionable version of this is: chunk the repo, embed every chunk, store the vectors, and at review time do a semantic nearest-neighbor search for context. I didn't build that, and the reasons are worth stating because they're tradeoffs, not dogma.

Import resolution is deterministic. An import statement is not a fuzzy signal about what a file depends on — it is thedeclaration of what it depends on. Following it gives you exactly the file the diff references, with no relevance threshold to tune and no chance of retrieving a plausible-but-wrong neighbor. For “where is this symbol defined,” a resolver beats a similarity search because the answer is exact, not ranked.

It also has no index to keep warm.An embedding index has to be built per repo, re-embedded as the code changes, and stored somewhere. It's stale the moment someone pushes. Resolution reads the actual files at the actual head SHA, every time — it works on a repo's very first pull request with zero warm-up, and it can never serve you last week's version of a function. No embedding spend, no vector store to operate.

I'm not claiming embeddings are useless here — they earn their place for the questions resolution can't answer: “what else in this codebase looks like the thing being changed,” blast-radius analysis, finding call sites of a modified function across files that don't import it directly. That's a real roadmap item, and when it happens it'll likely be pgvectorin the Postgres we already run rather than a separate vector database — one datastore, transactional with the rest of the data, is worth more to a small team than a marginally faster dedicated index. But that's an additionfor a different class of question. It was never the fix for “useStateis imported and you said it wasn't.” Reaching for a vector DB there would have been architecture cosplay.

Before and after

Before: the three findings above, each confidently wrong, on a clean PR. The kind of thing that teaches a developer to distrust the tool by the second review — which is exactly how a code review bot dies, because noise is indistinguishable from being wrong.

After, on a test PR built to reproduce the shape — a new method whose imports and helpers sit 40+ lines above the changed hunk — the bug detector fetched the full file, saw the declarations, and returned zero phantom findings. The pipeline logs the split so I can watch it in production:

[FULL-FILE-CTX] full context for 13 file(s), diff-only for 0
[AI-BUG-DETECTION] Full-file context for 13 file(s)

The honest version of “after” is that I can measure the absence of the specific false positives it targets; I can't yet publish a clean before/after false-positive rate across a large sample, because that needs a labeled corpus I'm still building. I'd rather say that than quote a number I made up.

What this still gets wrong

Context retrieval narrowed the problem; it didn't end it. The known gaps, honestly:

  • Dynamic and computed imports. The resolver reads static import statements. Runtime require(variable), dependency injection, and re-exports through barrel files can hide a dependency it never fetches.
  • The budget can truncate a large callee. Six related files, trimmed to 2500 characters each. A genuinely large module can lose the exact function the diff calls to the trim.
  • Cross-language and non-JS/Python graphs. The resolver is strongest where the import syntax is unambiguous. Other ecosystems get the full-file pass but weaker cross-file resolution.
  • Convention, not just correctness.The “caller already handles it” case and style complaints that contradict a repo's own patterns need something more than the files a diff imports — they need a model of how thisteam writes code. That's a different mechanism, and it's not solved by retrieval alone.

The takeaway

Most “the AI is wrong” complaints about code review tools aren't reasoning failures. They're context failures — the model was asked to judge code it couldn't fully see. The fix isn't a cleverer prompt or a bigger model; it's an architecture that retrieves the specific definitions a change depends on and nothing more. Follow the imports the way a compiler would, hand the model the file it's editing, and draw a hard line between “context” and “the thing to review.”

To the tester who sent this — thank you. Three lines of specific feedback did more for this product than a month of my own guessing.

Ready to try MicroReview?

Free for 2 repos. 30-second setup. No credit card required.

Get Started Free →