/ia-receiving-code-review

name: ia-receiving-code-review
class: discipline
description: >-
  Process code review feedback critically: check correctness before acting, push back
  on incorrect suggestions, no performative agreement. Use when responding to
  PR/MR review comments or implementing reviewer suggestions received from others.

Receiving Code Review

Core Principle

Verify before implementing. Technical correctness matters more than social comfort. A reviewer can be wrong -- blindly implementing bad suggestions creates bugs.

Response Pattern

For each piece of feedback, follow this sequence:

**0. Prior feedback check (re-reviews only)** -- if this is not the first review round, check whether previously flagged issues were addressed before processing new comments. Compare the current diff against prior review threads (`gh api repos/{owner}/{repo}/pulls/{pr}/comments`). Surface any that were ignored or only partially fixed -- these take priority over new feedback.

1. **Read** -- Understand what's being suggested and why 2. **Verify** -- Is the suggestion technically correct for THIS codebase? 3. **Evaluate** -- Does it improve the code, or is it preference/style? 4. **Respond** -- Agree with evidence, disagree with evidence, or ask for clarification 5. **Implement** -- Only after verification confirms the suggestion is correct

Triage all feedback first (see Implementation Order below), then implement one item at a time. Don't batch-implement everything at once.

Handling Unclear Feedback

When feedback is ambiguous or incomplete:

• **Stop** -- do not implement anything unclear
• Clarify ALL unclear items before implementing ANY of them (they may be related)
• Ask specific questions: "Are you suggesting X or Y?" not "Can you elaborate?"
• If the reviewer's intent is clear but the technical approach is wrong, say so

**Batched clarification for critical-path ambiguity:** When multiple ambiguous findings land on critical-path code (auth, payments, data migrations, permission checks) AND the `AskUserQuestion` tool is available, batch up to 4 of them into a single call rather than asking one at a time. Each question's header is the truncated filename and line, and the options are `Valid / False positive / Defer`. Skip the batched ask entirely when ambiguous findings are only on non-critical paths — just auto-triage those and move on. If `AskUserQuestion` is not available, fall back to a single prose block listing all ambiguous items numbered, asking for Valid/False-positive/Defer decisions. The batching limit is 4 because it caps cleanly at that size; asking more becomes noise rather than judgment.

Source-Specific Handling

From the user (project owner)

• Trusted context -- they know the codebase and business requirements
• Implement after understanding, but still verify technical correctness
• Ask clarifying questions when the intent is clear but the approach seems risky
• No performative agreement -- just acknowledge and implement

From automated review agents

• **Skeptical by default** -- agents lack full context
• Verify every suggestion against the actual codebase
• Check for YAGNI violations (agents love adding "just in case" code)
• Discard suggestions that contradict project conventions (check CLAUDE.md)
• Agents may flag things that are intentional design decisions -- check before changing

From external reviewers (PR comments, open source)

• Verify technical correctness for THIS stack and codebase
• Check if the suggestion applies to this version of the framework/library
• Push back if the reviewer lacks context about architectural decisions
• Distinguish between "this is wrong" and "I would do it differently"

When to Push Back

Push back (with evidence) when a suggestion:

• **Breaks existing functionality** -- "This would break X because Y depends on Z"
• **Violates project conventions** -- "Our CLAUDE.md specifies we do it this way because..."
• **Is technically incorrect** -- "This API was deprecated in v3. We're on v4 which uses..."
• **Adds unnecessary complexity** -- "This handles a case that can't occur because..."
• **Is unused (YAGNI)** -- when a reviewer suggests "implementing properly", grep the codebase for actual usage FIRST. Zero callers? Suggest removal: "This endpoint isn't called. Remove it (YAGNI)?" If used, implement properly.
• **Conflicts with architectural decisions** -- "We chose X over Y in the brainstorm because..."

**Valid evidence:** code references (`file:line`), test output, git blame/log, framework docs, reproduction steps, grep results showing usage patterns. **Not evidence:** "I think", "it should work", "it's fine", appeals to convention without citing the convention, or restating the original code as justification.

False-Positive Taxonomy (for dismissed suggestions)

When dismissing a suggestion (AUTO-DECLINE, manual push-back), tag the dismissal with one of four categories so the reviewer sees structured reasoning, not a bare "no":

| Category | Reviewer's response cited | Evidence required | Maps to "When to Push Back" | |----------|--------------------------|-------------------|------------------------------| | **FP-ASSUMPTION** | Reviewer assumed behavior that doesn't match the code | Quote the specific line that contradicts the assumption | "Is technically incorrect" | | **FP-CONVENTION** | Suggestion conflicts with this project's conventions | Cite the CLAUDE.md rule, ADR, or the established pattern in `file:line` | "Violates project conventions" | | **FP-ALREADY-HANDLED** | The concern is handled elsewhere (parent function, middleware, framework) | Show the existing handler in `file:line` | "Adds unnecessary complexity" | | **FP-OUT-OF-SCOPE** | Valid concern but belongs in a separate change | State where it will be tracked (issue, todo, next PR) | YAGNI / scope creep |

Use the tag in the reply: "FP-ALREADY-HANDLED: null check happens in `auth/middleware.ts:42` before this handler runs. Keeping as-is." Stru