You Can't Review What You Can't Read
Part 4 of 6 · Why AI Code Fails (And How to Fix It)
You Can't Review What You Can't Read
The PR had 623 lines across nine files. The description read: "Adds guest checkout functionality."
That was it. No requirements referenced. No architectural decision linked. No explanation of why userId was nullable, why there was a new rule in the access control file, or what the guestEmail index in the Cloud Function configuration was for. The diff showed what changed. Nothing showed why.
The senior engineer assigned to review it spent 45 minutes on it. They caught a naming inconsistency. They asked about the access control rule — got a reply in the comments, accepted the explanation, approved. The PR merged.
Three weeks later, the GDPR deletion handler ran its weekly batch. It cleaned up accounts by userId. Guest orders didn't have a userId. The guest order data was not deleted. It had never been configured to be deleted, because the deletion handler predated guest checkout and nobody knew — from reading the diff alone — that it needed to be updated.
This wasn't in the 623 lines. It was in the gap the 623 lines created.
A code review has two possible modes. The first is a style check: does the code look right, follow conventions, pass the tests, avoid obvious errors? The second is a conformance check: does this implementation match the requirements that were agreed, the architectural decisions that were made, and the system-wide constraints that govern this part of the codebase?
Most code reviews are style checks. When a reviewer has a 623-line diff with no context, style is the only thing they can realistically check. They can see what changed. They cannot see what was supposed to change, why it was supposed to change that way, or what was explicitly ruled out during planning.
The GDPR deletion handler was never in the diff. It was in the impact analysis that was never written — the document that would have asked, before a line of code was generated: "What existing services are affected by the fact that orders can now have a null userId?" The answer would have included the deletion handler. The plan would have added a task for it. The diff would have included the fix. The reviewer would have had a line to look at.
Without the impact analysis, the gap doesn't appear in any artifact. It doesn't appear in the diff. It doesn't appear in the tests. It doesn't appear in CI. It appears three weeks later, in a log file that shows guest email addresses that were supposed to be gone.
There's a specific document that transforms the nature of a code review: the PR description written from the implementation plan rather than from the diff.
Here's what the guest checkout PR description should have looked like:
## Guest Checkout — TASK-01 through TASK-08
Adds guest purchase and optional post-purchase account creation.
What changed:
- Order model: userId nullable, guestEmail added (ADR-012)
- Guest checkout screen added (REQ-CHECKOUT-00010)
- Access control rules: guest read by orderId + guestEmail pair
- Cloud Functions: guestEmail index added; GDPR deletion handler
updated to include guestEmail cleanup (ADR-012, consequence 3)
Why:
REQ-CHECKOUT-00010 — 34% cart abandonment at account creation.
REQ-CHECKOUT-00020 — Post-purchase account creation offer.
ADR:
ADR-012 — Guest orders in unified orders collection (userId: null).
Rejected: separate guest_orders collection — violates single-workflow
operations requirement, duplicates all order management logic.
Testing:
- Guest purchase end to end: confirmed on iOS simulator
- Registered user order history: guest orders absent (filter verified)
- GDPR deletion handler: guestEmail cleanup path confirmed
- All tests pass; coverage 84%
Closes #47
Notice what this description does. It doesn't summarise the diff — GitHub already does that. It tells the reviewer which requirements the implementation is supposed to satisfy, which architectural decision governs the data model, what was explicitly rejected and why, and what was manually tested before the PR opened.
A reviewer reading this description can do a conformance check. They can read ADR-012, look at the Order model changes, and confirm the implementation follows the decision. They can read REQ-CHECKOUT-00010, look at the test results, and confirm the acceptance criteria were covered. They can check that the GDPR deletion handler update is in the diff — because the description says it should be there.
And critically — if the GDPR update isn't in the diff, they notice. Because the description says it should be. The discrepancy is specific and actionable. It's not a feeling that something might be off. It's a line item in the description that doesn't have a corresponding change in the diff.
The description written from the plan creates accountability in both directions. The developer who writes it has to confirm that every item in the plan made it into the implementation. The reviewer who reads it has a checklist of what to look for. A discrepancy between description and diff is a finding, not a suspicion.
This is why the PR description is written from the implementation plan rather than from the diff. The plan was written before generation, when the team agreed on what would change, what wouldn't change, and what implications needed to be addressed. The PR description confirms that the implementation matches that agreement. If it doesn't — if the AI touched files that weren't in the plan, or missed a task that was — the description surfaces it before merge.
The senior engineer doing the review is now doing the job a senior engineer should be doing: a system-wide conformance check, not a line-by-line syntax review. They know what was agreed. They're verifying the agreement was kept. That's a fundamentally different review than "does this diff look okay?" — and it's the only kind of review that catches the gap between what was built and what the system needed.
The commit message matters too, for reasons that show up months later.
A commit that says feat(checkout): add guest order support tells you when guest checkout was added. A commit that says feat(checkout): extend Order model for guest orders — per REQ-CHECKOUT-00010, ADR-012, TASK-01 tells you when it was added, what requirement it was addressing, what architectural decision it followed, and which specific task in the plan produced it.
Six months later, a developer is looking at a line in order_service.dart that filters by where('userId', isEqualTo: uid). They wonder: should this also include explicit null checks? The commit message takes them to ADR-012, which takes them to the rejected alternatives section, which explains that the isEqualTo filter already excludes guest orders by semantics and an explicit null check is intentionally absent. They don't touch it. The guest order separation holds.
Without the commit message, they'd have to guess. Or ask someone who may not remember. Or read every file related to guest checkout to piece together intent from implementation. Guessing takes five minutes and is sometimes wrong. Asking depends on institutional memory that fades. Piecing together takes longer, and sometimes people don't bother — they just add the null check that felt safe, and it shows up in a bug report two months later.
A commit message is three extra minutes to write. The traceability it creates lasts for the lifetime of the codebase.
Code review is only as good as the information the reviewer has. Give a senior engineer a 623-line diff with no context and they'll tell you if the code looks right. Give them the same diff with a requirements reference, an ADR, a description written from the implementation plan, and a list of what was manually tested — and they can tell you whether the feature is correct.
Those are different things. A correct implementation satisfies the requirements, follows the architectural decisions, handles the edge cases the requirements specified, and doesn't break the system-wide constraints that existed before this feature was written.
A code review that only has the diff can't verify any of that. It can verify that the code is syntactically correct, conventionally structured, and consistent with itself. That's the style check. It's necessary. It's not sufficient.
The GDPR deletion handler gap would have appeared in the description. A reviewer reading the description would have looked for it in the diff. It wasn't there. The PR would have come back with a comment: "The description says GDPR deletion handler should be updated. I don't see that change in the diff." The developer would have implemented it. The batch job three weeks later would have found nothing to do, because there was nothing left.
That's the review worth calling a review.