Skip to content

Commit cd54f83

Browse files
authored
Merge pull request #3293 from AtCoder-NoviSteps/#3292
feat: Introduce writing-plans plugin and CodeRabbit CLI for plan quality (#3292)
2 parents eab9938 + 14e8e9f commit cd54f83

9 files changed

Lines changed: 350 additions & 66 deletions

File tree

.claude/rules/coding-style.md

Lines changed: 104 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,72 +1,45 @@
11
# Coding Style
22

3-
## Naming
3+
## Plan Time
4+
5+
### Pre-Implementation Layer Check
6+
7+
Before writing new logic, decide which layer it belongs to. Run this check at plan time (design/architecture phase, before writing any code):
8+
9+
| Layer | Directory | Key constraints |
10+
| -------------- | ------------------------------------------------------ | -------------------------------------------------------------------------- |
11+
| DB schema | `prisma/` | Migrations are immutable after apply |
12+
| DB access | `src/lib/server/` | Server-only; never import in client code |
13+
| Validation | `src/**/zod/` | `z.number().int()` for Int fields; comment dual-enforcement with SQL CHECK |
14+
| Domain types | `src/**/types/` (`_types/` inside `src/routes/`) | Plural aliases; TSDoc on every export; no `any` |
15+
| Test data | `src/**/fixtures/` (`_fixtures/` inside `src/routes/`) | Write before implementation (TDD); use realistic values |
16+
| Business logic | `src/**/services/` | Return pure values or `null`; no `Response`/`json()` |
17+
| Pure utilities | `src/**/utils/` (`_utils/` inside `src/routes/`) | No side effects; adjacent unit test required |
18+
| State | `src/**/stores/` | `.svelte.ts`; class + `$state()`; singleton export |
19+
| Route handlers | `src/routes/` | Page: `redirect()`; API: `error()` |
20+
| UI components | `src/**/*.svelte` | Svelte 5 Runes; business logic → `utils/` |
21+
22+
## Code Structure
23+
24+
### Naming
425

526
- **Abbreviations**: avoid non-standard abbreviations (`res``response`, `btn``button`). When in doubt, spell it out.
627
- **Lambda parameters**: no single-character names (e.g., use `placement`, `workbook`). Iterator index `i` is the only exception.
728
- **`upsert`**: only use when the implementation performs both insert and update. For insert-only, use `initialize`, `seed`, or another accurate verb.
829
- **`any`**: before using `any`, check the value's origin — adding a missing `@types/*` or `devDependency` often provides the correct type.
930
- **UI labels**: if a label does not match actual behavior, update it or add an inline comment explaining the intentional mismatch.
31+
- **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:
32+
- Custom files in routes (utilities, helpers, etc.): `snake_case` (e.g., `user_profile.ts`)
33+
- SvelteKit special files: follow framework conventions (`+page.svelte`, `+page.server.ts`, `+server.ts`)
34+
- Helper directories inside `src/routes/`: underscore-prefixed (`_utils/`, `_types/`, `_fixtures/`, `_components/`)
35+
- Other directories: `kebab-case` (e.g., `contest-table/`)
1036

11-
## TSDoc
12-
13-
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.
14-
15-
```typescript
16-
/** Returns the URL slug for a workbook, falling back to the workbook ID. */
17-
export function getUrlSlugFrom(workbook: WorkbookList): string { ... }
18-
```
19-
20-
## Syntax
37+
### Syntax
2138

2239
- **Braces**: always use braces for single-statement `if` blocks. Never `if () return;` — write `if () { return; }`.
2340
- **Plural type aliases**: define `type Placements = Placement[]` instead of using `Placement[]` directly in signatures and variables.
2441

25-
## Markdown Code Blocks
26-
27-
Always specify a language identifier on every fenced code block. Never write bare ` ``` `.
28-
29-
Common identifiers: `typescript`, `svelte`, `sql`, `bash`, `mermaid`, `json`, `prisma`, `html`, `css`.
30-
31-
## SvelteKit: Routes vs API Endpoints
32-
33-
- Page routes (`+page.server.ts`): use `redirect()` to navigate
34-
- 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
35-
36-
## Dual-Enforcement Constraints
37-
38-
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:
39-
40-
```typescript
41-
// XOR constraint: dual enforcement via Zod (early validation) and a CHECK in migration.sql (last line of defence).
42-
// Prisma lacks @@check, so the SQL constraint is maintained manually. Keep both in sync.
43-
.refine(...)
44-
```
45-
46-
## Optimistic Updates
47-
48-
Derive computed fields (flags, labels, etc.) from the canonical data source — don't
49-
re-implement the derivation inline. Divergence causes a "works after reload" bug where
50-
the server state is correct but the client-side update is wrong.
51-
52-
**Diagnostic**: "Not reflected live, but fixed after reload" → suspect the optimistic
53-
update payload, not the reactivity system.
54-
55-
## Server-side Logging
56-
57-
Do not log user-identifiable or content data (titles, names, IDs that map to users) in server-side `console.log`. Use generic messages instead:
58-
59-
```typescript
60-
// Bad: leaks content and user identity
61-
console.log(`Created workbook "${workBook.title}" by user ${author.id}`);
62-
63-
// Good
64-
console.log('Workbook created successfully');
65-
```
66-
67-
Prefer placing the single authoritative log in the service layer; remove duplicate logs in route handlers that cover the same event.
68-
69-
## No Hard-Coded Values
42+
### No Hard-Coded Values
7043

7144
Extract magic numbers and strings to named constants. Never embed literal values whose meaning is not self-evident from the type or immediate context.
7245

@@ -86,7 +59,7 @@ const SUBMIT_URL = '/api/workbooks/submit';
8659

8760
Place constants at the top of the file, or in a dedicated `constants/` module when shared across files.
8861

89-
## Function Ordering
62+
### Function Ordering
9063

9164
Within a file, order declarations as follows:
9265

@@ -95,7 +68,24 @@ Within a file, order declarations as follows:
9568

9669
Shared helper functions (used by two or more exports) should be grouped at the end of the file.
9770

98-
## Svelte 5: Prefer Official Docs Over Training Knowledge
71+
## Documentation
72+
73+
### TSDoc
74+
75+
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.
76+
77+
```typescript
78+
/** Returns the URL slug for a workbook, falling back to the workbook ID. */
79+
export function getUrlSlugFrom(workbook: WorkbookList): string { ... }
80+
```
81+
82+
### Markdown Code Blocks
83+
84+
Always specify a language identifier on every fenced code block. Never write bare ` ``` `.
85+
86+
Common identifiers: `typescript`, `svelte`, `sql`, `bash`, `mermaid`, `json`, `prisma`, `html`, `css`.
87+
88+
### Svelte 5: Prefer Official Docs Over Training Knowledge
9989

10090
When Svelte 5 behavior is unclear, fetch the official docs directly via WebFetch instead of relying on training knowledge.
10191

@@ -107,7 +97,42 @@ Examples:
10797
- Stores usage → `https://svelte.dev/docs/svelte/stores`
10898
- Runes overview → `https://svelte.dev/docs/svelte/what-are-runes`
10999

110-
## Async Rollback: Capture State Before `await`
100+
## SvelteKit Patterns
101+
102+
### Routes vs API Endpoints
103+
104+
- Page routes (`+page.server.ts`): use `redirect()` to navigate
105+
- 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
106+
107+
### Dual-Enforcement Constraints
108+
109+
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:
110+
111+
```typescript
112+
// XOR constraint: dual enforcement via Zod (early validation) and a CHECK in migration.sql (last line of defense).
113+
// Prisma lacks @@check, so the SQL constraint is maintained manually. Keep both in sync.
114+
.refine(...)
115+
```
116+
117+
## Security
118+
119+
### Server-side Logging
120+
121+
Do not log user-identifiable or content data (titles, names, IDs that map to users) in server-side `console.log`. Use generic messages instead:
122+
123+
```typescript
124+
// Bad: leaks content and user identity
125+
console.log(`Created workbook "${workBook.title}" by user ${author.id}`);
126+
127+
// Good
128+
console.log('Workbook created successfully');
129+
```
130+
131+
Prefer placing the single authoritative log in the service layer; remove duplicate logs in route handlers that cover the same event.
132+
133+
## Svelte Patterns
134+
135+
### Async Rollback: Capture State Before `await`
111136

112137
Capture `$state` values before the first `await` for safe rollback. A concurrent update can overwrite the variable while awaiting:
113138

@@ -119,3 +144,23 @@ try {
119144
items = previous;
120145
}
121146
```
147+
148+
### Optimistic Updates
149+
150+
Derive computed fields (flags, labels, etc.) from the canonical data source — don't
151+
re-implement the derivation inline. Divergence causes a "works after reload" bug where
152+
the server state is correct but the client-side update is wrong.
153+
154+
**Diagnostic**: "Not reflected live, but fixed after reload" → suspect the optimistic
155+
update payload, not the reactivity system.
156+
157+
## Code Review
158+
159+
### CodeRabbit Review: Severity Triage
160+
161+
When running `coderabbit review --plain` at a Phase milestone:
162+
163+
- **critical / high**: fix before starting the next Phase
164+
- **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)
165+
166+
Run once per Phase boundary — not on every commit.

.claude/settings.json

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"enabledPlugins": {
3+
"superpowers@superpowers-dev": true
4+
},
5+
"extraKnownMarketplaces": {
6+
"superpowers-dev": {
7+
"source": {
8+
"source": "git",
9+
"url": "https://github.com/obra/superpowers.git"
10+
}
11+
}
12+
}
13+
}

.claude/skills/refactor-plan/instructions.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ Scan the target code in this order (lowest risk first). List every concrete find
5252
- Order phases by risk: isolated mechanical changes first, structural changes last
5353
- State inter-phase dependencies explicitly
5454
- For uncertain changes, add an **investigation sub-step** before the implementation task
55+
- 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
56+
- "Implementation + test + commit" should fit within one task; if not, split further
5557

5658
## When to Skip Tests
5759

.coderabbit.yaml

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
language: 'ja-JP'
2+
tone_instructions: '簡潔に。重要度が high 以上のみ詳述し、low/info は箇条書きでまとめる。'
3+
4+
reviews:
5+
profile: 'assertive'
6+
auto_review:
7+
enabled: true
8+
drafts: false
9+
path_filters:
10+
- '!node_modules/**'
11+
- '!.pnpm-store/**'
12+
- '!pnpm-lock.yaml'
13+
- '!.svelte-kit/**'
14+
- '!.vercel/**'
15+
- '!coverage/**'
16+
path_instructions:
17+
# --- Data layer ---
18+
- path: 'prisma/*'
19+
instructions: |
20+
- CHECK constraints added manually to migration.sql must be documented in prisma/ERD.md.
21+
- path: 'prisma/migrations/**'
22+
instructions: |
23+
- Auto-generated by Prisma. Only flag security vulnerabilities (e.g. privilege escalation, SQL injection) or bugs that could cause data loss or critical errors.
24+
- Skip style, naming, SQL formatting, and documentation issues.
25+
- path: 'src/lib/server/**'
26+
instructions: |
27+
- This directory is server-only. Never import from client-side code or Svelte components.
28+
- DB client must be accessed only via `$lib/server/database`.
29+
# --- Schema / validation ---
30+
- path: 'src/**/zod/**'
31+
instructions: |
32+
- Use `z.number().int().positive()` for Prisma `Int` fields (not `z.number().positive()`, which passes decimals).
33+
- When a Zod constraint mirrors a SQL CHECK, add an inline comment noting both layers and the obligation to keep them in sync.
34+
# --- Domain types ---
35+
- path: 'src/**/*types/**'
36+
instructions: |
37+
- Array types must use a named plural alias: `type Placements = Placement[]`, not `Placement[]` directly in signatures.
38+
- Add TSDoc to every exported type.
39+
- Avoid `any`; check if a `@types/*` package exists first.
40+
# --- Test infrastructure (defined before implementation per TDD) ---
41+
- path: 'src/**/*fixtures/**'
42+
instructions: |
43+
- Use realistic domain values, not abstract placeholders ('t1', 'test1', etc.).
44+
- Shared fixture data must be extracted to a fixture file; avoid duplicating across test cases.
45+
# --- Business logic ---
46+
- path: 'src/**/services/**'
47+
instructions: |
48+
- Service functions must return pure values or null; never Response/json().
49+
- Prisma calls belong here; do not flag direct database access as a violation.
50+
- path: 'src/**/*utils/**'
51+
instructions: |
52+
- Must be pure functions with no side effects.
53+
- Each function must have an adjacent unit test.
54+
# --- State / UI ---
55+
- path: 'src/**/stores/**'
56+
instructions: |
57+
- Stores must use `.svelte.ts` extension, class-based pattern with `$state()`, and export a singleton.
58+
- Legacy stores using `writable()` must be migrated before adding features or extending them.
59+
- path: 'src/routes/**'
60+
instructions: |
61+
- Page routes (+page.server.ts): use redirect() for navigation.
62+
- API routes (+server.ts): use error() — redirect() causes fetch to silently receive the HTML page at the redirect target.
63+
- No direct Prisma calls; delegate to service layer.
64+
- path: 'src/**/*.svelte'
65+
instructions: |
66+
- Use Svelte 5 Runes ($props, $state, $derived, $effect).
67+
- Business logic belongs in utils/, not inside <script> blocks.

.devcontainer/devcontainer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"initializeCommand": "mkdir -p ~/.claude",
2626
//
2727
// Use 'postCreateCommand' to run commands after the container is created.
28-
"postCreateCommand": "npm install -g @anthropic-ai/claude-code && pnpm install",
28+
"postCreateCommand": "npm install -g @anthropic-ai/claude-code && (curl -fsSL https://cli.coderabbit.ai/install.sh | sh || echo 'CodeRabbit CLI installation failed, continuing...') && pnpm install",
2929
//
3030
// Configure tool-specific properties.
3131
"customizations": {

AGENTS.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ Always prefer simplicity over pathological correctness. YAGNI, KISS, DRY. No bac
88

99
**When implementing:**
1010

11-
1. Plan with a phased TODO list before starting (lower risk → higher risk order)
11+
1. Use `/writing-plans` to generate a phased plan (2–5-min tasks, lower risk → higher risk order). Verify each task before starting:
12+
- Which layer? (prisma / server / zod / types / fixtures / services / utils / stores / routes / components) — split if 2+ layers
13+
- Single responsibility: one purpose per task
14+
- Existing util/service/type? Search before creating
15+
- Test name: state it in the task description
1216
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
1317
3. Write tests first, then implement production code, then verify with `pnpm test:unit`
1418
4. Review critically after implementing: flag YAGNI violations, over-abstraction, missing tests

CONTRIBUTING.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,14 @@
6565
- [Visual Studio Code (VS Code)](https://code.visualstudio.com/)
6666
- [Visual Studio Code Dev Containers](https://code.visualstudio.com/docs/remote/containers)
6767

68+
### AI 支援ツール
69+
70+
- [Claude Code](https://docs.anthropic.com/en/docs/claude-code) - AI コーディングアシスタント(VS Code 拡張: `anthropic.claude-code`
71+
- [superpowers plugin](https://github.com/obra/superpowers) - `/writing-plans` スキルによる実装前の詳細計画生成
72+
- [CodeRabbit](https://coderabbit.ai/) - AI コードレビュー
73+
- CodeRabbit CLI (`coderabbit review --plain`) — milestone 区切りでのローカルレビュー
74+
- CodeRabbit CI — PR 作成後の自動レビュー(最終品質ゲート)
75+
6876
### ホスティング、CI・CD関連
6977

7078
- [Vercel](https://vercel.com/)

0 commit comments

Comments
 (0)