Code review

コードレビュー

目的

バグ発見に留まらず、設計の妥当性確認・知識共有・コードベースの一貫性維持を同時に達成する。レビューは攻撃ではなく、チームの集合知をコードに反映させる協調作業である。

使うタイミング

• PRレビューを依頼された・依頼する前の準備をしたい
• AI多視点レビュー(正しさ/セキュリティ/性能/規約)を並列実行したい
• レビュー観点の抜け漏れを防ぎたい
• レビュー文化・プロセスを整備したい

レビュー観点

優先順位が高い順に確認する。後から直すコストが高いものを先に潰す。

優先度	観点	確認内容
1	設計の問題	責務の分離、抽象化の適切さ、将来の変更容易性、インタフェース設計
2	正しさ・バグ	ロジックの誤り、境界値、エラーハンドリング、競合状態、データ整合性
3	テストの妥当性	カバレッジの充足、ハッピーパスだけでなく異常系・境界値のテスト存在
4	セキュリティ	入力検証、認証・認可の抜け、機密情報のログ出力、SQLインジェクション等
5	可読性	命名の明確さ、コメントの適切さ、複雑なロジックの説明
6	好み	linter/formatterに委譲。人間は原則指摘しない

進め方

レビュー依頼側の責務(PR作成者)

1. PR説明に「何を変えたか」「なぜ変えたか」「確認方法」「影響範囲」を記述する
2. PR提出前に自分のdiffを通読する(セルフレビュー。これだけで指摘が半減する)
3. PRサイズが400行を超える場合、論理的な単位で分割する。レビュアーは分割依頼してよい

レビュアーの進め方

1. PR説明とdiff全体をざっと俯瞰し、変更の意図を把握する
2. 優先度1(設計)の観点で通読する。設計に根本的な問題があれば他の観点より先にコメントする
3. 優先度2〜5の順で詳細確認する
4. コードを読むだけでなく、ローカルで動かす・テストを実行して挙動を確認する
5. 良い点・工夫した点にも言及する
6. 指摘をまとめてレビューサマリを添える

AI多視点レビューの実行手順

1. 司令塔が diff とコンテキストを準備する
2. 以下の観点別エージェントを並列起動する(詳細は「モデル委譲ガイド」参照)
   • エージェントA: 正しさ・バグ観点
   • エージェントB: セキュリティ観点
   • エージェントC: 性能・スケーラビリティ観点
   • エージェントD: 規約・可読性観点
3. 各エージェントの結果を受け取り、司令塔が重複排除・優先度付けを行う
4. 統合済みレビュー結果をサマリ付きで出力する

成果物テンプレート

指摘コメントの書式

[must] 理由を1文で。修正しないとマージ不可。
例: [must] NULL チェックが欠如しており、user が nil の場合にパニックが発生します。
修正案:
  if user == nil {
      return ErrUserNotFound
  }

[should] 理由を1文で。強く推奨するが判断はレビュイーに委ねる。
例: [should] このループは O(n²) の計算量になっています。件数が増えるとボトルネックになるため、マップを使った O(n) の実装を検討してください。

[nits] 好みの範囲・任意。取り入れなくてもマージを妨げない。
例: [nits] `GetUserByID` より `FindUserByID` の方が「見つからない場合は nil を返す」意図が伝わりやすいかもしれません。

[question] 仕様・意図の確認。指摘ではなく疑問。
例: [question] ここで既存レコードを上書きするのは意図通りですか？ 追記の場合は別メソッドの方が明示的かと思いました。

[good] 良い点への言及。
例: [good] エラーメッセージに operation context を付与しているのでデバッグしやすいです。

レビューサマリ

## レビューサマリ

### 全体所感
（変更の意図への理解・全体的な品質の印象を2〜3文で）

### 指摘件数
- [must]: X 件
- [should]: Y 件
- [nits]: Z 件

### 承認条件
- [ ] [must] の指摘がすべて解消されること
- [ ] （あれば追加条件）

### 良かった点
- （具体的に1〜2点）

チェックリスト

• PR説明に「何を/なぜ/確認方法/影響範囲」が記載されているか
• PRサイズが400行以内か(超える場合は分割依頼)
• 設計観点(責務分離・インタフェース)を最初に確認したか
• 異常系・境界値のテストが存在するか
• セキュリティ観点(入力検証・認証・機密情報)を確認したか
• 実際に動かして挙動を確認したか
• 良い点に言及したか
• mustとshouldを混同せず、承認条件を明確にしたか
• linterで検出できる指摘を人間がコメントしていないか

アンチパターン

• 好みの押し付け合戦: インデントや命名の好みを [must] で強制する。linterに委ねよ
• 巨大PRのLGTM素通し: 400行超のPRを「全体的に問題なし」で通過させる。分割依頼が正解
• 代案なしの指摘: 「これは良くない」だけでは相手が困る。修正案か方向性を示す
• レビュー放置: 48時間以上応答しないと開発が滞留する。着手できない場合は一言伝える
• linterの仕事を人間がやる: フォーマット・未使用変数等の機械的指摘は自動化で解決する
• 全行コメント: 重箱の隅をつつきすぎるとレビュイーが疲弊し、重要な指摘が埋もれる
• 文脈なしの指摘: コードの行だけ引用して「直してください」は伝わらない。理由を必ず書く

モデル委譲ガイド

共通原則は [[orchestration]] を参照。

役割	担当観点	具体的な使い方
司令塔(メインモデル)	統合・優先度付け・最終判断	diff とコンテキストを準備し、観点別エージェントを並列起動。結果を受け取り重複排除・優先度付けしてサマリを生成する
Opus相当	設計の妥当性・アーキテクチャ整合性	変更が既存アーキテクチャと整合しているか、将来の拡張性に問題がないかを深く分析させる
Sonnet相当	正しさ・セキュリティ・性能	バグ・脆弱性・パフォーマンスの3観点を並列エージェントとして同時起動する。それぞれに専用プロンプトを渡す
Haiku相当	規約準拠・命名・可読性チェック	linterでは拾えない命名の一貫性・コメントの過不足等を高速にスキャンさせる

多視点並列レビューの実行例

# 司令塔が以下を並列起動する
Agent(正しさ担当): "以下のdiffをバグ・ロジック誤り・境界値の観点のみでレビューせよ。[must]/[should]で分類し、修正案を添えよ。"
Agent(セキュリティ担当): "以下のdiffを入力検証・認証認可・機密情報漏洩の観点のみでレビューせよ。"
Agent(性能担当): "以下のdiffをN+1・不要なループ・キャッシュ機会の観点のみでレビューせよ。"

# 全エージェント完了後、司令塔が統合
- 同一箇所への重複指摘を1件にまとめる
- 優先度(設計>正しさ>テスト>セキュリティ>可読性)で並び替える
- レビューサマリを生成してPRにコメントする

関連スキル

• [[orchestration]] — 多エージェント並列実行の共通原則
• [[testing]] — テストの妥当性確認・テスト戦略
• [[security]] — セキュリティレビューの深掘り
• [[performance-optimization]] — 性能観点の詳細分析
• [[refactoring]] — 指摘を受けてコードを改善する
• [[git-workflow]] — PRの作り方・ブランチ戦略
• [[debugging]] — レビューで発見したバグの調査・修正