コードレビュー観点チェックリスト
1. 内部一貫性
1.1 隣のコードと揃っているか
- 発火条件: 一覧/詳細、合計/明細、iOS/Android、複数プラットフォーム実装など「対になる場所」が存在するコードを変更している
- 確認手順:
- (1) 変更箇所と対になるファイル・関数をgrepで探す
- (2) 両方を並べて開き、ロジック（丸め方、フォールバック値、null処理など）が一致しているか比較する
- (3) 共通処理を切り出した場合、旧名への参照が本番コード・テストの両方から本当に消えているかgrepで確認する
- 問い: 同じことをしている場所と食い違っていないか
- 備考:
- 指摘件数全体で最も件数が多く、87%が「別の場所を開く」だけの機械的動作から発見されている
- 最優先で自動化すべき領域
1.2 契約と意図は保存されているか
- 発火条件: マジックナンバー、固定値、既存実装からの移行・置き換えを含む差分
- 確認手順:
- (1) 数値リテラルや固定値を見たら、その根拠（計算式・仕様）を自力で解いてみる
- (2) 根拠が差分やコメントに書かれていなければ、一言コメントを求める
- (3) 移行・移植系のPRでは、旧実装のコミット履歴を遡り、引き継がれるべき設定値が漏れていないか確認する
- (4) PR説明文の因果関係を鵜呑みにせず、当時のコミットで裏取り（または反証）する
- 問い: 半年後の人が同じ判断に辿り着けるか
- 備考: PR説明は主張であって事実ではない、という前提で扱う
1.3 設計の見通し
- 発火条件: 型キャスト（
as string等）の多用、汎用的すぎる型定義（Record<string, unknown>等）
- 確認手順:
- (1) null/undefinedを返しうる関数の戻り値へのキャストを探し、存在チェック後の変数代入で不要にできないか検討する
- (2) ライブラリの型拡張ポイント（Register interface等）に独自キーが宣言されているか確認する
- 問い: 型や構造が本来防げるはずのミスを防げているか
- 言い換えのコツ: 「型が緩い」ではなく「タイポがコンパイルを通る」のように具体的な失敗モードで伝える
2. 正しさ・バグ相当
2.1 値そのものは正しいか
- 発火条件: 金額・数量・日時計算、税・単価・合計に関わる差分
- 確認手順:
- (1) 掛け算・単価×数量などの計算が消えていないか、diffの前後で式を比較する
- (2) 可能なら実機・実データで数値を突き合わせる
- (3) 日時型を扱う場合、タイムゾーン有無の型不一致がないか確認する（例:
timestamp vs timestamptz）
- (4) 「今は正しく動く」理由（環境変数のデフォルト値等、たまたま成立している条件）まで特定する
- 問い: この数字・この判定は本当に合っているか。今動いている理由は何か
- 書き方: 動く理由を認めた上で、依存条件（DB起動オプション等）が変わると壊れる旨を添えて代替案を提示する
2.2 データが壊れないか
- 発火条件: 中間テーブル、GROUP BY・集約クエリ、同時実行やカーディナリティに関わる差分
- 確認手順:
- (1) 関連するDBスキーマ定義を開き、実際に強制されている制約（UNIQUE等）を確認する
- (2) アプリコードが暗黙に前提としているカーディナリティ（1対1のつもりが1対多になりうるか）とスキーマの制約が一致しているか照合する
- (3) 参照が切れた場合（レコード削除・所属解除等）に孤立データが発生しないか経路を追う
- 問い: 障害時・同時実行時・画面遷移時にだけ壊れないか
3. 可観測性
3.1 失敗に気づけるか
- 発火条件: HTTPクライアントの初期化、try/except、エラーメッセージの新設
- 確認手順:
- (1) HTTPクライアントライブラリの初期化オプション（例:
throwHttpErrors）を確認し、デフォルトでエラーを例外化しない設定になっていないか調べる
- (2) try/catch・except節が実際にどの例外を捕捉するか、捕捉リストと発生しうる例外の型を突き合わせる
- (3) 丁寧なエラーメッセージを新設した場合、それが上位の汎用except節に握り潰されずに画面まで到達する経路を追う
- 問い: 壊れたとき人間は知れるか
3.2 検証できるか
- 発火条件: 外部通知・送信を伴う処理のテスト、CIワークフローのジョブ条件
- 確認手順:
- (1) 通知・送信系の処理を通るテストで、外部クライアントが実際にモックされているか確認する（同ファイル内の他テストと非対称になっていないか比較）
- (2) テスト用環境変数・設定キーの一覧に該当のキーが含まれているか確認する
- (3) CIのジョブにif条件やブランチ制限があり、マージ前に効果を確認できない構造になっていないか確認する
- 問い: そのテストは本当に効いているか（テストがあるから安心、を疑う）
4. 影響範囲
4.1 使う人から見てどうか
- 発火条件: 画面遷移・リダイレクト・ローディング表示・削除やキャンセル操作を伴う差分
- 確認手順:
- (1) 実際にその画面を操作する手順を思い浮かべ、絞り込み条件やページ状態がリダイレクト後も保持されるか確認する
- (2) ローディングオーバーレイ等のUI要素が、操作可能な要素（ボタン等）を実際に覆いきっているか、DOM構造・CSS上の親子関係を確認する
- (3) 誤操作によって処理中のデータが変更・削除できてしまう導線が残っていないか確認する
- 問い: 運用担当とユーザーの手元で何が起きるか
- 備考: 11領域中もっとも「実際に操作するシミュレーション」が必要で、自動化が最も難しい領域。UI操作エージェントとの組み合わせが望ましい。
4.2 変更はどこまで効くか
- 発火条件: 共通コンポーネントへのオプション追加、共有オブジェクトを引数に取る関数の変更
- 確認手順:
- (1) 変更した共通部品・関数の呼び出し元をすべてgrepで洗い出す
- (2) 新しいデフォルト値が、それを渡していない既存の呼び出し元にも影響しないか（opt-inになっているか）確認する
- (3) 引数として受け取ったオブジェクト・配列を関数内で直接書き換えていないか確認する（呼び出し元への副作用）
- 問い: 意図した範囲を超えて他に波及していないか
5. 外部・非機能
5.1 外部との境界
- 発火条件: SDK・外部API・CLIツール・フレームワークの呼び出しを含む差分
- 確認手順:
- (1) 呼び出しているSDK/APIの公式ドキュメントを検索し、複数回呼び出した場合の挙動（上書きか累積か）を確認する
- (2) lockfile（package-lock.json, Gemfile.lock等）で実際に使われているバージョンを特定し、バージョン間の挙動差がないか確認する
- (3) Content-Typeやヘッダ依存の挙動（JSON判定等）がある場合、相手側が正しいヘッダを送ってくる前提になっているか確認する
- 問い: 自分が書いていないものは本当にそう動くか
5.2 速度とコスト
- 発火条件: 新しい検索クエリ、外部通信のタイムアウト設定
- 確認手順:
- (1) 新しいクエリに対応するインデックスがモデル定義・スキーマに存在するか確認する
- (2) 同じクエリが他の場所でも呼ばれている場合、呼ばれる頻度（初回のみ／リクエスト毎）の違いを比較する
- (3) タイムアウト設定を変更した場合、本番の実測レイテンシ（p50/p99等）を確認し、他レイヤーのタイムアウト値と合算して矛盾がないか確認する
- 問い: 実測値で見て問題ないか（「重そう」で終わらせない）
6. 実行優先順位（着手順の目安）
- 差分を読むだけで着手できる順（ファイルを開く前に判定できるものから）:
- (1) 根拠の保存-- 数値リテラル・固定値に根拠があるか
- (2) 対の非対称-- 対になる名前（一覧/詳細等）を探して並べる
- (3) 二重定義-- 新しく定義した定数・値をgrepする
- (4) デッドコード-- 参照をgrepで数え、ガード条件を確認する
- (5) silent failure-- HTTPクライアント設定とcatchの到達条件を確認する
- (6) 変更の波及範囲-- 変更した部品の呼び出し元をgrepで数える
- この6項目は差分の外を1枚も開かずに始められるものが多く、着手コストが低い割に検出率が高い
7. エージェント実装上の注意
- 静的解析で代替可能な項目:
- 対ファイル比較
- クライアント設定検出
- モック検出
- → ルールベースツール化を優先
- LLMエージェントが本領を発揮する項目:
- 複数ファイル・複数コミット・外部ドキュメントを横断した調査 + 推論が必要
参考資料: