「難易度が低い」は「安全」ではない。レビューを軽くする話で最初にズレるところ

保守フェーズに入った機械学習アプリのコードレビューが、そこそこの負担になっていた。修正の中身にかかわらず全部同じ重さで見ているので、ログのメッセージを一行足しただけの変更にも、特徴量の作り方を変えた変更にも、同じだけ時間がかかる。さすがに効率が悪い。

そこで、発生し得るレビュー内容をカテゴライズしてほしい、という依頼が出た。カテゴリごとに重要度が分かれば、レビューの厚みを変えられるはずだ、と。

返ってきた表がよくできていた。よくできていたのだが、そのまま使えなかった。今日はその話をしたい。

何がズレていたか

出てきたのは、こういう分類だった。

・モデル・予測ロジック変更
・データ処理変更
・入出力と外部連携の変更
・例外処理・運用改善
・リファクタリング・保守性改善
・不具合修正
・設定・環境変更

粒度としては妥当だと思う。問題は、各カテゴリを評価する軸のほうだった。表には結果同等性、修正範囲、修正内容難易度の三つが並んでいた。

依頼していたのはクリティカルレベル、つまりミスがあったときに後続処理や業務にどれだけ響くかである。並んでいる三軸は、どれも作業する側から見た重さだ。範囲が広いか、直すのが難しいか、結果が変わるか。

近いようで、これは別の軸になる。

たとえば設定ファイルの一行を書き換える作業は、修正範囲も小さいし難易度も低い。だが、その一行が本番のジョブ設定で、変更が片方の環境にしか反映されていなかったら、翌朝バッチが動かない。作業は軽く、事故は重い。

逆に、大規模なリファクタリングは修正範囲が大きく難易度も高いが、仕様が変わらない前提であれば、業務への影響は原理的に小さい。作業は重く、事故は軽い。その前提が守られている限りは、という条件付きではあるが。

レビューを省くかどうかの判断に必要なのは後者のほうになる。見落としたときに何が起きるかを知りたいのであって、書くのが大変だったかを知りたいわけではない。

書き手の視点で作った表を、読み手の判断に流用しようとして詰まった、という話だと思う。よくある。

具体的に引っかかったのが設定・環境変更だ。表では難易度が最も低く評価されていた。作業としてはそのとおりだと思う。

だが中身を並べてみると、明らかに性質の違うものが同居している。

閾値やリトライ回数の変更。これは軽い。値が妥当かどうかを見れば終わる。

環境変数やジョブ設定の変更。これは見るべきポイントが値ではなく適用範囲に移る。開発環境には入ったが本番には入っていない、前後のジョブの締め時刻がずれる。差分を眺めていても気づけない類の事故で、レビュアーは差分ではなく、どこに効くのかを確認しないといけない。

そしてライブラリ更新と権限設定の変更。ここが一番厄介だった。

ライブラリ更新は、差分を見ても影響が分からない。バージョン番号が上がっているという事実しか読み取れない。何が壊れるかは実行してみないと分からないので、レビューという行為自体が原理的に無力になる。依存関係の差分とテスト結果がセットで出てこない限り、目視には意味がない。

権限設定は、付与より剥奪が危ない。付与は増えるだけなので、その場では何も壊れない。別の意味では危ないが、それはセキュリティの話になる。剥奪は、その権限を使っている処理が本番で初めて落ちる。しかも落ちるのは変更した瞬間ではなく、その処理が次に走るタイミングなので、原因の特定にも時間がかかる。

つまり難易度が低いカテゴリの中に、レビューの効きが最も悪いものが紛れ込んでいた。難易度と影響度を一つの軸に畳んでしまうと、こういうものが見えなくなる。

ではどう線を引くのか、と考えて、今のところ一番使えそうなのが切り戻せるかどうかだった。

元の値に戻すだけで復旧できるものは、思い切って軽くしていい。間違っていても、気づいた時点で戻せば済む。閾値も、ログレベルも、たいていの設定値もここに入る。

戻せないものだけ厚く見る。権限の剥奪、マイグレーションを伴うライブラリ更新、外部に投げてしまった出力。ここは失敗のコストが非対称なので、事前に時間をかける価値がある。

カテゴリで分けるより、この一問のほうが判断が速い気がしている。カテゴリはどうしても境界例が出るが、戻せるかどうかはだいたい即答できる。

この切り分けは、自分のブログ運用でも同じ形で使っている。何を自動化して何を人間の承認に残すかを、間違えたときに取り返しがつくかどうかで決めた。その話は自動化した部分と、人間に残した部分に書いた。

返信の下書きで、ライブラリ更新と権限変更について、ここは工数削減の対象外としたい、と書いた。

書いた後で読み返して、これは伝わらないな、と思った。

こちらの意図は現状の水準を維持したい、である。全体を軽くする流れの中で、ここだけは巻き込まないでほしい、という除外指定にすぎない。だが対象外という言葉は、文脈から切り離されると特別扱いに見える。前後の文が飛べば、もっと厚くレビューしろと言っている、と読まれてもおかしくない。

削減の議論をしているときの対象外は現状維持、増強の議論をしているときの対象外は放置になる。同じ言葉が逆を向く。

結局、削減の効果より事故リスクのほうが大きいため現状のまま据え置きたい、と書き直した。長いが、長いほうがいい場面だった。

こういうところで一日悩んでしまうのは、たぶん心配性だからだと思う。ただ、この手の言葉の設計をサボると、三か月後にあのとき言いましたよね、という会話が発生する。そちらのほうがコストが高い。

・作業の重さと、壊れたときの重さは別の軸。混ぜると判断に使えない
・難易度が低いカテゴリほど、レビューの効きが悪いものが紛れている
・迷ったら切り戻せるかで切る
・削減の文脈での対象外は、現状維持のつもりでも増強に読まれる

レビューを減らす話をしていたはずが、何を見るべきかを考え直す話になった。減らす議論は、たいていそうなる気がする。