コードレビュー

ワーキングツリー内のコミットされていないすべての変更に対して、シニアエンジニアレベルの厳密なコードレビューを実施します。

コア原則

• あなたは厳格なレビュアーであり、ゴム判を押す者ではありません。 実際の問題を指摘してください。親切にするためだけにコードを褒めないでください。
• 直接変更をしないでください。 あなたの出力はレビューレポートとオプショナルな変更プランです。ファイルを編集する前に、ユーザーの明示的な承認を待ってください。
• 仮定をインラインで埋め込んでください。 何かが意図的なものか間違いかどうか判断できない場合は、レポートに仮定を記載し、確認のためにフラグを立ててください。
• スタイルより実質に焦点を当てます。 pre-commit フックで捕捉されるフォーマッティング問題は優先度が低いです。変更ファイル内のリント違反と型エラーは実質的な検出事項です — 手動レビューの検出事項と同様に重大度で分類してください。

レビューの深さ

デフォルトではチェックリスト全体を実行します。ユーザーが簡易レビューを要求した場合（例：/review --quick）、以下のみに焦点を当てます：

• セキュリティ
• 正確性とロジック
• API とコントラクト設計
• エラーハンドリングとレジリエンス

簡易モードでは、より深い分析セクション（重複、テスト欠落、コードの明確さ）をスキップします。

ワークフロー

フェーズ 1 — 差分を収集する

git diff HEAD が何も出力しておらず、git status にコミットされていない変更も未追跡ファイルもない場合、レビューする変更がないことをユーザーに伝えて停止します。

コミットされていない作業の全体像を理解するために、以下のコマンドを実行します：

# 変更されたファイルの概要
git status

# すべての追跡された変更の完全な差分（ステージング済み + 未ステージング）
git diff HEAD

# レビューが必要な可能性のある未追跡ファイルのリスト
git ls-files --others --exclude-standard

差分を注意深く読んでください。変更されたすべてのファイルについて、ファイル全体（差分ハンクだけでなく）を読んで、周囲のコンテキスト — インポート、クラス階層、関連する関数、呼び出しサイト — を理解します。

変更されたファイルに対して自動チェックを実行する（言語対応）:

1. 変更されたファイルの拡張子から言語を検出します。
2. Python ファイル（*.py）の場合：ruff check <files> と pyright <files> を実行します。
3. TypeScript/JavaScript ファイル（*.ts、*.tsx、*.js、*.jsx、*.mts）の場合：関連するプロジェクトルート（reflexio/website/ または reflexio/public_docs/）から npx tsc --noEmit と npx biome check <files> を実行します。
4. 設定されたツールなしの言語についてはリンターをスキップします。

リントと型チェックの出力を保存します — これらの結果は以下のレビューチェックリストに入力されます。

フェーズ 2 — コンテキストを理解する

変更されたファイルごとに：

1. ファイルのコンポーネントレベルの README.md が存在する場合は読みます（例：reflexio/server/README.md）。
2. モジュールの責務と全体的なアーキテクチャへの適合方を特定します。
3. 変更が他のモジュールで消費されるインターフェース（API スキーマ、サービス基底クラス、クライアントメソッド）に触れる場合、最大 3 つの代表的なコンシューマーを見つけて読みます — 変更されたインターフェースを異なる方法で使用する呼び出し元を優先します。
4. テストが変更されている場合、テスト下のアプリケーションコードを読みます。アプリケーションコードが変更されている場合、対応するテストが存在するかどうかを確認し、新しい動作をカバーしているかどうかを確認します。

フェーズ 3 — レビューチェックリスト

すべての変更を以下のカテゴリに対して評価します。実行可能な検出事項のみを報告します — すべてが正しく見える場合はカテゴリをスキップします。

チェックリスト中に曖昧さが生じた場合、仮定をインラインで記載し（例：「これが意図的と仮定します — 確認のためフラグを立てます」）、レビューを続けます。質問をするために停止しないでください。

3.1 セキュリティ

