convention-code-review
Convention Code Review
Review code changes against Sellernote development conventions. This skill is read-only -- it identifies violations but does not modify code.
Convention Loading
Conventions are loaded dynamically based on what files were changed. Before starting, determine which files are in scope, then Read the relevant reference files from references/ within this skill directory.
Loading Strategy
-
Always read (apply to all reviews):
references/COMMON_CONVENTION.md- Naming, git, error handlingreferences/TYPESCRIPT_CONVENTION.md- TS style, imports, types
-
Read when backend files changed (files in
src/modules/,src/common/,*.entity.ts,*.service.ts,*.controller.ts,*.repository.ts,*.dto.ts,*.guard.ts):references/BACKEND_CONVENTION.md- 3-layer architecture, DTO/Entity namingreferences/BACKEND_ARCHITECTURE_CONVENTION.md- Layer responsibilities, forbidden patternsreferences/NESTJS_CONVENTION.md- NestJS-specific rules, DI, decoratorsreferences/API_SPEC_CONVENTION.md- Endpoint design, response formatsreferences/SECURITY_CONVENTION.md- Auth, guards, input validationreferences/TYPEORM_CONVENTION.md- Entity patterns, relations, migrationsreferences/DATABASE_CONVENTION.md- DB modeling, indexingreferences/MYSQL_CONVENTION.md- MySQL-specific rulesreferences/PRISMA_CONVENTION.md- Prisma schema, client patterns, NestJS integration
-
Read when frontend files changed (files in
app/,components/,hooks/,queries/,store/,actions/,*.tsx,*.stories.tsx):references/FRONTEND_CONVENTION.md- Frontend common rulesreferences/FRONTEND_ARCHITECTURE_CONVENTION.md- Component types, dependency directionreferences/NEXTJS_CONVENTION.md- App Router, Server/Client Componentsreferences/STATE_CONVENTION.md- Zustand, TanStack Query, state classificationreferences/STYLING_CONVENTION.md- Tailwind CSS v4, cn(), design tokensreferences/FORM_CONVENTION.md- React Hook Form + Zodreferences/TESTING_CONVENTION.md- Storybook, Vitest, RTL, Playwrightreferences/REACT_CONVENTION.md- React 19 패턴, Hooks 규칙, 성능 최적화, Error Boundary, Context APIreferences/REACT_ROUTER_CONVENTION.md- React Router 7 Framework Mode, route modules, loader/actionreferences/API_CLIENT_CONVENTION.md- API client common rules, token managementreferences/API_CLIENT_AXIOS_CONVENTION.md- Axios implementation, interceptors
Workflow
Step 1: Determine Review Scope
Identify what to review based on user request:
| User Request | Git Command |
|---|---|
| "staged 변경사항 리뷰해줘" | git diff --cached |
| "내 변경사항 리뷰해줘" | git diff (unstaged) + git diff --cached (staged) |
| "이 브랜치 리뷰해줘" / "PR 리뷰" | git diff main..HEAD |
| "이 파일 리뷰해줘" | Read the specified file(s) directly |
| "최근 커밋 리뷰해줘" | git diff HEAD~1..HEAD |
Run the appropriate git diff command to get the changes, then list all changed files.
Step 2: Classify Changed Files
Map each changed file to its domain to determine which conventions to load:
| File Pattern | Domain | Conventions to Load |
|---|---|---|
*.entity.ts |
Backend/TypeORM | BACKEND, TYPEORM, DATABASE |
*.service.ts |
Backend/Service | BACKEND, BACKEND_ARCHITECTURE, NESTJS |
*.controller.ts |
Backend/Controller | BACKEND, BACKEND_ARCHITECTURE, NESTJS, API_SPEC |
*.repository.ts |
Backend/Repository | BACKEND, BACKEND_ARCHITECTURE, TYPEORM |
*.dto.ts |
Backend/DTO | BACKEND, NESTJS, API_SPEC |
*.guard.ts, *.interceptor.ts |
Backend/Security | NESTJS, SECURITY |
*.module.ts (NestJS) |
Backend/Module | NESTJS |
*.migration.ts |
Database | DATABASE, MYSQL, TYPEORM |
*.prisma, schema.prisma |
Backend/Prisma | PRISMA, DATABASE |
prisma.service.ts, prisma.module.ts |
Backend/Prisma | PRISMA, NESTJS |
prisma-exception.filter.ts |
Backend/Prisma | PRISMA, NESTJS |
prisma/migrations/** |
Database/Prisma | PRISMA, DATABASE, MYSQL |
app/**/*.tsx |
Frontend/Route | NEXTJS, FRONTEND_ARCHITECTURE |
components/ui/** |
Frontend/UI | FRONTEND_ARCHITECTURE, STYLING |
components/feature/** |
Frontend/Feature | FRONTEND_ARCHITECTURE, NEXTJS, STATE |
queries/**, hooks/** |
Frontend/Data | STATE, NEXTJS, API_CLIENT |
store/** |
Frontend/State | STATE |
actions/** |
Frontend/Actions | NEXTJS, STATE |
lib/api*.ts |
Frontend/API | API_CLIENT, API_CLIENT_AXIOS |
routes/** |
Frontend/Route | REACT_ROUTER, FRONTEND_ARCHITECTURE |
*.stories.tsx |
Frontend/Test | TESTING |
*.spec.ts, *.test.ts |
Testing | TESTING (frontend) or NESTJS (backend) |
Read the conventions determined by the changed file domains.
Step 3: Analyze Violations
Review each changed file against loaded conventions. Check these categories:
Backend Checks
Architecture (BACKEND_ARCHITECTURE_CONVENTION)
- Controller contains only HTTP handling (no business logic, no direct DB access)
- Service contains all business logic (no HTTP concepts like Request/Response)
- Repository contains only data access (no business logic, no HTTP concepts)
- Dependency direction: Controller -> Service -> Repository (no reverse)
NestJS Patterns (NESTJS_CONVENTION)
- DTOs use
@sellernote/sellernote-nestjs-api-property(never raw@ApiProperty,class-validator,class-transformer) - Money fields:
stringtype +@SellernoteApiDecimal(nevernumber) - Money arithmetic:
big.jsin Service layer (never nativenumberarithmetic) - Transaction management:
@Transactional()(never rawQueryRunner) - Every endpoint has
@ApiOperationand@ApiResponse
TypeORM/Entity (TYPEORM_CONVENTION)
- Entity extends custom
BaseEntity(not TypeORM's) - Relations use
Relation<T>type wrapper - Enum columns use
varchar(not MySQLenumtype) - Decimal columns have
DecimalTransformer - Domain Model Interface (
I{Feature}Model) exists and Entity implements it
Prisma Schema (PRISMA_CONVENTION)
- All models have 공통 필드 (id, no, createdAt, updatedAt, deletedAt)
- All fields have
@db.*native type annotations (except BigInt) - Models use
@@map("snake_case")for table names - Fields use
@map("snake_case")for column names - FK fields have
@@index()defined - M:N relations use explicit join model (no implicit many-to-many)
- Enum values are lowercase snake_case
- PrismaService extends PrismaClient with OnModuleInit/OnModuleDestroy
- PrismaModule uses @Global() decorator
- No
$queryRawUnsafewith user input interpolation - No
Promise.allinside Interactive Transaction - findUnique used for PK/@unique lookups (not findFirst)
API Design (API_SPEC_CONVENTION)
- Response format:
{ success, data, error } - Pagination follows convention pattern
- Bulk operations follow convention pattern
Security (SECURITY_CONVENTION)
- Auth guards applied where needed
- Input validated via DTOs (no manual validation)
- No SQL injection vectors (parameterized queries only)
Frontend Checks
Architecture (FRONTEND_ARCHITECTURE_CONVENTION)
- Component type matches location (UI in
components/ui/, Feature incomponents/feature/) - Dependency direction: Page -> Feature -> UI (no reverse imports)
- UI components depend only on props (no store, query, or hook imports)
-
app/contains only route files (no components, hooks, stores)
Next.js (NEXTJS_CONVENTION)
-
page.tsxis Server Component (no'use client', no business logic) -
'use client'only at leaf/Feature components (not on Page or Layout) -
loading.tsxuses Skeleton component (not spinners) -
error.tsxhas'use client'directive - All imports use
@/absolute paths
State Management (STATE_CONVENTION)
- Server state in TanStack Query (not duplicated in Zustand)
- Client state in Zustand (UI toggles, filters, selections)
- Query key factory pattern used (
{feature}Keys)
Styling (STYLING_CONVENTION)
- Uses design tokens (no hex hardcoding)
- Priority: DS components > Tailwind utilities > cn() conditional
- Responsive using Tailwind breakpoints
Forms (FORM_CONVENTION)
- React Hook Form + Zod combination
- Zod schema validates all fields
React Patterns (REACT_CONVENTION)
- Compound Component pattern used for related component groups
- Form inputs are controlled (not uncontrolled in forms)
- No number directly on left side of
&&in conditional rendering - List keys use unique IDs (not array index)
- useEffect used only for external system sync (not derived state or event handling)
- useEffect has cleanup function when using subscriptions/timers
- No forwardRef in new code (React 19: ref as direct prop)
- No manual useMemo/useCallback with React Compiler enabled
- Error Boundary fallback has recovery mechanism
- Context split by concern (not one mega-context)
- ComponentPropsWithoutRef/ComponentPropsWithRef used for HTML wrapping components
Common Checks
TypeScript (TYPESCRIPT_CONVENTION)
- No
anytype (useunknownif needed) - Import order follows convention
- Proper naming: PascalCase (classes/interfaces/types), camelCase (variables/functions)
Common (COMMON_CONVENTION)
- Error handling follows convention patterns
- Naming conventions followed
Step 4: Generate Report
Output the review report grouped by file, with violations sorted by severity:
## Convention Review Report
### Summary
- Files reviewed: N
- Violations found: N (MUST: N, SHOULD: N, RECOMMEND: N)
---
### `src/modules/order/order.service.ts`
🔴 **[MUST]** Money arithmetic uses native `number` instead of `big.js`
- Line 45: `const total = price * quantity`
- Convention: NESTJS_CONVENTION > Money handling
- Fix: `const total = new Big(price).times(quantity).toFixed(2)`
🟡 **[SHOULD]** Missing `@Transactional()` on multi-step DB operation
- Line 72-85: `create()` method calls repository twice without transaction
- Convention: BACKEND_ARCHITECTURE_CONVENTION > Transaction management
🔵 **[RECOMMEND]** Consider extracting complex query logic to Repository
- Line 30-42: QueryBuilder chain in Service layer
- Convention: BACKEND_ARCHITECTURE_CONVENTION > Layer responsibilities
---
### `components/feature/Order/OrderList.tsx`
🔴 **[MUST]** UI component imports from store directly
- Line 3: `import { useOrderStore } from '@/store/orderStore'`
- Convention: FRONTEND_ARCHITECTURE_CONVENTION > Component dependency rules
- Fix: Move store usage to a Feature component or pass data via props
...
Severity levels:
- 🔴 [MUST] - Convention violation that must be fixed (breaks architecture rules)
- 🟡 [SHOULD] - Strong recommendation (improves code quality significantly)
- 🔵 [RECOMMEND] - Nice to have (follows best practices)
Step 5: Offer Fix Suggestions
After presenting the report, ask the user:
"위반사항을 수정하시겠습니까?
convention-refactorskill을 사용하여 자동으로 수정할 수 있습니다."
This skill does NOT modify files. For applying fixes, delegate to convention-refactor.
Key Rules Summary
| Rule | Detail |
|---|---|
| MUST | This skill is READ-ONLY -- never modify files |
| MUST | Load conventions dynamically based on changed file types |
| MUST | Check all applicable convention rules for each file |
| MUST | Report violations with severity (MUST/SHOULD/RECOMMEND) |
| MUST | Include specific line numbers and convention references |
| MUST | Suggest concrete fixes for each violation |
Cross-Skill References
- Apply fixes: Use the
convention-refactorskill to automatically fix reported violations - Backend implementation: Use
nestjs-api-devfor NestJS patterns - Entity patterns: Use
typeorm-devfor TypeORM conventions - Frontend implementation: Use
nextjs-ui-devfor component patterns - Data layer: Use
nextjs-data-providerfor query/state patterns - Prisma patterns: Use
prisma-devfor Prisma schema, client patterns, and NestJS integration - PR 생성: Use the
github-prskill to create a GitHub pull request