fable-review

Fable Review

変更を、パッチが問題ない理由を説明する実装者としてではなく、妥当な不具合を見つけようとする独立したエンジニアとしてレビューします。

目的

有用なレビューでは、具体的な行や変更を、具体的な障害の発生要因に結び付けます。スタイルの好みよりも、正しさ、不変条件、互換性、ライフサイクルの挙動、テストの妥当性を優先します。

ノイズを作り出すことなく、懐疑的な姿勢でレビューします。

有効にする場合

• 差分、PR、実装が独立した検査を受けられる状態にある場合；
• 検証は成功しているものの、人による確認や意味上のリスクが残っている場合；
• リポジトリの規約や公開契約に違反している可能性がある場合；
• リリースやマージに、根拠のあるレビュー結果が必要な場合。

有効にしない場合

• 差分や具体的な変更が存在しない場合；
• 主なタスクが自動実行の証拠収集（fable-verify）である場合；
• 依頼された作業が脅威モデリングやセキュリティの専門的な検討（fable-security）である場合；
• 既知のブロッキング動作に対して実装が必要な場合（fable-execute）。

レビューの分類

変更の種類に応じてレビューの深さを決めます。

変更	主なレビュー対象
バグ修正	根本原因、回帰テスト、関連する経路
新機能	契約、エラー状態、ライフサイクル、互換性
リファクタリング	不変条件の維持、意図しない動作の変化
並行処理	順序、共有状態、クリーンアップ、競合／デッドロック
永続化／マイグレーション	部分的な失敗、トランザクション、互換性、ロールバック
公開 API／CLI	呼び出し元、デフォルト値、エラー／終了時の挙動、バージョニング
依存関係のアップグレード	変更されたセマンティクス、推移的な挙動、設定
パッケージング／ビルド	エクスポート、成果物の内容、生成ファイル、実行時のエントリーポイント

レビュープロトコル

ステージ 1 — 意図を独自に把握する

以下を読みます。

• ユーザー／issue／カードの受け入れ条件；
• 適切なベースとの差分；
• 関連する既存の契約／テスト／指示。

実装を評価する前に、意図された動作を自分の言葉で説明します。

ステージ 2 — 差分全体を読んでから、リスクの高い変更を追跡する

孤立したスニペットだけでレビューしないでください。次の変化を特定します。

• 公開／観測可能な動作の変化；
• 状態／データフローの変化；
• 制御フロー／エラー処理の変化；
• ライフサイクル／リソースの変化；
• 並行処理の変化；
• 設定／生成物／パッケージの変化。

ローカルな差分だけでは正しさを判断できない場合は、重要な変更を呼び出し元や呼び出し先まで追跡します。

ステージ 3 — 不変条件と失敗経路を確認する

重要な変更ごとに、次の点を確認します。

• 前後で何が真であり続ける必要があるか；
• 無効な入力に対して何が起こるか；
• 依存先の呼び出しが失敗したり、一部だけ成功したりした場合に何が起こるか；
• クリーンアップ／ロールバックの経路は完全か；
• リトライによって副作用が重複する可能性はあるか；
• 非同期処理が所有者やライフサイクルの終了後も継続する可能性はあるか；
• 新旧の形式や呼び出し元が共存できるか。

ステージ 4 — テストを飾りではなく証拠として確認する

次の点を確認します。

• 該当する場合、修正前のバグや旧動作でテストが失敗するか；
• テストは実際に変更された境界を検証しているか；
• 重要な否定系／エラー／並行処理／互換性の経路が欠けていないか；
• テストが弱められていないか、スナップショットが安易に更新されていないか；
• テストが通っていても実装が誤っている可能性はないか。

意味のある動作をテストできない場合、些細な静的変更にテストを求めないでください。

ステージ 5 — 正しさの確認後にのみ、範囲と保守性を確認する

次の点を確認します。

• 関係のないリファクタリングが隠れていないか；
• ロジックの重複によって動作にばらつきが生じないか；
• 新しい抽象化が必要以上に複雑ではないか；
• API／設定名が実際の意味を誤って表していないか；
• コメント／ドキュメントが新しい動作と矛盾していないか。

リポジトリのルール上ブロッキングである場合や、可読性／正しさを大きく損なう場合を除き、スタイルだけに関する指摘は避けてください。

ステージ 6 — 指摘事項を調整する（Alibaba OCR の行レベルの紐付け）

各指摘事項は、Alibaba OCR のスキーマと正確な行の紐付けに従う必要があります。

• path: 相対ファイルパス
• start_line / end_line: 新しいファイル内の正確な行番号（両方が 0 の場合は紐付けなしを意味し、フォールバックでの確認が必要です）
• severity: critical / high / medium / low
• category: bug / security / performance / maintainability / test / style / documentation / other
• 具体的な障害シナリオ: このコードが失敗する、または不変条件に違反する理由
• suggestion_code: 実行可能なコード置換案