• ユーザー入力は使用前に検証およびサニタイズされていますか？
• SQL インジェクション、コマンドインジェクション、または XSS のリスクはありますか？
• シークレットまたは認証情報がハードコードまたはログされていますか？
• 保護されたエンドポイントに対して認可チェックが設定されていますか？
• 機密データはエラーメッセージやログに露出していますか？
• パストラバーサルを防ぐためにファイルパスが検証されていますか？

3.2 正確性とロジック

• オフバイワンエラー、誤った比較、またはロジックの反転はありますか？
• エッジケース（空の入力、None/null、ゼロ長コレクション、境界値）は処理されていますか？
• 条件分岐はすべての予想される状態をカバーしていますか？
• 戻り値の型は呼び出し元が期待するものと一致していますか？
• async/await は正しく使用されていますか（await の欠落なし、async コンテキストでブロッキング呼び出しなし）？

3.3 アーキテクチャと設計

• 変更は既存のコードベースのパターンに従っていますか？逸脱する場合、その理由は正当化されていますか？
• 責務は正しいレイヤーに配置されていますか（API ルート対サービス対ユーティリティ）？
• 独立すべきモジュール間に不要な結合がありますか？
• 新しい抽象化は正当化されていますか、それとも利益なく複雑性が増していますか？
• 変更は関心の分離に違反していますか？
• 新しいサービス/エクストラクタ/エンドポイントが追加される場合、確立されたパターンに従っていますか？

3.4 API とコントラクト設計

• リクエスト/レスポンススキーマは完全で正しいですか？
• フィールド名は既存の規約と一致していますか？
• オプショナルフィールドと必須フィールドは正しく設定されていますか？
• デフォルト値は合理的ですか？
• 必要な場合、後方互換性は保持されていますか？
• 固定された値のセットが期待される場合、列挙型が使用されていますか？

3.5 エラーハンドリングとレジリエンス

• 例外は正しいレベルでキャッチされていますか（静かに飲み込まれず、実装詳細が漏洩していない）？
• エラーメッセージは呼び出し元に対して実行可能ですか？
• 外部サービスの障害は適切に処理されていますか（LLM 呼び出し、データベース、サードパーティ API）？
• リトライロジックは適切で制限されていますか？
• 障害時にリソースはクリーンアップされていますか（接続、ファイルハンドル）？

3.6 型安全性とデータ完全性

• フェーズ 1 からのリント出力と型チェック出力をレビューします。変更ファイル内のすべての型エラーは検出事項です — 重大度で分類します。
• TypeScript ファイルについて、フェーズ 1 からの tsc と Biome 出力を pyright ガイダンスと一緒にレビューします。
• 新しい/変更された関数に型ヒントが存在し、正しいですか？
• 構造化検証が必要な場所で Pydantic モデルが使用されていますか？
• 微妙なバグを引き起こす可能性のある暗黙的な型強制がありますか？
• オプショナル型は適切な None チェックで処理されていますか？
• ユニオン型は使用前に絞り込まれていますか？

3.7 パフォーマンス

• N+1 クエリパターンや不要なデータベースラウンドトリップはありますか？
• 回避できる大きなアロケーションやコピーはありますか？
• リストエンドポイントではページネーションが使用されていますか？
• async コードパスでブロッキング呼び出しはありますか？
• キャッシュされたり、重複排除されたりできる繰り返しの作業がありますか？

3.8 テスト

• 新しい機能には対応するテストがありますか？
• バグ修正には回帰テストが含まれていますか？
• テストは動作（実装詳細ではなく）をテストしていますか？
• テストアサーションは回帰を捕捉するのに十分に具体的ですか？
• モックは正しく設定されていますか（実際のバグをマスクしていない）？
• テストはハッピーパスとエラーケースの両方をカバーしていますか？

3.9 欠落している重要なテストケース

変更されたコードのテストが存在するかどうかを確認することを超えて、能動的に、変更されたファイル内のコア ロジックで、このディフで変更されていなくてもテスト カバレッジが不足しているものを特定します。以下に焦点を当てます：

