code-design-review

コード設計レビュー

役割：コード変更を伴うソリューションを対象とする、実装前の設計品質レビューです。提案されたコードが問題を解決するかどうかだけでなく、適切に設計されているか（保守性、テスト容易性、適切な疎結合）を評価します。あらゆる種類のソリューションを対象に意思決定レベルのレビューを行う solution-review を、コードに特化して補完します。PDCA ホストは、このスキルをフロントマターの dependencies で宣言し、前提条件の確認後に呼び出します（このスキルが見つからない場合、ホストは起動時に中断します）。

詳細なフレームワーク項目（著者、年、しきい値、出典）と、ブロッキング／ノンブロッキングの全基準は、reference.md に記載されています。

このスキルが必要な理由

コードソリューションは正しく問題を解決していても、設計が不適切な場合があります。次の変更を難しくする結合、問題が求める以上の複雑さ、チームが抱えていることに気付いていない技術的負債を持ち込むことがあるためです。こうした設計上の欠陥は、「動作するか」を確認するテストでは見えません。数か月後に、保守の苦労、連鎖的なリグレッション、変更への恐れとして表面化します。

AI 時代の前提： AI 支援コーディングを含め、実装のコストが低い時代において、「動作し、短期的には保守できる」だけでは基準として不十分です。アーキテクチャの洗練度と長期的な保守性を、レビューでより重視する必要があります。より良い構造に書き直すほうが、負債を抱えたまま運用し続けるよりもコストが低いことが多いためです。

このスキルでは、ソフトウェアエンジニアリングの理論に基づき、意見ではなく、コードを一行も書く前に設計品質を評価します。すべてのチェック項目は、名前のあるフレームワーク（Brooks、Myers、Yourdon-Constantine、Fowler、McCabe、ISO 25010、Clean Architecture、OWASP）に基づいています。ここから得られる教訓は次のとおりです。

動作しても設計が不適切なコードは、明日の保守危機を招きます。方向転換がまだ容易な実装前に設計品質をレビューしてください。そして、より劣る設計でのリリースが容易だからといって、明らかに優れたアーキテクチャの採用を先送りしないでください。

使用するタイミング

次のいずれかに該当する場合は、このスキルの使用を強く推奨します。

• 提案されたソリューションがコード変更を伴い、実装前にその設計品質を評価する必要がある。
• 新しいモジュール、API 設計、リファクタリング、依存関係の変更など、コード構造に関する判断を含む設計ドキュメント、技術提案、アプローチをレビューしている。
• 「このコードの設計は適切か」「保守できるか」「この結合度は許容できるか」と尋ねられた。
• 新しい抽象化レイヤー、依存関係の方向変更、モジュール境界の変更など、アーキテクチャレベルの判断を伴う。

このスキルはコードソリューションのみを対象とします。コードを伴わないソリューション（設定、プロセス、ツール、コードを伴わないアーキテクチャ上の判断）には、solution-review を使用してください。ソリューションがコードを伴う場合は、両方を使用します。判断には solution-review を、コードの設計品質にはこのスキルを使用してください。

他のスキルとの関係

スキル	担当範囲	このスキルとの関係
solution-review	意思決定レベルのレビュー（有効性、リスク、可逆性、運用性、コスト）	包括的なスキルです。コードソリューションでは、solution-review が判断をレビューし、このスキルがコード設計をレビューします。両方を使用してください。
PDCA ホスト	ホストのワークフロー	フロントマターの dependencies を通じて PDCA ホストから参照されます（見つからない場合 → ホストは中断）。コードに影響するソリューションでは、「ソリューションのレビュー」フェーズを掘り下げて確認します。この依存関係の正となる情報源はホストです。

コード設計品質モデル

3 つのレイヤーを**重要度順（まずアーキテクチャ）**に並べ、ソリューションの範囲に応じた深さで確認します。提示順はレビューの優先順位を示しています。コードレベルの技術的な巧みさよりも先に、アーキテクチャレベルの特性と長期的な保守性にレビュー担当者の注意を向けます。

レイヤー A — アーキテクチャレベルの品質特性

