Ordinary-claude-skills Reviewing Code
Systematically evaluate code changes for security, correctness, performance, and spec alignment. Use when reviewing PRs, assessing code quality, or verifying implementation against requirements.
git clone https://github.com/Microck/ordinary-claude-skills
T=$(mktemp -d) && git clone --depth=1 https://github.com/Microck/ordinary-claude-skills "$T" && mkdir -p ~/.claude/skills && cp -r "$T/skills_categorized/code-quality/reviewing-code" ~/.claude/skills/microck-ordinary-claude-skills-reviewing-code-6f4ef5 && rm -rf "$T"
skills_categorized/code-quality/reviewing-code/SKILL.mdReviewing Code
Evaluate code changes across security, correctness, spec alignment, performance, and maintainability. Apply sequential or parallel review based on scope.
Quick Start
Sequential (small PRs, <5 files):
- Gather context from feature specs and acceptance criteria
- Review sequentially through focus areas
- Report findings by priority
- Recommend approval/revision/rework
Parallel (large PRs, >5 files):
- Identify independent review aspects (security, API, UI, data)
- Spawn specialist agents for each dimension
- Consolidate findings
- Report aggregate assessment
Context Gathering
Read documentation:
— Technical design and requirementsdocs/feature-spec/F-##-*.md
— Acceptance criteriadocs/user-stories/US-###-*.md
— Expected API signaturesdocs/api-contracts.yaml
— Event tracking requirements (if applicable)docs/data-plan.md
— UI/UX requirements (if applicable)docs/design-spec.md
— Architecture patterns (if available)docs/system-design.md
— Original implementation plan (if available)docs/plans/<slug>/plan.md
Determine scope:
- Files changed and features affected (F-## IDs)
- Stories implemented (US-### IDs)
- API, database, or schema changes
Quality Dimensions
Security (/25)
- Input validation and sanitization
- Authentication/authorization checks
- Sensitive data handling
- Injection vulnerabilities (SQL, XSS, etc.)
Correctness (/25)
- Logic matches acceptance criteria
- Edge cases handled properly
- Error handling complete
- Null/undefined checks present
Spec Alignment (/20)
- APIs match
docs/api-contracts.yaml - Data events match
docs/data-plan.md - UI matches
docs/design-spec.md - Implementation follows feature spec
Performance (/15)
- Algorithm efficiency
- Database query optimization
- Resource usage (memory, network)
Maintainability (/15)
- Code clarity and readability
- Consistent with codebase patterns
- Appropriate abstraction levels
- Comments where needed
Total: /100
Finding Priority
🔴 CRITICAL (Must fix before merge)
- Security vulnerabilities
- Broken functionality
- Spec violations (API contract breaks)
- Data corruption risks
Format:
Location: file.ts:123 Problem: [Description] Impact: [Risk/consequence] Fix: [Specific change needed] Spec reference: [docs/api-contracts.yaml line X]
🟡 IMPORTANT (Should fix)
- Logic bugs in edge cases
- Missing error handling
- Performance issues
- Missing analytics events
- Accessibility violations
🟢 NICE-TO-HAVE (Optional)
- Code style improvements
- Better abstractions
- Enhanced documentation
✅ GOOD PRACTICES
Highlight what was done well for learning
Review Strategies
Single-Agent Review
Best for <5 files, single concern:
- Review sequentially through focus areas
- Concentrate on 1-2 most impacted areas
- Generate unified report
Parallel Multi-Agent Review
Best for >5 files, multiple concerns:
-
Spawn specialized agents:
- Security:
for vulnerability assessmentsenior-engineer - Architecture:
for pattern complianceExplore - API Contracts:
for endpoint validationprogrammer - Frontend:
for UI/UX and accessibilityprogrammer - Documentation:
for comment quality and docsdocumentor
- Security:
-
Each agent reviews specific quality dimension
-
Consolidate findings into single report
Report Structure
# Code Review: [Feature/PR] ## Summary **Quality Score:** [X/100] **Issues:** Critical: [N], Important: [N], Nice-to-have: [N] **Assessment:** [APPROVE / NEEDS REVISION / MAJOR REWORK] ## Spec Compliance - [ ] APIs match `docs/api-contracts.yaml` - [ ] Events match `docs/data-plan.md` - [ ] UI matches `docs/design-spec.md` - [ ] Logic satisfies story AC ## Findings ### Critical Issues [Issues with fix recommendations] ### Important Issues [Issues that should be addressed] ### Nice-to-Have Suggestions [Optional improvements] ### Good Practices [What worked well] ## Recommendations [Next steps: approval, revision needed, etc.]
Fix Implementation
Offer options:
- Fix critical + important issues
- Fix only critical (minimum for safety)
- Provide detailed explanation for learning
- Review only (no changes)
Parallel fixes for large revisions:
- Spawn agents for independent fix areas
- Coordinate on shared dependencies
- Document each fix with location, change, and verification method
Document format:
✅ FIXED: [Issue name] File: [path:line] Change: [what changed] Verification: [how to test]
Documentation Updates
Check if specs need updates:
- Feature spec "Decisions" or "Deviations" if implementation differs
- Design spec if UI changed
- API contracts if endpoints modified (requires approval)
- Data plan if events changed
Always flag for user approval before modifying specs.
Key Points
- Read all context documents before starting
- Focus on most impacted areas first
- Be thorough with security-sensitive code, API changes, and critical user flows
- Use scoring framework for comprehensive reviews
- Parallel review scales to large PRs
- Flag spec deviations for user decision