正しいコミット2つが出会うとレビューでは止められない退行になる — Railsの自動紐付けが3ヶ月後に静かに死んだ理由

PRを2つとも読んでもバグが見えない退行がある。それぞれのPRが正しいからだ。読み終えると、自分のRailsアプリで同じずれを探す方法と、それを止めるスペックを1つ書けるようになる。

先に結論。データを入れるときのスコープと探すときのスコープは同じでないといけない。別々に手書きすると、いつか片方だけが狭くなる。コードレビューではこれを止められない。入れてから探し直す往復スペックなら止まる。

「全部、候補なし」という問い合わせ

仕事のプロジェクト（契約管理 SaaS）に、関連契約の自動紐付け機能がある。覚書や変更契約書のPDFを上げると、AIが本文から「どの契約の、いつ締結分に基づくか」を読み取り、システムが原契約を探して親子で紐付ける。

ある日、トライアル中のお客さんから問い合わせが来た。その日に上げた数十件がすべて「候補なし」だという。該当の書類を開くと、締結日は合っているし、文書名の類似度も自動紐付けの基準を余裕で超えている。それなのに候補一覧すら空だった。

類似度が高いのに候補0件？ 最初は意味がわからなかった。判定の順番を見てやっと腑に落ちた。

1. 日付     親の締結日・開始日が一致          SQL
2. 会社名   取引先を名前ではなくIDで検索      SQL   IDが0個なら return nil
3. 類似度   文書名・ファイル名を比較          Ruby  重い計算は最後に

安い条件から絞る、よくある順番だ。2で止まれば3は動かない。類似度がいくら高くても関係ない。

ずれた2つのスコープ

取引先マスタにない名前が出ると、OCRの結果から「未確認」の取引先を自動で作って契約に付ける。整理するとコードはこういう形だった（名前は変えてある）。

class Org < ApplicationRecord
  has_many :partners
  has_many :active_partners, -> { where(confirmed: true) }, class_name: 'Partner'
end

# 付ける側 — 未確認も再利用して重複を防ぐ
org.partners.find_or_create_by!(name: name)

# 探す側 — 未確認が見えない
ids = org.active_partners.where(name: name).pluck(:id)
return nil if ids.empty?

取引先を1件も登録していない組織なら、取引先はすべて未確認だ。付ける側にはその行が見えて、探す側には見えない。だから ids はいつも空配列で、2番目のゲートで終わる。

さらに紛らわしかったのは、取引先の名前は別の経路で取れて正常に出ていたことだ。名前は出るのにIDだけ出ない。ログだけ見ると問題なさそうに見える。

なぜ3ヶ月もかかったのか

git logを時系列に並べて、やっと見えた。

6月中旬     未確認取引先を導入。一覧画面だけ confirmed で絞る
26分後      絞り込みを active_partners の中に集約
            コミットメッセージ:「関連契約マッチングは影響なし」
約2週間後   別のリファクタで名前文字列のfallbackを削除 ← ここで退行が完成
8月         本番リリース
9月初め     トライアルのお客さんから問い合わせ

「影響なし」はその時点では事実だった。IDがなくても名前の文字列で照合するfallbackが生きていたからだ。2週間後にfallbackを消した側にも落ち度はない。そのPRのdiffに active_partners の定義は出てこない。使っていない道を片付けたようにしか見えない。

どちらのPRをレビューしても、これは止められなかったと思う。前のコミットの安全は後で消えるコードに支えられていて、その依存がコードではなくコミットメッセージに書いてあった。

再現してみた

本当にそうなのか、ActiveRecord 8.0.4 とインメモリのSQLiteで最小モデルを作って動かした。Aは正式な取引先がある組織、Bは未確認しかない組織。それぞれ契約を1件付けて、探し直す。

                         A(正式あり)   B(未確認のみ)
fallbackがあるとき        1             1
fallbackを消した後        1             0   ← ここ
探す側を直した後          1             1

