code-review

コードレビュー

価値： フィードバックとコミュニケーション — 構造化されたレビューによって、作成者自身では気づけない欠陥を発見できます。また、レビューを段階に分けることで、ある領域を徹底するあまり別の領域がおろそかになるのを防ぎます。

目的

仕様への準拠、コードの品質、ドメインの整合性を別々のパスで評価する、体系的な3段階のコードレビューを身につけます。各観点に重点的に注意を向けることで、複合的なレビューで問題が見落とされるのを防ぎます。

プラクティス

3段階を順番に実施する

コードは3つの段階を順番に進めてレビューします。段階を組み合わせてはいけません。各段階の焦点は1つです。前の段階で問題があれば、後の段階には進めません。仕様を満たしていないコードの品質をレビューしても意味がありません。

ステージ1：仕様への準拠。 コードは依頼された内容を実現していますか？それ以上でも、それ以下でもありません。

各受け入れ条件または要件について、次を行います。

1. それを実装しているコードを見つけます
2. それを検証するテストを見つけます
3. 実装が仕様に正確に一致していることを確認します

各条件に次のいずれかを付けます：PASS、FAIL（欠落／不完全／相違）、またはCONCERN（実装済みだが誤りの可能性あり）。要件を超えて実装されたものには、OVER-BUILTのフラグを付けます。

いずれかの条件がFAILの場合は、ここで止めます。続行する前に実装に戻ります。

アーキテクチャ準拠チェック（各条件の確認後、ステージ2に進む前に実施）：

• docs/ARCHITECTURE.md が存在する場合：この変更が、文書に記載されたすべての制約とパターン（Components、Patterns、Constraintsの各セクション）に準拠していることを確認します。準拠していない場合はFAILです。受け入れ条件の欠落と同じ重大度です。
• docs/ARCHITECTURE.md が存在しない場合：ステージ2のCONCERNとして、次のように報告します：「アーキテクチャ文書が見つかりません。アーキテクチャへの準拠を検証できません。」

ステージ1の出力には、次を含めます：Architecture Compliance: PASS / FAIL / N/A (no ARCHITECTURE.md)

垂直スライスのレイヤーカバレッジ

垂直スライス（ユーザーが確認できる動作を追加するタスク）を実装する場合は、以下のチェックを順番に実施してください。

1. エントリーポイントの配線チェック（差分ベース）： 変更セットに、アプリケーションのエントリーポイント、または配線／ルーティング層の変更が含まれているか確認します。スライスでユーザーが確認できる新しい動作を追加すると主張しているにもかかわらず、差分に配線やエントリーポイントのコードへの変更がない場合、作成者が既存の配線によって新しい動作にすでにルーティングされる理由を明示的に説明していない限り、レビューは不合格です。

2. エンドツーエンドの追跡可能性： アプリケーションの外部エントリーポイントから、インフラストラクチャ層または統合層を経由して新しいドメインロジックに至り、観測可能な出力に戻るまでの経路を追跡できることを確認します。この経路の一部が変更セットに含まれず、コードベースにもすでに存在しない場合は、その欠落を指摘してください。

3. 境界レベルのテストカバレッジ： 内部関数を直接呼び出すのではなく、アプリケーションの外部境界（HTTP リクエスト、CLI の呼び出し、キュー上のメッセージなど）を通して新しい動作を検証するテストが少なくとも 1 つあることを確認します。アプリケーションのアーキテクチャ上、自動化された境界テストが実現可能な場合、それがないことはレビュー上の懸念事項です。

4. テストレベルの臭いチェック： 変更セット内のすべてのテストが、統合テストや受け入れテストを含まない、分離された内部関数のユニットテストである場合は、懸念事項として指摘してください。このスライスでは、ドメインロジックが実装されていても、実行中のアプリケーションから到達可能であることが証明されていない可能性があります。

ステージ 2：コード品質。 コードは明確で、保守しやすく、十分にテストされていますか？

変更された各ファイルについて、以下を確認してください。

• 明確さ： 追加の文脈がなくてもコードの動作を理解できますか？ 名前は内容を適切に表していますか？構造は明確ですか？
• ドメイン型： プリミティブ型が使われている箇所で、意味を持つ型が使われていますか？プリミティブへの過度な依存の検出では、必ず domain-modeling スキルに従ってください。
• エラー処理： エラーは型付きエラーで処理されていますか？すべての経路が カバーされていますか？
• テストの品質： テストは実装ではなく動作を検証していますか？変更されたコードに対するカバレッジは 十分ですか？
• YAGNI： 未使用のコード、将来を見越した機能、時期尚早な 抽象化はありませんか？

指摘事項を重要度別に分類してください。

• CRITICAL：バグのリスクがあり、不具合につながる可能性が高い
• IMPORTANT：保守性に関する懸念があり、マージ前に修正する必要がある
• SUGGESTION：スタイル上の改善、または軽微な改善。対応は任意

