---
name: receiving-code-review
description: >-
  Use when receiving PR review feedback or code review comments.
  Enforces verification-first response to review feedback with
  anti-sycophancy classification and constructive disagreement protocol.
allowed-tools: Read, Grep, Glob, Bash(gh:*, git:*)
argument-hint: [pr-url-or-number]
---

# Receiving Code Review

## Overview

Blind agreement with review feedback is as harmful as ignoring it. Implementing unverified suggestions introduces bugs, degrades architecture, and teaches reviewers that superficial comments get results.

**Core principle:** Treat every review comment as a hypothesis to be tested, not an instruction to be followed. Evidence determines your response, not authority or politeness.

**Violating the letter of this process is violating the spirit of receiving code review.**

## The Iron Law

```
VERIFY BEFORE IMPLEMENTING — AGREEMENT WITHOUT EVIDENCE IS SYCOPHANCY
```

- Reviewer authority does not replace verification. Senior engineers make mistakes too.
- "Sounds good, fixing now" without reading the referenced code is failure.
- Implementing a wrong fix is worse than the original code — it adds confusion AND a bug.

## When to Use

**Always use for:**
- PR review comments requesting changes
- Code review feedback from teammates or AI agents
- EVAL mode feedback on your implementation
- Post-merge review comments requiring follow-up

**Lighter process acceptable for:**
- Typo fixes and formatting-only feedback (verify the typo exists, then fix)
- Documentation wording suggestions (read the suggestion, apply if clearer)

**Do NOT use for:**
- Automated linter/CI feedback (mechanical — just fix or configure)
- Pair programming feedback (synchronous — different workflow)
- Architectural review requiring re-planning (use `brainstorming` or `writing-plans` skills)
- Giving reviews (use `pr-review` skill instead)

## Quick Reference

### Feedback Classification

| Classification | Criteria | Verification Required | Action |
|---------------|----------|----------------------|--------|
| **Must-Fix** | Bug, crash, security vulnerability, data loss | YES — reproduce the issue | Fix immediately after verification |
| **Evaluate** | Alternative approach, refactoring, performance | YES — compare approaches | Implement if better, discuss if not |
| **Preference** | Style, naming, formatting, subjective taste | NO — check project conventions | Accept if convention-aligned, discuss if not |

### Decision Matrix

| Verification Result | Classification | Action |
|--------------------|---------------|--------|
| Issue reproduced | Must-Fix | Fix and cite the reproduction |
| Issue NOT reproduced | Must-Fix | Reply with evidence, ask for scenario |
| Alternative is better | Evaluate | Adopt with acknowledgment |
| Alternative is equivalent or worse | Evaluate | Decline with evidence and tradeoff analysis |
| Matches project convention | Preference | Accept silently |
| Contradicts project convention | Preference | Cite convention, keep current approach |

---

## Phase 1: Triage & Classify

**Goal:** Categorize every review comment before acting on any of them.

**Process:**
1. Read ALL comments before responding to any
2. Classify each comment as Must-Fix, Evaluate, or Preference
3. Order by priority: Must-Fix first, then Evaluate, then Preference

**Classification decision tree:**

```
Is the reviewer claiming a bug, crash, security issue, or data loss?
├── YES → Must-Fix
└── NO → Is the reviewer suggesting an alternative approach or refactoring?
    ├── YES → Evaluate
    └── NO → Preference
```

**Watch for misclassification:**

