品質レビュー は何をしますか?
対象: $ARGUMENTS
ターゲットの設計とテスト品質をレビューします。発見事項を報告し、コードは編集しません。
1. ルーブリックを読み込む
コードを見る前に、以下を両方とも読んでください:
~/.claude/CODING_STANDARDS.mdがルーブリックです。すべての発見事項は、そのセクション見出しのいずれかを引用します。- design-vocabulary.md には、発見事項を表現するための用語と、それらを検証するためのテストが含まれています。
リポジトリの AGENTS.md / CLAUDE.md も存在すれば読んでください。それらのファイルに、リポジトリが標準で推奨されていないことを意図的に行っていると書かれている場合、そのことはレポートで一度だけ述べ、すべてのインスタンスにフラグを立てないでください。あなたがコード内でのみ観察する習慣(すべてのモジュールが環境を読み取る、すべてのテストがモンキーパッチを使用する)は文書化された選択ではありません。それに従う新しいコードも依然として発見事項です。
2. ターゲットを解決する
- 引数なし: 現在のブランチをベースブランチと比較します。
git merge-base HEAD origin/main(またはmaster)、次にgit diff <base>...HEADに加えて、git diff HEADからの未コミットの変更。 - PR番号またはURL:
gh pr diff <n>およびgh pr view <n> --json baseRefName,headRefName,files。 - ブランチ:
git diff <base>...<branch>。 - パス、またはリポジトリ全体の場合は
.: そのパス以下の既存コードを、差分なしでそのままレビューします。これはコードベースモードです(下記参照)。
3. 設計を判断するのに十分なコードを読む
変更モード(差分、ブランチ、PR)
差分だけではモジュールが深いかどうかはわかりません。変更された各モジュールについて、ファイル全体を読み、その呼び出し元とテストをgrepしてください。深さ、所有権、重複は呼び出しサイトに関する判断であるため、それらを確認します。
発見事項は、変更によって導入された、または悪化したものに限定します。差分が触れるコード内の既存の問題は、変更がそれを修正する自然なタイミングであった場合に範囲内です。触れられていないコード内のものは範囲外です。
新しいモジュールには、コードベースモードと同様にモジュール規模のレビューを実施します。Types と Readability を他のセクションと同様に完全に評価します。新規または大幅に書き換えられた各モジュールについて、判断する前にすべての関数をそのパラメータと戻り値の型とともにリストアップし、それが受け入れ、返却、またはキーを読み取る型指定なしのマップ、dict、タプル、およびすべての一文字名を記録します。
Readability を判断する前に、変更されたすべてのソースファイルの循環的複雑度を、その言語の通常のツールで測定してください。例:Python では radon cc -s、Go では gocyclo、JavaScript/TypeScript では eslint の complexity ルール、Rust では clippy の cognitive_complexity、Java では PMD。リポジトリに独自のツールがあればそれを、なければワンオフランナー(uvx、npx、go run)を使用します。短絡ブールオペランドをカウントするツールを優先します。一部(ruffのC901など)はそれらをスキップし、最悪の関数を過小評価します。利用可能なツールがない場合は、ルーブリックで定義されているように決定ポイントを手動でカウントします。差分が追加する関数でスコアが10を超えるもの、または既存の関数を差分が10を超えて、またはさらに押し上げるものは候補となります。既存の関数の場合、ベースバージョンもスコアリングします(git show <base>:<file> をスクラッチパッドファイルに書き出してそれを測定)、これにより発見事項で変更が何を追加したかを示すことができます。
差分が約150行以上のソースを追加する場合、設計セクションをレビューしている間に、2つの general-purpose サブエージェントを並行して起動します。各サブエージェントには、差分、変更されたファイルリスト、およびそのセクションのステップ4検証ルールが与えられます。1つは Types、Readability および「報われる場所で不変データに傾く」、もう1つは Testing で、すべての新しいテストにアサーションチェックを適用します。保持する前に各サブエージェントが返すものを検証します。
コードベースモード(パスまたは .)
パス以下のすべてが範囲内ですが、発見事項のハードルは範囲のサイズとともに高くなります。単一のモジュールまたは小さなディレクトリ(おおよそ1,500行未満のソース)は、モジュール、関数、および命名レベルの発見事項を対象とします。Types と Readability を他のセクションと同様に完全に評価します。パッケージまたはリポジトリ全体は、大きな問題、つまりコードベースを妨げる設計上の決定とアーキテクチャパターンのみを対象とします。これは、ユーザーが機能やツール(例:「CLI全体」)を指定した結果、それが1つのモジュールであることが判明した場合も同様です。モジュール規模で判断します。パッケージまたはリポジトリ規模の各候補について以下を問います:
- その代償を払っているモジュール、テスト、または将来の変更はいくつあるか?1つの関数に限定されたコストは、この規模では些細なものです。
- それを修正すると、このリポジトリの新しいコードの書き方が変わるか?修正が呼び出し元に影響を与えない少数のローカルな行である場合、それはドロップします。
- それはパターンか、それとも一度きりのものか?個々のインスタンスは、パターンの証拠としてのみカウントされます。
大きな問題の例:所有モジュールがない外部システム。そのためすべての呼び出し元が認証とページネーションを再実装している。エントリポイントでポリシーとオーケストレーションが融合しており、決定をエンドツーエンドでしかテストできない。各モジュールがストレージレイアウトを知っている状態所有権がモジュール間で分割されている。クライアントと設定がインポート時に構築されるため、実際の環境なしでは何もテストできない。シームが存在しないため、内部をパッチするテストスイート。長い関数、誤った名前の変数、単一の飲み込まれた例外、または1つの弱いアサーションは、それだけでは大きな問題ではありません。
これらの問題はモジュールにまたがるため、ファイルを詳細に読む前にマップを作成します:
- モジュール(ファイルまたはパッケージ)と、それぞれがおおまかに何をするか。
git ls-files <path>と各モジュールの先頭のざっと見から。 - 主要な部分がどのように接続されているか:何が何を呼び出し、状態とデータがエントリポイント、モジュール、および外部システム間でどのように移動するか。
- 各外部システム(API、データベース、キュー、ファイル形式、モデル)と、それに直接通信するすべてのモジュール。
- 環境または設定が読み取られるすべての場所(
os.environ、getenv、process.env、フラグ解析)。 - モジュールレベルのミュータブルな状態とインポート時効果(モジュールスコープで構築されるクライアント、接続、キャッシュ、ロック)。
- エントリポイント、および各テストファイルがどのモジュールをテストしているかのテストディレクトリ。
これのほとんどはgrepで十分です。マップは短く保ちます。これはレポートの一部ではなく、作業用のインデックスです。
スコープが小さい場合(おおよそ3,000行未満のソース)、マップから直接レビューし、並行して2つの general-purpose サブエージェントを起動します。各サブエージェントには、ファイルリストとそのセクションのステップ4検証ルールが与えられます。1つはルーブリックの Types、Readability および「報われる場所で不変データに傾く」セクションのみを取得します。もう1つは Testing セクションのみを取得し、すべてのテストにアサーションチェックを適用する必要があります。これにより、これらのセクションは、以前のセクションの残りではなく、完全な注意を払うことができます。保持する前に各サブエージェントが返すものを検証します。そうでない場合は、関連するモジュールの領域(それぞれテストを含む)に分割し、各領域を general-purpose サブエージェントに並行して起動して与えます。各サブエージェントには、ルーブリックパス、語彙パス、マップ、ファイルリスト、ステップ4検証ルール、および上記の大きな問題の基準を与え、何も編集しないように指示します。自分の領域だけを見るサブエージェントは、ローカルな些細な問題に流れがちです。そのため、自分の領域で見られる設計パターンと各パターンの証拠(ファイル:行)を尋ね、行レベルの発見事項のリストは求めないでください。モジュール間の問題はマップから得られるため、それらは自分で解決し、サブエージェントのパターンと組み合わせ、異なる領域から見られた同じ根本的な決定であるものをマージし、報告する前に各証拠を再確認します。
4. 各セクションに対してレビューする
ルーブリックを順番に評価します:Modules、State、Types、Dependencies and configuration、Errors、Readability、Testing。発見事項の候補ごとに、保持する前に検証します:
- 浅いモジュールまたはパススルー:削除テストを適用します。呼び出し元を特定し、それらにどのような複雑さが再現されるかを述べる、または何も発生しないことを示します。
- その地位を獲得していない抽象化:シームのアダプターを数えます。テストのモックもカウントします。背後に何も隠されていない1つのアダプターは発見事項です。2つは真のシームです。
- 欠落している抽象化:反対方向も同様に厳密にチェックします。1つのモジュール内、およびモジュール間で、3つ以上の場所でインラインで処理されている懸念事項を探します。
- 欠落している所有者/重複した懸念事項:「これはいくつかの場所に現れる」ではなく、懸念事項が書かれている各場所をリストします(ファイル:行)。
- 所有者のない概念:それが表現、解析、または構築されているすべての場所(ファイル:行)、および異なる意味を持つ同じ型のパラメータが並んで配置されているシグネチャをリストします。識別子は、構造化データだけでなく、概念としてカウントされます。異なる種類の名前とIDが同じプリミティブとして渡される場合(ソースリポジトリ名とターゲットリポジトリ名、ブランチ名と参照、2種類のID)が主なキャッチケースです。次に、どのような種類の所有者が適切かを述べます。呼び出し側が概念を保持して渡す場合は、その型を推奨します。呼び出し側がそれを使って何かをする必要があるだけの場合は、その作業を行うモジュールの中に隠すことを推奨します。デフォルトでドメイン型にしないでください。
- 順序付け制約または手動構築されたキー/パス/ID:シーケンス処理または構築を行っている呼び出し元を示します。
- 共有ミュータブル状態、ロード時効果、深い場所での設定読み取り:グローバルまたはインポート時の呼び出し、およびそれがブロックするテストまたは2番目の設定を特定します。
- スナップショット書き込み:読み取り、長時間実行ステップ、および書き戻しを示します。
- 不変の方が良いミュータブルデータ:ミュータビリティに具体的なコストがある場合にのみ報告します。予期しない変更を確認する2番目の所有者、それが安全でなくするキャッシュまたはキー、または値が作成された場所から遠く離れて行われた変更を示します。1つの所有者が独自のコードを通じてそれを変更するミュータブル構造は発見事項ではありません。不変バージョンがパフォーマンスにコストがかかる場合は、何をメモ化または共有するか、およびそのキャッシュがどこにあるかを述べます。
- 表現可能な無効状態:決して発生してはならない状態を特定し、それを許可する型を示します(一緒に設定する必要があるオプションフィールド、ブール値の組み合わせ、少数の許可された値を持つ文字列)。それのためにのみ存在するガードまたは
Noneチェックをリストします(ファイル:行)。それを不可能にする形状を提案し、その変更がどのガードを削除するかを述べます。 - 概念としてのDictまたはタプル:読み取られるキーとそれらを読み取るすべての関数(ファイル:行)、およびdictが構築されるすべての場所をリストします。リポジトリの既存のスタイル(pydanticモデルまたはdataclass、TypeScriptインターフェース、Go構造体など、リポジトリがすでに使用しているもの)で、コードが使用するフィールドのみを保持するレコード型を提案し、ペイロードがそれに解析される場所を述べます。
- 外部システムを所有するモジュール:その関数がモジュールグローバルを通じて認証情報、ID、ベースURL、キャッシュを共有しているかどうか、または呼び出しごとの環境読み取り、およびテストがその関数のいくつかをパッチしているかどうかをチェックします。その場合、起動時に構築され渡される、ドメイン名のメソッドを持つクライアントクラスを推奨し、モックが置き換えるパッチを特定します。
- 欠落しているdocコメント:各非自明な関数でdocコメントがないもの(ファイル:行)をリストし、そのdocコメントがシグネチャが言及していない何を述べる必要があるかを示します(フォールバック、エラーモード、触れる状態、入力形状)。それらをモジュールごとに1つのReadability発見事項にグループ化します。
- 一文字名:各一文字名を(ファイル:行)と提案される名前とともにリストアップして、グループ化された1つのReadability発見事項として報告します。それらは1つの発見事項としてReadabilityの上限にカウントされ、それらを削除する理由にはなりません。
- 飲み込まれたエラー:失敗から、ログ行だけが記録する間に変化する結果へのパスを示します。
- 複数の意味を持つ名前:grepして、各サイトで何を保持しているかをリストします。
- そのレベルまたはオブジェクトを隠す名前:あいまいに読まれる呼び出しチェーンまたは呼び出しサイトを示し、置き換えを提案します。
- 複数のことを行う関数:その一文の要約を書いてみてください。書けない場合は、それを妨げるブランチ、tryブロック、またはモードパラメータを引用します。
- 複雑すぎる関数:複雑度測定の各候補について、そのスコアを与え、次にそれが実行するフェーズを順にリストします(入力をフィルタリング、フェッチ、主張、検証、構築、作成、記録、通知…)各行範囲と各フェーズの失敗ルール。分割を、フェーズまたは決定ごとに1つずつ、名前付き関数としてシグネチャとともに提案し、各抽出されたピースがパッチなしで何をテスト可能にするかを述べます。通常、決定は純粋関数になり、元の関数は名前付きステップの短いシーケンスとして読み取られます。関数ごとに1つの発見事項。20を超えると設計発見事項にランク付けされます。11〜20はReadability発見事項であり、Readabilityの上限にカウントされません。関数がエントリポイントである、またはリポジトリにより大きなものがあるという理由で分割をドロップしないでください。
- テストアサーション:アサーションを失敗させる製品バグを特定します。それができない場合、それが発見事項です。パラメータ化またはテーブル駆動のケースの場合、各入力をコードを通じてトレースし、そのどの部分がケースが命名された動作に到達するかを述べます。区別する入力が以前のブランチによって消費されるケースは、そのテストが存在しても、その動作をカバーしません。同様に、フィクスチャまたはモックが各ガードまたは拒否パスをトリガーする入力を実際に生成できるかどうかをチェックします。それが決してできない場合、ガードはテストされていません。
- テストダブル:テストが何についてであるかを述べ、次にダブルがその境界の外側にあるか、または製品ロジックを再実装しているかを述べます。
- 内部をパッチするテスト:引き出すべき決定、または渡されるべき依存関係を指摘します。それが本当の修正だからです。
- 発見事項の修正で提案されたシームまたはポート:ドメインで命名された操作(
close_ticket(id))を、トランスポート動詞(get/post/put)ではなく与えます。リクエストを単に転送するだけのポートは削除テストに失敗し、そのモックはルーティングを再実装する必要があります。報告する前に、提案されたすべてのシームを削除テストに対してチェックします。
前のセクションで多くの発見事項が生成された場合でも、Testing に前のセクションと同じ深さを与えます。上記のすべてのTestingチェックを各テストファイルに適用します。各アサーションについて、それを失敗させる製品バグを特定します。失敗するアサーションがないセキュリティ関連の動作(編集、認証チェック、承認ゲート)は、設計発見事項にランク付けされます。
具体化できないものはすべてドロップします。ルーブリックがカバーしていないスタイル上の些細な問題はドロップします。品質問題から直接生じる場合を除き、ここで正確性のバグを報告しないでください(例:データを失うスナップショット書き込み)。それらは1行で言及し、/code-review を指してください。
5. レポートを構成する
発見事項をルーブリックのセクションごとにグループ化します。各グループ内で、最も影響の大きいものから順に並べます。
各発見事項には次のものを含めます:
- ルーブリックセクション:違反しているルーブリックの見出し。
- 重大度:
重大(設計上の欠陥、広範囲に影響)、中(パターン、再利用可能な改善)、軽微(局所的なもの)のいずれか。 - 場所:該当するファイルと行。
- 説明:問題とその影響の明確な説明。
- 証拠:上記の検証手順からの具体的な証拠(ファイル:行の引用など)。
- 提案:修正または改善の具体的な提案。
全体的な品質評価でレポートを締めくくります。
6. 編集しない
いかなるコードも編集しないでください。このスキルは読み取り専用です。