これらは、システムレベルの品質特性に対するソリューションの影響を評価します。ここは、アーキテクチャ設計と長期的な保守性を担うレイヤーです。ソリューションでモジュールを追加する場合、依存関係の方向を変更する場合、モジュールの境界を越える場合、プロセスやレイヤーの境界を越える場合、または公開契約を変更する場合は、レイヤー A をすべて実施してください。クイックパス（新たな境界がなく、依存関係の方向にも影響しない、小規模で独立した変更）では、レイヤー A を簡易的に確認してもかまいません。その制限事項をレポートに記載してください。実装が簡単に見えるからといって、レイヤー A を省略しないでください。

1. テスト容易性（Clean Architecture — Uncle Bob） — UI、データベース、Web サーバー、外部サービスを使わずにビジネスロジックをテストできますか？ソリューションによってロジックを単独でテストできなくなる場合、それはアーキテクチャ違反です。テストが難しくなる兆候：隠れた依存関係、静的メソッドの呼び出し、シングルトン、デメテルの法則への違反。
2. モジュール性（ISO/IEC 25010 — 保守性 → モジュール性） — 1 つの変更による他のコンポーネントへの影響を最小限に抑えられますか？
3. 信頼性／レジリエンス（ISO/IEC 25010 — 信頼性 → 耐障害性、回復性） — ソリューションは、縮退、再試行、サーキットブレーカーなどの障害シナリオを考慮していますか？
4. スケーラビリティ（ISO/IEC 25010 — 性能効率 → 容量） — ソリューションは、予想されるデータ量や並行処理の増加に対応できますか？それともボトルネックを内包していますか？
5. 依存関係の方向（安定依存の原則 ＋ Clean Architecture の依存関係ルール） — 依存関係は安定性の高い方向に向いていますか（不安定なモジュールが安定したモジュールに依存していますか）？それとも安定したモジュールが不安定なモジュールに依存する、逆転した脆弱な状態になっていますか？アーキテクチャ境界の検証（プロセスやレイヤーの境界を越えるソリューションの場合）：(a) ランタイム初期化の場所 — 呼び出される機能（FFI ハンドル、データベース、キャッシュ、サービスのシングルトン）を実際に初期化するプロセス／レイヤーはどこですか？ (b) 境界の適法性 — 呼び出し元はその機能に正当にアクセスできますか？それとも、レイヤーを越えた静的または動的なインポートによって、呼び出し先の依存ツリー（循環依存を含む）が呼び出し元のバンドルグラフに取り込まれますか？バンドラーによる静的な事前スキャンがあるため、動的な require／import を使っても、これを回避することはできません。(c) 所有権の分類 — その機能はシステム機能（シェル／ホストレイヤーに属する）ですか、それともデータ／製品機能（サービス／製品レイヤーに属する）ですか？また、その分類は呼び出し元レイヤーの位置付けと一致していますか？

レイヤー B — コードレベルの設計メトリクス（すべてのコード変更で必須）

規模にかかわらず、あらゆるコード変更に適用されるメトリクスです。それぞれ、名称のあるソフトウェアエンジニアリングのフレームワークに基づいています。

