So far, others have reviewed your code. From now on you will also review code yourself: your colleagues' merge requests and — more and more — code that an agent wrote for you. Reviewing is a skill of its own. This lesson teaches you what to look for, how to write useful comments, and what is different about reviewing agent-generated code.
Important
Work on this lesson in
advisormode.
Learning goals
- You can explain what a code review is for: a code base that gets better over time, knowledge that spreads, bugs caught before users find them.
- You can review a change in order of importance — does it solve the right problem, is the design sound, is it correct, is it tested, is it readable — and treat style last.
- You can navigate a change: understand the intent first, then the big picture, then the details.
- You can write review comments that help: explain why, mark severity, ask instead of command where you are unsure, and say what is good.
- You can explain our review process: feature branches, the merge request checklist, gatekeeping.
- You can explain what is different about reviewing agent-generated code, and which failure modes to look for.
Resources
Read what's new to you, skim what's familiar, skip what you already master. Stop when you can meet the learning goals.
Your agent can also generate an overview, a tutorial or an explanation for anything here, tailored to what you already know. Just ask.
Reviewing in general
- 📄 Google's engineering practices: The standard of code review Show archive.org snapshot , What to look for Show archive.org snapshot , Navigating a CL Show archive.org snapshot and How to write comments Show archive.org snapshot — the canonical reviewer's guide; four short pages
- 📄
Conventional Comments
Show archive.org snapshot
— one vocabulary for marking severity (
nit:,suggestion:,issue:)
Our process
- 📄 Gatekeeping guide for developers — feature branches, reviews, merges
- 📄 Before you make a merge request: checklist for common mistakes — what the author should have covered before you review
Reviewing agent-generated code
- 📄 Review AI-generated code Show archive.org snapshot — GitHub's guide: what to check, typical failure modes, process
Learn
What a review is for
Read Google's Standard of Code Review Show archive.org snapshot and What to look for in a code review Show archive.org snapshot . Two ideas to take away:
- There is no perfect code, only better code. A review should improve the code base, not block progress until everything is polished.
- Look at design and correctness before naming and style. A well-formatted solution to the wrong problem is still wrong.
Then read Navigating a CL in review Show archive.org snapshot and How to write code review comments Show archive.org snapshot .
Writing comments
- Explain why. "Extract this into a method" is a command. "This logic is repeated in
MoviesController#update, extracting it avoids the two drifting apart" is a review comment. - Mark severity so the author knows what must change and what is optional. A common convention is a prefix like
nit:for small optional things. See Conventional Comments Show archive.org snapshot for one such vocabulary. - If you don't understand something, ask. "Why is this check needed?" is a legitimate review comment, and often surfaces a bug or a missing comment.
- Say what is good. Reviews that only list problems make people defensive and hide what should be repeated.
- Review the code, not the person.
Our process
Re-read the Gatekeeping guide and the merge request checklist from the card Our process. As a reviewer, the checklist tells you what the author should already have covered.
Reviewing agent-generated code
Code from an agent looks finished: consistent formatting, confident comments, tests that pass. That makes it harder to review, not easier. Treat it like code from a fast, well-read colleague who has never seen production and doesn't know your requirements. Read Review AI-generated code Show archive.org snapshot from the GitHub docs.
Typical failure modes to look for:
- Solves a neighbor problem. The code does something plausible that isn't quite what was asked. Check the change against the original requirement, not against the agent's summary of it.
- Invented APIs. Methods, options or gems that don't exist or don't behave as used. Look up anything you don't recognize.
- Tautological tests. Tests that assert what the implementation does instead of what the requirement says, or tests that can't fail. Read the assertions.
-
Deleted or weakened tests. A failing test "fixed" by changing the expectation. Check the diff of
spec/as carefully asapp/. - Missing edge cases. Blank input, nil, empty collections, unauthorized users, concurrent updates. Agents optimize for the happy path.
- Scope creep. Unrelated refactorings, extra abstractions, new dependencies. Every extra line needs a reason.
- Unverified claims. "All tests pass" is a claim. Run them.
And the rule that applies to everything you ship: if you can't explain a hunk, you haven't reviewed it.
Exercises
1. Review your past self
Pick a MovieDB commit you made three or more months ago, ideally a larger one. Review it as if a colleague had opened a merge request with it. Write your review comments into a text file: at least five comments, each with a severity marker and a why. Include at least one positive comment.
Discuss with the agent which of your comments are about design and correctness, and which are style. Would you have caught the important ones first?
2. Review a real merge request
Ask your mentor for a merged merge request from a real project, with the original review comments hidden. Review the diff blind and write your comments. Then compare with the actual review: what did the reviewer catch that you missed, and why? What did you catch that they didn't?
3. Review agent-generated code
For this exercise, switch to builder mode with /trainee-mode builder.
Ask the agent to implement a small MovieDB feature from this deliberately short description: "Users can mark movies as favorites and see a list of their favorites." Don't clarify anything, let the agent make its own assumptions.
Then review the result as thoroughly as you would a colleague's merge request:
- Check the change against the requirement. What did the agent assume that you would have decided differently?
- Go through the failure modes listed above one by one.
- Run the tests yourself. Read every assertion.
- Pick the two hunks you understand least and have the agent explain them until you could explain them to your mentor.
Write your review comments, then have the agent address them in a second round. Review that round, too.
Discuss with your mentor: how long did the review take compared to the implementation? Where would you have saved time with a better initial prompt?
4. Spot the planted bug
Still in builder mode, ask the agent to implement another small MovieDB feature — and to plant exactly two things, without telling you where:
- one subtle bug that would be fatal in production,
- one piece of code that ignores the style of the surrounding code base.
Tell the agent to skip its usual handover walkthrough for this change — for once you want to go in blind.
Review the change and find both. Write your comments as you would for a colleague, with severity markers.
Then check yourself: which one did you find first? A review that catches the style problem and misses the bug is a failed review — and it is the most common way agent-generated code gets waved through.
