code-review-patterns

コードレビューパターン

概要

コードレビューは、バグがリリースされる前に検出するためのものです。しかし、機能を確認する前にコードの品質をレビューするのは順序が逆です。

基本原則： まず動作することを確認し、その後で品質を確認します。

参照ファイル

現在のレビューに必要な参照ファイルだけを読みます。

• references/review-order-and-checkpoints.md：懸念事項を優先した読み進め方、レビューのチェックポイント、指摘がない場合の中断、再レビューのループについて
• references/security-review-checklist.md：認証、入力と出力、シークレット、ネットワーク、ストレージ、依存関係の確認について
• references/code-review-heuristics.md：保守性、パフォーマンス、隠れた失敗、エッジケース、ずさんなパターン、UI のクイックスキャンについて

シグナル品質ルール

確信がある場合にのみ指摘します。誤検知は信頼を損ない、修正サイクルを無駄にします。

指摘する	指摘しない
コンパイル／解析に失敗する（構文、型、インポートのエラー）	プロジェクトのガイドラインにないスタイル上の好み
すべての入力で誤った結果を生むロジックエラー	特定の入力／状態に依存する潜在的な問題
明確なガイドライン違反（該当するルールを正確に引用する）	主観的な改善案や細かな指摘

クイックレビューチェックリスト（参照パターン）

迅速なレビューでは、次の 8 項目を確認します。

• コードはシンプルで読みやすい
• 関数と変数の名前は適切
• コードの重複がない
• エラー処理が適切
• シークレットや API キーが公開されていない
• 入力検証が実装されている
• テストカバレッジが十分
• パフォーマンスが考慮されている

鉄則

NO CODE QUALITY REVIEW BEFORE SPEC COMPLIANCE

コードが要件を満たしていることを確認していなければ、コードの品質をレビューすることはできません。

2 段階のレビュープロセス

ステージ 1：仕様適合性のレビュー

依頼されたとおりに動作しますか？

1. 要件を読む

   • 何が依頼されましたか？
   • 受け入れ基準は何ですか？
   • エッジケースは何ですか？

2. 実装を追跡する

   • コードは各要件を実装していますか？
   • すべてのエッジケースに対処していますか？
   • 仕様に正確に一致していますか？

3. 機能をテストする

   • テストを実行する
   • 必要に応じて手動でテストする
   • 出力が期待どおりであることを確認する

ゲート： ステージ 1 に合格した場合にのみ、ステージ 2 に進みます。

ステージ 2：コード品質のレビュー

適切に記述されていますか？

次の優先順位でレビューします。

1. セキュリティ - 悪用される可能性のある脆弱性
2. 正確性 - ロジックエラー、見落とされたエッジケース
3. パフォーマンス - 不必要な遅さ
4. 保守性 - 理解や変更が難しい箇所
5. UX - ユーザー体験に関する問題（UI が関係する場合）
6. アクセシビリティ - アクセシビリティの問題（UI が関係する場合）

レビューの順序

コードを行ごとに確認する前に、 references/review-order-and-checkpoints.md を読み、生の差分順ではなく、 懸念事項ごとに変更内容を再構成します。

セキュリティレビュー

認証、データ、ネットワーク、ストレージ、または外部からアクセス可能なコードについては、 指摘をまとめる前に references/security-review-checklist.md を読みます。

LSP を活用したコード分析

レビュー中の意味的な理解には LSP を使用します。

タスク	LSP ツール	grep より優れている理由
関数の呼び出し元をすべて検索する	lspCallHierarchy(incoming)	文字列の一致ではなく、実際の呼び出しを見つけます
型／変数の使用箇所をすべて検索する	lspFindReferences	テキストベースではなく、意味に基づいて検索します
定義に移動する	lspGotoDefinition	実際の定義に移動します
関数が呼び出す対象を把握する	lspCallHierarchy(outgoing)	呼び出しチェーンを対応付けます

LSP を使ったレビューワークフロー：

1. localSearchCode → シンボルを検索し、lineHint を取得する
2. lspGotoDefinition(lineHint=N) → 実装を理解する
3. lspFindReferences(lineHint=N) → 一貫性を保っているか、すべての使用箇所を確認する
4. lspCallHierarchy(incoming) → 呼び出し元が変更に対応していることを確認する

重要： 必ず最初に localSearchCode から lineHint を取得してください。行番号を推測してはいけません。

レビューのヒューリスティクス

パフォーマンス、保守性、エッジケース、隠れた障害、型設計の乖離、UI 固有のチェックについては、references/code-review-heuristics.md を参照してください。

誤り／正しい例 — サイレントなオプショナルチェーン：

// WRONG: silently swallows null user
const name = user?.profile?.name ?? 'Unknown';

// RIGHT: log the gap, then degrade
const name = user?.profile?.name;
if (!name) {
  logger.warn('user profile missing name', { userId: user?.id });
}
return name ?? 'Unknown';