| Looks Like | Actually Is | How to Tell |
|-----------|-------------|-------------|
| Must-Fix ("this will crash") | Evaluate (edge case that can't happen in context) | Check call sites and input constraints |
| Preference ("rename this") | Must-Fix (name collision causes shadowing bug) | Check scope for conflicts |
| Evaluate ("use pattern X") | Preference (both patterns are equivalent here) | Compare actual tradeoffs in this context |

**Done when:** Every comment has a classification tag. If unsure, classify UP (Preference → Evaluate, Evaluate → Must-Fix).

---

## Phase 2: Verify

**Goal:** Build evidence for each Must-Fix and Evaluate item before acting.

### Verification Matrix

| Feedback Type | Verification Method | Evidence |
|--------------|-------------------|----------|
| "This will crash/fail on X" | Write a test with input X, run it | Test result (pass/fail) |
| "N+1 query / performance issue" | Add logging, hit the endpoint, measure | Query count, timing data |
| "This violates SRP / SOLID" | Evaluate against project conventions and codebase patterns | Convention references, pattern grep results |
| "Use pattern X instead" | Compare both approaches against existing codebase patterns | `grep` for pattern usage, tradeoff list |
| "Missing error handling" | Trace call path, check if error can actually reach this code | Call chain analysis |
| "Race condition possible" | Analyze concurrent access paths | Sequence diagram or test |
| "Style: rename this variable" | **Skip verification** — this is Preference | Check project conventions only |

### Verification Checklist

For each Must-Fix / Evaluate item:

- [ ] Read the exact code the reviewer references (not just the comment)
- [ ] Check if the claimed issue can actually occur given the call context
- [ ] If a bug claim: write a minimal test to reproduce
- [ ] If an alternative approach: grep codebase for both patterns
- [ ] If a convention claim: check `.ai-rules/` or project style guide
- [ ] Record the evidence (test output, grep results, or analysis)

**Done when:** Every Must-Fix and Evaluate item has a verification result: `reproduced`, `not reproduced`, or `tradeoff analyzed`.

**Guard against over-verification:** Preference items do NOT need verification. If you catch yourself writing a test for a variable rename, stop — you are stalling.

---

## Phase 3: Respond

**Goal:** Act on each item based on classification AND verification evidence.

### Processing Order

1. **Must-Fix items** (process first — they may invalidate other comments)
2. **Evaluate items** (some may become moot after Must-Fix changes)
3. **Preference items** (batch these for efficiency)

### Response Templates

**Must-Fix — Verified:**
> Confirmed. Reproduced with [test/scenario]. Fixed in [commit]. Added regression test.

**Must-Fix — Cannot Reproduce:**
> Attempted to reproduce with [specific steps]. Test passes with [input]. Could you share the exact scenario you're seeing? Here's what I tested: [evidence].

**Evaluate — Adopted:**
> Good point. Adopted [approach] because [evidence it's better in this context]. [Brief tradeoff acknowledged].

**Evaluate — Declined:**
> Considered this. Staying with current approach because [concrete evidence]. The tradeoff: [their approach advantage] vs [current approach advantage]. In this context, [reason current wins].

**Evaluate — Partial Agreement:**
> You're right about [the problem]. However, [proposed solution] would [issue]. Instead, I [alternative fix] which addresses the concern while [preserving X].

**Preference — Accepted:**
> Updated.

**Preference — Convention Conflict:**
> Project convention uses [X pattern] per [rule reference]. Keeping for consistency. Happy to discuss changing the convention project-wide if you'd like.

### Tone Calibration

| Reviewer | Tone | Example |
|---------|------|---------|
| Peer | Direct and evidence-based | "Tested this — passes with null input. What scenario triggers the crash?" |
| Senior / Tech Lead | Respectful but still evidence-based | "Great catch on [valid point]. For [other point], I verified with [test] and it passes — could you clarify the scenario?" |
| External Contributor | Welcoming but factual | "Thanks for the review! I checked [claim] and found [evidence]. Here's my reasoning: [explanation]." |

**Key principle:** Deference is not compliance. You can respect someone's expertise while still requiring evidence for their claims.

**Done when:** Every comment has a response: implemented (with commit reference), declined (with evidence), or discussed (with question).

---

## Phase 4: Re-request Review

**Goal:** Close the feedback loop cleanly.

**Checklist:**
- [ ] All Must-Fix items resolved (implemented or discussed with evidence)
- [ ] All Evaluate items resolved (adopted, declined with reasoning, or discussed)
- [ ] All Preference items addressed (accepted or convention-cited)
- [ ] Tests pass after all changes
- [ ] No unrelated changes mixed into fix commits
- [ ] Summary comment posted listing what was changed and what was discussed

**Summary comment template:**
```
Changes made:
- [Must-Fix] Fixed [issue] — added regression test (commit abc123)
- [Evaluate] Adopted [suggestion] for [reason] (commit def456)
- [Preference] Updated [items] (commit ghi789)

Discussed (no change):
- [item]: [brief reason with evidence link]

Ready for re-review.
```

---

## Disagreement Protocol

When you believe the reviewer is wrong, follow this escalation:

### Step 1: Lead with Evidence

```
Bad:  "I disagree with this suggestion."
Good: "I tested with [input] and it passes. The null case can't reach
       this code path because [caller] validates at line 42. Here's the test: [link]."
```

### Step 2: Acknowledge the Tradeoff

```
Bad:  "My approach is better."
Good: "Your approach has [advantage]. Mine has [advantage]. In this context,
       [mine/yours] fits better because [specific reason]."
```

### Step 3: Offer Alternatives

```
Bad:  "No, I won't change this."
Good: "Instead of [their suggestion], what about [alternative] which
       addresses your concern about [X] while preserving [Y]?"
```

### Step 4: Escalate Appropriately

If you cannot reach agreement after evidence exchange:
- Tag a third reviewer for a tiebreaker
- Reference project conventions or architectural decisions
- Accept and document: "Implementing as requested. Noting that [tradeoff] for future reference."

### When to Accept Despite Disagreement

- Reviewer has project-specific context you lack (institutional knowledge)
- The difference is genuinely marginal and not worth the discussion cost
- Project convention sides with the reviewer (convention trumps preference)
- You've been going back and forth for 3+ rounds (diminishing returns)

---

## Anti-Sycophancy Guide

### Prohibited Responses

| Don't Say | Why It's Harmful |
|-----------|-----------------|
| "Great catch! Fixing now." (without verifying) | You don't know if it's actually a catch |
| "You're absolutely right, I should have caught that." | You're performing contrition, not engineering |
| "Sounds good, will update." (without reading code) | Agreeing to changes you haven't evaluated |
| "Thanks for the thorough review!" (as the only response) | Politeness is not a substitute for engagement |
| "I'll fix all of these." (batch-agreeing) | Each comment needs individual evaluation |

### Anti-Patterns

| Anti-Pattern | What Happens | Instead |
|-------------|-------------|---------|
| **Apologetic capitulation** | "You're right, sorry" → implement unverified change | Verify first, then respond with evidence |
| **Scope creep acceptance** | "While you're at it, also refactor X" → silent addition | Acknowledge, create separate issue/PR |
| **Authority-driven compliance** | Senior said it → must be right → implement blindly | Seniors deserve evidence-based responses too |
| **Fix-then-justify** | Implement first, rationalize after | Evaluate BEFORE implementing |
| **Cherry-picking easy wins** | Fix trivial items, ignore hard critical feedback | Process Must-Fix items FIRST |
| **Silent disagreement** | Disagree privately, implement anyway | State your concern on the review thread |
| **Over-verification stalling** | "I need to verify" as excuse to avoid valid feedback | Preference items don't need verification |
| **Conflating preference with defect** | Treating style feedback as a bug fix | Classify correctly, respond accordingly |
| **Performative agreement** | Agreeing enthusiastically to avoid conflict | Evidence determines response, not comfort |

---

## Red Flags — STOP

These thoughts mean you are about to violate the Iron Law:

| Thought | Reality |
|---------|---------|
| "I'll just fix it, it's faster" | Faster than what? You haven't verified it's broken. |
| "The reviewer is senior, they must be right" | Seniority is not evidence. Verify. |
| "It's just a small change, no need to verify" | Small unverified changes compound into big problems. |
| "I don't want to seem argumentative" | Evidence-based responses are not arguments. |
| "They clearly know more about this" | They might. Verify and find out. |
| "I'll verify later after implementing" | That's fix-then-justify. Evaluate BEFORE implementing. |
| "All their other comments were valid, so this one must be too" | Each comment stands on its own evidence. |
| "Pushing back will delay the PR" | Implementing wrong changes delays it more. |

## Common Rationalizations

| Rationalization | Counter |
|----------------|---------|
| "Verifying takes too long" | Implementing a wrong fix takes longer. Reverting takes even longer. |
| "I trust this reviewer" | Trust is not a verification method. Even trusted reviewers make mistakes. |
| "It's not worth the discussion" | If it's not worth discussing, it's not worth implementing either. |
| "The reviewer will be annoyed if I push back" | Professional reviewers respect evidence-based responses. |
| "I already know they're right" | Then verification will be quick. Do it anyway. |
| "This is just bikeshedding" | If so, classify as Preference and handle accordingly. |

---

## Handling Edge Cases

### Conflicting Reviews

When multiple reviewers give contradictory feedback:
1. Surface the conflict explicitly in a comment tagging both reviewers
2. Present both positions with your analysis
3. Do NOT silently pick one — let the reviewers resolve the conflict
4. If unresolved, defer to project convention or escalate to tech lead

### Vague Feedback

When feedback lacks specifics ("this feels wrong", "not clean enough"):
1. Do NOT guess what the reviewer means
2. Ask a specific clarifying question: "Could you point to the specific part that concerns you? Is it the [data flow / naming / structure]?"
3. Wait for clarification before acting

### Feedback on Framework-Mandated Code

When the reviewer suggests changing code required by the framework:
1. Explain the constraint: "This pattern is required by [framework] — see [docs link]"
2. Offer alternatives within the framework's constraints if they exist

### Review of Test Code

Apply higher scrutiny to suggestions that weaken tests:
- Removing edge case tests → challenge firmly with coverage data
- Simplifying assertions → verify the simplified version still catches regressions
- "This test is unnecessary" → check if it covers a unique code path

---

## Related Skills

- **`pr-review`** — For conducting reviews (the giving side of this skill)
- **`systematic-debugging`** — For investigating issues found during verification
- **`test-driven-development`** — For writing verification tests in Phase 2
- **`writing-plans`** — For re-planning when architectural review feedback requires redesign