code-review

コードレビュー — 標準と仕様

固定点（コミット、ブランチ、タグ、またはマージベース）以降の変更を、次の2つの観点からレビューします。標準（コードがこのリポジトリで文書化されているコーディング規約に従っているか）と仕様（コードが元となった課題／仕様の要求に合致しているか）です。2つのレビューを並列サブエージェントで実行し、結果を並べて報告します。ブランチ、PR、作業中の変更をレビューしたい場合や、「X以降をレビューして」と依頼された場合に使用します。

このスキルは mattpocock/skills（MIT）からインポートされ、上流ではモデルによって呼び出されます。

このスキルを使用する場面

• 固定点（コミット、ブランチ、タグ、またはマージベース）以降の変更を、2つの観点からレビューします。標準（コードがこのリポジトリで文書化されているコーディング規約に従っているか）と仕様（コードが元となった課題／仕様の要求に合致しているか）です。
• 2つのレビューを並列サブエージェントで実行し、結果を並べて報告します。
• ブランチ、PR、作業中の変更をレビューしたい場合や、「X以降をレビューして」と依頼された場合に使用します。

手順

ユーザーが指定した固定点と HEAD の差分を、次の2つの観点からレビューします。

• 標準：コードは、このリポジトリで文書化されているコーディング規約に準拠していますか？
• 仕様：コードは、元となった課題／仕様を忠実に実装していますか？

各観点は並列サブエージェントで実行し、互いのコンテキストに影響を与えないようにします。その後、このスキルが結果をまとめます。

課題トラッカーの情報は提供されているはずです。docs/agents/issue-tracker.md がない場合は、/setup-matt-pocock-skills を実行するようユーザーに伝えてください。

プロセス

1. 固定点を確定する

ユーザーが指定したもの（コミット SHA、ブランチ名、タグ、main、HEAD~5 など）を固定点とします。指定がなければ、ユーザーに確認してください。

差分コマンドを一度だけ記録します：git diff <fixed-point>...HEAD（3点記法を使うため、マージベースとの比較になります）。また、コミット一覧も記録します：git log <fixed-point>..HEAD --oneline。

先に進む前に、固定点が解決できること（git rev-parse <fixed-point>）と、差分が空でないことを確認してください。不正な参照や空の差分があれば、並列サブエージェントに処理を渡さず、ここで終了します。

2. 仕様の参照元を特定する

次の順序で、元となった仕様を探します。

1. コミットメッセージにある課題への参照（#123、Closes #45、GitLab の !67 など）。docs/agents/issue-tracker.md の手順に従って取得します。
2. ユーザーが引数として指定したパス。
3. ブランチ名または機能名に一致する、docs/、specs/、または .scratch/ 配下の仕様ファイル。
4. 見つからなければ、仕様の場所をユーザーに確認します。仕様がないと言われた場合、仕様サブエージェントはレビューをスキップし、「利用可能な仕様はありません」と報告します。

3. 標準の情報源を特定する

CODING_STANDARDS.md や CONTRIBUTING.md など、コードの書き方を定めているリポジトリ内のものを探します。

リポジトリに記載されている内容に加えて、標準の軸には常に以下のコードの不吉な臭いの基準が含まれます。これは Fowler のコードの不吉な臭い（Refactoring 第3章）を固定したもので、リポジトリに何も記載されていない場合にも適用されます。次の2つのルールに従います。

• リポジトリの標準が優先されます。 リポジトリに明記された標準は常に優先されます。基準では問題とされるものでも、リポジトリの標準が認めている場合は、その不吉な臭いの指摘を抑制します。
• 常に判断が伴います。 それぞれの不吉な臭いは「Feature Envy の可能性」のようなラベル付きのヒューリスティックであり、決して確定的な違反ではありません。ここにある標準と同様、ツールですでにチェックされるものは対象外にします。

各項目は、何を指すか → どう修正するか の順に記載されています。差分と照らし合わせてください。

