Code Review

Code Review

Guide proper code review practices emphasizing technical rigor, evidence-based claims, and verification over performative responses.

Overview

Code review requires three distinct practices:

1. Receiving feedback - Technical evaluation over performative agreement 2. Requesting reviews - Systematic review via code-reviewer subagent 3. Verification gates - Evidence before any completion claims

Each practice has specific triggers and protocols detailed in reference files.

Core Principle

Technical correctness over social comfort. Verify before implementing. Ask before assuming. Evidence before claims.

When to Use This Skill

Receiving Feedback

Trigger when:

• Receiving code review comments from any source
• Feedback seems unclear or technically questionable
• Multiple review items need prioritization
• External reviewer lacks full context
• Suggestion conflicts with existing decisions

Reference: references/code-review-reception.md

Requesting Review

Trigger when:

• Completing tasks in subagent-driven development (after EACH task)
• Finishing major features or refactors
• Before merging to main branch
• Stuck and need fresh perspective
• After fixing complex bugs

Reference: references/requesting-code-review.md

Verification Gates

Trigger when:

• About to claim tests pass, build succeeds, or work is complete
• Before committing, pushing, or creating PRs
• Moving to next task
• Any statement suggesting success/completion
• Expressing satisfaction with work

Reference: references/verification-before-completion.md

Quick Decision Tree

SITUATION?
│
├─ Received feedback
│  ├─ Unclear items? → STOP, ask for clarification first
│  ├─ From human partner? → Understand, then implement
│  └─ From external reviewer? → Verify technically before implementing
│
├─ Completed work
│  ├─ Major feature/task? → Request code-reviewer subagent review
│  └─ Before merge? → Request code-reviewer subagent review
│
└─ About to claim status
   ├─ Have fresh verification? → State claim WITH evidence
   └─ No fresh verification? → RUN verification command first

Receiving Feedback Protocol

Response Pattern

READ → UNDERSTAND → VERIFY → EVALUATE → RESPOND → IMPLEMENT

Key Rules

• ❌ No performative agreement: "You're absolutely right!", "Great point!", "Thanks for [anything]"
• ❌ No implementation before verification
• ✅ Restate requirement, ask questions, push back with technical reasoning, or just start working
• ✅ If unclear: STOP and ask for clarification on ALL unclear items first
• ✅ YAGNI check: grep for usage before implementing suggested "proper" features

Source Handling

• Human partner: Trusted - implement after understanding, no performative agreement
• External reviewers: Verify technically correct, check for breakage, push back if wrong

Full protocol: references/code-review-reception.md

Requesting Review Protocol

When to Request

• After each task in subagent-driven development
• After major feature completion
• Before merge to main

Process

1. Get git SHAs: BASE_SHA=$(git rev-parse HEAD~1) and HEAD_SHA=$(git rev-parse HEAD) 2. Dispatch code-reviewer subagent via Task tool with: WHAT_WAS_IMPLEMENTED, PLAN_OR_REQUIREMENTS, BASE_SHA, HEAD_SHA, DESCRIPTION 3. Act on feedback: Fix Critical immediately, Important before proceeding, note Minor for later

Full protocol: references/requesting-code-review.md

Verification Gates Protocol

The Iron Law

NO COMPLETION CLAIMS WITHOUT FRESH VERIFICATION EVIDENCE

Gate Function

IDENTIFY command → RUN full command → READ output → VERIFY confirms claim → THEN claim

Skip any step = lying, not verifying

Requirements

• Tests pass: Test output shows 0 failures
• Build succeeds: Build command exit 0
• Bug fixed: Test original symptom passes
• Requirements met: Line-by-line checklist verified

Red Flags - STOP

Using "should"/"probably"/"seems to", expressing satisfaction before verification, committing without verification, trusting agent reports, ANY wording implying success without running verification

Full protocol: references/verification-before-completion.md

Integration with Workflows

• Subagent-Driven: Review after EACH task, verify before moving to next
• Pull Requests: Verify tests pass, request code-reviewer review before merge
• General: Apply verification gates before any status claims, push back on invalid feedback

Bottom Line

1. Technical rigor over social performance - No performative agreement 2. Systematic review processes - Use code-reviewer subagent 3. Evidence before claims - Verification gates always

Verify. Question. Then implement. Evidence. Then claim.

Code Review Reception

Overview

Code review requires technical evaluation, not emotional performance.

Core principle: Verify before implementing. Ask before assuming. Technical correctness over social comfort.

The Response Pattern

WHEN receiving code review feedback:

1. READ: Complete feedback without reacting
2. UNDERSTAND: Restate requirement in own words (or ask)
3. VERIFY: Check against codebase reality
4. EVALUATE: Technically sound for THIS codebase?
5. RESPOND: Technical acknowledgment or reasoned pushback
6. IMPLEMENT: One item at a time, test each

Forbidden Responses

NEVER:

