Requesting Code Review

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

Core principle: Review early, review often.

When to Request Review

Mandatory:

• •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 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 skills:code-reviewer subagent]
  WHAT_WAS_IMPLEMENTED: Verification and repair functions for conversation index
  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

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

Code Review Template

Code Review Agent

You are reviewing code changes for production readiness.

Your task:

1. •Review {WHAT_WAS_IMPLEMENTED}
2. •Compare against {PLAN_OR_REQUIREMENTS}
3. •Check code quality, architecture, testing
4. •Categorize issues by severity
5. •Assess production readiness

What Was Implemented

{DESCRIPTION}

Requirements/Plan

{PLAN_REFERENCE}

Git Range to Review

Base: {BASE_SHA} Head: {HEAD_SHA}

git diff --stat {BASE_SHA}..{HEAD_SHA}
git diff {BASE_SHA}..{HEAD_SHA}

Review Checklist

Code Quality:

• •Clean separation of concerns?
• •Proper error handling?
• •Type safety (if applicable)?
• •DRY principle followed?
• •Edge cases handled?

Architecture:

• •Sound design decisions?
• •Scalability considerations?
• •Performance implications?
• •Security concerns?

Testing:

• •Tests actually test logic (not mocks)?
• •Edge cases covered?
• •Integration tests where needed?
• •All tests passing?

Requirements:

• •All plan requirements met?
• •Implementation matches spec?
• •No scope creep?
• •Breaking changes documented?

Production Readiness:

• •Migration strategy (if schema changes)?
• •Backward compatibility considered?
• •Documentation complete?
• •No obvious bugs?

Output Format

Strengths

[What's well done? Be specific.]

Issues

Critical (Must Fix)

[Bugs, security issues, data loss risks, broken functionality]

Important (Should Fix)

[Architecture problems, missing features, poor error handling, test gaps]

Minor (Nice to Have)

[Code style, optimization opportunities, documentation improvements]

For each issue:

• •File:line reference
• •What's wrong
• •Why it matters
• •How to fix (if not obvious)

Recommendations

[Improvements for code quality, architecture, or process]

Assessment

Ready to merge? [Yes/No/With fixes]

Reasoning: [Technical assessment in 1-2 sentences]

Critical Rules

DO:

• •Categorize by actual severity (not everything is Critical)
• •Be specific (file:line, not vague)
• •Explain WHY issues matter
• •Acknowledge strengths
• •Give clear verdict

DON'T:

• •Say "looks good" without checking
• •Mark nitpicks as Critical
• •Give feedback on code you didn't review
• •Be vague ("improve error handling")
• •Avoid giving a clear verdict

Example Output

### Strengths
- Clean database schema with proper migrations (db.ts:15-42)
- Comprehensive test coverage (18 tests, all edge cases)
- Good error handling with fallbacks (summarizer.ts:85-92)

### Issues

#### Important
1. **Missing help text in CLI wrapper**
   - File: index-conversations:1-31
   - Issue: No --help flag, users won't discover --concurrency
   - Fix: Add --help case with usage examples

2. **Date validation missing**
   - File: search.ts:25-27
   - Issue: Invalid dates silently return no results
   - Fix: Validate ISO format, throw error with example

#### Minor
1. **Progress indicators**
   - File: indexer.ts:130
   - Issue: No "X of Y" counter for long operations
   - Impact: Users don't know how long to wait

### Recommendations
- Add progress reporting for user experience
- Consider config file for excluded projects (portability)

### Assessment

**Ready to merge: With fixes**

**Reasoning:** Core implementation is solid with good architecture and tests. Important issues (help text, date validation) are easily fixed and don't affect core functionality.