連載「AIにコードを書かせる技術 — 品質とセキュリティのガードレール設計」第 3 回。
第 1 回は規約を 5 層に落とす話、第 2 回は「テスト全件 PASS」を成立させるテスト設計でした。
今回は、出てきた PR をレビューする側の設計です。
この記事のゴール
第 1 回と第 2 回で、規約・権限・ローカルゲート・CI ゲートを積みました。全部緑になった PR が出てきます。ここで問題になるのは、その PR をどう受け取るかです。
レビューは最後の砦ですが、人間のレビュー能力は差分の量に比例しません。60 ファイルの差分を渡されたら、丁寧に読めるのは最初の数ファイルです。「全部を注意深く読む」は運用として成立しないので、先に決めておきます。
- 何を機械に任せるか
- どこから先は人が必ず介在するか
- 人が介在するとき、何を、どの順番で見るか
この記事はその線引きの話です。
| レビューで起きること | 対処 | 本記事 |
|---|---|---|
| 差分が再生成物で埋まって読めない | 機械側で差分から除外する | §2 |
| 何を・なぜ変えたのか PR から分からない | PR 本文の必須項目を決める | §2 |
| 「CI が緑だから大丈夫」で読み飛ばす | ゲートが保証していないものを明示する | §3 |
| テストが静かに弱められる | 人が最初に見るのはテストの差分にする | §4 |
| 実装は動くが要件と違う | 仕様書との突き合わせを人の担当に固定する | §5 |
| 生成量が読解速度を超え、全行レビューが破綻する | 生成物の種別ごとに読む深さを決め、未読を明示する | §6 |
| テストは緑だが画面が崩れている | 証跡画像を目視する | §7 |
| 指摘したのに意図と違う修正が返る | 指摘を判定可能な形で書く | §8 |
| 「全部緑なので自動マージ」 | Resolve とマージは人が閉じる | §9 |
全体像
機械のゲートは「読む価値のある PR だけを人に渡す」ためのフィルタです。品質を保証するものではありません。ここを取り違えると、緑を根拠にレビューが形骸化します。
1. 線の引き方 — 3 つの基準
自動化の範囲は、次の 3 つで決めています。1 つでも「人」側に該当したら、人が介在します。
| 基準 | 機械に任せてよい | 人が必ず介在する |
|---|---|---|
| 判定に仕様の解釈が要るか | 要らない(形式・数値・存在の有無) | 要る(要件との一致、命名の妥当性、責務の置き場所) |
| 間違ったときに気づけるか | 誤判定しても後段のテスト・証跡で追える | 誤ると本番まで誰も気づかない |
| 取り消せるか | 取り消せる(commit 前・push 前に止まる) | 取り消しにくい(マージ、公開、データ移行、外部通知) |
この基準で分けると、こうなります。
機械: 曖昧語の検出 / 秘密情報の混入 / テストの成否 / カバレッジ下限 / 証跡の有無 /
設定がコンテナ定義まで配線されているか / 依存の既知脆弱性
人 : 要件と実装の一致 / テストが弱められていないか / 設計判断 / 画面の見た目 /
指摘を解決済みにしてよいか / マージしてよいか
判断は返さない
自動化を進めるときの原則を 1 つ決めています。機械がやるのは「止める」か「通す」までで、「これでよい」と決めるのは人です。
たとえばレビュー指摘への修正を自動で走らせることは技術的には可能ですが、その修正でよいかどうかの判断(GitHub でいう Resolve conversation)は人が閉じます。指摘した本人以外に「解決した」と決められると、指摘は静かに消えます。この線を越えないことだけ決めておけば、その手前の自動化はどこまで進めても安全です。
本記事では、この線の手前側(レビューの準備と、人が見る順番)を扱います。
2. レビューが始まる前に負荷は決まっている
人が読む前の段階で、読みやすさの大半は決まります。ここは機械側の仕事です。
2.1 再生成物を差分に入れない
E2E を回すとスクリーンショットが毎回撮り直され、中身が同じでもバイト列が変わって全部が差分になります。数十枚混ざった時点で、差分を追うのは物理的に無理になります。
第 1 回で触れた設定で、コミット前に base 側へ戻します。
# 毎回再生成されるバイナリを commit 前に base 状態へ戻す。
# これを指定しないと大量のバイナリ差分が PR に混入する。
restore-paths: |
docs/screenshots/
証跡そのものは第 2 回で作った tests/results/ に残り、そちらは .gitignore 対象です。**「証跡は残すが、差分には出さない」**を両立させます。
2.2 1 PR = 1 目的にする
- **新規実装ブランチは main から切る**: 必ず main 同期直後に `git switch -c <new-branch>` で切る。
他の feature ブランチからの派生は禁止 (= 別 PR の差分が混入し、レビュー範囲が曖昧になるのを防ぐ)。
- **着手前の main 同期**: 着手前に必ず `git fetch && git switch main && git pull --ff-only`
で main を最新化、開発ブランチへ `git merge origin/main`。既存ブランチ再開でも省略しない。
- **PR コメント追従は同一ブランチに追加コミット**: 新しいブランチは切らない。
前のタスクのブランチに残ったまま次を始めると、そこから派生した PR ができ、レビュー範囲に別 PR の差分が混ざります。指摘しても作り直しになるので、規約で先に塞ぎます。
3 つ目も実務的に効きます。レビュー対応のたびに新しい PR が生えると、どの指摘がどこで直ったのかを人間が突き合わせる作業が発生します。
2.3 PR 本文に「何を・なぜ」と証拠を必須化する
PR 本文には **「何を・なぜ変えたか」の説明**と、**変更箇所のエビデンスを画像で**必ず含める。
- **変更説明**: 概要 / 採用した実装方針 (確認事項への回答があればその対応表) /
主要な変更ファイルの役割を箇条書きで書く。
- **エビデンス画像 (必須)**: 画面に関わる変更は **実画面のスクリーンショット**を貼る。
E2E が自動取得する操作実行前/後の証跡画像と、各ステップの画像を流用するのが基本。
画面を伴わない変更 (純ロジック等) は、テスト結果や図など変更を裏づける画像を貼る。
ここで効いているのは、貼るものが既に手元にあることです。証跡は第 2 回の仕組みが E2E の副作用として生成しているので、貼るコストはほぼゼロになります。「エビデンスを貼れ」という規約が守られるかは、貼るものを用意してあるかで決まります。
private リポジトリでは画像がインライン表示されない
これは実際に踏んだ罠です。
- 画像は **リポジトリ内 (`docs/evidence/<識別子>/`) にコミット**し、同ディレクトリの
`README.md` から **相対パス** (``) で表示する。
- 理由: **private リポジトリでは raw URL のインライン埋め込み (Camo 経由) が表示されない**
(画像が割れる)。リポジトリ内にコミットして相対パスで参照すれば、GitHub のファイル表示・
PR の Files changed タブで確実に表示される。
 は public では表示され、private では割れます。環境によって静かに壊れる書き方は、置き場所ごと規約で指定するのが早いです。
