skip to content
4 min#agents#review#tools

A narrow review should say so

My review bot could post a tidy, confident summary on a pull request whose biggest file it had never read. Not skimmed. Never read. The file was too large, so it got dropped, and the summary said nothing about it. Green checkmark energy, zero coverage. I built that.

A review that reads less than the change touches is normal - every automated review does. A review that reads less and acts like it read everything is a lie with formatting. The first one is a limitation. The second one is a choice, and I made it by not thinking about it.

What a diff does not contain

A diff is the set of lines that changed. The questions a reviewer actually has to answer live mostly outside it:

  • Callers. A changed signature, return value, or error behavior breaks code that is not in the patch.
  • Deletions. A removed check shows up as a few red lines. Whatever it used to protect does not show up at all.
  • Tests that now lie. A test that still passes because it never touched the changed path looks exactly like one that covers it.
  • Configuration and contracts. Defaults, schemas, and other services depend on the changed code without importing it.

A model handed a diff can be very good at the diff and still blind to all of that. Size makes it worse: when a change is too large for the context window (the text a model can consider in one request), something gets cut, so the review gets narrower exactly when the change is riskiest. That is the worst possible direction for the trade to go.

The narrow review I shipped

kReview did this, and the receipts are in its history. The repository index hard-skipped any file over 200 KB - no chunks, no symbols, no context, no mention (not even a log line). A large changed file could be trimmed out of the review diff entirely. The fix landed as a commit titled "Stop silently skipping large files in indexing and review", which is about as close to a confession as a commit message gets.

The model was not the problem. It did fine with what it got. The problem was me: I wrote a report where a file that was never read looked identical to a file that was read and passed. I spent my effort on making findings smarter and none on saying what the findings were based on. Backwards. (classic)

Now the index takes files up to 1.5 MB, a large file gets partially indexed instead of thrown away, and whatever still does not fit is logged instead of quietly vanishing. Better. Still not something I am proud of having needed.

Widening the view, with receipts

The fix is two moves. First, give the reviewer more of what the change actually touches - without trusting the pull request to say what that is, because the pull request is the thing under review:

  • A change-impact map. The repository index is built from the base branch, then combined with validated post-change definitions, so modified and new symbols map to the code that uses them.
  • Exact-head language servers. When an operator installs one, it supplies definitions and references for the PR head. It is optional and fail-open: a broken language server does not block a review.
  • Sandboxed analyzers. Operator-chosen linters and analyzers run against a read-only worktree in pinned, network-less containers. No command ever comes from the reviewed repository.
  • A verification pass. Findings are checked a second time before they publish, and in deep review they only publish when their quoted evidence can be anchored to the actual diff. A finding that cannot point at a line does not get to exist.

Second, make the narrowness visible, which is the part I skipped the first time. Every file ends up in one of four states - fully reviewed, partially reviewed, deliberately excluded, not reviewed - and the posted summary names which is which. A review of an old head is aborted as soon as a newer one arrives, instead of publishing an answer to a question nobody is asking anymore.

What it still cannot see

Wider is still not complete. A review bot reads code. It does not see production data, runtime timing, how the feature is actually used, or what the author meant to do. Fail-open enrichment means one review can be wider than the next, depending on which language server happened to be healthy that day. And a model reading a clean, fully covered diff can still be flat-out wrong (mine included).

None of that is a reason to stop automated review. It is a reason to read the report as a statement about coverage first and about bugs second - and to distrust any bot that does not give you the first part.

The rule I hold kReview to: a review is only as wide as it can show, and a file it cannot account for counts as not reviewed.