AIのレビュー指摘を採用するか — 妥当な指摘と過剰な指摘の見分け方

この記事で扱う問題#

私は個人開発で、Pull Request に AI のコードレビューをかけ、指摘を確認してから人がマージを判断しています（チームの標準として運用した実績は、まだありません）。AI のレビューは、人のレビューより速く、疲れず、差分の隅まで読みます。その一方で、扱いに迷う点もはっきりしてきました。

• どの指摘ももっともらしい：「〜の可能性があります」「〜を検討してください」と、どれも同じ丁寧さで書かれています。重大な指摘と好みの指摘が、文面からは区別できません
• 全部直すと、コードが悪くなることがある：起こり得ない null の確認、握りつぶす try/catch、使われない引数の検証が増え、コードが本当に守っている前提が見えにくくなります
• 全部流すと、本物を見逃す：「どうせいつもの過剰な指摘だろう」と読み飛ばした中に、同時実行の不具合が混ざっていることがあります
• 終わらない：指摘を直して再びレビューにかけると、直した箇所に新しい指摘が付きます。回数を決めずに回すと、いつまでもマージできません

AI の生成コードを人がどうレビューするかは、別の記事で書きました。この記事では向きを逆にして、AI のレビューが出した指摘を、人がどう採否するかを扱います。

判断#

指摘を「失敗の経路」に書き直せるかで分ける#

私は、指摘を受け取ったら、まず次の形に書き直せるかを試します。

どの呼び出し元から、どんな入力・状態のときに、何が誤った結果になるか。

書き直せた指摘は、本物の不具合の候補です。書き直せない指摘（「一般的には〜が望ましい」「将来の拡張に備えて」）は、設計の好みか、起きない条件の話です。そのうえで、直すために足すコードと、それで防げるものを比べます。

指摘の性質	扱い
経路が具体的に書け、要件・整合性・セキュリティに関わる	この PR で直す（Must）
経路は書けるが、前提がコードの別の場所で守られている	直さない。前提がどこにあるかを答え、繰り返されるなら前提を書き残す
仕様として決めていないことを突いている	AI にも自分にも、その場で決めさせない。Spec に戻して決める
既存の設計方針と逆の書き方を勧めている	採用しない
命名・書き方の好み	機械で揃えられるなら lint に任せる。それ以外は流す（May）

重さの付け方は、人のレビューで使う Must / Should / May と同じです。違うのは、人のレビューでは指摘する側が付ける優先度を、AI のレビューでは受け取る側の私が付け直すことです。

例：キャンセルの PR に付いた5つの指摘#

例えば、「未確定の注文をキャンセルできるようにする」PR に、AI のレビューが次の指摘を出したとします。CancelOrder・Order::cancel()・CancelOrderRequest は、生成コードのレビューの記事で既存の形に合わせて直した後の定義です。

1. CancelOrder::execute() の中で OrderCannotBeCancelled を catch し、ログを出して false を返すようにしてはどうか
2. 確定とキャンセルが同時に来ると、後から保存した方が先の更新を上書きする（lost update）。findForUpdate() で悲観ロックを取るべき
3. CancelOrderController が UseCase を RetryOnConcurrentModification で包んでいない。ほかの更新と同時に来ると、キャンセルできる注文でも 409 で失敗する
4. キャンセル済みの注文をもう一度キャンセルすると 409 になる。利用者が再送しただけでもエラーになるが、意図した仕様か
5. CancelOrder は CancelOrderUseCase という名前にしたほうが役割が分かりやすい

1つずつ、経路に書き直してみます。

**1 は採用しません。**経路を書こうとすると、「確定済みの注文をキャンセルしようとすると、例外が呼び出し元に伝わる」になります。これは不具合ではなく、意図した振る舞いです。ドメインの例外は、例外ハンドラで 409 に変換する方針です。ここで握りつぶして false を返すと、呼び出し元ごとに戻り値の確認を書くことになり、確認を忘れた経路では、失敗が成功に見えます。

