structural-review について
structural-reviewは、変更のコード構造・保守性・開発体験を評価する報告専用スキルです。具体的で実行可能な簡素化を重視し、正確性やセキュリティのレビューとは重複しません。
主な機能
- 01構造・保守性・開発体験のレビュー
- 02大きすぎるファイルや弱い抽象化の指摘
- 03標準レイヤーと型の配置の確認
- 04分岐や非アトミックな変更の特定
- 05実行可能な指摘を深刻度順に整理
活用シーン
- 01プルリクエストの構造的な退行の確認
- 02削除できる抽象化や分岐の特定
- 03変更のマージ前に保守性を確認
インストール
コマンドをコピーしてターミナルで実行します。
npx skills add https:/ / github.com/ shipshitdev/ skills --skill structural-reviewインストール後、Claude Code で/structural-reviewを入力して読み込まれたか確認します。
同梱ファイルplugin.json
詳細
構造レビュー
独自の見解に基づく、構造と保守性のレビュールーブリックです。報告のみを行います。このスキルは指摘事項を提示しますが、ファイルの変更や PR の作成は行いません。
核となる原則
構造に関するすべての指摘では、「作者は具体的に何を削除、統合、または改名すべきか。そして、どの特定のファイル/行を正規の配置先とするか」に答える必要があります。 曖昧な指摘(「もっとすっきりできそうです」)はノイズです。確信度が高く、実行可能な指摘のみを挙げてください。
このルーブリックは /code-review を明示的に補完するものです。そこで既に扱われている正確性のバグや、リポジトリの指示ファイルに関するルール違反を再度指摘しないでください。構造、保守性、開発者エクスペリエンスという直交する観点を担当します。変更によってモジュールがより深くなったか、浅くなったかを評価する際は、codebase-design の用語(モジュール、インターフェース、深さ、境界)を使ってください。
主なレビューポイント
個別の問題を探す前に、まず次の点を確認してください。
- この PR によって、コードベースは小さくなりますか、それとも大きくなりますか? 大きくなる場合、新しい抽象化はそれぞれ存在意義を果たしていますか。それとも、根拠なく複雑さを増やしていますか?
- 判断ロジックはどこにありますか? 正規のレイヤー(サーバーアクション、ドメインサービス、フック)にありますか。それとも、レンダーツリーやイベントハンドラーに散在していますか?
- 読者はこのモジュールを単独で理解できますか? それとも、理解するには 4 つ以上のファイルにまたがる状態やロジックを追う必要がありますか?
- 新しい型はそれぞれ、構造上適切な場所に置かれていますか? それとも、裸の
unknownやas Xキャストが将来の破損を覆い隠していませんか。また、*.types.tsに置くべきインターフェースをインラインで定義していませんか? - この PR によって、スタックは変更前より整理されますか? Tailwind v3 の残骸、
npmの呼び出し、shadcn プロジェクト内の生の HTML、Next.js 16 アプリ内のmiddleware.tsは、スタイルに関する指摘ではなく、後退です。
評価軸
1. ファイルサイズ — 約 1,000 行のルール
約 1,000 行を超えるファイルは、原則としてブロッカーです。厳密な数値のルールではありませんが、そのモジュールが責務を抱え込みすぎている可能性が高いことを示します。
次の場合に指摘します:
- この PR で変更されたファイルが 1,000 行を超えている。
- この PR によって追加された行数が十分に多く、もともと境界に近かったファイルが基準を超える。
推奨する対処法: PR をマージする前に、*.service.ts、*.queries.ts、*.types.ts、*.hooks.ts など、同じ場所にまとめた単位に分割してください。マージ後ではありません。
表現例: 「ファイルは現在 1,180 行です。これを反映する前にクエリレイヤーを *.queries.ts に分割してください。1,000 行のルールは、まさにこのような PR のためにあります。」
重大度: ブロッカー。
2. 抽象化に存在意義があること
新しい関数、フック、クラス、ファイルはすべて、その存在意義を示す必要があります。薄いラッパー、恒等ヘルパー、処理を加えず再エクスポートするだけのものは、例外なくこの基準を満たしません。
次の場合に指摘します:
- 新しいヘルパーが、名前を変更しただけの一行関数、または不変条件を追加しない単なる呼び出し経由の関数である。
- 新しいファイルが、変換を行わずに別のファイルのシンボルを再エクスポートしている。
- フックが、同じ場所に置かれたロジックなしで単一の
useStateをラップしている。 - インターフェースが一度だけ定義され、呼び出し元でも一度しか使われていない。インライン化してください。
推奨する対処法: 抽象化を削除し、実際のシンボルを呼び出し元にインライン化してください。Bun と TS のパスエイリアスを使えば、元のシンボルにも簡単にアクセスできます。
表現例: 「このヘルパーは formatDate の恒等関数です。これをインライン化して、ファイルを削除してください。コード柔道です。」
重大度: 変更を要求。
3. スパゲッティ分岐
既存のフローにその場しのぎの分岐を継ぎ足すことは、クリーンなコードベースが保守不能になる主な原因です。
次の場合は指摘します:
- この PR で変更された 2 つ以上のファイルに、同じ
if (isAdmin)/if (featureFlag)/if (isPremium)の分岐がある場合。 - すでに 3 つ以上の条件分岐がある既存のレンダー関数に、新たな分岐が追加されている場合。
- フィーチャーフラグの分岐が単一の判断箇所で解決されず、コンポーネントのレンダーパスに分散している場合。
推奨される解決策: すべての分岐を 1 つの標準的な判断箇所にまとめます。クライアントの状態にはフック、ミューテーションにはサーバーアクションまたはドメインサービスを使います。これにより、コンポーネントから分岐をなくせます。
表現例: 「レンダーツリー全体に 3 つの if (isAdmin) 分岐が散在しています。判断を 1 つのフックまたはサーバーアクションにまとめれば、コンポーネントから分岐をなくせます。」
重大度: 変更を要求。
4. 標準レイヤーの遵守
このスタックでは、ロジックを配置すべきレイヤーが決まっています。レイヤーモデルへの違反は、好みの問題ではなく構造上の欠陥です。
レイヤーマップ:
- ミューテーション → サーバーアクションまたはドメインサービス。生の
<form onSubmit>やuseEffectには記述しません。 - 派生したクライアント状態 → フック。JSX 内でインラインに再計算しません。
- インタラクティブ要素 → shadcn/ui のプリミティブまたは
packages/uiコンポーネント。生の<button>、<input>、<dialog>は使いません。 - ルートミドルウェア →
proxy.ts。middleware.tsは使いません(Next.js 16)。
次の場合は指摘します:
- プロジェクトにサーバーアクションがあるにもかかわらず、ミューテーションが DOM イベントハンドラーに直接結び付けられている場合。
- フックにできる計算が JSX 内にインラインで記述され、2 つ以上のコンポーネントで繰り返されている場合。
packages/uiまたは shadcn からすでにインポートしているファイルで、生の HTML 要素を使っている場合。- 独自のヘルパーが、コードベースにすでにある標準ユーティリティと重複している場合 — 似たものを新たに追加するのではなく、既存のヘルパーを再利用してください。
表現例: 「packages/ui から Button をすでにインポートしているコンポーネントで <button onClick={...}> が使われています。置き換えてください。このコードベースでは、生の HTML は後退です。」
重大度: 変更を要求。shadcn/packages/ui のプロジェクトで生のインタラクティブ要素を使っている場合は、ブロッカー。
5. 型の構造的な遵守
この観点では、構造上の配置と形状の問題を扱います。any が存在するかどうかは対象外で、/code-review のハーネスが担当します。
違反(すべて指摘):
- 隣接する型ガードなしの単独の
unknown— これは先送りされたanyであり、形状を未解決のままにすることと構造的に同等です。 - 型システムが絞り込みを推論できない理由を説明するコメントがない
as Xキャスト。 - コンポーネントまたはサービスのファイル内に定義されたインラインインターフェース — 同じ場所に配置した
*.types.tsまたはpackages/typesに移してください。
推奨される解決策: 形状を *.types.ts で定義します。unknown が入ってくる境界に型ガードを追加します。
表現例: 「型ガードなしの単独の unknown は、先送りされた any です。形状を *.types.ts で定義するか、ここにガードを追加してください。」
重大度: ガードなしの単独の unknown → ブロッカー。インラインインターフェース → 変更を要求。コメントのない as X → 軽微。
6. 非アトミックなミューテーション
データベースへの書き込みをそれぞれ行う await 呼び出しを順番に実行することは、正しさに関わる構造上の欠陥です。書き込みの途中でクラッシュすると、システムは半端な状態になります。
次の場合は指摘します:
- 同じ関数内に、トランザクションで囲まれていない
await db.update()またはawait db.insert()の呼び出しが 2 つ以上ある場合。 - 関数が書き込みを行い、外部サービスを呼び出してから、補償/ロールバックの経路なしに 2 回目の書き込みを行う場合。
推奨される解決策: DB トランザクションで囲むか、単一のアトミックな書き込みとしてモデル化します。外部呼び出しをトランザクション内で実行できない場合は、明示的な補償ロジックを追加します。
表現例: 「この 2 つの await db.update() 呼び出しは非アトミックです。間で失敗すると、行が無効な状態になります。トランザクションで囲むか、単一のアトミックな書き込みとしてモデル化してください。」
重大度: ブロッカー。
7. 逐次オーケストレーションのにおい
中間の抽象化なしに、5 つ以上のステップを順次オーケストレーションする関数は、将来的な保守の妨げになります。テストや再利用が難しく、単独では理解しにくいためです。
次の場合に指摘してください:
- 新しい関数に、中間の名前付きステップやヘルパー関数がなく、5 つ以上の逐次的な
await呼び出しがある。 - ルートハンドラーやサーバーアクションが、読み取り、変換、検証、書き込み、通知をすべてインラインで行っている。
- 関連のない独立した処理(2 つの無関係な読み取りやフェッチ)を、理由もなく逐次的に待機している。フローがより簡潔になる場合は、並列実行してください。
**推奨する対処法:**名前付きのフェーズに分割します。各フェーズを単独でテストできるようになり、オーケストレーターは意図を読みやすく列挙したものになります。
**重大度:**変更を要求します。
8. スタックの健全性(重大なリグレッション)
これはスタイル上の指摘ではありません。PR によって持ち込まれたリグレッションであり、マージ前に修正する必要があります。
| 問題 | ルール | 指摘文 |
|---|---|---|
middleware.ts が作成されている、または残されている | Next.js 16 ではエントリーポイントが proxy.ts に変更されており、古い名前は無視されます | "これは middleware.ts です。エントリーポイントは Next.js 16 で変更されたため、proxy.ts に名前を変更してください。" |
tailwind.config.ts または tailwind.config.js が追加されている | v4 では CSS ベースの設定(CSS 内の @theme)を使用します。JS 設定は v3 のものです | "この PR で tailwind.config.ts が作成されています。これは v3 の設定です。削除し、v4 に従ってテーマトークンを CSS の @theme ブロックに移してください。" |
@apply ディレクティブまたは bg-opacity-* クラスが追加されている | v3 のパターンです。v4 のスラッシュ構文に移行する必要があります | 該当する行と v4 での同等の記述を具体的に示してください。 |
スクリプト、CI YAML、またはドキュメント内で npm run、npx、yarn add、pnpm exec が使われている | パッケージマネージャーは Bun のみです。bun run/bunx/bun add に置き換えてください | "2 行下の npx prisma migrate は bunx prisma migrate にしてください。" |
**重大度:**すべてブロッカーです。
9. 設計の純粋性 — 同じ動作を、より簡潔な形で
構造について最も重要な問いは「これは正しいか?」ではなく、「同じ動作を、大幅に少ない構造で表現できないか?」です。避けられる複雑さの上に正しい動作を実装する変更は、単純化の機会を逃しています。そして、こうした機会損失は積み重なります。
次の場合に指摘してください:
- 新しいステートマシン、設定オブジェクト、または分岐ツリーが、単一の導出値やデフォルト値で表せる判断を符号化している。
- 妥当なデフォルトや早期リターンによって一般的な処理経路に統合すれば分岐全体を削除できるのに、特殊ケースが追加されている。
- リファクタリングでコードを並べ替えているだけで、読者が一度に把握する必要のある構成要素が減っていない。
**推奨する対処法:**状態モデルを組み立て直します。特殊ケースをデフォルトに統合します。磨き上げるより、削除して統合するほうが効果的です。「コード柔道」で複雑さの分類自体をなくし、整えるだけにとどめないでください。
指摘文:「この 3 状態フラグは、導出された 1 つのブール値に置き換えられます。動作は同じまま、分岐全体を削除できます。磨き上げるのではなく、組み立て直してください。」
**重大度:**変更を要求します(より簡潔な形によって正しさを損なう落とし穴も取り除ける場合は、ブロッカーです)。
10. 直接的な実装とマジックの比較
巧妙な仕組みより、分かりやすい仕組みを優先してください。過度に汎用的な抽象化、隠れた前提、制御フローを見えにくくする間接化は、記述の手間を省く以上に、読み解くコストを増やします。
次の場合に指摘してください:
- 具体的な呼び出し元が 1 つしかないのに、汎用的/パラメーター化された仕組みが導入されている(「将来を見越した汎用化」)。
- 呼び出し箇所を見ても分からない暗黙の順序、グローバル変数、命名規則、リフレクション/メタプログラミングなどの隠れた前提に動作が依存している。
- その間接化を正当化する不変条件がないのに、動的ディスパッチ、イベントによる間接化、複雑な設定が直接呼び出しに置き換わっている。
**推奨する対処法:**実際の呼び出し元が 1 つだけなら、直接的な形にインライン化します。境界で前提を明示するか、マジックを取り除いてください。
指摘文:「ハンドラーが 1 つしかないのに、汎用レジストリーを導入しています。削除して、ハンドラーを直接呼び出してください。2 つ目の呼び出し元が実際に現れたときに、抽象化を追加してください。」
**重大度:**変更を要求します。
対象外
- 構造上の欠陥に根差していないスタイル上の好み(インデント、命名の細かな違い)。
- 正確性に関するバグやテストカバレッジの不足。これらは
/code-reviewの対象です。 - セキュリティ上の問題。これらは
/security-auditの対象です。 - 正確性レビューのハーネスですでに扱われている、リポジトリの指示ファイルに関するルール違反。
- ドキュメントの不足(インラインドキュメント、README の更新)。明示的に依頼されていない限り対象外です。
- カバレッジ率の下限。特定の数値ではなく、新たに追加された重要な経路にテストがない場合のみ指摘します。
出力形式
重要度の高い順に指摘事項を並べます。
## Structural Review
### Blockers (must fix before merge)
[File-size violations, non-atomic mutations, stack regressions (middleware.ts, tailwind config, npm/yarn usage), bare unknown without guard, design-purity simplifications that also remove a correctness footgun]
### Request Changes (significant structural debt)
[Abstraction-earns-keep failures, canonical-layer violations, spaghetti branching, sequential orchestration smells, inline interfaces, missed design-purity simplifications, speculative generality / magic indirection]
### Minor (low-debt, flag for awareness)
[as X casts without comments, small layer suggestions with obvious inlines]
### Approved Axes
[Axes with no findings — confirm clean]
指摘事項ごとに、以下を記載します。
- ファイルと行の範囲(概算ではなく正確な範囲)。
- 構造上の欠陥を述べる1文。
- 具体的な修正方法を述べる1文。
- レビュアーの定型句(上記の例を使うか、対象コードに合わせて調整してください)。
承認基準
ブロッカーおよび変更依頼の基準は、上記の各観点に記載されています。判断が曖昧な場合は、変更依頼を既定としてください。曖昧さによる負担を負うのは、コードベースではなく作成者です。