# Code Review

> 固定点（コミット、ブランチ、タグ、マージベース）以降の変更を2つの軸でレビューする: Standards（このリポジトリのコーディング標準に沿っているか）と Spec（元となったissue/specが求めた内容と一致しているか）。両方のレビューを並列サブエージェントで実行し、並べて報告する。ユーザーがブランチ・PR・作業中の変更のレビューを求めたとき、または「Xからの変更をレビューして」と言われたときに使う。

- Skill: `sinnlosses/code-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add sinnlosses/code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/sinnlosses/code-review/raw
- Safety review: pending (external: skill-scanner PASS, skillspector PASS)
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: sinnlosses (https://skillmd.com/u/sinnlosses)
- Updated: 2026-09-22
- Page: https://skillmd.com/skills/sinnlosses/code-review

---


`HEAD` とユーザーが指定した固定点との間の diff を、2つの軸でレビューする:

- **Standards**: コードがこのリポジトリのコーディング標準に沿っているか
- **Spec**: コードが元となったissue/specの内容を忠実に実装しているか

両方の軸は**並列サブエージェント**として実行し、互いのコンテキストを汚染しないようにしてから、このスキルが両者の所見を集約する。

issueトラッカーの情報は事前に共有されているはずである。もしこのリポジトリでissue/specの参照方法が分かるドキュメントが見つからなければ、issueやspecがどこで管理されているかをユーザーに確認する。

## 手順

### 1. 固定点を固定する

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

diffコマンドを一度確定させる: `git diff <fixed-point>...HEAD`（3ドット記法で、マージベースとの比較にする）。あわせて `git log <fixed-point>..HEAD --oneline` でコミット一覧も控える。

先に進む前に、固定点が解決できること（`git rev-parse <fixed-point>`）と diff が空でないことを確認する。参照が不正だったり diff が空だったりする場合は、並列サブエージェント2つの内部ではなく、ここで失敗させる。

### 2. spec の出典を特定する

以下の順で、元となった spec を探す:

1. コミットメッセージ内のissue参照（`#123`、`Closes #45`、GitLabの `!67` など）。このリポジトリのissue管理方法に従って内容を取得する。
2. ユーザーが引数として渡したパス。
3. ブランチ名や機能名と一致する `docs/`、`specs/`、`.scratch/` 配下のspecファイル。
4. 何も見つからなければ、specがどこにあるかをユーザーに確認する。「specはない」と言われた場合は、**Spec** サブエージェントをスキップし「利用可能なspecなし」と報告する。

### 3. 標準（standards）の出典を特定する

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

リポジトリが何を明文化しているかに加えて、Standards軸は常に以下の**スメル・ベースライン**を併せ持つ。これはFowlerのコードスメル一覧（『リファクタリング』第3章）に基づく固定セットで、リポジトリが何も明文化していない場合でも適用される。これには2つのルールが伴う:

- **リポジトリの規約が優先する。** 明文化されたリポジトリの標準は常に勝つ。ベースラインが指摘する内容をリポジトリの標準が容認している場合は、そのスメル指摘を抑制する。
- **常に判断の余地がある。** 各スメルはラベル付けされたヒューリスティック（「Feature Envyの可能性」など）であり、絶対的な違反ではない。ここでの他の標準と同様、既にツールが強制しているものはスキップする。

各スメルは「*それが何か*」→「*どう直すか*」の順で読み、diffと照合する:

