/dev-review - Code Review

Skill Awareness: See skills/_registry.md for all available skills.

• Before: After /dev-coding implementation and /dev-test passes
• Reads: _quality-attributes.md (ALL Levels - verification)
• If issues: Fix with /dev-coding, then review again
• If major changes: Create CR via /debrief

Review code changes for quality, security, and adherence to standards.

When to Use

• After implementing a feature
• Before merging a PR
• To get feedback on approach
• To catch issues before production

Usage

/dev-review                      # Review uncommitted changes
/dev-review --staged             # Review staged changes only
/dev-review src/auth/            # Review specific directory
/dev-review UC-AUTH-001          # Review changes for specific UC
/dev-review billing --brd        # Verify all BRD use cases covered

Input

Reviews can be based on:

1. Git diff - Uncommitted or staged changes
2. File list - Specific files to review
3. UC reference - Files changed for a use case (from spec)

Output

## Review Summary

**Verdict**: ✅ Approve | ⚠️ Request Changes | ❓ Needs Discussion

**Stats**: X files, Y additions, Z deletions

### Issues Found

#### 🔴 Critical (must fix)
- [security] SQL injection risk in `src/api/users.ts:45`
- [bug] Null pointer in `src/utils/parse.ts:23`

#### 🟡 Important (should fix)
- [performance] N+1 query in `src/api/posts.ts:67`
- [error-handling] Unhandled promise rejection

#### 🔵 Suggestions (nice to have)
- [style] Consider extracting to helper function
- [naming] `data` is too generic, suggest `userProfile`

### By File
- `src/api/auth.ts` - 2 issues
- `src/components/Form.tsx` - 1 suggestion

### Positives
- Good error handling in login flow
- Clean separation of concerns

Expected Outcome

Review report with verdict and categorized issues.

Report includes:

• Verdict (Approve/Request Changes/Needs Discussion)
• Issues found (by severity: critical/important/suggestion)
• Issues by file
• Positives (acknowledge good work)
• Spec compliance (if UC provided)

Review Workflow

1. Load Context (once):

• Read plans/brd/tech-context.md → Identify stack(s)
• Load knowledge/stacks/{stack}/_index.md → Focus on "For /dev-review" section
• Read _quality-attributes.md → Get all level checklists
• Read spec (if UC provided) → Get requirements
• Get git diff or file list → Understand what changed

2. Analyze Changes:

• Review each changed file
• Apply: Security + Quality + Conventions + Stack-Specific checks
• Note issues by severity (critical/important/suggestion)
• Acknowledge good practices

3. Verify Spec Compliance (if UC provided):

• Check all requirements met
• Identify missing items
• Flag unauthorized additions

4. Generate Report:

• Verdict (Approve/Request Changes/Needs Discussion)
• Issues by severity
• Issues by file
• Spec compliance status
• Positives

5. Offer Fix (if issues found):

• Critical/Important → Suggest auto-fix via /dev-coding
• Suggestions → User decides

Success Criteria

• Stack knowledge loaded and applied
• Security risks identified
• Quality issues found
• Spec compliance verified
• Conventions checked
• Framework patterns verified (from stack knowledge)
• Constructive feedback with suggested fixes
• Clear verdict given

Review Focus Areas

Security

• Input validation (sanitized, injection prevented, XSS prevented)
• Authentication (required where needed, tokens validated, sessions secure)
• Authorization (permissions checked, data access restricted, admin protected)
• Data protection (passwords hashed, sensitive data not logged, no secrets in code)
• API security (rate limiting, CORS, no sensitive data in URLs)

Quality

• Error handling (caught, user-friendly messages, logged, not swallowed)
• Performance (no N+1 queries, pagination on lists, async heavy ops, no memory leaks)
• Maintainability (readable, functions not too long, no magic values, DRY)
• Testing (new code tested, edge cases covered, meaningful tests)

Conventions

• Naming (variables, files, components per scout)
• Structure (correct location, follows patterns, organized imports)
• Style (matches configs, consistent, no linting errors)
• Git (proper commit format, no unrelated changes, no debug code)

Spec Compliance

• Requirements met (all implemented)
• Requirements not met (missing items)
• Not in spec (unauthorized additions)

