102 lines
2.7 KiB
Markdown
102 lines
2.7 KiB
Markdown
---
|
|
description: Perform a comprehensive code review of a feature
|
|
argument-hint: <feature-slug>
|
|
allowed-tools: Bash, Read, Glob, Grep, Write
|
|
---
|
|
|
|
Review feature: $ARGUMENTS
|
|
|
|
## Instructions
|
|
|
|
### 1. Load Feature Context
|
|
|
|
```bash
|
|
sdlc feature show $ARGUMENTS --json
|
|
```
|
|
|
|
Read the spec and design to understand what was intended:
|
|
- `.sdlc/features/$ARGUMENTS/spec.md`
|
|
- `.sdlc/features/$ARGUMENTS/design.md`
|
|
|
|
### 2. Identify Changed Files
|
|
|
|
Determine what files were created or modified for this feature. Check git history, task descriptions, or search for recent changes in relevant directories.
|
|
|
|
### 3. Review Each Dimension
|
|
|
|
| Dimension | Key Question |
|
|
|-----------|--------------|
|
|
| **Correctness** | Does the code do what the spec requires? |
|
|
| **Test Coverage** | Is every acceptance criterion tested? |
|
|
| **Error Handling** | Are failures handled, not swallowed? |
|
|
| **Security** | Input validation, auth checks, data exposure? |
|
|
| **Performance** | N+1 queries, unbounded loops, missing timeouts? |
|
|
| **Code Style** | Follows existing patterns and conventions? |
|
|
| **Documentation** | Public APIs documented, complex logic commented? |
|
|
|
|
### 4. Categorize Findings
|
|
|
|
| Severity | Meaning |
|
|
|----------|---------|
|
|
| **BLOCKER** | Cannot ship -- must fix before merge |
|
|
| **WARNING** | Quality concern -- should fix |
|
|
| **SUGGESTION** | Improvement -- nice to have |
|
|
|
|
### 5. Write Review Report
|
|
|
|
Write to `.sdlc/features/$ARGUMENTS/review.md`:
|
|
|
|
```markdown
|
|
# Code Review: [Feature Title]
|
|
|
|
## Summary
|
|
[Overall assessment: PASS / NEEDS_FIX]
|
|
|
|
## Findings
|
|
|
|
### Blockers
|
|
- [ ] [FILE:LINE] [Description] -- [Why it matters]
|
|
|
|
### Warnings
|
|
- [ ] [FILE:LINE] [Description] -- [Suggested fix]
|
|
|
|
### Suggestions
|
|
- [ ] [FILE:LINE] [Description]
|
|
|
|
## Spec Alignment
|
|
[Does the implementation match the spec? Any gaps?]
|
|
|
|
## Test Coverage Assessment
|
|
[Which acceptance criteria have tests? Which are missing?]
|
|
```
|
|
|
|
### 6. Register and Evaluate the Artifact
|
|
|
|
Create the artifact:
|
|
|
|
```bash
|
|
sdlc artifact create $ARGUMENTS review
|
|
```
|
|
|
|
Then evaluate the review results and set the appropriate status:
|
|
|
|
- If the review has **NO blockers**: mark as passed
|
|
```bash
|
|
sdlc artifact pass $ARGUMENTS review
|
|
```
|
|
- If the review has **blockers**: mark as needs-fix
|
|
```bash
|
|
sdlc artifact needs-fix $ARGUMENTS review
|
|
```
|
|
|
|
This status drives the SDLC classifier to either advance to audit or trigger fix-review-issues.
|
|
|
|
## Critical Rules
|
|
|
|
- ALWAYS read spec and design before reviewing code
|
|
- NEVER skip the security review dimension
|
|
- ALWAYS check test coverage against acceptance criteria
|
|
- ALWAYS provide actionable findings with file locations
|
|
- NEVER mark a review as passed if it has unresolved blockers
|
|
- ALWAYS set the artifact status (pass or needs-fix) after writing the review
|