Requesting Code Review

Requesting Code Review

Dispatch super-agent-skills:code-reviewer subagent to catch issues before they cascade. The reviewer gets precisely crafted context for evaluation — never your session's history. This keeps the reviewer focused on the work product, not your thought process, and preserves your own context for continued work.

Core principle: Review early, review often.

Related: super-agent-skills:receiving-code-review — for responding to review feedback with technical rigor.

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 super-agent-skills: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 super-agent-skills: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]

The Five-Axis Review Framework

Instruct the code-reviewer to evaluate across these dimensions:

1. Correctness

• Does the code match spec/task requirements?
• Are edge cases handled (null, empty, boundary values)?
• Are error paths handled (not just the happy path)?
• Do tests actually test the right things?

2. Readability & Simplicity

• Are names descriptive and consistent with project conventions?
• Is the control flow straightforward?
• Could this be done in fewer lines?
• Are abstractions earning their complexity?
• Are there dead code artifacts?

3. Architecture

• Does the change follow existing patterns or introduce a new one? If new, is it justified?
• Does it maintain clean module boundaries?
• Is there code duplication that should be shared?
• Are dependencies flowing in the right direction (no circular dependencies)?
• Does this change increase or decrease coupling between modules?
• Are new abstractions justified by multiple use cases (not speculative)?
• Could this change break consumers of the modified interfaces (Hyrum's Law)?
• Is the dependency graph acyclic? Does this change introduce cycles?
• For new public APIs: is the interface minimal? Could it be made smaller?

4. Security

• Is user input validated and sanitized?
• Are secrets kept out of code, logs, and version control?
• Are SQL queries parameterized?
• Are outputs encoded to prevent XSS?
• Is data from external sources treated as untrusted?
• See references/security-checklist.md for full checklist.

5. Performance

• Any N+1 query patterns?
• Any unbounded loops or unconstrained data fetching?
• Any synchronous operations that should be async?
• Any missing pagination on list endpoints?
• See references/performance-checklist.md for full checklist.

Change Sizing

Small, focused changes are easier to review:

~100 lines changed  → Good. Reviewable in one sitting.
~300 lines changed  → Acceptable if it's a single logical change.
~1000 lines changed → Too large. Split it.

If a change is too large, ask the author to split using: vertical slices, by file group, or horizontal layers.

Review Sizing Gate

Before dispatching the reviewer, enforce the sizing rules as a hard gate:

# Count lines changed
LINES_CHANGED=$(git diff --stat $BASE_SHA..$HEAD_SHA | tail -1 | awk '{print $4+$6}')

This enforces what "Change Sizing" recommends. A 2000-line review catches fewer bugs than two 1000-line reviews because reviewer attention degrades with size.

Domain Skill Sub-Checks

For security-sensitive changes (auth, user input, external data), the reviewer should additionally invoke super-agent-skills:security-and-hardening for a focused security review.

For performance-sensitive changes (database queries, rendering, data processing), the reviewer should additionally invoke super-agent-skills:performance-optimization for a focused performance review.

For architecture-significant changes (new abstractions, modified module boundaries, new public APIs), additionally dispatch super-agent-skills:architecture-reviewer for a focused design review.

Self-Healing Review Loop

When the reviewer finds Critical or Important issues, automate the fix-and-re-review cycle instead of manual back-and-forth.

The Loop

Reviewer returns issues
    │
    ├── Critical/Important issues found
    │   │
    │   ▼
    │   Dispatch fix agent with reviewer feedback as instructions
    │   │
    │   ▼
    │   Fix agent makes changes and commits
    │   │
    │   ▼
    │   Re-dispatch reviewer to verify fixes
    │   │
    │   ├── Issues resolved → Proceed
    │   └── Issues remain → Loop (max 3 rounds)
    │
    ├── Only Minor/Nit issues → Proceed (note for later)
    │
    └── No issues → Proceed

Rules

• Max 3 rounds. If the issue isn't fixed after 3 review-fix cycles, escalate to human. Three rounds of the same issue means the problem is in the spec or architecture, not the implementation.
• Fix agent gets the reviewer's exact feedback — file:line references, what's wrong, how to fix. No guessing.
• Each round re-reviews only the fix — don't re-review the entire change from scratch.
• Track the loop count. If a project consistently hits 3 rounds, the plans need more detail or the acceptance criteria are ambiguous.

Anti-Rationalizations

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

Anti-Rationalizations

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

Handoff

When review is complete and all issues are resolved, prompt the user:

"Review passed. What would you like to do next?"

A) Wrap up — update backlog, changelog, commit, move to next item B) Ship it — pre-merge checklist, merge/PR, branch cleanup C) Keep going — continue working, more changes needed

Or tell me what you'd like to do.

Route based on response:

• A / "wrap up" / "done" / "checkpoint" → invoke super-agent-skills:wrap-up
• B / "ship" / "merge" / "PR" → invoke super-agent-skills:finishing-a-development-branch
• C / "keep going" / "more changes" → return to implementation
• Specific instruction → follow it