rails-code-review

Rails コードレビュー（Rails Way）

Rails コードをレビューする際は、以下の項目に照らして分析します。新しいコードを作成する際は、rails-code-conventions（原則、ログ記録、パスのルール）と rails-stack-conventions（スタック固有の UI と Rails のパターン）に従います。

基本原則： 早めに、頻繁にレビューします。PR の前にセルフレビューを行います。大幅な変更後は再レビューします。

事前確認

レビューを始める前に、以下を確認してください。

• Rails プロジェクトであることを確認：Rails アプリケーション内にいることを確認します（config/application.rb、rails を含む Gemfile、または bin/rails を確認してください）
• 差分を確認できること：コード変更にアクセスできることを確認します（git diff、PR の差分、またはファイルパス）
• ファイルの存在を確認：差分で参照されているすべてのファイルがリポジトリ内に存在することを確認します
• レビュー範囲を明確化：PR 全体、特定の機能、または対象ファイルのレビューかどうかを確認します

事前確認に失敗した場合：

• Rails プロジェクトではない → 技術スタックに適したレビュー用スキルを使用します
• 差分を確認できない → ユーザーに差分またはファイルパスを求めます
• ファイルが見つからない → Critical として指摘し、続行する前に説明を求めます

ハードゲート：実装後（PR 前）

After green tests + linters pass + YARD + doc updates:
1. Self-review the full branch diff using the Review Order below.
2. Fix Critical items; resolve or ticket Suggestion items.
3. Only then open the PR.
generate-tasks must include a "Code review before merge" task.

クイックリファレンス

領域	主な確認項目
ルーティング	RESTful、浅いネスト、名前付きルート、制約
コントローラー	スリムであること、ストロングパラメーター、before_action のスコープ
モデル	構造の順序、inverse_of、enum の値、コールバックよりスコープを優先
クエリ	N+1 の防止、present? より exists?、バッチ処理には find_each
マイグレーション	元に戻せること、インデックス、外部キー、並行インデックス
セキュリティ	ストロングパラメーター、パラメーター化クエリ、html_safe の不適切な使用がないこと
キャッシュ	フラグメントキャッシュ、ネストされたキャッシュ、ETag
ジョブ	冪等性、再試行可能性、適切なバックエンド

レビューの順序

差分を次の順序で確認します。詳細な基準は REVIEW_CHECKLIST.md を参照してください。PR の基本項目をまとめた 1 ページのチェックリストは assets/checklist.md を参照してください。指摘の例（JSON とコメントの形式）は assets/examples.md を参照してください。

設定 → ルーティング → コントローラー → ビュー → モデル → アソシエーション → クエリ → マイグレーション → バリデーション → I18n → セッション → セキュリティ → キャッシュ → ジョブ → テスト

エッジケースへの対応：

• 空の差分：変更されたファイルがない場合は「レビュー対象のコード変更はありません」と伝え、結論に進みます
• 大規模な差分（50 ファイル超）：まず Critical の確認を優先し、次に Suggestion 項目について主要ファイルをサンプル確認します。対象を絞った追加レビューが必要であることを指摘します
• 単一ファイルの変更：そのファイルに関連するすべてのレビュー領域を適用します。差分が小さいという理由で項目を省略しないでください
• テストのみの変更：テストの品質、カバレッジ、構成に重点を置き、アプリケーションコードの確認は省略します

直ちに確認すべき Critical 項目：

# N+1 — one query per record in a collection
posts.each { |post| post.author.name }       # Bad
posts.includes(:author).each { |post| post.author.name }  # Good

# Privilege escalation via permit!
params.require(:user).permit!                # Bad — never in production
params.require(:user).permit(:name, :email)  # Good

常に Critical（該当するすべての箇所を Critical として指摘）：

• params.require(...).permit! — マスアサインメント／権限昇格
• ユーザー提供のコンテンツに適用された html_safe または raw — XSS
• 機密性の高いアクションに対する認可チェックの欠如
• コントローラーアクション内のビジネスロジック — 価格、税金、割引、複数ステップのワークフロー、またはインラインで行われるドメイン計算。調整（1 つのサービスの呼び出し、レスポンスのレンダリング）を超えるコントローラーアクションは、Suggestion ではなく Critical です。
• パラメーター化されていない SQL／文字列展開された SQL — インジェクション
• 大規模なテーブルに対する、安全な移行手段のない破壊的なマイグレーション