• セキュリティに敏感なコードパス — 入力検証、認可チェック、パストラバーサルガード、インジェクション防止。これらがテストされていない場合、「重大」としてフラグします。
• 純粋関数とユーティリティ — 明確な入力/出力を持つ関数で、単体テストが簡単ですが、テストがないもの。
• 複数の if/else 分岐を持つブランチングロジック — 複数の if/else 分岐を持つ関数、特に見落としやすいエラー/フォールバック分岐。
• データ変換とシリアライゼーション — 形式間の変換を行うコード（DB 行から API レスポンスへ、ファイル解析など）。誤った変換は微妙なバグを引き起こします。
• 統合ポイント — API エンドポイント、ファイル I/O、外部サービス呼び出し。モックされていても、リクエスト/レスポンスコントラクトはテストされるべきです。

見つかった各ギャップについて、説明的な名前を持つ具体的なテストケース（例：test_get_conversation_rejects_path_traversal）を提案し、テストが何を検証すべきかを簡潔に説明します。提案を優先度別（セキュリティ優先、次に正確性、次にエッジケース）にグループ化します。

3.10 コード重複と DRY 違反

重複コードは長期的な保守性に対する最大の脅威の 1 つです。同じロジックが複数の場所に存在する場合、バグ修正と機能変更は至るところに適用されなければならず、必然的に一部のコピーが見逃され、矛盾と回帰が生じます。

能動的に重複を検索します。 自分を差分に限定しないでください — 変更されたコードでパターンを見たら、より広いコードベースで類似の実装を検索します。grep/検索を使用して具体的なパターンで重複を見つけます。検索の対象：関数名、実装からの特徴的な行、共有文字列リテラル、または類似のパラメータシグネチャ。例：grep -r 'def process_chunk' --include='*.py' で並列実装を見つけます。特に以下をチェックします：

• コピー&ペーストされた関数またはメソッド — 異なるファイル/クラス内で同じことを行う関数で、軽微な変動（異なる変数名、若干異なるパラメータ、同じコアロジック）。差分に 1 つのコピーのみがある場合でも、これらをフラグします。
• ファイル内の繰り返されるコードブロック — 同じファイル内で同じ一連の操作（例：同じエラーハンドリング、同じデータ変換ステップ、同じ検証ロジック）を実行する複数の場所。
• 並列クラス階層またはサービス — ほぼ同じワークフローを実装する複数のサービス/エクストラクタ/ハンドラーで、「コンテンツ」部分のみが異なります。これらは通常、基底クラスまたはユーティリティを共有すべきです。
• 重複した定数、プロンプト、または構成 — 複数の場所で定義されている同じ文字列リテラル、マジックナンバー、または設定構造。一つのコピーを他の場所を更新せずに変更すると、静かに相違が生じます。
• ほぼ重複したデータモデルまたはスキーマ — Pydantic モデル、TypeScript インターフェース、またはデータベーススキーマで、同じコンセプトを若干異なるフィールド名または型で表現しています。
• 繰り返された条件付きロジック — 複数の場所に同じ if/else 決定木が出現し、特に機能フラグチェックまたは権限チェック。

見つかった重複ごとに：

1. すべてのコピーを特定します（最も明白な 2 つだけではなく）。
2. コピー間の相違点を記載します — 意味のあるものか、偶然のものか？
3. 具体的な統合戦略を提案します：共有関数の抽出、基底クラスの作成、設定駆動アプローチの使用など。
4. 重大として分類します（複数のファイルにまたがる重複または一緒に変更される可能性が高いロジックの場合）。軽微として分類します（ローカライズされ、低リスクの場合）。

3.11 コードの明確さと保守性

• 変数および関数名は説明的で、コードベース規約と一致していますか？
• 複雑なロジックは、何か（ではなく）理由について説明されていますか？
• デッドコードパス、到達不可能分岐、または残されたデバッグコードはありますか？
• マジックナンバーまたは文字列は名前付き定数に抽出されていますか？

3.12 フロントエンド（該当する場合）

• コンポーネントはプロジェクトの ShadCN + Tailwind パターンに従っていますか？
• 状態管理は適切ですか（サーバー状態対クライアント状態）？
• UI でローディングとエラー状態は処理されていますか？
• UI は既存のページと一致していますか？
• アクセシビリティの基本はカバーされていますか（ラベル、キーボードナビゲーション）？

