2026-02-12 14:49:35 +09:00

8.4 KiB

Raw Blame History

レビューポリシー

全レビュアーが共有する判断基準と行動原則を定義する。

原則

原則	基準
即座修正	軽微でも「次のタスク」にしない。今修正できる問題は今修正させる
曖昧さ排除	「もう少し整理して」等の曖昧な指摘は禁止。ファイル・行・修正案を具体的に示す
ファクトチェック	推測ではなく実コードを確認してから指摘する
実践的修正案	理想論ではなく実装可能な対策を提示する
ボーイスカウト	変更したファイルに問題があれば、タスクスコープ内で改善させる

スコープ判定

状況	判定	対応
今回の変更で導入された問題	ブロッキング	REJECT
今回の変更により未使用になったコード（引数、import、変数、関数）	ブロッキング	REJECT（変更起因の問題）
変更ファイル内の既存問題	ブロッキング	REJECT（ボーイスカウトルール）
変更モジュール内の構造的問題	ブロッキング	スコープ内なら REJECT
変更外ファイルの問題	非ブロッキング	記録のみ（参考情報）
タスクスコープを大きく逸脱するリファクタリング	非ブロッキング	提案として記載

判定基準

REJECT（差し戻し）

以下のいずれかに該当する場合、例外なく REJECT する。

テストがない新しい振る舞い
バグ修正にリグレッションテストがない
any 型の使用
フォールバック値の乱用（?? 'unknown'）
説明コメント（What/How のコメント）
未使用コード（「念のため」のコード）
オブジェクト/配列の直接変更
エラーの握りつぶし（空の catch）
TODO コメント（Issue化されていないもの）
本質的に同じロジックの重複（DRY違反）
同じことをするメソッドの増殖（構成の違いで吸収すべき）
特定実装の汎用層への漏洩（汎用層に特定実装のインポート・分岐がある）
内部実装のパブリック API エクスポート（インフラ層の関数・内部クラスが公開されている）
リファクタリングで置き換えられた旧コード・旧エクスポートの残存
関連フィールドのクロスバリデーション欠如（意味的に結合した設定値の不変条件が未検証）

Warning（警告）

ブロッキングではないが改善を推奨する。

エッジケース・境界値のテスト不足
テストが実装の詳細に依存
関数/ファイルが複雑すぎる
命名が不明確
TODO/FIXME の放置（Issue番号付きは許容）
理由なしの @ts-ignore、eslint-disable

APPROVE（承認）

全ての REJECT 基準をクリアし、品質基準を満たしている場合に承認する。「条件付き承認」はしない。問題があれば差し戻す。

ファクトチェック

指摘する前に必ず事実を確認する。

やるべきこと	やってはいけないこと
ファイルを開いて実コードを確認	「修正済みのはず」と思い込む
grep で呼び出し元・使用箇所を検索	記憶に基づいて指摘する
型定義・スキーマを突合	推測でデッドコードと判断する
生成ファイル（レポート等）とソースを区別	生成ファイルをソースコードとしてレビュー

具体的な指摘の書き方

全ての指摘には以下を含める。

どのファイルの何行目か
何が問題か
どう修正すべきか

❌ 「構造を見直してください」
❌ 「もう少し整理してください」
❌ 「リファクタリングが必要です」

✅ 「src/auth/service.ts:45 — validateUser() が3箇所で重複。
     共通関数に抽出してください」

指摘ID管理（finding_id）

同じ指摘の堂々巡りを防ぐため、指摘をIDで追跡する。

REJECT時に挙げる各問題には finding_id を必須で付ける
同じ問題を再指摘する場合は、同じ finding_id を再利用する
再指摘時は状態を persists とし、未解決である根拠（ファイル/行）を必ず示す
新規指摘は状態 new とする
解消済みは状態 resolved として一覧化する
finding_id のない指摘は無効（判定根拠として扱わない）
REJECTは new または persists の問題が1件以上ある場合のみ許可する

再オープン条件（resolved → open）

解消済み指摘を再オープンする場合は、再現可能な根拠を必須とする。

前回 resolved の指摘を再オープンする場合、以下3点を必須で提示する
1. 再現手順（コマンド/入力）
2. 期待結果と実結果
3. 失敗箇所のファイル/行
上記3点が欠ける再オープンは無効（REJECT根拠に使わない）
再現手順が変わる場合は別問題として新規 finding_id を発行する

finding_id の意味固定

同じ ID に別問題を混在させない。

同一 finding_id は同一問題にのみ使用する
問題の意味・根拠ファイル・再現条件が変わる場合は新規 finding_id を発行する
同一 finding_id の説明を後から別問題に差し替えることを禁止する

テストファイルの扱い

テストファイルの長さや重複は、原則として保守性の警告として扱う。

テストファイルの行数超過・重複コードは原則 Warning
以下の実害が再現できる場合のみ REJECT 可能
- テスト不安定化（フレーク）
- 誤検知/検知漏れ
- 回帰検出不能
「長すぎる」「重複がある」だけでは REJECT しない

ボーイスカウトルール

来たときよりも美しく。

対象

変更したファイル内の既存の問題（未使用コード、不適切な命名、壊れた抽象化）
変更したモジュール内の構造的な問題（責務の混在、不要な依存）

対象外

変更していないファイル（既存問題として記録のみ）
タスクスコープを大きく逸脱するリファクタリング（提案として記載、非ブロッキング）

判定

状況	判定
変更ファイル内に明らかな問題がある	REJECT — 一緒に修正させる
冗長な式（同値の短い書き方がある）	REJECT
不要な分岐・条件（到達しない、または常に同じ結果）	REJECT
数秒〜数分で修正可能な問題	REJECT（「非ブロッキング」にしない）
変更の結果として未使用になったコード（引数・import等）	REJECT — 変更起因であり「既存問題」ではない
修正にリファクタリングが必要（スコープが大きい）	記録のみ（技術的負債）

既存コードの踏襲を理由にした問題の放置は認めない。既存コードが悪い場合、それに合わせるのではなく改善する。

判定ルール

変更対象ファイル内で検出した問題は、既存コードであっても全てブロッキング（REJECT対象）として扱う
「既存問題」「非ブロッキング」に分類してよいのは、変更対象外のファイルの問題のみ
「コード自体は以前から存在していた」は非ブロッキングの理由にならない。変更ファイル内にある以上、ボーイスカウトルールが適用される
問題が1件でもあればREJECT。「APPROVE + 警告」「APPROVE + 提案」は禁止

堂々巡りの検出

同じ種類の指摘が繰り返されている場合、修正指示の繰り返しではなくアプローチ自体を見直す。

同じ問題が繰り返されたら

同じ種類の問題が繰り返されていないか確認
繰り返されている場合、細かい修正指示ではなくアプローチ自体の代替案を提示
REJECT する場合でも、「別のアプローチを検討すべき」という観点を含める

「もう一度修正して」と繰り返すより、立ち止まって別の道を示す。

8.4 KiB Raw Blame History Unescape Escape