2軸コードレビュースキル
HEADとユーザーが指定した固定点との差分を2軸でレビューします:
- 標準(Standards):コードはこのリポジトリの文書化されたコーディング標準に準拠していますか?
- 仕様(Spec):コードは元のIssue/仕様を忠実に実装していますか?
両軸は並列サブエージェントとして実行され、互いのコンテキストを汚染しません。その後、このスキルが所見を集約します。
Issue トラッカーは事前に提供されている必要があります。提供されていない場合は、ユーザーに /setup-matt-pocock-skills を実行するよう伝えてください。
処理手順
1. 固定点を確定する
ユーザーが指定した固定点(コミットSHA、ブランチ名、タグ、main、HEAD~5など)を使用します。指定がない場合は、それを尋ねてください。
差分コマンドを一度だけ記録します:git diff <固定点>...HEAD(三点ドット。マージベースとの比較になります)。併せて git log <固定点>..HEAD --oneline でコミットのリストも記録します。
先に進む前に、固定点が解決できること(git rev-parse <固定点>)と、差分が空でないことを確認します。不正な参照や空の差分は、2つの並列サブエージェントの中ではなく、ここで失敗させるべきです。
2. 仕様のソースを特定する
元の仕様を以下の順序で探します:
- コミットメッセージ内のIssue参照(
#123、Closes #45、GitLab!67など)。トラッカードキュメントのワークフローを使って取得します。 - ユーザーが引数として渡したパス。
docs/、specs/、または.scratch/ディレクトリ内の、ブランチ名や機能に一致する仕様ファイル。- 何も見つからない場合は、ユーザーに仕様の場所を尋ねます。ユーザーが仕様はないと言った場合、Spec サブエージェントはスキップし、「仕様なし」と報告します。
3. 標準のソースを特定する
リポジトリ内で、コードの書き方を文書化したファイルをすべて検索します。CODING_STANDARDS.md または CONTRIBUTING.md が存在する場合は、必ずリストに含めます。
リポジトリが文書化している内容に加えて、標準軸は常に以下のスメルベースラインを持ちます。これは、リポジトリに何も文書化されていない場合でも適用される、Fowlerのコードスメル(『Refactoring』第3章)の固定セットです。2つのルールがこれを制約します:
- リポジトリが優先される。 文書化されたリポジトリ標準が常に優先され、ベースラインがフラグを立てるであろう内容をリポジトリ標準が支持している場合、そのスメルは抑制されます。
- 常に判断の余地がある。 各スメルはラベル付きのヒューリスティック(「可能性のあるFeature Envy」)であり、厳格な違反ではありません。ここでの他の標準と同様に、ツールが既に強制しているものはスキップします。
各スメルは それが何か → 修正方法 として読まれ、それを差分に照合します。
- 怪しい名前(Mysterious Name):関数、変数、型の名前が、その目的や保持する内容を明らかにしていない。→ 名前を変更する;正直な名前が思いつかないなら、設計が曖昧である。
- 重複コード(Duplicated Code):同じロジック形状が変更内の複数のハンクやファイルに出現する。→ 共有部分を抽出し、両方から呼び出す。
- Feature Envy:あるメソッドが自身のオブジェクトよりも他のオブジェクトのデータに多くアクセスする。→ そのメソッドを、羨んでいるデータを持つオブジェクトに移動する。
- データの塊(Data Clumps):同じいくつかのフィールドやパラメータが常に一緒に使われる(新たな型として生まれるべき)。→ それらを一つの型にまとめ、それを渡す。
- プリミティブ強迫(Primitive Obsession):プリミティブや文字列が、独自の型を持つべきドメイン概念の代わりに使われている。→ その概念に独自の小さな型を与える。
- 繰り返されるスイッチ(Repeated Switches):同じ型に対する同じ
switch/ifの連鎖が変更全体で繰り返される。→ ポリモーフィズムで置き換えるか、両方のサイトで共有する一つのマップにする。 - ショットガン手術(Shotgun Surgery):一つの論理的な変更が、多くのファイルに散らばった編集を強いる。→ 変更すべきものを一つのモジュールにまとめる。
- ** divergent な変更(Divergent Change)**:一つのファイルやモジュールが、無関係な複数の理由で編集される。→ 各モジュールが一つの理由でのみ変更されるように分割する。
- 推測による汎用性(Speculative Generality):仕様にないニーズのために追加された抽象化、パラメータ、フック。→ 削除する;本当の必要が現れるまでインライン化して戻す。
- メッセージチェーン(Message Chains):長い
a.b().c().d()のナビゲーションを呼び出し元が依存すべきでない。→ 最初のオブジェクトの一つのメソッドの背後にその遷移を隠す。 - 中間者(Middle Man):クラスや関数が、主に委譲を行うだけのもの。→ それを削除し、実際のターゲットを直接呼び出す。
- 相続の拒否(Refused Bequest):サブクラスや実装者が、継承したもののほとんどを無視またはオーバーライドする。→ 継承をやめ、コンポジションを使用する。
4. 両方のサブエージェントを並列に起動する
両方のサブエージェント呼び出しをフォアグラウンドで同時に発行し、返ってきたレポートを集約します。
標準サブエージェントのプロンプトには以下を含めます:
- 完全な差分コマンドとコミットリスト。
- ステップ3で見つけた標準ソースファイルのリスト と、ステップ3からのスメルベースラインを全文貼り付けたもの(サブエージェントはそれにアクセスできません)。
- 指示:「ファイル/ハンクごとに関連する場合、以下を報告せよ:(a) 差分が文書化された標準に違反しているすべての箇所:標準(ファイル+ルール)を引用せよ;そして (b) 発見したベースラインスメル:名前を挙げ、ハンクを引用せよ。厳格な違反と判断の余地があるものとを区別せよ:文書化された標準の違反は厳格であり得るが、ベースラインスメルは常に判断の余地があり、文書化されたリポジトリ標準はベースラインを上書きする。ツールが強制するものはスキップせよ。400ワード未満で。」
仕様サブエージェントのプロンプトには以下を含めます:
- 差分コマンドとコミットリスト。
- 仕様のパスまたは取得した内容。
- 指示:「報告せよ:(a) 仕様が要求したが欠落している、または部分的にしか実装されていない要件;(b) 差分に含まれるが要求されていない振る舞い(スコープクリープ);(c) 実装されているように見えるが実装が間違っているように見える要件。各所見について仕様の行を引用せよ。400ワード未満で。」
仕様がない場合は、Specサブエージェントをスキップし、最終レポートにその旨を記載します。
5. 集約する
二つのレポートを ## Standards と ## Spec の見出しの下に、そのままか軽く整形して提示します。所見をマージしたり再順位付けしたりしてはいけません(2軸である理由を参照)。
最後に、軸ごとの所見の総数と、各軸内での最悪の問題(あれば)を一文でまとめます。軸をまたいで単一の勝者を選ばないでください:それを防ぐために分離しているのです。
2軸である理由
ある変更は一方の軸を通過し、他方では不合格になることがあります:
- すべての標準に従っているが、間違ったものを実装しているコード → 標準合格、仕様不合格。
- Issueが要求したことを正確に行っているが、プロジェクトの規則を破っているコード → 仕様合格、標準不合格。
これらを別々に報告することで、一方の軸が他方を隠すことを防ぎます。