エッジケース分類の分類体系

レビュー中にエッジケースを体系的に洗い出すためのチェックリストです。

カテゴリ	例	検出方法
else/default の欠落	default のない switch、null 許容型に対して else のない if	switch/if の網羅性を確認する
ガードされていない入力	ユーザー入力の検証なし、null チェックの欠落	検証せずにパラメーターを直接使用している箇所
オフバイワン	ループ境界、配列インデックス、ページネーション	すべての < と <=、array[length] と array[length-1] を確認する
算術演算のエッジケース	ゼロ除算、整数オーバーフロー、浮動小数点	除数の検証なしで使用されている / 演算子
暗黙的な型変換	=== ではなく ==、文字列から数値への変換、truthy/falsy	=== ではない ==、異なる型が混在する +
競合状態	共有された可変状態、ロックなしの非同期処理	非同期処理内で変更される共有変数
タイムアウト／再試行の不足	ネットワーク呼び出しにタイムアウトがない、再試行回数の上限がない	タイムアウト設定なしの fetch/axios

ステージ 2 の品質レビューで使用してください。変更されたコードに関連するカテゴリのみを確認してください。

簡潔さより明瞭さを優先

• ネストした三項演算子 a ? b ? c : d : e → if/else または switch を使用する
• 2 行を節約する密なワンライナー → ひねった 1 行より、明快な 3 行を優先する
• 複雑なコールバックを含む連結された .map().filter().reduce() → 名前付きの中間変数を使用する

パターン認識の基準

レビュー中に、文書化する価値のあるパターンを特定してください。

基準	注目する内容	例
暗黙知	新しい開発者にはわからない知識	「すべての API レスポンスはエンベロープ構造を使用する」
独自の方針	異なる選択肢もあり得る、特定の選択	「DB では snake_case、JS では camelCase を使用する」
非標準	標準的なフレームワークのパターンではないもの	「バックオフ付きの独自の再試行ロジック」
一貫性	複数のファイルで繰り返されるもの	「すべてのサービスにヘルスチェックエンドポイントがある」

レビュー中にこれらを見つけた場合：

1. レビューフィードバックにパターンを記載する
2. **メモリノート（Patterns セクション）**に記載する — ルーターが Memory Update タスクを通じて patterns.md に保存します
3. 確立されたパターンとの不一致を指摘する

重大度の分類

重大度	定義	対応
CRITICAL	セキュリティ脆弱性、または機能を妨げる問題	マージ前に必ず修正する
MAJOR	機能に影響する問題、または重大な品質上の問題	マージ前に修正することが望ましい
MINOR	スタイル上の問題、小さな改善	マージ後の修正でもよい
NIT	純粋にスタイル上の好み	任意

マルチシグナルレビューの方法

ステージ 2 の各パスは、独立したシグナルを生成します。各評価軸を個別に採点してください。

HARD シグナル（いずれかが失敗すると承認不可）：

• Security： 実際の脆弱性が 1 件でもあれば、評価軸のスコアは 0
• Correctness： 誤った出力を生むロジックエラーが 1 件でもあれば、評価軸のスコアは 0

SOFT シグナル（懸念事項として記録するが、それだけでは承認を妨げない）：

• Performance： すぐに影響はないが、スケーリング上の懸念がある
• Maintainability： 複雑だが機能するコード
• UX/A11y： 中核的なフローは動作するが、状態が欠けている

集約ルール：

1. HARD シグナルのスコアが 0 のものが 1 つでもある場合 → STATUS: CHANGES_REQUESTED（交渉不可）
2. 正確な CONFIDENCE の計算式と重大度の用語は code-reviewer エージェントの契約で定められています — ここで計算式を再掲しないでください（重複した記載は乖離するため）。エージェントの CONFIDENCE = min(HARD scores) capped by avg(SOFT scores) - 10 を適用し、HARD:0 が 1 つでもあれば CONFIDENCE:0 とします。
3. 対象を絞った修正のため、Router Handoff にシグナルごとの内訳を含めてください

指摘ゼロの場合のレビュー停止

セキュリティ、正確性、パフォーマンス、保守性、UX／アクセシビリティのすべての観点で、レビューの指摘がゼロの場合、レビューを必ず停止して再確認してください。重要な変更に対する指摘ゼロは、コードが完璧であることではなく、レビューの深さが不十分であることを示しています。対応：変更されたファイルをすべて読み直してください。references/code-review-heuristics.md のヒューリスティックによるスキャンを再実行してください。references/security-review-checklist.md のセキュリティトリアージを再実行してください。意図的に再確認した後も指摘がゼロの場合は、次のように記録してください：「[N files, M lines changed] を強制的に再確認した結果、指摘ゼロを確認しました。レビューした項目：[実施した具体的なチェックの一覧]。」再確認の証拠がない単なる「問題なし」は無効です。

