diff --git a/.claude/rules/coding-style.md b/.claude/rules/coding-style.md index 591e9be71..8b82be77e 100644 --- a/.claude/rules/coding-style.md +++ b/.claude/rules/coding-style.md @@ -8,6 +8,15 @@ - **`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. +## 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 - **Braces**: always use braces for single-statement `if` blocks. Never `if () return;` — write `if () { return; }`. @@ -43,6 +52,20 @@ 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. + ## 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: diff --git a/.claude/rules/prisma-db.md b/.claude/rules/prisma-db.md index 6f464f8b3..f7f5a5c03 100644 --- a/.claude/rules/prisma-db.md +++ b/.claude/rules/prisma-db.md @@ -33,6 +33,26 @@ paths: Use `prisma.$transaction()` for multi-step operations. +## Parallel Queries + +When `+page.server.ts` `load` makes multiple independent queries, run them concurrently with `Promise.all` to reduce page load latency: + +```typescript +// NG: sequential — each awaits the previous +const workbooks = await getWorkBooksWithAuthors(); +const tasks = await getTasksByTaskId(); +const results = await getTaskResultsOnlyResultExists(userId, true); + +// OK: all three fire at once +const [workbooks, tasks, results] = await Promise.all([ + getWorkBooksWithAuthors(), + getTasksByTaskId(), + getTaskResultsOnlyResultExists(userId, true), +]); +``` + +Only applies when queries are **independent** (no output of one used as input to another). + ## N+1 Queries Replace per-item DB calls in loops with a bulk fetch + `Map`: diff --git a/.claude/rules/svelte-components.md b/.claude/rules/svelte-components.md index 4903254c4..b28649340 100644 --- a/.claude/rules/svelte-components.md +++ b/.claude/rules/svelte-components.md @@ -39,6 +39,30 @@ Referencing `$props()` inside `$state()` initializer triggers "This reference on let count = $state(untrack(() => initialCount)); // intentional: prop is initial seed only ``` +## `$effect` — Store Reading + +Inside `$effect`, use `$store` syntax, not `get(store)`. `get()` bypasses the signal graph — the effect will not re-run when the store updates: + +```svelte +// Bad: get() takes a snapshot; effect won't react to store changes +$effect(() => { + const grade = get(myStore).get(key) ?? fallback; +}); + +// Good: $store subscribes and re-runs the effect on updates +$effect(() => { + const grade = $myStore.get(key) ?? fallback; +}); +``` + +## `$derived` — No Arrow Wrapper + +Use `$derived(expr)`, not `$derived(() => expr)`. The arrow form makes the derived value a _function_, not a reactive value — dependencies may not be tracked and the template call site is confusing. + +## `{@const}` Placement + +`{@const}` must be an **immediate child** of a block statement (`{#if}`, `{#each}`, `{:else}`, `{#snippet}`, etc.). Placing it inside an HTML element is a compile error: + ## `{#snippet}` Placement Define snippets at the **top level**, outside component tags. Inside a tag = named slot = type error: @@ -57,6 +81,8 @@ Define snippets at the **top level**, outside component tags. Inside a tag = nam Prefer `{#snippet}` when: (1) needs direct `$state` access, (2) pure display only, (3) same-file DRY. Promote to component when: independent state/lifecycle needed, exceeds ~30 lines, or reused across files. +**Sibling consistency** — when one sibling block warrants snippet extraction, extract its parallel siblings too, even if they are short. A parent template that mixes inline markup with `{@render}` calls is harder to scan than one where every top-level section is a named snippet. + ## Component Boundaries - One component, one responsibility: don't mix display, state management, and data fetching @@ -79,7 +105,17 @@ export function buildUpdatedUrl(url: URL, activeTab: ActiveTab): URL { ... } replaceState(buildUpdatedUrl($page.url, activeTab), {}); ``` -## Empty-list Fallback in `{#each}` +## `{#each}` — Keys and Empty-list Fallback + +Always provide a key expression when the list or its items may change dynamically. This is especially critical when the block contains an inner `{#if}` — without a key, Svelte reuses DOM nodes by position, so filtering can silently bind data to the wrong element: + +```svelte +{#each workbooks as workbook (workbook.id)} + {#if canRead(workbook)} + + {/if} +{/each} +``` Use `{:else}` to render a placeholder when the list is empty — no wrapper conditional needed: @@ -91,6 +127,10 @@ Use `{:else}` to render a placeholder when the list is empty — no wrapper cond {/each} ``` +## Directory Structure: `list/` Subdirectories + +Consider introducing a `list/` subdirectory (or other domain-scoped subdirectory) when the component count in a directory starts to feel unwieldy — roughly 20 files is a reasonable prompt to reconsider. Below that threshold, flat organization is preferred — subdirectories add navigation cost without proportional benefit. + ## Eliminate Branching with Records Replace `if`/ternary chains with `Record`: @@ -103,3 +143,17 @@ const TAB_CONFIGS: Record = { ``` Use the enum type as the key type, not `string`. + +When not all enum keys need an entry, use `Partial>` as a **type annotation** — not `satisfies`. `as const satisfies Partial>` preserves the narrowed literal type, so indexing with other enum values causes a type error: + +```typescript +// NG: satisfies narrows the type — obj[key] errors for keys not in the literal +const map = { [WorkBookType.SOLUTION]: SolutionTable } + as const satisfies Partial>>; + +// OK: type annotation makes map[key] return Component | undefined +const map: Partial>> = { + [WorkBookType.SOLUTION]: SolutionTable, +}; +// Safe to guard with: {#if map[type]} or if (map[type]) +``` diff --git a/.claude/rules/svelte-docs.md b/.claude/rules/svelte-docs.md new file mode 100644 index 000000000..01aca4d30 --- /dev/null +++ b/.claude/rules/svelte-docs.md @@ -0,0 +1,18 @@ +--- +description: Svelte official documentation reference +paths: + - 'src/**/*.svelte' + - 'src/**/*.ts' +--- + +# Svelte Official Documentation + +When Svelte 5 behavior is unclear, fetch the official docs directly via WebFetch instead of relying on training knowledge. + +URL pattern: `https://svelte.dev/docs/svelte/{section}` + +Examples: + +- `$effect` behavior → `https://svelte.dev/docs/svelte/$effect` +- Stores usage → `https://svelte.dev/docs/svelte/stores` +- Runes overview → `https://svelte.dev/docs/svelte/what-are-runes` diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index cd9b06c5e..753060db1 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -9,6 +9,10 @@ paths: # Testing +## Test Titles + +Write all test titles in English. Use descriptive sentences that state the expected behavior (e.g., `'returns empty array when workbooks is empty'`). Japanese is only acceptable in inline comments or fixture strings that represent real user-facing content. + ## Test Integrity - Never delete, comment out, or weaken assertions (e.g. `toEqual` → `toBeDefined`) to make tests pass @@ -26,6 +30,7 @@ paths: - Use `toBe(true)` / `toBe(false)` over `toBeTruthy()` / `toBeFalsy()` - For DB query tests, assert `orderBy`, `include`, and other significant parameters with `expect.objectContaining` — not just `where` - Enum membership: `in` traverses the prototype chain; use `Object.hasOwn(Enum, value)` instead +- **E2E state transitions**: after an interaction that changes element state (active tab, toggle, selection), assert the _new_ state — not just that the element is visible, which may have been true before the interaction. Assert an active CSS class, `aria-selected`, or similar attribute instead of `toBeVisible()` ## Cleanup in Tests @@ -45,19 +50,42 @@ try { - Use realistic fixture values (real task IDs, grade names) instead of placeholders like `'t1'` - Extract shared data into fixture files; inline is fine for single-use cases - After `.filter()` on fixtures, verify actual contents — same ID may refer to a different entity after fixture updates +- **Description ↔ code path alignment**: when a test name describes a specific scenario (e.g. "tie-break"), verify the fixture actually exercises that code path. A test that passes without reaching the branch it claims to cover gives false confidence ## Mock Helpers -Extract repeated mock patterns into a helper in the test file: +Extract repeated mock patterns into helpers in the test file. For Prisma service tests, define the return type alias once and use it across all helpers: ```typescript -function mockFindMany(value: WorkBookPlacements) { - vi.mocked(prisma.workBookPlacement.findMany).mockResolvedValue( - value as unknown as Awaited>, +type PrismaWorkBook = Awaited>; +type PrismaWorkBookRow = Awaited>[number]; + +function mockFindUnique(value: PrismaWorkBook) { + vi.mocked(prisma.workBook.findUnique).mockResolvedValue(value); +} + +function mockFindMany(value: PrismaWorkBookRow[]) { + vi.mocked(prisma.workBook.findMany).mockResolvedValue( + value as unknown as Awaited>, ); } + +function mockCount(value: number) { + vi.mocked(prisma.workBook.count).mockResolvedValue(value); +} ``` +Extract `mockFindUnique`, `mockFindMany`, and `mockCount` as the standard trio for service tests that touch a single Prisma model. Add `mockCreate`, `mockTransaction`, and `mockDelete` when those operations are also tested. + +## Component Vitest Unit Tests + +Omit Vitest unit tests for a Svelte component when **both** conditions hold: + +1. The component is template-only (no logic beyond prop bindings and basic conditionals) +2. The component is covered by E2E tests + +When a component contains extracted logic (e.g. derived values, event handlers, utility calls), add unit tests for that logic in the nearest `utils/` file instead of testing the component directly. + ## Testing Extracted Utilities - Add tests at extraction time, not later @@ -81,3 +109,17 @@ Stop the split if internal helpers (e.g. `fetchUnplacedWorkbooks`) would be frag ## HTTP Mocking Use Nock for external HTTP calls. See `src/test/lib/clients/` for examples. + +## Flowbite Toggle in E2E Tests + +Flowbite's `Toggle` renders an `sr-only` `` inside a `