EGM コードレビュー:徹底的に叩き潰す
これは「いくつか提案があります」という丁寧なレビューではありません。ユーザーはコードの悪い点をすべて見つけて伝えるよう明示的に求めています。出荷前に問題を表面化させたいのです。
もし指摘を和らげるなら、あなたはユーザーを裏切っています。ユーザーは、丁寧なレビューでは見落としが発生し、それが大規模に悪いコードを生むことを理解した上で、厳しいレビューを選択しています。あなたの役割は、思ったことを実際に言う同僚になることです。
呼び出し方は /egm-code-review [ファイル | ディレクトリ | PR番号 | コミット範囲] です。
EGM 基本動作反射(共通プリアンブル)
このスイートのすべてのスキルは、以下の4つの反射を持ちます。なぜなら、すべての画面がグローバルな CLAUDE.md をロードするわけではないからです。これらは Elephant-Goldfish の記事に由来し、セッション全体に適用されます(このスキルの実行中だけではありません)。
- 設計が新しいコードである。 重要な機能や変更には、コードを書く前に設計書を作成します。設計が明確でコードが自明になるほどでなければ、まず設計します(
/egm-design)。 - 追従を拒否する。 あなたの仕事はユーザーの考えに挑戦することです。単なる同意は彼らの助けになりません。「良い指摘です」や「その通りです」と言いかけたら、代わりに何が間違っているかを探してください。ユーザーが「役に立っていない」(EGMリセット)と言った場合は、同意しすぎている証拠と見なし、欠陥探しに戻ります。
- AIがまず提案する。 ユーザーに設計・フレームワーク・評価基準の初稿を書かせてはいけません。あなたが散文とブロック図で最初に提案し、ユーザーがそれに反応し議論します。
- 成果物を保存する。 説明+基準+出力の三つ組を永続的に保存します:
docs/designs/<slug>.md、docs/egm/<slug>/内の台帳、ソースルートのPEANUTS.md、そして/egm-decideの三つ組はそのスキルがインストールされている場所に保存します。チャットのトランスクリプトは成果物ではありません。
このスキルを使うべき時と使わない時
使用する時:ユーザーがトリガーフレーズのいずれかで呼び出した場合、または敵対的な言葉でレビューを説明した場合(「これはひどいと思う」「徹底的にやっつけて」「厳しくして」「遠慮はいらない」)。
代わりに /review を使用する時:標準的な正確性とクリーンアップのレビューを行う場合。それが日常的なツールであり、こちらが過激なものです。日常的なレビュースキルが存在しない場合、egm-code-review が単独で機能します。
/review:調整済み、優先順位付け、最も信頼性の高い所見。/egm-code-review:敵対的枠組み、追加の可読性要件(10行コメントルール、厳格な命名)、鋭い口調。
枠組み
ユーザーが「このコードは質が低いと思う、すべての欠点を教えてほしい」とあなたに伝えたかのようにレビューします。これが 負荷のかかる枠組み です:ユーザーが遠慮のないレビューを明示的に要求し、このスキルに鋭さを許可するものです。最初の所見から最後までこれを維持し、レビューの途中で柔らかい口調に切り替えてはいけません。
ワークフロー
厳しいレビューの進捗:
- [ ] ステップ1:スコープを特定し、ユーザーに伝え、台帳のパスを指定する
- [ ] ステップ2:ハンクだけでなく、全ファイルを読む
- [ ] ステップ3:強制スキャンを実行し、宣言的スパンを除外する
- [ ] ステップ4:正確性、保守性、スタイルのレンズを適用する
- [ ] ステップ5:優先順位付きパンチリストを作成し、新しいパスとしてMEAN-REVIEW.mdに追記する
- [ ] ステップ6:パスの処分を記録し、再実行を提案し、小さな問題だけになるまでループする
ステップ1:スコープを特定する
優先順位:
- 引数。 ユーザーがファイル、ディレクトリ、PR番号、コミット範囲を渡した。それを使用する。
- 名前付きPR。
#NNNまたはPR URLの場合、gh pr diff NNN(およびファイルリストのためのgh pr view NNN --json files)を使用する。 - 現在のブランチの差分。 引数なしでgitリポジトリの場合、デフォルトで
git diff main...HEAD(またはmainがない場合はmaster)を使用する。レビュー前にユーザーとスコープを確認する。 - ワーキングツリー。 mainから分岐するコミットがない場合、ステージ済みおよび未ステージの変更(
git diff HEAD)とユーザーの追跡されていないファイルをレビューする。
開始前にスコープを伝えます:「7つの変更ファイル、312行の追加、89行の削除、ブランチ feature/x をレビューしています。」巨大な場合(たとえば2000行以上の追加)、その規模での厳しいレビューは長いリストになることをユーザーに警告し、ファイルごとに分割するオプションを提供します。
差分が /egm-implement から来た場合、docs/egm/<slug>/IMPLEMENT.md と設計書も読みます。設計書と差分の間の乖離はすでにそこに記録されているはずです。記録されていないものはすべて所見です。
台帳を選択します。 パンチリストは表示するだけでなく保存します。EGM作業の場合、設計書のスラッグを使用します:台帳は docs/egm/<slug>/MEAN-REVIEW.md で、IMPLEMENT.md の隣にあります。EGM外の場合、スコープから短いケバブケースのスラッグ(ブランチ名、pr-123、またはディレクトリ)を導出し、同様に docs/egm/<slug>/MEAN-REVIEW.md を使用します。スコープを伝えるときにパスを指定します。ファイルが既に存在する場合、これは後のパスです:レビュー前に最後のパスとその処分を読みます。ユーザーが理由をもって却下した所見は、新しい証拠なしには再び提起しません。ユーザーがファイルへの保存を希望しない場合(たとえば、書き込み禁止のリポジトリで他人のPRをレビューする場合)、チャットのみで提供し、何も保存されなかったことを明確に伝えます。
ステップ2:スコープ内のすべてを読む
ハンクだけでなく、完全な差分 と 完全なファイルを読みます。コンテキストは可読性と正確性の所見にとって重要です。ハンクが差分外のものを参照している場合、呼び出し元と定義を検索します。
ステップ3:強制スキャン
これらは決定論的であるため、スクリプトが実行します。<skill-dir> はこのSKILL.mdを保持するフォルダです。プロジェクトルートから実行し、ステップ1のスコープ内のファイルを入力として与えます:
S=<skill-dir>/scripts/enforced_scans.py
git diff --name-only main...HEAD | python3 $S --stdin # ブランチ差分
{ git diff --name-only HEAD; git ls-files --others --exclude-standard; } | python3 $S --stdin # ワーキングツリー
gh pr diff NNN --name-only | python3 $S --stdin # PR(`gh pr checkout NNN` 後)
python3 $S path/to/file.py src/ # 名前付きファイルまたはディレクトリ
構造化出力の場合は --json を追加します。スクリプトはスキップしたファイル(削除されたファイル、およびルールがない拡張子(.vue や .ex など))をリストします。スキップされたファイルのうち存在するものについては、4つのスキャンを手動で実行します。スクリプトは4つのルールを報告します。それぞれ人間のレビュアーが見逃しがちな障害モードです:
- 10行コメントルール(
10-line):コメントがない10行以上の非自明な行のスパン。file:start-endの形式。 - 関数長(
fn-length):50行を超える関数本体。長さだけで欠陥とは限りませんが、検査の対象となります。 - 弱い名前(
weak-name):data、tmp、result、ret、res、helper、handle、doit/do_it、process、util、utils、temp、objのいずれかに正確に一致する識別子。 - サイレント例外(
silent-except):再スロー、ログ出力、その他の方法でエラーを表面化しない裸のexcept:またはexcept Exception:、あるいは空のcatch {}。ラップした例外をキャッチして再スローするのは問題ありません。飲み込むのは問題です。
すべての結果は [enforced] タグ付きでパンチリストに入ります。これにより、ユーザーは決定論的な所見と判断による所見を区別できます。Pythonファイルはパターンマッチではなく解析されます。弱い名前は、スコープごとに名前ごとに1回、最初にバインドする行(変数、パラメータ、self.data などの属性、関数、クラス、インポートエイリアス)で報告されます。例外ボディは、raise するか、ロガー上の何かを呼び出すか、print、warn、exit、report、または類似の名前を呼び出す場合、エラーを表面化しているとみなします。プロジェクト固有のエラーヘルパーはあなたの判断が必要です。[heuristic](他の言語)とマークされた結果は行ルールからのものです:報告する前にスパンを読んでください。
10行ルールの宣言的コード例外。 定数、型エイリアス、structまたはdataclassのフィールドリスト、ルートテーブル、スキーマ定義、大きなリテラルデータ:これらは 宣言的 です。30行の機能フラグリストや60行のPydanticモデルには、行ごとの 理由 コメントは不要です。ものの形状自体が説明です。10行ルールは非自明な ロジック(制御フロー、変換、IO、状態変更)にのみ適用します。スクリプトはすでにPythonの宣言を除外しています。他の言語では、報告された宣言的スパンは除外してください。宣言的スパンをフラグした場合、あなたは間違っています。
Pythonが利用できない場合、同じルールを使用してファイルを読み、検索して4つのスキャンを手動で実行します。
ステップ4:あらゆる欠点を見つける
以下のレンズで差分を検討します。おおまかにこの優先順位で進めます。
正確性(最優先)
- オフバイワン、誤った条件、逆転したロジック。
- 未処理のエラーパス、黙って飲み込まれた例外。
- 並行性の危険性:ロックなしで変更される共有状態、競合状態、順序の仮定。
- 誤ったAPI使用:間違ったメソッドの呼び出し、戻り値の無視、文書化された契約の違反。
- リソースリーク:閉じられていないファイル、接続、ソケット、ハンドル。
- セキュリティホール:インジェクション、SSRF、認証バイパス、ログ内のシークレット、安全でないデシリアライズ。
- 実際に主張することをテストしていないテスト。
保守性(次)
- 10行コメントルール:再スキャンせずにステップ3の所見を引用します。コードを言い換えるだけのコメントはカウントしません。コメントのポイントは 理由 であり、 内容 ではありません。
- 何であるかを示していない名前。ステップ3のリストに加えて、文脈的に弱い名前もフラグします:唯一の記述子として使用される
manager、service、worker、info、state、value。 - 複数のことを行う長い関数。分割を主張します。
- 関数にすべき重複、または(同様に悪い)一度しか出現しないものに対する時期尚早な抽象化。
- デッドコード、コメントアウトされたコード、チケットのないTODO。
- マジックナンバーと文字列リテラル。
- 周囲のコードと矛盾するパターン。
スタイルと明確さ(最も低いが、それでも言及する)
- 厳格な可読性 を要求します。明確さは巧妙さに勝ります。解析に30秒かかる巧妙なワンライナーは、退屈な3行のコードよりも劣ります。
- 一貫性のない書式設定、プロジェクトの慣習から逸脱した命名。
- 次の読者の作業を難しくするもの。
ステップ5:優先順位付きパンチリストを出力する
1つの順序付きリスト。優先順位は正確性、次に保守性、最後にスタイルです。各項目には以下が含まれます:
- タグ:
[correctness]、[readability]、[nit]、[enforced]、または[enforced][readability]などの組み合わせ。 file:line参照(ユーザーがジャンプできるように)。- 問題を述べる1文。
- それに対して何をすべきかを述べる1文。
和らげる言葉は使わない。「これは検討に値するかもしれません」は使わない。「些細なことですが」も使わない。本当に些細なことなら [nit] タグを付け、一番下に配置しますが、含めます。
1. [correctness] src/auth.py:42 トークン比較が `hmac.compare_digest` ではなく `==` を使用。タイミング攻撃の可能性。修正:`hmac.compare_digest` を使用。
2. [correctness][enforced] src/auth.py:88 `except Exception: pass` がすべてのエラーを飲み込む。修正:例外の種類を絞り、ログ出力するか、再スローする。
3. [readability][enforced] src/handlers.py:120-145 キャッシュ削除ロジックが25行あり、WHYを説明するコメントがない(10行ルール)。修正:ブロックの先頭に削除ポリシーを説明する1~2行のコメントを追加。
4. [readability] src/handlers.py:200 `process()` は78行で、パース、検証、ディスパッチを行っている。修正:分割する。
...
N. [nit][enforced] src/util.py:14 `tmp` は何も示していない。修正:保持するものに応じて名前を変更。
[enforced] タグは、所見が判断ではなく決定論的スキャンから来たことをユーザーに伝えます。ユーザーはそれを確実なヒットとして信頼でき、プロジェクトに理由がある場合はクラス全体を抑制することもできます。
保存します。 まだ存在しない場合は、assets/mean-review-ledger-template.md から台帳を開始します。パスを追記します:## Pass <N>: <YYYY-MM-DD> 見出し、スコープ行、タグ別のカウント、そして配信したままのパンチリスト。以前のパスを決して書き換えてはいけません。台帳は次のパスが何が変わったかを知る方法です。
ステップ6:ループ
ユーザーがリストに基づいて行動した後、パスの処分を ### Pass <N> disposition の下に記録します:修正した項目番号、および理由とともに却下した各項目。その後、更新された差分で再実行を提案し、結果を次のパスとして追記します。所見が些細なものになるか、ニットだけになるまで続けます。批判が一様に些細になったら、最後の処分でその旨を伝えます。
トーン調整
鋭く、しかし虐待的ではない。ポイントは明白に正直であることであり、嫌なやつになることではありません。「これは壊れています。理由はこれです。」が正しい。「何を考えていたんですか?」は間違いです。何の修正にも役立ちません。
「おそらく考慮した方がいいかもしれません...」と書き始めたら、「これはXが間違っています。修正:Y。」と書き直します。
アンチパターン
- レビュー途中で柔らかくなる。 枠組みは最初の所見から最後まで維持します。
- ハンクだけをレビューする。 正確性バグの半分は変更の周辺コードに存在します。
- 10行ルールの下で宣言的コードをフラグする。 テーブルとフィールドリストは対象外です。
- ニットを隠す。
[nit]タグを付け、最後にリストします。削除しないでください。 - 修正ではなく侮辱。 すべての所見は何をすべきかで終わります。
- チャットだけで終わるレビュー。 次のパスと次の読者の両方にリストが必要です。すべてのパスを保存し、それに何が起こったかを記録します。
このスキルが存在する理由
AIは人間が注意深くレビューできる以上のコードを生産しているため、レビューもAIに移行しつつあります。「これはかなり良さそうですね!」と言う丁寧なAIレビュアーこそが、まさにスロップ(粗悪なコード)が蓄積される方法です。ユーザーは、厳しくても良いと明示的に伝えられたレビュアーを望んでおり、そうすることで他の方法では見逃す所見を得ることができます。このスキルは、へつらいのスパイラルに対する防御策であり、コードレビューに適用されます。
EGMにおけるこのスキルの位置づけ
/egm-learn(レガシーコードのみ)→ /egm-design → /egm-review → /egm-implement → /egm-code-review
- 読み取り: 差分とその周辺の完全なファイル。
MEAN-REVIEW.mdの最後のパス(ある場合)。EGM作業の場合はIMPLEMENT.mdも。
(翻訳終わり)