| name | full-code-review |
| description | PR 提出前にコードオーナー視点でフルレビューする。WHY の理解 → 設計 → AST/基盤型 → 実装(複数 Agent の順次レビュー)の順に、各フェーズでユーザーと対話しながら指摘を修正し、品質に問題がなくなるまで繰り返す。「/full-code-review」「フルレビュー」「レビューして」などの依頼時に使用。rsvelte(公式 Svelte コンパイラの Rust ポート)専用。 |
| allowed-tools | Read, Grep, Glob, Bash, Edit, Write, Agent, Skill, WebSearch, WebFetch |
| effort | max |
Full Code Review — コードオーナー視点のフルレビュー(rsvelte 版)
PR を提出する前に、リポジトリ全体のコードオーナー視点でフルレビューするスキル。
各フェーズでレビュー結果をユーザーに提示し、対話を通じて指摘事項を修正し、品質に問題がなくなるまで繰り返す。
このリポジトリの前提
rsvelte は 公式 Svelte コンパイラ(svelte/packages/svelte/src/compiler/)の Rust ポートである。
レビュー時は常にこれら 3 つのプロジェクトゴールを意識すること:
- 100% テスト互換性 — 公式 Svelte のテストスイートを完全にパスすること
- 100x パフォーマンス — 公式 JS コンパイラの 100 倍速を目指すこと
- OXC エコシステム統合 — OXC のパターン(arena allocation、typed AST、
Atom<'a>)と整合すること
このため、一般的な Web サービスのレビュー(DB スキーマ、RLS、フロントエンド UX)とは観点が大きく異なる。
最重要観点は「公式 Svelte 実装との一致」と「ホットパスのパフォーマンス」 である。
前提条件
- レビュー対象の PR があること(ブランチ上の変更 or PR 番号)
- ユーザーはコードオーナーであり、最終的な判断権限を持つ
- 修正はユーザーの承認を得てから行う
実行手順
Step 0: 変更差分の把握
まずレビュー対象の全体像を把握する。
ベースブランチの決定(優先順位):
- ユーザーから明示的に比較ブランチが指定されている場合 → 指定されたブランチをそのまま使う
- PR 番号が指定されている場合 →
gh pr view <PR番号> --json baseRefName でマージ先ブランチを取得し、origin/<baseRefName> をベースブランチとする
- 上記以外(デフォルト) →
origin/main、origin/master、origin/develop のうち存在するものから HEAD との差分が小さいもの を採用する(rsvelte は通常 origin/main がベース)
注: ローカルの main は古い可能性があるため、原則として origin/main を使う。事前に git fetch origin を済ませておくことが望ましい。
if [ -n "${USER_SPECIFIED_BASE:-}" ]; then
BASE_BRANCH="${USER_SPECIFIED_BASE}"
elif [ -n "${PR_NUMBER:-}" ]; then
BASE_BRANCH="origin/$(gh pr view "${PR_NUMBER}" --json baseRefName -q '.baseRefName')"
else
candidates=()
for ref in origin/main origin/master origin/develop; do
git rev-parse --verify --quiet "$ref" >/dev/null 2>&1 && candidates+=("$ref")
done
if [ ${#candidates[@]} -eq 0 ]; then
echo "Error: no candidate base branch (origin/main, origin/master, origin/develop). Run 'git fetch origin' or specify the base branch." >&2
exit 1
elif [ ${#candidates[@]} -eq 1 ]; then
BASE_BRANCH="${candidates[0]}"
else
best="${candidates[0]}"
best_lines=$(git diff --numstat "${best}...HEAD" | awk '{s+=$1+$2} END {print s+0}')
for ref in "${candidates[@]:1}"; do
lines=$(git diff --numstat "${ref}...HEAD" | awk '{s+=$1+$2} END {print s+0}')
if [ "$lines" -lt "$best_lines" ]; then best="$ref"; best_lines="$lines"; fi
done
BASE_BRANCH="$best"
fi
fi
git diff --name-only ${BASE_BRANCH}...HEAD
git diff --stat ${BASE_BRANCH}...HEAD
git log --oneline ${BASE_BRANCH}...HEAD
決定したベースブランチをユーザーに必ず報告する(例: 「ベースブランチ: origin/main を採用」)。
以降のフェーズでも git diff を使う際はすべてこのベースブランチを基準にする。
変更ファイルを以下のカテゴリに分類する:
- AST/基盤型:
src/ast/ 配下(コンパイラ全体の基盤データ構造)
- Phase 1 Parse:
src/compiler/phases/1_parse/ 配下
- Phase 2 Analyze:
src/compiler/phases/2_analyze/ 配下
- Phase 3 Transform:
src/compiler/phases/3_transform/ 配下(Client / Server / CSS)
- Error:
src/error/ 配下(エラー・警告メッセージ)
- NAPI / バイナリ:
src/lib.rs、src/napi*、src/bin/ 配下
- テスト:
tests/、benches/、examples/ 配下
- インフラ:
.github/、scripts/、build.rs、Cargo.toml、Cargo.lock、Dockerfile 系
- ドキュメント:
*.md、docs/ 配下
- サブモジュール / fixture:
svelte/、vite-plugin-svelte/、language-tools/、fixtures/(レビュー対象外)
サブモジュールと fixture の変更は レビュー対象外 として扱う(外部由来 / 自動生成のため)。
ただし、サブモジュールのバージョンが変わっている場合はその旨を Phase 1 で報告する。
分類結果と変更規模をユーザーに報告し、PR 番号があれば PR の説明欄も取得して提示する。
Phase 1: WHY の理解と妥当性の判断
目的: この PR がなぜ必要なのか、技術的・戦略的背景を理解する。コードベースだけでは判断できない文脈(解決する公式 Svelte の互換性問題、改善するパフォーマンスのボトルネック、対応する OXC 統合上の必要性)を明確にし、この PR の存在意義が妥当かを判断する。
rsvelte でよくある PR の動機:
| 動機 | 兆候 | 判断観点 |
|---|
| 公式 Svelte の新バージョンへの追従 | サブモジュール更新 + AST/transform 修正 | 公式実装の差分を正確に追えているか |
| テスト互換性向上(落ちていたテストを通す) | fixture 比較で差分があった箇所の修正 | 修正で他のテストが落ちていないか |
| パフォーマンス改善 | hot path の clone / 割り当て削減、データ構造変更 | ベンチマークの数値根拠があるか |
| バグ修正 | 特定 fixture や issue の再現テスト | 根本原因 vs 対症療法か |
| リファクタリング / 簡素化 | 大規模な構造変更、行数減少 | テストが pass していて意味のある簡素化か |
| 新機能 / 新 Svelte 構文の対応 | 新しい AST ノード、新しい構文パーサー | 公式実装をミラーしているか |
| 開発者体験 / インフラ | scripts/、CI、Dockerfile 系の変更 | 本来の目的に沿っているか |
進め方:
-
入手可能な情報をすべて集める
- PR がある場合:
gh pr view で PR のタイトル・説明欄・コメント・リンクされた Issue を取得する
- コミットメッセージから意図を読み取る
- 変更内容から「何をしようとしているか」を把握する
- 変更の性質(互換性向上 / 新機能 / バグ修正 / パフォーマンス / リファクタリング / インフラ)を分類する
- サブモジュール(
svelte/)が更新されている場合、新旧バージョンを記録し、関連 changelog を確認する
-
ユーザーに WHY を確認する
上記の情報だけでは WHY が十分に理解できない場合、ユーザーに質問する。
確認したい点:
- 背景: なぜ今この変更が必要なのか?(落ちていたテスト / 報告された不整合 / パフォーマンスボトルネック / 新 Svelte バージョン対応)
- 影響範囲: テスト互換性 / パフォーマンス / 公開 API(NAPI、CLI)のどれに影響するか
- リファレンス: 対応する公式 Svelte の commit / PR / issue があるか
PR の説明欄や commit メッセージから WHY が十分に読み取れる場合は、質問せずに理解した内容を提示して確認を求める。
-
妥当性を判断する
ユーザーの回答(または PR から読み取った情報)を踏まえ、以下を評価する:
- 公式実装との整合性: 公式 Svelte の対応する変更を正しくミラーしているか(or なぜ意図的に逸脱するか)
- テスト戦略: この変更によって増減するテスト pass 数は?(
pnpm run compatibility-report で確認可能)
- パフォーマンスへの影響: hot path に影響する変更か?影響するなら計測されているか?
- 過剰実装ではないか: 必要最小限の修正か、不要なリファクタリングが混入していないか
ユーザーへの報告フォーマット:
PR から WHY が十分に読み取れない場合:
## Phase 1: WHY の理解と妥当性
### コードと PR から読み取った概要
[変更内容の要約、PR 説明欄から得られた情報、サブモジュールバージョン変化があれば記載]
### 確認したいこと
WHY をより深く理解するために教えてください:
1. なぜ今この変更が必要なのですか?(落ちていたテスト / 報告された不整合 / パフォーマンス改善 / Svelte バージョン追従?)
2. 公式 Svelte の対応する変更(PR / commit)はありますか?
3. テスト互換性 / パフォーマンスへの想定インパクトはどの程度ですか?
ユーザーの回答を受け取ったら(または PR から十分に読み取れた場合):
### WHY の理解
- **背景**: [何がきっかけか]
- **解決する問題**: [テスト互換性 / バグ / パフォーマンス / 新機能 等]
- **公式実装の対応箇所**: [svelte/packages/svelte/src/compiler/<path> など、または「該当なし(rsvelte 固有の改善)」]
- **想定インパクト**: テスト pass 数 X→Y、ベンチ X→Y、API 変更の有無
- **この PR による解決**: [どう解決するか]
### 妥当性の判断
- **判定**: 妥当 / 懸念あり / 要議論
- **理由**: [具体的な理由]
### 懸念事項(あれば)
1. [懸念1]
妥当と判断した場合: そのまま Phase 2 へ進む(ユーザーの明示的な承認は不要)。
懸念がある場合: 懸念事項を提示し、ユーザーの回答を待ってから判断する。
Phase 2: 全体設計のレビュー
目的: 変更全体の設計を理解し、その妥当性を判断する。rsvelte ではこのフェーズで「公式 Svelte 実装との整合性」「コンパイラパイプライン全体での一貫性」「パフォーマンスへの構造的影響」を見る。
確認事項:
-
公式 Svelte 実装との整合性(最重要)
- 対応する公式実装(
svelte/packages/svelte/src/compiler/)と同じアルゴリズム・同じ責務分割になっているか
- ディレクトリ構造・ファイル名・関数名が公式実装をミラーしているか
- 公式実装にない独自抽象を勝手に追加していないか(必要なら理由を確認)
- 公式実装の最新版(サブモジュールが指す commit)と差分が広がっていないか
-
コンパイラパイプライン全体の一貫性
1_parse → 2_analyze → 3_transform のデータフローが論理的に一貫しているか
- フェーズ間で AST を直接渡しているか(中間でシリアライズ → 再パースしていないか)
- 同じ概念が複数のフェーズで重複定義されていないか
- エラー型と警告型が
src/error/ に集約されているか
-
AST / 基盤型への影響
src/ast/ の型に影響する変更が含まれている場合、Phase 3 で集中レビューする旨を予告する
- AST 変更が下流(analyze、transform、tests)に正しく伝播しているか
- 新しい AST バリアントを追加した場合、すべての visitor / matcher / transform で扱われているか
-
パフォーマンスへの構造的影響
- hot path(パース、ノードトラバーサル、コード生成)に新しい割り当てや clone が増えていないか
serde_json::Value を使った動的データを増やしていないか(OXC 流の typed AST から逆行していないか)
- ライフタイム / 借用パターンが
'a を保持できているか、不必要に String / CompactString を返していないか
Vec、HashMap、Box を新たに hot path に導入する場合、SmallVec / FxHashMap / arena 割り当てを検討したか
- 計測が必要な変更なら、
./scripts/bench/bench.sh --quick 等で計測されているか
-
設計の複雑性
- 不必要に複雑な抽象化(trait 階層、ジェネリクス、マクロ)を導入していないか
- YAGNI 原則: 仮説的な将来要件のために汎用化されていないか
- "高速なプログラムはシンプルなデータ構造に対してシンプルなコードが書かれている" という原則(
/perf スキル参照)に沿っているか
-
NAPI / 公開境界
- NAPI 経由の外部公開 API に破壊的変更が含まれていないか
- 含まれる場合、公式 Svelte コンパイラの API 互換性を保てているか
- vite-plugin-svelte などの consumer から見える挙動が変わっていないか
ユーザーへの報告フォーマット:
## Phase 2: 全体設計レビュー
### 設計の概要
[変更の設計を図示・説明。どのフェーズの何をどう変えたか]
### 評価
| 観点 | 判定 | コメント |
|------|------|----------|
| 公式実装との整合性 | OK / 要改善 | [差分があるなら箇所と理由] |
| パイプライン一貫性 | OK / 要改善 | [詳細] |
| AST / 基盤型への影響 | OK / Phase 3 で詳細レビュー / N/A | [詳細] |
| パフォーマンス(構造) | OK / 要改善 | [hot path への影響] |
| 複雑性 | OK / 要改善 | [詳細] |
| NAPI / 公開境界 | OK / 要改善 / N/A | [破壊的変更の有無] |
### 指摘事項(あれば)
1. **[Critical/Major/Minor]** [指摘内容] → [改善案]
→ 指摘事項を対応しますか?それとも Phase 3 に進みますか?
ユーザーの承認を待ってから Phase 3 へ進む。指摘事項への対応が必要な場合はここで修正を行う。
Phase 3: AST・基盤型レビュー
目的: コンパイラ全体の基盤となるデータ構造を徹底的にレビューする。一般的な Web プロジェクトの「DB スキーマレビュー」に相当するフェーズ。AST 型は Parse / Analyze / Transform / Error すべてが依存する基盤であり、後から変更するコストが最も高いため、最も慎重にレビューする。
対象: src/ast/ 配下(変更がない場合はスキップ)
主なファイル:
src/ast/mod.rs — AST 全体のルート / 公開 API
src/ast/template.rs — Svelte テンプレート AST(Element、Text、Block 等)
src/ast/js.rs — JS 式 / 文の AST
src/ast/typed_expr.rs — 型付き式 AST(JsNode 列挙)
src/ast/css.rs — CSS AST
src/ast/span.rs — ソース位置情報(u32 ベース)
src/ast/arena.rs — アリーナ割り当て関連
確認事項:
-
公式 Svelte AST との一致
- ノードの kind 文字列(
"Element"、"IfBlock" など)は公式 AST と完全に一致しているか
- フィールド名・必須/オプショナルが公式 AST と一致しているか
- 公式 AST にない独自フィールドを追加していないか(追加する場合は明確な理由が必要)
- 公式 Svelte が新しいノード kind を導入している場合、対応する Rust enum バリアントが追加されているか
-
メモリレイアウト
- 位置情報は
u32 で保持しているか(usize を使っていないか — メモリ膨張の原因)
- 文字列は
CompactString / &'a str / Atom<'a> のどれを使うか適切か
- ソースから借用できるなら
&'a str(最速)
- 新規生成で頻出なら
Atom<'a> / 共有
- 短く・所有が必要なら
CompactString
- 大きく・所有が必要なら
String
Box<T> / Vec<T> は arena 化(bumpalo::boxed::Box<'a, T> / bumpalo::collections::Vec<'a, T>)の余地がないか
- enum バリアントのサイズが極端にばらついていないか(一部だけ巨大なら
Box で indirection)
-
ライフタイム設計
- ライフタイムパラメータ
'a がソース文字列 / arena から一貫して引かれているか
'static を不必要に強制していないか
- 新しい型を追加する際、既存の
'a 設計と整合しているか
-
enum / variant の網羅性
- 新バリアント追加時、
match の網羅性で(#[non_exhaustive] でない限り)コンパイラエラーが出るか確認
- すべての visitor(
walk_*、visit_*)に対応する分岐が追加されているか
- serializer(公式 AST との JSON 互換出力がある場合)に対応する分岐が追加されているか
-
命名規則
- 構造体名・フィールド名・enum バリアントは公式 Svelte(JS 側)の命名と整合しているか
- 短縮名を使っていないか(
exp ではなく expression、attr ではなく attribute)
- Rust の慣習(
snake_case フィールド、PascalCase 型)に従っているか
-
不変条件 / バリデーション
- 構築時に守るべき不変条件があれば、コンストラクタ or
Result で保証されているか
Option<T> の意味(必須なのに省略 / 真にオプショナル)が明確か
-
下流への影響(特に重要)
AST 変更があった場合、以下すべてに対応する変更が含まれているか確認する。ひとつでも欠けると compile エラーやテスト失敗の原因になる:
-
設計判断の妥当性
AST 変更に設計の分岐点が含まれる場合、以下の考慮ポイントに基づいて評価する。
判断を自動で下すのではなく、ユーザーに質問して設計の見落としを防ぐ。
8-1: 文字列の所有形態の選択
検出トリガー: 新規 AST フィールドが文字列型になっている場合
考慮ポイント:
- その文字列はソース文字列に存在するか?(YES →
&'a str で借用)
- 短い識別子のような文字列か?(YES →
Atom<'a> で interning 候補)
- 短いがソース由来でない(生成された文字列)か?(YES →
CompactString)
- 大きく、書き換えが頻繁な文字列か?(YES →
String)
質問テンプレート:
- 「この文字列はソース文字列のスライスとして借用できますか?」
- 「同じ文字列が大量に出現しますか?(→ Atom 化候補)」
- 「ライフタイム
'a を引き連れることでホットパスから割り当てを除けますか?」
8-2: 新規 enum バリアント vs 既存バリアント拡張
検出トリガー: 既存 enum へのバリアント追加 or 新規 enum 作成
考慮ポイント:
- 公式 Svelte の対応する型はどう設計されているか(最優先で参照)
- そのバリアントが既存バリアントと共通フィールドを多く持つなら、共通構造体 + バリアントタグの形にできるか
- 新バリアントが極端に大きいなら
Box で indirection し、enum サイズを抑えるべきか
質問テンプレート:
- 「公式 Svelte の対応箇所はどう表現していますか?同じ構造にすべきです」
- 「このバリアントのサイズは他のバリアントと比べて何倍ですか?極端なら
Box を検討」
8-3: ライフタイム導入 vs 所有
検出トリガー: 新規 AST 型に String / Vec<T> / Box<T> を導入している
考慮ポイント:
- ソース文字列または arena から借用できるなら、ライフタイム
'a を導入する方が速い
- 既存の AST が
'a を使っているなら、新規型もそれに合わせるべき
- 一方で、すでに
'static が広範に前提されている部分なら、無理に 'a 化しないほうが良い
質問テンプレート:
- 「周囲の AST 型は
'a を引いていますか?整合性が取れていますか?」
- 「この型の所有が必要な理由は何ですか?借用で十分ではないですか?」
ユーザーへの報告フォーマット:
## Phase 3: AST・基盤型レビュー
### 変更されたファイル
- `src/ast/<file>.rs`: [変更の要約]
### 型別レビュー
#### 型名: `XxxNode`(または `Element` 等)
| 観点 | 判定 | コメント |
|------|------|----------|
| 公式 AST との一致 | OK / 要改善 / N/A | [公式 AST との差分の有無と妥当性] |
| メモリレイアウト | OK / 要改善 | [u32 / 文字列型 / Box 化の検討] |
| ライフタイム | OK / 要改善 | [`'a` 設計が周囲と整合しているか] |
| 網羅性 | OK / 要改善 | [`match` / visitor の網羅] |
| 命名 | OK / 要改善 | [公式・Rust 慣習との整合] |
| 下流への伝播 | OK / 要対応 | [Parse/Analyze/Transform/Error/tests のどれが未対応か] |
| 設計判断 | OK / 要確認 / N/A | [検出された設計分岐点と評価] |
### 指摘事項
1. **[Critical/Major/Minor]** [ファイル:行番号] [指摘内容] → [改善案]
→ 指摘事項を対応しますか?それとも Phase 4 に進みますか?
ユーザーの承認を待ってから Phase 4 へ進む。
Phase 4: 実装レビュー(チャンク分割 → 順次 Agent → 対話 → 再レビューの反復)
目的: 実装品質を複数の観点から検証し、指摘事項を修正し、品質に問題がなくなるまで繰り返す。
対象: Parse / Analyze / Transform / Error / NAPI / Tests / Infra / Docs / その他の変更ファイル(変更がないカテゴリの Agent はスキップ)。
AST(src/ast/)は Phase 3 で対応済みのためスキップ。サブモジュールと fixture は対象外。
前提となる考え方(重要):
PR が大規模になると、PR 全体を一括でレビューする AI ツール(CodeRabbit / Codex / simplify / security-review など)は、入力が大きすぎて以下の問題が起きやすい:
- 検知漏れ(重要な指摘を見落とす)
- 指摘の薄さ(表層的なコメントしか出ない)
- 出力の打ち切り(途中で止まる)
そこで Phase 4 では、変更を チャンク(小さな単位) に分割し、ツールごとに チャンク単位で繰り返し実行 する。各チャンクは「指摘がゼロになるまで」修正と再レビューのループを回す。
チャンク分割は LLM の判断ブレを排除するため、スクリプト .claude/skills/full-code-review/scripts/plan-review-chunks.mjs で機械的に決定する(カテゴリ単位 + 500 行 / 10 ファイルの上限で分割、AST は Phase 3 で対応済みのため除外、submodule/fixture は対象外)。
Step 4-0: チャンク計画(スクリプトで機械的に分割)
スクリプトを実行してチャンク計画を取得する。Step 0 で決定した ${BASE_BRANCH} を必ず引数で渡す。
node .claude/skills/full-code-review/scripts/plan-review-chunks.mjs --format=md ${BASE_BRANCH}
node .claude/skills/full-code-review/scripts/plan-review-chunks.mjs ${BASE_BRANCH} > /tmp/review-chunks.json
引数を省略してスクリプト単体で実行した場合、スクリプトは origin/main → origin/master → origin/develop の順で存在するものから HEAD との差分が小さいものを自動採用する(Step 0 と同じロジック)。
スクリプトのロジック:
git diff --numstat ${BASE_BRANCH}...HEAD から変更ファイルと行数を取得
- ファイルをカテゴリに分類(ast / submodule / parse / analyze / transform / error / napi / tests / infra / docs / other_src / other)
- ast は Phase 3 で対応済みのためスキップ
- submodule / fixture は対象外としてスキップ
- 各カテゴリ内でパス順に並べ、ディレクトリ境界 + 上限値(500 行 / 10 ファイル)で貪欲法分割
- カテゴリ順序: parse → analyze → transform → error → napi → tests → other_src → infra → docs → other(コンパイラパイプライン順)
ユーザーへの報告:
スクリプトが出力した Markdown テーブルをそのまま提示し、以下を確認する:
## Phase 4 チャンク計画
[スクリプト出力の Markdown テーブル]
→ このチャンク分割で進めてよいですか?分割方針に変更があれば(特定ファイルを別チャンクに、または同一チャンクにまとめるなど)お知らせください。
注意点:
- 単一ファイルが極端に大きい(数千行)チャンクは、ツールが見落とす可能性が依然として残る。その旨をユーザーに伝え、必要なら手動でファイル内の論理単位(関数・モジュール・テスト suite)に分けるか相談する。
- ユーザーの指示で分割を変更する場合は、変更後の最終チャンク一覧を再提示してから Step 4-1 に進む。
Step 4-1: チャンク × ツールのレビュー実行
各チャンクに対して、以下の Agent を チャンク内で 1 つずつ順番に 起動する。1 つのチャンク × 1 つの Agent の結果が出たら、Step 4-2 で対話・修正、Step 4-3 で再レビューを完了させてから次の Agent に進む。
全体ループ構造:
for chunk in /tmp/review-chunks.json.chunks:
for agent in [reference, compiler, perf, tests, security, simplify, coderabbit, codex]:
if agent 対象外(チャンクに該当ファイルが含まれない、CLI 未インストール 等): continue
loop:
agent をチャンク範囲に限定して実行
指摘ゼロ ⇒ break
Step 4-2 で対話・修正
⇒ 同じチャンク・同じ agent を再実行(ループ先頭へ)
次の agent へ
次のチャンクへ
Agent 実行順序(チャンク内):
rsvelte 固有の優先順位:
- 公式実装整合性レビュー(最優先 — 100% テスト互換性に直結)
- コンパイラ実装レビュー(Rust / コンパイラ固有のチェック)
- パフォーマンスレビュー(100x 目標に直結)
- テストカバレッジレビュー(Svelte fixture との対応)
- セキュリティ / メモリ安全性レビュー(
security-review ビルトインスキル)
- コード簡素化レビュー(
simplify ビルトインスキル)
- CodeRabbit レビュー(インストール済みの場合のみ)
- Codex レビュー(インストール済みの場合のみ)
チャンクのカテゴリと Agent 適用:
| カテゴリ | Agent 1 公式整合性 | Agent 2 コンパイラ実装 | Agent 3 パフォーマンス | Agent 4 テスト | Agent 5 セキュリティ | Agent 6 簡素化 | Agent 7 CodeRabbit | Agent 8 Codex |
|---|
| parse / analyze / transform / error | ◯ | ◯ | ◯ | ◯ | ◯ | ◯ | ◯ | ◯ |
| napi | △(公式に対応する API があれば) | ◯ | ◯ | ◯ | ◯ | ◯ | ◯ | ◯ |
| tests | △(公式テストの fixture と整合) | ◯ | △ | ◯ | △ | ◯ | ◯ | ◯ |
| infra | × | △(shell / Cargo の品質) | × | △ | ◯ | ◯ | ◯ | ◯ |
| docs | × | × | × | × | × | △(誤情報チェック) | △ | △ |
| other_src / other | △ | ◯ | △ | △ | △ | ◯ | ◯ | ◯ |
◯ = 必ず実行 / △ = 変更内容次第で実行 / × = スキップ
重要: 各 Agent への指示には 必ず「対象チャンクのファイルパス一覧」を明示 し、チャンク外のファイルはレビュー対象から除外させる。これによりツール側のスコープを小さく保ち、検知漏れを防ぐ。
ファイル一覧の埋め込み方:
- スクリプト出力 JSON の
chunks[i].files 配列をそのまま列挙する
- Agent 指示の「対象ファイル」セクションに、1 行 1 ファイルで貼り付ける
Agent 1: 公式実装整合性レビュー(rsvelte 最重要)
公式 Svelte コンパイラ(svelte/packages/svelte/src/compiler/)との整合性レビューを、以下の対象ファイルに絞って実行してください。
## 対象チャンク
[チャンク名]
## 対象ファイル(このリストに含まれるファイルのみレビュー対象。それ以外は無視)
- <file1>
- <file2>
- ...
## ベースブランチ
${BASE_BRANCH}
## 手順
1. 対象ファイルが Phase 1 Parse / Phase 2 Analyze / Phase 3 Transform / Error のどれに属するかを判別する
2. 公式 Svelte の対応するファイルを特定する(rsvelte は公式のディレクトリ構造をミラーしている)
- `src/compiler/phases/1_parse/<x>.rs` ↔ `svelte/packages/svelte/src/compiler/phases/1-parse/<x>.js`
- `src/compiler/phases/2_analyze/<x>.rs` ↔ `svelte/packages/svelte/src/compiler/phases/2-analyze/<x>.js`
- `src/compiler/phases/3_transform/<client|server|css>/<x>.rs` ↔ `svelte/packages/svelte/src/compiler/phases/3-transform/<x>.js`
- `src/error/*.rs` ↔ `svelte/packages/svelte/src/compiler/{errors,warnings}.js`
3. `git diff <ベースブランチ>...HEAD -- <対象ファイル一覧>` で対象ファイルの変更差分を確認
4. 公式実装と Rust 実装を比較する
## 重視する観点
- **アルゴリズムの一致**: 同じロジック・同じ判定順序・同じエッジケース処理になっているか
- **出力の一致**: 同じ入力に対して同じコード(バイト列)を生成するか
- **エラーメッセージ**: エラーコード・メッセージ文言が公式と一致しているか
- **命名の一致**: 関数名・引数名・フィールド名が公式の翻字(snake_case)になっているか
- **意図的な逸脱**: 公式と異なる箇所があれば、その理由がコメントや commit message に明記されているか
- **公式の最新変更**: サブモジュールが指す最新版の公式実装で、追従していない変更がないか
## 出力フォーマット
ファイルごとに指摘を報告。各指摘に **[Critical/Major/Minor]** [ファイル:行番号] [公式実装の参照箇所] [指摘内容] → [改善案] を含める。
- **Critical**: 公式と挙動が異なり、テストに影響する
- **Major**: 公式と構造が異なり、メンテナンス性を損なう(将来の追従コストを増やす)
- **Minor**: 命名・コメントの不一致など軽微なもの
指摘がない場合は「指摘なし」と報告。チャンク外のファイルへの指摘は出さないこと。コードの修正は行わず、レビュー結果の報告のみ。
Agent 2: コンパイラ実装レビュー
Rust コンパイラ実装のコードレビューを、以下の対象ファイルに絞って実行してください。
## 対象チャンク
[チャンク名]
## 対象ファイル
- <file1>
- <file2>
- ...
## 手順
1. `.claude/skills/full-code-review/implementation-checklist.md` を読み込み、レビュー観点を把握する
2. `git diff <ベースブランチ>...HEAD -- <対象ファイル一覧>` で対象ファイルの変更差分を確認する
3. 対象ファイルを読み込み、レビューする
## 特に重視する観点
- **メモリ安全性**: production コードパスでの `unwrap()` / `expect()` / `panic!()` / `todo!()` / `unimplemented!()` の使用
- **エラーハンドリング**: `Result` 型での伝播、`?` 演算子の活用、独自 error 型の整合性
- **ライフタイム**: 不要な `'static` 強制、不要なクローン、`'a` 借用の活用
- **イディオム**: `match` の網羅性、`Option`/`Result` の組み合わせ、イテレータの活用
- **clippy**: `cargo clippy --all-targets --all-features -- -D warnings` で警告が出ないか
- **DRY**: パース / トラバーサル / 出力の重複ロジックがないか
- **命名**: 公式 Svelte と整合した snake_case、構造体・enum・関数名の慣習
- **コメント**: WHY を説明しているか、WHAT の冗長な説明になっていないか
## 出力フォーマット
ファイルごとに指摘を報告。各指摘に **[Critical/Major/Minor]** [ファイル:行番号] [指摘内容] → [改善案] を含める。
指摘がない場合は「指摘なし」と報告。チャンク外のファイルへの指摘は出さないこと。コードの修正は行わず、レビュー結果の報告のみ。
Agent 3: パフォーマンスレビュー
パフォーマンス観点のコードレビューを、以下の対象ファイルに絞って実行してください。
## 対象チャンク
[チャンク名]
## 対象ファイル
- <file1>
- <file2>
- ...
## 手順
1. `.claude/skills/perf/SKILL.md` を読み込み、rsvelte のパフォーマンス目標と最適化ロードマップを把握する
2. `.claude/skills/full-code-review/implementation-checklist.md` のパフォーマンスセクションを読み込む
3. `git diff <ベースブランチ>...HEAD -- <対象ファイル一覧>` で対象ファイルの変更差分を確認する
4. 対象ファイルを読み込み、レビューする
## 特に重視する観点(rsvelte 100x 目標)
- **hot path での割り当て**: ループ内 / 再帰内での `String::new`、`Vec::new`、`Box::new`、`format!()` の発生
- **不要な clone**: AST ノードや大きな構造体に対する `.clone()`(参照や `Cow<'a, T>` で済まないか)
- **`serde_json::Value` の使用**: typed AST から逆行していないか
- **アロケータ選択**: arena (`bumpalo`) を使うべき箇所で std アロケータを使っていないか
- **文字列**: ソースから借用できるのに `String` / `CompactString` を新規割り当てしていないか
- **コレクション**: 小さなサイズが想定される箇所に `SmallVec` ではなく `Vec` を使っていないか / `HashMap` ではなく `FxHashMap` の検討
- **`format!()`**: hot path の文字列構築で `format!` ではなく `write!` を使うべき箇所
- **early return**: 早期 return できる条件が遅延されていないか
- **重複計算**: ループ外で計算可能な値がループ内で計算されていないか
## 出力フォーマット
ファイルごとに指摘を報告。各指摘に **[Critical/Major/Minor]** [ファイル:行番号] [hot path かどうか] [指摘内容] → [改善案 / 期待される改善幅] を含める。
- **Critical**: 確実に hot path で実測可能な悪化を引き起こす
- **Major**: hot path 上だが影響は実測次第
- **Minor**: cold path / 影響は限定的
指摘がない場合は「指摘なし」と報告。チャンク外のファイルへの指摘は出さないこと。コードの修正は行わず、レビュー結果の報告のみ。
Agent 4: テストカバレッジレビュー
テストカバレッジ観点のレビューを、以下の対象ファイルに絞って実行してください。
## 対象チャンク
[チャンク名]
## 対象ファイル
- <file1>
- <file2>
- ...
## 手順
1. `.claude/skills/full-code-review/implementation-checklist.md` のテストカバレッジセクションを読み込む
2. `git diff <ベースブランチ>...HEAD -- <対象ファイル一覧>` で変更差分を確認する
3. 対象ファイルを読み込み、対応するテストファイルや fixture が存在するか確認する
4. このリポジトリのテスト構造(`tests/` 配下、および公式 Svelte の fixture を使った互換性テスト)を考慮する
## 特に重視する観点
- **公式 Svelte fixture との互換性**: 変更がコード生成(client.js / server.js / css)に影響する場合、対応する fixture を `pnpm run compatibility-report` でカバーしているか
- **新規ロジックのテスト**: 新しい関数・新しいエッジケース処理が、対応するテストでカバーされているか
- **境界値**: パーサーや AST 走査で off-by-one が起きやすい箇所のテスト
- **エラーケース**: エラー / 警告メッセージ変更があれば、対応する compiler-errors / validator テストの追加・更新があるか
- **既存テストの維持**: 既存テストを変更している場合、本来カバーしていたケースが失われていないか
- **テストコードの品質**: コメントとアサーションが矛盾していないか、冗長なアサーションがないか
## 出力フォーマット
ファイルごとに指摘を報告。各指摘に **[Critical/Major/Minor]** [ファイル:行番号] [指摘内容] → [改善案] を含める。
指摘がない場合は「指摘なし」と報告。チャンク外のファイルへの指摘は出さないこと。コードの修正は行わず、レビュー結果の報告のみ。
Agent 5: セキュリティ / メモリ安全性レビュー
セキュリティおよびメモリ安全性観点のコードレビューを、以下の対象ファイルに絞って実行してください。
## 対象チャンク
[チャンク名]
## 対象ファイル
- <file1>
- <file2>
- ...
Skill ツールを使って `security-review` を呼び出してください。
ただし、上記の対象ファイル一覧の範囲に絞ってレビューするように指示し、結果をそのまま報告してください。
チャンク外のファイルへの指摘は出力に含めないでください。
## rsvelte 固有の補足観点
コンパイラなので Web アプリ的な脆弱性(XSS、SQL injection、認証)は通常該当しません。代わりに以下を重視してください:
- **`unsafe` ブロック**: 必要性と安全性の証明がコメントで明示されているか
- **整数オーバーフロー**: u32 ベースの位置情報やインデックスでオーバーフローが起き得ないか
- **入力検証**: コンパイラへの入力(Svelte ソース文字列)の異常系(巨大ファイル、深くネストした構文、不正 UTF-8)でパニックしないか
- **DoS リスク**: 悪意ある入力で指数的時間 / 空間を消費するパスがないか
- **NAPI 境界**: 外部から渡される値の検証、`unwrap()` 多用の有無
- **依存ライブラリ**: 新規依存の追加に対するレビュー(`cargo audit` 相当)
## 出力フォーマット
チャンク外のファイルへの指摘は除外してください。コードの修正は行わず、レビュー結果の報告のみ行ってください。
Agent 6: コード簡素化レビュー
コードの簡素化・品質改善レビューを、以下の対象ファイルに絞って実行してください。
## 対象チャンク
[チャンク名]
## 対象ファイル
- <file1>
- <file2>
- ...
Skill ツールを使って `simplify` を呼び出し、上記対象ファイルに限定したレビューを依頼してください。
チャンク外のファイルへの指摘は除外してください。
## rsvelte 固有の補足観点
- "高速なプログラムはシンプルなデータ構造に対してシンプルなコードが書かれている" という原則を意識すること(`/perf` スキル参照)
- 不要な抽象化(trait 階層、ジェネリクス、マクロ)が hot path に紛れ込んでいないか
- 公式 Svelte と差異がある「rsvelte 独自の仕組み」が本当に必要なものか(不要なら公式と同じ構造に戻す方が良い)
Agent 7: CodeRabbit レビュー(チャンク版・インストール済みの場合のみ)
CodeRabbit による AI コードレビューを、以下の対象ファイルに絞って実行してください。
## 対象チャンク
[チャンク名]
## 対象ファイル
- <file1>
- <file2>
- ...
## 手順
1. `coderabbit:review` スキルが利用可能か確認する
- 利用不可なら「CodeRabbit 未インストールのためスキップ」と報告して終了
2. 利用可能なら、Skill ツールを使って `coderabbit:review` を呼び出し、上記対象ファイルに絞った結果のみを報告してください。
チャンク外のファイルへの指摘は除外してください。
Agent 8: Codex レビュー(チャンク版・Codex CLI がインストールされている場合のみ)
Codex CLI を使った包括的コードレビューを、以下の対象ファイルに絞って実行してください。
## 対象チャンク
[チャンク名]
## 対象ファイル
- <file1>
- <file2>
- ...
## ベースブランチ
${BASE_BRANCH}(Step 0 で決定したブランチ)
## 手順
1. `which codex` で Codex CLI のインストール状況を確認する
- 未インストールなら「Codex CLI 未インストールのためスキップ」と報告して終了
2. インストール済みなら、Skill ツールで `codex-review` スキルを呼び出し、上記対象ファイルに限定したレビューを依頼する
- 対象ファイル一覧と ${BASE_BRANCH} はスキルへの指示文に明示すること
チャンク外のファイルへの指摘は除外してください。コードの修正は行わず、レビュー結果の報告のみ行ってください。
Step 4-2: レビュー結果の個別報告・修正・対話
各 Agent のレビュー結果を受け取るたびに、以下の流れで進める:
- 指摘事項をユーザーに報告する(チャンク名・Agent 名・反復回数を必ず明示)
- Critical/Major の指摘に対して 修正案を提示 する
- ユーザーに確認を取り、承認されたら修正を実施する
- ユーザーがスキップを指示した場合はその旨記録して次へ進む
- 修正が発生したら 同じチャンク・同じ Agent で再レビュー(Step 4-3) に進む
- 指摘がゼロになったら、次の Agent(または次のチャンク)に進む
報告フォーマット:
## [チャンク #N: チャンク名] / [Agent名] レビュー結果(反復 M 回目)
### 指摘事項
1. **[Critical]** [ファイル:行番号] [指摘内容]
→ 修正案: [具体的な修正内容]
2. **[Major]** [ファイル:行番号] [指摘内容]
→ 修正案: [具体的な修正内容]
3. **[Minor]** [ファイル:行番号] [指摘内容]
(指摘がない場合は「指摘なし。次の Agent に進みます」)
→ 上記の修正案で対応してよいですか?変更点やスキップしたいものがあればお知らせください。
Step 4-3: 同チャンク・同 Agent での再レビュー(指摘ゼロまで反復)
Step 4-2 で修正が発生した場合、修正された同じチャンクに対して、同じ Agent を再実行する。これにより:
- 修正による副作用や、修正自体に対する新たな指摘を検出する
- 反復回数をカウントしながら、指摘がゼロになるまで Step 4-1 → Step 4-2 → Step 4-3 を繰り返す
- 反復が 5 回を超えた場合 は、ユーザーに「反復が長期化しています。残った指摘をスキップして次に進みますか?」と確認する(無限ループ防止)
1 つのチャンク × 1 つの Agent のループ終了条件(いずれかを満たしたら次の Agent へ):
- すべての Critical/Major 指摘が解消されている(Minor のみ残)
- Major 指摘が残っているが、ユーザーが明示的にスキップを承認している
- Minor のみが残っている(Minor はユーザーの判断に委ねる)
1 つのチャンクの完了条件: そのチャンクに対する全 Agent が上記ループ終了条件を満たしたら次のチャンクへ進む。
Phase 4 全体の完了条件: 全チャンクが完了したら Phase 5 へ進む。
進捗表示: 反復のたびに、現在どのチャンクのどの Agent の何回目の反復にいるかをユーザーに伝える。
進捗: チャンク 2/4 (transform: client/visitors/) / Agent 3/8 (Performance) / 反復 2 回目
Phase 5: テスト実行と最終サマリー
Step 5-1: テスト・Lint 実行
すべてのフェーズが完了したら、変更内容を最終確認するためにフルテストを実行する。
cargo fmt --all
cargo clippy --all-targets --all-features -- -D warnings
cargo test --release
pnpm run generate-fixtures
pnpm run compatibility-report
./scripts/bench/bench.sh --quick
NAPI 経由の挙動に影響する変更があれば、vitest も実行する:
cargo build --release -p rsvelte_napi --lib
cp target/release/librsvelte_napi.dylib svelte/rsvelte.darwin-arm64.node
cd svelte
USE_RSVELTE=true npx vitest run packages/svelte/tests/runtime-runes/test.ts packages/svelte/tests/runtime-legacy/test.ts
いずれかが失敗した場合、「失敗の原因がレビュー対象の変更によるものか、もともと失敗していたものか」を判別する:
- もともと失敗(regression ではない)→ 別 issue として記録、本 PR では対応しない選択肢を提示
- 本 PR の変更が原因 → Phase 4 に戻り、該当箇所を再レビュー / 修正
Step 5-2: 最終サマリー
## レビュー完了サマリー
### 総合判定: Approve / Request Changes / 要議論
### フェーズ別結果
| フェーズ | 判定 | Critical | Major | Minor |
|----------|------|----------|-------|-------|
| 1. WHY と妥当性 | OK / NG | - | - | - |
| 2. 全体設計 | OK / NG | N件 | N件 | N件 |
| 3. AST / 基盤型 | OK / NG / Skip | N件 | N件 | N件 |
| 4. 実装レビュー | OK / NG / Skip | N件 | N件 | N件 |
| 5. テスト / Lint | OK / NG | - | - | - |
### Phase 4 チャンク別実装レビュー詳細
| チャンク # | チャンク名 | 公式整合性 | コンパイラ実装 | パフォーマンス | テスト | セキュリティ | 簡素化 | CodeRabbit | Codex |
|-----------|-----------|-----------|---------------|---------------|--------|------------|--------|-----------|-------|
| 1 | [チャンク1名] | 指摘なし(1回) | 指摘なし(2回) | 指摘なし(1回) | N/A | 指摘なし(1回) | 指摘なし(1回) | スキップ | スキップ |
| 2 | [チャンク2名] | スキップ承認(N件残, 3回) | 指摘なし(1回) | 指摘なし(1回) | 指摘なし(1回) | 指摘なし(1回) | 指摘なし(1回) | 指摘なし(1回) | 指摘なし(1回) |
| ... | ... | ... | ... | ... | ... | ... | ... | ... | ... |
凡例: `指摘なし(N回)` = N 回の反復で指摘ゼロに到達 / `スキップ承認(N件残, M回)` = ユーザー承認のもと N 件残してスキップ / `スキップ` = 対象外(CLI 未インストールなど)/ `N/A` = チャンクに該当ファイルなし
### Phase 4 全体集計
| レビューツール | 総反復回数 | 指摘ゼロ達成チャンク | 残指摘ありチャンク |
|---------------|-----------|---------------------|-------------------|
| 公式整合性 | N回 | N/N | N |
| コンパイラ実装 | N回 | N/N | N |
| パフォーマンス | N回 | N/N | N |
| テスト | N回 | N/N | N |
| セキュリティ | N回 | N/N | N |
| 簡素化 | N回 | N/N | N |
| CodeRabbit | N回 | N/N | N |
| Codex | N回 | N/N | N |
### テスト・パフォーマンス計測結果
- **cargo test**: PASS / FAIL(失敗があれば内訳)
- **clippy / fmt**: クリーン / 警告 N 件
- **互換性レポート**: X / Y (Z%) — 変更前と比較して +A / -B
- **ベンチマーク**: 変更前 X ms → 変更後 Y ms(P / S / F %)
### 未対応の指摘事項(あれば)
1. [指摘内容と理由 — 別 issue 化推奨など]
### 対応済みの指摘事項
1. [対応内容]
### コードオーナーへの申し送り事項
- [手動確認が必要な事項]
- [長期的な改善候補(本 PR スコープ外)]
Phase 6: コミット・プッシュ・CI 監視
レビューと修正がすべて完了したら、以下の順に進める。
Step 6-1: ブランチの安全確認
現在のブランチ名を確認する。main、master、release で始まるブランチの場合、直接プッシュすると事故になるため、先にユーザーに確認する:
現在のブランチは `[ブランチ名]` です。
このブランチに直接プッシュすると影響が大きいため、新しいブランチを切ることを推奨します。
→ 新しいブランチを作成しますか?ブランチ名を指定してください。
それ以外のブランチの場合はそのまま Step 6-2 に進む。
Step 6-2: コミットとプッシュ
修正内容がある場合、ユーザーに確認する:
レビューで修正した内容をコミット・プッシュしますか?
ユーザーが承認した場合、変更を 論理単位ごとにアトミックにコミット してプッシュする。
コミットメッセージは既存リポジトリの慣習(fix: / refactor: / perf: / feat: / chore: / docs: 等のプレフィックス)に従う。最近のログ:
git log --oneline -10
Step 6-3: CI パス・レビューコメント解決ループ(PR がある場合のみ)
プッシュが完了し、かつ PR が存在する場合、ユーザーに確認する:
CI 監視 → レビューコメント対応 → CI 再監視 のループを実行し、
CI がパスかつレビューコメントがすべて解決済みの状態にしますか?
ユーザーが承認した場合、以下のループを実行する:
- CI 監視:
gh run list / gh pr checks で CI 状態を確認し、失敗があれば原因を分析・修正する
- レビューコメント対応:
gh api repos/{owner}/{repo}/pulls/{pr_number}/comments で未解決コメントを取得し、1 つずつ対応する
- 再度 CI 監視: レビューコメント対応でコード修正が発生した場合、再度 CI を監視する
- ループ終了条件: CI が全パス かつ 未解決のレビューコメントがゼロになるまで繰り返す
指摘の重要度定義
| 重要度 | 定義 | 対応 |
|---|
| Critical | 公式 Svelte と挙動が異なりテスト失敗を招く / メモリ安全性違反 / 計測可能な性能退行 | 必ず修正。Approve 不可 |
| Major | 公式実装と構造が乖離(追従コスト増)/ パフォーマンス低下リスク / テスト不足 | 原則修正。正当な理由があればスキップ可 |
| Minor | 命名・コメント・微小な可読性改善 | 推奨。費用対効果で判断 |
レビュー時の心構え
- コードオーナーの代理 として、プロダクト全体の品質を守る視点でレビューする
- 段階的に進める: 各フェーズで問題を発見したら、そのフェーズで解決してから次に進む
- 対話重視: 一方的に指摘するのではなく、ユーザーとの対話を通じて最適解を見つける
- 公式実装との整合性を最重視: 100% テスト互換性が rsvelte の最重要目標。独自抽象は慎重に判断する
- パフォーマンスは計測ベース: 「速くなりそう」ではなく
./scripts/bench/bench.sh で計測した数値で議論する
- False Positive に注意: 特に
unwrap() の指摘で「テストコード内 / unsafe 内で正当性が証明された箇所」を誤って指摘しないこと
- 品質に妥協しない: Critical / Major 指摘が残っている状態で Approve しない
- テスト実行は必ずリリースビルドで: rsvelte は
cargo test --release で実行する(debug ビルドは遅すぎる)
参照ドキュメント
| フェーズ | 参照先 |
|---|
| Phase 1 (WHY) | PR 説明欄、関連 Issue、サブモジュール CHANGELOG |
| Phase 2 (設計) | CLAUDE.md、AGENTS.md、README.md、PERF_ROADMAP.md、/perf スキル |
| Phase 3 (AST / 基盤型) | src/ast/、対応する公式 AST 定義(svelte/packages/svelte/src/compiler/types/) |
| Phase 4 (実装レビュー) | 各 Agent が以下を読み込んで独立にレビュー: |
| - Agent 1: 公式 Svelte コンパイラ(svelte/packages/svelte/src/compiler/) |
| - Agent 2: implementation-checklist.md |
| - Agent 3: implementation-checklist.md(パフォーマンスセクション)+ /perf スキル |
| - Agent 4: implementation-checklist.md(テストセクション) |
| - Agent 5: security-review スキル呼び出し(組み込みスキル) |
| - Agent 6: simplify スキル呼び出し |
| - Agent 7: coderabbit:review スキル呼び出し(インストール済みの場合のみ) |
| - Agent 8: codex-review スキル呼び出し(CLI インストール済みの場合のみ) |
implementation-checklist.md は .claude/skills/full-code-review/implementation-checklist.md に配置されており、
公式実装整合性・メモリ安全性・パフォーマンス・テストカバレッジ・DRY 原則・命名規則などの実装レベルのレビュー観点を網羅している。