# Code Review Excellence

> 建設的なフィードバックを提供し、早期にバグを発見し、チームの士気を維持しながら知識共有を促進する効果的なコードレビュー実践をマスター。プルリクエストのレビュー、レビュー基準の確立、開発者のメンタリング時に使用。 Use when this capability is needed.

- Skill: `tomevault-io/code-review-excellence-2` (Agent Skill, multi-file: 2 files)
- Install (CLI): `npx skillmds@latest add tomevault-io/code-review-excellence-2`
- Raw SKILL.md: https://api.skillmd.com/api/skills/tomevault-io/code-review-excellence-2/raw
- Safety review: pending (external: skill-scanner PASS, skillspector PASS)
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: tomevault-io (https://skillmd.com/u/tomevault-io)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/tomevault-io/code-review-excellence-2

---


> **[English](../../../../../../plugins/developer-essentials/skills/code-review-excellence/SKILL.md)** | **日本語**

# コードレビューの卓越性

建設的なフィードバック、体系的な分析、協力的な改善を通じて、コードレビューを門番から知識共有へと変革します。

## このスキルを使用するタイミング

- プルリクエストとコード変更のレビュー
- チームのコードレビュー基準の確立
- レビューを通じたジュニア開発者のメンタリング
- アーキテクチャレビューの実施
- レビューチェックリストとガイドラインの作成
- チームコラボレーションの改善
- コードレビューサイクルタイムの削減
- コード品質基準の維持

## コア原則

### 1. レビューマインドセット

**コードレビューの目標：**
- バグとエッジケースを捕捉
- コードの保守性を確保
- チーム全体で知識を共有
- コーディング基準を適用
- 設計とアーキテクチャを改善
- チーム文化を構築

**目標ではないこと：**
- 知識を誇示
- フォーマットの細かい指摘（リンターを使用）
- 不必要に進捗をブロック
- 自分の好みに書き直す

### 2. 効果的なフィードバック

**良いフィードバックとは：**
- 具体的で実行可能
- 教育的、批判的でない
- 人ではなくコードに焦点
- バランスが取れている（良い仕事も褒める）
- 優先順位付け（重要 vs あれば良い）

```markdown
❌ 悪い例：「これは間違っています。」
✅ 良い例：「複数ユーザーが同時にアクセスすると競合状態を引き起こす
         可能性があります。ここでミューテックスの使用を検討してください。」

❌ 悪い例：「なぜXパターンを使わなかったのですか？」
✅ 良い例：「リポジトリパターンを検討されましたか？テストが
         容易になります。例はこちら：[リンク]」

❌ 悪い例：「この変数名を変更してください。」
✅ 良い例：「[nit] 明確さのために`uc`ではなく`userCount`を
         検討してください。そのままにしたい場合はブロックしません。」
```

### 3. レビュー範囲

**レビューすべきもの：**
- ロジックの正確性とエッジケース
- セキュリティ脆弱性
- パフォーマンスへの影響
- テストカバレッジと品質
- エラーハンドリング
- ドキュメントとコメント
- API設計と命名
- アーキテクチャの適合性

**手動でレビューすべきでないもの：**
- コードフォーマット（Prettier、Blackなどを使用）
- インポート整理
- リンター違反
- 単純なタイポ

## レビュープロセス

### フェーズ1：コンテキスト収集（2-3分）

```markdown
コードに飛び込む前に理解する：

1. PR説明とリンクされたissueを読む
2. PRサイズを確認（>400行？分割を依頼）
3. CI/CDステータスをレビュー（テスト合格？）
4. ビジネス要件を理解
5. 関連するアーキテクチャ決定を記録
```

### フェーズ2：ハイレベルレビュー（5-10分）

```markdown
1. **アーキテクチャと設計**
   - ソリューションは問題に適合？
   - よりシンプルなアプローチは？
   - 既存パターンと一貫性がある？
   - スケールする？

2. **ファイル構成**
   - 新規ファイルは適切な場所にある？
   - コードは論理的にグループ化されている？
   - 重複ファイルは？

3. **テスト戦略**
   - テストはある？
   - テストはエッジケースをカバー？
   - テストは読みやすい？
```

### フェーズ3：行ごとのレビュー（10-20分）

```markdown
各ファイルについて：

1. **ロジックと正確性**
   - エッジケースは処理されている？
   - オフバイワンエラーは？
   - Null/undefinedチェックは？
   - 競合状態は？

2. **セキュリティ**
   - 入力検証は？
   - SQLインジェクションリスクは？
   - XSS脆弱性は？
   - 機密データの露出は？

3. **パフォーマンス**
   - N+1クエリは？
   - 不要なループは？
   - メモリリークは？
   - ブロッキング操作は？

4. **保守性**
   - 明確な変数名？
   - 関数は一つのことをしている？
   - 複雑なコードにコメントは？
   - マジックナンバーは抽出されている？
```

### フェーズ4：まとめと決定（2-3分）

```markdown
1. 主要な懸念事項を要約
2. 良かった点を強調
3. 明確な決定を下す：
   - ✅ 承認
   - 💬 コメント（軽微な提案）
   - 🔄 変更を要求（必須対応）
4. 複雑な場合はペアリングを提案
```

## レビューテクニック

### テクニック1：チェックリスト方式

```markdown
## セキュリティチェックリスト
- [ ] ユーザー入力が検証・サニタイズされている
- [ ] SQLクエリがパラメータ化されている
- [ ] 認証/認可がチェックされている
- [ ] シークレットがハードコードされていない
- [ ] エラーメッセージが情報を漏らさない

## パフォーマンスチェックリスト
- [ ] N+1クエリがない
- [ ] データベースクエリがインデックス化されている
- [ ] 大きなリストがページネーションされている
- [ ] 高コストな操作がキャッシュされている
- [ ] ホットパスにブロッキングI/Oがない

## テストチェックリスト
- [ ] ハッピーパスがテストされている
- [ ] エッジケースがカバーされている
- [ ] エラーケースがテストされている
- [ ] テスト名が説明的
- [ ] テストが決定論的
```

### テクニック2：質問アプローチ

問題を述べる代わりに質問して思考を促す：

```markdown
❌ 「リストが空の場合、これは失敗します。」
✅ 「`items`が空配列の場合、何が起こりますか？」

❌ 「ここにエラーハンドリングが必要です。」
✅ 「API呼び出しが失敗した場合、これはどう動作すべきですか？」

❌ 「これは非効率です。」
✅ 「すべてのユーザーをループしているようです。10万ユーザーで
    パフォーマンスへの影響を検討しましたか？」
```

### テクニック3：命令ではなく提案

```markdown
## 協力的な言葉遣いを使用

❌ 「async/awaitに変更する必要があります」
✅ 「提案：async/awaitを使うと読みやすくなるかもしれません：
    ```typescript
    async function fetchUser(id: string) {
        const user = await db.query('SELECT * FROM users WHERE id = ?', id);
        return user;
    }
    ```
    どう思いますか？」

❌ 「これを関数に抽出してください」
✅ 「このロジックは3箇所に現れています。共有ユーティリティ
    関数に抽出するのは意味がありますか？」
```

### テクニック4：重要度の区別

```markdown
優先度を示すラベルを使用：

🔴 [blocking] - マージ前に必ず修正
🟡 [important] - 修正すべき、異議があれば議論
🟢 [nit] - あれば良い、ブロックしない
💡 [suggestion] - 検討すべき代替アプローチ
📚 [learning] - 教育的コメント、アクション不要
🎉 [praise] - 良い仕事、継続を！

例：
「🔴 [blocking] このSQLクエリはインジェクションに脆弱です。
パラメータ化クエリを使用してください。」

「🟢 [nit] 明確さのために`data`を`userData`にリネーム検討。」

「🎉 [praise] 素晴らしいテストカバレッジ！エッジケースを捕捉します。」
```

## 言語固有のパターン

### Pythonコードレビュー

```python
# Python固有の問題を確認

# ❌ ミュータブルなデフォルト引数
def add_item(item, items=[]):  # バグ！呼び出し間で共有される
    items.append(item)
    return items

# ✅ Noneをデフォルトに使用
def add_item(item, items=None):
    if items is None:
        items = []
    items.append(item)
    return items

# ❌ 広すぎるキャッチ
try:
    result = risky_operation()
except:  # KeyboardInterruptまで捕捉してしまう！
    pass

# ✅ 特定の例外を捕捉
try:
    result = risky_operation()
except ValueError as e:
    logger.error(f\"無効な値: {e}\")
    raise

# ❌ ミュータブルなクラス属性の使用
class User:
    permissions = []  # すべてのインスタンスで共有される！

# ✅ __init__で初期化
class User:
    def __init__(self):
        self.permissions = []
```

### TypeScript/JavaScriptコードレビュー

```typescript
// TypeScript固有の問題を確認

// ❌ anyを使うと型安全性が台無し
function processData(data: any) {  // anyを避ける
    return data.value;
}

// ✅ 適切な型を使用
interface DataPayload {
    value: string;
}
function processData(data: DataPayload) {
    return data.value;
}

// ❌ 非同期エラーを処理していない
async function fetchUser(id: string) {
    const response = await fetch(`/api/users/${id}`);
    return response.json();  // ネットワーク失敗時はどうなる？
}

// ✅ エラーを適切に処理
async function fetchUser(id: string): Promise<User> {
    try {
        const response = await fetch(`/api/users/${id}`);
        if (!response.ok) {
            throw new Error(`HTTP ${response.status}`);
        }
        return await response.json();
    } catch (error) {
        console.error('ユーザー取得失敗:', error);
        throw error;
    }
}

// ❌ propsのミューテーション
function UserProfile({ user }: Props) {
    user.lastViewed = new Date();  // propをミューテート！
    return <div>{user.name}</div>;
}

// ✅ propsをミューテートしない
function UserProfile({ user, onView }: Props) {
    useEffect(() => {
        onView(user.id);  // 親に更新を通知
    }, [user.id]);
    return <div>{user.name}</div>;
}
```

## 高度なレビューパターン

### パターン1：アーキテクチャレビュー

```markdown
重要な変更をレビューする際：

1. **設計ドキュメントを先に**
   - 大きな機能の場合、コード前に設計ドキュメントを要求
   - 実装前にチームで設計をレビュー
   - 手戻りを避けるためにアプローチに合意

2. **段階的にレビュー**
   - 最初のPR：コア抽象化とインターフェース
   - 2番目のPR：実装
   - 3番目のPR：統合とテスト
   - レビューが容易、反復が高速

3. **代替案を検討**
   - 「[パターン/ライブラリ]の使用を検討しましたか？」
   - 「よりシンプルなアプローチとのトレードオフは？」
   - 「要件が変わるにつれてこれはどう進化しますか？」
```

### パターン2：テスト品質レビュー

```typescript
// ❌ 貧弱なテスト：実装詳細のテスト
test('カウンター変数を増加', () => {
    const component = render(<Counter />);
    const button = component.getByRole('button');
    fireEvent.click(button);
    expect(component.state.counter).toBe(1);  // 内部状態をテスト
});

// ✅ 良いテスト：動作のテスト
test('クリック時に増加したカウントを表示', () => {
    render(<Counter />);
    const button = screen.getByRole('button', { name: /increment/i });
    fireEvent.click(button);
    expect(screen.getByText('Count: 1')).toBeInTheDocument();
});

// テストのレビュー質問：
// - テストは実装ではなく動作を説明している？
// - テスト名は明確で説明的？
// - テストはエッジケースをカバーしている？
// - テストは独立している（共有状態なし）？
// - テストは任意の順序で実行可能？
```

### パターン3：セキュリティレビュー

```markdown
## セキュリティレビューチェックリスト

### 認証と認可
- [ ] 必要な場所で認証が要求されている？
- [ ] すべてのアクション前に認可チェックがある？
- [ ] JWT検証が適切（署名、有効期限）？
- [ ] APIキー/シークレットが適切に保護されている？

### 入力検証
- [ ] すべてのユーザー入力が検証されている？
- [ ] ファイルアップロードが制限されている（サイズ、タイプ）？
- [ ] SQLクエリがパラメータ化されている？
- [ ] XSS保護（出力のエスケープ）？

### データ保護
- [ ] パスワードがハッシュ化されている（bcrypt/argon2）？
- [ ] 機密データが保存時に暗号化されている？
- [ ] 機密データにHTTPSが強制されている？
- [ ] PIIが規制に従って処理されている？

### よくある脆弱性
- [ ] eval()や類似の動的実行がない？
- [ ] ハードコードされたシークレットがない？
- [ ] 状態変更操作にCSRF保護がある？
- [ ] 公開エンドポイントにレート制限がある？
```

## 難しいフィードバックの提供

### パターン：サンドイッチ法（改良版）

```markdown
従来型：褒める + 批判 + 褒める（わざとらしい）

より良い方法：コンテキスト + 具体的な問題 + 役立つ解決策

例：
「決済処理ロジックがコントローラー内にインライン化されている
ことに気づきました。これはテストと再利用を難しくします。

[具体的な問題]
calculateTotal()関数が税計算、割引ロジック、データベース
クエリを混在させており、単体テストと理解を困難にしています。

[役立つ解決策]
これをPaymentServiceクラスに抽出できますか？テスト可能で
再利用可能になります。必要であればペアリングできます。」
```

### 意見の不一致への対処

```markdown
作成者があなたのフィードバックに同意しない場合：

1. **理解を求める**
   「あなたのアプローチを理解させてください。このパターンを
    選んだ理由は何ですか？」

2. **妥当な点を認める**
   「Xについては良い点ですね。考えていませんでした。」

3. **データを提供**
   「パフォーマンスが心配です。アプローチを検証するために
    ベンチマークを追加できますか？」

4. **必要に応じてエスカレーション**
   「[アーキテクト/シニア開発者]にこれについて意見を
    求めましょう。」

5. **いつ譲歩するか知る**
   動作していて重要な問題でなければ、承認する。
   完璧は進歩の敵。
```

## ベストプラクティス

1. **迅速にレビュー**：24時間以内、理想的には同日
2. **PRサイズを制限**：効果的なレビューには最大200-400行
3. **時間ブロックでレビュー**：最大60分、休憩を取る
4. **レビューツールを使用**：GitHub、GitLab、または専用ツール
5. **できることを自動化**：リンター、フォーマッター、セキュリティスキャン
6. **信頼関係を構築**：絵文字、賞賛、共感は重要
7. **対応可能に**：複雑な問題にはペアリングを提案
8. **他者から学ぶ**：他者のレビューコメントをレビュー

## よくある落とし穴

- **完璧主義**：軽微なスタイル好みでPRをブロック
- **スコープクリープ**：「ついでに、これもできますか...」
- **不一貫性**：人によって異なる基準
- **レビュー遅延**：PRを何日も放置
- **音信不通**：変更を要求してから姿を消す
- **形だけの承認**：実際にレビューせずに承認
- **些細な議論**：些細な詳細を広範に議論

## テンプレート

### PRレビューコメントテンプレート

```markdown
## サマリー
[レビューした内容の簡単な概要]

## 良い点
- [うまくできていたこと]
- [良いパターンやアプローチ]

## 必須変更
🔴 [ブロッキング問題1]
🔴 [ブロッキング問題2]

## 提案
💡 [改善1]
💡 [改善2]

## 質問
❓ [Xについての明確化が必要]
❓ [代替アプローチの検討]

## 結論
✅ 必須変更対応後に承認
```

## リソース

- **references/code-review-best-practices.md**：包括的なレビューガイドライン
- **references/common-bugs-checklist.md**：言語固有の注意すべきバグ
- **references/security-review-guide.md**：セキュリティ重視のレビューチェックリスト
- **assets/pr-review-template.md**：標準レビューコメントテンプレート
- **assets/review-checklist.md**：クイックリファレンスチェックリスト
- **scripts/pr-analyzer.py**：PR複雑度を分析し、レビュアーを提案

---
> Converted and distributed by [TomeVault](https://tomevault.io/claim/amurata) — claim your Tome and manage your conversions.
<!-- tomevault:4.0:skill_md:2026-04-12 -->