3. ゲートが保証していないものを明示する
レビューが形骸化する最大の原因は、「CI が緑だから大丈夫」です。緑は「手続きを踏んだ」であって「正しい」ではありません。 第 1 回・第 2 回で作ったゲートを、保証する/しないで棚卸しします。
| ゲート | 保証すること | 保証しないこと |
|---|---|---|
権限(deny) |
破壊的操作が実行されていない | 実装方針の妥当性 |
| pre-commit | 秘密情報が入っていない / 先頭コメントがある | コメントの内容が正しいか |
静的検証(TC-MAC) |
構成・設定が壊れていない / 設定がコンテナ定義まで配線されている | その設定値が業務要件に合っているか |
ユニット + カバレッジ下限(TC-UT) |
ロジックが実行され、退行していない | 期待値が仕様どおりか |
機能・シナリオ + 証跡(TC-UI / TC-SCN) |
画面が動き、テストが実際に走った | 見た目の崩れ / 要件との一致 |
右列が、人が見る対象そのものです。特に **「期待値が仕様どおりか」**は機械にはどうやっても判定できません。テストは「そうあるべき仕様」を人が書いたものなので、その仕様が正しいかを検証できるのは仕様を決めた人だけです。
4. 人が最初に見るのは「テストの差分」
実装差分から読み始めると、たいてい最後まで体力がもちません。最初に見るのはテストの差分にしています。
4.1 なぜテストからか
第 2 回で、テストを弱める変更を名指しで禁止しました。
| ❌ テストケースの削除 | ❌ `test.skip()` / コメントアウト |
| ❌ 期待値を実際の出力に合わせて書き換える |
| ❌ アサーションを緩める (`toBe` → `toBeTruthy`、`toEqual` → `toMatchObject`、
全列照合 → 件数のみ、値の確認 → 存在の確認) |
| ❌ 時刻の照合を諦めて比較対象外にする |
| ❌ カバレッジ下限を下げる / 計測対象から除外する |
| ❌ タイムアウトを伸ばして通す |
このうち機械で拾えるのは一部だけです。
| 弱化の形 | 機械で拾えるか |
|---|---|
| カバレッジ下限を下げる | ✅ 拾える(下限判定・更新漏れ判定) |
| 計測対象から除外する | 🔺 一部(未計測モジュール一覧との突き合わせが要る) |
| テストケースの削除 / skip | 🔺 件数の減少としては見えるが、足しながら消すと相殺される |
| アサーションを緩める | ❌ 拾えない。toEqual → toMatchObject は緑のまま通る |
| 期待値を実測に合わせて書き換える | ❌ 拾えない。テストは緑になる |
下 2 つは、差分を人が読む以外に検出手段がありません。そして第 2 回で書いたとおり、弱化は「要件が黙って消える」形で効いてきます。レビューで最も価値が高い作業がここです。
4.2 実務
テストの差分だけを先に開きます。
# テスト関連の差分だけを先に読む
git diff origin/main... -- 'tests/' '*.spec.ts' '*.test.ts'
# 削除された行だけを拾う (= 消えた要件を探す)
git diff origin/main... -- 'tests/' | grep -E '^-[^-]'
見るポイントは 4 つです。
- 削除行: 消えたケース・消えたアサーションはあるか。あるなら PR 本文に理由が書かれているか
-
緩んだ比較:
toEqual→toMatchObject、配列一致 → 件数のみ、値 → 存在 -
除外の追加: カバレッジ除外、比較除外列、
test.skip - バグ修正 PR なのにテストが増えていない: 再発を捕まえるテストが無ければ、同じ不具合がまた来ます
4 つ目は次回の主題なので、ここでは「テストが増えていない修正 PR は差し戻す」とだけ決めておきます。
5. 次に見るのは「要件との一致」
テストの差分の次は、PR 本文の変更説明と、仕様書・Issue の突き合わせです。実装差分より先にここを見ます。
見るのは 3 点です。
| 見るもの | 差し戻す条件 |
|---|---|
| 変更説明と要件の対応 | PR 本文に書かれた方針が、要件・仕様書のどこにも根拠を持たない |
| 仕様変更の順序 | 仕様が変わっているのに、仕様書が更新されていない(第 2 回の「仕様書 → テスト → 実装」に反する) |
| 曖昧だった箇所の扱い | 判断が必要な箇所を推測で埋めている(質問として残っていない) |
2 番目が重要です。第 2 回で「要件を変えるなら、テストではなく仕様書から直す」と決めました。この順序が守られたかを検証できるのは、PR の差分に仕様書が含まれているかどうかです。実装とテストだけが動いていて仕様書が動いていない PR は、仕様変更を実装で追認している可能性があります。
実装そのものの差分(設計判断・責務の置き場所・命名)は、この後に読みます。ここまでで差し戻す PR は、実装を読む前に返せます。
6. ソース本体はどこまで読むのか
ここが一番議論になるところです。「AI が書いたコードを人が全部読む必要があるのか」。
先に結論を書くと、全行読むのは前提として成立しません。実装速度が人の読解速度を超えるからです。1 日に数千行が出てくる状況で「全部読む」と宣言すると、実態は「最初の数ファイルだけ読んで残りは眺める」になります。この状態が一番危険で、どこを読んでいないのかが誰にも分からなくなります。
なので、読む場所と読まない場所を先に決めて、読まないことを明示する運用にしています。
6.1 生成物の種別ごとの要否
読む深さは 3 段階です。🔴 全行読む / 🟡 構造だけ見る / 🟢 読まない。
| 生成物の種別 | 要否 | 人が見るもの |
|---|---|---|
| 認証・認可(トークン検証、権限判定、ガード) | 🔴 全行 | 判定の抜け、デフォルト許可になっていないか、サーバ側で判定しているか |
| DB スキーマ / マイグレーション | 🔴 全行 | 破壊的変更、ロールバック手段、既存データの移行漏れ |
| 秘密情報の取り扱い(環境変数、ログ出力、API 応答) | 🔴 全行 | 応答・ログ・URL に漏れていないか |
| 金額・数量・日付の計算 | 🔴 全行 | 端数処理、境界、タイムゾーン。テストの想定自体が誤っている可能性を潰す |
| 削除・一括更新の処理 | 🔴 全行 | 条件の抜け(全件削除になる経路)、取り消し可能性 |
| 外部への送信(通知、メール、外部 API) | 🔴 全行 | 宛先、送信条件、リトライで多重送信しないか |
| ユニットテストコード | 🔴 全行 | §4 のとおり。アサーション緩和と期待値の書き換えは機械で拾えない |
| E2E / シナリオテストコード | 🔴 全行 | 検証の粒度、エラー確認で終わっていないか、reload() で経路を避けていないか |
| CI ワークフロー / 運用スクリプト | 🔴 全行 | 権限、破壊的コマンド、ゲートを迂回していないか |
依存の追加(package.json 等) |
🔴 全行 | 追加された依存そのもの(点数は少ないが影響が大きい) |
| 仕様書・規約の差分 | 🔴 全行 | それが要件そのもの。実装より先に読む(§5) |
| API ハンドラ(入出力・バリデーション) | 🟡 構造 | 入出力の形、エラー分岐の有無。値の妥当性はテストに委ねる |
| 状態を持つ画面コンポーネント | 🟡 構造 | 状態遷移と副作用の位置。表示の細部は証跡画像で見る(§7) |
| ビジネスロジック(金額以外) | 🟡 構造 | 責務の置き場所、分岐の網羅。値はテストの期待値で見る |
| エラー処理 | 🟡 構造 | 握り潰していないか、種別が区別されているか |
| 表示専用コンポーネント | 🟢 不要 | 崩れは証跡画像で落ちる |
| 定型の結線(ルート登録、DI、re-export) | 🟢 不要 | 壊れれば起動時・実行時テストで落ちる |
| 期待値が完全一致で固定された純ロジック | 🟢 不要 | テストが値まで凍結している(第 2 回 §5) |
設定ファイルの機械的追加(.env.example のキー等) |
🟢 不要 | 配線検査の静的テストが担保する(第 2 回 §10.2) |
| 自動再生成物(lock ファイル、スクリーンショット、ビルド成果物) | 🟢 不要 | そもそも差分に出さない(§2.1) |
🔴 に並んだものは、第 1 回で deny を書いた対象・第 2 回で OWASP の網羅表を作った対象と重なります。連載を通して同じ場所が繰り返し出てくるのは偶然ではなく、そこが「取り消せない」領域だからです。
6.2 判断基準 — なぜその要否になるのか
上の表は、次の 3 つの問いで機械的に決めています。新しい種別が出てきたら、この順に当てはめます。
| # | 問い | 「はい」なら |
|---|---|---|
| 1 | 壊れたときに取り消せないか(データ消失 / 外部送信 / 秘密の露出 / 課金) | 🔴 全行 |
| 2 | 壊れてもテスト・証跡で気づけないか(本番でしか通らない経路、テストの想定自体が誤りうる箇所) | 🔴 全行 |
| 3 | 判定に仕様の解釈が要るか(責務の置き場所、状態遷移の妥当性) | 🟡 構造 |
| — | 1〜3 がすべて「いいえ」 | 🟢 不要 |
§1 の線引き基準と同じ 3 つです。自動化の線引きと、ソースを読む深さの線引きは同じ軸で決まります。
テストコードが 🔴 なのは、問い 2 に該当するからです。テストを弱める変更はテスト自身では検出できない(緩めたテストも緑になる)ので、気づける仕組みが人の目しかありません。
6.3 「読まない」を選んでよい条件
🟢 に置けるのは、次の 3 つがすべて成り立つときだけです。
- その振る舞いがテストで値まで固定されている — 存在確認や部分一致ではなく、全項目の完全一致(第 2 回 §5)。ここが緩いと、テストは「読まない理由」になりません
- 壊れたときにテストか証跡で気づける — 落ちる仕組みがあるか。本番でしか出ない経路なら読む側に倒す
- 影響が取り消せる — データを壊さない / 外部に出さない / 秘密を露出しない
3 つ揃わないなら 1 段上げます。逆に言えば、テストの期待値が甘い場所は、自動的にレビューコストとして跳ね返ってきます。第 2 回で期待値を全項目一致にしたのは、ここで「読まなくてよい」と言うためでもあります。
6.4 無言の未読を禁止する
読まない選択自体は許容しますが、黙って読まないのは禁止です。
読まないと判断した範囲は、レビューコメントか PR の会話に残す。
例: 「`components/*View.tsx` は表示専用のため差分は構造のみ確認。値は TC-UI-30〜35 に委ねた」
第 2 回で決めた「無言の除外禁止」(カバレッジ除外は理由を書く)と同じ形です。未読の範囲が明示されていれば、後から不具合が出たときに「そこは読んでいなかった」と分かります。 何も書かれていないと、「読んだのに見落とした」のか「読んでいなかった」のかが区別できず、次に何を直せばいいかが決まりません。
6.5 読むコストは、書かせる側で下げられる
レビューが重いとき、レビュー側だけで解決しようとすると破綻します。読みやすさは生成物の側の要件として規約に入れておきます。
| 規約 | レビューでの効き方 |
|---|---|
| why を書く(第 1 回のコメント規約) | 「なぜこの実装なのか」がコードにあるので、意図を推測する時間が消える |
| 1 PR = 1 目的(§2.2) | 読む文脈が 1 つに固定される |
命名規約・data-testid(第 2 回) |
差分の意味が名前から読める |
| 仕様書を同時更新(第 2 回) | 実装差分を読む前に、仕様差分で意図を確認できる |
つまり、レビューを軽くする作業のほとんどは、レビューより前の段階に置けます。
6.6 量が増えたら、単位を「行」から「境界」へ上げる
生成量が増えたときの現実的な対処は、読む粒度を上げることです。
行を読む → 差分の 1 行 1 行を追う(量が増えると破綻する)
境界を読む → インターフェース / DB スキーマ / 権限判定 / 外部 I/O の形だけを見る
境界が正しければ、中身は差し替え可能です。逆に境界が間違っていると、中身がどれだけ綺麗でも作り直しになります。レビューの時間を境界に集中させるほうが、同じ工数で防げる事故が増えます。
そして境界は、行数が増えてもあまり増えません。ここが「全行読む」との決定的な違いです。
7. 画面はエビデンス画像で目視する
- **レイアウト変更時はエビデンス画像で目視確認**: テスト PASS の後に
UI スクリーンショットを開き、文字の重なり / 圧縮 / はみ出しを点検。
「テストが緑」と「画面が壊れていない」は別です。 要素の存在と文言を完全一致で検証しても、重なり・はみ出し・折り返しの崩れは緑のまま通ります。第 2 回で撮っている証跡(各テストの最終状態、シナリオの各ステップ)をそのまま開けば済むので、ここは今のところ人間の目が最も安い手段です。
逆に言えば、証跡を撮る仕組みが無いと、この確認は「手元で動かす」ことになり、実質的に誰もやらなくなります。
8. 指摘は「判定可能な形」で書く
ここは人間側の作法の話です。第 1 回で規約から曖昧語を排除しましたが、同じことがレビューコメントにも当てはまります。
| ❌ 曖昧な指摘 | ✅ 判定可能な指摘 |
|---|---|
| ここ、適切にエラーハンドリングして |
catch で握り潰さず、ValidationError を投げて code='SEATS_OUT_OF_RANGE' を付ける。呼び出し側の分岐も合わせる |
| 表示がおかしい |
total-amount の表記を ¥12,300 形式(3 桁区切り + 円記号前置)に。TC-UI-12 の期待値も同じコミットで直す |
| テストが足りない | 境界値(最小 / 最大 / 最大 +1)をユニットで追加。機能テストには足さない(第 2 回のピラミッド原則) |
| 他にも同じ箇所がありそう |
grep -rn "formatYen" src/ で出る 4 箇所すべてを同じ形に。直さない箇所があれば理由を返信に書く |
指摘は仕様の書き換えです。 曖昧に書けば曖昧な修正が返り、2 往復目が発生します。「どこまでやるか」(この 1 箇所か、類似箇所も含むか)まで書くと、往復が 1 回で終わります。
これは相手が人でも同じですが、AI が実装する前提では、指摘の曖昧さがそのまま修正の曖昧さになるぶん、効き方が大きくなります。第 1 回で「規約に曖昧語を書くな」と決めたのと同じ原則を、レビューコメントにも適用するだけです。
9. Resolve とマージは人が閉じる
**Q. 自動マージはしないのか?**
A. しない。書かれたコードは必ず人間がレビューしてマージする運用。
- **`PR を自動マージしない`**。`gh pr merge --auto` 系のオプションは禁止
ここまで積んでおきながら自動マージしない理由は、§3 の表に尽きます。機械が検証しているのは「手続きを踏んだか」で、「要件を満たしたか」ではありません。 テストが全件 PASS し、カバレッジも下限を満たし、証跡もある PR が、要件と違うものを作っていることは普通にあります。
同じ理由で、指摘の Resolve は指摘した人が閉じます。「直したから解決済み」を修正側が宣言できると、レビュアが確認する前に会話が畳まれ、畳まれた指摘は二度と目に入りません。
この記事で作ったのは「レビュアが見るべきものだけを見られる状態」であって、レビュアを消す仕組みではありません。
10. 導入チェックリスト
- 自動化の線引きを 3 基準(仕様の解釈が要るか / 間違いに気づけるか / 取り消せるか)で決める
-
再生成物を差分から外す(
restore-paths) - 「他ブランチからの派生禁止」「レビュー対応は同一ブランチ」を規約に書く
- PR 本文に 変更説明 + エビデンス画像を必須化する(private リポジトリなら置き場所も指定)
- ゲートが保証しないものを一覧化し、レビュー観点として明文化する
- レビューは テストの差分から読む。削除行を最初に見る
- 仕様変更を伴う PR は、仕様書の差分が含まれているかを確認する
- ソースを読む深さを 3 段階(全行 / 構造のみ / 読まない)で決める。全行読む対象を先に列挙しておく
- 「読まない」の 3 条件(値まで固定されたテスト / 落ちる仕組み / 取り消せる影響)を規約に書く
- 未読の範囲を PR に明記する(無言の未読を禁止する)
- レイアウト変更は証跡画像で目視する
- レビューコメントに曖昧語を使わない。「どこまでやるか」を書く
- Resolve は指摘した人が閉じる。自動マージを禁止する
まとめ
- 機械のゲートは「読む価値のある PR だけを人に渡す」ためのもの。品質の保証ではない
- 線引きは 3 基準 —仕様の解釈が要るか / 間違いに気づけるか / 取り消せるか
- 自動化はどこまで進めてもよいが、判断は返さない(Resolve とマージは人が閉じる)
- 人が最初に見るのは テストの差分。アサーションの緩和と期待値の書き換えは、機械では拾えない
- 次に見るのは 要件との一致。仕様書が動いていない仕様変更を通さない
- ソースは 種別ごとに読む深さを決める(🔴 全行 / 🟡 構造 / 🟢 読まない)。「全部読む」と言って読めないより、読まない範囲を宣言するほうが安全
第 1 回で規約を機械に落とし、第 2 回でテストの緑を信用できるようにしました。その 2 つがあって初めて、レビューを「全部読む作業」から「決めた順に見る作業」に降ろせます。
次回
第 4 回は、不具合を回帰テストに変える話を書く予定です。ここまでのゲートを全部通したのに本番で不具合が出たとき、報告された 1 件を直して終わりにしないための手順 —「類似確認 → 修正 + テスト追加 → なぜ検知できなかったかの言語化」を扱います。エラー系テストが検知力を持たない典型(「エラーが出た」で確認を終えている)にも触れます。
筆者について
個人開発で PWA・API・IaC 周りを触りつつ、AI エージェントに実装を任せるための足場(規約・CI・自動化)を整えるのが最近の関心です。この記事で紹介した規約とレビュー運用は、テンプレートリポジトリとして手元で運用しています。
- GitHub: @KenichiOsakada
「うちではこの線で引いている」という運用があれば、コメントで教えてもらえると嬉しいです。