コードレビューと品質

概要

品質ゲート付きの多次元的なコードレビューです。すべての変更は マージ前にレビューされます。例外はありません。レビューは5つの軸をカバーします：正確性、可読性、アーキテクチャ、セキュリティ、パフォーマンスです。

承認の基準： 変更がコード全体の健全性を確実に改善する場合、それが完璧でなくても変更を承認します。完璧なコードは存在しません。目標は継続的な改善です。あなたの書き方と異なるからという理由だけで変更をブロックしてはいけません。コードベースを改善し、プロジェクトの慣例に従っていれば、承認してください。

使用するタイミング

• PR やあらゆる変更をマージする前
• 機能実装を完了した後
• 別のエージェントまたはモデルが生成したコードを評価する必要があるとき
• 既存コードをリファクタリングするとき
• バグ修正の後（修正とリグレッションテスト両方をレビュー）

5つの軸によるレビュー

すべてのレビューは以下の次元でコードを評価します：

1. 正確性

コードは主張していることを実際に行っていますか？

• 仕様またはタスク要件に一致していますか？
• エッジケース（null、空、境界値）が処理されていますか？
• エラーパス（ハッピーパスだけでなく）が処理されていますか？
• すべてのテストが通っていますか？テストは実際に正しいことをテストしていますか？
• オフバイワンエラー、競合状態、状態の不一貫性はありませんか？

2. 可読性とシンプルさ

別のエンジニア（またはエージェント）は、作成者の説明なしにこのコードを理解できますか？

• 名前は説明的でプロジェクト慣例と一貫していますか？（文脈のない temp、data、result はダメ）
• 制御フロー は単純ですか？（ネストされた三項演算子、深いコールバックを避ける）
• コードは論理的に組織されていますか？（関連コードがグループ化され、モジュール境界が明確）
• 単純化すべき「賢い」トリックがありませんか？
• これをもっと少ない行数で実装できませんか？ （100行で済むのに1000行は失敗）
• 抽象化はその複雑性に見合っていますか？ （3番目のユースケースまで待つまで一般化しない）
• コメントが非明白な意図を明確にするのに役立つでしょうか？（ただし、明白なコードにはコメントを付けない）
• デッドコードの遺物がありませんか：ノーオペ変数（_unused）、後方互換性の shim、または // removed コメント？

3. アーキテクチャ

変更はシステムの設計に合致していますか？

• 既存パターンに従っていますか？それとも新しいパターンを導入していますか？新しい場合、その正当性はありますか？
• クリーンなモジュール境界を保持していますか？
• 共有すべきコードの重複がありませんか？
• 依存関係は正しい方向に流れていますか？（循環依存なし）
• 抽象化レベルは適切ですか？（過度に設計されていない、過度に結合されていない）

4. セキュリティ

詳細なセキュリティ ガイダンスについては、security-and-hardening を参照してください。変更は脆弱性を導入していませんか？

• ユーザー入力は検証およびサニタイズされていますか？
• シークレットはコード、ログ、バージョン管理から除外されていますか？
• 認証/認可は必要な場所でチェックされていますか？
• SQL クエリはパラメータ化されていますか？（文字列連結なし）
• 出力は XSS を防止するためにエンコードされていますか？
• 依存関係は信頼できるソースから取得され、既知の脆弱性はありませんか？
• 外部ソース（API、ログ、ユーザーコンテンツ、設定ファイル）からのデータは信頼されていないものとして扱われていますか？
• 外部データフローはロジックやレンダリングで使用される前に、システム境界で検証されていますか？

5. パフォーマンス

詳細なプロファイリングと最適化については、performance-optimization を参照してください。変更はパフォーマンス問題を導入していませんか？

• N+1 クエリパターンがありませんか？
• 無限ループまたは無制限のデータ取得がありませんか？
• 非同期であるべき同期操作がありませんか？
• UI コンポーネントで不要な再レンダリングがありませんか？
• リストエンドポイントでペジネーションが不足していませんか？
• ホットパスで大きなオブジェクトが作成されていませんか？