• 不明瞭な名前（Mysterious Name）：関数、変数、型の名前から、その役割や保持するものがわかりません。→ 名前を変更します。適切な名前が思いつかない場合は、設計が曖昧です。
• 重複コード（Duplicated Code）：変更箇所の複数のハンクまたはファイルに、同じ形のロジックが現れます。→ 共通部分を抽出し、両方から呼び出します。
• Feature Envy：あるメソッドが、自分のデータよりも他のオブジェクトのデータを多く参照しています。→ そのメソッドを、依存しているデータの側に移動します。
• データの群れ（Data Clumps）：同じ少数のフィールドやパラメーターが、いつも一緒に渡されています（新しい型にまとめるべき状態です）。→ それらを1つの型にまとめ、その型を渡します。
• プリミティブ執着（Primitive Obsession）：独自の型を持つべきドメイン概念を、プリミティブ型や文字列で表しています。→ その概念のための小さな型を作ります。
• 繰り返される switch（Repeated Switches）：同じ型に対する同じ switch／if の連鎖が、変更箇所の複数の場所で繰り返されています。→ ポリモーフィズムに置き換えるか、両方の箇所で共有するマップを1つ用意します。
• ショットガン手術（Shotgun Surgery）：1つの論理的な変更のために、差分内の多くのファイルに散らばった編集が必要です。→ 変更対象を1つのモジュールにまとめます。
• 発散した変更（Divergent Change）：1つのファイルまたはモジュールが、関連のない複数の理由で編集されています。→ モジュールを分割し、それぞれが1つの理由で変更されるようにします。
• 推測による汎用性（Speculative Generality）：仕様で必要とされていないニーズのために、抽象化、パラメーター、フックが追加されています。→ 削除し、実際の必要性が明らかになるまではインライン化します。
• メッセージチェーン（Message Chains）：呼び出し元が依存すべきでない、長い a.b().c().d() のようなアクセスの連鎖があります。→ 最初のオブジェクトに、アクセスの連鎖を隠すメソッドを用意します。
• 仲介人（Middle Man）：クラスや関数が、ほとんどの場合に処理をそのまま別の場所へ委譲しています。→ 仲介を取り除き、実際の対象を直接呼び出します。
• 継承の拒否（Refused Bequest）：サブクラスや実装クラスが、継承した機能の大半を無視するか、オーバーライドしています。→ 継承をやめ、コンポジションを使います。

4. 2つのサブエージェントを並行して起動する

標準サブエージェントへのプロンプトには、次の内容を含めます。

• 差分を確認する完全なコマンドとコミットの一覧。
• 手順3で見つけた標準の情報源となるファイルの一覧に加え、手順3のコードの不吉な臭いの基準全体を貼り付けます（サブエージェントはそれ以外からこの内容を参照できません）。
• 次の要旨：「関連する場合はファイル／ハンクごとに、（a）差分が明記された標準に違反しているすべての箇所を報告してください。標準（ファイル名とルール）を示すこと。（b）見つかった基準上の不吉な臭いも報告してください。名称を示し、該当するハンクを引用すること。確定的な違反と判断を要する指摘を区別してください。明記された標準への違反は確定的な場合がありますが、基準上の不吉な臭いは常に判断を要する指摘であり、リポジトリに明記された標準は基準より優先されます。ツールでチェックされるものは対象外にします。400語未満。」

仕様サブエージェントへのプロンプトには、次の内容を含めます。

• 差分を確認するコマンドとコミットの一覧。
• 仕様のパスまたは取得した内容。
• 次の要旨：「次の点を報告してください。（a）仕様で求められているが、未実装または一部のみ実装されている要件。（b）差分に含まれているが、仕様で求められていない動作（スコープの逸脱）。（c）実装されているように見えるものの、実装に問題がある要件。それぞれの指摘について、該当する仕様の行を引用してください。400語未満。」

仕様が見つからない場合は、仕様サブエージェントを起動せず、その旨を最終レポートに記載します。

5. 集約

2 つのレポートを、それぞれ ## Standards と ## Spec の見出しの下に、そのまま、または軽く整えて提示します。2 つの軸は意図的に分けられているため、指摘事項を統合したり、順位を付け直したりしないでください（_2 つの軸を設ける理由_を参照）。

最後に、各軸の指摘件数と、各軸における最も重大な問題（ある場合）を 1 行でまとめます。軸をまたいで単一の優劣を決めないでください。軸を分けているのは、順位を付け直すのを防ぐためです。

2 つの軸を設ける理由

変更が一方の軸には合格し、もう一方の軸には不合格となることがあります。

• すべての標準に従っていても、誤ったものを実装しているコード → Standards は合格、Spec は不合格です。
• イシューの依頼どおりに実装していても、プロジェクトの規約に反するコード → Spec は合格、Standards は不合格です。

別々に報告することで、一方の軸がもう一方を覆い隠すのを防げます。

前回の jeo-skills 改訂版から引き継いだローカルルーティング

リポジトリに依存しないレビューのルーティング概要（重大度の段階、証拠不足の確認、 debugging、testing-strategies、code-refactoring へのルーティング）は、現在 references/jeo-review-routing.md にあります。依頼が 「今すぐレビューを実行して」ではなく「どのようにレビューすべきか」である場合に参照してください。

例

• まずこのスキルを狭い範囲に適用し、出力が上記の形式に合っていることを確認してから、タスク全体に広げます。
• 手順でリポジトリや環境の情報が必要になった場合は、ユーザーに尋ねるのではなく、調べて確認します。

ベストプラクティス

• 上流の手順をそのまま維持し、独自の判断で変更するのではなく、変更点を明示的に記録します。
• このスキルでユーザーの判断が必要とされているすべての時点で作業を止め、操作をユーザーに戻します。
• 小さく元に戻せる変更を優先し、前提を埋もれさせずに明示します。

参考資料

• 上流のスキル：mattpocock/skills skills/engineering/code-review/SKILL.md（コミット 3cca18b、MIT）
• プロジェクト標準：.agent-skills/skill-standardization/SKILL.md
• バリデータースクリプト：.agent-skills/skill-standardization/scripts/validate_skill.sh