Line-by-line review feels thorough, but it is a poor first pass. Reviewers spend attention on local details before they know whether the change is attached to the right boundary, protects the right resource, or even solves the intended problem.
A better Git diff review moves from intent to architecture to behavior to evidence. Each pass asks a different question, which reduces fatigue and exposes risks that syntax-focused review misses.
Pass 0: confirm what you are reviewing
Before opening a file, identify the exact comparison.
git status --short --branch
git merge-base HEAD origin/main
git diff --stat origin/main...HEAD
git log --oneline --decorate origin/main..HEAD
The three-dot range compares the branch against its merge base with the target. That is usually closer to the change a pull request will introduce. A two-dot range describes a different relationship and can include target-branch changes depending on the refs.
Also inspect staged and unstaged work. A local review can otherwise prove a branch while the eventual commit contains additional material.
Record the base ref, head commit, and whether generated files are expected.
Pass 1: compare the diff with the intended outcome
Read the request, issue, or acceptance statement first. Summarize it without using implementation language.
Then scan filenames and top-level statistics. Ask:
- Which files prove the user-visible outcome?
- Which files are incidental refactoring?
- Which expected surface is missing?
- Did configuration, tests, documentation, or migration files change with the feature?
- Does the change cross more trust boundaries than the request implies?
If a billing change modifies only a button and no server authorization or webhook path, the missing work is visible before reading any function.
Large unrelated formatting or renaming changes should be separated when practical. They hide semantic changes and make later archaeology harder.
Pass 2: inspect boundaries before internals
Open public routes, commands, event handlers, exported functions, schemas, migrations, and configuration first. Boundaries determine what the rest of the code is allowed to assume.
For each boundary, identify:
- Input source
- Validation and normalization
- Authentication and authorization
- Side effects
- Output and error shape
- Observability
An endpoint with perfect internal types is still unsafe if untrusted JSON is cast into those types. A desktop command with careful Rust code is still dangerous if any webview content can invoke it with an arbitrary path.
Check allowlists over denylists for roles, redirect URIs, file extensions, event names, and configuration. Denylists age badly because they require anticipating future invalid cases.
Pass 3: trace one success and one failure path
Choose the main user action and trace it across layers. Do not assume names imply behavior. Follow the value from input to storage or external effect and back to the UI.
On the success path, verify that the final state matches the promise. On the failure path, choose the most consequential realistic failure: expired session, database conflict, provider timeout, partial upload, or invalid state transition.
Ask what the user sees, what persists, and whether retry is safe.
This exposes partial-state defects. For example, a release artifact row might be created before its upload fails. That is acceptable only if the row remains an explicit incomplete draft and can be retried or removed safely.
Pass 4: review data transformations
Data bugs often hide between valid individual functions. Trace identifiers, dates, currency, optional fields, and state enums across serialization boundaries.
Check for:
- Seconds versus milliseconds
- Local time versus UTC
- Display email versus normalized email
- Decimal currency versus integer minor units
null, missing, and empty-string behavior- Boolean values stored as integers
- Enum values that differ between provider and application
- Encoded versus decoded webhook bodies
- Public IDs versus database primary keys
Read SQL as application logic. Verify constraints, join cardinality, pagination order, and whether a query can return another account's row. Parameterization protects values from injection; it does not make the authorization predicate correct.
Pass 5: review state transitions
Write down the allowed transition graph for important records.
For a release:
draft -> published -> yanked
For a rotating session:
active -> rotated
active -> revoked
active -> expired
rotated + reuse -> revoke family
Then compare every mutation with that graph. Updates should include the expected current state in the database predicate. A read-then-write check without a conditional update can race.
Look for operations that appear reversible in the UI but are irreversible in storage. Copy and confirmation text must match actual semantics.
Pass 6: review security as data flow
Search for sensitive sources and sinks rather than only suspicious words.
Sources include passwords, refresh tokens, OAuth codes, license keys, repository diffs, database queries, API request bodies, cookies, private keys, and environment values. Sinks include logs, analytics properties, browser storage, URLs, process arguments, error responses, email HTML, and database columns.
For each path, ask:
- Is collection necessary?
- Is transport encrypted?
- Is storage appropriate?
- Is output encoded for its context?
- Is the value redacted from logs?
- Can an attacker control adjacent markup or SQL?
- How is the value rotated or deleted?
Pay special attention to code that constructs HTML email, shell commands, redirect URLs, object storage keys, and dynamic SQL identifiers.
Pass 7: interrogate the tests
Tests are claims about risk. Review what each test proves and which failure would still pass.
Strong tests cover:
- The important public behavior
- Authorization and cross-account access
- Boundary values and invalid state
- Dependency failure
- Idempotency or retry
- Data migration compatibility
- Output encoding for attacker-controlled text
Avoid approving tests only because they mirror the implementation. A test that reconstructs the same algorithm can reproduce the same misunderstanding.
Run focused tests, type checking, and the production build. Confirm the expected test count and inspect warnings.
Pass 8: inspect release impact
The final pass asks what changes outside the codebase.
- Are new environment variables documented in
env.example? - Does deployment order matter?
- Is the database backward compatible during rollout?
- Do OAuth or webhook dashboards need changes?
- Will old desktop clients keep working?
- Does the change affect analytics consent or privacy language?
- Does an artifact need a new signature or updater target?
- Is rollback possible after new data is written?
A locally correct feature can still be operationally incomplete.
Leave comments at the right level
Review feedback should name the observed condition, its consequence, and a useful direction.
Weak: "This might be insecure."
Useful: "This handler verifies authentication but does not constrain the license row to the actor's account. A valid user could submit another UUID and disclose its last four characters. Include account ownership in the query or require an administrative role."
Separate blockers from improvements. If everything has equal severity, the author must guess what prevents approval.
A diff is evidence, not the whole change
The diff tells you what text changed. It does not tell you whether the base is correct, the deployed configuration exists, the migration will succeed, the external provider accepts the request, or the artifact contains what you reviewed.
Review from intent outward. Confirm the comparison, inspect boundaries, trace behavior, validate transformations and transitions, follow sensitive data, challenge the tests, and close with release impact. That sequence finds more real defects with less reviewer fatigue than treating every changed line as equally important.