Code Review Standards
Systematic approach to code review. Use this skill when:
- Performing code reviews
- Defining review criteria for a project
- Understanding severity levels for findings
- Creating code review documents
Review Focus Areas
Use this checklist when reviewing implementation code:
| Category |
What to Review |
| Architecture Alignment |
Does implementation match Architect's design? Follows system-architecture.md patterns? |
| SOLID Principles |
SRP, OCP, LSP, ISP, DIP violations (load engineering-standards for detection patterns) |
| DRY/YAGNI/KISS |
Duplication, speculative generalization, over-complexity |
| TDD Compliance |
TDD Compliance table present in implementation doc? All rows show test-first? |
| Code Smells |
Long Method, Large Class, Feature Envy, etc. (see engineering-standards) |
| Documentation & Comments |
Appropriate inline comments explaining "why" (not "what"), function docstrings, module-level docs, complex logic explained |
| Naming & Clarity |
Self-documenting names, appropriate abstractions, readable code |
| Error Handling |
Defensive coding, graceful failures, appropriate exceptions |
| Security Quick Scan |
Obvious vulnerabilities (injection, exposed secrets, hardcoded creds) |
| Performance |
Obvious inefficiencies, N+1 patterns, memory leaks |
| Observability |
Appropriate logging, telemetry for debugging |
Severity Levels
| Severity |
Definition |
Action |
| CRITICAL |
Security vulnerability, data loss risk, architectural violation |
REJECT - must fix |
| HIGH |
Anti-pattern, significant maintainability issue, missing tests |
REJECT - must fix |
| MEDIUM |
Code smell, minor design issue, unclear code |
Fix recommended, may approve with comments |
| LOW |
Style preference, minor optimization opportunity |
Note for future, approve |
| INFO |
Observation, suggestion for improvement |
FYI only |
When to Reject
- Any CRITICAL finding → REJECT
- Any HIGH finding → REJECT
- 3+ MEDIUM findings in same file → Consider REJECT
- Pattern of MEDIUM findings across files → Consider REJECT
Finding Format
When documenting findings, use this format:
**[SEVERITY] [Category]**: [Brief title]
- **Location**: `path/to/file.py:L42-L55`
- **Issue**: [What's wrong and why it matters]
- **Recommendation**: [Specific fix suggestion]
Example:
**[HIGH] Documentation**: Missing docstrings on public API
- **Location**: `src/api/handlers.py:L15-L45`
- **Issue**: Public functions `create_user()` and `delete_user()` lack docstrings. Future maintainers won't understand expected inputs/outputs.
- **Recommendation**: Add Google-style docstrings with Args, Returns, and Raises sections.
Code Review Document Template
Create in agent-output/code-review/ matching plan name:
# Code Review: [Plan Name]
**Plan Reference**: `agent-output/planning/[plan-name].md`
**Implementation Reference**: `agent-output/implementation/[plan-name]-implementation.md`
**Date**: [date]
**Reviewer**: Code Reviewer
## Changelog
| Date | Agent Handoff | Request | Summary |
|------|---------------|---------|---------|
| YYYY-MM-DD | [Who handed off] | [What was requested] | [Brief summary] |
## Architecture Alignment
**System Architecture Reference**: `agent-output/architecture/system-architecture.md`
**Alignment Status**: ALIGNED / MINOR_DEVIATIONS / MAJOR_DEVIATIONS
[Assessment of how implementation aligns with architectural decisions]
## TDD Compliance Check
**TDD Table Present**: Yes / No
**All Rows Complete**: Yes / No
**Concerns**: [Any issues with TDD compliance]
## Findings
### Critical
[List of critical findings, or "None"]
### High
[List of high findings, or "None"]
### Medium
[List of medium findings, or "None"]
### Low/Info
[List of low/info findings, or "None"]
## Positive Observations
[Acknowledge good patterns, well-written code, or improvements]
## Verdict
**Status**: APPROVED / APPROVED_WITH_COMMENTS / REJECTED
**Rationale**: [Brief explanation]
## Required Actions
[If rejected: specific list of fixes required]
[If approved with comments: optional improvements]
## Next Steps
[Handoff to Implementer for fixes / Handoff to QA for testing]
Review Best Practices
- Be constructive: Explain WHY something is an issue, not just THAT it's an issue
- Be specific: File paths, line numbers, code snippets
- Provide solutions: Concrete fix suggestions, not just criticism
- Acknowledge good work: Note positive patterns when you see them
- Prioritize: Focus on impactful issues, not nitpicks
- Context matters: Consider the plan's constraints and timeline
Converted and distributed by TomeVault — claim your Tome and manage your conversions.
1---2name: code-review-standards-23description: Code review checklist, severity definitions, and document templates. Load when performing code reviews or defining review criteria. Use when this capability is needed.4---56# Code Review Standards78Systematic approach to code review. Use this skill when:9- Performing code reviews10- Defining review criteria for a project11- Understanding severity levels for findings12- Creating code review documents1314---1516## Review Focus Areas1718Use this checklist when reviewing implementation code:1920| Category | What to Review |21|----------|----------------|22| **Architecture Alignment** | Does implementation match Architect's design? Follows system-architecture.md patterns? |23| **SOLID Principles** | SRP, OCP, LSP, ISP, DIP violations (load `engineering-standards` for detection patterns) |24| **DRY/YAGNI/KISS** | Duplication, speculative generalization, over-complexity |25| **TDD Compliance** | TDD Compliance table present in implementation doc? All rows show test-first? |26| **Code Smells** | Long Method, Large Class, Feature Envy, etc. (see `engineering-standards`) |27| **Documentation & Comments** | Appropriate inline comments explaining "why" (not "what"), function docstrings, module-level docs, complex logic explained |28| **Naming & Clarity** | Self-documenting names, appropriate abstractions, readable code |29| **Error Handling** | Defensive coding, graceful failures, appropriate exceptions |30| **Security Quick Scan** | Obvious vulnerabilities (injection, exposed secrets, hardcoded creds) |31| **Performance** | Obvious inefficiencies, N+1 patterns, memory leaks |32| **Observability** | Appropriate logging, telemetry for debugging |3334---3536## Severity Levels3738| Severity | Definition | Action |39|----------|------------|--------|40| **CRITICAL** | Security vulnerability, data loss risk, architectural violation | REJECT - must fix |41| **HIGH** | Anti-pattern, significant maintainability issue, missing tests | REJECT - must fix |42| **MEDIUM** | Code smell, minor design issue, unclear code | Fix recommended, may approve with comments |43| **LOW** | Style preference, minor optimization opportunity | Note for future, approve |44| **INFO** | Observation, suggestion for improvement | FYI only |4546### When to Reject4748- Any CRITICAL finding → REJECT49- Any HIGH finding → REJECT50- 3+ MEDIUM findings in same file → Consider REJECT51- Pattern of MEDIUM findings across files → Consider REJECT5253---5455## Finding Format5657When documenting findings, use this format:5859```markdown60**[SEVERITY] [Category]**: [Brief title]61- **Location**: `path/to/file.py:L42-L55`62- **Issue**: [What's wrong and why it matters]63- **Recommendation**: [Specific fix suggestion]64```6566**Example:**67```markdown68**[HIGH] Documentation**: Missing docstrings on public API69- **Location**: `src/api/handlers.py:L15-L45`70- **Issue**: Public functions `create_user()` and `delete_user()` lack docstrings. Future maintainers won't understand expected inputs/outputs.71- **Recommendation**: Add Google-style docstrings with Args, Returns, and Raises sections.72```7374---7576## Code Review Document Template7778Create in `agent-output/code-review/` matching plan name:7980```markdown81# Code Review: [Plan Name]8283**Plan Reference**: `agent-output/planning/[plan-name].md`84**Implementation Reference**: `agent-output/implementation/[plan-name]-implementation.md`85**Date**: [date]86**Reviewer**: Code Reviewer8788## Changelog8990| Date | Agent Handoff | Request | Summary |91|------|---------------|---------|---------|92| YYYY-MM-DD | [Who handed off] | [What was requested] | [Brief summary] |9394## Architecture Alignment9596**System Architecture Reference**: `agent-output/architecture/system-architecture.md`97**Alignment Status**: ALIGNED / MINOR_DEVIATIONS / MAJOR_DEVIATIONS9899[Assessment of how implementation aligns with architectural decisions]100101## TDD Compliance Check102103**TDD Table Present**: Yes / No104**All Rows Complete**: Yes / No105**Concerns**: [Any issues with TDD compliance]106107## Findings108109### Critical110[List of critical findings, or "None"]111112### High113[List of high findings, or "None"]114115### Medium116[List of medium findings, or "None"]117118### Low/Info119[List of low/info findings, or "None"]120121## Positive Observations122123[Acknowledge good patterns, well-written code, or improvements]124125## Verdict126127**Status**: APPROVED / APPROVED_WITH_COMMENTS / REJECTED128**Rationale**: [Brief explanation]129130## Required Actions131132[If rejected: specific list of fixes required]133[If approved with comments: optional improvements]134135## Next Steps136137[Handoff to Implementer for fixes / Handoff to QA for testing]138```139140---141142## Review Best Practices1431441. **Be constructive**: Explain WHY something is an issue, not just THAT it's an issue1452. **Be specific**: File paths, line numbers, code snippets1463. **Provide solutions**: Concrete fix suggestions, not just criticism1474. **Acknowledge good work**: Note positive patterns when you see them1485. **Prioritize**: Focus on impactful issues, not nitpicks1496. **Context matters**: Consider the plan's constraints and timeline150151---152> Converted and distributed by [TomeVault](https://tomevault.io/claim/groupzer0) — claim your Tome and manage your conversions.153<!-- tomevault:4.0:skill_md:2026-04-11 -->
tomevault-io