modify template
This commit is contained in:
+103
@@ -0,0 +1,103 @@
|
||||
---
|
||||
name: requesting-code-review
|
||||
description: Use when completing tasks, implementing major features, or before merging to verify work meets requirements
|
||||
---
|
||||
|
||||
# Requesting Code Review
|
||||
|
||||
Dispatch a 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.
|
||||
|
||||
## 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:**
|
||||
```bash
|
||||
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 `general-purpose` type, fill template at `code-reviewer.md`
|
||||
|
||||
**Placeholders:**
|
||||
- `{DESCRIPTION}` - Brief summary of what you built
|
||||
- `{PLAN_OR_REQUIREMENTS}` - What it should do
|
||||
- `{BASE_SHA}` - Starting commit
|
||||
- `{HEAD_SHA}` - Ending commit
|
||||
|
||||
**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]
|
||||
DESCRIPTION: Added verifyIndex() and repairIndex() with 4 issue types
|
||||
PLAN_OR_REQUIREMENTS: Task 2 from docs/superpowers/plans/deployment-plan.md
|
||||
BASE_SHA: a7981ec
|
||||
HEAD_SHA: 3df7661
|
||||
|
||||
[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 task or at natural checkpoints
|
||||
- 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
|
||||
Vendored
+3
@@ -0,0 +1,3 @@
|
||||
interface:
|
||||
display_name: "Requesting Code Review"
|
||||
short_description: "Request focused code review before merging"
|
||||
Vendored
+168
@@ -0,0 +1,168 @@
|
||||
# Code Reviewer Prompt Template
|
||||
|
||||
Use this template when dispatching a code reviewer subagent.
|
||||
|
||||
**Purpose:** Review completed work against requirements and code quality standards before it cascades into more work.
|
||||
|
||||
```
|
||||
Task tool (general-purpose):
|
||||
description: "Review code changes"
|
||||
prompt: |
|
||||
You are a Senior Code Reviewer with expertise in software architecture,
|
||||
design patterns, and best practices. Your job is to review completed work
|
||||
against its plan or requirements and identify issues before they cascade.
|
||||
|
||||
## What Was Implemented
|
||||
|
||||
{DESCRIPTION}
|
||||
|
||||
## Requirements / Plan
|
||||
|
||||
{PLAN_OR_REQUIREMENTS}
|
||||
|
||||
## Git Range to Review
|
||||
|
||||
**Base:** {BASE_SHA}
|
||||
**Head:** {HEAD_SHA}
|
||||
|
||||
```bash
|
||||
git diff --stat {BASE_SHA}..{HEAD_SHA}
|
||||
git diff {BASE_SHA}..{HEAD_SHA}
|
||||
```
|
||||
|
||||
## What to Check
|
||||
|
||||
**Plan alignment:**
|
||||
- Does the implementation match the plan / requirements?
|
||||
- Are deviations justified improvements, or problematic departures?
|
||||
- Is all planned functionality present?
|
||||
|
||||
**Code quality:**
|
||||
- Clean separation of concerns?
|
||||
- Proper error handling?
|
||||
- Type safety where applicable?
|
||||
- DRY without premature abstraction?
|
||||
- Edge cases handled?
|
||||
|
||||
**Architecture:**
|
||||
- Sound design decisions?
|
||||
- Reasonable scalability and performance?
|
||||
- Security concerns?
|
||||
- Integrates cleanly with surrounding code?
|
||||
|
||||
**Testing:**
|
||||
- Tests verify real behavior, not mocks?
|
||||
- Edge cases covered?
|
||||
- Integration tests where they matter?
|
||||
- All tests passing?
|
||||
|
||||
**Production readiness:**
|
||||
- Migration strategy if schema changed?
|
||||
- Backward compatibility considered?
|
||||
- Documentation complete?
|
||||
- No obvious bugs?
|
||||
|
||||
## Calibration
|
||||
|
||||
Categorize issues by actual severity. Not everything is Critical.
|
||||
Acknowledge what was done well before listing issues — accurate praise
|
||||
helps the implementer trust the rest of the feedback.
|
||||
|
||||
If you find significant deviations from the plan, flag them specifically
|
||||
so the implementer can confirm whether the deviation was intentional.
|
||||
If you find issues with the plan itself rather than the implementation,
|
||||
say so.
|
||||
|
||||
## 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 polish]
|
||||
|
||||
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:** [1-2 sentence technical assessment]
|
||||
|
||||
## Critical Rules
|
||||
|
||||
**DO:**
|
||||
- Categorize by actual severity
|
||||
- Be specific (file:line, not vague)
|
||||
- Explain WHY each issue matters
|
||||
- Acknowledge strengths
|
||||
- Give a clear verdict
|
||||
|
||||
**DON'T:**
|
||||
- Say "looks good" without checking
|
||||
- Mark nitpicks as Critical
|
||||
- Give feedback on code you didn't actually read
|
||||
- Be vague ("improve error handling")
|
||||
- Avoid giving a clear verdict
|
||||
```
|
||||
|
||||
**Placeholders:**
|
||||
- `{DESCRIPTION}` — brief summary of what was built
|
||||
- `{PLAN_OR_REQUIREMENTS}` — what it should do (plan file path, task text, or requirements)
|
||||
- `{BASE_SHA}` — starting commit
|
||||
- `{HEAD_SHA}` — ending commit
|
||||
|
||||
**Reviewer returns:** Strengths, Issues (Critical / Important / Minor), Recommendations, Assessment
|
||||
|
||||
## 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.
|
||||
```
|
||||
Reference in New Issue
Block a user