refactor(code-review): defer finding IDs until consolidation
This commit is contained in:
parent
b8eeb78cff
commit
5fcdae02b5
|
|
@ -43,15 +43,14 @@ Review `{git_discrepancies}` and create findings:
|
|||
|
||||
| Discrepancy Type | Severity |
|
||||
| --- | --- |
|
||||
| Files changed but not in story File List | MEDIUM |
|
||||
| Story lists files but no git changes | HIGH |
|
||||
| Uncommitted changes not documented | MEDIUM |
|
||||
| Files changed but not in story File List | Medium |
|
||||
| Story lists files but no git changes | High |
|
||||
| Uncommitted changes not documented | Medium |
|
||||
|
||||
For each discrepancy, add to `{context_aware_findings}`:
|
||||
For each discrepancy, add to `{context_aware_findings}` (no IDs yet - assigned after merge):
|
||||
|
||||
```
|
||||
{
|
||||
id: "CAF-{n}",
|
||||
source: "git-discrepancy",
|
||||
severity: "...",
|
||||
description: "...",
|
||||
|
|
@ -66,15 +65,14 @@ For EACH AC in `{acceptance_criteria}`:
|
|||
1. Read the AC requirement
|
||||
2. Search implementation files in `{comprehensive_file_list}` for evidence
|
||||
3. Determine status: IMPLEMENTED, PARTIAL, or MISSING
|
||||
4. If PARTIAL or MISSING → add HIGH severity finding
|
||||
4. If PARTIAL or MISSING → add High severity finding
|
||||
|
||||
Add to `{context_aware_findings}`:
|
||||
|
||||
```
|
||||
{
|
||||
id: "CAF-{n}",
|
||||
source: "ac-validation",
|
||||
severity: "HIGH",
|
||||
severity: "High",
|
||||
description: "AC {id} not fully implemented: {details}",
|
||||
evidence: "Expected: {ac}, Found: {what_was_found}"
|
||||
}
|
||||
|
|
@ -86,16 +84,15 @@ For EACH task marked [x] in `{tasks_with_status}`:
|
|||
|
||||
1. Read the task description
|
||||
2. Search files for evidence it was actually done
|
||||
3. **CRITICAL**: If marked [x] but NOT DONE → CRITICAL finding
|
||||
3. **Critical**: If marked [x] but NOT DONE → Critical finding
|
||||
4. Record specific proof (file:line) if done
|
||||
|
||||
Add to `{context_aware_findings}` if false:
|
||||
|
||||
```
|
||||
{
|
||||
id: "CAF-{n}",
|
||||
source: "task-audit",
|
||||
severity: "CRITICAL",
|
||||
severity: "Critical",
|
||||
description: "Task marked complete but not implemented: {task}",
|
||||
evidence: "Searched: {files}, Found: no evidence of {expected}"
|
||||
}
|
||||
|
|
@ -137,10 +134,10 @@ Present context-aware findings:
|
|||
**Phase 1: Context-Aware Review Complete**
|
||||
|
||||
**Findings:** {count}
|
||||
- CRITICAL: {count}
|
||||
- HIGH: {count}
|
||||
- MEDIUM: {count}
|
||||
- LOW: {count}
|
||||
- Critical: {count}
|
||||
- High: {count}
|
||||
- Medium: {count}
|
||||
- Low: {count}
|
||||
|
||||
Proceeding to Phase 2: Adversarial Review...
|
||||
```
|
||||
|
|
|
|||
|
|
@ -93,12 +93,11 @@ Capture findings from adversarial review.
|
|||
|
||||
Evaluate severity (Critical, High, Medium, Low) and validity (Real, Noise, Undecided).
|
||||
|
||||
Create `{asymmetric_findings}` list:
|
||||
Add each finding to `{asymmetric_findings}` (no IDs yet - assigned after merge):
|
||||
|
||||
```
|
||||
{
|
||||
id: "AAF-{n}",
|
||||
source: "adversarial-review",
|
||||
source: "adversarial",
|
||||
severity: "...",
|
||||
validity: "...",
|
||||
description: "...",
|
||||
|
|
@ -115,15 +114,15 @@ Present adversarial findings:
|
|||
|
||||
**Reviewer Context:** Pure diff review (no story knowledge)
|
||||
**Findings:** {count}
|
||||
- CRITICAL: {count}
|
||||
- HIGH: {count}
|
||||
- MEDIUM: {count}
|
||||
- LOW: {count}
|
||||
- Critical: {count}
|
||||
- High: {count}
|
||||
- Medium: {count}
|
||||
- Low: {count}
|
||||
|
||||
**Validity Assessment:**
|
||||
- Real issues: {count}
|
||||
- Noise/false positives: {count}
|
||||
- Needs judgment: {count}
|
||||
- Real: {count}
|
||||
- Noise: {count}
|
||||
- Undecided: {count}
|
||||
|
||||
Proceeding to findings consolidation...
|
||||
```
|
||||
|
|
|
|||
|
|
@ -57,53 +57,39 @@ Keep the MORE DETAILED version:
|
|||
- If adversarial finding has better technical detail → keep that
|
||||
- When in doubt, keep context-aware (has more context)
|
||||
|
||||
Mark duplicates as merged:
|
||||
|
||||
```
|
||||
{
|
||||
id: "CF-{n}",
|
||||
merged_from: ["CAF-3", "AAF-2"],
|
||||
kept_version: "CAF-3",
|
||||
reason: "Context-aware version includes AC reference"
|
||||
}
|
||||
```
|
||||
Note which findings were merged (for transparency in the summary).
|
||||
|
||||
### 3. Normalize Severity
|
||||
|
||||
Apply consistent severity scale:
|
||||
|
||||
| Severity | Icon | Criteria |
|
||||
| --- | --- | --- |
|
||||
| CRITICAL | RED | Security vuln, data loss, tasks marked done but not, broken core functionality |
|
||||
| HIGH | ORANGE | Missing AC implementation, logic errors, missing critical error handling |
|
||||
| MEDIUM | YELLOW | Performance issues, incomplete features, documentation gaps |
|
||||
| LOW | GREEN | Code style, minor improvements, suggestions |
|
||||
Apply consistent severity scale (Critical, High, Medium, Low).
|
||||
|
||||
### 4. Filter Noise
|
||||
|
||||
Review adversarial findings marked as NOISE:
|
||||
Review adversarial findings marked as Noise:
|
||||
|
||||
- If clearly false positive (e.g., style preference, not actual issue) → exclude
|
||||
- If questionable → keep with UNDECIDED validity
|
||||
- If context reveals it's actually valid → upgrade to REAL
|
||||
- If questionable → keep with Undecided validity
|
||||
- If context reveals it's actually valid → upgrade to Real
|
||||
|
||||
**Do NOT filter:**
|
||||
|
||||
- Any CRITICAL or HIGH severity
|
||||
- Any Critical or High severity
|
||||
- Any context-aware findings (they have story context)
|
||||
|
||||
### 5. Create Consolidated Table
|
||||
### 5. Sort and Number Findings
|
||||
|
||||
Sort by severity (Critical → High → Medium → Low), then assign IDs: F1, F2, F3, etc.
|
||||
|
||||
Build `{consolidated_findings}`:
|
||||
|
||||
```markdown
|
||||
| ID | Severity | Source | Description | Location |
|
||||
|----|----------|--------|-------------|----------|
|
||||
| CF-1 | CRITICAL | task-audit | Task 3 marked [x] but not implemented | src/auth.ts |
|
||||
| CF-2 | HIGH | ac-validation | AC2 partially implemented | src/api/*.ts |
|
||||
| CF-3 | HIGH | adversarial | Missing error handling in API calls | src/api/client.ts:45 |
|
||||
| CF-4 | MEDIUM | git-discrepancy | File changed but not in story | src/utils.ts |
|
||||
| CF-5 | LOW | adversarial | Magic number should be constant | src/config.ts:12 |
|
||||
| F1 | Critical | task-audit | Task 3 marked [x] but not implemented | src/auth.ts |
|
||||
| F2 | High | ac-validation | AC2 partially implemented | src/api/*.ts |
|
||||
| F3 | High | adversarial | Missing error handling in API calls | src/api/client.ts:45 |
|
||||
| F4 | Medium | git-discrepancy | File changed but not in story | src/utils.ts |
|
||||
| F5 | Low | adversarial | Magic number should be constant | src/config.ts:12 |
|
||||
```
|
||||
|
||||
### 6. Present Consolidated Findings
|
||||
|
|
@ -115,10 +101,10 @@ Build `{consolidated_findings}`:
|
|||
|
||||
**Summary:**
|
||||
- Total findings: {count}
|
||||
- CRITICAL: {count}
|
||||
- HIGH: {count}
|
||||
- MEDIUM: {count}
|
||||
- LOW: {count}
|
||||
- Critical: {count}
|
||||
- High: {count}
|
||||
- Medium: {count}
|
||||
- Low: {count}
|
||||
|
||||
**Deduplication:** {merged_count} duplicate findings merged
|
||||
|
||||
|
|
@ -126,16 +112,16 @@ Build `{consolidated_findings}`:
|
|||
|
||||
## Findings by Severity
|
||||
|
||||
### CRITICAL (Must Fix)
|
||||
### Critical (Must Fix)
|
||||
{list critical findings with full details}
|
||||
|
||||
### HIGH (Should Fix)
|
||||
### High (Should Fix)
|
||||
{list high findings with full details}
|
||||
|
||||
### MEDIUM (Consider Fixing)
|
||||
### Medium (Consider Fixing)
|
||||
{list medium findings}
|
||||
|
||||
### LOW (Nice to Fix)
|
||||
### Low (Nice to Fix)
|
||||
{list low findings}
|
||||
|
||||
---
|
||||
|
|
|
|||
Loading…
Reference in New Issue