review-code
自分が書いた差分を、そのリポジトリの規約と設計の境界に照らしてレビューし、must / imo / nits の3段階で会話へ返す。GitHub へは投稿しない。ユーザーが「レビューして」「軽く見て」「セルフレビューして」「/review-code」等を指示したときに使う。「深くレビューして」「徹底的にレビューして」「多観点で見て」「レイヤー別に見て」と言われたら --deep を付けて、サブエー
SKILL.mdhome/.claude/skills/review-code/SKILL.md
GitHub自分の差分を must / imo / nits の3段階で指摘します。会話へ返すだけで、GitHub へは投稿しません。
観点が狭いものは既存スキルが持っています。 ここでは重ねて出しません。
| 見るもの | 担当 |
|---|---|
| 設計の境界・複雑度・規約違反・失敗シナリオ | 本スキル |
| コメントの粒度 | /review-code-comments |
| テストと Story の追随 | /review-tests |
| 他人の PR(投稿まで) | /review-pr |
| 多観点の並列レビュー | --deep ( references/deep.md ) |
プロジェクト側に同名があれば、そちらを使う
この名前はリポジトリ側の .claude/skills/ にも置かれていることがあります。 そのときはリポジトリ側を優先する。ディレクトリ構成と規約の対応を知っているのはそちらで、こちらが組み直すと精度が落ちます。
- 判定はファイルの場所で行う。 同じ名前のスキルは個人側しか一覧に出ないので、リポジトリの直下の
.claude/skills/<名前>/SKILL.mdと.claude/commands/を直接見る - 同名でなくてもよい。 「差分をレビューする」と読めるものがあれば、それを使う
- 本スキルは、リポジトリ側に無いときの土台として働く
--deep のとき
手順は references/deep.md に従う。 サブエージェントを並列起動し、束ごとのレビュアーと層をまたぐフロー整合性レビュアーを同時に走らせる。判定表と出力形式は下の手順と同じ severity.md と output.md を使う。
進め方
1. 範囲を確定する
引数があればそれを使い、無ければ origin/HEAD...HEAD を既定にします。
二点ドットを既定にしない。 二点ドットは先端同士の直接比較なので、ベース側が先行しているとその先行コミットが逆向きの差分として現れ、レビューの対象に混ざります。三点ドットはマージベース起点なので、ベースの進行に影響されません。
git diff <範囲> --name-only で一覧を取り、次を除外します(指摘の集計からも除く)。
| 除外するもの | 例 |
|---|---|
| 生成物 | モック・スナップショット・自動生成コード・ビルド成果物 |
| ロックファイル | 依存の解決結果 |
| 依存の実体 | ベンダリングされたコード |
*.md 単独の変更 |
実装と一緒に変わっているなら対象に含める |
対象が0件なら「レビュー対象の差分がありません」と報告して終わる。
2. そのリポジトリの規約を探して読む
規約の置き場はリポジトリごとに違います。 名前を決め打ちせず、実際に見てから読む。
| 見る場所 | 何が入っているか |
|---|---|
.claude/rules/ |
入口。@ 参照の先が本体のことがあるので辿る |
CLAUDE.md / AGENTS.md |
全体の前提とワークフロー |
ls docs/ |
規約本体。ディレクトリ名は揺れるので一覧で確かめる |
CONTRIBUTING.md |
外部向けの規約 |
| lint / formatter の設定 | 機械が防いでいるもの。ここで弾かれるものは指摘しない |
記憶で規約を語らない。 該当ファイルを Read し、その記述を根拠にします( verify_before_asserting.md )。
機械が弾くものを指摘しない。 formatter・linter・型検査が落とすものを人が指摘しても、二重管理になるだけです。
3. 変更が乗る流れを、入口から出口まで辿る
差分の行だけを読んで判定しない。 差分の中だけを見て変更が正しく見えても、その変更が乗る流れの中で何が起きるかは分からない。
| 順 | やること |
|---|---|
| 1 | 変更が乗る流れを1文で言う(例: 未ログインで記事を開く → ログイン画面 → ログイン → 記事へ戻る) |
| 2 | その流れを入口から出口まで、差分の外のファイルも読んで辿る。呼び出し元・呼び出し先・同じ流れで動く別の処理(ガード・副作用・イベントの購読)を含める |
| 3 | 流れの挙動がフレームワークやライブラリに依るなら、手元に入っているバージョンのソースを読む( npm pack <pkg>@<version> で取れる)。記憶や記事で決めない |
| 4 | 原因や直し方を指摘に書くなら、その仕組みが実際に起きることを動かして確かめる(下記) |
消去法で候補を1つに絞っても、それが走ることの裏付けにはならない。 ソースを読んで「走る経路が無い」と判断しても、その判断も裏付けにはならない。どちらも読んだ範囲での推測で、確かめられるのは動かしたときだけ。
実際に、ログイン後の遷移が競合する不具合を直す PR を、差分とその直近だけ読んでレビューした。消去法で残ったフックを原因として直し方のコード片を投稿し、後からフレームワークのソースを読んで「このフックがもう一度走る経路は無い」と判断し、案を取り下げた。動かして記録すると、そのフックは実際に呼ばれていて、取り下げた案で直った。さらに、PR が直っていた理由は PR が説明していた「1秒待つ」ではなく、別の変更の副作用だった。どれも、流れを通しで動かすまで分からなかった。
動かせるなら動かす。 画面の流れなら、API をブラウザの自動操作の側で差し替えれば、手元の BE が無くても通せる。router.push などの呼び出しを包んでスタックを記録すると、どこが呼んでいるかを推測せずに特定できる。変更の前(ベース)と後の両方で流し、差を見る。PR の説明が挙げる理由(「待つから直る」)も、その部分だけを外して流すと確かめられる。
動かせないなら、原因や直し方を断定せずに「この経路は動かして確かめていない」と書き、重要度を1段下げる( references/severity.md )。
4. 4つの観点で見る
責務・境界の違反
そのリポジトリが境界を宣言しているときだけ見ます(依存方向・パッケージの役割分担・共有コードの範囲)。宣言が無ければこの観点は飛ばす。
- 宣言された依存方向に逆らった参照が入っていないか
- 内側・共有側に、外側・利用側の関心事が混ざっていないか
- 変更全体として「どこに何を足したか」が、その境界の意図と合っているか
コメント文にも責務違反は現れます。 そのファイルが属する範囲より外側の語彙をコメントが知っているなら、依存関係が文章の側で逆流しています。書くべきは値・型・関数そのものの意味で、誰がどこで使うかではありません。
複雑度
目安を超えたら見る、という順序で使う。 数字は入口で、判断は中身です。
| 見るもの | 目安 |
|---|---|
| 1関数の行数 | 50行超で分割を検討 |
| ネストの深さ | 3階層以上 |
| 引数の数 | 5個以上 |
| 真偽値フラグ引数で挙動が分岐 | 1個でも |
深い else ネスト |
早期 return にできないか |
| マジックナンバー・マジック文字列 | 名前付き定数にできないか |
| 意図不明な抽象化 | 過剰な interface / wrapper が増えていないか |
| 同じ処理の散在 | 既存ヘルパーの再発明になっていないか |
複雑度の指摘には「なぜ複雑になっているか」と「どう分割・整理するか」を必ず添える。 「複雑です」だけでは行動が変わりません。
抽象化の良し悪しを指摘するときは、/mattpocock-skills:codebase-design の語彙で書く。入口(呼ぶ側が知るべきこと)の大きさに比べて中身が薄いものを「浅いモジュール」、差し替えの効く位置を「継ぎ目(seam)」と呼び、どこへ寄せれば深くなるかまで添える。
目安に当たらないコードの不吉な臭い(Feature Envy・Shotgun Surgery など)は、/mattpocock-skills:code-review の smell baseline の一覧を差分に照らして探す。リポジトリの規約が許していれば指摘しない。
読みやすさ・削れるもの・テストの当て方は、pstack の原則を当てて見る。principle-minimize-reader-load (辿る層と抱える状態の数)、principle-subtract-before-you-add (足す前に消せるものがないか)、principle-test-behavior-not-implementation (振る舞いを壊すと落ち、内部を変えても通るか)。
設計の形は差分が当たる場面ごとに pstack の原則を当てて見る。型や関数の形を変えたなら principle-type-system-discipline (取りえない状態を型で表せてしまわないか)、検証やエラー処理を足したなら principle-boundary-discipline (境界に集まっているか、内側へ散っていないか)、状態を持つ処理や分岐を増やしたなら principle-model-the-domain (真偽値の組を状態の一覧にできないか)、内部の API を新しくしたなら principle-migrate-callers-then-delete-legacy-apis (呼び出し側を移し、旧 API を同じ変更で消しているか)。TypeScript の差分には typescript-best-practices も当てる。目の前の問題を解くのに要らない作り直しは must にしない( solve_with_minimal_change.md )。
規約違反
手順2で読んだ規約に照らします。引用は実ファイルの記述から採る。
正確性・失敗シナリオ
「この入力・状態でこう壊れる」を、実コード上で追えるものだけ出します。
5. 重要度を確定する
references/severity.md を Read し、判定表と裏取り手順に従います。
6. 出力する
references/output.md を Read し、その形式で会話へ返します。
出力の目安
| 結果 | 返し方 |
|---|---|
| 指摘なし | 「N ファイルを見て指摘なし」と2〜3行。指摘ゼロで構いません |
| 指摘あり | references/output.md の形式 |
must と imo の合計が15件を超えたら、削るのではなく集約をやり直す。 同種の指摘がまとまっていないサインです。
やってはいけないこと
- GitHub へ投稿すること。会話へ返すまでが責務です
- 自動で修正すること。指摘に留め、直すのはユーザーの指示を得てから
- 差分の行だけを読んで判定すること。変更が乗る流れを、差分の外も含めて入口から出口まで辿る(手順3)
- 原因や直し方を、動かして確かめずに断定すること。消去法で絞った候補も、ソースを読んで出した「走らない」も推測にすぎない。動かせないなら、確かめていないと書いて1段下げる
- 規約を記憶で語ること。該当ファイルを Read して引用する
- 機械が弾くものを指摘すること。formatter・linter・型検査の担当です
- 境界の宣言が無いリポジトリで、責務違反を指摘すること。宣言が無ければその観点は飛ばす
/review-code-commentsや/review-testsと同じ指摘を重ねること。観点が被ったらあちらへ譲る- 二点ドットで差分を取ること。ベースが先行していると逆向きの差分が混ざる
- 複雑度の指摘に、分割・整理の方針を添えないこと
- 件数を稼ぐこと。読んで行動が変わらない指摘は出さない
関連
/review-code-comments//review-tests: 観点が狭い分を担当。重ねて出さない/review-pr(他人の PR): 対象と出力先が違う。あちらは投稿まで行う/finalize-prs(PR の仕上げ): 点検の入口だが、本スキルは呼ばない。境界や複雑度まで見たいときは、本スキルを単独で呼ぶverify_before_asserting.md(断定の前に裏を取る): 規約の引用も裏取りの対象verify_the_check_worked.md(検査が成立していたかを疑う): 走査が空振りしていないかを確かめるbackground_heavy_work.md(重い作業の流し方):--deepでサブエージェントを待つ間の使い方solve_with_minimal_change.md(最小の変更で解決する): 目の前の問題に要らない改善は must にせず、imo に今回は対応する必要がないと添えて出す