review-changes

変更のレビュー

機能ブランチのコード変更をレビューし、マージ前に問題を特定します。

ワークフロー

1. レビュー対象の特定

• リモート PR：ユーザーが PR 番号または URL を指定した場合（例：「PR #123 をレビューして」、「https://github.com/org/repo/pull/123 をレビューして」）：

  1. PR をチェックアウトします：gh pr checkout <PR_NUMBER>
  2. PR のコンテキストを確認します：gh pr view <PR_NUMBER> --json title,body,comments
  3. PR の説明とコメントを、レビューの追加コンテキストとして使用します

• ローカルの変更：PR が指定されていない場合は、デフォルトブランチと比較して現在のブランチをレビューします（手順 2 に進みます）

1.5 リポジトリのガイドラインを読み込む

リポジトリ固有のコーディングガイドラインを検索します。これらは組み込みのガイドラインより優先されます。

検索順序（優先度の高い順）：

1. CLAUDE.md、.claude/CLAUDE.md
2. CODE_GUIDELINES.md、.github/CODE_GUIDELINES.md、docs/CODE_GUIDELINES.md
3. STYLE_GUIDE.md、.github/STYLE_GUIDE.md、docs/STYLE_GUIDE.md
4. CONTRIBUTING.md、.github/CONTRIBUTING.md、docs/CONTRIBUTING.md

レビューに関連する内容（コーディング規約、エラー処理、テスト、セキュリティ、命名規則）を抽出します。レビューに関係のない内容（イシューテンプレート、行動規範）は対象外とします。ユーザーのガイドラインは組み込みのガイドラインより優先され、追加で適用されます。リモート PR をレビューする場合は、リモートリポジトリからガイドラインを読み込みます。

2. ブランチの検出

# Get current branch
git branch --show-current

# Detect default branch (try remote HEAD, fall back to main)
git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@' || echo "main"

3. 変更されたファイルの取得

# Assess scope first
git diff --stat <default-branch>...HEAD

# Get full diff
git diff <default-branch>...HEAD

パフォーマンス上の制約：

1. ロックファイル、.min.js、.min.css、生成ファイル、コンパイル済み出力は対象外とします
2. 変更されたファイルが 30 個を超える場合は、テストや設定よりソースを優先し、対象外にしたファイルを要約します
3. 1 つのファイルで 500 行を超える変更がある場合は、行ごとのレビューではなく要約します
4. 差分全体が 3000 行を超える場合は、重要度の高いファイルを約 20 個選び、残りを要約します

4. 変更の分析

差分を主な入力として使用します。ファイルごとではなく、差分全体にガイドラインを適用します。変更内容を理解するために前後のコンテキストが必要な場合に限り、ファイル全体を読みます。

レビュープロセス：

1. 適用されるガイドラインを特定します：リポジトリ固有（手順 1.5 から）、一般（以下）、言語固有（参照ファイルから）
2. まず重大な問題を確認し、軽微な問題を探す前に報告します
3. 見つかった問題ごとに、違反したガイドライン、ファイルと行番号、問題の説明、修正案を記録します
4. 適用されないガイドラインは対象外とします（データベース操作がない場合は、「データベースと永続化」を対象外とします）

5. テストカバレッジの確認

ビジネスロジックを含む新規または変更済みのファイルごとに、次を行います：

• 対応するテストファイルが存在するか確認します
• テストが存在する場合は、新しいコードパスがカバーされているか確認します
• 重要なパスにテストが不足している場合は指摘します

6. 出力の形式

重大度ごとに指摘事項をまとめ、Critical → Major → Minor の順に並べます。

## Branch Review: `feature/xyz` → `main`

**Repository guidelines loaded:**
- `.github/CONTRIBUTING.md` - coding standards, testing requirements

*(These take precedence over built-in rules where they conflict)*

### 🔴 Critical (X issues)

**1. [Brief title]**
- **File**: `path/to/file.ts:42`
- **Problem**: Clear description of what's wrong
- **Fix**: Specific solution

### 🟠 Major (X issues)

**1. [Brief title]** `[repo]`
- **File**: `path/to/file.ts:87`
- **Problem**: Description
- **Fix**: Solution

### 🟡 Minor (X issues)

**1. [Brief title]**
- **File**: `path/to/file.ts:123`
- **Problem**: Description
- **Fix**: Solution

---
**Summary**: X critical, Y major, Z minor issues found.
[Ready to merge / Needs fixes before merge]

カテゴリ内に指摘事項がない場合は、そのセクションを省略します。最後に、マージに関する明確な推奨事項を記載します。[repo] とマークされた指摘は、リポジトリ固有のガイドラインに基づくものです。リポジトリのガイドラインを読み込んでいない場合は、「リポジトリのガイドラインを読み込みました」セクションを省略します。

フィードバックのトーン： 建設的な姿勢で、変更が必要な理由を説明します。実行可能な提案を行います。相手に悪意がないことを前提とします。承認する場合は、貢献の具体的な価値に触れます。

---

一般的なガイドライン

これらは、言語にかかわらずすべてのレビューに適用されます。

API と互換性を損なう変更

• 非推奨期間を設けずに公開関数やメソッドを削除または名前変更する
• 関数シグネチャを変更する（新しい必須パラメーター、戻り値の型の変更）
• API エンドポイントのレスポンス形式を変更する（フィールドの削除、型の変更）
• 既存の動作に影響するデフォルト値の変更
• マイグレーションスクリプトを伴わないデータベーススキーマの変更

認証と認可（重大）

