code-review-standards

コードレビュー基準

目的

このスキルでは、コード生成パイプラインのステージ 4 で code-critic エージェントが適用する、体系的なチェックリストを定義します。チェック項目には重大度が付けられているため、PM とエンジニアの双方が、どの指摘が納品を妨げ、どれが参考情報なのかを正確に把握できます。エンジニアは、レビュー担当にチェックを依頼する前のセルフレビューにこのスキルを使用できます。

このチェックリストが必要なのは、マルチエージェントシステムで体系化されていないコードレビューを行うと、一貫性のない指摘が生じるためです。あるレビュー担当は命名を指摘し、別のレビュー担当はセキュリティを指摘する一方、同じ項目を指摘するとは限りません。重大度を付けることで、レビュー担当が変わってもレビューの結果が安定します。

重大度付きチェックリスト

CRITICAL（必ず修正。納品を妨げます）

• シークレット、API キー、認証情報をハードコードしていない
• SQL インジェクションの攻撃経路がない（パラメーター化クエリーのみを使用）
• 任意のコード実行経路がない（eval、exec、制限のない pickle.loads を使用しない）
• 認証／認可を回避できない
• 脱出条件のない無限ループがない

HIGH（必ず修正。納品を妨げます）

• すべての公開関数とクラスに型ヒントがある
• mypy --strict がエラー 0 件で通る
• pytest が失敗 0 件で通る
• 新規コードのテストカバレッジが 90% 以上
• bare except 節がない
• 可変なデフォルト引数がない
• グローバルな可変状態がない
• async 関数内で同期 I/O を行っていない
• N+1 クエリのパターンがない
• エラーケースを明示的に処理している（黙って無視していない）

MEDIUM（報告書に記載して指摘し、そのまま進めます）

• 関数は 20 行以下（できれば 10 行以下）
• ハッシュマップで計算量を減らせる箇所に、ネストしたループを使用していない
• 該当する場合、list.pop(0) を deque.popleft() に置き換えている
• 適切な場合、asyncio.gather で return_exceptions=True を使用している
• 非同期処理に明示的なタイムアウトを設定している
• 公開メソッドに docstring がある（Google または NumPy 形式）
• 本番コードのパスに Any 型を使用していない

効率性（criteria-efficiency.md を参照）：

• ハッシュマップ検索にするべき 2 つのコレクションを対象としたネストしたループがない（O(n*m) → O(n+m)）
• 反復ごとの I/O（ループ内のクエリー／RPC／フェッチ）がない。ループの外で一括処理する（頻繁に実行されるパスでは HIGH。「N+1 クエリのパターンがない」を参照）
• 繰り返し行う深いプロパティ／セレクターの解決結果をローカル変数にキャッシュしている（貪欲なデータアクセスを避ける）
• ループ内で文字列を連結する場合、+= ではなく list+join／StringBuilder を使用している
• 本番クエリのパスに SELECT * や過剰なデータ取得がない

移植性（criteria-transferability.md を参照）：

• デッドコード／到達不能コードがない（無条件の return／break／raise の後にある文、呼び出されないプライベートメンバー）
• 1 つのキーに対する長い if/else if チェーン（分岐が 3 つ以上）を switch/match またはディスパッチマップに置き換えている
• ネストした switch／match がない。内側の switch を名前付き関数に切り出している
• 実際の値と矛盾するなど、誤解を招く名前がない

LOW（記録のみ）

• PEP 8 に準拠している（black + isort で自動的に処理）
• 変数名が明確で、内容を適切に表している
• コメントアウトされたコードが残っていない
• インポートの順序が整っている
• 変更箇所全体で命名が一貫している（同じ概念には同じ名前を使い、大文字小文字の使い方も統一）
• 1 つのファイルが多くの責務を抱えていない（800 行のファイル上限を参照）

判定フォーマット

クリティックの出力は、必ず1行目に判定を記載し、その後に検出事項の表、最後に要約の段落を続けてください。

1行目のフォーマット：

VERDICT: APPROVE