CRITICAL の問題がある場合は、そこで止めてください。実装に戻ります。

ステージ 3：ドメインの完全性。 最終ゲートとして、コードがドメイン境界を尊重しているか確認します。

以下を確認してください。

1. コンパイル時の強制に関する機会： 型システムで強制できる内容を、テストで チェックしていませんか？
2. ドメイン型の一貫性： すべての境界で意味を持つ型が使われていますか？それとも プリミティブ型が漏れ出していますか？
3. 検証の配置： 検証はビジネスロジック内に散在させず、構築時に行われていますか（検証せずにパースする）？
4. 状態の表現： 型で無効な状態を表現できてしまいませんか？bool フィールドが状態機械を隠していませんか？（domain-modeling の bool-as-state チェックを参照してください。）
5. 規約への準拠： 型やパターンはプロジェクトの規約に沿っていますか？ 先例より規約を優先するルールを適用してください。既存コードが規約に違反していても、 正当化の理由にはなりません。

問題を指摘してください。ただし、先例より規約を優先するルールに基づきブロッキングとなる規約違反を除き、提案事項を理由にブロックしないでください。

レビュー結果

3 つのステージをすべて終えた後、構造化された概要を作成してください。

REVIEW SUMMARY
Stage 1 (Spec Compliance): PASS/FAIL
Stage 2 (Code Quality): PASS/FAIL/PASS with suggestions
Stage 3 (Domain Integrity): PASS/FAIL/PASS with flags

Overall: APPROVED / CHANGES REQUIRED

If CHANGES REQUIRED:
  1. [specific required change]
  2. [specific required change]

構造化レビュー証拠

