PRの影響範囲をコードグラフで出してみた

結論

コードグラフと「PR でテストが変わったか」を組み合わせると、PR の影響で危ない場所が分かりそうです。

コードグラフだけでは、影響が届く候補が出るところまでです。どれが本当に壊れるかは分かりません。そこにテストの変更を重ねると、「影響が届いたのに、誰もテストで確かめていない場所」が残ります。実際に後から直された取りこぼしで試すと、この残った場所が、後でバグが出たパッケージそのものでした。

試せたのはまだ1件なので、「分かる」とまでは言えません。ただ、人がレビューで見に行く場所を決める手がかりにはなります。

前回の記事では、Alibaba のコードレビューツール open-code-review のソースを呼び出しグラフにして描きました。今回はその続きで、同じグラフを実際のプルリクエスト(PR)に当てて、変更の影響を出せるか試しました。

試し方

答え合わせができるように、「前の PR で対応が漏れていた」と本文に書かれている修正 PR を探しました。漏れの原因になった PR の時点でグラフを作り、後で直された場所をグラフが指していたかを見ます。

題材にしたのは PR #1248 です。トークン予算の上限を、レビューの途中でも毎ラウンド確認するようにした変更で、LLM との対話ループ RunMainTask に、予算超過で止まる新しい停止理由を足しています。

RunMainTask を呼んでいる場所は2つあります。差分レビュー(review)側の agent.executeGroupSubtask と、ファイル単位の検査(scan)側の scan.executeSubtask です。#1248 は review 側だけ新しい停止理由を受け取るようにし、scan 側は漏れたままマージされました。約1か月後の #1745 で scan 側が直されるまで、ocr scan は予算で止まっても budget_exceeded: false と報告していました。

グラフには Go 公式の callgraph を使いました。Go の型を解決してから呼び出し先を決めるので、a.runner.RunMainTask(...) のようなメソッド呼び出しも取れます。前回比べた graphify は構文だけで解析するため、このメソッド呼び出しへの線が1本もありませんでした。

影響が届く範囲は出せるか

PR で変わった関数を差分の行番号から決め、その関数を直接呼んでいるのに PR では触っていない関数を、グラフを逆向きにたどって出しました。#1248 では6関数です。

6関数の中に、後で直された scan.Agent.executeSubtask が入っていました。変更された RunMainTask と MainLoopStop.Reason の両方を、1ホップで呼んでいる関数です。

#1248 には新しいテストがあり、CI も通っていました。lint も漏れた場所を指摘していません。scan 側は停止理由を if !completed でまとめて受けていて、理由が1つ増えてもコンパイルは通ります。テストが足されたのは review 側と対話ループ側だけで、scan 側を通るテストは変わっていませんでした。変更がどこへ届くかを出せたのは、グラフだけでした。

影響度は出せるか

ただし、グラフが出すのは候補です。6関数のうち、本当に直す必要があったのは scan.Agent.executeSubtask だけでした。グラフからは、どれが壊れるかまでは分かりません。

そこで、テストの変更と突き合わせました。6関数と変更した関数が属するパッケージごとに、PR の中でテストファイルが変わったかを見ます。

作者がテストで確かめたのは agent と llmloop で、scan と cmd は確かめていません。この2つの片方が、後でバグが出た internal/scan でした。影響度そのものは出せませんが、「影響が届いたのに誰も確かめていない場所」までは絞れます。

出せないケース

同じ形の組をほかにも試すと、グラフでは出ないケースがありました。

PR #1680 は、変更一覧に表示するファイルパスを d.NewPath から effectivePath(d) に変えました。ところが、ファイルを読むツールに渡す対応表を作る injectDiffMap は d.NewPath のままで、削除されたファイルを一覧のパスで読もうとすると失敗しました。#1698 で直されています。

2つの関数は同じフィールドを読んでいるだけで、呼び出し関係がありません。呼び出しグラフをどうたどっても injectDiffMap には着きません。データでつながる影響は、呼び出しグラフの外にあります。

PR に出すとこうなる

グラフが出すのは候補なので、エラーにしてマージを止めると誤検知が増えます。結果は「要確認」として、PR にコメントを1つ付ける形が合っています。#1248 に付けていたら、次のようなコメントになる想定です。中身は今回の結果そのままで、行番号は #1248 のコミットのものです。

影響範囲チェック (callgraph static)
このPRが変更した関数を直接呼んでいるのに、このPRで触っていない関数: 6個

[要確認] テストが変わっていないパッケージ
  internal/scan
    scan.Agent.executeSubtask  (internal/scan/agent.go:823)
      -> llmloop.Runner.RunMainTask を呼んでいる
      -> llmloop.MainLoopStop.Reason を呼んでいる
  cmd/opencodereview
    executeScan                -> scan.NewAgent
    executeReviewContext       -> agent.New
    registerReviewFlags        -> addConcurrencyFlags
    init                       -> registerScanFlags

[参考] テストも変えたパッケージ
  internal/agent
    agent.Agent.dispatchSubtasks -> agent.Agent.executeGroupSubtask

ここまでは静的解析だけで、LLM は使いません。「executeSubtask は新しい停止理由を扱っているか」の判定まで LLM に任せる場合も、渡すのは差分と候補の関数だけです。#1248 で入力は約1万9千トークン、料金はモデルによって約4〜15円でした。

この #1248 を、実際に AIレビューのツール (open-code-review) にかけ、取りこぼしを拾えるかをクラウドとローカルの LLM で比べた結果は、AIレビューをローカルLLMで動かしてみたにまとめています。

限界

• 原因と修正の組で試したのは3つです。ここに書いた2つのほか、#845→#902 は修正された2関数ともグラフに出ましたが、テストは届いた全パッケージで変わっていて、突き合わせでは絞れませんでした
• テストの変更はパッケージ単位で見ています。テストファイルが変わっていても、漏れた経路を通るとは限りません
• callgraph は -algo=static を使いました。インターフェース経由の呼び出しは含まれていません。静的解析ではそもそも取れない呼び出しの話は、静的解析コードKGが落とす61%の動的呼び出しにまとめています
• 対象は Go の1リポジトリです

この記事の元になった本

コードをグラフにして、変更の影響をホップ数でたどる考え方は、拙著『ナレッジグラフ活用大全』の「第8章: コードをグラフで理解する」にまとめています。実際の PR で取りこぼしを出せたかを確かめたのは、本には入っていない追試です。

本の紹介ページを見る