または

VERDICT: WARN

または

VERDICT: BLOCK

検出事項の表のフォーマット：

重大度	ファイル	行	問題	必要な修正
CRITICAL	auth.py	47	ハードコードされた API キー sk-...	環境変数に移し、.env.example に追加する
HIGH	fetcher.py	23	async def 内で requests.get() が呼び出されている	await httpx.AsyncClient().get() に置き換える
MEDIUM	parser.py	88	関数が34行ある	_parse_headers() ヘルパーを切り出す

判定の定義：

判定	条件	PM の対応
APPROVE	CRITICAL の検出事項がゼロ、HIGH の検出事項がゼロ	ステージ5（セキュリティ）に進む
WARN	CRITICAL の検出事項がゼロ、HIGH の検出事項が1件以上	検出事項をドキュメント引き継ぎに記録し、ステージ5に進む
BLOCK	CRITICAL の検出事項が1件以上	パイプラインを停止し、検出事項をユーザーに提示して指示を待つ

APPROVE は、実装がセキュリティレビューを受ける準備ができていることを意味します。APPROVE のレビューにおける MEDIUM および LOW の検出事項は、メモとしてドキュメントエージェントに引き継がれます。これらはデリバリーを妨げませんが、今後の参照用に保持されます。

WARN は、実装に修正すべき構造上の問題があるものの、悪用可能な欠陥や正確性の問題ではないことを意味します。PM はセキュリティレビューに進み、WARN の検出事項の表をドキュメント引き継ぎメッセージに追加します。また、PM は検出事項を（KB エントリーまたは TODO として）記録し、黙って破棄されないようにします。

BLOCK は、リリースするとセキュリティ脆弱性、データ損失のリスク、またはサイレントな障害を引き起こす欠陥が少なくとも1件あることを意味します。PM は直ちにパイプラインを停止し、クリティックの検出事項の表を一字一句変えずにユーザーに提示し、明示的な指示を待ちます。PM はユーザーの入力なしにエンジニアへ自動で再試行を依頼してはなりません。

80% の信頼度フィルター

問題のないレビューも、有効なレビューです。検出事項を作り出してはなりません。

実際の問題である確信度が80%を超える問題のみを報告してください。以下は指摘しないでください。

• スタイル上の好みを HIGH または CRITICAL として扱うこと
• 具体的な根拠のない「エッジケースで理論上問題になる可能性がある」という指摘
• 意図的で正しい可能性がある、見慣れないパターン
• 対象範囲に含まれていない機能の欠如（ステージ1の仕様を確認してください）

確信度が80%未満の場合は、検出事項の表ではなく、要約の段落で懸念点を質問として記載してください。例：「process_batch() 関数は空の入力を処理していないようです。呼び出し元が空でないバッチを保証しているか確認してください。」

このフィルターにより、クリティックがノイズの発生源となり、PM が検出事項を無視するようになるのを防ぎます。表の各検出事項は、対応可能な内容にしてください。エンジニアが読めば修正内容を正確に把握でき、確認を求めずに修正できる必要があります。

ナビゲーション

詳細な基準と例については、以下を参照してください。

• CRITICAL Criteria：各 CRITICAL 項目の詳しい説明と例
• HIGH Criteria：各 HIGH 項目の詳しい説明と例
• MEDIUM Criteria：各 MEDIUM 項目の詳しい説明と例
• LOW Criteria：各 LOW 項目の詳しい説明と例
• Efficiency Criteria：アルゴリズム／データアクセスのパターン（ネストしたループ、ループ内のフェッチ、貪欲なアクセス、ループ内の連結、過剰なデータ取得）
• Transferability Criteria：保守性のパターン（デッドコード、長い if/else-if チェーン、ネストした switch、命名の一貫性）
• Verdict Protocol：各判定に対する PM の動作の全容、失敗ループのテンプレート

Efficiency と Transferability の基準は、CAST Highlight のコード品質指標（https://doc.casthighlight.com/）に基づいており、独自の例を用いて言い換えています。