work-review について
work-review は、保守性、抽象の質、コード構造を厳密に確認するエージェントスキルです。複数の視点、測定可能なチェック、網羅性確認を組み合わせ、優先順位付きの構造化された指摘を出します。
主な機能
- 01差分範囲とファイル行数の測定
- 02ビルド、型チェック、テストの実行
- 03構造と品質に関する複数のレビュー視点
- 04指摘の重複排除と順位付け
- 05リポジトリ全体の網羅性確認
- 06構造化されたレビュー結果
活用シーン
- 01ブランチ変更の保守性監査
- 02局所的な整理を超えた構造改善の発見
- 03リファクタ後の関連呼び出し箇所の確認
- 04コードレビュー向け優先度付き指摘の作成
インストール
コマンドをコピーしてターミナルで実行します。
npx skills add https:/ / github.com/ djalmajr/ skills --skill work-reviewインストール後、Claude Code で/work-reviewを入力して読み込まれたか確認します。
詳細
厳格なコード品質レビュー
実装の品質、保守性、抽象化の品質、コードベースの健全性に焦点を当てた、特に厳格なレビューにはこのスキルを使用してください。
何よりも、このスキルではコード構造について積極的に踏み込んだ提案を行うよう、レビュアーに促します。局所的な整理の機会を見つけるだけにとどめないでください。動作を維持しながら、実装を劇的にシンプルに、小さく、直接的で、洗練されたものにする再構成、つまり「コード柔術」の手を積極的に探してください。
このスキルは名前を指定した場合にのみ呼び出されます(disable-model-invocation: true)。自動的に起動することはありません。/work-review を直接呼び出してください(例:オーケストレーターのレビューの流れで work-check 経由で呼び出されます)。
これは品質システムにおけるレビューパターンです。ワークフローモードで実行します。これは、サブエージェントを編成する際にエージェントが手作業で実行する、決定論的な規律です。オーケストレーションモデル(3つの形状、実行の仕組み、スキーマ規約)は ../work/references/workflow-mode.md に記載されています。防止対象の失敗モード(L1~L8)は ../work/references/anti-error-lessons.md に記載されています。このスキルでは、それらのドキュメントの内容を繰り返さず、参照先を示し、単独でレビューを実行するために必要な内容だけをインラインに記載します。
ワークフローの形状
パイプライン → 1つのバリア統合。 作業の自然な単位は (ファイル × レンズ) です。変更された各ファイルは、それぞれ独立してレビューの各レンズを通過します。ファイル A がレイヤリングの確認中でも、ファイル B は構造の確認中であり、レンズ間にバリアはありません(実時間はレンズ数の合計ではなく、最も時間のかかる単一ファイルの処理チェーンによって決まります)。パイプラインとして実行してください。最後に、実質的な項目間依存関係が1つだけあります。複数のレンズやファイルから報告された同じ構造上の問題を1つの指摘にまとめ、全体の網羅性確認を対象範囲の結合に対して実行する重複排除・統合バリアです。このバリアには正当な理由があります(項目間の重複排除と、単一の優先順位付き出力)。また、これは単純な集合演算であり、エージェント呼び出しではありません。保守性に特化した workflow-mode.md §2 のレビュー用ハーネスは、「レンズ(並列)→ 重複排除バリア → 網羅性確認 → 優先順位付き出力」です。これはループではありません。差分は既知の有限な対象範囲であり、回数が不明な「すべて見つける」処理を繰り返すことはありません。
コアプロンプト
次の基本方針から始めてください。
現在のブランチの変更について、コード品質を徹底的に監査してください。 動作に影響を与えずにコード品質を大きく改善できるよう、変更の構造や実装方法を見直してください。 抽象化とモジュール性を改善し、スパゲッティコードを減らし、簡潔さと読みやすさを向上させてください。 積極的に取り組んでください。コードベースの一部を再構成して実装を改善する明確な方法があれば、実行してください。 極めて徹底的かつ厳密にレビューしてください。慎重に確認してから変更してください。
ステップ 0 — 測定(意見の前に判定材料)— L1
意見を一つでも持つ前に、手軽で決定論的な判定材料を実行します。 「二度測ってから一度切る」は、ここでは単なる標語ではなく、ゲートです。ファイルサイズを目測したり、「大きくなりすぎた」と断言したりしてはいけません。数値で証明してください。後続フェーズが参照する baseline オブジェクトにすべて記録します(重大度に関する主張はこのオブジェクトを根拠とする必要があります)。
- 差分の範囲を確認する —
git diff --numstat(およびgit diff --stat)を実行して、変更されたファイルの正確な一覧と、ファイルごとの追加・削除行数を取得します。これが対象範囲であり、その他すべての確認もこの範囲に限定されます。 - 1,000 行ルールを証明する — 変更された各ファイルについて、
wc -l <file>で 現在の 行数を取得し、差分適用前の行数を復元します(現在の行数 − 追加行数 + 削除行数、またはgit show HEAD:<file> | wc -l)。§1 の「1,000 行未満から 1,000 行超へ」という懸念は、2 つの数値に関する事実であり、決して推測ではありません。差分適用後に 1,180 行となったファイルでも、もともと 1,140 行あった場合と、PR によってしきい値をまたいで 920 行から 1,060 行になった場合とでは、指摘の重みは異なります。 - ビルド/型チェック/テストを実行する — プロジェクトの型チェックとテストのコマンドを実行します。ビルドやテストを壊す構造改善案は、よりクリーンとは言えません。動作を維持するという主張(§Approval Bar、L8)はすべて、このベースラインを基準に確認します。
- ベースラインを記録する — 後続フェーズと人間が監査できるよう、ベースラインを一度だけ出力します。
{
"baseline": {
"diffFiles": [{ "file": "src/upload-handler.ts", "added": 140, "removed": 12 }],
"lineCounts": [{ "file": "src/upload-handler.ts", "before": 920, "after": 1048, "crosses1k": true }],
"build": { "command": "<typecheck> && <test>", "status": "pass", "notes": "" },
"oraclesRun": ["git diff --numstat", "wc -l", "<typecheck>", "<test>"]
}
}
判定材料が利用できない場合(テストコマンドがない、ファイルが切り離されているなど)は、その旨を baseline.build.notes に明記してから続行します。何も記載しないと「実行して合格した」と受け取られます(L7)。
出力契約(構造化)— L6
すべての指摘はスキーマ検証済みのオブジェクトであり、呼び出し元が grep しなければならないような散文ではありません。このスキルをサブエージェントとして実行する場合、以下の契約に合致する単一のフェンス付き ```json ブロックを返信の末尾に出力します。呼び出し元は形式を検証し、不正な結果は失敗扱いにする前に一度だけ再要求します(workflow-mode.md §3–4 を参照)。
{
"id": "cr-upload-host-branch",
"file": "src/http/routes/upload-handler.ts",
"lineRange": "168-211",
"lens": "structural",
"severity": "major",
"reReadSources": ["src/http/routes/upload-handler.ts:168-211"],
"workDone": true,
"evidence": "upload-handler.ts:171 — new `if (req.query.hostname)` branch bolted onto the auth path; quote: `const tenant = req.query.hostname ?? session.tenant`",
"suggestedRefactor": "Hoist tenant resolution into a single `resolveTenant(session)` policy; the host-override branch disappears from the request path.",
"behaviorPreservingProof": "Same tenant for every existing caller (session.tenant), verified against Step-0 typecheck+tests baseline=pass; only the dead host-override path is removed.",
"confidence": "high"
}
フィールドのルール:
lens— 列挙値structural | duplication | abstraction | layering | orchestration | size | completeness。severity— 列挙値blocker | major | minor | consider。雰囲気ではなく、検証済みの影響(L6)に基づきます。blocker= Approval Bar に照らしてマージを止めるべきと考えられるもの、major= 保守性の実質的な悪化、minor= 対応する価値はあるが影響範囲が狭いもの、consider= 任意の仕上げです。推測に基づく整理をblockerに格上げしたり、実際のスパゲッティ化をconsiderに格下げしたりしてはいけません。evidence— 具体的なfile:lineと、呼び出し元が再確認できる短い原文引用です。根拠がなければ、その指摘はblockerではなく判断保留です(L8)。reReadSources—string[]。この指摘の確認のため、現在のソースを独自に再度開いた、正確なfile:lineの範囲です(L2/L8)。空の場合、その指摘は再読せずに主張されています。workDone—boolean。この指摘を裏付ける再読や判定作業を実際に実行した場合に限りtrueです。初回確認時の記憶に基づくものは含みません。falseは検証作業が行われていないことを示します。- 判断保留のルール(
work-refuteと同様):reReadSourcesが空、またはworkDone:falseのblocker/majorは判断保留です。必要な作業が行われていないため、重大度をminor/considerに下げて記録します(マージを止めることはできません)。blockerとして提示してはいけません。 suggestedRefactor— 「X の名前を変更する」ではなく、構造上の変更として示す code-judo の手法です。behaviorPreservingProof— 構造に関するblocker/majorの主張では必須です。ステップ 0 のbaselineと照合した、リファクタリングによって動作が維持されるという根拠を記載します。confidence— 列挙値high | medium | low。以下のトリアージに使用します。
統合レビューでは、何も黙って破棄されないよう、次のエンベロープを返します(L7)。
{
"baseline": { "...": "see Step 0" },
"findings": [ { "...": "objects above, ranked by severity then confidence" } ],
"droppedTail": [ { "id": "cr-x", "file": "...", "oneLiner": "minor: var name shadows outer scope" } ],
"caps": { "lensesRun": ["structural","duplication","..."], "lensesSkipped": [], "tier": "high", "reason": "" },
"verdict": "request-changes"
}
verdict — 列挙値 approve | approve-with-nits | request-changes | block。
レビューレンズ(多角的なパス)— L3
同じプロンプトで N 回パスを実行しても、網羅性ではなく冗長性にすぎません(L3)。重要な差分では、それぞれ異なるレンズを使って重点的にレビューしてください。多様な視点こそが、単一の一括レビューでは見落とすものを捉えます。レンズはそれぞれ 1 つの列挙値に対応します。
- structural — コード柔術:分岐/ヘルパー/モード/レイヤー全体をなくせますか?変更が必然に感じられるような捉え直しはありますか?(§0、「野心的な変更」の要件をカバーします。)
- duplication — コピー&ペーストされたロジック、ほぼ重複するヘルパー、すでに標準的なものがあるのに独自に作られたユーティリティ(§6、「フラグを立てる項目」)。
- abstraction — 薄いラッパー、ID/パススルーのヘルパー、単純なデータ形状を隠す汎用的な「マジック」、実際のコントラクトを曖昧にするキャスト/
any/unknown/オプショナル性(§4、§5)。 - layering — 共有パスへの機能ロジックの漏出、API を通じた実装詳細の漏出、誤ったパッケージ/モジュール/レイヤーに置かれたロジック(§6)。
- orchestration — 独立した作業を不要に逐次実行すること、アトミックでない部分更新フロー(§7)。
- size — ファイル/コンポーネントのサイズと分割を、ステップ 0 の数値に照らして評価します(§1)。
- completeness — 差分の外側を対象にした一括調査(次のセクション)。
パイプラインの仕組み: バックグラウンドで、適切なサブエージェントの役割を割り当てて、(file, lens) ごとに重点を絞ったサブエージェントを 1 つ起動します(または、下位ティアでは差分全体を対象にレンズごとに起動します)。これらは読み取り専用なので、ワークツリーの分離は不要です。すべて完了するまで待ち、結果をそれぞれ収集します。その後、単一の BARRIER マージを実行します。同じ file、重複する lineRange、同じ根本原因を示す指摘を重複排除し(重大度が最も高く、根拠が最も明確なものを残し)、その後にのみ次へ進みます。重複排除は単純な集合操作であり、エージェント呼び出しではありません。
並行処理と予算(上限を黙って設けない — L7): min(16, cores−2) 件ずつウェーブで起動し、超過分はキューに入れます。予算が少なくなった場合は、レンズまたはファイルを明示的に除外し、その除外内容を caps.lensesSkipped / caps.reason に記録してください(また、ハーネスの todo/計画ツールにも反映してください)。網羅範囲を黙って縮小してはいけません。停止またはエラーになったレンズのサブエージェントは .filter(Boolean) で除外し、「問題なし」のパスとして数えず、省略したことを caps に記録します(workflow-mode.md の null 処理と同様です)。長時間の監査では、{ phasesDone, lensesRun, findings, droppedDueToCaps } を quality-journals/<task-id>.json(エージェントのデータディレクトリ内)に記録し、再開時にカバレッジを再実行せず読み込めるようにします。
網羅性の調査(差分の外側)— L5
最も価値の高い網羅性の確認は、レビュー済みの箇所を再確認することではなく、差分が一度も触れていないものの、本来変更すべき箇所を見つけることです(L5)。レンズによるレビューの後、各カテゴリで少なくとも 1 つのリポジトリ全体を対象にした調査手段を実行してください。
- 抽出/名前変更/リファクタリングされたシンボルすべて →
grep/ripgrep でリポジトリ全体を検索し、ほかの呼び出し箇所を確認します。シグネチャの変更や抽出により、差分内の呼び出し箇所だけが更新され、兄弟箇所が古い形式のまま呼び出している場合、これは差分内のレンズでは見つけられない実際の指摘です。 - 重複ブロックすべて → 兄弟モジュールを grep して同じパターンを確認します。これが 3 つ目のコピーなら今すぐ抽出すべきでしょうか、それとも再利用できる標準ヘルパーがすでにありますか(§6)。
- 削除すべて → 現在のデッドコードを grep して確認します。削除されたエクスポートへの参照、到達不能になった分岐、どこからも読み取られない設定キーを探します。
「網羅性に関する指摘なし」と「調査を省略」を区別できるよう、実行したグローバル調査をそれぞれ caps に記録してください(L7)。この調査は、差分を対象とする ../work-gaps/SKILL.md のパターンと同じ最終ゲートです。
譲れない追加基準
上記の基本プロンプトに加え、以下の明示的なレビュー規則を適用してください。
-
構造の簡素化を積極的に目指してください。
- 「もう少しすっきりできるかもしれない」というところで止めないでください。
- 変更を再構成し、分岐、ヘルパー、モード、条件分岐、レイヤー全体をなくせる機会を探してください。
- 振り返ったときに、必然だったと感じられる解決策を優先してください。
- 既存のアーキテクチャをより効果的に活用し、変更を大幅に簡素化して、より洗練されたものにする「コード柔術」のような方法があることを前提にしてください。
- 複雑さを組み替えるのではなく削除できる道が見えたら、その方向を強く推してください。
-
非常に強い理由がない限り、PR によって 1,000 行未満のファイルが 1,000 行を超えることを許容しないでください。
- これは原則として、コード品質上の重大な懸念事項として扱ってください。
- ファイルが 1,000 行を超えて肥大化するのではなく、ヘルパー、サブコンポーネント、モジュール、局所的な抽象化を切り出すことを優先してください。
- 差分によってこのしきい値を超える場合は、まずコードを分割すべきではないか、明確に問いかけてください。
- 構造上やむを得ない理由があり、分割後のファイルも明確に整理されている場合に限り、例外を認めてください。
-
既存コードに無秩序なスパゲッティ状の複雑さを加えないでください。
- 新たな場当たり的な条件分岐、散在する特殊ケース、無関係な処理フローに挿入された単発の分岐を厳しく警戒してください。
- 「あちこちに奇妙な if 文が追加される」変更は、単なるスタイル上の指摘ではなく、設計上の問題として扱ってください。
- 既存の処理フローを絡ませるのではなく、専用の抽象化、ヘルパー、ステートマシン、ポリシーオブジェクト、または別モジュールにロジックを移すことを優先してください。
- 技術的には動作していても、周辺コードの理解を難しくする変更は指摘してください。
-
動作するコードを受け入れるだけでなく、設計の改善を重視してください。
- 動作を変えずに構造を大きくすっきりさせられるなら、より整理された実装を強く推してください。
- コードベースをより複雑にしたままの「動けばよい」実装を安易に承認しないでください。
- 同じ複雑さを別の場所に移すだけのリファクタリングより、要素そのものを取り除く簡素化を強く優先してください。
-
ハック的、または魔法のようなコードより、直接的で地味でも保守しやすいコードを優先してください。
- 脆弱で場当たり的な振る舞いや「魔法」のような処理は、コード品質上の問題として扱ってください。
- 単純なデータ形状の前提を隠す汎用的な仕組みには慎重になってください。
- 明瞭さにつながらない薄い抽象化、同一性を保つだけのラッパー、処理をそのまま通すだけのヘルパーは指摘してください。
-
保守性に関わる場合は、型と境界の明確さを強く求めてください。
- より明確な型境界を定義できるのに、不要なオプショナル型、
unknown、any、型キャストの多用に頼っていないか問い直してください。 - 形の曖昧な場当たり的オブジェクトより、明示的な型付きモデルや共有コントラクトを優先してください。
- 不明瞭な不変条件を暗黙のフォールバックで補う分岐がある場合は、境界を明示すべきではないか問いかけてください。
- より明確な型境界を定義できるのに、不要なオプショナル型、
-
ロジックは正規のレイヤーに置き、既存のヘルパーを再利用してください。
- 機能ロジックが共有経路に漏れたり、実装の詳細が API に漏れたりしていないか指摘してください。
- 独自の一回限りの実装より、既存の正規ユーティリティやヘルパーを優先してください。
- アーキテクチャのずれをそのまま定着させるのではなく、適切なパッケージ、サービス、モジュールにコードを配置してください。
-
より明快な構造が明らかな場合、不要な逐次オーケストレーションや非アトミックな更新を設計上の懸念として扱ってください。
- 独立した処理が理由なく直列化されている場合は、並列に実行すべきではないか問いかけてください。
- 関連する更新によって状態の一部だけが適用された状態になり得る場合は、よりアトミックな構造を強く推してください。
- マイクロ最適化を過度に重視する必要はありませんが、実装を脆くする回避可能なオーケストレーションの複雑さは指摘してください。
主なレビュー質問
意味のある変更ごとに、次の点を確認してください。
- 大幅にシンプルにできる「コード柔道」のような工夫はありませんか?
- 必要な概念、分岐、ヘルパー層を減らせるように、この変更を捉え直せませんか?
- これはローカルアーキテクチャを改善していますか、それとも悪化させていますか?
- より適切な抽象化が必要な箇所に、この差分で分岐の複雑さが加わっていませんか?
- 以前はまとまりのあったモジュールが、より結合し、状態を持ち、見通しにくくなっていませんか?
- このロジックは適切なファイルとレイヤーにありますか?
- この変更によって、ファイルやコンポーネントが健全なサイズの上限を超えていませんか?
- 繰り返される条件分岐は、不足しているモデルやヘルパーを示していませんか?
- 実装は直接的で読みやすいですか。それとも特殊ケースや偶発的な制御フローに依存していますか?
- この抽象化は本当に役に立っていますか。それとも単なるラッパーですか?
- この差分で、実際の不変条件を見えにくくするキャスト、オプショナル性、場当たり的なオブジェクト形状が導入されていませんか?
- このロジックは正規のレイヤーにありますか。それとも差分によって境界を越えて詳細が漏れていませんか?
- このオーケストレーションは、必要以上に逐次的、またはアトミック性が低くなっていませんか?
積極的に指摘すべき点
次のような点が見られたら、問題として積極的に取り上げてください。
- よりすっきりと捉え直すことで、複雑さのカテゴリ全体を取り除けるのに、複雑な実装になっている。
- コードを移動するだけのリファクタリングで、読者が頭に入れておく必要のある概念の数が減っていない。
- PR によってファイルが 1000 行を超える。特に、新しいコードを別ファイルに分割できる場合。
- 無関係なコードパスに新たな条件分岐が付け足されている。
- 既存の制御フローを複雑にする、一度きりのブール値、null 許容のモード、フラグ。
- 機能固有のロジックが汎用モジュールに漏れている。
- 単純な構造を覆い隠し、コードを理解しにくくする汎用的な「マジック」処理。
- シンプルにするものがないのに間接性を加える薄いラッパーや同一性を表すだけの抽象化。
- 実際の契約を曖昧にする不要なキャスト、
any、unknown、オプショナルなパラメーター。 - ヘルパーを抽出せずに、ロジックをコピー&ペーストしている。
- すでに複雑な関数の途中に、限定的なエッジケース処理を実装している。
- テストには通るものの、コードのモジュール性や可読性を損なうリファクタリング。
- 恒久的な負債になりそうな「一時的な」分岐。
- コードベースにすでに標準のユーティリティがあるのに、独自のヘルパーを作っている。
- より中心的な場所に置くべきロジックが、誤ったレイヤーやパッケージに追加されている。
- 明らかに独立した処理を、並列実行よりもシンプルで明快にできるのに、非同期処理を逐次実行している。
- 必要以上に状態のアトミック性を損なう部分更新ロジック。
推奨する改善策
コード品質の問題を見つけた場合は、次のような改善案を優先してください。
- 磨き上げるのではなく、間接化の層を丸ごと削除します。
- 条件分岐を集約するのではなく、条件分岐が不要になるように状態モデルを見直します。
- 機能が既存の抽象化を自然に拡張するものとなるよう、所有権の境界を変更します。
- 特例を減らし、例外の少ないシンプルな既定フローに特例処理を置き換えます。
- ヘルパーや純粋関数を抽出します。
- 大きなファイルを、目的を絞った小さなモジュールに分割します。
- 機能固有のロジックを専用の抽象化の背後に移します。
- 条件の連鎖を、型付きモデルまたは明示的なディスパッチャーに置き換えます。
- オーケストレーションとビジネスロジックを分離します。
- 重複した分岐を、より明快な単一のフローにまとめます。
- APIを明確にする効果のないラッパーを削除します。
- 似たものを新たに作るのではなく、既存の標準ヘルパーを再利用します。
- 型の境界をより明確にして、制御フローをシンプルにします。
- ロジックを、その概念をすでに所有しているパッケージ/モジュール/レイヤーに移します。
- オーケストレーションもシンプルになる場合は、独立した作業を並列化します。
- 部分的な状態のほうが把握しづらくなる場合は、関連する更新をよりアトミックなフローに再構成します。
本質的な問題が構造にある場合に、「名前を変えてみてはどうでしょう」といったフィードバックだけで済ませないでください。 大幅にシンプルにできる見込みがあるのに、単に同じ複雑なアイデアを少しきれいにしただけで満足しないでください。
レビューのトーン
品質に対して、率直で真剣に、妥協のない姿勢で臨んでください。 失礼にはならないようにしつつ、保守性の大きな問題を軽い提案のように和らげないでください。 コードベースをさらに複雑にしている場合は、そのことを明確に伝えてください。 実装で大幅な単純化の機会を逃している場合も、そのことをはっきり伝えてください。
よい表現の例:
この変更でファイルが1,000行を超えてしまいます。まず分割できませんか?この変更は、すでに複雑なフローに特例分岐をもう1つ追加しています。独自の抽象化の背後に移せませんか?動作はしますが、周辺のコードをさらにスパゲッティ化しています。動作は維持しつつ、実装を再構成しましょう。機能ロジックが共有パスに漏れ出しているように感じます。切り離せませんか?この抽象化は不要に思えます。直接的なフローのままにできませんか?なぜここでキャスト/オプショナルが必要なのでしょうか?境界をもっと明確にできませんか?既存の別の場所にあるもの専用のヘルパーを新たに作っているようです。標準のものを再利用できませんか?ここには、これを大幅にシンプルにする巧みな手があると思います。分岐がなくなるように見直せませんか?このリファクタリングは複雑さを移動させているだけで、実際には削減できていません。モデル自体をシンプルにする方法はありませんか?
出力に関する期待事項
以下の順に指摘事項を優先してください。
- 構造的なコード品質の後退
- 大幅な簡素化/コード柔術的な再構成の機会の見落とし
- スパゲッティ化/分岐の複雑さの増大
- コードの理解を難しくする境界/抽象化/型コントラクトの問題
- ファイルサイズと分割に関する懸念
- モジュール性と抽象化の問題
- 可読性と保守性に関する懸念
トリアージし、省略しないでください(L7)。「細かな指摘であふれさせない」とは、順位を下げることであり、黙って除外することではありません。severity(blocker > major > minor > consider)で、次にconfidenceで、すべての指摘に順位を付けてください。
- 確信度の高い項目(
blocker/majorと、確信度がhighのminor)は、evidence、suggestedRefactor、および(構造上のblocker/majorについては)behaviorPreservingProofを含め、詳細に提示してください。 - 残りの項目は下位に回してください。詳細に展開しないことにした
minor/considerはすべて、droppedTailに一行で記載し、無かったことにしないでください。読者が「差分に問題がない」のか「詳細な記述を途中でやめた」のかを判断できる必要があります。黙って省略すると、「すべて確認済み」という印象を与えてしまいます。
つまり、確信度の高い指摘を少数だけ詳細に記載し、さらに**省略した項目を漏れなく簡潔に列挙するdroppedTail**を付けてください。長々とした見た目だけの指摘リストにも、説明を黙って短縮することにもしてはいけません。
承認基準
動作が正しいように見えるというだけで承認してはいけません。 承認基準は次のとおりです。
- 明らかな構造上のリグレッションがない
- シンプルさを大幅に向上できる明確な方法があるのに、それを活用できていないという見落としがない
- 不当なファイルサイズの肥大化がない
- 特別扱いの分岐によって、明らかにスパゲッティ化が進んでいない
- コードの理解を難しくする、明らかに場当たり的、または魔法のような抽象化がない
- 本来の設計を見えにくくする、不要なラッパー/キャスト/オプショナリティの変更がない
- 明らかなアーキテクチャ境界の漏出や、回避可能な共通ヘルパーの重複がない
- 保守性を実質的に向上させる、明確な分割の機会を逃していない
作成者が明確に正当化できない限り、次の項目は阻止要因とみなします。
- 妥当なコード整理によって削除できる複雑さがあるのに、PR が多くの偶発的な複雑さをそのまま残している
- PR によって、1000 行未満のファイルが1000 行を超える
- PR が場当たり的な分岐を追加し、既存のフローをさらに複雑にしている
- PR が、共有コード全体に機能チェックを散在させて局所的な問題を解決している
- PR が、設計をより間接的にする不要な抽象化、ラッパー、またはキャストの多い契約を追加している
- 明確に適切な配置先があるのに、既存のヘルパーを重複させたり、ロジックを誤ったレイヤーに置いたりしている
阻止判定には第2の軸(L6 + L8)があります。 構造上/「コード整理」に関する項目を推定上の blocker とするのは、次の両方を満たす場合に限ります。
- 証拠を独立して再確認している(L2/L8)。 現在のソースで該当する
file:lineを開き直して問題を確認していること。引用されたスニペットや最初に読んだときの記憶を信頼してはいけません。指摘には空でないevidenceが必要です。再確認なしに主張された項目は判断保留であり、最大でもminor/considerとして記録し、blockerにしてはいけません。 - 動作を維持するという主張を Step-0 のベースラインと照合している(L1/L8)。 「この再構成は影響がない」という主張が §Step-0 の
baseline(型チェック/テストの状態、行数)と整合しない場合、その主張は根拠として機能しておらず、判定を引き下げます。成功しているベースラインに反する、または実際には一度も実行されていないbehaviorPreservingProofは、安易な判定であり、blockerの根拠にはなりません。
これは L6 の両面からの調整です。誤検知(バグではない項目、見た目だけの項目、「こちらのほうがきれいだと思う」という項目をマージ阻止要因に格上げしないこと)と、見逃し(「技術的には動く」からと、本当に絡み合いを生むリグレッションを consider に格下げしないこと)の両方を防ぎます。blocker とするには、保守性への悪影響が実際に確認でき、その根拠が示されていなければなりません。推測上の懸念は決して blocker ではありません。
これらの条件を満たさない場合は、明確で実行可能なフィードバックを残し、より適切な分割を促してください。
工数ティア
依頼内容に応じて使用する仕組みを調整します(workflow-mode.md §5)。網羅性を監査できるよう、ティアと省略した観点を caps に記録してください(L7)。
| ティア | 観点 | 網羅性チェック | 並列化 | 適用場面 |
|---|---|---|---|---|
| low | 構造とサイズのみ。信頼度 high の指摘のみ | 省略(記録する) | インライン。処理の振り分けなし | ごく小規模/機械的な差分、少数ファイル |
| medium | 構造、重複、抽象化。信頼度の高いものを優先 | 省略(記録する) | インライン、または数回の確認 | 通常の機能差分 |
| high | すべての観点 | 完全実施(§Completeness にあるすべてのグローバルオラクル) | パイプラインで振り分け、min(16, cores−2) 件ずつのウェーブ | 実質的な変更/リスクの高い変更/複数ファイルにまたがる差分 |
| ultra | すべての観点をファイルごとに並列実施(file × lens) | 完全実施 | 制御された最大並行数 | 大規模な差分、詳細監査、「すべてを見つける」依頼 |
low/medium では、意図的に網羅性よりシグナルを優先します。網羅性の grep を省略し、最高信頼度の項目のみを提示します。このトレードオフは caps.reason に明記し、黙って省略してはいけません。high では差分の範囲外も含めた完全なチェックを追加します。ultra ではさらに、ファイルごとに観点を並列化するため、各 (file, lens) が読み取り専用のサブエージェントとして処理され、最後の重複排除バリアで結果を統合します。
修正方法 — 修正を委任し、こっそり紛れ込ませない
このスキルは、問題を検出して優先順位を付けるものであり、差分をひそかに書き換えるものではありません。ユーザーが品質改善を適用するよう求めた場合(またはオーケストレーターを --fix 付きで実行した場合)は、優先順位付けされた findings を /simplify に渡してください。これは、作業ツリーに再利用/簡素化/抽象化/レイヤリングの修正を適用する兄弟スキルです。役割分担を明確に保ってください。/work-review は構造化された判定を作成し、/simplify はそこで示されたコード整理を実行します。blocker に対処する前に、特定の動作維持に関する主張を検証する場合は、視点の異なるレンズ(L3)を用いて ../work-refute/SKILL.md に回してください。
実例(簡潔な説明用)
2 ファイルの差分:upload-handler.ts(+140/−12、920 → 1,048 行)と tenant.ts(+8/−0)。
- ステップ 0 —
git diff --numstatとwc -lにより、upload-handler.tsが 1k を超えたこと(crosses1k:true)を確認します。型チェックとテストはpass。結果をbaselineに記録します。 - レンズ(upload-handler、高ティア) —
structuralは認証パスに追加されたif (req.query.hostname)分岐を指摘します(Output Contract の検出例)。sizeは、1k 超過の原因が同じブロックであることを確認します。layeringは、テナントロジックがtenant.tsではなくルートハンドラーに置かれるようになった点を指摘します。 - バリアマージ — 3 つのレンズが捉えたものは、重複する
lineRangeが示すように、同じ根本的な問題です。そのため、3 件ではなく、1 件のmajor/blockerの検出事項に重複排除されます。 - 完全性の確認 — リポジトリ全体で
rg 'resolveTenant\|\.tenant'を実行すると、すでにセッションベースのテナント解決を行っている別の呼び出し箇所が 2 つ見つかります。つまり、ホストによる上書きの分岐は冗長であり、既存の処理とも異なります。これは差分外の検出事項で、差分内のレンズでは見つけられませんでした。 - ブロッカーのゲート(L6+L8) — ソースから
upload-handler.ts:171を読み直し(証拠を確認)、behaviorPreservingProofをステップ 0 の成功したベースラインと照合して、blockerに昇格させます。verdict: "request-changes"とし、唯一のminor(シャドーイングされた変数)はdroppedTailに移します。修正方法を/simplifyに渡します。