From e48c267562c35b1f3cf2e8b897acf784e3384c38 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Sun, 8 Feb 2026 17:33:36 +0900 Subject: [PATCH] =?UTF-8?q?=E3=82=88=E3=82=8A=E5=9F=BA=E6=BA=96=E3=82=92?= =?UTF-8?q?=E5=8E=B3=E6=A0=BC=E3=81=AB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- builtins/en/instructions/ai-review.md | 6 ++++++ builtins/en/instructions/review-ai.md | 6 ++++++ builtins/en/instructions/review-arch.md | 6 ++++++ builtins/en/instructions/review-cqrs-es.md | 6 ++++++ builtins/en/instructions/review-frontend.md | 6 ++++++ builtins/en/instructions/review-qa.md | 6 ++++++ builtins/en/instructions/review-security.md | 6 ++++++ builtins/en/personas/expert-supervisor.md | 14 +++----------- builtins/en/policies/review.md | 9 +++++++++ builtins/ja/instructions/ai-review.md | 6 ++++++ builtins/ja/instructions/review-ai.md | 6 ++++++ builtins/ja/instructions/review-arch.md | 6 ++++++ builtins/ja/instructions/review-cqrs-es.md | 6 ++++++ builtins/ja/instructions/review-frontend.md | 6 ++++++ builtins/ja/instructions/review-qa.md | 6 ++++++ builtins/ja/instructions/review-security.md | 6 ++++++ builtins/ja/personas/expert-supervisor.md | 8 +++----- builtins/ja/policies/review.md | 9 +++++++++ 18 files changed, 108 insertions(+), 16 deletions(-) diff --git a/builtins/en/instructions/ai-review.md b/builtins/en/instructions/ai-review.md index 0a485ff..00ee7d5 100644 --- a/builtins/en/instructions/ai-review.md +++ b/builtins/en/instructions/ai-review.md @@ -8,3 +8,9 @@ Review the code for AI-specific issues: - Plausible but incorrect patterns - Compatibility with the existing codebase - Scope creep detection + +## Judgment Procedure + +1. Review the change diff and detect issues based on the AI-specific criteria above +2. For each detected issue, classify as blocking/non-blocking based on Policy's scope determination table and judgment rules +3. If there is even one blocking issue, judge as REJECT diff --git a/builtins/en/instructions/review-ai.md b/builtins/en/instructions/review-ai.md index 43b86f6..6122161 100644 --- a/builtins/en/instructions/review-ai.md +++ b/builtins/en/instructions/review-ai.md @@ -3,3 +3,9 @@ Review the code for AI-specific issues: - Plausible but incorrect patterns - Compatibility with the existing codebase - Scope creep detection + +## Judgment Procedure + +1. Review the change diff and detect issues based on the AI-specific criteria above +2. For each detected issue, classify as blocking/non-blocking based on Policy's scope determination table and judgment rules +3. If there is even one blocking issue, judge as REJECT diff --git a/builtins/en/instructions/review-arch.md b/builtins/en/instructions/review-arch.md index d345896..f0a6036 100644 --- a/builtins/en/instructions/review-arch.md +++ b/builtins/en/instructions/review-arch.md @@ -8,3 +8,9 @@ Do not review AI-specific issues (already covered by the ai_review movement). - Test coverage - Dead code - Call chain verification + +## Judgment Procedure + +1. Review the change diff and detect issues based on the architecture and design criteria above +2. For each detected issue, classify as blocking/non-blocking based on Policy's scope determination table and judgment rules +3. If there is even one blocking issue, judge as REJECT diff --git a/builtins/en/instructions/review-cqrs-es.md b/builtins/en/instructions/review-cqrs-es.md index ce16f07..88f9609 100644 --- a/builtins/en/instructions/review-cqrs-es.md +++ b/builtins/en/instructions/review-cqrs-es.md @@ -10,3 +10,9 @@ AI-specific issue review is not needed (already covered by the ai_review movemen **Note**: If this project does not use the CQRS+ES pattern, review from a general domain design perspective instead. + +## Judgment Procedure + +1. Review the change diff and detect issues based on the CQRS and Event Sourcing criteria above +2. For each detected issue, classify as blocking/non-blocking based on Policy's scope determination table and judgment rules +3. If there is even one blocking issue, judge as REJECT diff --git a/builtins/en/instructions/review-frontend.md b/builtins/en/instructions/review-frontend.md index 9e4b591..8c33631 100644 --- a/builtins/en/instructions/review-frontend.md +++ b/builtins/en/instructions/review-frontend.md @@ -10,3 +10,9 @@ Review the changes from a frontend development perspective. **Note**: If this project does not include a frontend, proceed as no issues found. + +## Judgment Procedure + +1. Review the change diff and detect issues based on the frontend development criteria above +2. For each detected issue, classify as blocking/non-blocking based on Policy's scope determination table and judgment rules +3. If there is even one blocking issue, judge as REJECT diff --git a/builtins/en/instructions/review-qa.md b/builtins/en/instructions/review-qa.md index d50442a..457d51e 100644 --- a/builtins/en/instructions/review-qa.md +++ b/builtins/en/instructions/review-qa.md @@ -6,3 +6,9 @@ Review the changes from a quality assurance perspective. - Error handling - Logging and monitoring - Maintainability + +## Judgment Procedure + +1. Review the change diff and detect issues based on the quality assurance criteria above +2. For each detected issue, classify as blocking/non-blocking based on Policy's scope determination table and judgment rules +3. If there is even one blocking issue, judge as REJECT diff --git a/builtins/en/instructions/review-security.md b/builtins/en/instructions/review-security.md index 609cd1a..65e0f8b 100644 --- a/builtins/en/instructions/review-security.md +++ b/builtins/en/instructions/review-security.md @@ -3,3 +3,9 @@ Review the changes from a security perspective. Check for the following vulnerab - Authentication and authorization flaws - Data exposure risks - Cryptographic weaknesses + +## Judgment Procedure + +1. Review the change diff and detect issues based on the security criteria above +2. For each detected issue, classify as blocking/non-blocking based on Policy's scope determination table and judgment rules +3. If there is even one blocking issue, judge as REJECT diff --git a/builtins/en/personas/expert-supervisor.md b/builtins/en/personas/expert-supervisor.md index 4a7f71e..6f6c7a7 100644 --- a/builtins/en/personas/expert-supervisor.md +++ b/builtins/en/personas/expert-supervisor.md @@ -38,6 +38,7 @@ Judge from a big-picture perspective to avoid "missing the forest for the trees. | Contradictions | Are there conflicting findings between experts? | | Gaps | Are there areas not covered by any expert? | | Duplicates | Is the same issue raised from different perspectives? | +| Non-blocking validity | Are items classified as "non-blocking" or "existing problems" by reviewers truly issues in files not targeted by the change? | ### 2. Alignment with Original Requirements @@ -86,7 +87,7 @@ Judge from a big-picture perspective to avoid "missing the forest for the trees. When all of the following are met: -1. All expert reviews are APPROVE, or only minor findings +1. All expert reviews are APPROVE 2. Original requirements are met 3. No critical risks 4. Overall consistency is maintained @@ -100,16 +101,6 @@ When any of the following apply: 3. Critical risks exist 4. Significant contradictions in review results -### Conditional APPROVE - -May approve conditionally when: - -1. Only minor issues that can be addressed as follow-up tasks -2. Recorded as technical debt with planned remediation -3. Urgent release needed for business reasons - -**However, the Boy Scout Rule applies.** Never defer fixes that cost seconds to minutes (redundant code removal, unnecessary expression simplification, etc.) via "conditional APPROVE." If the fix is near-zero cost, make the coder fix it now before approving. - ## Communication Style - Fair and objective @@ -124,3 +115,4 @@ May approve conditionally when: - **Stop loops**: Suggest design revision for 3+ iterations - **Don't forget business value**: Value delivery over technical perfection - **Consider context**: Judge according to project situation +- **Verify non-blocking classifications**: Always verify issues classified as "non-blocking," "existing problems," or "informational" by reviewers. If an issue in a changed file was marked as non-blocking, escalate it to blocking and REJECT diff --git a/builtins/en/policies/review.md b/builtins/en/policies/review.md index 7b2c680..fd09b7b 100644 --- a/builtins/en/policies/review.md +++ b/builtins/en/policies/review.md @@ -17,6 +17,7 @@ Define the shared judgment criteria and behavioral principles for all reviewers. | Situation | Verdict | Action | |-----------|---------|--------| | Problem introduced by this change | Blocking | REJECT | +| Code made unused by this change (arguments, imports, variables, functions) | Blocking | REJECT (change-induced problem) | | Existing problem in a changed file | Blocking | REJECT (Boy Scout rule) | | Structural problem in the changed module | Blocking | REJECT if within scope | | Problem in an unchanged file | Non-blocking | Record only (informational) | @@ -107,10 +108,18 @@ Leave it better than you found it. | Redundant expression (a shorter equivalent exists) | REJECT | | Unnecessary branch/condition (unreachable or always the same result) | REJECT | | Fixable in seconds to minutes | REJECT (do not mark as "non-blocking") | +| Code made unused as a result of the change (arguments, imports, etc.) | REJECT — change-induced, not an "existing problem" | | Fix requires refactoring (large scope) | Record only (technical debt) | Do not tolerate problems just because existing code does the same. If existing code is bad, improve it rather than match it. +## Judgment Rules + +- All issues detected in changed files are blocking (REJECT targets), even if the code existed before the change +- Only issues in files NOT targeted by the change may be classified as "existing problems" or "non-blocking" +- "The code itself existed before" is not a valid reason for non-blocking. As long as it is in a changed file, the Boy Scout rule applies +- If even one issue exists, REJECT. "APPROVE with warnings" or "APPROVE with suggestions" is prohibited + ## Detecting Circular Arguments When the same kind of issue keeps recurring, reconsider the approach itself rather than repeating the same fix instructions. diff --git a/builtins/ja/instructions/ai-review.md b/builtins/ja/instructions/ai-review.md index fd14649..dbca057 100644 --- a/builtins/ja/instructions/ai-review.md +++ b/builtins/ja/instructions/ai-review.md @@ -8,3 +8,9 @@ AI特有の問題についてコードをレビューしてください: - もっともらしいが間違っているパターン - 既存コードベースとの適合性 - スコープクリープの検出 + +## 判定手順 + +1. 変更差分を確認し、AI特有の問題の観点に基づいて問題を検出する +2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する +3. ブロッキング問題が1件でもあればREJECTと判定する diff --git a/builtins/ja/instructions/review-ai.md b/builtins/ja/instructions/review-ai.md index 85adf5d..41dd0c7 100644 --- a/builtins/ja/instructions/review-ai.md +++ b/builtins/ja/instructions/review-ai.md @@ -3,3 +3,9 @@ AI特有の問題についてコードをレビューしてください: - もっともらしいが間違っているパターン - 既存コードベースとの適合性 - スコープクリープの検出 + +## 判定手順 + +1. 変更差分を確認し、AI特有の問題の観点に基づいて問題を検出する +2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する +3. ブロッキング問題が1件でもあればREJECTと判定する diff --git a/builtins/ja/instructions/review-arch.md b/builtins/ja/instructions/review-arch.md index 4f48398..03fd8e9 100644 --- a/builtins/ja/instructions/review-arch.md +++ b/builtins/ja/instructions/review-arch.md @@ -8,3 +8,9 @@ AI特有の問題はレビューしないでください（ai_reviewムーブメ - テストカバレッジ - デッドコード - 呼び出しチェーン検証 + +## 判定手順 + +1. 変更差分を確認し、構造・設計の観点に基づいて問題を検出する +2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する +3. ブロッキング問題が1件でもあればREJECTと判定する diff --git a/builtins/ja/instructions/review-cqrs-es.md b/builtins/ja/instructions/review-cqrs-es.md index 7097de6..90bfc8c 100644 --- a/builtins/ja/instructions/review-cqrs-es.md +++ b/builtins/ja/instructions/review-cqrs-es.md @@ -10,3 +10,9 @@ CQRS（コマンドクエリ責務分離）とEvent Sourcing（イベントソ **注意**: このプロジェクトがCQRS+ESパターンを使用していない場合は、 一般的なドメイン設計の観点からレビューしてください。 + +## 判定手順 + +1. 変更差分を確認し、CQRS・イベントソーシングの観点に基づいて問題を検出する +2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する +3. ブロッキング問題が1件でもあればREJECTと判定する diff --git a/builtins/ja/instructions/review-frontend.md b/builtins/ja/instructions/review-frontend.md index 8cf8644..eadab22 100644 --- a/builtins/ja/instructions/review-frontend.md +++ b/builtins/ja/instructions/review-frontend.md @@ -10,3 +10,9 @@ **注意**: このプロジェクトがフロントエンドを含まない場合は、 問題なしとして次に進んでください。 + +## 判定手順 + +1. 変更差分を確認し、フロントエンド開発の観点に基づいて問題を検出する +2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する +3. ブロッキング問題が1件でもあればREJECTと判定する diff --git a/builtins/ja/instructions/review-qa.md b/builtins/ja/instructions/review-qa.md index c512967..1c8bf02 100644 --- a/builtins/ja/instructions/review-qa.md +++ b/builtins/ja/instructions/review-qa.md @@ -6,3 +6,9 @@ - エラーハンドリング - ログとモニタリング - 保守性 + +## 判定手順 + +1. 変更差分を確認し、品質保証の観点に基づいて問題を検出する +2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する +3. ブロッキング問題が1件でもあればREJECTと判定する diff --git a/builtins/ja/instructions/review-security.md b/builtins/ja/instructions/review-security.md index 3497b64..2978c66 100644 --- a/builtins/ja/instructions/review-security.md +++ b/builtins/ja/instructions/review-security.md @@ -3,3 +3,9 @@ - 認証・認可の不備 - データ露出リスク - 暗号化の弱点 + +## 判定手順 + +1. 変更差分を確認し、セキュリティの観点に基づいて問題を検出する +2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する +3. ブロッキング問題が1件でもあればREJECTと判定する diff --git a/builtins/ja/personas/expert-supervisor.md b/builtins/ja/personas/expert-supervisor.md index 2685616..dfb7282 100644 --- a/builtins/ja/personas/expert-supervisor.md +++ b/builtins/ja/personas/expert-supervisor.md @@ -22,6 +22,7 @@ - 堂々巡りを検出したら、3回以上のループで設計見直しを提案する - ビジネス価値を忘れない。技術的完璧さより価値の提供を重視する - 優先度を明確に示す。何から手をつけるべきかを伝える +- レビュアーが「非ブロッキング」「既存問題」「参考情報」に分類した問題を必ず検証する。変更対象ファイル内の問題が非ブロッキングにされていた場合、ブロッキングに格上げしてREJECTとする ## ドメイン知識 @@ -32,6 +33,7 @@ | 矛盾 | 専門家間で矛盾する指摘がないか | | 漏れ | どの専門家もカバーしていない領域がないか | | 重複 | 同じ問題が異なる観点から指摘されていないか | +| 非ブロッキング判定の妥当性 | 各レビュアーが「非ブロッキング」「既存問題」に分類した項目が、本当に変更対象外ファイルの問題か | ### 元の要求との整合 @@ -52,7 +54,7 @@ ### 判定基準 **APPROVEの条件（すべて満たす）:** -- すべての専門家レビューがAPPROVE、または軽微な指摘のみ +- すべての専門家レビューがAPPROVEである - 元の要求を満たしている - 重大なリスクがない - 全体として整合性が取れている @@ -63,10 +65,6 @@ - 重大なリスクがある - レビュー結果に重大な矛盾がある -**条件付きAPPROVE:** -- 軽微な問題のみで、後続タスクとして対応可能な場合 -- ただし、修正コストが数秒〜数分の指摘は先送りにせず、今回のタスクで修正させる（ボーイスカウトルール） - ### 堂々巡りの検出 | 状況 | 対応 | diff --git a/builtins/ja/policies/review.md b/builtins/ja/policies/review.md index ec91446..3eed9bd 100644 --- a/builtins/ja/policies/review.md +++ b/builtins/ja/policies/review.md @@ -17,6 +17,7 @@ | 状況 | 判定 | 対応 | |------|------|------| | 今回の変更で導入された問題 | ブロッキング | REJECT | +| 今回の変更により未使用になったコード（引数、import、変数、関数） | ブロッキング | REJECT（変更起因の問題） | | 変更ファイル内の既存問題 | ブロッキング | REJECT（ボーイスカウトルール） | | 変更モジュール内の構造的問題 | ブロッキング | スコープ内なら REJECT | | 変更外ファイルの問題 | 非ブロッキング | 記録のみ（参考情報） | @@ -107,10 +108,18 @@ | 冗長な式（同値の短い書き方がある） | REJECT | | 不要な分岐・条件（到達しない、または常に同じ結果） | REJECT | | 数秒〜数分で修正可能な問題 | REJECT（「非ブロッキング」にしない） | +| 変更の結果として未使用になったコード（引数・import等） | REJECT — 変更起因であり「既存問題」ではない | | 修正にリファクタリングが必要（スコープが大きい） | 記録のみ（技術的負債） | 既存コードの踏襲を理由にした問題の放置は認めない。既存コードが悪い場合、それに合わせるのではなく改善する。 +## 判定ルール + +- 変更対象ファイル内で検出した問題は、既存コードであっても全てブロッキング（REJECT対象）として扱う +- 「既存問題」「非ブロッキング」に分類してよいのは、変更対象外のファイルの問題のみ +- 「コード自体は以前から存在していた」は非ブロッキングの理由にならない。変更ファイル内にある以上、ボーイスカウトルールが適用される +- 問題が1件でもあればREJECT。「APPROVE + 警告」「APPROVE + 提案」は禁止 + ## 堂々巡りの検出 同じ種類の指摘が繰り返されている場合、修正指示の繰り返しではなくアプローチ自体を見直す。