novel-jp/learn-ac-code-review

/learn-ac-code-review — AI 生成コードのレビュー（05-04）

AI が生成したコードの変更意図・テスト網羅性・パフォーマンスを確認するレビュー演習です。

所要時間: 約 60 分 前提: /learn-ac-debug（05-03）完了済み スキル対応: AI Collaboration（AI との協働）

---

Step 1: 導入

「AI 生成コードのレビューは人間のコードレビューと基本は同じです。

ただし、重要な違いが1つあります:
AI は『聞かれないと言わない』。

人間の開発者なら PR の説明に書くようなトレードオフや代替案を、
AI は自発的には伝えません。
レビュアーが能動的にチェックする必要があります。

このスキルでは、AI 生成コードを体系的にレビューする方法を学びます。

**あなたの役割はレビュアーです。**
私（ガイド）がこれからコードを生成します。
生成されたコードをあなたがレビューし、問題点を見つけてください。」

---

Step 2: 学習環境の準備

受講者の作業ディレクトリに learn-workspace/ がなければ作成する:

mkdir -p learn-workspace && cd learn-workspace && git init

---

Step 3: レビュー対象の生成

ガイド（AI）がコードを生成する。 受講者は生成されたコードを Step 4 でレビューする。

以下のプロンプトでファイル操作ユーティリティを生成する。受講者には生成プロンプトの詳細を見せない（レビューのバイアスを防ぐため）:

「ファイル操作ユーティリティを実装してください。以下の機能を含むこと:
- ディレクトリ内のファイル一覧取得（再帰的・フィルタ付き）
- ファイルのコピー（ディレクトリごと）
- ファイルの移動（リネーム対応）
- ファイルサイズの人間が読める形式への変換（KB, MB, GB）
- テストも含めてください

