feat(story-6.2): implement code reviewer deep audit with independent get_impact and find_dead_code queries
The code-review skills (bmad-code-review, gds-code-review) now independently query Memtrace during reviews. Step 1 runs get_impact on modified symbols and find_dead_code on modified files via memtrace-adapter.mjs with --check-freshness --summarize. Step 2 Acceptance Auditor cross-references structural data against the diff, raising decision_needed findings for unhandled downstream dependencies and patch findings for dead code and high blast radius. FR12: Code Review Agent queries get_impact on PRs for downstream dependency audit FR13: Code Review Agent executes find_dead_code to block orphaned code 9 files modified across source templates and installed copies.
This commit is contained in:
Magal
parent
5d5ecb60d5
commit
afde2efcf6
|
|
@ -32,6 +32,7 @@ activation_steps_append = []
|
|||
|
||||
persistent_facts = [
|
||||
"file:{project-root}/**/project-context.md",
|
||||
"Memtrace structural deep audit is available for independent code review verification. Use the adapter at _bmad/scripts/memtrace/memtrace-adapter.mjs with --query get_impact --target <symbol> --check-freshness --summarize to independently verify the blast radius of modified symbols. Use --query find_dead_code --target <file> --check-freshness for dead code detection in modified modules. Run these during Step 1 (gather context) after constructing the diff. All queries MUST use sequential for...of with await — NEVER Promise.all. Index freshness check via list_indexed_repositories is mandatory before trusting graph output. Structural audit is advisory/supplemental — NEVER block the review on Memtrace availability. Timeout (MEMTRACE_MCP_ERROR_TIMEOUT) or stale index ([FRESHNESS] in STDERR) means skip and continue.",
|
||||
]
|
||||
|
||||
# Scalar: executed when the workflow reaches its final step,
|
||||
|
|
|
|||
|
|
@ -3,6 +3,8 @@ diff_output: '' # set at runtime
|
|||
spec_file: '' # set at runtime (path or empty)
|
||||
review_mode: '' # set at runtime: "full" or "no-spec"
|
||||
story_key: '' # set at runtime when discovered from sprint status
|
||||
memtrace_blast_radius: '' # set at runtime: structured blast radius data or "unavailable"/"partial"
|
||||
memtrace_dead_code: '' # set at runtime: structured dead code data or "unavailable"/"partial"
|
||||
---
|
||||
|
||||
# Step 1: Gather Context
|
||||
|
|
@ -75,9 +77,45 @@ story_key: '' # set at runtime when discovered from sprint status
|
|||
- If the user opts to chunk: agree on the first group, narrow `{diff_output}` accordingly, and list the remaining groups for the user to note for follow-up runs.
|
||||
- If the user declines: proceed as-is with the full diff.
|
||||
|
||||
7. **Run structural deep audit queries (Memtrace).** If the repository is indexed by Memtrace, independently verify the diff's structural impact. This step is DIAGNOSTIC — the review continues regardless of availability.
|
||||
|
||||
**Check Availability:**
|
||||
- Use `list_indexed_repositories` to confirm the project repo is indexed
|
||||
- Check the `last_indexed_at` value — if older than 30 minutes, flag as stale and skip graph queries
|
||||
- If not indexed or stale, set `{memtrace_blast_radius}` = `"unavailable"` and `{memtrace_dead_code}` = `"unavailable"`, then skip to CHECKPOINT
|
||||
|
||||
**Extract modified symbols from the diff:**
|
||||
- Parse `{diff_output}` to identify modified functions, methods, classes, and exported variables
|
||||
- Extract symbol names from changed lines (lines starting with `+` or `-` in function/class/method declarations)
|
||||
- De-duplicate and limit to at most 15 symbols (prioritize: exported > internal, functions > variables)
|
||||
- For each symbol, note its containing file path
|
||||
- If no modified symbols are extracted (e.g., only config files, comments, or whitespace changes), skip both blast radius and dead code queries — set `{memtrace_blast_radius}` = `"empty"` and `{memtrace_dead_code}` = `"empty"`
|
||||
|
||||
**Run blast radius audit (`get_impact`):**
|
||||
- For each extracted symbol, call the adapter:
|
||||
`node _bmad/scripts/memtrace/memtrace-adapter.mjs --target <symbol> --query get_impact --check-freshness --summarize`
|
||||
- Process STRICTLY SEQUENTIALLY using `for...of` with `await` — NEVER `Promise.all`
|
||||
- On exit 0: collect `summarized.critical_dependents`, `summarized.module_impact`, `summarized.total_affected`
|
||||
- On exit 1 with `[FRESHNESS]` in STDERR: note "Index stale — skipping blast radius for <symbol>" and continue
|
||||
- On exit 1 with `MEMTRACE_MCP_ERROR_TIMEOUT`: note "MCP timeout — skipping blast radius for <symbol>" and continue
|
||||
- Set `{memtrace_blast_radius}` to the structured results (or `"partial"` if some queries failed, or `"unavailable"` if ALL queries failed)
|
||||
|
||||
**Run dead code audit (`find_dead_code`):**
|
||||
- For each UNIQUE modified file (not per-symbol), call the adapter:
|
||||
`node _bmad/scripts/memtrace/memtrace-adapter.mjs --target <file-path> --query find_dead_code --check-freshness`
|
||||
- Process STRICTLY SEQUENTIALLY using `for...of` with `await`
|
||||
- On exit 0: collect the list of dead code symbols in that file
|
||||
- On failure: note the failure and continue with remaining files
|
||||
- Set `{memtrace_dead_code}` to the structured results (or `"partial"` if some queries failed, or `"unavailable"` if ALL queries failed)
|
||||
|
||||
**Graceful Degradation:**
|
||||
- If `list_indexed_repositories` returns empty or the project repo is not indexed: skip ALL queries, set both variables to `"unavailable"`
|
||||
- If any individual query times out or fails: skip that query only, mark results as `"partial"`, continue with remaining symbols/files
|
||||
- NEVER block or halt the review on Memtrace availability — the structural audit is supplemental intelligence
|
||||
|
||||
### CHECKPOINT
|
||||
|
||||
Present a summary before proceeding: diff stats (files changed, lines added/removed), `{review_mode}`, and loaded spec/context docs (if any). HALT and wait for user confirmation to proceed.
|
||||
Present a summary before proceeding: diff stats (files changed, lines added/removed), `{review_mode}`, loaded spec/context docs (if any), and structural audit status (if `{memtrace_blast_radius}` is not empty/unavailable: if `"partial"`, note "structural audit partially complete — {partial_count}/{total_count} symbol queries failed"; if fully available, include "blast radius queried for {count} symbols, dead code checked in {count} files"). HALT and wait for user confirmation to proceed.
|
||||
|
||||
|
||||
## NEXT
|
||||
|
|
|
|||
|
|
@ -50,6 +50,32 @@ failed_layers: '' # set at runtime: comma-separated list of layers that failed o
|
|||
> - **If the story involves dead-code removal (find_dead_code, dead-code in tasks) BUT no "Dead Code Pitfall Validation Report" exists in the spec file** → raise a `patch` finding: "Missing Dead Code Pitfall Validation Report — story involved dead-code removal but no pitfall validation was performed via validate-dead-code.mjs."
|
||||
> - **If neither dead-code removal nor a pitfall validation report exists**: skip this gate (story does not involve dead-code).
|
||||
>
|
||||
> **Structural Deep Audit — Memtrace Verification:**
|
||||
>
|
||||
> If `{memtrace_blast_radius}` and `{memtrace_dead_code}` are available (not `"unavailable"` or empty), you have access to independently-computed structural data. Use this to verify the diff's claims against the actual codebase graph.
|
||||
>
|
||||
> **Blast Radius Verification:**
|
||||
> - Cross-reference each modified symbol in the diff against `{memtrace_blast_radius}` results.
|
||||
> - If a symbol has `critical_dependents` (any depth) that are NOT modified in the diff → raise a `decision_needed` finding: **"Unhandled downstream dependency: `<dependent-name>` depends on modified `<symbol>` — diff does not include test or mitigation."**
|
||||
> - Evidence: list the specific dependent name(s) and the blast radius data
|
||||
> - If the blast radius `total_affected` for any symbol exceeds 20 → raise a `patch` finding: **"High blast radius: `<symbol>` affects `<count>` dependents (depth `<N>`) — consider narrower refactor scope or expanded test coverage."**
|
||||
> - If the diff or commit message claims "no downstream impact" but blast radius shows dependents → raise a `decision_needed` finding: **"Downstream impact claim falsified: diff claims no impact but blast radius shows `<count>` affected symbols at depth 1+."**
|
||||
> - If a blast radius query returned `"partial"` for some symbols: note which symbols were not verified and proceed with available data only.
|
||||
>
|
||||
> **Dead Code Audit:**
|
||||
> - Check whether any `{memtrace_dead_code}` findings overlap with lines ADDED in modified files (new code in `+` lines).
|
||||
> - If a modified file introduces a NEW function/method/class that also appears in the dead code results → raise a `patch` finding: **"New dead code introduced: `<symbol>` in `<file>` — added but has zero callers in the codebase graph."**
|
||||
> - If a modified file (`+` or `-` lines) contains EXISTING dead code symbols that were NOT removed → raise a `patch` finding: **"Pre-existing dead code unaddressed: `<symbol>` in `<file>` — appears in dead code results but was not cleaned up in this change."**
|
||||
> - Do NOT flag dead code in files that the diff did not touch.
|
||||
>
|
||||
> **If `{memtrace_blast_radius}` or `{memtrace_dead_code}` is `"unavailable"` or empty:**
|
||||
> - Note in the review output: "Structural deep audit unavailable — Memtrace not indexed or queries failed. Proceeding with text-based review only."
|
||||
> - DO NOT raise any structural-audit-specific findings (no blast radius or dead code flags).
|
||||
>
|
||||
> **If `{memtrace_blast_radius}` or `{memtrace_dead_code}` is `"partial"`:**
|
||||
> - Apply the blast radius rules ONLY if `{memtrace_blast_radius}` has complete data; apply dead code rules ONLY if `{memtrace_dead_code}` has complete data.
|
||||
> - Note which symbols were not verified due to query failures.
|
||||
>
|
||||
> Output findings as a Markdown list. Each finding: one-line title, which quality gate rule it violates, and evidence from the diff/story file.
|
||||
|
||||
3. **Subagent failure handling**: If any subagent fails, times out, or returns empty results, append the layer name to `{failed_layers}` (comma-separated) and proceed with findings from the remaining layers.
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ activation_steps_append = []
|
|||
|
||||
persistent_facts = [
|
||||
"file:{project-root}/**/project-context.md",
|
||||
"Memtrace structural deep audit is available for independent code review verification. Use the adapter at _bmad/scripts/memtrace/memtrace-adapter.mjs with --query get_impact --target <symbol> --check-freshness --summarize to independently verify the blast radius of modified symbols. Use --query find_dead_code --target <file> --check-freshness for dead code detection in modified modules. Run these during Step 1 (gather context) after constructing the diff. All queries MUST use sequential for...of with await — NEVER Promise.all. Index freshness check via list_indexed_repositories is mandatory before trusting graph output. Structural audit is advisory/supplemental — NEVER block the review on Memtrace availability. Timeout (MEMTRACE_MCP_ERROR_TIMEOUT) or stale index ([FRESHNESS] in STDERR) means skip and continue.",
|
||||
]
|
||||
|
||||
# Scalar: executed when the workflow reaches Step 4 (Present and Act),
|
||||
|
|
|
|||
|
|
@ -3,6 +3,8 @@ diff_output: '' # set at runtime
|
|||
spec_file: '' # set at runtime (path or empty)
|
||||
review_mode: '' # set at runtime: "full" or "no-spec"
|
||||
story_key: '' # set at runtime when discovered from sprint status
|
||||
memtrace_blast_radius: '' # set at runtime: structured blast radius data or "unavailable"/"partial"
|
||||
memtrace_dead_code: '' # set at runtime: structured dead code data or "unavailable"/"partial"
|
||||
---
|
||||
|
||||
# Step 1: Gather Context
|
||||
|
|
@ -75,9 +77,45 @@ story_key: '' # set at runtime when discovered from sprint status
|
|||
- If the user opts to chunk: agree on the first group, narrow `{diff_output}` accordingly, and list the remaining groups for the user to note for follow-up runs.
|
||||
- If the user declines: proceed as-is with the full diff.
|
||||
|
||||
7. **Run structural deep audit queries (Memtrace).** If the repository is indexed by Memtrace, independently verify the diff's structural impact. This step is DIAGNOSTIC — the review continues regardless of availability.
|
||||
|
||||
**Check Availability:**
|
||||
- Use `list_indexed_repositories` to confirm the project repo is indexed
|
||||
- Check the `last_indexed_at` value — if older than 30 minutes, flag as stale and skip graph queries
|
||||
- If not indexed or stale, set `{memtrace_blast_radius}` = `"unavailable"` and `{memtrace_dead_code}` = `"unavailable"`, then skip to CHECKPOINT
|
||||
|
||||
**Extract modified symbols from the diff:**
|
||||
- Parse `{diff_output}` to identify modified functions, methods, classes, and exported variables
|
||||
- Extract symbol names from changed lines (lines starting with `+` or `-` in function/class/method declarations)
|
||||
- De-duplicate and limit to at most 15 symbols (prioritize: exported > internal, functions > variables)
|
||||
- For each symbol, note its containing file path
|
||||
- If no modified symbols are extracted (e.g., only config files, comments, or whitespace changes), skip both blast radius and dead code queries — set `{memtrace_blast_radius}` = `"empty"` and `{memtrace_dead_code}` = `"empty"`
|
||||
|
||||
**Run blast radius audit (`get_impact`):**
|
||||
- For each extracted symbol, call the adapter:
|
||||
`node _bmad/scripts/memtrace/memtrace-adapter.mjs --target <symbol> --query get_impact --check-freshness --summarize`
|
||||
- Process STRICTLY SEQUENTIALLY using `for...of` with `await` — NEVER `Promise.all`
|
||||
- On exit 0: collect `summarized.critical_dependents`, `summarized.module_impact`, `summarized.total_affected`
|
||||
- On exit 1 with `[FRESHNESS]` in STDERR: note "Index stale — skipping blast radius for <symbol>" and continue
|
||||
- On exit 1 with `MEMTRACE_MCP_ERROR_TIMEOUT`: note "MCP timeout — skipping blast radius for <symbol>" and continue
|
||||
- Set `{memtrace_blast_radius}` to the structured results (or `"partial"` if some queries failed, or `"unavailable"` if ALL queries failed)
|
||||
|
||||
**Run dead code audit (`find_dead_code`):**
|
||||
- For each UNIQUE modified file (not per-symbol), call the adapter:
|
||||
`node _bmad/scripts/memtrace/memtrace-adapter.mjs --target <file-path> --query find_dead_code --check-freshness`
|
||||
- Process STRICTLY SEQUENTIALLY using `for...of` with `await`
|
||||
- On exit 0: collect the list of dead code symbols in that file
|
||||
- On failure: note the failure and continue with remaining files
|
||||
- Set `{memtrace_dead_code}` to the structured results (or `"partial"` if some queries failed, or `"unavailable"` if ALL queries failed)
|
||||
|
||||
**Graceful Degradation:**
|
||||
- If `list_indexed_repositories` returns empty or the project repo is not indexed: skip ALL queries, set both variables to `"unavailable"`
|
||||
- If any individual query times out or fails: skip that query only, mark results as `"partial"`, continue with remaining symbols/files
|
||||
- NEVER block or halt the review on Memtrace availability — the structural audit is supplemental intelligence
|
||||
|
||||
### CHECKPOINT
|
||||
|
||||
Present a summary before proceeding: diff stats (files changed, lines added/removed), `{review_mode}`, and loaded spec/context docs (if any). HALT and wait for user confirmation to proceed.
|
||||
Present a summary before proceeding: diff stats (files changed, lines added/removed), `{review_mode}`, loaded spec/context docs (if any), and structural audit status (if `{memtrace_blast_radius}` is not empty/unavailable: if `"partial"`, note "structural audit partially complete — {partial_count}/{total_count} symbol queries failed"; if fully available, include "blast radius queried for {count} symbols, dead code checked in {count} files"). HALT and wait for user confirmation to proceed.
|
||||
|
||||
|
||||
## NEXT
|
||||
|
|
|
|||
|
|
@ -50,6 +50,32 @@ failed_layers: '' # set at runtime: comma-separated list of layers that failed o
|
|||
> - **If the story involves dead-code removal (find_dead_code, dead-code in tasks) BUT no "Dead Code Pitfall Validation Report" exists in the spec file** → raise a `patch` finding: "Missing Dead Code Pitfall Validation Report — story involved dead-code removal but no pitfall validation was performed via validate-dead-code.mjs."
|
||||
> - **If neither dead-code removal nor a pitfall validation report exists**: skip this gate (story does not involve dead-code).
|
||||
>
|
||||
> **Structural Deep Audit — Memtrace Verification:**
|
||||
>
|
||||
> If `{memtrace_blast_radius}` and `{memtrace_dead_code}` are available (not `"unavailable"` or empty), you have access to independently-computed structural data. Use this to verify the diff's claims against the actual codebase graph.
|
||||
>
|
||||
> **Blast Radius Verification:**
|
||||
> - Cross-reference each modified symbol in the diff against `{memtrace_blast_radius}` results.
|
||||
> - If a symbol has `critical_dependents` (any depth) that are NOT modified in the diff → raise a `decision_needed` finding: **"Unhandled downstream dependency: `<dependent-name>` depends on modified `<symbol>` — diff does not include test or mitigation."**
|
||||
> - Evidence: list the specific dependent name(s) and the blast radius data
|
||||
> - If the blast radius `total_affected` for any symbol exceeds 20 → raise a `patch` finding: **"High blast radius: `<symbol>` affects `<count>` dependents (depth `<N>`) — consider narrower refactor scope or expanded test coverage."**
|
||||
> - If the diff or commit message claims "no downstream impact" but blast radius shows dependents → raise a `decision_needed` finding: **"Downstream impact claim falsified: diff claims no impact but blast radius shows `<count>` affected symbols at depth 1+."**
|
||||
> - If a blast radius query returned `"partial"` for some symbols: note which symbols were not verified and proceed with available data only.
|
||||
>
|
||||
> **Dead Code Audit:**
|
||||
> - Check whether any `{memtrace_dead_code}` findings overlap with lines ADDED in modified files (new code in `+` lines).
|
||||
> - If a modified file introduces a NEW function/method/class that also appears in the dead code results → raise a `patch` finding: **"New dead code introduced: `<symbol>` in `<file>` — added but has zero callers in the codebase graph."**
|
||||
> - If a modified file (`+` or `-` lines) contains EXISTING dead code symbols that were NOT removed → raise a `patch` finding: **"Pre-existing dead code unaddressed: `<symbol>` in `<file>` — appears in dead code results but was not cleaned up in this change."**
|
||||
> - Do NOT flag dead code in files that the diff did not touch.
|
||||
>
|
||||
> **If `{memtrace_blast_radius}` or `{memtrace_dead_code}` is `"unavailable"` or empty:**
|
||||
> - Note in the review output: "Structural deep audit unavailable — Memtrace not indexed or queries failed. Proceeding with text-based review only."
|
||||
> - DO NOT raise any structural-audit-specific findings (no blast radius or dead code flags).
|
||||
>
|
||||
> **If `{memtrace_blast_radius}` or `{memtrace_dead_code}` is `"partial"`:**
|
||||
> - Apply the blast radius rules ONLY if `{memtrace_blast_radius}` has complete data; apply dead code rules ONLY if `{memtrace_dead_code}` has complete data.
|
||||
> - Note which symbols were not verified due to query failures.
|
||||
>
|
||||
> Output findings as a Markdown list. Each finding: one-line title, which quality gate rule it violates, and evidence from the diff/story file.
|
||||
|
||||
3. **Subagent failure handling**: If any subagent fails, times out, or returns empty results, append the layer name to `{failed_layers}` (comma-separated) and proceed with findings from the remaining layers.
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ activation_steps_append = []
|
|||
|
||||
persistent_facts = [
|
||||
"file:{project-root}/**/project-context.md",
|
||||
"Memtrace structural deep audit is available for independent code review verification. Use the adapter at _bmad/scripts/memtrace/memtrace-adapter.mjs with --query get_impact --target <symbol> --check-freshness --summarize to independently verify the blast radius of modified symbols. Use --query find_dead_code --target <file> --check-freshness for dead code detection in modified modules. Run these during Step 1 (gather context) after constructing the diff. All queries MUST use sequential for...of with await — NEVER Promise.all. Index freshness check via list_indexed_repositories is mandatory before trusting graph output. Structural audit is advisory/supplemental — NEVER block the review on Memtrace availability. Timeout (MEMTRACE_MCP_ERROR_TIMEOUT) or stale index ([FRESHNESS] in STDERR) means skip and continue.",
|
||||
]
|
||||
|
||||
# Scalar: executed when the workflow reaches its final step,
|
||||
|
|
|
|||
|
|
@ -3,6 +3,8 @@ diff_output: '' # set at runtime
|
|||
spec_file: '' # set at runtime (path or empty)
|
||||
review_mode: '' # set at runtime: "full" or "no-spec"
|
||||
story_key: '' # set at runtime when discovered from sprint status
|
||||
memtrace_blast_radius: '' # set at runtime: structured blast radius data or "unavailable"/"partial"
|
||||
memtrace_dead_code: '' # set at runtime: structured dead code data or "unavailable"/"partial"
|
||||
---
|
||||
|
||||
# Step 1: Gather Context
|
||||
|
|
@ -75,9 +77,45 @@ story_key: '' # set at runtime when discovered from sprint status
|
|||
- If the user opts to chunk: agree on the first group, narrow `{diff_output}` accordingly, and list the remaining groups for the user to note for follow-up runs.
|
||||
- If the user declines: proceed as-is with the full diff.
|
||||
|
||||
7. **Run structural deep audit queries (Memtrace).** If the repository is indexed by Memtrace, independently verify the diff's structural impact. This step is DIAGNOSTIC — the review continues regardless of availability.
|
||||
|
||||
**Check Availability:**
|
||||
- Use `list_indexed_repositories` to confirm the project repo is indexed
|
||||
- Check the `last_indexed_at` value — if older than 30 minutes, flag as stale and skip graph queries
|
||||
- If not indexed or stale, set `{memtrace_blast_radius}` = `"unavailable"` and `{memtrace_dead_code}` = `"unavailable"`, then skip to CHECKPOINT
|
||||
|
||||
**Extract modified symbols from the diff:**
|
||||
- Parse `{diff_output}` to identify modified functions, methods, classes, and exported variables
|
||||
- Extract symbol names from changed lines (lines starting with `+` or `-` in function/class/method declarations)
|
||||
- De-duplicate and limit to at most 15 symbols (prioritize: exported > internal, functions > variables)
|
||||
- For each symbol, note its containing file path
|
||||
- If no modified symbols are extracted (e.g., only config files, comments, or whitespace changes), skip both blast radius and dead code queries — set `{memtrace_blast_radius}` = `"empty"` and `{memtrace_dead_code}` = `"empty"`
|
||||
|
||||
**Run blast radius audit (`get_impact`):**
|
||||
- For each extracted symbol, call the adapter:
|
||||
`node _bmad/scripts/memtrace/memtrace-adapter.mjs --target <symbol> --query get_impact --check-freshness --summarize`
|
||||
- Process STRICTLY SEQUENTIALLY using `for...of` with `await` — NEVER `Promise.all`
|
||||
- On exit 0: collect `summarized.critical_dependents`, `summarized.module_impact`, `summarized.total_affected`
|
||||
- On exit 1 with `[FRESHNESS]` in STDERR: note "Index stale — skipping blast radius for <symbol>" and continue
|
||||
- On exit 1 with `MEMTRACE_MCP_ERROR_TIMEOUT`: note "MCP timeout — skipping blast radius for <symbol>" and continue
|
||||
- Set `{memtrace_blast_radius}` to the structured results (or `"partial"` if some queries failed, or `"unavailable"` if ALL queries failed)
|
||||
|
||||
**Run dead code audit (`find_dead_code`):**
|
||||
- For each UNIQUE modified file (not per-symbol), call the adapter:
|
||||
`node _bmad/scripts/memtrace/memtrace-adapter.mjs --target <file-path> --query find_dead_code --check-freshness`
|
||||
- Process STRICTLY SEQUENTIALLY using `for...of` with `await`
|
||||
- On exit 0: collect the list of dead code symbols in that file
|
||||
- On failure: note the failure and continue with remaining files
|
||||
- Set `{memtrace_dead_code}` to the structured results (or `"partial"` if some queries failed, or `"unavailable"` if ALL queries failed)
|
||||
|
||||
**Graceful Degradation:**
|
||||
- If `list_indexed_repositories` returns empty or the project repo is not indexed: skip ALL queries, set both variables to `"unavailable"`
|
||||
- If any individual query times out or fails: skip that query only, mark results as `"partial"`, continue with remaining symbols/files
|
||||
- NEVER block or halt the review on Memtrace availability — the structural audit is supplemental intelligence
|
||||
|
||||
### CHECKPOINT
|
||||
|
||||
Present a summary before proceeding: diff stats (files changed, lines added/removed), `{review_mode}`, and loaded spec/context docs (if any). HALT and wait for user confirmation to proceed.
|
||||
Present a summary before proceeding: diff stats (files changed, lines added/removed), `{review_mode}`, loaded spec/context docs (if any), and structural audit status (if `{memtrace_blast_radius}` is not empty/unavailable: if `"partial"`, note "structural audit partially complete — {partial_count}/{total_count} symbol queries failed"; if fully available, include "blast radius queried for {count} symbols, dead code checked in {count} files"). HALT and wait for user confirmation to proceed.
|
||||
|
||||
|
||||
## NEXT
|
||||
|
|
|
|||
|
|
@ -22,8 +22,35 @@ failed_layers: '' # set at runtime: comma-separated list of layers that failed o
|
|||
|
||||
- **Edge Case Hunter** — receives `{diff_output}` and read access to the project. Invoke via the `bmad-review-edge-case-hunter` skill.
|
||||
|
||||
- **Acceptance Auditor** (only if `{review_mode}` = `"full"`) — receives `{diff_output}`, the content of the file at `{spec_file}`, and any loaded context docs. Its prompt:
|
||||
> You are an Acceptance Auditor. Review this diff against the spec and context docs. Check for: violations of acceptance criteria, deviations from spec intent, missing implementation of specified behavior, contradictions between spec constraints and actual code. Output findings as a Markdown list. Each finding: one-line title, which AC/constraint it violates, and evidence from the diff.
|
||||
- **Acceptance Auditor** (only if `{review_mode}` = `"full"`) — receives `{diff_output}`, the content of the file at `{spec_file}`, and any loaded context docs. Its prompt:
|
||||
> You are an Acceptance Auditor. Review this diff against the spec and context docs. Check for: violations of acceptance criteria, deviations from spec intent, missing implementation of specified behavior, contradictions between spec constraints and actual code.
|
||||
>
|
||||
> **Structural Deep Audit — Memtrace Verification:**
|
||||
>
|
||||
> If `{memtrace_blast_radius}` and `{memtrace_dead_code}` are available (not `"unavailable"` or empty), you have access to independently-computed structural data. Use this to verify the diff's claims against the actual codebase graph.
|
||||
>
|
||||
> **Blast Radius Verification:**
|
||||
> - Cross-reference each modified symbol in the diff against `{memtrace_blast_radius}` results.
|
||||
> - If a symbol has `critical_dependents` (any depth) that are NOT modified in the diff → raise a `decision_needed` finding: **"Unhandled downstream dependency: `<dependent-name>` depends on modified `<symbol>` — diff does not include test or mitigation."**
|
||||
> - Evidence: list the specific dependent name(s) and the blast radius data
|
||||
> - If the blast radius `total_affected` for any symbol exceeds 20 → raise a `patch` finding: **"High blast radius: `<symbol>` affects `<count>` dependents (depth `<N>`) — consider narrower refactor scope or expanded test coverage."**
|
||||
> - If the diff or commit message claims "no downstream impact" but blast radius shows dependents → raise a `decision_needed` finding: **"Downstream impact claim falsified: diff claims no impact but blast radius shows `<count>` affected symbols at depth 1+."**
|
||||
> - If a blast radius query returned `"partial"` for some symbols: note which symbols were not verified and proceed with available data only.
|
||||
>
|
||||
> **Dead Code Audit:**
|
||||
> - Check whether any `{memtrace_dead_code}` findings overlap with lines ADDED in modified files (new code in `+` lines).
|
||||
> - If a modified file introduces a NEW function/method/class that also appears in the dead code results → raise a `patch` finding: **"New dead code introduced: `<symbol>` in `<file>` — added but has zero callers in the codebase graph."**
|
||||
> - If a modified file (`+` or `-` lines) contains EXISTING dead code symbols that were NOT removed → raise a `patch` finding: **"Pre-existing dead code unaddressed: `<symbol>` in `<file>` — appears in dead code results but was not cleaned up in this change."**
|
||||
> - Do NOT flag dead code in files that the diff did not touch.
|
||||
>
|
||||
> **If `{memtrace_blast_radius}` or `{memtrace_dead_code}` is `"unavailable"` or empty:**
|
||||
> - Note in the review output: "Structural deep audit unavailable — Memtrace not indexed or queries failed. Proceeding with text-based review only."
|
||||
> - DO NOT raise any structural-audit-specific findings (no blast radius or dead code flags).
|
||||
>
|
||||
> **If `{memtrace_blast_radius}` or `{memtrace_dead_code}` is `"partial"`:**
|
||||
> - Apply the blast radius rules ONLY if `{memtrace_blast_radius}` has complete data; apply dead code rules ONLY if `{memtrace_dead_code}` has complete data.
|
||||
>
|
||||
> Output findings as a Markdown list. Each finding: one-line title, which AC/constraint it violates, and evidence from the diff.
|
||||
|
||||
3. **Subagent failure handling**: If any subagent fails, times out, or returns empty results, append the layer name to `{failed_layers}` (comma-separated) and proceed with findings from the remaining layers.
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue