Freddy-CHEN-ux Code Review
Review code for quality and compliance. Report findings.
Announce: "I'm using code-review to review these changes before commit."
This Skill vs @code-reviewer Agent
| Situation | Use |
|---|---|
| Small change (< 100 lines) | This skill |
| Single domain (DB only or frontend only) | This skill |
| Quick sanity check before commit | This skill |
| Large change (> 300 lines) | Dispatch @code-reviewer |
| Cross-domain (DB + frontend) | Dispatch @code-reviewer |
| Security audit needed | Dispatch @code-reviewer |
| Need fresh eyes (you wrote the code) | Dispatch @code-reviewer |
To dispatch: Load subagent-workflow skill first.
Iron Law
REPORT FINDINGS - DO NOT FIX
Your job is to identify issues. The main agent decides what to fix.
Review Checklist
1. Architecture Compliance
Database-First (from architecture rule):
- Business logic is in PostgreSQL, not frontend?
- Frontend only calls
supabase.rpc(), no direct queries? - No game logic, scoring, or calculations in frontend?
Source-Based Workflow:
- Database changes are in
supabase/db/, not migrations? - Both source files AND migrations are included?
Security:
- RLS policies validate
auth.uid()? - SECURITY DEFINER functions check permissions?
- No sensitive data exposed in frontend state?
2. Spec Alignment
Run openspec list --specs and check:
- Does implementation match spec requirements?
- Are all scenarios covered?
- Any spec conflicts?
If change has associated proposal:
- Does implementation match
tasks.md? - All tasks completed?
- No unrequested changes?
3. Code Quality
TypeScript:
- Types are correct (not
any)? - Error handling present?
- No console.log left in?
SQL:
- Uppercase keywords, lowercase identifiers?
- Functions have proper error handling?
- Tests cover edge cases?
Tests:
- New code has tests?
- Tests verify behavior, not implementation?
- All tests passing?
4. Scope Compliance
Compare what was requested vs what was changed:
- Changes are within scope?
- No "while I was here" additions?
- No unrequested refactoring?
Verification Commands
Run these and report results:
bun run type-check # TypeScript
bun run lint # ESLint
bun run test:db # Database tests
bun run test:e2e # End-to-end tests
bun run test:security # Security tests
bun run test:unit # Unit tests
Output Format
## Code Review: [scope/description]
### Verdict: PASS | NEEDS CHANGES | BLOCKED
### Critical Issues (blocks merge)
- [ ] [Issue]: [Description] - [file:line]
### Major Issues (should fix)
- [ ] [Issue]: [Description] - [file:line]
### Minor Issues (nice to have)
- [ ] [Issue]: [Description] - [file:line]
### Observations
- [Non-blocking observations]
### Verification Results
- Type check: PASS/FAIL
- Lint: PASS/FAIL
- Unit tests: PASS/FAIL
- DB tests: PASS/FAIL
### Spec Compliance
- [Spec checked]: Aligns / Conflicts
- Scope: Within bounds / Out of scope items: [list]
Severity Definitions
| Severity | Definition | Action |
|---|---|---|
| Critical | Breaks functionality, security issue, data loss risk | Must fix before merge |
| Major | Significant issue but not breaking | Should fix, discuss if controversial |
| Minor | Style, optimization, nice-to-have | Optional, note for future |
What Happens Next
After review, the main agent will:
- PASS → Proceed to commit/merge
- NEEDS CHANGES → Recall implementing agent with session_id to fix issues
- BLOCKED → Escalate for architectural discussion
Red Flags - Note These
If you observe:
- Business logic in frontend
- Direct database queries (not RPC)
- Missing tests for new code
- Scope creep beyond task list
- Security patterns violated
Report them. Don't fix them.