Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 3 additions & 44 deletions .claude/rules/coding-style.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ Shared helper functions (used by two or more exports) should be grouped at the e

Write all project documentation (plans, dev-notes, guides, refactor notes) in Japanese. Write all source code comments, TSDoc, commit messages, and test titles in English. This keeps documentation readable for the team while keeping code comments universally accessible and searchable.

**Exception**: The `## CodeRabbit Findings` section in `refactor.md` must quote findings verbatim in their original language (English). Do not translate CodeRabbit output.
**Exception**: The `## CodeRabbit Findings` section in `plan.md` must quote findings verbatim in their original language (English). Do not translate CodeRabbit output.

### TSDoc

Expand Down Expand Up @@ -106,23 +106,6 @@ Examples:
- Stores usage → `https://svelte.dev/docs/svelte/stores`
- Runes overview → `https://svelte.dev/docs/svelte/what-are-runes`

## 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
Expand All @@ -139,30 +122,6 @@ 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:

```typescript
const previous = items; // capture before await
try {
await saveToServer(items);
} catch {
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
Expand All @@ -171,7 +130,7 @@ Run `coderabbit review --plain` once after all phases are complete (not on every

**Triage by severity:**

- **critical / high / potential_issue (medium)**: Write all findings verbatim to a `## CodeRabbit Findings` section in `refactor.md`. The user decides which to fix before opening the PR. Do not fix any of these findings unilaterally.
- **critical / high / potential_issue (medium)**: Write all findings verbatim to a `## CodeRabbit Findings` section in `plan.md`. The user decides which to fix before opening the PR. Do not fix any of these findings unilaterally.
- **nitpick / info**: Defer to PR CI — CodeRabbit will re-comment on the open PR.

Writing medium-and-above findings to `refactor.md` serves a dual purpose: it gives the user full visibility for a fix/defer decision, and it builds the implementer's understanding of recurring quality issues.
Writing medium-and-above findings to `plan.md` serves a dual purpose: it gives the user full visibility for a fix/defer decision, and it builds the implementer's understanding of recurring quality issues.
10 changes: 10 additions & 0 deletions .claude/rules/prisma-db.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,16 @@ automatically excluded. This is not an IS NOT NULL check; the mechanism is the J
When documenting this behavior, write "excluded by INNER JOIN" rather than
"implicitly includes IS NOT NULL".

## Dual-Enforcement Constraints

When the same constraint is enforced in both Zod (early validation) and SQL `CHECK` (last line of defense), 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(...)
```

## Validate Constraints

Prisma does not support `@@check`. To add one:
Expand Down
49 changes: 6 additions & 43 deletions .claude/rules/svelte-components.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,48 +37,6 @@ Import from `flowbite-svelte`. Use Tailwind CSS v4 utility classes. Dark mode: `

When copying button styles from a reference component, always check all three axes: `color`, `size`, and `class`. Omitting `color` applies Flowbite's default (filled blue).

## `let`/`const` — Reactive Data Requires `$derived`

Plain `let` or `const` in Svelte 5 component `<script>` executes once at component creation. Values derived from props or server data must use `$derived()`:

```md
// Bad: captures only the initial value — won't update when data reloads
let user = data.loggedInUser; const categories = availableCategories.filter(...);

// Good
let user = $derived(data.loggedInUser); let categories = $derived(availableCategories.filter(...));
```

`pnpm check` warns: "This reference only captures the initial value."

## `$state()` Initialization with `$props()`

Referencing `$props()` inside `$state()` initializer triggers "This reference only captures the initial value". Wrap with `untrack` if intentional:

