From 56394ca824ec3a22a06289f57e48bb6b0610d211 Mon Sep 17 00:00:00 2001 From: Kato Hiroki Date: Fri, 20 Mar 2026 06:19:45 +0000 Subject: [PATCH 1/5] docs: Add plan for plan quality improvement (#3292) Co-Authored-By: Claude Sonnet 4.6 --- .../plan-quality-improvement/plan.md | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 docs/dev-notes/2026-03-20/plan-quality-improvement/plan.md diff --git a/docs/dev-notes/2026-03-20/plan-quality-improvement/plan.md b/docs/dev-notes/2026-03-20/plan-quality-improvement/plan.md new file mode 100644 index 000000000..3b81a5d36 --- /dev/null +++ b/docs/dev-notes/2026-03-20/plan-quality-improvement/plan.md @@ -0,0 +1,166 @@ +# Plan Quality Improvement (#3292) + +## Goal + +初期planの精度を上げ、実装後の大規模refactorサイクル（現状: 3-4回/PR）を削減する。 + +## 背景・現状 + +| 指標 | 現状 | +| --------------------- | ----------- | +| 大規模refactor | 3-4回/PR | +| 中小refactor | ~10回/PR | +| CodeRabbit CIコメント | 10-100件/PR | + +根本原因: TODOリストの粒度が粗いため、アーキテクチャ違反・責務混在・命名の不統一などが +初期planに混入したまま実装が進む。 + +## 改善方針 + +1. **writing-plans** (superpowers plugin) — 2-5分単位の粒度でplanを生成 +2. **Rules強化** — planフェーズで参照できる設計判断チェックリストを追加 +3. **CodeRabbit CLI** — planのmilestoneで都度レビューを差し込む +4. **CodeRabbit CI** — 現状維持（最終ゲートとして機能） + +## レビューチェーン（完成後の姿） + +``` +writing-plans で plan生成 + → 実装（2-5分タスク単位） + → milestone到達時に CodeRabbit CLI レビュー + → PR作成後に CodeRabbit CI（最終ゲート） +``` + +## 計測指標（最初のタスク適用後に評価） + +| 指標 | 現状 | 目標 | +| -------------------------- | -------- | ------------ | +| 大規模refactor回数/PR | 3-4回 | 1回以下 | +| 中小refactor回数/PR | ~10回 | 5回以下 | +| CodeRabbit CIコメント数/PR | 10-100件 | 半減 | +| ccusage | 計測開始 | 許容範囲確認 | + +--- + +## Phase 0: 事前調査 + +- [ ] `/plugin install superpowers@claude-plugins-official` の書き込み先を確認する + - `~/.claude/settings.json`（ユーザーレベル・git非追跡）か + `.claude/settings.json`（プロジェクトレベル・git追跡）かを確認 + - **判断**: ユーザーレベルの場合、複数人チームでは各自インストールが必要になるため + `.claude/skills/` 手動配置を採用する。プロジェクトレベルなら plugin install を採用。 +- [ ] `.coderabbit.yaml` の有無と現在のCodeRabbit CI設定を確認する + +## Phase 1: writing-plans 導入 + +依存: Phase 0 の調査結果が必要 + +- [ ] **1a**: Phase 0の判断に基づき、以下のいずれかを実施する + - Option A (plugin install): `/plugin install superpowers@claude-plugins-official` + - Option B (手動配置): superpowers の writing-plans skill を + `.claude/skills/writing-plans/` にコピー +- [ ] **1b**: `docs/guides/claude-code.md` のスキル一覧表を更新する + - writing-plans を追加（用途: 新機能・追加実装の計画生成） + - `/refactor-plan` との使い分けを明記 + - writing-plans: 実装前の詳細計画生成 + - /refactor-plan: リファクタリング調査チェックリスト +- [ ] **1c**: `AGENTS.md` の実装フロー Step 1 を更新する + - 変更前: `"Plan with a phased TODO list before starting"` + - 変更後: writing-plans を使ってplanを生成する旨と、タスク粒度の基準（2-5分単位）を明記 + +## Phase 2: Rules 強化 + +依存: Phase 1 完了後（writing-plans との統合を前提に記述するため） + +### 2a: AGENTS.md — planレビューチェックリスト追加 + +実装フロー Step 1 に、plan生成後の自己検証チェックリストを追加する: + +```markdown +Plan checklist — verify each task before starting: + +- [ ] Which layer does this touch? (routes / types / services / utils / stores / components) +- [ ] Is this one responsibility? If it touches 2+ layers, split into separate tasks +- [ ] Does a types/util/service already exist for this? (search before creating) +- [ ] What is the test for this task? (name it in the task description) +``` + +### 2b: `coding-style.md` — 命名確認の明示化 + +planフェーズで行う命名確認ルールを追加する: + +- 新ファイル作成前に既存の命名パターンをgrepして確認する +- ルーティング `snake_case` / ディレクトリ `kebab-case` の確認タイミングを明記 + +### 2c: `svelte-components.md` か `prisma-db.md` — 設計判断フロー追加 + +planフェーズで参照できるアーキテクチャ判断フローを追加する: + +```text +新しいロジックを書く前の確認: +純粋関数か? → utils/ へ +DB操作か? → service/ へ +UI表示のみか? → snippet か component か (svelte-components.md の基準を参照) +ルート固有か共有か? → _utils/ か shared utils/、_types/ or shared types/ +``` + +### 2d: `.claude/skills/refactor-plan/instructions.md` — 粒度チェック追加 + +"Phase Design Principles" セクションに、planレビュー時の粒度確認を追加する: + +- 各タスクが単一責務か確認する +- 「実装 + テスト + コミット」が1タスク内に収まる粒度かを確認する + +## Phase 3: CodeRabbit CLI 導入 + +依存: Phase 2 完了後 + +- [ ] **3a**: CodeRabbit CLI をインストールする +- [ ] **3b**: `.coderabbit.yaml` を作成する（存在しない場合）または既存設定を見直す +- [ ] **3c**: writing-plans のmilestoneタスクに CodeRabbit CLI 呼び出しを組み込む規約を定義する + +**milestone タスクのテンプレート:** + +```markdown +## Milestone: Phase N complete + +- [ ] Run `cr review` on changed files +- [ ] Address critical/high severity issues before proceeding to next phase +- [ ] Log CodeRabbit comment count (for metrics) +``` + +**呼び出し頻度の指針:** + +- phaseの区切りで実行（毎コミットごとではない） +- critical/high のみ対応、low/info は次のPhaseで判断 + +> Lefthook pre-commit hookとしての自動化は今回スコープ外。 +> コスト・速度のトレードオフを計測した後に検討する。 + +## Phase 4: CodeRabbit CI ドキュメント整備 + +依存: Phase 3 完了後 + +- [ ] **4a**: `docs/guides/claude-code.md` にレビューチェーンの説明を追加する + - writing-plans → 実装 → CodeRabbit CLI（milestone） → CodeRabbit CI（最終ゲート） + - 各レイヤーの役割と対応コストの目安を記載 +- [ ] **4b**: `.coderabbit.yaml` のルールをPhase 3の設定に合わせて調整する（必要に応じて） + +--- + +## 変更対象ファイル一覧 + +| ファイル | Phase | 変更内容 | +| ---------------------------------------------------------- | ------ | ---------------------------------- | +| `.claude/settings.json` or `.claude/skills/writing-plans/` | 1a | writing-plans 追加 | +| `docs/guides/claude-code.md` | 1b, 4a | スキル一覧・レビューチェーン追加 | +| `AGENTS.md` | 1c, 2a | 実装フロー・planチェックリスト更新 | +| `coding-style.md` | 2b | 命名確認タイミング追加 | +| `svelte-components.md` or `prisma-db.md` | 2c | 設計判断フロー追加 | +| `.claude/skills/refactor-plan/instructions.md` | 2d | 粒度チェック追加 | +| `.coderabbit.yaml` | 3b, 4b | 新規作成または見直し | + +## 参考 + +- [superpowers writing-plans skill](https://github.com/obra/superpowers) +- Issue: #3292 From ac2b8177d57b4d8a86de8fab8f30fdee579582d1 Mon Sep 17 00:00:00 2001 From: Kato Hiroki Date: Fri, 20 Mar 2026 08:20:00 +0000 Subject: [PATCH 2/5] feat: Introduce writing-plans plugin and CodeRabbit CLI for plan quality - Add superpowers plugin (.claude/settings.json) for /writing-plans skill - Add .coderabbit.yaml with path_instructions for all project layers - Update devcontainer.json: install CodeRabbit CLI in postCreateCommand - AGENTS.md: revamp Step 1 with /writing-plans, task granularity, and plan checklist - coding-style.md: add Pre-Implementation Layer Check table and naming timing rule - refactor-plan/instructions.md: add single-responsibility and granularity checks - claude-code.md: add /writing-plans skill, review chain, and milestone template - CONTRIBUTING.md: add AI tools section (Claude Code + CodeRabbit) - Update plan.md: mark all phases complete, add design notes and lessons learned Closes #3292 Co-Authored-By: Claude Sonnet 4.6 --- .claude/rules/coding-style.md | 21 +++ .claude/settings.json | 13 ++ .claude/skills/refactor-plan/instructions.md | 2 + .coderabbit.yaml | 65 +++++++ .devcontainer/devcontainer.json | 2 +- AGENTS.md | 6 +- CONTRIBUTING.md | 8 + .../plan-quality-improvement/plan.md | 173 ++++++++++-------- docs/guides/claude-code.md | 44 ++++- 9 files changed, 249 insertions(+), 85 deletions(-) create mode 100644 .claude/settings.json create mode 100644 .coderabbit.yaml diff --git a/.claude/rules/coding-style.md b/.claude/rules/coding-style.md index f21ac499c..a7e6544ce 100644 --- a/.claude/rules/coding-style.md +++ b/.claude/rules/coding-style.md @@ -1,5 +1,22 @@ # Coding Style +## Pre-Implementation Layer Check + +Before writing new logic, decide which layer it belongs to. Run this check at plan time, 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/` | `.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/` | + ## Naming - **Abbreviations**: avoid non-standard abbreviations (`res` → `response`, `btn` → `button`). When in doubt, spell it out. @@ -7,6 +24,9 @@ - **`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: + - Route/API files: `snake_case` (e.g., `user_profile.ts`) + - Directories: `kebab-case` (e.g., `user-profile/`) ## TSDoc @@ -119,3 +139,4 @@ try { items = previous; } ``` + 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..67fa17242 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 — verify it touches exactly one layer (prisma / server / zod / types / fixtures / services / utils / stores / routes / components) +- "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..d45fe79d6 --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,65 @@ +language: 'ja-JP' +tone_instructions: '簡潔に。重要度が high 以上のみ詳述し、low/info は箇条書きでまとめる。' + +reviews: + profile: 'assertive' + auto_review: + enabled: true + drafts: false + path_filters: + - '!node_modules/**' + - '!prisma/migrations/**' + - '!.pnpm-store/**' + - '!pnpm-lock.yaml' + - '!.svelte-kit/**' + - '!.vercel/**' + - '!coverage/**' + path_instructions: + # --- Data layer --- + - path: 'prisma/**' + instructions: | + - Migration files must not be edited after they are applied. + - CHECK constraints added manually to migration.sql must be documented in prisma/ERD.md. + - 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(). + - No direct Prisma calls — delegate to service layer. + - 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