変更のサイズ

小さく焦点を絞った変更の方が、レビューしやすく、マージが速く、デプロイが安全です。以下のサイズを目標にしてください：

~100 行の変更   → 良い。1回のセッションでレビュー可能。
~300 行の変更   → 1つの論理的な変更であれば許容可能。
~1000 行の変更  → 大きすぎる。分割してください。

「1つの変更」とは何か： 1つのこと に対応し、関連するテストを含み、提出後もシステムが機能する、自己完結した単一の変更。機能全体ではなく、機能の1つの部分です。

変更が大きすぎる場合の分割戦略：

戦略	方法	使用タイミング
スタック	小さな変更を提出し、次の変更をそれに基づいて開始	順序依存関係がある場合
ファイルグループ別	異なるレビュアーが必要なグループに対して別々の変更を作成	横断的な関心事
水平	共有コード/stub を最初に作成し、次に消費者を作成	階層化されたアーキテクチャ
垂直	機能の小さいフルスタック スライスに分割	機能作業

大きな変更が許容される場合： 完全なファイル削除とレビュアーが意図を確認するだけで済むすべての行を確認する必要のない自動リファクタリング。

リファクタリングと機能作業を分離する。 既存コードをリファクタリングして新しい動作を追加する変更は2つの変更です。別々に提出してください。小さなクリーンアップ（変数のリネーム）はレビュアーの裁量で含めることができます。

変更の説明

すべての変更には、バージョン管理の履歴で自立して機能する説明が必要です。

1行目： 短く、命令形で、自立したものにします。「FizzBuzz RPC を削除する」は「FizzBuzz RPC を削除しています」ではなく、「FizzBuzz RPC を削除する」にします。差分を読まなくても履歴を検索する人が変更を理解できるほど十分な情報が必要です。

本文： 何が変更され、なぜ変更されるのか。コード自体には見えないコンテキスト、決定、推論を含めます。バグ番号、ベンチマーク結果、または設計ドキュメントへのリンクを関連する場合は含めます。アプローチの欠点が存在する場合は認めます。

アンチパターン： 「バグ修正」「ビルド修正」「パッチ追加」「A から B へのコード移動」「フェーズ 1」「便利な関数を追加」。

レビュープロセス

ステップ 1：コンテキストを理解する

コードを見る前に、意図を理解します：

- この変更は何を達成しようとしていますか？
- どの仕様またはタスクを実装していますか？
- 期待される動作の変更は何ですか？

ステップ 2：最初にテストをレビューする

テストは意図とカバレッジを明かします：

- 変更のテストが存在しますか？
- 実装の詳細ではなく動作をテストしていますか？
- エッジケースはカバーされていますか？
- テストは説明的な名前を持っていますか？
- テストはコードが変更された場合のリグレッションを検出しますか？

ステップ 3：実装をレビューする

5つの軸を念頭に置きながらコードをウォークスルーします：

変更された各ファイルについて：
1. 正確性：このコードはテストが言うべきことを実現していますか？
2. 可読性：助けなしにこれを理解できますか？
3. アーキテクチャ：これはシステムに適合していますか？
4. セキュリティ：脆弱性がありますか？
5. パフォーマンス：ボトルネックがありますか？

ステップ 4：見つけたものを分類する

すべてのコメントに重大度のラベルを付けて、作成者が必須と オプション を区別できるようにします：

プリフィックス	意味	作成者の対応
（プリフィックスなし）	必須の変更	マージ前に対応する必要があります
Critical:	マージをブロック	セキュリティ脆弱性、データ損失、機能の破損
Nit:	軽微、オプション	作成者は無視してもよい—フォーマット、スタイル設定
Optional: / Consider:	提案	検討する価値があるが必須ではない
FYI	情報提供のみ	対応不要—将来の参考用のコンテキスト

