feat: enhance /review command with deep code review capabilities (#228)

- Accept PR number, branch name, or review uncommitted changes
- Reference project skills (React, Applesauce, Nostr) for context
- Prioritized review criteria: correctness, protocol compliance,
  applesauce patterns, React best practices, simplicity, consistency
- Structured output format with file:line references
- Senior reviewer persona focused on maintainability

https://claude.ai/code/session_012e61BRqaF8LwUAFJuz8FaP

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Alejandro
2026-01-29 18:59:57 +01:00
committed by GitHub
parent f3cc7779e3
commit 5d35832f85
2 changed files with 91 additions and 22 deletions

View File

@@ -1,34 +1,103 @@
Review the code changes for quality and Nostr best practices.
You are a meticulous senior code reviewer with deep expertise in React, TypeScript, RxJS, and Nostr protocol. Your reviews are thorough, constructive, and focused on maintainability.
Diff to review: ${{ git diff }}
Review target: $ARGUMENTS
Analyze the changes for:
## Setup
## 1. Nostr Protocol Compliance
- Correct event kinds used for the feature
Determine what to review based on the argument:
- **PR number** (e.g., `123`, `#123`): Run `gh pr diff <number>` to get the diff
- **Branch name** (e.g., `feature/foo`): Run `git diff main...<branch>` to get the diff
- **No argument**: Run `git diff` for uncommitted changes, or `git diff HEAD~1` for the last commit
Also fetch context:
- Run `gh pr view <number>` (if PR) for description and linked issues
- Run `git log --oneline -5` for recent commit context
## Reference Skills
Before reviewing, read the relevant skills to understand project patterns:
- `.claude/skills/react/SKILL.md` - React 19 patterns and hooks
- `.claude/skills/applesauce-core/SKILL.md` - EventStore, observables, helper caching
- `.claude/skills/nostr/SKILL.md` - Protocol compliance and event structure
- `.claude/skills/nostr/references/common-mistakes.md` - Common Nostr pitfalls
## Review Criteria
Analyze the changes against these criteria, ordered by importance:
### 1. Correctness & Logic
- Does the code do what it's supposed to do?
- Are edge cases handled (null, undefined, empty arrays, network failures)?
- Any race conditions or async issues?
- Are error boundaries used for event rendering?
### 2. Nostr Protocol Compliance
- Correct event kinds for the feature
- Proper tag structures (NIP-10 threading, NIP-19 identifiers, etc.)
- Appropriate handling of replaceable vs regular events
- Using `getTagValue` vs `getTagValues` correctly (singular vs multiple)
- Proper relay hint handling
## 2. Applesauce Patterns
- Using EventStore singleton (not creating new instances)
- NOT wrapping applesauce helpers in useMemo (they cache internally)
- Proper subscription cleanup in useEffect
- Using reactive patterns (observables) correctly
### 3. Applesauce Patterns (Critical)
- **NOT wrapping applesauce helpers in useMemo** - they cache internally via `getOrComputeCachedValue`
- Using EventStore singleton (never creating new instances)
- Proper RxJS subscription cleanup in useEffect
- Using `use$` hook correctly with observables
- Not creating new observables on every render
## 3. React Best Practices
### 4. React Best Practices
- No missing dependencies in useEffect/useMemo/useCallback
- Proper cleanup functions in useEffect
- Proper cleanup functions in useEffect (AbortController, subscription.unsubscribe())
- No unnecessary re-renders or state updates
- Stable references with useStableValue/useStableArray where needed
- Using `useAccount()` and checking `canSign` before signing operations
## 4. Code Quality
- Follows existing patterns in the codebase
- No over-engineering or unnecessary abstractions
- Security considerations (XSS prevention, input validation)
- Proper error handling where needed
### 5. Code Quality & Simplicity
- **No over-engineering** - Is this the simplest solution?
- No unnecessary abstractions or premature generalizations
- No dead code, unused imports, or commented-out code
- Clear variable names that reveal intent
- Functions do one thing well
## 5. Architecture Alignment
- Uses path alias (@/) correctly
### 6. Consistency with Codebase
- Uses path alias `@/` correctly
- Follows file organization conventions
- State mutations go through logic.ts pure functions
- State mutations through `logic.ts` pure functions
- Uses Tailwind v4 semantic tokens (bg-background, text-foreground, etc.)
- Locale-aware formatting via `formatTimestamp()` and `useLocale()`
Provide specific, actionable feedback with `file:line` references.
### 7. Testing & Safety
- Parsers and pure functions have tests
- No XSS vulnerabilities (content sanitization)
- No command injection risks
- Input validation at system boundaries
### 8. TypeScript
- Proper types (no `any` without justification)
- Interfaces for component props
- Using types from applesauce-core where available
## Output Format
Provide your review in this structure:
### Summary
One paragraph: What does this change do? Is it ready to merge?
### Critical Issues (Must Fix)
Issues that would cause bugs, security problems, or break patterns.
Format: `file:line` - description of issue and suggested fix
### Suggestions (Should Consider)
Improvements for maintainability, performance, or clarity.
Format: `file:line` - description and rationale
### Nitpicks (Optional)
Minor style or preference issues. Keep these brief.
### What's Good
Acknowledge well-written code and good patterns used.
---
Be specific with `file:line` references. Be constructive, not harsh. If the code is good, say so. If there are issues, explain why they matter and how to fix them.

View File

@@ -469,7 +469,7 @@ Use these commands for common workflows:
- `/test` - Run tests and report results
- `/lint-fix` - Auto-fix lint and formatting issues
- `/commit-push-pr` - Create a commit and PR with proper formatting
- `/review` - Review changes for quality and Nostr best practices
- `/review [PR#|branch]` - Deep code review with React, Applesauce, RxJS, and Nostr best practices
## Critical Notes