保守性レビュー
基準は一つです。次の人(人間でもエージェントでも)がこのコードを修正するときに、いくつの場所を開く必要があるか。 正確性のバグは扱いません。それは code-review や pr-review の役割です。
0. モード
| モード | 対象 | 比較基準 | blocking 対象 |
|---|---|---|---|
| セルフチェック | コミット前の自分の diff | レポ全体 | 修正する(報告せずに直接) |
| 変更レビュー | PR diff | レポ全体 | 今回の変更が作った問題のみ |
| 点検 | モジュールまたはディレクトリ全体 | レポ全体 | すべて。影響が大きい順に |
変更レビューで元々あった問題は blocking として上げません。「既存負債」として別途記載します。
1. プロジェクト基準を読む
docs/review/architecture.mdを探します。なければ CLAUDE.md・AGENTS.md が示す構造ドキュメントを使います。- プロジェクト基準はこのスキルの一般基準より優先します。特に以下を基準ドキュメントから取得します。
- モジュールマップと依存方向
- 再利用すべき抽象化のリスト(HTTP クライアント、結果型、テストヘルパーなど)
- 既知の負債リスト。このリストにあるパターンは、新しいコードが従うべき先例ではありません。
- 基準ドキュメントがない場合は、ディレクトリ構造と各モジュールの
indexexport からモジュールマップを直接描き、報告の最初の行に「基準ドキュメントなし」と記載します。
2. 確認すべき項目
上から順に見ます。上にあるほど後で負担が大きく、エージェントが見逃しがちです。
2-1. 既存の抽象化を迂回していないか
エージェントは1つのイシューだけを見て作業するため、以前のイシューが作ったモジュールを知らずに同じものを再度作ってしまいます。最も一般的で最もコストの高い失敗です。
新しく作られた関数、型、エラークラス、定数、外部呼び出しごとに、レポ内で同じことをしているコードを探します。
git grep -nE "class \w+Error extends" # エラークラス
git grep -nE "https?://[a-z.]+\.(com|io|dev)" # 外部 API アドレスとヘッダー
git grep -nE "^(export )?type \w+ = \{" # 似た名前の型(例: XxxEntry)
git grep -nF "<新しいコードの特徴的な文字列・正規表現>"
- 同じ外部システムを呼ぶラッパーが二つ以上ある場合、呼び出し元がエラーを複数のクラスに分けて検査することになります。その痕跡(
a instanceof X || b instanceof Y)も一緒に探します。 - 同じリスト定数(拡張子、ステータス値)が二か所にあると、片方だけ変更されることになります。
2-2. 依存方向
- 基準ドキュメントのレイヤールールに違反していないか(例: ドメインモジュールが UI や外部 API を import)。
- 下位モジュールが上位モジュールを import している、または二つのモジュールが互いに import していないか。
2-3. 単位
サイズではなく変更理由で分割します。
- 一つのファイルに互いに呼び出さない関数のまとまりが二つ以上ある場合、変更理由が二つ以上ある兆候です(例: フォームマッピング + ネストキーアクセス + 日付フォーマット)。
- 複数の画面ファイルが同じ順序の準備処理(認証 → 設定読み込み → エラー分岐)を繰り返している場合、その処理は名前を持つ関数であるべきです。
2-4. 抽象化の質
- 欠如した API: 二つ以上の呼び出し元が同じ前処理・後処理を繰り返している場合、その処理をカプセル化した関数がモジュールにありません。例: すべての呼び出し元が
doc.items.find((s) => s.kind === "x")の後にparseX(...)を呼ぶなら、readX(doc)が不足しています。 - 過剰な抽象化: 使用箇所が一か所しかないインターフェース、ファクトリ、オプション、ジェネリック。今は不要な拡張ポイントです。
- 境界の型: 失敗を呼び出し元が処理しなければならない場合、そのことは結果型(判別ユニオン)で示されていますか? 呼び出し元がメッセージ文字列を比較して分岐していませんか?
2-5. パターン
- レポが既に使っているパターン(結果型、依存注入、エラー処理、モジュール export 方式)に従っていますか。
- 新しいパターンは、既存のパターンでできない理由がある場合のみ受け入れます。理由が PR やコメントにない場合は指摘します。
2-6. コメントと名前
- コメントが間違った対象に付いていませんか。修正コミットが関数の間に新しい関数を挟み込み、JSDoc が別の関数の上に残る事故がよくあります。
- 名前がする仕事と合っていますか。名前だけ見て別の動作を期待してしまう場合は指摘します。
3. 判定
blocking(マージしない)
- 同じ責務のコードが既にあるのに新しく作った。既存のものが使えない理由が PR やコメントに記載されていれば除外します。
- 基準ドキュメントのレイヤールールに違反した。
- 新しい API のために、二つ以上の呼び出し元が同じ処理を繰り返すことになった。
- コメントが別の対象に付いていて、読む人を誤解させる。
suggestion(マージは止めず、後続作業として残す)
- ファイル分割、名前の改善、元々あった重複。
記載しないもの:好み(関数型・命令型、行の長さ)、リンターやフォーマッターが捕捉するもの、イシュー範囲外の新しいライブラリ導入。
4. 検証
指摘ごとにコードで確認します。間違った指摘は実装エージェントの時間を無駄にします。
- 「重複」なら、両方の場所を開いて本当に同じ仕事をするかを確認します。見た目は似ていても守っている仕様が異なる場合は除外します。
- 「Nか所」なら grep で数えます。
- 「今回の変更が作った」なら
git diff <base>...HEADにその行があるかを確認します。
5. 報告
最初の行:確認範囲(モード、対象パス、基準ドキュメントの有無)と件数(blocking N、suggestion N、既存負債 N)。
各項目:
- タイトル: 何がコストかを結果で書きます。(「GitHub 401 を呼び出し元がエラークラス3つに分けて検査する」○ / 「エラー処理を改善」×)
- 位置: すべての関連位置を
ファイル:行で。重複なら両方とも。 - コスト: 次に何を変更するときにいくつの場所を修正する必要があるか。(「API バージョンヘッダーを変更すると3つのファイルを修正する必要がある」)
- 方向: どちら側に統合するかを1〜2文で。
- 等級: blocking / suggestion / 既存負債。
指摘することがなければ「なし」と記載します。埋めるために指摘しません。
してはいけないこと
- 行数やファイルサイズだけを見て分割を要求すること。
- 方向のない指摘(「もっと抽象化すると良いでしょう」)。
- 見た目だけ似ているコードを重複と呼ぶこと。
- 元々あった問題を今回の PR の blocking として上げること。