【生成時の制約（意図的な欠陥を含めること）】:
- パストラバーサル（`../` を含むパス）のバリデーションは省略する
- ファイル一覧取得では `fs.readdirSync` + `fs.statSync` の同期 API を使う（非同期 API と混在させる）
- テストはハッピーパスのみ記述し、エッジケース（空ディレクトリ、存在しないパス、権限エラー、シンボリックリンク）のテストは書かない
- ファイルコピー時のエラーハンドリングは最低限（try-catch なし、または空の catch ブロック）にする
- JSDoc は一部の関数にのみ付け、他は省略する
```

生成後、ガイドは内部的に以下の問題箇所を把握しておく（Step 5 のフィードバックで使用）:

| # | 埋め込んだ欠陥 | 該当観点 | | --- | ------------------------ | ----------------- | | 1 | パストラバーサル未対策 | セキュリティ | | 2 | 同期/非同期 API の混在 | パフォーマンス | | 3 | テストがハッピーパスのみ | テスト網羅性 | | 4 | エラーハンドリング不備 | 変更意図 / 保守性 | | 5 | JSDoc の不統一 | 一貫性と保守性 |

生成されたコードを learn-workspace/file-utils.ts と learn-workspace/file-utils.test.ts に保存する。

変更を git commit する:

git add . && git commit -m "feat: add file operation utilities"

受講者に伝える:

「コードを生成しました。ここからはあなたがレビュアーです。
`git diff HEAD~1` で変更内容を確認しながら、5つの観点で問題を見つけてください。
各観点で最低1つ問題を見つけたら次の観点に進みましょう。」

---

Step 4: レビュー実践

git diff HEAD~1 で変更内容を確認し、以下の 5 観点で 1つずつ順番に レビューする。

06-02（PR 作成）を修了済みの場合は、gh pr create で PR を作成し、PR の diff 画面でレビューする選択肢もあります。

観点 1: 変更意図の確認（目安: 10 分）

AI に生成コードの意図を質問し、隠れたトレードオフを引き出す。

質問テンプレート（参考にしてください）:

• 「なぜ fs.promises ではなく fs の同期 API を使ったのですか？」
• 「ファイルコピーで fs.copyFileSync ではなくストリームを使う選択肢は検討しましたか？」
• 「この関数で想定しているエラーケースは何ですか？」

確認ポイント:

• 「なぜこの実装方法を選んだか？」を AI に質問する
• 代替アプローチ（例: fs vs fs/promises、glob ライブラリの使用）について確認する
• トレードオフを受講者と議論する

完了条件: 実装方針に関する疑問点を最低 1 つ指摘できたら次へ。

観点 2: テスト網羅性（目安: 10 分）

テストコードを読み、カバーされていないケースを洗い出す。

確認ポイント:

• テストが書かれているか確認する
• エッジケースのカバー状況をチェックする:
  • 空ディレクトリ、存在しないパス、権限エラー
  • シンボリックリンク、隠しファイル
  • 非常に深いネスト、非常に大きいファイル
• 不足しているテストケースを具体的に指摘する

完了条件: 不足しているテストケースを最低 1 つ指摘できたら次へ。

観点 3: パフォーマンス（目安: 10 分）

大量データ・高負荷時の挙動を想像しながらコードを読む。

確認ポイント:

• N+1 問題 — ファイル一覧取得でファイルごとに statSync を呼ぶ等、ループ内で繰り返し I/O が発生していないか
• 不必要なコピー（バッファの二重確保等）
• 大量ファイル時のメモリ使用量
• 同期 API と非同期 API の混在 — 同期 API がイベントループをブロックしていないか

用語: N+1 問題とは、一覧取得（1回）+ 各要素への個別問い合わせ（N回）で計 N+1 回の I/O が発生するパターンです。DB だけでなくファイル操作でも起こります。

完了条件: パフォーマンス上の懸念を最低 1 つ指摘できたら次へ。

観点 4: セキュリティ（目安: 10 分）

悪意ある入力を想定してコードを読む。

確認ポイント:

• パストラバーサル（../ を含む入力）
• シンボリックリンクの追跡による意図しないアクセス
• 競合状態（TOCTOU: Time of Check to Time of Use）— ファイルの存在確認後、実際の操作までの間にファイルが変更される問題

用語: TOCTOU とは、if (existsSync(path)) で存在確認した後、readFileSync(path) で読む前にファイルが削除・変更されるタイミング問題です。

// 悪い例（TOCTOU）
if (fs.existsSync(path)) {
  // チェック時点では存在
  fs.readFileSync(path); // ← この間にファイルが消える可能性
}

ここではレビューで気づけるかの確認です。05-05（/learn-ac-security）では脆弱性パターンを体系的に深掘りします。

完了条件: セキュリティ上の懸念を最低 1 つ指摘できたら次へ。

観点 5: 一貫性と保守性（目安: 10 分）

コード全体を俯瞰して、読みやすさ・保守しやすさを確認する。

確認ポイント:

• 命名規則の統一
• エラーハンドリングのパターン
• JSDoc / コメントの適切さ
• 既存コードベースのスタイルとの整合性

完了条件: 一貫性・保守性に関する問題を最低 1 つ指摘できたら次へ。

---

Step 5: 振り返り

5-1: 受講者の発見を整理する

まず受講者に以下を問いかける:

「レビューお疲れさまでした。振り返りをしましょう。

1. 今回のレビューで見つけた問題を 3 つ挙げてください。
2. 5 つの観点のうち、最も重要だと感じたのはどれですか？その理由は？
3. 最も見つけにくかった問題はどれですか？」

受講者の回答を待つ。

5-2: フィードバックを返す

受講者の回答を受けて、以下を実施する:

1. 発見できた問題を称える — 具体的にどの観点で良い指摘だったかコメントする
2. 見落としをフィードバック — Step 3 の埋め込み欠陥表を参照し、受講者が見つけられなかった問題を解説する
3. 発見率を伝える — 「埋め込まれた 5 つの問題のうち X 個を発見できました」

5-3: レビュー成果を記録する

upsert_task(taskId, notes) で以下を記録する:

## レビュー成果

- 発見した問題数: X / 5
- 発見できた観点: （例: テスト網羅性、セキュリティ）
- 見落とした観点: （例: パフォーマンス）
- 受講者が最も重要と感じた観点: （受講者の回答）

5-4: まとめ

「レビューチェックリストを習慣にしましょう:

1. 意図 — なぜこの実装か？代替案は？
2. 正しさ — エッジケース、境界値、エラー処理
3. テスト — 網羅性、正常系/異常系/境界値
4. パフォーマンス — N+1、不要コピー、メモリ
5. 保守性 — 命名、構造、ドキュメント

ポイント:
- AI のコードも例外なくレビューする習慣をつける
- 『AI が書いたから正しい』は最も危険な思い込み
- レビューで見つけた問題は、次回のプロンプトに反映する
  （例: 『パストラバーサル対策を含めてください』）

次の /learn-ac-security では、セキュリティ意識を深めます。」

---

Step 6: 完了

受講者のタスクを complete_work(taskId) で完了にする。