Receiving code review

receiving-code-review

When to use

• A PR has review comments (from bots like Copilot, Greptile, Augment, or from human reviewers) and the next step is to address them
• Someone pasted review feedback into the conversation and asks you to "fix it" or "handle it"
• A pair-programming partner gave verbal suggestions about code you just wrote
• You are tempted to reply "you are absolutely right" before reading the rest of the comment

Do NOT use when:

• You are writing a review yourself (different discipline)
• The user explicitly asks for blind implementation of a specific change that is not framed as review feedback
• The feedback is pure style/formatting that a linter should decide automatically — just run the linter

Goal

Treat review feedback as suggestions to evaluate, not orders to execute. Separate correct feedback from reviewer confusion. Push back with technical reasoning when the suggestion is wrong for this codebase. Never agree performatively.

The Iron Law

NO IMPLEMENTATION UNTIL THE FEEDBACK IS UNDERSTOOD AND VERIFIED.

A "fix" implemented against a misread comment is worse than no fix — it ships the wrong behavior under the label of "addressed feedback".

Procedure

1. Read the full comment set before touching code

Read every open comment on the PR first. Comments often relate to each other — fixing comment #3 in isolation can conflict with comment #5. Group them:

• Blocking — breaks behavior, introduces a bug, security issue
• Important — logic is correct but design / readability issue
• Minor — naming, comment style, formatting the linter missed
• Wrong — reviewer misunderstood the code, or suggestion would regress another behavior

2. Restate each comment in your own words

For every comment, write (internally or to the user): "The reviewer is asking me to X because Y."

If you cannot complete that sentence confidently → the comment is unclear. Ask for clarification before implementing anything. Do not implement the clear ones first and ask later — they may be linked.

3. Verify each claim against the code

For each comment classified as blocking/important:

• Reproduce the alleged issue locally (run the test, hit the endpoint, read the actual runtime value — see systematic-debugging)
• Check what the code actually does, not what the reviewer thinks it does
• Check whether the suggested fix would break another test or caller
• Check git blame / history — the current code may be the way it is for a reason
• Consult memory for prior context. Via memory-access, call retrieve(types=["historical-patterns"], keys=<files in the review>, limit=3). A registered historical pattern may confirm the reviewer's concern (accept). For architectural rationale ("why is the current shape intentional?"), check the ADR index docs/decisions/INDEX.md — push back with the cited ADR number.

4. Decide: accept, push back, or escalate

Situation	Response
Reviewer is right, fix is local, no caller impact	Implement, reference the comment in the commit message
Reviewer is right but fix affects other callers	Note the downstream effects in the reply, then implement
Reviewer is wrong — based on misreading the code	Reply with evidence (specific line / test / value), do not change code
Reviewer suggests a feature the codebase does not use (YAGNI)	Reply asking whether the feature is actually needed, do not build speculatively
Reviewer and user / architecture disagree	Escalate to the user before implementing either path

5. Reply in the right place, with the right tone

• For inline PR comments → reply in the thread of that comment, not as a top-level PR comment
• Quote evidence, not opinion — "line 47 already handles this via $x->isNull()" beats "I think that's fine"
• No flattery. No "great catch", "absolutely right", "thanks for noticing". The existing language-and-tone rule already bans this — actions are the acknowledgement
• If you were wrong in your earlier pushback, state it factually and move on. No long apology

6. Implement in priority order, one at a time

1. Blocking issues first
2. Important issues next
3. Minor issues last (or bundle them into a single commit)
4. Wrong / YAGNI: no code change, only a reply with reasoning

Run the relevant tests and linters between each group — do not batch four changes and then run tests once. See verify-before-complete.

Output format

When reporting back to the user after handling review:

1. Triage table — comment → classification → decision
2. Implemented changes — bullet per change with file + commit ref
3. Pushed back — bullet per rejected comment with the evidence
4. Outstanding — anything awaiting clarification, with the specific question

Gotchas

• Bot comments (Copilot, Greptile) are not automatically right. They frequently flag false positives on patterns the codebase uses deliberately. Verify like you would a human comment.
• A comment that reads like a question ("should this be X?") is often a polite way of saying "change it to X". Ask if unclear instead of guessing the register.
• Resolving a comment in the GitHub UI without an accompanying fix or reply silently marks it handled — reviewers may not notice the lack of substance.
• Stacked PRs — a comment on the base PR may already be fixed in the child PR. Check both before touching code.
• A suggestion that passes review aesthetics but fails the test suite is still a regression. Run tests even when "the change is trivial".
• Flattery leaks in as "good point" or "thanks". Delete before sending. The code change itself is the acknowledgement.

Do NOT

• Do NOT reply "you are absolutely right", "great catch", or any flattery variant — actions acknowledge, words do not
• Do NOT implement a suggestion before verifying it against the code
• Do NOT fix the clear items first and ask about the unclear ones later when the items are linked
• Do NOT accept a reviewer's suggestion that conflicts with an explicit architectural decision without raising it
• Do NOT batch multiple unrelated fixes into one commit — reviewers cannot re-review selectively
• Do NOT mark a comment resolved without either a code change or a reply with reasoning

Anti-patterns

• Replying "Fixed!" after a commit that does not actually address the comment (wrong file, missed case)
• Rewriting the comment author's suggestion into your own words without checking whether the reinterpretation still matches intent
• Implementing the YAGNI-suggested feature "just in case" the reviewer comes back
• Silent disagreement — ignoring a comment without a reply

When to hand over to another skill / command

• Executing the fixes across many comments → fix-pr-comments (handles both bot + human reviewers in one pass)
• Running the full verification gate before pushing replies → verify-before-complete
• Writing the commit message that references the review → conventional-commits-writing
• Digging into why the reviewer's scenario actually fails → systematic-debugging

Validation checklist

Before considering review handling done:

• Every open comment has been read, classified, and has a decision
• No comment was implemented without a one-sentence restatement
• Every implemented fix has been verified by a test or runtime check
• Every rejected comment has a reply quoting evidence
• No comment was marked resolved without code change or reply
• No reply contains flattery
• Linters and relevant tests pass after the changes