```svelte
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:
Expand Down Expand Up @@ -127,7 +85,12 @@ replaceState(buildUpdatedUrl($page.url, activeTab), {});

## `{#each}` — Keys and Empty-list Fallback

Always provide a key expression when the list or its items may change dynamically.
Always provide a key expression on every `{#each}` block. Prefer a unique field (`id`, `name`, etc.); fall back to the index `(i)` only when no unique field exists.

```svelte
{#each items as item (item.id)} <!-- unique field preferred -->
{#each labels as label (i)} <!-- index fallback -->
```

**Filter before `{#each}`, not inside it.** When visibility depends on a predicate (e.g. `canRead`), derive a filtered list once and iterate over it — never repeat the predicate inside a `{#if}` within the loop. This avoids computing the condition twice and keeps the template clean:

Expand Down
99 changes: 99 additions & 0 deletions .claude/rules/svelte-runes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
---
description: Svelte 5 Runes and reactivity rules
paths:
- 'src/**/*.svelte'
- 'src/lib/stores/**/*.svelte.ts'
- 'src/features/**/stores/**'
---

# Svelte Runes & Reactivity

## `SvelteMap<K, V>` — Always Provide Type Parameters

`SvelteMap` without type parameters makes `.get()` return `unknown`. Always declare with explicit types:

```typescript
// Bad: .get() returns unknown — causes type errors at call sites
const map = new SvelteMap();

// Good: .get() returns TaskResults | undefined
const map = new SvelteMap<TaskGrade, TaskResults>();
```

`$state()` wrapping is unnecessary — `SvelteMap` is already reactive (`svelte/no-unnecessary-state-wrap`). Reset with `.clear()` rather than reassigning.

`svelte/prefer-svelte-reactivity` targets only `$state` contexts. Inside `$derived`, a plain `new Map()` is sufficient — the reactive dependency is tracked at the `$derived` level, not via `SvelteMap` mutation signals. Do not introduce `SvelteMap` inside `$derived` solely to satisfy this rule.

## `let`/`const` — Reactive Data Requires `$derived`

Plain `let` or `const` in Svelte 5 component `<script>` executes once at component creation. Values derived from props or server data must use `$derived()`:

```typescript
// Bad: captures only the initial value — won't update when data reloads
let user = data.loggedInUser;

// Good
let user = $derived(data.loggedInUser);
```

`pnpm check` warns: "This reference only captures the initial value."

## `$state()` Initialization with `$props()`

Referencing `$props()` inside `$state()` initializer triggers "This reference only captures the initial value". Wrap with `untrack` if intentional:

```svelte
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` — Prefer Over `$state` + `$effect`

When a value is purely computed from other state, use `$derived` instead of initializing with `$state` and updating in `$effect`:

```typescript
// Bad: unnecessary mutation via $effect
let items = $state<Item[]>([]);
$effect(() => {
items = source.filter(isActive);
});

// Good: $derived — reactive, no mutation needed
let items = $derived(source.filter(isActive));
```

**Do not pass an arrow function to `$derived`**: `$derived(() => fn(x))` stores the arrow function itself as the derived value — `fn` is never called and `x` is not tracked as a dependency. Use `$derived(fn(x))` for single expressions, or `$derived.by(() => { ... })` when multiple statements are needed. `$derived(expr)` is equivalent to `$derived.by(() => expr)` — the `.by` variant exists solely to allow a multi-statement function body.

## 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:

```typescript
const previous = items; // capture before await
try {
await saveToServer(items);
} catch {
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.
41 changes: 41 additions & 0 deletions .claude/rules/sveltekit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
description: SvelteKit routing and navigation patterns
paths:
- 'src/routes/**'
- 'src/**/*.svelte'
---

# 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

## Internal Navigation: `resolve()` Wrapping

`svelte/no-navigation-without-resolve` requires all internal navigation to use `resolve()` from `$app/paths`. Two patterns apply:

**Parameterized routes** — type-safe, preferred:

```typescript
import { resolve } from '$app/paths';
resolve('/workbooks/[slug]', { slug: workbook.slug });
```

**Static routes and computed string paths** — TypeScript declaration merging causes `ReturnType<AppTypes['RouteId']>` to resolve as `string` (the base overload in `@sveltejs/kit/types/index.d.ts` wins), making all routes require 2 arguments. Suppress with a description, and pre-compute in `<script>` when used in templates (where `@ts-expect-error` cannot be placed inline). A wrapper function does not help — the ESLint rule may not recognize `resolve()` calls inside wrappers.

```typescript
// @ts-expect-error svelte-check TS2554: AppTypes declaration merging causes RouteId to resolve as string, requiring params. Runtime behavior is correct.
const homeHref = resolve('/');
```

**External links** — add `rel="noreferrer external"` instead of wrapping with `resolve()`.

**Do not pass query strings to `resolve()`** — `resolve()` accepts route patterns only (e.g. `'/workbooks'`). A path like `'/workbooks?tab=foo'` is not a valid route pattern and causes a type error. Split path and search before passing: `resolve(url.pathname) + url.search`.

**Do not apply `resolve()` to URLs derived from `$page.url`** — `$page.url` is the actual browser URL with the base path already applied. Wrapping a value derived from it with `resolve()` double-applies the base path. Pass the `URL` object directly to `replaceState` or extract `.pathname + .search` without `resolve()`.

## Page Component Props

SvelteKit page components (`+page.svelte`) accept only `data` and `form` as props (`svelte/valid-prop-names-in-kit-pages`). Commented-out features that reference other props are not "dead code" — remove only the violating prop declaration, preserve the feature code.
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Always prefer simplicity over pathological correctness. YAGNI, KISS, DRY. No bac
2. Before writing a new function, search `src/lib/utils/`, `src/lib/services/`, `src/features/*/utils/` and `src/features/*/services/` for existing implementations; extract shared logic there when it appears in 2+ places
3. Write tests first, then implement production code, then verify with `pnpm test:unit`
4. Review critically after implementing: flag YAGNI violations, over-abstraction, missing tests
5. After all phases complete (feature and refactor branches only — not hotfixes or dependency bumps): run a mandatory refactor cycle. Produce `refactor.md` in the same directory as the plan, documenting: design decisions made, changes explicitly rejected and why, remaining tasks, and per-phase lessons. Transfer all lessons to `plan.md`, then discard `phase-N.md` files. Run `coderabbit review --plain`; write all findings of `critical` / `high` / `potential_issue` (medium) to a `## CodeRabbit Findings` section in `refactor.md`. The user decides which to fix before opening a PR; do not fix any finding unilaterally. `nitpick` findings defer to PR CI.
5. After all phases complete (feature and refactor branches only — not hotfixes or dependency bumps): run a mandatory refactor cycle. Write to `plan.md`: novel lessons (implementation blockers, non-obvious patterns not already in rules) and remaining tasks. Discard `phase-N.md` files. Run `coderabbit review --plain`; write all findings of `critical` / `high` / `potential_issue` (medium) to a `## CodeRabbit Findings` section in `plan.md`. The user decides which to fix before opening a PR; do not fix any finding unilaterally. `nitpick` findings defer to PR CI.
6. Run `/session-close` at the end of each session: updates plan checklist, proposes rule/skill additions, checks for bloat, and detects repeated instructions

## Tech Stack
Expand Down
Loading
Loading