重大度レベル

以下のラベルのみを使用します（High／Low、P0–P2 などは使用しません）。Critical | Suggestion | Nice to have。

• Critical — セキュリティ、データ損失、クラッシュ、または常に Critical に該当するルール → マージをブロックし、修正後に差分を再確認します。
• Suggestion — 規約／パフォーマンス → PR 内で修正するか、大規模な再設計が必要な場合はチケットを作成します。
• Nice to have — 小さなスタイル改善やマイクロ最適化 → 作成者が任意で対応します。

出力スタイル

指摘事項は ### Critical ／ ### Suggestion ／ ### Nice to have の下に分類してください（該当しないセクションは省略します）。重大度の異なる項目を、単一のフラットなリストに混在させないでください。

## Review — <PR title or area>

### Critical
- [path/to/file.rb:LINE] (Area) One-line risk. **Mitigation:** concrete next step.

### Suggestion
- [path/to/file.rb:LINE] (Area) … **Mitigation:** …

### Nice to have
- …

**Actions required:** <one line per severity level that appeared — e.g. Critical → block merge + re-review; Suggestion → …>

テンプレートのルール： 各箇条書きは [file:line] (Area) + リスク + Mitigation:（必須）の形式にします。(Area) は次から選んでください：Controllers、Routing、Views、Models、Queries、Migrations、Validations、Security、Caching、Jobs、Tests — 差分が複数の領域に関わる場合、レビュー全体で異なる領域を4つ以上取り上げてください。

出力の検証：

• [file:line] の参照先にあるすべてのファイルパスがリポジトリ内に存在することを確認します
• 行番号が各ファイルの有効な範囲内にあることを確認します
• 各指摘に必須の Mitigation: フィールドが含まれていることを確認します
• 重大度のセクションが適切に分類されていることを確認します
• Actions required: セクションにすべての指摘が正確にまとめられていることを確認します

ファイル参照が無効な場合：

• 該当する指摘を除外し、次のように記載します：[path/to/file.rb:LINE] — リポジトリ内にファイルが見つからないため、スキップします
• 最終レビューに含める前に、ユーザーに確認を求めます

マージ前の再レビュー

Critical の修正を行った後（必須）、Suggestion の修正が3件を超えた後、またはフィードバック中にロジック／アーキテクチャを変更した後（推奨）、あるいは修正によってクエリ、認証、マイグレーションが変更される可能性がある場合は、ブランチの差分を再確認してください。Nice to have のみのフィードバック、または動作に変更のない軽微な1行編集の場合に限り、省略できます。

レビューのアンチパターン（チェックリストに追加する項目であり、チェックリストの代わりにはなりません）

• 薄いコントローラー → 肥大化したモデル： オーケストレーションは巨大なモデルメソッドではなく、サービス（PORO／*.call）に切り出します。
• 開発環境でのN+1： 少量のシードデータではN+1が見えにくいため、関連付けがループ内で実行される場合、「ここでは速い」と決めつけず、クエリ数を数えます（リクエストスペック、rack-mini-profiler、ログなど）。
• ホットテーブルのマイグレーション： 並行インデックスの追加や大規模なバックフィルは、元に戻せるスキーマ変更とは別のデプロイ手順に分けます（判断に迷う場合は rails-migration-safety を連携します）。
• コールバックとジョブ： 永続化フックのみをコールバックに含めます。外部I/Oや複数ステップのワークフローは、明確な冪等性を備えたサービス／ジョブにします。

連携

スキル	連携するタイミング
rails-review-response	開発者がフィードバックを受け取り、実装する内容を判断する必要がある場合
rails-architecture-review	レビューで構造上の問題が見つかった場合
rails-security-review	レビューでセキュリティ上の懸念が見つかった場合
rails-migration-safety	大規模テーブルのマイグレーションをレビューする場合
refactor-safely	レビューでリファクタリングが提案された場合