• 新しいエンドポイントやルートに認証チェックがない
• 権限を引き下げる（管理者限定から公開へ）
• 認証情報や API キーをハードコードする
• JWT／セッションの問題：有効期限がない、シークレットが弱い、不適切な検証
• CORS の設定ミス：許可するオリジンが過度に広い（本番環境で *）

データベースと永続化

• 複数ステップのアトミックな操作にトランザクションがない
• N+1 クエリパターン：JOIN やイーガーローディングではなく、ループ内で関連データを取得している
• 頻繁にクエリされる列にインデックスがない
• 上限のないクエリ：大きなテーブルに対して LIMIT を指定せずに SELECT * を実行している
• SQL インジェクション：パラメーター化クエリではなく、クエリ内で文字列を連結している

並行処理（データ破損に直結する重大な問題）

• 共有の可変状態に、同期せずアクセスしている
• 共有リソースの変更時にロックやミューテックスがない
• アトミック性を確保せずにチェックしてから実行するパターン（TOCTOU）
• デッドロックの可能性：複数のロックを一貫しない順序で取得している

非同期コード

• await の後は、前提が依然として有効かどうかを確認してください。状態が変わっている可能性があります
• 非同期操作が期待どおりの結果になったことを確認せずに、コードが成功を返す場合は指摘してください

外部 API の処理

• HTTP リクエストにタイムアウトがない
• 一時的な障害に対するリトライ処理がない
• 繰り返し障害が発生するサービスにサーキットブレーカーがない
• 4xx エラーと 5xx エラーを区別していない
• レート制限への対応がない（429 でバックオフしない）

エッジケースと境界値

• 配列やリストが空でないとコードが仮定している
• オプション値に対する null／undefined チェックがない
• ループのオフバイワンエラー、不正な範囲チェック
• 予期しない動作につながる型変換の問題
• Unicode／エンコーディングの問題：ASCII を前提としている、文字列の長さを誤って扱っている

防御的プログラミング

• 外部入力（API／ユーザー）を検証せずに使用している
• 失敗する可能性のある操作の失敗ケースを処理していない
• インデックスが有効か確認せずに配列にアクセスしている
• 文書化されていない動作や順序に依存している

入力のサニタイズ（重大）

• コマンドインジェクション：サニタイズされていない入力をシェルコマンドに使用している。文字列補間ではなく、引数配列を使用してください
• パストラバーサル：ユーザー入力をファイルパスに使用した結果、意図したディレクトリの外に出てしまう。パスを解決して検証してください
• XSS：ユーザー入力を HTML としてレンダリングしている。エスケープするか安全な API を使用し、URL プロトコルを検証してください
• ログインジェクション：サニタイズされていない入力によって偽のログエントリが作られる。構造化ロギングを使用してください
• SQL インジェクション：クエリ内で文字列を連結している。パラメーター化クエリを使用してください

メモリとパフォーマンス

• 上限のないコレクション（制限なく増加する配列／マップ）
• メモリリーク：イベントリスナーが削除されない、クロージャーが参照を保持している
• 再利用できるのにループ内で大きなメモリを割り当てている
• メインスレッドでブロッキングする同期 I/O や CPU 負荷の高い処理を行っている
• ページネーションがなく、データセット全体を読み込んでいる

ファイルシステム操作

• ファイルの存在を確認せずに読み込んでいる
• 親ディレクトリの存在を確認せずにファイルを書き込んでいる
• ファイル操作のエラー処理がない

ロギングと可観測性

• 重要な操作に対するログが不十分
• ノイズやパフォーマンスの問題を招く過剰なロギング
• サービスをまたぐトレースに必要な相関 ID がない
• 重大：機密データ（PII、パスワード、トークン）をログに記録している
• 可観測性のためのフックがない新機能

テストの品質

• 意味のあるアサーションがないテスト
• エッジケースのカバレッジがない（正常系のみ）
• 不安定なテスト：競合状態、時間依存、外部依存
• テストの汚染：共有状態、クリーンアップ漏れ
• モックへの過度な依存：実際のコードパスをテストしていない

アクセシビリティ

• 画像に代替テキストがない
• セマンティックでない HTML（ボタンに div を使用、フォームラベルがない）
• インタラクティブな要素にキーボードでアクセスできない
• アイコンのみのボタンに ARIA ラベルがない
• 色だけに頼ったインジケーター

コードスタイル

• ネストした if 文より早期リターンを優先してください。フラットなコードのほうが読みやすくなります

エラーメッセージ

• エラーメッセージは、実行可能な対応につながる、文脈に即した具体的なものにしてください
• デバッグに役立つ識別子（注文 ID、ユーザー ID）を含めてください
• スタックトレースや内部情報をエンドユーザーに公開しないでください
• プログラムから処理できるよう、エラーコードを含めてください

---

言語別ガイドライン

差分に含まれるファイルの拡張子に応じて、該当するリファレンスを読み込んでください。

• .ts、.js、.mjs、.cjs → references/typescript.md を読む
• .tsx、.jsx → references/react.md を読む（typescript.md も読み込む）
• .py → references/python.md を読む
• .go → references/go.md を読む
• .rs → references/rust.md を読む
• .java、.kt → references/java-kotlin.md を読む

変更されたファイルに含まれる言語のリファレンスだけを読み込んでください。

---

重大度の定義

• Critical — マージをブロックしてください。コードの破損、セキュリティの脆弱性、データ漏えい、本番環境をクラッシュさせるランタイム障害が該当します。
• Major — 修正してください。未処理の非同期処理、エラーハンドリングの欠如、リソースリーク、競合状態、検証の欠如が該当します。
• Minor — 可能であれば修正してください。コードの明瞭さ、一貫性、パフォーマンス、デッドコード、重複、未解決の TODO が該当します。