軽微で重大度の低いスタイル上の指摘やノイズは除外してください。もっともらしい障害モードを説明できない場合、それは不具合ではありません。

ステージ 7 — 判定を出す

• APPROVE: 重大または高重大度の不具合が見つからず、残りの提案は任意です。
• CHANGES_REQUIRED: 誤動作、データ破損、脆弱性、または契約違反を引き起こす可能性のある重大または高重大度の問題が少なくとも 1 つあります。
• INCOMPLETE: 必要なコンテキスト、差分、または証拠が不足しているか、確認範囲率が不完全なため、正確性を確立できません。

判定ルール

• 確認済みのベースとの差分を実際に読まずに承認してはいけません。
• テストスイートの合格によって一部のリスクは下がりますが、差分で確認できるコードレベルの不具合が打ち消されるわけではありません。
• 疑わしいパターンは、現実的な障害モードと結び付けられるまでは指摘事項ではありません。
• テストがないことがブロッキングとなるのは、未テストの動作が重要で、既存の証拠ではカバーできない場合に限ります。
• 並行処理については、Promise が await されているかだけでなく、インターリーブや所有権を考慮してください。
• エラー処理については、エラーがどこに伝播するのか、またその時点で状態がすでに変更されている可能性があるかを追跡してください。
• マイグレーションでは、部分的な実行とバージョンが混在する運用を考慮してください。
• パッケージや設定の変更では、ソースの形だけでなく、ユーザーがインストールして実行する成果物の経路を確認してください。
• ブロッキングの問題が限定的で原因が明確な場合は、fable-execute 宛てに修正カードを 1 つ作成してください。根本原因が不明確、または問題が繰り返し発生している場合は、fable-recover に回してください。

不変条件

• レビューは独立して行い、読み取り専用とします。
• 指摘事項は、変更されたコードまたは直接影響を受ける契約に基づくものとします。
• 指摘数を埋めるために重大度を水増ししません。
• スタイル上の好みを正確性の問題として扱いません。
• セキュリティレビューを実施していない場合、承認によってセキュリティを証明したと主張しません。
• レビュー判定は、実際に確認した差分とベースを対象とします。

失敗の分類

追認

レビュアーがテストや作者の要約に依存し、差分をほとんど読まない状態です。差分全体のレビューをやり直してください。

障害モデルのないパターンマッチ

レビュアーが「問題がありそう」という理由だけでパターンを指摘し、影響を示せない状態です。調査するか、指摘を取り下げてください。

局所的なレビュー

変更自体は局所的に正しくても、呼び出し元、契約、またはライフサイクルを壊している状態です。影響を受ける境界を追跡してください。

テストへの過度な依存

レビュアーが、テストの成功によってすべての意味的な正しさが証明されると思い込んでいる状態です。テストの妥当性と、変更によって生じるリスクを確認してください。

スコープの見落とし

大きな差分の中に、無関係な変更や意図しない動作変更が紛れ込んでいる状態です。カードや非目標と照らし合わせてください。

ノイズの過多

多くの見た目だけの提案が、実際の不具合を見えにくくしている状態です。影響度に基づいて優先順位を付け、件数合わせのコメントを削除してください。

アンチパターン

• PR の説明だけを根拠に承認すること。
• 動作を理解する前に、行ごとのスタイルについてコメントすること。
• 失敗するエラーパスを示さずに「エラー処理を追加する」と指摘すること。
• 不足しているリスクを示さずに「テストを追加する」と指摘すること。
• 変更による影響を考慮せず、すべての any、TODO、長い関数を指摘すること。
• Promise が await されていれば並行処理が安全だと思い込むこと。
• ファイル間の公開契約を呼び出し元まで追跡せず、変更されたファイルだけをレビューすること。
• 指摘がないことを、レビューが十分に深かった証拠として扱うこと。

指摘テンプレート

Path: src/example.ts
Lines: L42-L46
Severity: critical | high | medium | low
Category: bug | security | performance | maintainability | test
Changed behavior/invariant:
Failure scenario:
Why current evidence does not cover it:
Suggested bounded repair:

完了条件

以下の条件を満たした時点でレビューは完了です。

• 関連する差分とベース全体を確認した
• 意図された動作と変更によって生じるリスクを把握した
• 該当する場合、重要な不変条件、エラー、並行処理、互換性、テスト対象を確認した
• 報告するすべての問題について、具体的な障害の発生条件と場所を示した
• 価値の低いノイズを取り除いた
• 根拠を示して、判定を APPROVE、CHANGES_REQUIRED、または INCOMPLETE とした

段階的なリソース

• Alibaba OCR ルールセットと委任ガイド：references/alibaba-ocr-rulesets.md
• 詳細ガイド：references/behavioral-diff-review-playbook.md
• 既存のチェックリスト：references/diff-review-checklist.md
• 例：examples/code-review-finding.md