**2 は、前提が成り立ちません。**経路は「確定とキャンセルが同じ未確定の注文を読み、両方が保存する」と書けます。ただ、その先を追うと、楽観ロックの記事のとおり、OrderRepository::save() が読み込んだ時点の version を条件に UPDATE するので、後から保存した方は OrderWasModifiedConcurrently で失敗し、上書きは起きません。AI のレビューは差分とその周辺を見るので、差分にない save() の実装までは見ていないことがあります。衝突が多いと分かっていない処理に悲観ロックを足しても、待ちが増えるだけです。

ただし、2 を追ったおかげで、3 が本物だと分かります。

3 は採用します。OrderWasModifiedConcurrently は、例外ハンドラで 409（「ほかの人が先に更新しました」）に変換しています。これは、画面で見た内容をもとにする操作の衝突を、利用者に返すためのものです。経路は「同じ注文に明細の追加とキャンセルが同時に来る → 明細の追加が先に保存する → キャンセルの save() が OrderWasModifiedConcurrently を投げ、409 になる」と、具体的に書けます。注文はまだ未確定なので、本当はキャンセルできたはずです。キャンセルの可否は保存されている状態だけで決まるので、利用者に返さずに読み直してやり直せば、相手が確定なら OrderCannotBeCancelled（409）に、明細の追加ならキャンセルの成功になります。直すのに足すコードは数行です。

use Illuminate\Http\JsonResponse;

final class CancelOrderController
{
    public function __construct(
        private CancelOrder $useCase,
        private RetryOnConcurrentModification $retry,
    ) {
    }

    public function __invoke(CancelOrderRequest $request, string $orderId): JsonResponse
    {
        // authorize() で注文の存在と所有者を確かめた後なので、ここに届く ID は実在する注文のもの
        $id = new OrderId($orderId);

        // ほかの更新と保存が衝突したら、読み直してやり直す。相手が確定なら、やり直しで OrderCannotBeCancelled（409）になる
        $this->retry->run(fn () => $this->useCase->execute($id));

        // ConfirmOrderController と同じく 200 で返す（既存のテストは assertOk() で確かめている）
        return response()->json(['id' => $id->value, 'status' => OrderStatus::Cancelled->value]);
    }
}

同時実行の不具合は、Feature Test の正常系・異常系では表に出ません。AI のレビューが一番役に立つのは、こういう「テストが通っていても残る経路」を指してくれるときだと感じています。

**4 は、その場で決めません。**経路は書けますが、「誤った結果」かどうかは、Spec で決めることです。再送でも 409 を返すのか、すでにキャンセル済みなら成功として扱うのか。AI のレビューに従ってコードを直すと、仕様を AI が決めたことになります。Spec に戻って人が決め、決めた振る舞いをテストに書きます。409 のままにすると決めたなら、それもテストに残します。

public function test_キャンセル済みの注文をもう一度キャンセルすると409を返す(): void
{
    $order = OrderModel::factory()->cancelled()->create();   // pending()・confirmed() と同じく、factory に状態を足す前提

    $this->actingAs($order->user)
        ->postJson("/api/orders/{$order->id}/cancel")
        ->assertConflict();
}

テストにしておけば、次に同じ指摘が来たとき、「意図した仕様で、テストに書いてある」と答えられます。

**5 は採用しません。**既存の UseCase は ConfirmOrder のように動詞で名付けています。この PR だけ名前の付け方を変えると、かえって揃わなくなります。命名の規則は、レビューで答えるより、設計方針の文書に書いておきます。

採用しなかった指摘は、理由を1行で残す#

採用しなかった指摘には、その場で1行の理由を返します。2 なら、次のような形です。

採用しない：上書きは起きない。OrderRepository::save() が version を条件に UPDATE し、
後から保存した方は OrderWasModifiedConcurrently になる（同時に来たときの 500 は、指摘 3 で直す）

同じ指摘が別の PR でも繰り返されるなら、返事を書き続けるより、前提をコードの近くに書き残します。2 なら、save() が何を約束しているかを Interface に書きます。人が読んでも、AI のレビューが読んでも、差分の近くで前提が見えるようになります。

interface OrderRepository
{
    public function findOrFail(OrderId $id): Order;

    /** トランザクションの中で呼ぶ。コミットかロールバックまで、同じ注文の findForUpdate() と save() を待たせる */
    public function findForUpdate(OrderId $id): Order;