Aだけ見ればまったく壊れていない。これが3ヶ月もった理由だ。取引先を登録している組織はそのまま動くので、全体の紐付け件数は目に見えて落ちず、指標に出なかった。後で数えると、正式な取引先が0件の組織が3分の1強あった。その組織ではリリース後、自動紐付けが必ず0件だったことになる。

直したのは1行だ。探す側のスコープを付ける側に合わせた。直すのに1行、見つけるのに3ヶ月。

スコープが2つあること自体は間違いではなかった

最初は「なんで未確認を外したんだ」と思ったが、数字を見てやめた。取引先テーブルの8割近くがOCRの作った未確認だった。オートコンプリートやマスタ画面にそのまま出すと、人が登録した数万件がAIの作った10万件台に埋もれる。表示から外すのは正しい。

間違いは、表示用のスコープに active_partners という名前を付けたことだと思う。「生きている取引先」と読めるから、検索にも使いたくなる。listable_partners だったら、マッチングのコードに書く前に一度は手が止まったはずだ。

同じ形がもう積もっていた

同じ組織に同じ名前の取引先が重複したグループが数百あり、ほとんどが正式と未確認の混在だった。「未確認が見えない関連で探し、なければ新しく作る」という登録経路が残した跡だ。これも再現する。

org.partners.where(name: 'Acme').pluck(:id, :confirmed)
# => [[2, false]]

org.active_partners.find_or_create_by!(name: 'Acme')

org.partners.where(name: 'Acme').pluck(:id, :confirmed)
# => [[2, false], [3, true]]   同じ名前が2つになった

scope付きの関連で create すると where の条件まで値として入る。だから見えない未確認の隣に、正式な取引先が新しくできる。名前1つからバグが次々に出てきたわけだ。

これを何と呼ぶか

近い言葉は Martin Fowler の semantic conflict だ。テキストとしては問題なくマージできるのに、プログラムの振る舞いが変わる変更のこと。ただFowlerの記事は並行ブランチの話で、こちらは2週間の間隔で順番に入った。それでも答えは同じだ。止めるのはテストで、レビュアーの目ではない。

防ぎ方、いつ何を

方法                  防げるもの                コスト        いつ
往復スペック          今回の退行すべて          スペック1つ   まずこれ
探す側を1か所に定義   次の人がまた手書きする    リファクタ    探す箇所が2つ以上
名前を変える          選び間違い                一括置換      scopeの意味が狭いとき
組織別の0件指標       防げないが早く気づく      ダッシュボード  自動処理の機能
コードレビュー        この種類は止まらない      —             頼らない

1つ選ぶなら往復スペックだ。付けるコードでデータを入れて、探すコードで探し直す。手作りのフィクスチャでスコープを試してはいけない。フィクスチャは正式な取引先で作りがちで、そうすると今回のバグがそのまま通る。

it '未確認の取引先で付けた契約も見つける' do
  org = create(:org)                       # 正式な取引先0件
  ContractAttacher.call(org, 'Acme', date) # 本物の付けるコード

  expect(ParentFinder.call(org, 'Acme', date)).to be_present
end

上の再現モデルで動かすと、fallbackを消した版でこの形の検査が落ちる。

自分のコードでやること

# 条件付きの関連・scope の一覧
grep -rnE "has_many .*-> *\{|scope :" app/models
# 空の結果で静かに抜ける箇所
grep -rnE "return( nil)? (if|unless) .*(empty\?|blank\?|none\?|any\?|present\?)" app

2つのリストが交わるところが候補だ。表示用のスコープがマッチング・重複チェック・find_or_create_by に使われていたら、スペックを1つ足す。そしてfallbackを消すPRを見るときは、まず「このfallbackに頼っているコードは何か」を聞く。

自動紐付けのように静かに0件になる機能は、全体の合計で見てはいけない。組織別に割らないと見えない。

Railsアプリでこういう静かな退行を探したり、スペックで防いだりするのを一緒に見てほしければ、negabaro に聞いてもらって構わない。