Reading a diff and reviewing a pull request

An agent has opened a pull request. It says “fix date handling in the survey export”. The change touches eleven files, adds 2,400 lines and removes 1,900. You could approve it because the tests pass, or read all 4,300 lines, and neither is a real option. The skill in between is reading a diff — the machine-generated summary of what changed — knowing what it shows plainly, what it makes look small, and what it cannot show at all.

The anatomy of a hunk

A diff is a list of files, each containing hunks: regions that changed, with a few unchanged lines around them for orientation.

diff --git a/api/export.py b/api/export.py
index 8c3f9a1..b21d044 100644
--- a/api/export.py
+++ b/api/export.py
@@ -14,7 +14,9 @@ def parse_submitted(raw: str) -> datetime:
     if not raw:
         raise ValueError("empty timestamp")
-    return datetime.fromisoformat(raw)
+    dt = datetime.fromisoformat(raw)
+    # naive timestamps are assumed UTC, matching the survey platform
+    return dt if dt.tzinfo else dt.replace(tzinfo=UTC)

Read it in four moves. --- a/ is the old file and +++ b/ the new one. The header @@ -14,7 +14,9 @@ says: seven lines starting at line 14 in the old version, nine lines starting at line 14 in the new — so two lines were added net. The text after the second @@ is git’s guess at the enclosing function, printed for orientation only. Then, within the hunk, a leading space is an unchanged context line, - is a line that is gone, and + is a line that is new.

A modified line shows as a - and + pair. This is why a one-character fix looks like a two-line change, and why the pairs that look nearly identical are the ones to slow down on:

-    if retries >= max_retries:
+    if retries > max_retries:

That is one extra attempt per call against a flaky upstream API, which across a nightly batch of 5,000 survey fetches is a meaningfully different load profile. > for >=, and for or, a removed not, a default flipped from True to False — an agent rewriting a function changes these incidentally, and they are the easiest thing in a diff to skim past.

Start with the shape, not the code

Before reading a line, get the file list.

git fetch origin && git diff --stat origin/main...HEAD
#  api/export.py               |   14 +-
#  api/models.py               |    8 +-
#  api/auth.py                 |   22 +--      ← nobody mentioned auth
#  web/src/ExportTable.tsx     |   31 ++-
#  pnpm-lock.yaml              | 2103 ++++++++
#  api/legacy_export.py        |  412 ------   ← a whole module deleted

Ten seconds, and the review has changed completely. Two thousand of the lines are machine-written lockfile churn. A whole module has been deleted. And the change touches authentication, which the description never mentioned. Compare the description against the file list: scope creep is normal in agent-authored changes and is the main reason to look at the shape first.

Now remove the noise so the real diff is readable:

# -w ignores whitespace-only changes (a formatter ran); pathspecs drop generated files
git diff -w origin/main...HEAD -- . \
  ':(exclude)pnpm-lock.yaml' ':(exclude)uv.lock' ':(exclude)dist'

# The lockfile is reviewed as a version list, not line by line
git diff origin/main...HEAD -- pnpm-lock.yaml | grep -E "^[+-]\s+version:" | sort -u

What a diff hides

Four things, and all four have bitten real deployments.

Deleted files look small. Removing a 412-line module is one collapsed entry in most review interfaces — visually less than a three-line typo fix. Nothing in the diff tells you whether something still imports it, or whether a cron schedule still calls it.

# Before approving any deletion, ask who still needs it
git grep -n "legacy_export" -- . ':(exclude)api/legacy_export.py'
git grep -rn "legacy_export" .github/ infra/ *.toml *.yaml

Moved code looks like a rewrite. Git diffs files, not code blocks, so relocating a function is a big deletion in one file and a big addition in another, with nothing linking them. Check it rather than reading it twice:

git diff --color-moved=zebra origin/main...HEAD     # moved lines tinted differently
git show origin/main:api/old_path.py | diff - api/new_path.py   # empty output = pure move

A reformat drowns the change. When a formatter has run in the same commit as a logic change, git diff -w is the difference between a reviewable diff and an unreadable one.

And the important one: the file that should have changed and did not. A diff can only show you lines that exist. It cannot show an absence.

 class SurveyResponse(Base):
     id:      Mapped[int]    = mapped_column(primary_key=True)
     answers: Mapped[dict]   = mapped_column(JSON)
+    locale:  Mapped[str]    = mapped_column(String(8), server_default="en-GB")

The model is Python’s description of the table, not the table. With no matching migration in the same change, local development works — the database was recreated from the models — and the first production query selecting that column fails outright. The same shape recurs everywhere: a new setting with no entry in the deployment configuration, a new route with no authentication dependency, a new field in the API response that the React table silently ignores.

Review in dependency order

Review interfaces list files alphabetically, which is the worst possible order. Go bottom-up, because the lower layers define the contract the upper ones must respect:

1. Migrations and schema. Is it reversible? Does adding a NOT NULL column to a populated table have a default? Does a rename break anything still reading the old name?
2. Models and data access. Do the SQLAlchemy models match the migration exactly? Is every query still bounded by a LIMIT or a date filter?
3. Services and routes. Does the input model validate what the logic assumes? Is there still an auth dependency on every endpoint that needs one?
4. The front end. Does it handle the loading and error states, and read every field the response now returns?

Reviewing the React table first means judging it against assumptions you have not checked yet.

Using history to answer “when did this break?”

Three commands cover almost everything.

git log with the pickaxe finds when a string entered or left the codebase — perfect for a setting name or a function that no longer exists:

git log -S"AZURE_OPENAI_KEY" --oneline        # commits where the count of that string changed
git log -G"requests\.get" --oneline -p        # commits whose diff text matches a pattern
git log --oneline --follow -- api/export.py   # follows the file across renames

git blame tells you which commit last touched a line — scoped, or it is unusable:

git blame -L 40,60 api/export.py              # just the range you care about
git blame --ignore-rev 9f2c1ab api/export.py  # skip the "chore: run formatter" commit

That second flag matters. A bulk reformat makes blame attribute every line in the file to one meaningless commit. Record such commits in a .git-blame-ignore-revs file and point blame.ignoreRevsFile at it, and every blame skips them from then on.

git bisect finds the commit that changed behaviour, which is a different question from who touched a line. It binary-searches, so 150 commits take about eight tests — and if you can express the test as a command, it does the whole search unattended:

git bisect start HEAD v1.4.0                      # bad commit first, then a known-good one
git bisect run pytest -q tests/test_export.py     # exit code decides good or bad
# ... prints "<sha> is the first bad commit"
git bisect reset                                  # always return to where you started

Bisect is the single highest-leverage git command for agent-written code, because you often have no idea which of last week’s forty commits changed the behaviour, and it does not require you to.

What to check in your own code

• Run git diff --stat first, and compare the file list against the pull request description. Anything unmentioned is where to look hardest.
• Re-read every -/+ pair that looks almost identical, character by character, asking what happens at the boundary value.
• For every deleted file, git grep its name across the repo, the workflows and the infrastructure config before approving.
• For every model or schema change, confirm a migration exists in the same change, and that it is reversible.
• For every new response field, confirm the front end reads it; for every new error case, confirm the UI shows it.
• Diff the lockfile as a list of version changes, not line by line — and check nothing jumped a major version quietly.
• When behaviour regressed and you do not know when, reach for git bisect run before reading commits by hand.