    /**
     * まだ保存していない注文（version 0）は INSERT する。保存済みの注文は、読み込んだ時点の version が
     * DB と同じときだけ保存し、DB の version を1つ進める（手元の Order の version は進めない）。
     * 先にほかの処理が保存していれば、上書きせずに OrderWasModifiedConcurrently を投げる
     */
    public function save(Order $order): void;
}

何回まで回すか：回数ではなく、止める条件で決める#

指摘を直したら、もう一度レビューにかけます。止めるかどうかは、回数ではなく、出てきた指摘の中身で決めます。

• 止める：新しい回で、具体的な失敗の経路を書ける指摘が出なくなった。残っているのが好みの指摘か、前の回で理由を付けて採用しなかった指摘の繰り返しだけなら、そこで止めます
• 止めて、書き残す：採用しなかった指摘が、回をまたいで言い方を変えて戻ってくる。レビューから前提が見えていないので、上のように前提をコードの近くか設計方針の文書に書きます
• 止めて、Plan に戻る：直した箇所に、毎回新しい Must が出る。私の目安では、同じ箇所で2回続いたら、AI に直させ続けるのをやめます。直し方ではなく、責務の置き場所や処理の順序といった設計の側を疑います
• 止めて、捨てる：指摘が往復する（A に直すと B を勧められ、B に直すと A を勧められる）。どちらでも正しさが変わらない好みの問題なので、既存のコードに近い方を選んで打ち切ります

採用した指摘のうち、影響範囲が限られ、テストで確かめられる修正（3 のような）は、AI に直させてかまいません。仕様を決める修正（4）や、設計の方針に関わる修正は、人に戻します。AI に実装を任せるときと同じ線引きです。

なぜそう判断したか#

全部採用しない理由#

AI のレビューが見ているのは、主に差分とその周辺です。Spec で何を決めたか、この設計にした理由は何か、差分の外のどこで前提が守られているかは、書かれていなければ分かりません。そのため、指摘には「前提を知らなければ正しい」ものが混ざります。それをすべて直すと、守られている前提の上に、もう一度守るコードを重ねることになります。起こり得ない null の確認は、読む人に「ここは null になり得る」と誤った前提を伝えます。足したコードは、将来の変更のたびに読まれ、保守されます。逆に全部流せば、3 のようにテストでは見えない経路を指す指摘まで捨てることになります。

回数で区切らない理由#

「レビューは3回まで」のように回数で区切ると、本物の指摘が4回目に出たときに捨てることになり、逆に1回目で十分なときにも3回回すことになります。止めたいのは、「新しく分かることがなくなったとき」と「直し方ではなく設計に問題があると分かったとき」です。どちらも、回数ではなく、指摘の中身に表れます。

どこから設計過剰になるか#

• 小さな変更：typo の修正や依存パッケージの更新に、指摘を1つずつ経路に書き直す必要はありません。差分の範囲と CI の結果を確かめれば十分です
• 捨てる前提の PoC：確かめたい失敗条件に関わる指摘だけを見ます。設計方針との違いや命名の指摘は、本番に持ち込む段階で考えます
• 採用しなかった指摘を、毎回文書にする：理由の1行で足ります。文書やコードの近くに書き残すのは、同じ指摘が繰り返されたときだけです
• 指摘の採否を別の AI に判定させる：見落としを減らす手段としては考えられますが、採否は人が決めます。判定する側も、Spec と設計の理由を知らない点は同じだからです
• セキュリティの指摘を経路だけで退ける：認可や入力の扱いに関する指摘は、経路をすぐに書けなくても、それだけでは退けません。確かめるまで保留にします

まとめ#

• AI のレビューの指摘は、「どこから・どんな入力で・何が誤るか」に書き直し、書けたものだけを直す候補にする
• 前提が別の場所で守られている指摘は理由を1行で返し、繰り返されたら前提をコードの近くに書き残す。仕様の問いは Spec に戻して人が決める
• レビューは回数ではなく、新しい経路が出なくなったら止める。同じ箇所に Must が出続けたら、直し方ではなく設計を疑う