| name | review-methodology |
| description | This skill should be used when performing a code review to apply the standard 6-step review process. |
| user-invocable | false |
| allowed-tools | Read, Grep, Glob, Bash |
Review Methodology
The canonical review process for all Devflow review agents. Ensures consistent, fair, and actionable code reviews.
Iron Law
NEVER BLOCK FOR PRE-EXISTING ISSUES
Only issues in YOUR CHANGES can block a PR. Pre-existing issues are informational only.
If you didn't add it, you don't own it. Fair reviews focus on the diff, not the codebase.
Core Philosophy
- Focus on changed lines first - Developer introduced these
- Context matters - Issues near changes should be fixed together
- Be fair - Don't block PRs for legacy code
- Be specific - Exact file:line with examples
- Be actionable - Clear fixes, not vague complaints
6-Step Review Process
Step 1: Identify Changed Lines
Get the diff to understand what changed. Identify base branch and extract changed files/lines.
Step 2: Categorize Issues
| Category | Scope | Priority | Action |
|---|
| 1. Issues in Your Changes | Lines ADDED/MODIFIED in this branch | BLOCKING | Must fix before merge |
| 2. Issues in Code You Touched | Same file/function, but not your line | HIGH | Should fix while here |
| 3. Pre-existing Issues | Lines you didn't touch at all | INFORMATIONAL | Fix in separate PR |
Note: All categories and severities — including suggestions — are reported for resolution. Categories affect PR merge-blocking, not whether issues get resolved. The resolve workflow evaluates everything.
Step 3: Analyze with Domain Expertise
Apply your specialized lens (security, performance, tests, etc.) to each category:
- Category 1 - Maximum scrutiny, any issue blocks PR
- Category 2 - Should fix together with your changes
- Category 3 - Note but don't block, suggest separate issues
Step 4: Prioritize by Severity
| Severity | Description | Examples |
|---|
| CRITICAL | Immediate risk, must fix | Security vulnerabilities, data loss risks, breaking API changes |
| HIGH | Significant risk, should fix | Performance degradation, missing error handling |
| MEDIUM | Moderate risk, consider fixing | Style inconsistencies, missing documentation |
| LOW | Minor improvements | Naming suggestions, optional optimizations |
Step 5: Create Actionable Comments
For each issue, provide:
- Location - Exact file:line reference
- Problem - Clear description
- Impact - Why this matters
- Fix - Specific code solution
- Category - Which of the 3 categories
Step 6: Generate Report
Create report with all three issue sections, summary counts, and merge recommendation.
Key Principles Summary
| Principle | Description |
|---|
| Diff-Aware | Focus on actual changes, not pre-existing issues |
| Fair | Don't block PRs for legacy problems |
| Specific | Exact file:line, not vague complaints |
| Actionable | Show the fix, not just the problem |
| Categorized | Clear distinction between blocking and informational |
| Prioritized | Critical > High > Medium > Low |
| Documented | Reports saved for future reference |
Extended References
For detailed implementation:
| Reference | Content |
|---|
references/report-template.md | Full report template with all sections |
references/patterns.md | Diff commands (lines 29–113) and PR comment API integration (lines 117–181) |
references/violations.md | Review process anti-patterns and violations |
Integration
This methodology is used by the Reviewer agent with different focus areas:
| Focus | Pattern Skill |
|---|
security | devflow:security |
performance | devflow:performance |
architecture | devflow:architecture |
testing | devflow:testing |
consistency | devflow:consistency |
complexity | devflow:complexity |
regression | devflow:regression |
dependencies | devflow:dependencies |
documentation | devflow:documentation |
typescript | devflow:typescript |
database | devflow:database |
The Reviewer agent loads all pattern skills and applies the relevant one based on the focus area specified in its invocation prompt.