これにより、作成者はすべてのフィードバックを必須として扱い、オプションの提案に時間を無駄にすることを防ぎます。

ステップ 5：検証の検証を確認する

作成者の検証ストーリーをチェックします：

- どのテストが実行されましたか？
- ビルドは成功しましたか？
- 変更は手動でテストされましたか？
- UI 変更のスクリーンショットがありますか？
- 変更前/変更後の比較がありますか？

マルチモデル レビュー パターン

異なるレビュー視点に異なるモデルを使用します：

モデル A がコードを書く
    │
    ▼
モデル B が正確性とアーキテクチャをレビューする
    │
    ▼
モデル A がフィードバックに対応する
    │
    ▼
人間が最終的な判断を下す

これは単一のモデルが見落とす可能性のある問題をキャッチします。異なるモデルには異なるブラインドスポットがあります。

レビュー エージェントのプロンプト例：

正確性、セキュリティ、およびプロジェクト慣例への準拠についてこの
コード変更をレビューしてください。仕様では [X] と言っています。
変更は [Y] すべきです。すべての問題を Critical、Important、または
Suggestion としてフラグを立ててください。

デッドコード衛生

リファクタリングまたは実装変更の後、孤立したコードをチェックします：

1. 到達不可能または未使用のコードを特定する
2. 明示的にリストアップする
3. 削除前に確認する： 「これらの今は使用されていない要素を削除すべきですか：[リスト]?」

デッドコードを放置してはいけません。それは将来の読み手とエージェントを混乱させます。ただし、確実でないものを黙って削除しないでください。疑問がある場合は、質問してください。

デッドコード検出：
- src/utils/date.ts の formatLegacyDate() — formatDate() で置き換え
- src/components/ の OldTaskCard コンポーネント — TaskCard で置き換え
- src/config.ts の LEGACY_API_URL 定数 — 残りの参照なし
→ これらは削除しても安全ですか？

レビュー速度

遅いレビューはチーム全体をブロックします。レビューに文脈切り替えのコストは、他の人に課せられる待機コストより少ないです。

• 営業日以内に応答する —これは最大値であり、目標値ではありません
• 理想的なペース： レビューリクエストが届いた直後に応答します（焦点を絞ったコーディングの最中を除く）。典型的な変更は1日で複数のレビューラウンドを完了する必要があります
• 最終承認の高速化より個別の高速応答を優先する。 迅速なフィードバックは複数のラウンドが必要であっても、フラストレーションを軽減します
• 大きな変更： レビュアーに1つの大規模な変更セットをレビューさせるのではなく、変更を分割するよう作成者に要求します

不同意の処理

レビュー紛争を解決する際は、この階層を適用します：

1. 技術的事実とデータ は意見と設定を上回ります
2. スタイルガイド はスタイル問題の絶対的な権限です
3. ソフトウェア設計 は個人的な設定ではなく、エンジニアリング原則で評価される必要があります
4. コードベース一貫性 は全体的な健全性を低下させなければ許容されます

「後でクリーンアップします」を受け入れないでください。 経験から、延期されたクリーンアップはめったに実行されません。提出前にクリーンアップを要求してください。本当の緊急事態の場合を除きます。このスコープで対応できない周辺の問題がある場合は、自己割り当てでバグファイリングを要求します。

レビューの正直さ

あなた、別のエージェント、または人間が書いたコードをレビューする場合：

• ゴム印を押してはいけません。 レビューの証拠なしに「LGTM」は誰も助けません。
• 実際の問題を柔らかくしてはいけません。 「これは軽微な懸念かもしれません」本番環境に達するバグなら不正直です。
• 可能な場合は問題を定量化する。 「この N+1 クエリはリスト内の項目ごとに ~50ms を追加します」は「これは遅い可能性があります」より優れています。
• 明らかな問題のあるアプローチに反論する。 社交辞令はレビューの失敗モードです。実装に問題がある場合は、直接言って代替案を提案してください。
• 上書きを優雅に受け入れる。 作成者が完全なコンテキストを持っていて同意しない場合は、彼らの判断に従ってください。人について コメントしないでください。コード自体に焦点を当てるため、個人的な批判を再フレーミングしてください。

