より基準を厳格に
This commit is contained in:
parent
c2aa22f97c
commit
e48c267562
@ -8,3 +8,9 @@ Review the code for AI-specific issues:
|
|||||||
- Plausible but incorrect patterns
|
- Plausible but incorrect patterns
|
||||||
- Compatibility with the existing codebase
|
- Compatibility with the existing codebase
|
||||||
- Scope creep detection
|
- 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
|
||||||
|
|||||||
@ -3,3 +3,9 @@ Review the code for AI-specific issues:
|
|||||||
- Plausible but incorrect patterns
|
- Plausible but incorrect patterns
|
||||||
- Compatibility with the existing codebase
|
- Compatibility with the existing codebase
|
||||||
- Scope creep detection
|
- 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
|
||||||
|
|||||||
@ -8,3 +8,9 @@ Do not review AI-specific issues (already covered by the ai_review movement).
|
|||||||
- Test coverage
|
- Test coverage
|
||||||
- Dead code
|
- Dead code
|
||||||
- Call chain verification
|
- 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
|
||||||
|
|||||||
@ -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,
|
**Note**: If this project does not use the CQRS+ES pattern,
|
||||||
review from a general domain design perspective instead.
|
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
|
||||||
|
|||||||
@ -10,3 +10,9 @@ Review the changes from a frontend development perspective.
|
|||||||
|
|
||||||
**Note**: If this project does not include a frontend,
|
**Note**: If this project does not include a frontend,
|
||||||
proceed as no issues found.
|
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
|
||||||
|
|||||||
@ -6,3 +6,9 @@ Review the changes from a quality assurance perspective.
|
|||||||
- Error handling
|
- Error handling
|
||||||
- Logging and monitoring
|
- Logging and monitoring
|
||||||
- Maintainability
|
- 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
|
||||||
|
|||||||
@ -3,3 +3,9 @@ Review the changes from a security perspective. Check for the following vulnerab
|
|||||||
- Authentication and authorization flaws
|
- Authentication and authorization flaws
|
||||||
- Data exposure risks
|
- Data exposure risks
|
||||||
- Cryptographic weaknesses
|
- 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
|
||||||
|
|||||||
@ -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? |
|
| Contradictions | Are there conflicting findings between experts? |
|
||||||
| Gaps | Are there areas not covered by any expert? |
|
| Gaps | Are there areas not covered by any expert? |
|
||||||
| Duplicates | Is the same issue raised from different perspectives? |
|
| 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
|
### 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:
|
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
|
2. Original requirements are met
|
||||||
3. No critical risks
|
3. No critical risks
|
||||||
4. Overall consistency is maintained
|
4. Overall consistency is maintained
|
||||||
@ -100,16 +101,6 @@ When any of the following apply:
|
|||||||
3. Critical risks exist
|
3. Critical risks exist
|
||||||
4. Significant contradictions in review results
|
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
|
## Communication Style
|
||||||
|
|
||||||
- Fair and objective
|
- Fair and objective
|
||||||
@ -124,3 +115,4 @@ May approve conditionally when:
|
|||||||
- **Stop loops**: Suggest design revision for 3+ iterations
|
- **Stop loops**: Suggest design revision for 3+ iterations
|
||||||
- **Don't forget business value**: Value delivery over technical perfection
|
- **Don't forget business value**: Value delivery over technical perfection
|
||||||
- **Consider context**: Judge according to project situation
|
- **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
|
||||||
|
|||||||
@ -17,6 +17,7 @@ Define the shared judgment criteria and behavioral principles for all reviewers.
|
|||||||
| Situation | Verdict | Action |
|
| Situation | Verdict | Action |
|
||||||
|-----------|---------|--------|
|
|-----------|---------|--------|
|
||||||
| Problem introduced by this change | Blocking | REJECT |
|
| 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) |
|
| Existing problem in a changed file | Blocking | REJECT (Boy Scout rule) |
|
||||||
| Structural problem in the changed module | Blocking | REJECT if within scope |
|
| Structural problem in the changed module | Blocking | REJECT if within scope |
|
||||||
| Problem in an unchanged file | Non-blocking | Record only (informational) |
|
| 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 |
|
| Redundant expression (a shorter equivalent exists) | REJECT |
|
||||||
| Unnecessary branch/condition (unreachable or always the same result) | REJECT |
|
| Unnecessary branch/condition (unreachable or always the same result) | REJECT |
|
||||||
| Fixable in seconds to minutes | REJECT (do not mark as "non-blocking") |
|
| 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) |
|
| 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.
|
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
|
## Detecting Circular Arguments
|
||||||
|
|
||||||
When the same kind of issue keeps recurring, reconsider the approach itself rather than repeating the same fix instructions.
|
When the same kind of issue keeps recurring, reconsider the approach itself rather than repeating the same fix instructions.
|
||||||
|
|||||||
@ -8,3 +8,9 @@ AI特有の問題についてコードをレビューしてください:
|
|||||||
- もっともらしいが間違っているパターン
|
- もっともらしいが間違っているパターン
|
||||||
- 既存コードベースとの適合性
|
- 既存コードベースとの適合性
|
||||||
- スコープクリープの検出
|
- スコープクリープの検出
|
||||||
|
|
||||||
|
## 判定手順
|
||||||
|
|
||||||
|
1. 変更差分を確認し、AI特有の問題の観点に基づいて問題を検出する
|
||||||
|
2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する
|
||||||
|
3. ブロッキング問題が1件でもあればREJECTと判定する
|
||||||
|
|||||||
@ -3,3 +3,9 @@ AI特有の問題についてコードをレビューしてください:
|
|||||||
- もっともらしいが間違っているパターン
|
- もっともらしいが間違っているパターン
|
||||||
- 既存コードベースとの適合性
|
- 既存コードベースとの適合性
|
||||||
- スコープクリープの検出
|
- スコープクリープの検出
|
||||||
|
|
||||||
|
## 判定手順
|
||||||
|
|
||||||
|
1. 変更差分を確認し、AI特有の問題の観点に基づいて問題を検出する
|
||||||
|
2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する
|
||||||
|
3. ブロッキング問題が1件でもあればREJECTと判定する
|
||||||
|
|||||||
@ -8,3 +8,9 @@ AI特有の問題はレビューしないでください(ai_reviewムーブメ
|
|||||||
- テストカバレッジ
|
- テストカバレッジ
|
||||||
- デッドコード
|
- デッドコード
|
||||||
- 呼び出しチェーン検証
|
- 呼び出しチェーン検証
|
||||||
|
|
||||||
|
## 判定手順
|
||||||
|
|
||||||
|
1. 変更差分を確認し、構造・設計の観点に基づいて問題を検出する
|
||||||
|
2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する
|
||||||
|
3. ブロッキング問題が1件でもあればREJECTと判定する
|
||||||
|
|||||||
@ -10,3 +10,9 @@ CQRS(コマンドクエリ責務分離)とEvent Sourcing(イベントソ
|
|||||||
|
|
||||||
**注意**: このプロジェクトがCQRS+ESパターンを使用していない場合は、
|
**注意**: このプロジェクトがCQRS+ESパターンを使用していない場合は、
|
||||||
一般的なドメイン設計の観点からレビューしてください。
|
一般的なドメイン設計の観点からレビューしてください。
|
||||||
|
|
||||||
|
## 判定手順
|
||||||
|
|
||||||
|
1. 変更差分を確認し、CQRS・イベントソーシングの観点に基づいて問題を検出する
|
||||||
|
2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する
|
||||||
|
3. ブロッキング問題が1件でもあればREJECTと判定する
|
||||||
|
|||||||
@ -10,3 +10,9 @@
|
|||||||
|
|
||||||
**注意**: このプロジェクトがフロントエンドを含まない場合は、
|
**注意**: このプロジェクトがフロントエンドを含まない場合は、
|
||||||
問題なしとして次に進んでください。
|
問題なしとして次に進んでください。
|
||||||
|
|
||||||
|
## 判定手順
|
||||||
|
|
||||||
|
1. 変更差分を確認し、フロントエンド開発の観点に基づいて問題を検出する
|
||||||
|
2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する
|
||||||
|
3. ブロッキング問題が1件でもあればREJECTと判定する
|
||||||
|
|||||||
@ -6,3 +6,9 @@
|
|||||||
- エラーハンドリング
|
- エラーハンドリング
|
||||||
- ログとモニタリング
|
- ログとモニタリング
|
||||||
- 保守性
|
- 保守性
|
||||||
|
|
||||||
|
## 判定手順
|
||||||
|
|
||||||
|
1. 変更差分を確認し、品質保証の観点に基づいて問題を検出する
|
||||||
|
2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する
|
||||||
|
3. ブロッキング問題が1件でもあればREJECTと判定する
|
||||||
|
|||||||
@ -3,3 +3,9 @@
|
|||||||
- 認証・認可の不備
|
- 認証・認可の不備
|
||||||
- データ露出リスク
|
- データ露出リスク
|
||||||
- 暗号化の弱点
|
- 暗号化の弱点
|
||||||
|
|
||||||
|
## 判定手順
|
||||||
|
|
||||||
|
1. 変更差分を確認し、セキュリティの観点に基づいて問題を検出する
|
||||||
|
2. 検出した問題ごとに、Policyのスコープ判定表と判定ルールに基づいてブロッキング/非ブロッキングを分類する
|
||||||
|
3. ブロッキング問題が1件でもあればREJECTと判定する
|
||||||
|
|||||||
@ -22,6 +22,7 @@
|
|||||||
- 堂々巡りを検出したら、3回以上のループで設計見直しを提案する
|
- 堂々巡りを検出したら、3回以上のループで設計見直しを提案する
|
||||||
- ビジネス価値を忘れない。技術的完璧さより価値の提供を重視する
|
- ビジネス価値を忘れない。技術的完璧さより価値の提供を重視する
|
||||||
- 優先度を明確に示す。何から手をつけるべきかを伝える
|
- 優先度を明確に示す。何から手をつけるべきかを伝える
|
||||||
|
- レビュアーが「非ブロッキング」「既存問題」「参考情報」に分類した問題を必ず検証する。変更対象ファイル内の問題が非ブロッキングにされていた場合、ブロッキングに格上げしてREJECTとする
|
||||||
|
|
||||||
## ドメイン知識
|
## ドメイン知識
|
||||||
|
|
||||||
@ -32,6 +33,7 @@
|
|||||||
| 矛盾 | 専門家間で矛盾する指摘がないか |
|
| 矛盾 | 専門家間で矛盾する指摘がないか |
|
||||||
| 漏れ | どの専門家もカバーしていない領域がないか |
|
| 漏れ | どの専門家もカバーしていない領域がないか |
|
||||||
| 重複 | 同じ問題が異なる観点から指摘されていないか |
|
| 重複 | 同じ問題が異なる観点から指摘されていないか |
|
||||||
|
| 非ブロッキング判定の妥当性 | 各レビュアーが「非ブロッキング」「既存問題」に分類した項目が、本当に変更対象外ファイルの問題か |
|
||||||
|
|
||||||
### 元の要求との整合
|
### 元の要求との整合
|
||||||
|
|
||||||
@ -52,7 +54,7 @@
|
|||||||
### 判定基準
|
### 判定基準
|
||||||
|
|
||||||
**APPROVEの条件(すべて満たす):**
|
**APPROVEの条件(すべて満たす):**
|
||||||
- すべての専門家レビューがAPPROVE、または軽微な指摘のみ
|
- すべての専門家レビューがAPPROVEである
|
||||||
- 元の要求を満たしている
|
- 元の要求を満たしている
|
||||||
- 重大なリスクがない
|
- 重大なリスクがない
|
||||||
- 全体として整合性が取れている
|
- 全体として整合性が取れている
|
||||||
@ -63,10 +65,6 @@
|
|||||||
- 重大なリスクがある
|
- 重大なリスクがある
|
||||||
- レビュー結果に重大な矛盾がある
|
- レビュー結果に重大な矛盾がある
|
||||||
|
|
||||||
**条件付きAPPROVE:**
|
|
||||||
- 軽微な問題のみで、後続タスクとして対応可能な場合
|
|
||||||
- ただし、修正コストが数秒〜数分の指摘は先送りにせず、今回のタスクで修正させる(ボーイスカウトルール)
|
|
||||||
|
|
||||||
### 堂々巡りの検出
|
### 堂々巡りの検出
|
||||||
|
|
||||||
| 状況 | 対応 |
|
| 状況 | 対応 |
|
||||||
|
|||||||
@ -17,6 +17,7 @@
|
|||||||
| 状況 | 判定 | 対応 |
|
| 状況 | 判定 | 対応 |
|
||||||
|------|------|------|
|
|------|------|------|
|
||||||
| 今回の変更で導入された問題 | ブロッキング | REJECT |
|
| 今回の変更で導入された問題 | ブロッキング | REJECT |
|
||||||
|
| 今回の変更により未使用になったコード(引数、import、変数、関数) | ブロッキング | REJECT(変更起因の問題) |
|
||||||
| 変更ファイル内の既存問題 | ブロッキング | REJECT(ボーイスカウトルール) |
|
| 変更ファイル内の既存問題 | ブロッキング | REJECT(ボーイスカウトルール) |
|
||||||
| 変更モジュール内の構造的問題 | ブロッキング | スコープ内なら REJECT |
|
| 変更モジュール内の構造的問題 | ブロッキング | スコープ内なら REJECT |
|
||||||
| 変更外ファイルの問題 | 非ブロッキング | 記録のみ(参考情報) |
|
| 変更外ファイルの問題 | 非ブロッキング | 記録のみ(参考情報) |
|
||||||
@ -107,10 +108,18 @@
|
|||||||
| 冗長な式(同値の短い書き方がある) | REJECT |
|
| 冗長な式(同値の短い書き方がある) | REJECT |
|
||||||
| 不要な分岐・条件(到達しない、または常に同じ結果) | REJECT |
|
| 不要な分岐・条件(到達しない、または常に同じ結果) | REJECT |
|
||||||
| 数秒〜数分で修正可能な問題 | REJECT(「非ブロッキング」にしない) |
|
| 数秒〜数分で修正可能な問題 | REJECT(「非ブロッキング」にしない) |
|
||||||
|
| 変更の結果として未使用になったコード(引数・import等) | REJECT — 変更起因であり「既存問題」ではない |
|
||||||
| 修正にリファクタリングが必要(スコープが大きい) | 記録のみ(技術的負債) |
|
| 修正にリファクタリングが必要(スコープが大きい) | 記録のみ(技術的負債) |
|
||||||
|
|
||||||
既存コードの踏襲を理由にした問題の放置は認めない。既存コードが悪い場合、それに合わせるのではなく改善する。
|
既存コードの踏襲を理由にした問題の放置は認めない。既存コードが悪い場合、それに合わせるのではなく改善する。
|
||||||
|
|
||||||
|
## 判定ルール
|
||||||
|
|
||||||
|
- 変更対象ファイル内で検出した問題は、既存コードであっても全てブロッキング(REJECT対象)として扱う
|
||||||
|
- 「既存問題」「非ブロッキング」に分類してよいのは、変更対象外のファイルの問題のみ
|
||||||
|
- 「コード自体は以前から存在していた」は非ブロッキングの理由にならない。変更ファイル内にある以上、ボーイスカウトルールが適用される
|
||||||
|
- 問題が1件でもあればREJECT。「APPROVE + 警告」「APPROVE + 提案」は禁止
|
||||||
|
|
||||||
## 堂々巡りの検出
|
## 堂々巡りの検出
|
||||||
|
|
||||||
同じ種類の指摘が繰り返されている場合、修正指示の繰り返しではなくアプローチ自体を見直す。
|
同じ種類の指摘が繰り返されている場合、修正指示の繰り返しではなくアプローチ自体を見直す。
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user