シグナルごとの証拠要件： 各シグナルには、具体的なファイルと行番号を必ず記載してください。証拠のないシグナルは報告しません。

2 つの独立した評価と WEAVE による照合

cc10x は意図的に code-reviewer と silent-failure-hunter を並行して実行します。

独立性が必須である理由： 両方の評価を 1 つの推論主体にまとめると、気づかないうちにアンカリングが起きます。最初に読んだ懸念が 2 つ目の評価に影響し、広範なレビュー担当者は隠れた失敗の兆候を合理化して見過ごし、2 つの「独立した」シグナルがいつの間にか 1 つの意見に収束します。独立性こそが、この手法の要です。各評価は、もう一方の指摘を見ることなく、指摘事項をまとめてください。照合は、両方がそれぞれの一覧を確定した後にのみ行います。

WEAVE プロトコル（単に「厳しい方の判定を採用する」方式に代わるもの）： 指摘を振り分ける前に分類してください。

分類	定義	振り分け
AGREED	両方の評価で独立して検出された（同じファイルと行番号、同じ根本原因）	最も信頼度が高い — 報告します。収束は強いシグナルです
DETECTOR-ONLY	どちらか一方の評価だけが検出した（reviewer XOR silent-failure-hunter）	残します — もう一方が見落とした指摘こそ、2 つの評価を実施する価値であり、ノイズではありません。検出した評価の証拠と深刻度をそのまま引き継ぎます
FALSE-POSITIVE	トリアージで疑わしいと判断されたもの：ファイルと行番号がない、変更前から存在する、リンターが検出する、またはもう一方の評価の証拠で反証されている	「指摘しないこと」とシグナル品質ルールに従って除外します

照合の順序：AGREED → DETECTOR-ONLY → FALSE-POSITIVE のトリアージ。もう一方の評価が沈黙していたという理由だけで、DETECTOR-ONLY の指摘を格下げしてはいけません。沈黙は反証ではありません。

フォールバック — 厳しい方の判定を採用： 照合後、両方の評価で同じ指摘について真っ向から矛盾する場合（例：reviewer は安全と判断し、hunter は失敗が握りつぶされていると判断したものの、どちらの証拠も相手の主張を反証していない場合）は、厳しい方の判定を採用します。同点の場合は CHANGES_REQUESTED とします。これは最終手段であり、最初に適用するルールではありません。まず照合し、真に決着がつかない場合にのみエスカレーションしてください。

指摘しないこと（誤検出の防止）

• この変更によって生じたものではない、変更前からの問題
• 疑わしく見えるだけの正しいコード
• シニアエンジニアが指摘しないような細かすぎる粗探し
• リンターがすでに検出する問題（ツールの指摘を重複させないでください）
• プロジェクトのガイドラインで求められていない、一般的な品質上の懸念
• lint-ignore コメントによって明示的に無視されている問題

優先度別の出力形式（フィードバックのグループ化）

フィードバックは優先度順に整理してください（参照パターンに従う）：

## Code Review Feedback

### Critical (must fix before merge)
- [95] SQL injection at `src/api/users.ts:45`
  → Fix: Use parameterized query `db.query('SELECT...', [userId])`

### Warnings (should fix)
- [85] N+1 query at `src/services/posts.ts:23`
  → Fix: Batch query with WHERE IN clause

### Suggestions (consider improving)
- [70] Function `calc()` could be renamed to `calculateTotal()`
  → More descriptive naming

各問題の修正方法を、具体例を挙げて必ず示してください。 「これは間違いです」と述べるだけではなく、正しい対応方法を示してください。

危険信号 — 停止して再レビュー

次のような状態に気づいた場合：

• 機能を確認する前にコードスタイルをレビューしている
• テストを実行していない
• セキュリティチェックリストを省略している
• 一般的なフィードバック（「問題なさそうです」）をしている
• ファイルと行番号を示していない
• 何が問題なのかを説明していない
• 修正方法を提案していない

停止してください。ステージ 1 からやり直してください。

合理化による見落としの防止

言い訳	実態
「テストが通るから問題ない」	テストでは要件を見落とすことがあります。仕様への準拠を確認してください。
「コードはきれいに見える」	きれいなコードでも間違っていることがあります。機能を検証してください。
「この開発者を信頼している」	信頼しつつ検証してください。誰でも間違いを犯します。
「小さな変更だから」	小さな変更が大きなバグを引き起こすことがあります。徹底的にレビューしてください。
「完全なレビューをする時間がない」	バグ対応にはレビューより時間がかかります。適切に行ってください。
「セキュリティ対策はやりすぎだ」	1つの脆弱性が会社を危機に陥れることがあります。確認してください。

任意：基準が明確な0〜4の品質評価基準（厳格なレビュー向け）

