diff --git a/.claude/rules/coding-style.md b/.claude/rules/coding-style.md index f21ac499c..4f492d4e1 100644 --- a/.claude/rules/coding-style.md +++ b/.claude/rules/coding-style.md @@ -1,72 +1,45 @@ # Coding Style -## Naming +## Plan Time + +### Pre-Implementation Layer Check + +Before writing new logic, decide which layer it belongs to. Run this check at plan time (design/architecture phase, before writing any code): + +| Layer | Directory | Key constraints | +| -------------- | ------------------------------------------------------ | -------------------------------------------------------------------------- | +| DB schema | `prisma/` | Migrations are immutable after apply | +| DB access | `src/lib/server/` | Server-only; never import in client code | +| Validation | `src/**/zod/` | `z.number().int()` for Int fields; comment dual-enforcement with SQL CHECK | +| Domain types | `src/**/types/` (`_types/` inside `src/routes/`) | Plural aliases; TSDoc on every export; no `any` | +| Test data | `src/**/fixtures/` (`_fixtures/` inside `src/routes/`) | Write before implementation (TDD); use realistic values | +| Business logic | `src/**/services/` | Return pure values or `null`; no `Response`/`json()` | +| Pure utilities | `src/**/utils/` (`_utils/` inside `src/routes/`) | No side effects; adjacent unit test required | +| State | `src/**/stores/` | `.svelte.ts`; class + `$state()`; singleton export | +| Route handlers | `src/routes/` | Page: `redirect()`; API: `error()` | +| UI components | `src/**/*.svelte` | Svelte 5 Runes; business logic → `utils/` | + +## Code Structure + +### Naming - **Abbreviations**: avoid non-standard abbreviations (`res` → `response`, `btn` → `button`). When in doubt, spell it out. - **Lambda parameters**: no single-character names (e.g., use `placement`, `workbook`). Iterator index `i` is the only exception. - **`upsert`**: only use when the implementation performs both insert and update. For insert-only, use `initialize`, `seed`, or another accurate verb. - **`any`**: before using `any`, check the value's origin — adding a missing `@types/*` or `devDependency` often provides the correct type. - **UI labels**: if a label does not match actual behavior, update it or add an inline comment explaining the intentional mismatch. +- **New files**: before naming a new file or directory, grep the relevant `src/` directory to confirm existing conventions. Confirm at plan time, not during implementation: + - Custom files in routes (utilities, helpers, etc.): `snake_case` (e.g., `user_profile.ts`) + - SvelteKit special files: follow framework conventions (`+page.svelte`, `+page.server.ts`, `+server.ts`) + - Helper directories inside `src/routes/`: underscore-prefixed (`_utils/`, `_types/`, `_fixtures/`, `_components/`) + - Other directories: `kebab-case` (e.g., `contest-table/`) -## TSDoc - -Add TSDoc comments to every exported function, type, and class. The minimum required fields are `@param` (for non-obvious parameters) and `@returns` (when the return value is not evident from the type). One-liner `/** ... */` is sufficient for simple cases; use multi-line only when behaviour needs explanation. - -```typescript -/** Returns the URL slug for a workbook, falling back to the workbook ID. */ -export function getUrlSlugFrom(workbook: WorkbookList): string { ... } -``` - -## Syntax +### Syntax - **Braces**: always use braces for single-statement `if` blocks. Never `if () return;` — write `if () { return; }`. - **Plural type aliases**: define `type Placements = Placement[]` instead of using `Placement[]` directly in signatures and variables. -## Markdown Code Blocks - -Always specify a language identifier on every fenced code block. Never write bare ` ``` `. - -Common identifiers: `typescript`, `svelte`, `sql`, `bash`, `mermaid`, `json`, `prisma`, `html`, `css`. - -## SvelteKit: Routes vs API Endpoints - -- Page routes (`+page.server.ts`): use `redirect()` to navigate -- API routes (`+server.ts`): use `error()` — throwing `redirect()` returns a 3xx response; `fetch` follows it by default and receives the HTML page at the redirect target instead of a JSON error - -## Dual-Enforcement Constraints - -When the same constraint is enforced in two layers (e.g. Zod validation + SQL `CHECK`), add an inline comment stating each layer's role and the obligation to keep them in sync: - -```typescript -// XOR constraint: dual enforcement via Zod (early validation) and a CHECK in migration.sql (last line of defence). -// Prisma lacks @@check, so the SQL constraint is maintained manually. Keep both in sync. -.refine(...) -``` - -## Optimistic Updates - -Derive computed fields (flags, labels, etc.) from the canonical data source — don't -re-implement the derivation inline. Divergence causes a "works after reload" bug where -the server state is correct but the client-side update is wrong. - -**Diagnostic**: "Not reflected live, but fixed after reload" → suspect the optimistic -update payload, not the reactivity system. - -## Server-side Logging - -Do not log user-identifiable or content data (titles, names, IDs that map to users) in server-side `console.log`. Use generic messages instead: - -```typescript -// Bad: leaks content and user identity -console.log(`Created workbook "${workBook.title}" by user ${author.id}`); - -// Good -console.log('Workbook created successfully'); -``` - -Prefer placing the single authoritative log in the service layer; remove duplicate logs in route handlers that cover the same event. - -## No Hard-Coded Values +### No Hard-Coded Values Extract magic numbers and strings to named constants. Never embed literal values whose meaning is not self-evident from the type or immediate context. @@ -86,7 +59,7 @@ const SUBMIT_URL = '/api/workbooks/submit'; Place constants at the top of the file, or in a dedicated `constants/` module when shared across files. -## Function Ordering +### Function Ordering Within a file, order declarations as follows: @@ -95,7 +68,24 @@ Within a file, order declarations as follows: Shared helper functions (used by two or more exports) should be grouped at the end of the file. -## Svelte 5: Prefer Official Docs Over Training Knowledge +## Documentation + +### TSDoc + +Add TSDoc comments to every exported function, type, and class. The minimum required fields are `@param` (for non-obvious parameters) and `@returns` (when the return value is not evident from the type). One-liner `/** ... */` is sufficient for simple cases; use multi-line only when behavior needs explanation. + +```typescript +/** Returns the URL slug for a workbook, falling back to the workbook ID. */ +export function getUrlSlugFrom(workbook: WorkbookList): string { ... } +``` + +### Markdown Code Blocks + +Always specify a language identifier on every fenced code block. Never write bare ` ``` `. + +Common identifiers: `typescript`, `svelte`, `sql`, `bash`, `mermaid`, `json`, `prisma`, `html`, `css`. + +### Svelte 5: Prefer Official Docs Over Training Knowledge When Svelte 5 behavior is unclear, fetch the official docs directly via WebFetch instead of relying on training knowledge. @@ -107,7 +97,42 @@ Examples: - Stores usage → `https://svelte.dev/docs/svelte/stores` - Runes overview → `https://svelte.dev/docs/svelte/what-are-runes` -## Async Rollback: Capture State Before `await` +## SvelteKit Patterns + +### Routes vs API Endpoints + +- Page routes (`+page.server.ts`): use `redirect()` to navigate +- API routes (`+server.ts`): use `error()` — throwing `redirect()` returns a 3xx response; `fetch` follows it by default and receives the HTML page at the redirect target instead of a JSON error + +### Dual-Enforcement Constraints + +When the same constraint is enforced in two layers (e.g. Zod validation + SQL `CHECK`), add an inline comment stating each layer's role and the obligation to keep them in sync: + +```typescript +// XOR constraint: dual enforcement via Zod (early validation) and a CHECK in migration.sql (last line of defense). +// Prisma lacks @@check, so the SQL constraint is maintained manually. Keep both in sync. +.refine(...) +``` + +## Security + +### Server-side Logging + +Do not log user-identifiable or content data (titles, names, IDs that map to users) in server-side `console.log`. Use generic messages instead: + +```typescript +// Bad: leaks content and user identity +console.log(`Created workbook "${workBook.title}" by user ${author.id}`); + +// Good +console.log('Workbook created successfully'); +``` + +Prefer placing the single authoritative log in the service layer; remove duplicate logs in route handlers that cover the same event. + +## Svelte Patterns + +### Async Rollback: Capture State Before `await` Capture `$state` values before the first `await` for safe rollback. A concurrent update can overwrite the variable while awaiting: @@ -119,3 +144,23 @@ try { items = previous; } ``` + +### Optimistic Updates + +Derive computed fields (flags, labels, etc.) from the canonical data source — don't +re-implement the derivation inline. Divergence causes a "works after reload" bug where +the server state is correct but the client-side update is wrong. + +**Diagnostic**: "Not reflected live, but fixed after reload" → suspect the optimistic +update payload, not the reactivity system. + +## Code Review + +### CodeRabbit Review: Severity Triage + +When running `coderabbit review --plain` at a Phase milestone: + +- **critical / high**: fix before starting the next Phase +- **low / info**: review before the next Phase starts; fix immediately only if security- or regression-related; otherwise defer to final PR review (alongside CodeRabbit CI comments) + +Run once per Phase boundary — not on every commit. diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 000000000..de4225dd6 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,13 @@ +{ + "enabledPlugins": { + "superpowers@superpowers-dev": true + }, + "extraKnownMarketplaces": { + "superpowers-dev": { + "source": { + "source": "git", + "url": "https://github.com/obra/superpowers.git" + } + } + } +} diff --git a/.claude/skills/refactor-plan/instructions.md b/.claude/skills/refactor-plan/instructions.md index d988a3eaf..9a3725920 100644 --- a/.claude/skills/refactor-plan/instructions.md +++ b/.claude/skills/refactor-plan/instructions.md @@ -52,6 +52,8 @@ Scan the target code in this order (lowest risk first). List every concrete find - Order phases by risk: isolated mechanical changes first, structural changes last - State inter-phase dependencies explicitly - For uncertain changes, add an **investigation sub-step** before the implementation task +- Each task must have single responsibility — implement one logical change (e.g., "add a field", "rename a type") even if it spans multiple layers; avoid mixing unrelated changes in one task +- "Implementation + test + commit" should fit within one task; if not, split further ## When to Skip Tests diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 000000000..ef1487b36 --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,67 @@ +language: 'ja-JP' +tone_instructions: '簡潔に。重要度が high 以上のみ詳述し、low/info は箇条書きでまとめる。' + +reviews: + profile: 'assertive' + auto_review: + enabled: true + drafts: false + path_filters: + - '!node_modules/**' + - '!.pnpm-store/**' + - '!pnpm-lock.yaml' + - '!.svelte-kit/**' + - '!.vercel/**' + - '!coverage/**' + path_instructions: + # --- Data layer --- + - path: 'prisma/*' + instructions: | + - CHECK constraints added manually to migration.sql must be documented in prisma/ERD.md. + - path: 'prisma/migrations/**' + instructions: | + - Auto-generated by Prisma. Only flag security vulnerabilities (e.g. privilege escalation, SQL injection) or bugs that could cause data loss or critical errors. + - Skip style, naming, SQL formatting, and documentation issues. + - path: 'src/lib/server/**' + instructions: | + - This directory is server-only. Never import from client-side code or Svelte components. + - DB client must be accessed only via `$lib/server/database`. + # --- Schema / validation --- + - path: 'src/**/zod/**' + instructions: | + - Use `z.number().int().positive()` for Prisma `Int` fields (not `z.number().positive()`, which passes decimals). + - When a Zod constraint mirrors a SQL CHECK, add an inline comment noting both layers and the obligation to keep them in sync. + # --- Domain types --- + - path: 'src/**/*types/**' + instructions: | + - Array types must use a named plural alias: `type Placements = Placement[]`, not `Placement[]` directly in signatures. + - Add TSDoc to every exported type. + - Avoid `any`; check if a `@types/*` package exists first. + # --- Test infrastructure (defined before implementation per TDD) --- + - path: 'src/**/*fixtures/**' + instructions: | + - Use realistic domain values, not abstract placeholders ('t1', 'test1', etc.). + - Shared fixture data must be extracted to a fixture file; avoid duplicating across test cases. + # --- Business logic --- + - path: 'src/**/services/**' + instructions: | + - Service functions must return pure values or null; never Response/json(). + - Prisma calls belong here; do not flag direct database access as a violation. + - path: 'src/**/*utils/**' + instructions: | + - Must be pure functions with no side effects. + - Each function must have an adjacent unit test. + # --- State / UI --- + - path: 'src/**/stores/**' + instructions: | + - Stores must use `.svelte.ts` extension, class-based pattern with `$state()`, and export a singleton. + - Legacy stores using `writable()` must be migrated before adding features or extending them. + - path: 'src/routes/**' + instructions: | + - Page routes (+page.server.ts): use redirect() for navigation. + - API routes (+server.ts): use error() — redirect() causes fetch to silently receive the HTML page at the redirect target. + - No direct Prisma calls; delegate to service layer. + - path: 'src/**/*.svelte' + instructions: | + - Use Svelte 5 Runes ($props, $state, $derived, $effect). + - Business logic belongs in utils/, not inside