依存関係の規律

コードレビューの一部は依存関係レビューです：

あらゆる依存関係を追加する前に：

1. 既存のスタックはこれを解決しますか？（多くの場合、解決します。）
2. 依存関係のサイズはどのくらいですか？（バンドル影響をチェック）
3. アクティブに保守されていますか？（最後のコミット、未解決の問題をチェック）
4. 既知の脆弱性がありますか？（npm audit）
5. ライセンスはどれですか？（プロジェクトと互換性がある必要があります。）

ルール： 新しい依存関係より標準ライブラリと既存ユーティリティを優先します。すべての依存関係は責務です。

レビュー チェックリスト

## レビュー：[PR/変更タイトル]

### コンテキスト
- [ ] この変更が何をしてなぜするのかを理解している

### 正確性
- [ ] 変更が仕様/タスク要件と一致している
- [ ] エッジケースが処理されている
- [ ] エラーパスが処理されている
- [ ] テストが変更を十分にカバーしている

### 可読性
- [ ] 名前が明確で一貫している
- [ ] ロジックが単純である
- [ ] 不要な複雑性がない

### アーキテクチャ
- [ ] 既存パターンに従っている
- [ ] 不要な結合または依存関係がない
- [ ] 適切な抽象化レベル

### セキュリティ
- [ ] コードにシークレットがない
- [ ] 入力が境界で検証されている
- [ ] インジェクション脆弱性がない
- [ ] 認証チェックが実施されている
- [ ] 外部データソースが信頼されていないものとして扱われている

### パフォーマンス
- [ ] N+1 パターンがない
- [ ] 無限の操作がない
- [ ] リストエンドポイントでペジネーション が実装されている

### 検証
- [ ] テストが通る
- [ ] ビルドが成功する
- [ ] 手動検証が完了している（該当する場合）

### 判定
- [ ] **承認** — マージ準備完了
- [ ] **変更をリクエスト** — 問題に対応する必要があります

関連項目

• 詳細なセキュリティレビューガイダンスについては、references/security-checklist.md を参照してください
• パフォーマンスレビューチェックについては、references/performance-checklist.md を参照してください

一般的な言い訳

言い訳	現実
「動作すれば十分」	読めない、安全でない、またはアーキテクチャ的に間違ったコードは複合する負債を作成します。
「自分で書いたから正しいに決まっている」	作成者は自分の仮定に盲目です。すべての変更は別の目で恩恵を受けます。
「後でクリーンアップします」	後は来ません。レビューは品質ゲートです。使用してください。マージ後ではなく、マージ前にクリーンアップを要求してください。
「AI 生成コードはおそらく大丈夫」	AI コードはスキャン が少ない必要があります。自信を持っていて見どころがありますが、間違っている場合があります。
「テストが通れば大丈夫」	テストは必要ですが十分ではありません。それらはアーキテクチャ問題、セキュリティ問題、または可読性の懸念をキャッチしません。

レッドフラグ

• レビューなしでマージされた PR
• テストが通るかどうかだけをチェックするレビュー（他の軸を無視）
• 実際のレビューの証拠のない「LGTM」
• セキュリティに焦点を当てたレビューなしのセキュリティに敏感な変更
• 「適切にレビューするには大きすぎる」大きな PR（分割する）
• バグ修正 PR でリグレッションテストなし
• 重大度ラベルなしのレビューコメント—必須と オプション が不明確
• 「後で修正します」を受け入れる—実現されません

検証

レビューが完了した後：

• すべての Critical 問題が解決されている
• すべての Important 問題が解決されているか、正当化と共に明示的に延期されている
• テストが通る
• ビルドが成功する
• 検証ストーリーがドキュメント化されている（何が変更され、どのように検証されたか）