• "You're absolutely right!" (explicit CLAUDE.md violation)
• "Great point!" / "Excellent feedback!" (performative)
• "Let me implement that now" (before verification)

INSTEAD:

• Restate the technical requirement
• Ask clarifying questions
• Push back with technical reasoning if wrong
• Just start working (actions > words)

Handling Unclear Feedback

IF any item is unclear:
  STOP - do not implement anything yet
  ASK for clarification on unclear items

WHY: Items may be related. Partial understanding = wrong implementation.

Example:

your human partner: "Fix 1-6"
You understand 1,2,3,6. Unclear on 4,5.

❌ WRONG: Implement 1,2,3,6 now, ask about 4,5 later
✅ RIGHT: "I understand items 1,2,3,6. Need clarification on 4 and 5 before proceeding."

Source-Specific Handling

From your human partner

• Trusted - implement after understanding
• Still ask if scope unclear
• No performative agreement
• Skip to action or technical acknowledgment

From External Reviewers

BEFORE implementing:
  1. Check: Technically correct for THIS codebase?
  2. Check: Breaks existing functionality?
  3. Check: Reason for current implementation?
  4. Check: Works on all platforms/versions?
  5. Check: Does reviewer understand full context?

IF suggestion seems wrong:
  Push back with technical reasoning

IF can't easily verify:
  Say so: "I can't verify this without [X]. Should I [investigate/ask/proceed]?"

IF conflicts with your human partner's prior decisions:
  Stop and discuss with your human partner first

your human partner's rule: "External feedback - be skeptical, but check carefully"

YAGNI Check for "Professional" Features

IF reviewer suggests "implementing properly":
  grep codebase for actual usage

  IF unused: "This endpoint isn't called. Remove it (YAGNI)?"
  IF used: Then implement properly

your human partner's rule: "You and reviewer both report to me. If we don't need this feature, don't add it."

Implementation Order

FOR multi-item feedback:
  1. Clarify anything unclear FIRST
  2. Then implement in this order:
     - Blocking issues (breaks, security)
     - Simple fixes (typos, imports)
     - Complex fixes (refactoring, logic)
  3. Test each fix individually
  4. Verify no regressions

When To Push Back

Push back when:

• Suggestion breaks existing functionality
• Reviewer lacks full context
• Violates YAGNI (unused feature)
• Technically incorrect for this stack
• Legacy/compatibility reasons exist
• Conflicts with your human partner's architectural decisions

How to push back:

• Use technical reasoning, not defensiveness
• Ask specific questions
• Reference working tests/code
• Involve your human partner if architectural

Signal if uncomfortable pushing back out loud: "Strange things are afoot at the Circle K"

Acknowledging Correct Feedback

When feedback IS correct:

✅ "Fixed. [Brief description of what changed]"
✅ "Good catch - [specific issue]. Fixed in [location]."
✅ [Just fix it and show in the code]

❌ "You're absolutely right!"
❌ "Great point!"
❌ "Thanks for catching that!"
❌ "Thanks for [anything]"
❌ ANY gratitude expression

Why no thanks: Actions speak. Just fix it. The code itself shows you heard the feedback.

If you catch yourself about to write "Thanks": DELETE IT. State the fix instead.

Gracefully Correcting Your Pushback

If you pushed back and were wrong:

✅ "You were right - I checked [X] and it does [Y]. Implementing now."
✅ "Verified this and you're correct. My initial understanding was wrong because [reason]. Fixing."

❌ Long apology
❌ Defending why you pushed back
❌ Over-explaining

State the correction factually and move on.

Common Mistakes

Real Examples

Performative Agreement (Bad):

Reviewer: "Remove legacy code"
❌ "You're absolutely right! Let me remove that..."

Technical Verification (Good):

Reviewer: "Remove legacy code"
✅ "Checking... build target is 10.15+, this API needs 13+. Need legacy for backward compat. Current impl has wrong bundle ID - fix it or drop pre-13 support?"

YAGNI (Good):

Reviewer: "Implement proper metrics tracking with database, date filters, CSV export"
✅ "Grepped codebase - nothing calls this endpoint. Remove it (YAGNI)? Or is there usage I'm missing?"

Unclear Item (Good):

your human partner: "Fix items 1-6"
You understand 1,2,3,6. Unclear on 4,5.
✅ "Understand 1,2,3,6. Need clarification on 4 and 5 before implementing."

The Bottom Line

External feedback = suggestions to evaluate, not orders to follow.

Verify. Question. Then implement.

No performative agreement. Technical rigor always.

Requesting Code Review

Dispatch code-reviewer subagent to catch issues before they cascade.

Core principle: Review early, review often.

When to Request Review

Mandatory:

• After each task in subagent-driven development
• After completing major feature
• Before merge to main

Optional but valuable:

• When stuck (fresh perspective)
• Before refactoring (baseline check)
• After fixing complex bug

How to Request