CONFIDENCE スコアが測るのは、結果の品質ではなく、指摘事項に対する確信度です。厳格なレビューでは、レビュー担当者は判定に加えて、基準が明確な0〜4の品質スコアを付けてもかまいません。それぞれの整数値には明文化された基準があり、複数のレビュー担当者が同じスコアを付けられるようになっています。

スコア	基準
0	動作しない、または安全ではありません。仕様を満たしていない、重大な問題（セキュリティホール、誤った出力など）が含まれている、または失敗を隠しています。マージしないでください
1	想定どおりのケースでは動作しますが、脆弱です。エラー処理、エッジケース、検証に実際の不足があり、通常の使用で問題になります
2	堅実で正しいものの、特筆すべき点はありません。保守性や明確さに多少の負債がありますが、マージを妨げるものはありません
3	良好です。正しく、読みやすく、十分にテストされ、エッジケースも考慮されています。安心して責任を持てるコードの基準です
4	本当に優れています。正しく、明確で、防御的な配慮も行き届いており、次の変更も容易な設計です。まれなレベルです

合計スコアの範囲 → 対応：

スコア	対応
0	CHANGES_REQUESTED — 譲れない条件です（HARD-signal:0 に対応）
1	CHANGES_REQUESTED — 脆弱な箇所を正確に指摘してください
2	指摘したフォローアップ事項を添えて APPROVE
3-4	APPROVE

評価の調整： 実際のコードの多くは中間の範囲（2）に収まります。4は本当に優れたコードに付けるスコアです。安易に高く評価しないでください。すべてのレビューで3〜4を付けているなら、問題があるのはコードではなく評価基準です。

これは追加の評価であり、code-reviewer エージェントが定める既存の判定／CONFIDENCE の契約を置き換えるものではありません。二択の APPROVE よりも明確な判断材料をルーターに提供できるのが利点です。0〜4の段階評価により、ルーターは「ぎりぎり合格、要注意」（1〜2）と「このまま出荷可能」（3〜4）を区別し、それに応じて修正作業を割り当てられます。

出力形式

## Code Review: [PR Title/Component]

### Stage 1: Spec Compliance ✅/❌

**Requirements:**
- [x] Requirement 1 - implemented at `file:line`
- [x] Requirement 2 - implemented at `file:line`
- [ ] Requirement 3 - NOT IMPLEMENTED

**Tests:** PASS (24/24)

**Verdict:** [Meets spec / Missing requirements]

---

### Stage 2: Code Quality

**Security:**
- [CRITICAL] Issue at `file:line` - Fix: [recommendation]
- No issues found ✅

**Performance:**
- [MAJOR] N+1 query at `file:line` - Fix: Use batch query
- No issues found ✅

**Quality:**
- [MINOR] Unclear naming at `file:line` - Suggestion: rename to X
- No issues found ✅

**UX/A11y:** (if UI code)
- [MAJOR] Missing loading state - Fix: Add spinner
- No issues found ✅

---

### Summary

**Decision:** Approve / Request Changes

**Critical:** [count]
**Major:** [count]
**Minor:** [count]

**Required fixes before merge:**
1. [Most important fix]
2. [Second fix]

レビューループの手順

変更を依頼した後：

1. 修正を待つ - 開発者が問題に対処します
2. 再レビューする - 修正によって実際に問題が解決したことを確認します
3. リグレッションがないことを検証する - テストを再実行します
4. 承認するか、追加の変更を依頼する - 必要に応じて繰り返します

修正の効果を検証せずに承認してはいけません。

フェーズの一部のみを対象とするレビュー

複数フェーズの計画のうち、単一フェーズのコードをレビューする場合：

対象範囲に関する質問	ルール
このフェーズの変更のみをレビューしますか？	はい — 対象範囲を将来のフェーズに広げないでください
レビュー中に見つかった、未変更のコードの問題を指摘しますか？	フォローアップ事項として記録し、このフェーズをブロックしないでください
フェーズの終了条件を検証しますか？	はい — 計画では各フェーズに終了条件が定義されています。最終成果物ではなく、その条件を検証してください
将来のフェーズとの統合箇所をレビューしますか？	インターフェース上の懸念のみを指摘してください。将来のフェーズの実装は要求しないでください

重要な原則： 一部のフェーズのみを対象とするレビューは、フェーズの終了条件が満たされ、リグレッションがない場合に成功です。計画に後続のフェーズがあるなら、「機能が未完成である」ことは妥当な却下理由ではありません。

最終確認

承認する前に：

• ステージ1が完了している（仕様への準拠を検証済み）
• ステージ2が完了している（すべてのチェックリストをレビュー済み）
• すべての重大／主要な問題に対処済み
• テストが成功している
• リグレッションが発生していない
• 各指摘事項の根拠を記録済み