6. 偶発的複雑性（Fred Brooks、No Silver Bullet、1986年）— 解決策によって、問題に内在する複雑性に対応しない間接化、設定、抽象化が導入されていませんか？ 新しい抽象化にはすべて、今存在する理由が必要です。「後で必要になるかもしれない」は偶発的複雑性です。レビューで取り除いてください。
7. 結合の分類（Myersの結合分類体系＋Connascence、Page-Jones 1996年）— この変更によって、どのタイプの結合が導入されますか？ データ結合（データの受け渡し、許容可能）→ スタンプ結合（複合構造の共有、リスクは低い）→ 制御結合（別のモジュールのロジックを制御するフラグの受け渡し、判断の漏出）→ 共通結合（共有された可変状態、リスクが高い）→ 内容結合（別のモジュールの内部を直接操作、許容不可）。Connascenceの分類を加えると、名前／型のConnascenceは問題ありませんが、アルゴリズム／実行順序／タイミングのConnascenceは脆弱です。
8. 凝集度の段階（Yourdon & Constantine、Structured Design、1979年）— 変更後、モジュールの要素は1つの目的に収束しますか（機能的凝集度、最も強い）、それとも時間／ロジック／偶然によってまとめられていますか（時間的／論理的／偶発的凝集度、最も弱い）？ 凝集度の低下は、設計劣化の初期兆候です。
9. 変更の波及（Fowler、「Shotgun Surgery」コードのにおい＋オープン・クローズドの原則）— 解決策によって頻繁な変更を局所化できますか、それとも1つの論理的な変更が多くのファイルに分散しますか？ 優れた設計では、変わりやすい判断（ビジネスルール、設定）と安定した中核部分（データモデル、アルゴリズム）を分離します。
10. 技術的負債の分類（Fowler、Technical Debt Quadrant）— 負債は「慎重・意図的」（返済計画を立てて認識したうえで選択、許容可能）ですか、それとも「無謀・不注意」（より良い設計を知らずに導入、God Object、Speculative Generality、Primitive Obsession）ですか？ 「無謀・不注意」な負債は、負債を抱える側がその存在を知らず、決して返済しないため、阻止する必要があります。
11. 複雑性メトリクス — サイクロマティック複雑度（McCabe、1976年）：判定ポイント数＋1で算出します。10超はレビューが必要、15超はリスクが高く、20超は単一の関数では許容できません。認知的複雑度（SonarSource、2016年）：ネスト、再帰、論理演算子に重み付けし、人が読み解く際の難しさを捉えます。15超はレビューが必要です。認知的複雑度を優先してください。サイクロマティック複雑度では捉えられないもの、つまり分岐が少なくても深いネストは読みにくいことを検出できます。
12. デメテルの法則（Lieberherr & Holland、1989年）— メソッドがやり取りするのは、見知らぬ相手ではなく、直接の知り合いだけにする必要があります。a.b().c().d()（「列車事故」）は、呼び出し元を遠くにあるオブジェクトの内部構造に結び付けます。提案する設計で、2階層を超える呼び出しチェーンにはフラグを立ててください。

レイヤー C — セキュリティチェック（解決策が信頼境界に関わる場合）

13. OWASP Top 10の簡易レビュー — 解決策が認証、認可、入力処理、データ保存、外部通信に関わる場合は、OWASPの簡易レビューを実施します。インジェクション（パラメーター化されていないクエリ）、認証の不備、機密データの漏えい、XXE、アクセス制御の不備、セキュリティ設定の誤りを確認してください。これは完全なセキュリティ監査ではありません。設計段階のセキュリティ上の誤りがコードになる前に発見するためのものです。

各項目の詳細な手法、しきい値、出典はreference.mdに記載されています。

適用方法

1. ソリューションにコードが関係することを確認します。 関係しない場合は、代わりに solution-review を使用してください。
2. 範囲のルールに従ってレイヤー A（アーキテクチャ）を実行します。フルパスとクイックパスのどちらを使う場合も、レポートに必ず明記してください。
3. コード変更のたびにレイヤー Bを実行します。7 つの指標それぞれについて、提案されたコード構造に基づく具体的な根拠とともに合格／不合格を示してください。
4. ソリューションが信頼境界（認証、入力、データ、外部との通信）に関わる場合に限り、レイヤー Cを実行します。それ以外の場合は省略してください。
5. 項目ごとの判定、ブロッキング／ノンブロッキングの分類、総合結論を含む構造化されたレビューレポートを作成します（下記の出力を参照）。
6. ブロッキングの問題があれば通過させないでください。 ブロッキングの問題がある場合、コード設計は不合格です。実装コストが低いこと（AI の支援を含む）は、重大なアーキテクチャ上の欠陥をノンブロッキングの指摘に格下げする理由にはなりません。

ブロッキングとノンブロッキング（概要）

ブロッキング（いずれか 1 つでも該当すれば、コード設計は不合格です）：