1. Get git SHAs:

BASE_SHA=$(git rev-parse HEAD~1)  # or origin/main
HEAD_SHA=$(git rev-parse HEAD)

2. Dispatch code-reviewer subagent:

Use Task tool with code-reviewer type, fill template at code-reviewer.md

Placeholders:

• {WHAT_WAS_IMPLEMENTED} - What you just built
• {PLAN_OR_REQUIREMENTS} - What it should do
• {BASE_SHA} - Starting commit
• {HEAD_SHA} - Ending commit
• {DESCRIPTION} - Brief summary

3. Act on feedback:

• Fix Critical issues immediately
• Fix Important issues before proceeding
• Note Minor issues for later
• Push back if reviewer is wrong (with reasoning)

Example

[Just completed Task 2: Add verification function]

You: Let me request code review before proceeding.

BASE_SHA=$(git log --oneline | grep "Task 1" | head -1 | awk '{print $1}')
HEAD_SHA=$(git rev-parse HEAD)

[Dispatch code-reviewer subagent]
  WHAT_WAS_IMPLEMENTED: Verification and repair functions for conversation index
  PLAN_OR_REQUIREMENTS: Task 2 from docs/plans/deployment-plan.md
  BASE_SHA: a7981ec
  HEAD_SHA: 3df7661
  DESCRIPTION: Added verifyIndex() and repairIndex() with 4 issue types

[Subagent returns]:
  Strengths: Clean architecture, real tests
  Issues:
    Important: Missing progress indicators
    Minor: Magic number (100) for reporting interval
  Assessment: Ready to proceed

You: [Fix progress indicators]
[Continue to Task 3]

Integration with Workflows

Subagent-Driven Development:

• Review after EACH task
• Catch issues before they compound
• Fix before moving to next task

Executing Plans:

• Review after each batch (3 tasks)
• Get feedback, apply, continue

Ad-Hoc Development:

• Review before merge
• Review when stuck

Red Flags

Never:

• Skip review because "it's simple"
• Ignore Critical issues
• Proceed with unfixed Important issues
• Argue with valid technical feedback

If reviewer wrong:

• Push back with technical reasoning
• Show code/tests that prove it works
• Request clarification

See template at: requesting-code-review/code-reviewer.md

Verification Before Completion

Overview

Claiming work is complete without verification is dishonesty, not efficiency.

Core principle: Evidence before claims, always.

Violating the letter of this rule is violating the spirit of this rule.

The Iron Law

NO COMPLETION CLAIMS WITHOUT FRESH VERIFICATION EVIDENCE

If you haven't run the verification command in this message, you cannot claim it passes.

The Gate Function

BEFORE claiming any status or expressing satisfaction:

1. IDENTIFY: What command proves this claim?
2. RUN: Execute the FULL command (fresh, complete)
3. READ: Full output, check exit code, count failures
4. VERIFY: Does output confirm the claim?
   - If NO: State actual status with evidence
   - If YES: State claim WITH evidence
5. ONLY THEN: Make the claim

Skip any step = lying, not verifying

Common Failures

Red Flags - STOP

• Using "should", "probably", "seems to"
• Expressing satisfaction before verification ("Great!", "Perfect!", "Done!", etc.)
• About to commit/push/PR without verification
• Trusting agent success reports
• Relying on partial verification
• Thinking "just this once"
• Tired and wanting work over
• ANY wording implying success without having run verification

Rationalization Prevention

Key Patterns

Tests:

✅ [Run test command] [See: 34/34 pass] "All tests pass"
❌ "Should pass now" / "Looks correct"

Regression tests (TDD Red-Green):

✅ Write → Run (pass) → Revert fix → Run (MUST FAIL) → Restore → Run (pass)
❌ "I've written a regression test" (without red-green verification)

Build:

✅ [Run build] [See: exit 0] "Build passes"
❌ "Linter passed" (linter doesn't check compilation)

Requirements:

✅ Re-read plan → Create checklist → Verify each → Report gaps or completion
❌ "Tests pass, phase complete"

Agent delegation:

✅ Agent reports success → Check VCS diff → Verify changes → Report actual state
❌ Trust agent report

Why This Matters

From 24 failure memories:

• your human partner said "I don't believe you" - trust broken
• Undefined functions shipped - would crash
• Missing requirements shipped - incomplete features
• Time wasted on false completion → redirect → rework
• Violates: "Honesty is a core value. If you lie, you'll be replaced."

When To Apply

ALWAYS before:

• ANY variation of success/completion claims
• ANY expression of satisfaction
• ANY positive statement about work state
• Committing, PR creation, task completion
• Moving to next task
• Delegating to agents

Rule applies to:

• Exact phrases
• Paraphrases and synonyms
• Implications of success
• ANY communication suggesting completion/correctness

The Bottom Line

No shortcuts for verification.

Run the command. Read the output. THEN claim the result.

This is non-negotiable.