# 1 · Understand: what the change is for, before what it says A senior reviewer doesn't open the diff first. They open the ticket. A diff tells you what changed; only the intent tells you whether it should have. This step takes a few minutes and decides what the rest of the review is about. On a small change, I1 is enough: one line of intent, then read. --- ### I1 · Know what the change is for **Rule:** Before reading any code, find out what the change is meant to do. Read the pull request description, the linked ticket or issue, and any discussion on it. If the person asked you to review it, read what they said about it. Then write the intent in one line, in the author's terms: "Stop the invoice total counting cancelled items twice." If there's no description and no ticket, say so, and write the intent you inferred from the code as an assumption the author can correct. **Why:** A change can be clean, tested and wrong: it fixes the symptom on one page and leaves it on three, or solves a different problem from the one in the ticket. Without the intent you can only judge whether the code is tidy, and the author already knows how to write tidy code. They need someone to check that it does the job. **Agent trap:** going straight to the diff and reviewing what's on the screen. The ticket says "users on the free plan see the export button and get an error", the change hides the button, and you approve it. Nobody checked that paying users still see it, because you never read who the button was for. **Check:** your review starts from a one-line intent you can point to (the ticket, the description, or your stated assumption). Each blocking comment you write connects to that intent, to harm (D2), or to the project's rules (I4). ### I2 · Know what kind of review is wanted **Rule:** Find out what the person wants from this review, and review at that level. A draft or "is this the right approach?" wants comments on the approach, not on names. A ready-to-merge change wants a full review. "Can you look at the caching part?" wants that part, done well, and a note that you didn't review the rest. If nothing says, a change marked ready gets a full review; a draft gets the approach. **Why:** Line comments on a draft waste the author's time: half those lines will be rewritten. A quick glance at a change about to ship gives false confidence. Either way the person got a review, just not the one they needed. **Agent trap:** twenty line comments on a draft whose author asked one question: "should this be a background job or run on request?". You never answered it. Or approving a change after reviewing only the file you were pointed at, without saying the rest wasn't looked at. **Check:** say in one line what kind of review you're doing ("Approach only, it's a draft", "Full review", "The caching part only, as asked"). Your summary repeats it (F2). ### I3 · Find out how big and how risky it is **Rule:** Before reading line by line, look at the shape of the change: how many files and lines, which areas it touches, and whether any of them can hurt someone badly if wrong: sign-in and permissions, payments, deleting or migrating stored data, anything other code or other teams depend on (a public API, a shared library, a database column), and anything that runs on its own (a scheduled job, a queue worker). Run `checks/diffmap.js` on a large change. Spend your attention in proportion to the risk, not to the line count. **Why:** The dangerous line is rarely in the biggest file. A two-line change to a permission check can expose every customer's data; a 300-line new page can, at worst, look wrong. A reviewer who reads top to bottom gives the same attention to both. **Agent trap:** spending the review on the long new React component because it's most of the diff, and skimming past the one-line change in `middleware/auth.js` that turned `&&` into `||`. **Check:** before you start reading, name the riskiest one to three places in the change and why. Your review covers each of them closely, or says it didn't. ### I4 · Learn this project's rules before you judge by yours **Rule:** Before commenting on style or structure, find out how this project does things: its lint and formatter config, `CONTRIBUTING.md`, `AGENTS.md` or `CLAUDE.md`, a pull request template or review checklist, and two or three files like the ones changed (Code craft B2). Judge the change against the project, not against your habits. Don't comment on anything the formatter or linter already decides; if those tools would fail, say so once. **Why:** A reviewer who asks for their own preferences ("I'd use a class here", "prefer `async`/`await`") in a codebase that does it the other way pushes the author to make the code less consistent. The author also has to argue with you about taste, which takes longer than fixing a bug. **Agent trap:** asking for named exports, early returns, or `const` arrow functions because that's common on the internet, in a project whose every other file does the opposite. Or flagging line length in a project with a formatter that sets it. **Check:** for each style or structure comment, name the project rule or the neighbouring file it comes from. If you can't, drop it or mark it as your preference, a nit (W1). **Limit:** if the project's own pattern is unsafe (secrets in code, queries built from input: Code craft C8, C9), being consistent doesn't make it fine. Flag it in the change you're reviewing, and mention the existing ones once.