diff --git a/.claude/commands/review.md b/.claude/commands/review.md index abc743c..da268ae 100644 --- a/.claude/commands/review.md +++ b/.claude/commands/review.md @@ -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 ` to get the diff +- **Branch name** (e.g., `feature/foo`): Run `git diff main...` 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 ` (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. diff --git a/CLAUDE.md b/CLAUDE.md index 9e7c7fa..f5c49f8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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