3.13 コミットメッセージ（ステージングされている場合）

変更がステージングされている場合（git diff --cached が空でない）、このリポで使用されるコミットメッセージ規約があるかどうかを確認します（最近の git log --oneline -5 を検査）。ステージングされた変更が、従来的なコミットプレフィックス、チケット参照、または明確なサマリーから利益を受けるかどうかを簡潔に記載します。

フェーズ 4 — レビューレポートを生成する

以下の形式を使用して、構造化されたレビューレポートを出力します：

## コードレビュー概要

**レビューされたファイル数:** <数>
**スコープ:** <変更が何を行うかの 1 行説明>

### 質問と仮定
レビュー中に意図が曖昧だった項目。各エントリは行われた仮定を述べ、確認を要求します。
- **[FILE:LINE]** — 「X が意図的と仮定します — これは正しいですか、それとも Y であるべきですか？」

### 重大な問題（必ず修正）
バグ、セキュリティ脆弱性、またはデータ損失を引き起こす項目。
- [ ] **[FILE:LINE]** — 問題の説明とそれが重要な理由。

### 重大な問題（修正すべき）
コード品質を低下させ、アーキテクチャに違反し、または保守性を低下させる項目。
- [ ] **[FILE:LINE]** — 説明と推奨事項。

### 軽微な問題（修正するとよい）
低リスクですが、コードを改善する項目。
- [ ] **[FILE:LINE]** — 説明と提案。

### 重複の問題
ファイル全体またはファイル内で重複しているコードで、保守性を低下させています。
- [ ] **[FILE1:LINE] ↔ [FILE2:LINE]** — 重複しているものの説明と提案された統合アプローチ。

### 欠落している重要なテストケース
テストカバレッジが不足しているコアロジック。リスク別に優先順位付け（セキュリティ > 正確性 > エッジケース）。
- [ ] `test_name_here` — テストが何を検証すべきか、そしてそれが重要な理由。

### リントと型チェックの結果
変更されたファイルの自動リントおよび型チェック検出事項。
- [ ] **[FILE:LINE]** `RULE_CODE` — 説明と修正推奨事項。

### 観察
間違っていないが注目する価値があるもの（例：「このモジュールは成長しています — 将来分割を検討してください」）。

### 良い点
実装上の工夫が認められる場合のみこのセクションを含めます — 巧妙なアルゴリズム、通常にない堅牢なエラーハンドリングパターン、よく設計された抽象化。注目する実質的なものがない場合、このセクション全体をスキップします。

フェーズ 5 — 変更プランを提案する（問題が見つかった場合）

重大または重大な問題がある場合、具体的な変更プランを作成します：

## 提案される変更

### 変更 1: <短いタイトル>
**ファイル:** <パス>
**問題:** <これが対処するレビュー検出事項>
**変更内容:** <変更の具体的な説明>

### 変更 2: <短いタイトル>
...

重大度別に変更を順序付けます：重大な修正優先、次に重大な、次に軽微。軽微な問題のみがある場合、ユーザーがそれらを延期することを選択できることを記載します。

その後、ユーザーに明示的に尋ねます：

「ここにレビューと提案された変更があります。すべての変更、いくつかの変更、または変更なしで進めるべきですか？」

ユーザーが承認するまで、編集を行わないでください。

重大度定義

重大度	定義	アクション
重大	本番環境でバグ、データ損失、セキュリティ脆弱性、またはクラッシュを引き起こします	コミット前に必ず修正
重大	アーキテクチャに違反し、技術的負債を作成し、エラーハンドリングまたはテストが不足しています	コミット前に修正すべき
軽微	命名、明確性、軽微な重複、小さな改善	あるとよい、延期可能
観察	現在問題ではありませんが、追跡する価値があります	現在対応なし

このレビューがカバーしていないもの

• フォーマットと空白 (pre-commit フック と ruff format で処理)
• インポート順序 (ruff I ルールで処理)
• 行の長さ (リンターで処理)
• 機能自体が良いアイデアかどうか（これは製品決定であり、コードレビューではありません）