code-review について
code-review は、ブランチ、プルリクエスト、未コミットの変更をリスク優先でレビューするスキルです。指摘にはコード上の根拠を求め、段階的な監査と相応の検証を定め、依頼があれば修正と再レビューを行います。
主な機能
- 01基準点と範囲の事前確認
- 02順序に沿った6種類の監査
- 03ファイル・行・コードの根拠
- 04リスクに応じた検証指針
- 05条件付きの修正と再レビュー
- 06公開前のセルフチェック
活用シーン
- 01プルリクエストやブランチのレビュー
- 02ステージ済み・未ステージ変更の監査
- 03セキュリティと信頼性リスクの優先確認
- 04全差分を使った修正内容の検証
インストール
コマンドをコピーしてターミナルで実行します。
npx skills add https:/ / github.com/ whhe/ ai-workshop --skill code-reviewインストール後、Claude Code で/code-reviewを入力して読み込まれたか確認します。
同梱ファイル
SKILL.md と一緒にインストールされる参考資料やスクリプト。
README.mdreferences/code-quality-checklist.mdoutput-template.mdremoval-plan.mdsecurity-checklist.mdsolid-checklist.md詳細
絶対原則:すべての指摘には、ファイル、行範囲、コードの根拠を必ず記載してください。根拠がない場合は指摘しないでください。
コードレビュー — リスク優先の段階的検証
ワークフロー
- ステップ 1:事前確認 ⚠️ 必須
- ステップ 2:リスク優先監査(パス A~F)⚠️ 必須
- ステップ 3:指摘事項の提示 ⚠️ 必須
- ステップ 4:修正ループ(条件付き — 範囲はステップ 3 で設定)
- ステップ 5:公開前のセルフチェック ⚠️ 必須
1. 事前確認 — ベースラインの確立 ⚠️ 必須
PR URL の場合は、プラットフォームの差分とメタデータ(ベース/ヘッドの参照、説明、コメント、CI ステータス)を取得します。ローカルのベースラインを計算する前に、ローカルでレビューするヘッドがその PR に対応していることを確認します。
レビュー範囲を確定し、レビューを始める前に baseline_commit を固定します。
- ブランチまたは PR のレビューでは、ベースブランチを解決し、初期レビュー対象のヘッドとのマージベース(
git merge-base <base> HEAD)をbaseline_commitに設定します。ユーザーまたは PR のメタデータで別のブランチが指定されていない限り、baseはmainにします。 - 未コミットの変更をレビューする場合は、初期
HEADをbaseline_commitに設定します。 - ユーザーが指定したパスの制限を記録します。修正がコミットされた場合も含め、修正ループ全体を通じてベースラインを固定します。
その範囲内で、以下を並行して収集します。
- ブランチの状態:
git status --short --branch - ベースライン以降のコミット:
git log --oneline --decorate --no-merges <baseline_commit>..HEAD - コミット済みの変更、ステージ済みの変更、ステージされていない変更を含む追跡対象の変更:
git diff --stat <baseline_commit>、git diff --name-only <baseline_commit>、git diff <baseline_commit> - ステージングの各状態:ステージ済みの編集とステージされていない編集が共存する場合は、
git diff --cached <baseline_commit>とgit diffも確認し、最終的な作業ツリーで相殺される編集によってステージ済みの状態が隠れないようにします。 - 新しいファイル:
git ls-files --others --exclude-standard。追跡対象の差分には内容が含まれないため、範囲内の各ファイルを個別に読みます。
関連するモジュール、使用箇所、契約、エントリーポイント、所有権の境界、重要な処理経路を調査します。
2. リスク優先監査 ⚠️ 必須
確定した範囲内のすべての変更をレビューします。該当する場合は、ブランチ/PR のすべてのコミットと、範囲内の未コミットの変更および新しいファイルも含めます。各パスを順番に実行します。
パス A — 動作のリグレッションと互換性
- ブランチのロジック、デフォルト値、機能フラグ、入出力の契約に変更はありましたか?
- API のパラメーター、デフォルト値、旧形式データの処理は後方互換性がありますか?
- 命名や戻り値の意味は既存のコードベースと一致していますか?「インターフェースは X を示すが、実装は Y を行う」という不一致を指摘してください。
既存ロジックに影響する変更については、それぞれ「元の動作 → 新しい動作、影響範囲、リスク」を明記します。
パス B — SOLID とアーキテクチャ
- SRP:このモジュールを変更する理由は 1 つだけですか?
- OCP:既存のコードを編集せずに新しいバリアントを追加できますか?
- LSP:呼び出し側に気付かれずに、任意のサブクラスで親クラスを置き換えられますか?
- ISP:すべての実装者がインターフェースのすべてのメソッドを使用していますか?
- DIP:ビジネスロジックが具体的な I/O、ストレージ、ハードコードされた値に結び付いていませんか?
- ドメインの制約を覆い隠す、過度に緩い型(
Any/any/dict/型指定なし)はありませんか?
大規模な書き直しより、段階的なリファクタリング計画を優先してください。詳しいコード上の問題点を確認するには solid-checklist.md を読み込んでください。
パス C — セキュリティと信頼性
- 認証されていないユーザー、または権限のないユーザーがこのコードパスに到達できますか?テナント A のデータがテナント B に漏れる可能性はありますか?
- 2 つのリクエストが同時にこの処理を実行すると、どうなりますか?確認から実行までの間に状態が変わる可能性はありますか?
- この処理が途中で失敗した場合、データの整合性は保たれますか?
- バージョンが固定されていない依存関係、依存関係の取り違え、既知の CVE はありませんか?
- HTTP クライアントで TLS 検証が無効になっていませんか(
verify=False、InsecureSkipVerify、rejectUnauthorized: false)?
悪用可能性と影響の両方を明記してください。脅威モデルを詳しく分析するには security-checklist.md を読み込んでください。
パス D — パフォーマンスと安定性
- データ量が10倍/100倍になった場合、どうなりますか?リソースの使用量は制限され、解放されますか?
- N+1クエリはありませんか?キャッシュまたはバッチ処理が可能な、ホットパスでのCPU負荷の高い処理はありませんか?
- 障害発生時、デバッグに十分なコンテキストがありますか?ログやエラー出力にシークレットが漏れていませんか?
- この値がnull、0、または空の場合、どうなりますか?
詳細なサブチェックについては、code-quality-checklist.md を読み込んでください。
パス E — 削除候補
- 未使用、冗長、または機能フラグで無効化されているコードを特定します。
- 今すぐ安全に削除と計画を立てて延期を区別します。
候補が見つかった場合は、removal-plan.md を読み込んでください。
パス F — テストカバレッジと言語固有のチェック
- 変更された外部から観測可能な契約について、そのリスクに見合う、十分に識別力のある検証があるか判断します。新しいテストを追加する前に、既存のテストとチェックを再利用してください。
- 変更されたロジックが実質的に未検証で、より小規模な既存のチェック、静的チェック、または手動チェックではリスクをカバーできない場合に限り、テスト不足を回帰リスクとして扱います。
- バグ修正では、現在のテストスイートで元の不具合を検出できず、そのケースが安定していて保存する価値がある場合に限り、焦点を絞った回帰テストを追加します。同じ元の不具合に対して修正前に既存の識別力のあるテストがすでに失敗する場合、または変更がドキュメント、設定、配線のみ、もしくは動作を変えないリファクタリングである場合は、重複するテストを追加しないでください。
- モジュール間の契約または境界が焦点を絞ったチェックでは十分に検証できない場合に限り、統合レベルのカバレッジを求めます。同じ外部から観測可能な結果を証明するユニットテストと統合テストを重複させないでください。
- アサーションの識別力:フィクスチャとアサーションによって、実際の動作とバイパス/何もしない動作を区別できることを確認します。これには、使用したデータではフィルター適用後と未適用の結果が同じに見えるケースや、正常系が1つあるだけでは分岐が実行されたことを証明できないケースも含まれます。機能に応じて、件数、識別情報、否定ケース、または観測可能な効果を用いて検証を強化してください。
言語固有の注意点(一般的なパスでは見落とす可能性がある、分かりにくい項目):
| 言語 | チェック項目 | 該当する場合 | パス |
|---|---|---|---|
| Python | naive/aware な datetime の混在 | 明示的な変換なしに、naive と aware の datetime オブジェクトを比較、減算、または同じフィールドに代入している | A |
| Python | デタッチされたインスタンスに対するORMの遅延読み込み | オブジェクトを読み込んだセッションが閉じた後に、ORMのリレーションシップまたは遅延カラムにアクセスしている | D |
| Python | ORMの方言間の移植性 | プロジェクトがサポートするすべてのバックエンド向けにORMが変換できるAPIではなく、エンジン固有のSQL、ベンダー専用関数、生のSQLフラグメント、または方言固有の型に依存するクエリを使用している。サポート対象の各方言/バージョンに対して、コンパイルまたは実行を確認する | A, D |
| TS/React | useEffect/useMemo/useCallback の依存関係の問題 | リアクティブな依存関係が欠落または冗長である、あるいは不安定な参照(レンダーごとに新しいオブジェクト/配列/関数が作成される)になっている | D |
| JS/Node | ガードのないグローバルパッチ/ポリフィル | モンキーパッチまたはポリフィルが無条件に実行される。環境チェック、機能検出、スコープの分離がない | A |
3. 調査結果の提示 ⚠️ 必須
まず報告してください — 変更を加える前に調査結果を提示してください。 ドキュメントの構成、重要度レベル、証拠に関するルールについては、output-template.md を読み込んでください。
提示後:
- ユーザーの最初の依頼に修正が明示的に含まれている場合(例:「レビューして修正」「すべて修正」「レビューしてからすべての問題を修正」):確認を求めず、すべての調査結果を対象として自動的に修正ループに進みます。ユーザーが修正範囲を明示的に限定している場合(例:「重要度 High のみ修正」)に限り、これを優先します。否定例 — 次の表現は修正依頼に該当しません:「この修正をレビューして」「自分のバグ修正ブランチをレビューして」「この修正が正しいか確認して」 — ここでの「修正」は既存の作業を指しており、修正を適用する指示ではありません。
- それ以外の場合:どのように進めるか(すべて修正/重要度 High のみ修正/特定の項目を修正/変更なし)をユーザーに確認してください。ユーザーが確認するまで変更を実装しないでください。
4. 修正ループ(条件付き)
⛔ ステップ 3 で修正範囲が確定した後にのみ開始してください。範囲内の問題のみ修正してください。
各イテレーション:
-
修正前のベースラインを確立するため、関連する最小限の検証を実行します。
-
最小限の修正を適用します(新しい挙動を持ち込まないようにします)。リスク評価から変更した契約を保護する必要があると判断された場合に限り、対象を絞ったテストを追加または更新します。
-
同じ検証を再実行して、修正を確認します。
-
ステップ 1 で固定した
baseline_commitに対する、蓄積された変更全体を敵対的に再レビューします。git diff <baseline_commit>と、範囲内の新規ファイルすべてを読み直し、該当する場合はステップ 1 のステージング層ごとの個別の差分も確認します。これには、コミット済み、ステージ済み、未ステージ、未追跡のいずれであっても、元の変更とすべての修正が含まれます。<base>..HEADのようなコミットのみの差分では未コミットの修正が漏れ、作業ツリーのみの差分ではブランチのコミットが漏れます。元の範囲とベースラインを維持し、直近のパッチだけをレビューしないでください。再レビューは敵対的でなければなりません。修正によって新たな問題が生じたという前提を標準とします。修正を受け入れる前に、レビュアーはその正しさを積極的に反証しようとする必要があります。本番環境で観測可能な障害を引き起こす、または文書化された契約に違反する問題のみを指摘してください。起こりそうにない複数の条件が重ならなければ生じない理論上の懸念は、調査結果に含めません。
独立した判断:再レビューのたびに、現在の差分全体、プロジェクトの規約、影響を受ける呼び出し元を読み直してください。以前の調査結果や修正の根拠は、現在のコードに照らして検証すべき主張として扱います。追跡可能性を保つため、イテレーションの履歴を残してください。
パス A、C、D、F を適用します(B と E は初回監査で扱います)。変更した各領域について、受け入れる前に現実的な障害シナリオを考えてください。独立したレビュアーを利用でき、委任が許可されている場合は、意図する結論や修正の根拠を先に示さず、プロジェクトの背景情報と差分全体を提供してください。それ以外の場合は、同じ証拠に基づくレビューをその場で行います。会話の文脈が消去されたと主張してはいけません。
-
再レビュー(ステップ 4)で合意済みの修正範囲内に新たな問題が見つかった場合に限り、範囲を拡大します。範囲外の調査結果が見つかった場合は、修正ループの反復表に記録し、修正ループの終了時にユーザーへ報告してください。
関連する検証が成功し、差分全体の最新の再レビューで範囲内の問題が残っていないと確認できるまで、反復回数に上限を設けず続けてください。前提条件の不足、許可された範囲外の判断が必要なこと、または新たな証拠や改善のない試行の繰り返しにより先に進めない場合は、未解決の調査結果、検証状況、具体的な阻害要因を報告して停止してください。反復回数だけを理由に停止したり、許可の再取得を求めたりしてはいけません。 イテレーションの追跡:各イテレーションの調査結果と修正内容を記録します。最終レポートには、ユーザーが修正履歴を追跡できるよう、イテレーションごとの概要(イテレーション番号、対応した問題、適用した修正、検証結果、再レビューの結果)を含めてください(output-template.md § 修正ループのイテレーションを参照)。
テスト戦略(プロジェクトに応じて調整してください):
- 変更した契約を検証できる、既存の最小限のチェックを実行してから、変更が境界をまたぐ場合、または対象を絞ったチェックで関連する依存関係が明らかになった場合に限り、範囲を広げます。
- Python:まず対象のテストまたは既存の対象を絞ったチェックを実行し、変更内容から必要と判断される場合にのみ、関連ディレクトリのチェックを実行します。
- フロントエンド:まず影響を受けるコンポーネント/ページのチェックを実行し、変更がそれらの契約をまたぐ場合にのみ、関連するテストスイートを実行します。
- API の変更:既存のカバレッジを使って、変更したリクエスト/レスポンスまたはエラーの契約を検証します。その契約がほかの方法で検証されていない場合に限り、判別に有効なケースを1つ追加します。
- カバレッジを増やすこと、実装の分岐をそのままテストに反映すること、またはレポートを完全に見せることだけを目的にテストを作成しないでください。起こり得る回帰で失敗しないテストは有用ではありません。既存のチェックで十分な場合、または対象のリスクが低い場合は、新しいテストが不要だった理由を記録してください。
- 前提条件の不足(コマンドが見つからない、依存関係が不足している):影響範囲を明記して「未検証」と記載します。再現可能なコマンドを提示し、結果を偽装しないでください。
5. 公開前のセルフチェック ⚠️ 必須
最終化する前に、output-template.md § 公開前のセルフチェックにあるセルフチェックを実行してください。
制約とアンチパターン
絶対に行わないこと:
- デフォルト値の変更による暗黙的な動作変更を無視する。
- 未検証の項目を「合格」とする。
- テストを通すために重要なチェックを回避する(認証を削除する、例外を握りつぶす、セキュリティを弱める)。
- ユーザーから明示的な依頼がない限り、コミットまたはプッシュする。
- 調査結果を提示する前に修正する。
避けること:
- セキュリティや正確性より先に、スタイルや命名の問題を列挙する。
- チェックした内容を列挙せずに「問題は見つかりませんでした」と報告する。
- 問題点を説明せずにコードをコピー&ペーストする。
- すべての指摘を同じ重大度として扱う。
- 実際のコードや差分を読まずに動作を推測する。
リソース
必要な場合にのみ読み込みます。指摘の詳しい分析が必要なときに限ります。
| ファイル | 読み込むタイミング |
|---|---|
| solid-checklist.md | Pass B の指摘に、コードの臭いやリファクタリングのヒューリスティックが必要な場合 |
| security-checklist.md | Pass C の指摘に、脅威モデルの詳細が必要な場合 |
| code-quality-checklist.md | Pass D の指摘に、安定性/パフォーマンスのサブチェックが必要な場合 |
| removal-plan.md | Pass E で削除候補が見つかった場合 |
| output-template.md | Step 3 と 5 — ドキュメントの構成、根拠のルール、セルフチェック |