Reviewing Agent Diffs Like a Human
Agents can open PRs quickly, but merge is still a human decision. An agent diff looks like any other in git diff / PR Files, yet it often mixes out-of-scope “helpful” refactors, skipped or softened tests, secret-adjacent edits, and a body that only claims “tests passed.” You need a human review checklist.
This post covers only what to look at first · whether to merge without tests · rollback criteria. Branch names, CODEOWNERS, path allowlists, and splitting PRs per agent belong to multi-agent-pr-workflow. Here the focus is how a human skims one already-open diff. No pricing, plans, or token counts.
What should you check first?
One-line answer: Start with the file list and stats, confirm intent (ticket / PR body) matches the paths, then inspect risk surfaces (auth, data, deploy, secrets, schema) and test/doc pairing. Line-by-line style comes last.
Recommended order when you open an agent PR like a human one:
| Step | Look at | Ask |
|---|---|---|
| 1 | Files changed / git diff --stat | Do file count and churn match the ticket? Did lockfiles, generated trees, or whole i18n dumps appear suddenly? |
| 2 | PR body vs diff | Do summary, out-of-scope, and test/repro notes match reality? Claims without evidence paths? |
| 3 | Risk paths | Auth, billing, permissions, migrations, CI/deploy, .env* / key files, public API signatures? |
| 4 | Deletes / moves | Is a large delete “cleanup” or a regression? Were call sites updated? |
| 5 | Tests / fixtures | Is behavior change paired with tests? Any skip/xit/weakened assertions to fake green? |
| 6 | Style / naming | Only after the above. Pure format-only belongs in a separate PR. |
Copy-paste checklist:
[ ] stat: expected paths only? no lockfile/generated explosion?
[ ] intent: PR Done/Out-of-scope matches the diff?
[ ] risk: secrets, auth, schema, deploy scripts explicitly reviewed?
[ ] delete/rename: imports and config updated together?
[ ] tests: paired coverage or a one-line reason why not?
[ ] do not Approve on agent “self-review” prose aloneAgent-specific traps (more common than in human-written diffs):
- Out-of-scope “while I was here” — drive-by renames, bulk lint, unused-import sweeps that bloat the diff.
- Story vs code mismatch — body says “bugfix only”; diff changes API signatures or defaults.
- Tests in name only — present but miss failure paths, or only raise flaky timeouts.
- Tooling noise — formatter, codegen, or snapshots updated without intent.
Boundary vs multi-agent-pr-workflow: that post is who merges on which branch under which gate; this section is the order a human reads the diff in front of that gate.
Merge without tests?
One-line answer: Default no. Do not merge if the agreed minimum verification is missing or red locally/in CI. Exceptions are only docs/comments/pure config typos with clearly no runtime impact, and only when the team records an exception label and reason on the PR. “The agent said it ran tests” is not evidence.
Decision table:
| Change type | Minimum verification | Merge without tests? |
|---|---|---|
| Behavior, API, schema, auth | Related unit/integration or documented manual scenario + CI green | No |
| Bugfix | Repro red → same command green after patch (prefer a regression test) | No (need repro evidence) |
| Refactor (“behavior unchanged”) | Existing suite holds + smoke on critical paths | No (no suite ⇒ no “unchanged” claim) |
| Docs / README / comments only | Link/typo check; preview if applicable | Conditional yes (no runtime code) |
| CI / deploy scripts | Dry-run or pipeline validation job | No |
| Lockfile-only | Install, build, core tests | No (supply chain / breakage) |
Practice rules:
- Pin evidence on the PR — commands run, CI check names, manual scenario checklist. You do not need the full agent chat dump.
- Do not fake green — skips, timeout-only bumps,
@retryhiding flakes, deleted assertions → hold merge and request revert of those “fixes.” - If “hard to test” — say why, what manual check ran, and the follow-up ticket ID. Empty answers → no merge.
- Boundary vs agent TDD loop — that post is red→green while writing; this is the human gate before merge. Even if the agent claims green, re-check the command and CI yourself.
When you allow an exception, require a label (e.g. risk:docs-only) plus a one-line rollback note (next section) so “just merge” does not become habit.
What are the rollback criteria?
One-line answer: Before merge, decide the revert unit (revert commit / PR revert / feature flag). After merge, if symptom, blast radius, or time-to-clarity crosses the threshold, roll back first instead of stacking agent hotfixes.
Pre-merge rollback design:
| Item | Decide |
|---|---|
| Revert unit | Is a single PR revert safe? If PRs stacked, who owns the integration revert? |
| Flags | Is there a feature flag / config switch on the risky path? |
| Signals | Which metrics/logs/alerts mean “failed”? (error rate, 5xx, queue depth, data mismatch) |
| Comms | Channel and OWNER for the rollback call |
Prefer immediate rollback when:
- User or data harm — bad writes, privilege expansion, billing/delete path faults.
- Service level — agreed error/latency/failure budget breached in time with this deploy.
- Forward fix is unclear — three or more cause candidates, or agent follow-ups only enlarge the diff.
- Migration failure — schema broken without expand/contract. (Data migrations may need more than code revert—confirm backup / forward-fix runbook.)
- Suspected secret leak — rollback plus key rotation and log review. Code revert alone is not enough.
Rollback execution checks:
[ ] Did revert/flag reduce the symptom?
[ ] Was a prevent-recurrence issue opened on main/deploy?
[ ] Did you avoid “keep patching the same branch” with the agent? (new branch, minimal repro)
[ ] One-line human note: what did review miss? (stats? tests? risk paths?)Choose a forward fix only when the cause fits in one sentence, a test already reproduces red, and the fix surface is smaller than the original PR. Otherwise rollback is the default.
FAQ
How is this different from multi-agent-pr-workflow?
That post is per-agent branches, human Approve gates, and path-collision prevention. This post is the checklist for how a human reads the diff that arrived at the gate.
Are agent self-review or review-bot Approves enough?
No. Summaries and missing-test nits help, but intent, secrets, data, and rollback stay with humans (or CODEOWNERS).
Local green but no CI?
Before merge, write the team-agreed minimum commands and their pass/fail on the PR. Missing CI is itself a risk signal—be more conservative on behavior-changing PRs.
Formatter churn mixed with logic?
Review cost explodes. Split format/generated from logic, or review logic files first and defer format to a follow-up PR.
Sources
- Team practice: PR Files /
git diff --statfirst; risk paths, test evidence, revertability — checklists above - Adjacent axes: multi-agent-pr-workflow (branches/gates), agent-tdd-loop (red→green while writing), git-worktree-agent (worktree isolation)