3つのステージをすべて完了した後、次の内容を含む REVIEW_RESULT 証拠パケットを作成します。 ステージごとの判定 {stage, verdict（PASS/FAIL）, findings [{severity, description, file, line?, required_change?}]、overall_verdict、 required_changes_count、blocking_findings_count。

コンテキストメタデータに pipeline-state が指定されている場合、コードレビュースキルはパイプラインモードで動作し、証拠を .factory/audit-trail/slices/<slice-id>/review.json に保存します。単独で実行する場合、証拠は参考情報のみであり、保存されません。

factory モードでは、パイプラインがコードをプッシュする前にチーム全体でレビューします。これは、ビルド中の合意形成に代わる品質チェックポイントです。ブロッキングとなるレビュー指摘は、プッシュ前にすべて対応する必要があります。factory モードのレビューに関するサブセクションは references/mob-review.md を参照してください。

ファイルベースのレビュー成果物

レビューの指摘は、デフォルトの永続化手段として .reviews/ 内のファイルに必ず書き込んでください。メッセージは補助的な連携シグナルにすぎず、コンテキストの圧縮後には残りません。

• 命名: <reviewer-name>-<task-slug>.md（例：kent-beck-user-login.md）
• 内容: 構造化されたレビュー出力の全内容（3つのステージすべて）
• 場所: .reviews/ ディレクトリ（.gitignore に追加）
• メッセージには「review posted to .reviews/」と記載します。実質的なフィードバックはファイルのみに記載します

これにより、レビューの指摘がコンテキストの圧縮、エージェントの再起動、エージェント間メッセージ機能のないハーネスを経ても保持されます。

非ブロッキングフィードバックのエスカレーション

異なるスライスのレビューで、非ブロッキング項目（SUGGESTION 重大度）が2回以上連続して現れた場合、ブロッキング（IMPORTANT 重大度）にエスカレーションする必要があります。.reviews/ 内の以前のレビュー ファイルを確認し、再発状況を追跡してください。

これにより、継続的な品質上の問題が「単なる提案」として先送りされ続けることを防ぎます。

ユーザー向け動作の検証

GWT シナリオがユーザーに見える動作（UI 要素、表示メッセージ、視覚的な変更）を記述している場合、変更セットには、その表示内容を実現するコードを必ず含める必要があります。シナリオで UI 操作を記述しているのに API のみを実装するのは、仕様への準拠に失敗している状態であり、そのスライスは未完成です。

既存の実例より規約を優先

文書化された規約は、確認されたパターンより優先されます。レビューの指摘がプロジェクトの規約（CLAUDE.md、AGENTS.md、クレートレベルのドキュメント、アーキテクチャ決定記録）に反する一方で、コードベース内の既存コードと一致する場合でも、その指摘は有効です。規約に違反する既存コードは技術的負債であり、前例ではありません。

ルール：

1. 既存コードを理由に重大度を下げてはいけません。 同じ誤りを含むファイルがいくつあっても、規約違反はブロッキングです。
2. 新しいコードをブロッキングとして指摘します。 既存コードがなかった場合と同じ重大度を適用してください。
3. 既存の違反はリファクタリング候補として記録します。 レビュー出力に、同じアンチパターンがすでに存在するファイルを列挙した別の注記を追加し、チームがクリーンアップを計画できるようにしてください。

例：プロジェクトの規約で「状態機械には typestate パターンを使用する」と定められているとします。既存の3つのファイルが同じ方法を使っているため、新しいコードでは struct Foo { is_active: bool } を使用しています。レビューでは新しいコードをブロッキングにするとともに、既存の3つのファイルを技術的負債として指摘する必要があります。

意見の相違への対処

レビューの指摘が実装方法と相反する場合：

1. 該当するコードを具体的に示して懸念を述べます
2. リスクを説明します。何が問題になり得るかを示してください
3. 代替案を提示します
4. 1回のやり取りで合意に至らなければ、ユーザーにエスカレーションします

あなたの役割は、進行を妨げることではなく、作成者が見落とした点を見つけることです。

ビジネス価値と UX への配慮

ステージ 1 では、次の点も検討してください。

• このスライスは、ユーザーに見える価値をもたらしますか？
• 受け入れ基準は具体的で、テスト可能ですか（曖昧ではありませんか）？
• この変更後も、ユーザーの利用の流れに一貫性がありますか？
• エッジケースやエラー状態は、ユーザーの視点で適切に処理されていますか？

これらはブロッキングとなる懸念事項ではありませんが、該当する場合は記載してください。

強制に関する注記

• スタンドアロンモード：助言のみです。エージェントはレビューなしでのマージを機械的に防ぐことはできません。
• パイプラインモード：ゲートとして機能します。レビューの判定によってパイプラインの進行がブロックされ、FAIL または CHANGES REQUIRED の場合はスライスが停止します。

厳格な制約：

• ステージ 1 の FAIL は後続ステージをブロックします：[H]
• 規約違反（「先例より規約を優先」ルール）：[RP]

パイプラインの手戻り予算の競合については、テンプレートディレクトリの CONSTRAINT-RESOLUTION.md を参照してください。

制約

• ブロックしないエスカレーション：エスカレーションルールは、異なるスライスの連続する 2 回以上のレビューで、同じカテゴリだけでなく、同じ問題が確認された場合に発動します。同じ懸念事項の表現を変えてエスカレーションを回避することは違反です。判断基準は、人間のレビュアーがこれらを同じ根本的な問題だと認識するかどうかです。
• 先例より規約を優先：「文書化された規約」とは、CLAUDE.md、AGENTS.md、ADR、またはクレート／パッケージレベルのドキュメントに記載された規約を指します。既存のコードでのみ確認できるパターンは先例であり、規約ではありません。ただし、あるパターンが普遍的（ファイルの 100% がそれに従っている）で、矛盾する文書化された規約がない場合は、無視するのではなく、文書化の候補として指摘してください。
• 垂直スライスのカバレッジ：レイヤーのカバレッジチェックは、変更がユーザーから観察可能な動作を変更する場合に適用されます。「ユーザーから観察可能」とは、アプリケーションを使う人が違いに気付くことを意味します。動作を変更しないパフォーマンス向上のためのインフラ変更は、ユーザーから観察可能ではありません。UI がまだなくても、新しい API エンドポイントはユーザーから観察可能です。

検証

このスキルに沿ったレビューを完了したら、次の項目を確認してください。

• 3 つのステージすべてを、個別に順番どおり実施した
• docs/ARCHITECTURE.md への準拠を、ステージ 1 の項目として明示的に確認した
• すべての受け入れ基準を、ステージ 1 でコードとテストに対応付けた
• 変更した各ファイルについて、ステージ 2 で明瞭さとドメイン型の使用状況を評価した
• ステージ 3 で、コンパイル時の強制が可能かという観点からドメインの整合性を確認した
• 既存コードと一致することを理由に、規約違反を軽視しなかった
• 各ステージの PASS／FAIL が明確な、構造化された要約を作成した
• CHANGES REQUIRED の項目には、具体的で実行可能な修正を記載した
• レビュー結果をメッセージだけでなく .reviews/ ファイルにも記録した
• 2 回以上のレビューで確認された、繰り返し発生する非ブロッキング項目をブロッキングにエスカレーションした
• UI の説明を含むシナリオに、ユーザー向け動作の検証を適用した

基準を満たしていない場合は、最終化する前に該当するステージを再確認してください。

依存関係

このスキルは単独で動作します。ワークフローを強化するため、次のスキルと連携できます。

• domain-modeling：ステージ 2 とステージ 3 で参照されるプリミティブ執着と「検証するな、パースせよ」の原則を提供します
• tdd：レビューは TDD サイクルの後に行われることが多く、このスキルはそのサイクルの成果を検証します
• mutation-testing：追加の品質ゲートとして、コードレビューの後に実施できます

依存関係が不足していますか？次のコマンドでインストールしてください。

npx skills add jwilger/agent-skills --skill domain-modeling