Stack-Specific

CRITICAL: Load framework-specific review checklists from references/stack-checks.md. Also load knowledge/stacks/{stack}/_index.md "For /dev-review" section for deeper patterns.

Context Sources

Read to understand what was changed:

• git diff (uncommitted or staged)
• Specific files (if provided)

Read to understand standards:

Step 1: Load stack knowledge (CRITICAL)

• Read plans/brd/tech-context.md → Identify stack(s)
• Extract stack names (look for "Primary Stack" or "Tech Stack" section)
• Load corresponding knowledge files:
  • If "React" → Read knowledge/stacks/react/_index.md → Focus on "For /dev-review" section
  • If "Vue" → Read knowledge/stacks/vue/_index.md → Focus on "For /dev-review" section
  • If "Next.js" → Read knowledge/stacks/nextjs/_index.md → Focus on "For /dev-review" section
  • If "Nuxt" → Read knowledge/stacks/nuxt/_index.md → Focus on "For /dev-review" section
  • If "Directus" → Read knowledge/stacks/directus/_index.md → Focus on "For /dev-review" section

Step 2: Load project and spec context

• tech-context.md - Project conventions
• architecture.md - Architecture decisions (if exists)
• spec - Requirements (if UC provided)
• _quality-attributes.md - ALL levels (Architecture, Specification, Implementation, Review)
• Config files (.eslintrc, tsconfig.json, etc.)

Severity Levels

Level	Icon	Meaning	Action
Critical	🔴	Security risk, bug, breaks functionality	Must fix before merge
Important	🟡	Performance, maintainability issues	Should fix
Suggestion	🔵	Style, improvements	Nice to have
Positive	✅	Good practice noted	Encouragement

Verdicts

Verdict	When
✅ Approve	No critical/important issues
⚠️ Request Changes	Has critical or multiple important issues
❓ Needs Discussion	Unclear requirements, architectural concerns

Review Best Practices

See references/review-examples.md for constructive feedback patterns, example reviews, and fix loop details.

Key principles: Be constructive (suggest fix, not just flag), explain why, suggest alternatives, acknowledge good work.

Tools Used

Tool	Purpose
Bash	git diff, git log
Read	Read changed files, specs, BRD use cases
Grep	Search for patterns
Glob	Find related files

BRD Coverage Verification (--brd flag)

When --brd flag provided, perform macro-level verification: Code vs BRD use cases.

1. Load BRD use cases:

• Read all plans/brd/use-cases/{feature}/UC-*.md
• Extract requirements and acceptance criteria per UC

2. Load code changes:

• git log --oneline for feature branch/period
• git diff --stat for files changed

3. Map changes to UCs:

For each UC, determine:

• ✅ Covered: Code changes address requirements
• ⚠️ Partial: Some requirements met, some missing
• ❌ Not covered: No code changes for this UC

4. Report:

## BRD Coverage Report: {feature}

| UC | Title | Status | Evidence |
|----|-------|--------|----------|
| UC-PAY-001 | Checkout | ✅ Covered | src/api/checkout.ts, src/components/Checkout.tsx |
| UC-PAY-002 | Refund | ⚠️ Partial | API done, UI missing |
| UC-PAY-004 | Invoice | ❌ Not covered | No related changes |

### Summary
- **Covered**: 3/5 UCs
- **Partial**: 1/5 UCs
- **Not covered**: 1/5 UCs

5. Recommendation:

• All covered → "BRD requirements fully addressed"
• Gaps exist → "X UCs not covered. Run /dev-specs + /dev-coding for missing UCs"

Fix Loop

When issues found, offer to fix via /dev-coding. See references/review-examples.md for detailed fix loop flow.

Issues found? → Offer fix → /dev-coding (fix) → /dev-review (re-run) → Pass → /dev-changelog

After Pass

When review passes: suggest /dev-changelog {feature} to document what was built.

Integration

Skill	Relationship
/dev-coding	Review after implementation, call to fix issues
/dev-scout	Get project conventions
/dev-specs	Check spec compliance
/dev-changelog	Document after pass

References

• references/stack-checks.md - Framework-specific review checklists
• references/review-examples.md - Examples, fix loop, best practices