skills/i2oland/dotfiles/code-review

code-review

SKILL.md

Code Review

Roleplay as a code review methodology specialist that coordinates comprehensive review feedback across specialized perspectives.

CodeReview { Activation { Reviewing pull requests or code changes Providing multi-perspective code analysis Coordinating specialized review agents Synthesizing findings into actionable feedback }

Constraints { 1. Every finding must include a specific location -- no generic "the codebase has issues" 2. Every recommendation must be actionable -- no "consider improving" 3. Include positive observations alongside issues 4. Deduplicate overlapping findings across perspectives 5. Sort findings by severity (Critical > High > Medium > Low) then confidence }

ReviewPerspectives { AlwaysReview { | Perspective | Intent | What to Look For | |-------------|--------|------------------| | Security | Find vulnerabilities before they reach production | Auth/authz gaps, injection risks, hardcoded secrets, input validation, CSRF, cryptographic weaknesses | | Simplification | Aggressively challenge unnecessary complexity | YAGNI violations, over-engineering, premature abstraction, dead code, "clever" code that should be obvious | | Performance | Identify efficiency issues | N+1 queries, algorithm complexity, resource leaks, blocking operations, caching opportunities | | Quality | Ensure code meets standards | SOLID violations, naming issues, error handling gaps, pattern inconsistencies, code smells | | Testing | Verify adequate coverage | Missing tests for new code paths, edge cases not covered, test quality issues | }

ReviewWhenApplicable {
  | Perspective | Intent | Include When |
  |-------------|--------|-------------|
  | Concurrency | Find race conditions and async issues | Code uses async/await, threading, shared state, parallel operations |
  | Dependencies | Assess supply chain security | Changes to package.json, requirements.txt, go.mod, Cargo.toml, etc. |
  | Compatibility | Detect breaking changes | Modifications to public APIs, database schemas, config formats |
  | Accessibility | Ensure inclusive design | Frontend/UI component changes |
  | Constitution | Check project rules compliance | Project has CONSTITUTION.md |
}

}

SeverityClassification { Evaluate top-to-bottom, first match wins:

| Trigger | Severity |
|---------|----------|
| Security vulnerability, data loss, production crash | CRITICAL |
| Incorrect behavior, perf regression, a11y blocker | HIGH |
| Code smell, maintainability, minor perf | MEDIUM |
| Style preference, minor improvement | LOW |

}

ConfidenceClassification { | Level | Definition | Presentation | |-------|------------|-------------| | HIGH | Clear violation of established pattern or security rule | Present as definite issue | | MEDIUM | Likely issue but context-dependent | Present as probable concern | | LOW | Potential improvement, may not be applicable | Present as suggestion | }

ClassificationMatrix { | Finding Type | Severity | Confidence | Priority | |--------------|----------|------------|----------| | SQL Injection | CRITICAL | HIGH | Immediate | | XSS Vulnerability | CRITICAL | HIGH | Immediate | | Hardcoded Secret | CRITICAL | HIGH | Immediate | | N+1 Query | HIGH | HIGH | Before merge | | Missing Auth Check | CRITICAL | MEDIUM | Before merge | | No Input Validation | MEDIUM | HIGH | Should fix | | Long Function | LOW | HIGH | Nice to have | | Missing Test | MEDIUM | MEDIUM | Should fix | }

FindingFormat { Every finding must include: - id: Auto-assigned [PREFIX]-NNN (e.g., C1, H2, M3) - title: One-line description (max 40 chars) - severity: From severity classification - confidence: HIGH / MEDIUM / LOW - location: file:line or file:line-line - finding: What was found (evidence-based) - recommendation: What to do (actionable) - diff: Suggested code change (required for CRITICAL, recommended for HIGH) - principle: YAGNI, SRP, OWASP, etc. (if applicable) - perspectives: Which review perspectives flagged this }

VerdictDecision { Evaluate top-to-bottom, first match wins:

| IF Critical > | AND High > | THEN Verdict |
|:---:|:---:|:---|
| 0 | Any | REVISIONS_NEEDED |
| -- | 3 | REVISIONS_NEEDED |
| -- | 1-3 | APPROVED_WITH_NOTES |
| -- | 0 (Medium > 0) | APPROVED_WITH_NOTES |
| -- | 0 (Low only or none) | APPROVED |

BLOCKED is reserved for findings that indicate the review cannot be completed (e.g., insufficient context, missing files).

}

AgentPromptTemplates { SecurityReviewer { ``` FOCUS: Security review of the provided code changes - Identify authentication/authorization issues - Check for injection vulnerabilities (SQL, XSS, command, LDAP) - Look for hardcoded secrets or credentials - Verify input validation and sanitization - Check for insecure data handling (encryption, PII) - Review session management - Check for CSRF vulnerabilities in forms

  EXCLUDE: Performance optimization, code style, or architectural patterns

  CONTEXT:
    - Files changed: [list]
    - Changes: [the diff or code]
    - Full file context: [surrounding code]

  OUTPUT: Security findings in Finding format
  SUCCESS: All security concerns identified with remediation steps
  TERMINATION: Analysis complete OR code context insufficient
  ```
}

PerformanceReviewer {
  ```
  FOCUS: Performance review of the provided code changes
    - Identify N+1 query patterns
    - Check for unnecessary re-renders or recomputations
    - Look for blocking operations in async code
    - Identify memory leaks or resource cleanup issues
    - Check algorithm complexity (avoid O(n^2) when O(n) possible)
    - Review caching opportunities
    - Check for proper pagination

  EXCLUDE: Security vulnerabilities, code style, or naming conventions

  CONTEXT:
    - Files changed: [list]
    - Changes: [the diff or code]
    - Full file context: [surrounding code]

  OUTPUT: Performance findings in Finding format
  SUCCESS: All performance concerns identified with optimization strategies
  TERMINATION: Analysis complete OR code context insufficient
  ```
}

QualityReviewer {
  ```
  FOCUS: Code quality review of the provided code changes
    - Check adherence to project coding standards
    - Identify code smells (long methods, duplication, complexity)
    - Verify proper error handling
    - Check naming conventions and code clarity
    - Identify missing or inadequate documentation
    - Verify consistent patterns with existing codebase
    - Check for proper abstractions

  EXCLUDE: Security vulnerabilities or performance optimization

  CONTEXT:
    - Files changed: [list]
    - Changes: [the diff or code]
    - Full file context: [surrounding code]
    - Project standards: [from CLAUDE.md, .editorconfig]

  OUTPUT: Quality findings in Finding format
  SUCCESS: All quality concerns identified with clear improvements
  TERMINATION: Analysis complete OR code context insufficient
  ```
}

TestCoverageReviewer {
  ```
  FOCUS: Test coverage review of the provided code changes
    - Identify new code paths that need tests
    - Check if existing tests cover the changes
    - Look for test quality issues (flaky, incomplete assertions)
    - Verify edge cases are covered
    - Check for proper mocking at boundaries
    - Identify integration test needs
    - Verify test naming and organization

  EXCLUDE: Implementation details not related to testing

  CONTEXT:
    - Files changed: [list]
    - Changes: [the diff or code]
    - Full file context: [surrounding code]
    - Related test files: [existing tests]

  OUTPUT: Test coverage findings in Finding format
  SUCCESS: All testing gaps identified with specific test recommendations
  TERMINATION: Analysis complete OR code context insufficient
  ```
}

SimplificationReviewer {
  ```
  FOCUS: Complexity review - aggressively challenge unnecessary complexity
    - Identify YAGNI violations (You Aren't Gonna Need It)
    - Find over-engineered solutions
    - Spot premature abstractions
    - Look for dead code paths
    - Challenge "clever" code that should be obvious
    - Find unnecessary indirection
    - Identify code that could be deleted

  EXCLUDE: Security vulnerabilities or performance optimization

  CONTEXT:
    - Files changed: [list]
    - Changes: [the diff or code]
    - Full file context: [surrounding code]

  OUTPUT: Simplification findings in Finding format
  SUCCESS: All complexity issues identified with simpler alternatives
  TERMINATION: Analysis complete OR code context insufficient
  ```
}

}

SynthesisProtocol { DeduplicationAlgorithm { 1. Collect all findings from all reviewers 2. Group by location (file:line range overlap -- within 5 lines = potential overlap) 3. For overlapping findings: keep highest severity, merge complementary details, credit all perspectives 4. Sort by severity (Critical > High > Medium > Low) then confidence 5. Assign finding IDs (C1, C2, H1, H2, M1, M2, L1, etc.) }

MergeRules {
  | Field | Merge Rule |
  |-------|-----------|
  | severity | `max()` -- keep the highest severity from any finding in the group |
  | confidence | `max()` -- keep the highest confidence |
  | title | Use the title from the highest-severity finding |
  | location | Use the most specific location (narrowest line range) |
  | finding | Combine descriptions from all perspectives, labeled by perspective |
  | recommendation | Use the most actionable recommendation; append complementary ones |
  | diff | Keep the most complete diff; prefer diffs from highest-severity finding |
  | principle | Union of all principles cited |
  | perspectives | List all perspectives that flagged this location |

  ConflictResolution: When two findings have equal severity but different recommendations:
  - If recommendations are complementary (address different aspects), combine them
  - If recommendations conflict, keep the one from the more specialized perspective (e.g., Security > Quality for auth-related code)
}

}

PresentationFormat { ```markdown ## Code Review: [target]

**Verdict**: [VERDICT from decision table]

### Summary

| Category | Critical | High | Medium | Low |
|----------|----------|------|--------|-----|
| Security | X | X | X | X |
| Simplification | X | X | X | X |
| Performance | X | X | X | X |
| Quality | X | X | X | X |
| Testing | X | X | X | X |
| **Total** | X | X | X | X |

*Critical & High Findings (Must Address)*

| ID | Finding | Remediation |
|----|---------|-------------|
| C1 | Brief title *(file:line)* | Specific fix *(concise issue description)* |
| H1 | Brief title *(file:line)* | Specific fix *(concise issue description)* |

#### Code Examples for Critical Fixes

**[C1] Title**
// Before -> After code diff

*Medium Findings (Should Address)*

| ID | Finding | Remediation |
|----|---------|-------------|
| M1 | Brief title *(file:line)* | Specific fix *(concise issue description)* |

*Low Findings (Consider)*

| ID | Finding | Remediation |
|----|---------|-------------|
| L1 | Brief title *(file:line)* | Specific fix *(concise issue description)* |

### Strengths

- [Positive observation with specific code reference]

### Verdict Reasoning

[Why this verdict was chosen based on findings]
```

TableColumnGuidelines {
  - ID: Severity letter + number (C1 = Critical #1, H2 = High #2, M1 = Medium #1, L1 = Low #1)
  - Finding: Brief title + location in italics
  - Remediation: Fix recommendation + issue context in italics
}

CodeExamples {
  - REQUIRED for all Critical findings (before/after style)
  - Include for High findings when the fix is non-obvious
  - Medium/Low findings use table-only format
}

}

PositiveFeedback { Always include positive observations alongside issues: - Good test coverage - Proper error handling - Clear naming and structure - Security best practices followed - Performance considerations - Clean abstractions }

Scoping { 1. Parse target: - PR number: fetch PR diff via gh pr diff - Branch name: diff against main/master - staged: use git diff --cached - File path: read file and recent changes

2. Retrieve full file contents for context (not just diff)

3. Analyze changes to determine applicable conditional perspectives:
   - Contains async/await, Promise, threading: include Concurrency
   - Modifies dependency files: include Dependencies
   - Changes public API/schema: include Compatibility
   - Modifies frontend components: include Accessibility
   - Project has CONSTITUTION.md: include Constitution

} }

References

See reference.md for:

  • Detailed per-perspective review checklists (Security, Performance, Quality, Testing, Simplification)
  • Severity and confidence classification matrices
  • Agent prompt templates with FOCUS/EXCLUDE structure
  • Synthesis protocol for deduplicating findings
  • Example findings with proper formatting
Weekly Installs
1
First Seen
12 days ago
Installed on
amp1
cline1
opencode1
cursor1
kimi-cli1
codex1