| name | my-code-review |
| description | 現在のリポジトリの変更(staged/unstaged含む)またはdefault branch (main/master) との差分とその周辺コードを読み、実装の良し悪しを評価するコードレビュースキル。保守性・テスト十分性・spec適合・実装の筋の良さなどの観点から、プロの視点で良い点と問題点を指摘する。ユーザーが「レビューして」「コードレビューして」「この実装どう?」「変更を見て」「品質チェックして」「マージ前に確認して」と言ったとき、あるいは実装が一段落して評価が欲しそうなときに必ず使うこと。バグ修正だけが目的の場合は別だが、設計・実装の質を評価する文脈では積極的に発動する。 |
| allowed-tools | Read, Bash, Grep, Glob, Agent, WebFetch, WebSearch, AskUserQuestion |
コードレビュースキル
現在のリポジトリの変更箇所とその周辺コードを読み込み、実装の質を多角的に評価する。
バグを見つけて直すのではなく、プロのレビュアーとして「この実装は良いか/長く保守できるか」を評価し、根拠とともに伝えることが目的。
基本姿勢
- 指摘の質で勝負する。粗探しではない。 重箱の隅をつつくより、本質的に効く指摘を少数精鋭で出すほうが価値が高い。逆に重大な問題は遠慮なく指摘する
- 必ず良い点も挙げる。 優れた設計判断・丁寧なエラーハンドリング・気の利いた抽象化は、明示的に評価する。レビューは改善のためであると同時に、良い判断を強化するためでもある
- 「なぜそう思うか」を必ず添える。 「ここはこうすべき」だけでは弱い。「なぜなら〜」「代わりに〜という選択肢もある」まで書いて初めて、相手が判断できる
- 断定しすぎない。 コードには文脈がある。レビュアーが見落としている制約があるかもしれない。確信度が低い指摘は「これは意図的かもしれないが」と前置きする
- そのプロジェクトの流儀を尊重する。 一般論として正しくても、そのリポジトリの既存パターンと衝突する提案は価値が低い。まず既存コードの慣習を読み取る
進め方
Step 1: レビュー対象(差分)の確定
何を見るかを最初に決める。判断材料を集める:
git branch --show-current
git status --porcelain
git log --oneline -10
git symbolic-ref --quiet refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@'
確定ロジック:
| 状況 | レビュー対象 |
|---|
| feature branchにいる | default branchとのmerge-baseからの差分(ブランチの仕事全体)+未コミットのstaged/unstaged |
| default branch (main/master) にいて未コミット変更がある | staged + unstaged の変更 |
| 引数やユーザー指示で範囲が明示されている | それに従う(特定コミット範囲、特定ファイルなど) |
差分の取得例:
BASE=$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)
git diff --stat "$BASE"...HEAD
git diff "$BASE"...HEAD
git diff HEAD
git diff --cached
範囲が曖昧で、かつ判断が結果を大きく左右する場合のみ AskUserQuestion で確認する。たいていは上のロジックで決まるので、決めて進めて冒頭で「〜を対象にレビューしました」と明示すればよい。
差分が巨大な場合(数十ファイル超)は、Agent(Explore)に並列でファイル群を読ませて要約させ、main contextを軽く保つ。CLAUDE.mdのSubagent Delegation方針に従う。
Step 2: 周辺コードと文脈の把握
差分のhunkだけを見て指摘してはいけない。 diffは変更行しか映さないので、それだけで質を判断すると的外れになる。最低限、以下を読む:
- 変更されたファイルの全体(関数・型の前後関係、既存の書き方)
- 変更された関数・型の呼び出し元と呼び出し先(影響範囲、契約の整合)
- 同種の処理の既存の実装パターン(このプロジェクトの流儀。新コードがそれに倣っているか)
- そのリポジトリの
CLAUDE.md / .claude/rules/(プロジェクト固有のルール。これは最優先の評価基準になる)
- テストファイル(後述の観点2で使う)
Step 3: specとの照合
仕様が書かれていることが多い場所を探す:
ls .spec/ 2>/dev/null
.spec/{slug}/spec.md、.cckiro/issues/、docs/ などに仕様・設計意図がないか確認する。
specがある場合の評価方針(重要):
- 完全な一字一句の一致は求めない。 実装中に判明した事情で合理的にずれるのは正常
- 見るべきは「設計の心を受け継いでいるか」。specが達成しようとした目的・非機能要件・制約が、実装で守られているか
- specから外れている箇所があれば、「それが合理的な逸脱か、それとも見落とし・手抜きか」を判断する。合理的ならむしろ良い判断として評価し、そうでなければ指摘する
- specが見つからない場合はその旨を述べ、コミットメッセージ・PR説明・コード自体から意図を推測してレビューする
Step 4: 多角的な評価
以下の観点で評価する。すべてを機械的に埋める必要はない——その変更で関係する観点に注力する。各指摘には根拠・具体的な代替案・確信度を添える。
観点1: 長期的な保守性・抽象化
- 冗長な構造になっていないか。同じ動作を別々のコードで表現していないか(DRY違反、コピペ実装)
- 逆に過剰な抽象化になっていないか。「将来使うかも」で導入された使われない柔軟性は負債
- 長期を見据えた適切な抽象化がされているか。変更が一箇所に閉じるか、それとも横断的に散らばるか
- 命名・構造だけでコードの意図が読めるか(コメントで補わないと読めない=構造に難あり)
- 責務の分離。一つの関数・型に責務が詰め込まれていないか
観点2: テストの十分性
- 実装した機能が期待通りに動くことを担保するテストがあるか
- 異常系のハンドリングが適切で、かつテストされているか(エラー時に何が起きるかが定義され、検証されているか)
- テストが「何を保証するか(WHAT/契約)」を表現しているか。内部実装に密結合した、リファクタで壊れる脆いテストになっていないか
- 境界値・エッジケースの考慮
- 外部サービスへの依存(LLM・SaaS API・クラウドサービス・外部DB・決済など)や、時刻・乱数といった不確定要素を含むものは、ユニットテスト(モック)だけでは動作保証にならないことが多い。モックは「自分が書いた期待」を検証するだけで、相手が本当にその通り振る舞うかは保証しない。実サービスに接続して、期待通りの導線で end-to-end に1本通るか(粗くてよい)を確認する仕組みがあるか。なければ「この種の機能はモックだけでは保証にならない。実接続のスモークテストを推奨」と指摘する
- カバレッジの形式値ではなく、「壊れたときにこのテストが落ちるか」で考える
観点3: spec適合(設計の心)
Step 3の結果をここで評価に反映する。設計意図を継いでいるか、逸脱が合理的か。
観点4: 実装の筋の良さ
- その言語のプロフェッショナルとして適切な書き方になっているか。イディオムに沿っているか
- 言語・標準ライブラリ・フレームワークの新しめの機能を適切に使えているか(古い書き方を惰性で踏襲していないか)。逆に、新機能を衒学的に使って読みにくくなっていないか
- エラーハンドリングの作法(言語・プロジェクトの規約に沿っているか。エラーを握り潰していないか、文脈を付けて伝播しているか)
- 並行性・リソース管理(リーク、競合、クローズ漏れ)
観点5: その他(変更内容に応じて)
- 正確性・バグ: ロジックの誤り、off-by-one、nil/null、競合状態。レビューの本筋ではないが、目についた明確なバグは指摘する
- セキュリティ: 入力検証、機密情報の露出、injection、認可。深いセキュリティ監査が要るなら
/security-review を案内する
- API・公開範囲の設計: 不必要に公開されている要素はないか。外部から変更されたら困るものを晒していないか
- マルチインスタンス安全性: プロセスメモリに跨りリクエストの状態を持っていないか(水平スケール前提で壊れないか)
- 後方互換・移行: 公開API・スキーマ・設定の破壊的変更に、移行手段や告知の配慮があるか
- 観測性: 適切なログ・メトリクス。逆にログにPII/秘密を漏らしていないか
- ドキュメント: 新機能・API変更・新依存・新設定に対し、対応するドキュメント更新が差分に含まれているか
プロジェクトの CLAUDE.md / .claude/rules/ に固有ルールがある場合、それらは上記の一般観点より優先される具体的評価基準として扱う。違反は明確な指摘対象。
Step 5: レポート出力
指摘事項だけをまとめる。 全体総評・良い点・観点別の評価表などは出力しない(求められたら別途答える)。深刻度で整理し、各指摘に file:line・根拠・提案・確信度を付ける。
冒頭に「何を対象にレビューしたか」だけは1行で明示する(範囲の取り違えを防ぐため)。指摘がゼロの観点・深刻度は、その見出しごと省く。
深刻度の凡例:
- 🔴 Must-fix — マージ前に直すべき。バグ、規約違反、設計上の重大な問題
- 🟡 Should-fix — 直したほうが良い。保守性・テスト・筋の問題で、放置すると後で効いてくる
- 🟢 Consider — 任意。好みや軽微な改善。指摘するが強制しない
## コードレビュー結果
**対象**: [例: `feat/foo` ブランチの origin/main からの差分(12ファイル)+未コミット分]
### 🔴 Must-fix
#### 1. [タイトル] — `path/to/file:42`
- **問題**: [何が問題か]
- **なぜ**: [なぜ問題か。何が起きるか]
- **提案**: [具体的にどうするか。コード片があるとなお良い]
- **確信度**: 高 / 中 / 低(低い場合は「意図的なら無視してよい」と添える)
### 🟡 Should-fix
[同上の形式]
### 🟢 Consider
[簡潔に。箇条書きでも可]
指摘が一件も無ければ、無理にひねり出さず「重大な指摘なし」と一言で伝える。
やらないこと
- コードの修正そのもの。 これは評価スキル。修正してほしいと言われたら、それは別のアクション(指摘を提示した上で、ユーザーが「直して」と言えば対応する)
- 粗探しのための粗探し。 価値のない指摘でレポートを水増ししない
- PRのレビューコメント対応。 それは
/check-pr の役割
- ツールベースの機械的スキャン。 secret/依存脆弱性のスキャンは
/security-scan、深いセキュリティ監査は /security-review を案内する