• 依存関係の方向が逆転している — 安定したモジュールが不安定なモジュールに依存している（SDP 違反）。
• （レイヤー A、フルパス） 代替策がないまま、ビジネスロジックを外部依存関係から分離してテストできない（クリーンアーキテクチャのテスト容易性に違反）。
• （レイヤー A、境界をまたぐソリューション） 呼び出す機能を呼び出し元のプロセス／レイヤーで実行できない、またはレイヤーをまたぐインポートにより、無関係な依存関係ツリーが呼び出し元に取り込まれる — 返済計画を伴う Prudent-Deliberate debt として明示的に受け入れられている場合を除き、ブロッキングです。短期的なコスト上の利点（既存実装の再利用、単一リポジトリ内での変更、チーム間調整が不要であること）を理由に、この判定を格下げしてはいけません。
• 他のモジュールの内部を直接操作するコンテンツ結合、または代替策のない共有可変状態による共通結合。
• Reckless-Inadvertent tech debt — より適切な設計があると認識せずに、God Object／Speculative Generality／Primitive Obsession を導入している。
• 直接的なビジネス上の関係がない 3 つ以上のモジュールに連鎖的な編集が必要となる単一のビジネス変更（Shotgun Surgery）で、収束戦略がない。
• （セキュリティ） OWASP Top 10 のカテゴリに設計レベルで違反している（例：設計上、インジェクションやアクセス制御の不備が可能になっている）。
• 明らかに優れたモジュール化／依存関係／境界の設計が特定され、それが現在の変更範囲内で実現可能であり、長期的な保守性を大きく向上させるにもかかわらず、チームが返済計画を伴う Prudent-Deliberate debt として明示的に受け入れていない — 弱い設計が正しく、短期的には保守可能であるという理由だけで合格にしてはいけません。

ノンブロッキング（推奨事項として記載し、ブロックしないでください）：

• 構造、結合度、変更波及性に影響しない、スタイルや命名の好み。
• 返済計画が文書化された Prudent-Deliberate tech debt（明示的に受け入れられた弱いアーキテクチャを含む）。
• 本質的に複雑なドメインで、循環的複雑度／認知的複雑度がしきい値をわずかに超えている場合（根拠を記載してください）。
• より洗練された実装の詳細（同じアーキテクチャ内のもの）が存在するものの、現在の実装が正しく、長期的な保守性を損なわない場合。

フレームワーク固有のしきい値を含む、ブロッキング／ノンブロッキングの詳しい基準については、reference.md を参照してください。

出力

次の構造に従ったコード設計レビューレポート：

【Code design review report】
- Subject: [solution name / code area]
- Path: [quick / full] + [security pass: yes/no]
- Layer A architecture quality attributes (full path):
  1. Testability: ✅/❌/N/A
  2. Modularity: ✅/❌/N/A
  3. Reliability / resilience: ✅/❌/N/A
  4. Scalability: ✅/❌/N/A
  5. Dependency direction: ✅/❌/N/A
- Layer B code-level design metrics:
  6. Accidental complexity: ✅/❌ [reasoning]
  7. Coupling type: ✅/❌ [type identified + risk level]
  8. Cohesion: ✅/❌ [cohesion level + trend]
  9. Change amplification: ✅/❌ [shotgun surgery check]
  10. Tech-debt classification: ✅/❌ [quadrant identified]
  11. Complexity measures: ✅/❌ [cyclomatic / cognitive estimate]
  12. Law of Demeter: ✅/❌ [train-wreck check]
- Layer C security pass (trust boundary):
  13. OWASP lightweight pass: ✅/❌/N/A
- Issue list: [#] [description] [severity: blocking/non-blocking]
- Verdict: ✅ pass / ❌ fail
- [On ❌] Optimization guidance: [per-issue improvement direction]

アンチパターン（禁止事項）

1. 設計審査を通さず、「動く」コードを承認すること — 正しさは必要条件ですが、十分条件ではありません。テストが成功したという理由でレイヤー A／B／C をすべて省略すると、設計上の負債が見えないまま蓄積します。
2. あらゆる変更にレイヤー C（セキュリティ）を実行すること — OWASP 審査の対象は、信頼境界に関わる変更に限られます。過剰に適用するとレビューの労力を無駄にし、信頼境界で適用しないと危険です。