- **Mysterious Name（謎めいた名前）**: 何をするか・何を保持しているかが名前から分からない関数・変数・型。→ 名前を変える。誠実な名前が思いつかなければ、設計自体が曖昧である証拠。
- **Duplicated Code（重複コード）**: 変更内の複数のハンクやファイルに同じロジックの形が現れている。→ 共通の形を抽出し、両方から呼び出す。
- **Feature Envy（機能の横恋慕）**: 自分のデータより他のオブジェクトのデータに多くアクセスするメソッド。→ そのメソッドを、横恋慕しているデータの側に移す。
- **Data Clumps（データの群れ）**: 同じ数個のフィールドや引数がいつも一緒に移動している（本来1つの型になりたがっている）。→ それらを1つの型にまとめ、その型を渡す。
- **Primitive Obsession（基本データ型への執着）**: 本来は専用の型を持つべきドメイン概念の代わりにプリミティブや文字列が使われている。→ その概念に小さな専用の型を与える。
- **Repeated Switches（繰り返されるswitch）**: 変更内の複数箇所で、同じ型に対する同じ `switch`/`if`カスケードが繰り返されている。→ ポリモーフィズムに置き換えるか、両方の箇所で共有する1つのmapに置き換える。
- **Shotgun Surgery（散弾銃手術）**: 1つの論理的な変更のために、diff内の多数のファイルに散らばった編集が必要になっている。→ 一緒に変化するものを1つのモジュールにまとめる。
- **Divergent Change（変更の分岐）**: 1つのファイル/モジュールが、いくつもの無関係な理由で編集されている。→ 各モジュールが1つの理由でのみ変化するように分割する。
- **Speculative Generality（不必要な一般化）**: specが求めていないニーズのために抽象化・パラメータ・フックが追加されている。→ 削除する。本当に必要になるまでインライン化して戻す。
- **Message Chains（メッセージの連鎖）**: 呼び出し元が依存すべきでない長い `a.b().c().d()` のようなナビゲーション。→ その辿り方を最初のオブジェクトの1つのメソッドの裏に隠す。
- **Middle Man（中間人）**: ほとんど処理を右から左に流しているだけのクラスや関数。→ それを取り除き、本来の対象を直接呼ぶ。
- **Refused Bequest（拒否された遺産）**: 継承した内容のほとんどを無視またはオーバーライドしているサブクラス/実装。→ 継承をやめ、コンポジションを使う。

### 4. 両方のサブエージェントを並列に起動する

**Standards サブエージェントへのプロンプト**には以下を含める:

- diffコマンド全文とコミット一覧。
- 手順3で見つけた標準の出典ファイルの一覧、**加えて手順3のスメル・ベースライン全文**（サブエージェントは他にこれを参照する手段を持たないため、そのまま貼り付ける）。
- 指示内容: 「該当箇所ごとに (a) diffが明文化された標準に違反している箇所をすべて報告する。標準の出典（ファイル＋ルール）を明記すること。(b) 気づいたベースラインのスメルがあれば、その名前を挙げてハンクを引用すること。ハードな違反か判断が分かれるものかを区別すること。明文化された標準への違反はハードな違反になりうるが、ベースラインのスメルは常に判断の余地があり、明文化されたリポジトリの標準はベースラインより優先される。ツールが強制するものはスキップする。400語以内。」

**Spec サブエージェントへのプロンプト**には以下を含める:

- diffコマンドとコミット一覧。
- specのパスまたは取得した内容。
- 指示内容: 「以下を報告する: (a) specが求めていたが欠けている、または不完全な要件、(b) 求められていないのにdiffに含まれている振る舞い（スコープクリープ）、(c) 実装されているように見えるが、実装が間違っているように見える要件。それぞれの所見についてspecの該当行を引用すること。400語以内。」

specが見つからない場合はSpecサブエージェントをスキップし、最終報告にその旨を記載する。

### 5. 集約する

2つの報告を `## Standards` と `## Spec` の見出しの下に、そのまま、あるいは軽く整形して提示する。2つの軸はあえて分離しているため（「なぜ2つの軸か」を参照）、所見を**統合したり、再ランク付けしたりしない**こと。

最後に1行の要約をつける: 各軸ごとの所見の総数と、（あれば）**各軸内で**最も深刻な問題。軸をまたいで単一の「勝者」を選ばないこと。それはこの分離が防ごうとしている再ランク付けそのものである。

## なぜ2つの軸か

ある変更は、片方の軸には合格し、もう片方には不合格になり得る:

- あらゆる標準に従っているが、間違ったものを実装しているコード → **Standardsは合格、Specは不合格。**
- issueが求めた通りに動くが、プロジェクトの規約を破っているコード → **Specは合格、Standardsは不合格。**

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