feat(workflows): integrate dead-code pitfall validation into dev and review workflows
Insert Dead Code Pitfall Validation substep (step 5) in bmad-dev-story SKILL.md, after mathematical gate and before approve/reject gate. Add Step 5c to bmad-quick-dev step-03-implement.md and step-oneshot.md. Expand code-review Acceptance Auditors in bmad-code-review and gds-code-review with Dead Code Validation Report ground-truth checks. Include Ignored section rendering for FALSE_POS/GHOST entries.
This commit is contained in:
Magal
parent
792fb50745
commit
7a9d66364a
|
|
@ -32,12 +32,17 @@ failed_layers: '' # set at runtime: comma-separated list of layers that failed o
|
|||
> - Reference any blast radius report embedded in the spec/story file to cross-validate the affected modules listed in the justification.
|
||||
> - **Fallback detection:** If no section titled "Test Coverage Justification" is found, search for a markdown table with columns containing "Module", "Affected Symbols", "Test Files", and "Coverage". If such a table exists, treat it as meeting the quality gate regardless of section title.
|
||||
>
|
||||
> **Quality Gate — Mathematical Gate Output:** Check whether the spec/story file includes a "Mathematical Quality Gate Output" section (JSON output from `qa-memtrace.mjs`). Apply these rules:
|
||||
> - **If the spec file contains "Mathematical Quality Gate Output"**: use the `uncovered_nodes` from the script's JSON output as the ground truth for uncovered nodes — NOT the agent's textual claims in the justification table. Cross-validate: if the script found a node as uncovered but the justification table lists it as `Yes` coverage → raise a `decision_needed` finding per mismatch: "Coverage mismatch — agent claimed [node] is covered but mathematical gate shows it is not."
|
||||
> - **If the spec file contains a blast radius report AND a Test Coverage Justification BUT no "Mathematical Quality Gate Output"**: this is a Phase 1-level story using only textual justification. Flag as `patch` (not `decision_needed`): "Phase 1 story — consider upgrading to mathematical quality gate via qa-memtrace.mjs."
|
||||
> - **If the spec file contains only a blast radius report (no test justification, no mathematical gate)**: raise a `decision_needed` finding: "Missing both Test Coverage Justification and Mathematical Quality Gate Output — Phase 2 story must include qa-memtrace.mjs execution results."
|
||||
>
|
||||
> Output findings as a Markdown list. Each finding: one-line title, which quality gate rule it violates, and evidence from the diff/story file.
|
||||
> **Quality Gate — Mathematical Gate Output:** Check whether the spec/story file includes a "Mathematical Quality Gate Output" section (JSON output from `qa-memtrace.mjs`). Apply these rules:
|
||||
> - **If the spec file contains "Mathematical Quality Gate Output"**: use the `uncovered_nodes` from the script's JSON output as the ground truth for uncovered nodes — NOT the agent's textual claims in the justification table. Cross-validate: if the script found a node as uncovered but the justification table lists it as `Yes` coverage → raise a `decision_needed` finding per mismatch: "Coverage mismatch — agent claimed [node] is covered but mathematical gate shows it is not."
|
||||
> - **If the spec file contains a blast radius report AND a Test Coverage Justification BUT no "Mathematical Quality Gate Output"**: this is a Phase 1-level story using only textual justification. Flag as `patch` (not `decision_needed`): "Phase 1 story — consider upgrading to mathematical quality gate via qa-memtrace.mjs."
|
||||
> - **If the spec file contains only a blast radius report (no test justification, no mathematical gate)**: raise a `decision_needed` finding: "Missing both Test Coverage Justification and Mathematical Quality Gate Output — Phase 2 story must include qa-memtrace.mjs execution results."
|
||||
>
|
||||
> **Quality Gate — Dead Code Pitfall Validation:** Check whether the spec/story file includes a "Dead Code Pitfall Validation Report" section (JSON output from `validate-dead-code.mjs`). Apply these rules:
|
||||
> - **If the spec file contains "Dead Code Pitfall Validation Report"**: verify that the `suspects` list entries were addressed in the implementation (check if corresponding tasks exist in Tasks/Subtasks, or if deleted files match suspect entries). If SUSPECT entries were not addressed → raise a `decision_needed` finding per unaddressed suspect: "SUSPECT dead-code entry not addressed: [name] in [file] — pitfall validation flagged this as truly dead code but no removal task was completed."
|
||||
> - **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).
|
||||
>
|
||||
> 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.
|
||||
|
||||
|
|
|
|||
|
|
@ -422,6 +422,65 @@ Activation is complete. Begin the workflow below.
|
|||
|
||||
<anchor id="quality_gate_done" />
|
||||
|
||||
<!-- DEAD CODE PITFALL VALIDATION -->
|
||||
<critical>If the story involves dead-code removal, validate candidates against pitfalls-catalog.json before proceeding.</critical>
|
||||
|
||||
<check if="story involves dead-code removal (look for find_dead_code usage in story context or tasks)">
|
||||
<action>Call `memtrace_find_dead_code` via MCP to retrieve dead-code candidates for the target module(s). Process sequentially using `for...of` with `await`.</action>
|
||||
<action>Serialize the dead-code candidates to a temporary JSON file in the system temp directory.</action>
|
||||
<action>Execute: `node _bmad/scripts/memtrace/validate-dead-code.mjs --candidates <temp-file>`</action>
|
||||
<action>Read the script's STDOUT (JSON output) and capture the exit code.</action>
|
||||
|
||||
<check if="script exits with code 0 (classification completed)">
|
||||
<action>Log the script output to the story file's Dev Agent Record → Completion Notes as "Dead Code Pitfall Validation Report".</action>
|
||||
<action>Append to the report:
|
||||
"Dead Code Pitfall Validation: PASSED
|
||||
- SUSPECT (truly dead): N entries — review required
|
||||
- FALSE_POS (matched catalog): N entries — ignored
|
||||
- GHOST (file deleted): N entries — ignored"
|
||||
</action>
|
||||
<check if="script output has suspects.length > 0">
|
||||
<action>Present the SUSPECT list for manual review before the Approve/Reject prompt:
|
||||
"**SUSPECT Entries (truly dead code — review before removal):**
|
||||
{{for each suspect in script output.suspects}}
|
||||
- `{{suspect.name}}` in `{{suspect.file}}` (line {{suspect.line}})"
|
||||
</action>
|
||||
</check>
|
||||
|
||||
<action>Also render the "Ignored" section with FALSE_POS and GHOST entries with their reasons (AC #4):
|
||||
"**Ignored (safe to skip):**
|
||||
{{if script output has false_positives}}
|
||||
**FALSE_POS (matched pitfalls catalog):**
|
||||
{{for each fp in script output.false_positives}}
|
||||
- `{{fp.name}}` — {{fp.reason}} (pitfall: {{fp.pitfall_id}})
|
||||
{{end}}
|
||||
{{end}}
|
||||
{{if script output has ghosts}}
|
||||
**GHOST (source file deleted):**
|
||||
{{for each ghost in script output.ghosts}}
|
||||
- `{{ghost.name}}` — {{ghost.reason}}
|
||||
{{end}}
|
||||
{{end}}"
|
||||
</action>
|
||||
|
||||
<check if="script output has suspects.length == 0">
|
||||
<output>✅ No SUSPECT dead-code entries found. All candidates are FALSE_POS (safe) or GHOST (already deleted).</output>
|
||||
</check>
|
||||
</check>
|
||||
|
||||
<check if="script exits with code 1 (error or timeout)">
|
||||
<action>Log the error to the story file's Dev Agent Record → Completion Notes as "Dead Code Pitfall Validation Error".</action>
|
||||
<action>Append to the report: "Dead Code Pitfall Validation: ERROR — classification failed ({{error}}). Proceeding without pitfall validation."</action>
|
||||
<output>⚠️ Dead Code Pitfall Validation encountered an error. Proceeding without pitfall validation. See story file for details.</output>
|
||||
</check>
|
||||
|
||||
<action>Clean up temporary JSON files.</action>
|
||||
</check>
|
||||
|
||||
<check if="story does NOT involve dead-code removal">
|
||||
<action>Skip this substep entirely.</action>
|
||||
</check>
|
||||
|
||||
<!-- HALT for user decision -->
|
||||
<ask>Decision: Proceed with modification? [A] Approve — proceed to implementation | [R] Reject — halt execution</ask>
|
||||
|
||||
|
|
|
|||
|
|
@ -88,7 +88,16 @@ Verify `{spec_file}` resolves to a non-empty path and the file exists on disk. I
|
|||
- Read the script's STDOUT and capture its exit code
|
||||
- **If exit 0**: log the output to `{spec_file}` completion notes under "Mathematical Quality Gate Output" and continue
|
||||
- **If exit 1**: persist the output to `{spec_file}` completion notes, present the uncovered nodes, then HALT: "Mathematical quality gate failed. N of M required nodes are not covered by tests. Agent must write/update tests for the listed uncovered nodes before proceeding. Do NOT proceed until the quality gate passes."
|
||||
- The qa-memtrace.mjs exit code is the FINAL authority. Exit 1 is a HARD BLOCK on implementation.
|
||||
- The qa-memtrace.mjs exit code is the FINAL authority. Exit 1 is a HARD BLOCK on implementation.
|
||||
|
||||
5c. **Dead Code Pitfall Validation**: If the story involves dead-code removal (find_dead_code usage in context or tasks):
|
||||
- Call `memtrace_find_dead_code` via MCP for the target module(s). Process sequentially — NEVER use `Promise.all`.
|
||||
- Serialize candidates to a temp JSON file.
|
||||
- Run: `node _bmad/scripts/memtrace/validate-dead-code.mjs --candidates <temp-file>`
|
||||
- If exit 0: log output to `{spec_file}` completion notes as "Dead Code Pitfall Validation Report". Present SUSPECT entries for manual review. FALSE_POS and GHOST are ignored.
|
||||
- If exit 1: log error to completion notes and proceed without pitfall validation (error is logged, not a hard block).
|
||||
- Clean up temp files.
|
||||
- If story does NOT involve dead-code removal, skip this step entirely.
|
||||
|
||||
6. **HALT for decision**: Ask the user: "Decision: Proceed with modification? [A] Approve — proceed to implementation | [R] Reject — halt execution"
|
||||
- If **Approve**: Continue to the Baseline step below
|
||||
|
|
|
|||
|
|
@ -72,7 +72,16 @@ deferred_work_file: '{implementation_artifacts}/deferred-work.md'
|
|||
- Read the script's STDOUT and capture its exit code
|
||||
- **If exit 0**: log the output to `{spec_file}` completion notes under "Mathematical Quality Gate Output" and continue
|
||||
- **If exit 1**: persist the output to `{spec_file}` completion notes, present the uncovered nodes, then HALT: "Mathematical quality gate failed. N of M required nodes are not covered by tests. Agent must write/update tests before proceeding."
|
||||
- The qa-memtrace.mjs exit code is the FINAL authority. Exit 1 is a HARD BLOCK on implementation.
|
||||
- The qa-memtrace.mjs exit code is the FINAL authority. Exit 1 is a HARD BLOCK on implementation.
|
||||
|
||||
5c. **Dead Code Pitfall Validation**: If the story involves dead-code removal (find_dead_code usage):
|
||||
- Call `memtrace_find_dead_code` via MCP. Process sequentially — NEVER use `Promise.all`.
|
||||
- Serialize candidates to a temp JSON file.
|
||||
- Run: `node _bmad/scripts/memtrace/validate-dead-code.mjs --candidates <temp-file>`
|
||||
- If exit 0: log output to `{spec_file}` completion notes as "Dead Code Pitfall Validation Report". Present SUSPECT entries for manual review. Ignore FALSE_POS and GHOST.
|
||||
- If exit 1: log error and proceed (error is logged, not a hard block).
|
||||
- Clean up temp files.
|
||||
- If story does NOT involve dead-code removal, skip this step entirely.
|
||||
|
||||
6. **HALT for decision**: Ask user: "[A] Approve — proceed | [R] Reject — halt"
|
||||
- Approve → continue to Implement below
|
||||
|
|
|
|||
|
|
@ -32,12 +32,17 @@ failed_layers: '' # set at runtime: comma-separated list of layers that failed o
|
|||
> - Reference any blast radius report embedded in the spec/story file to cross-validate the affected modules listed in the justification.
|
||||
> - **Fallback detection:** If no section titled "Test Coverage Justification" is found, search for a markdown table with columns containing "Module", "Affected Symbols", "Test Files", and "Coverage". If such a table exists, treat it as meeting the quality gate regardless of section title.
|
||||
>
|
||||
> **Quality Gate — Mathematical Gate Output:** Check whether the spec/story file includes a "Mathematical Quality Gate Output" section (JSON output from `qa-memtrace.mjs`). Apply these rules:
|
||||
> - **If the spec file contains "Mathematical Quality Gate Output"**: use the `uncovered_nodes` from the script's JSON output as the ground truth for uncovered nodes — NOT the agent's textual claims in the justification table. Cross-validate: if the script found a node as uncovered but the justification table lists it as `Yes` coverage → raise a `decision_needed` finding per mismatch: "Coverage mismatch — agent claimed [node] is covered but mathematical gate shows it is not."
|
||||
> - **If the spec file contains a blast radius report AND a Test Coverage Justification BUT no "Mathematical Quality Gate Output"**: this is a Phase 1-level story using only textual justification. Flag as `patch` (not `decision_needed`): "Phase 1 story — consider upgrading to mathematical quality gate via qa-memtrace.mjs."
|
||||
> - **If the spec file contains only a blast radius report (no test justification, no mathematical gate)**: raise a `decision_needed` finding: "Missing both Test Coverage Justification and Mathematical Quality Gate Output — Phase 2 story must include qa-memtrace.mjs execution results."
|
||||
>
|
||||
> Output findings as a Markdown list. Each finding: one-line title, which quality gate rule it violates, and evidence from the diff/story file.
|
||||
> **Quality Gate — Mathematical Gate Output:** Check whether the spec/story file includes a "Mathematical Quality Gate Output" section (JSON output from `qa-memtrace.mjs`). Apply these rules:
|
||||
> - **If the spec file contains "Mathematical Quality Gate Output"**: use the `uncovered_nodes` from the script's JSON output as the ground truth for uncovered nodes — NOT the agent's textual claims in the justification table. Cross-validate: if the script found a node as uncovered but the justification table lists it as `Yes` coverage → raise a `decision_needed` finding per mismatch: "Coverage mismatch — agent claimed [node] is covered but mathematical gate shows it is not."
|
||||
> - **If the spec file contains a blast radius report AND a Test Coverage Justification BUT no "Mathematical Quality Gate Output"**: this is a Phase 1-level story using only textual justification. Flag as `patch` (not `decision_needed`): "Phase 1 story — consider upgrading to mathematical quality gate via qa-memtrace.mjs."
|
||||
> - **If the spec file contains only a blast radius report (no test justification, no mathematical gate)**: raise a `decision_needed` finding: "Missing both Test Coverage Justification and Mathematical Quality Gate Output — Phase 2 story must include qa-memtrace.mjs execution results."
|
||||
>
|
||||
> **Quality Gate — Dead Code Pitfall Validation:** Check whether the spec/story file includes a "Dead Code Pitfall Validation Report" section (JSON output from `validate-dead-code.mjs`). Apply these rules:
|
||||
> - **If the spec file contains "Dead Code Pitfall Validation Report"**: verify that the `suspects` list entries were addressed in the implementation (check if corresponding tasks exist in Tasks/Subtasks, or if deleted files match suspect entries). If SUSPECT entries were not addressed → raise a `decision_needed` finding per unaddressed suspect: "SUSPECT dead-code entry not addressed: [name] in [file] — pitfall validation flagged this as truly dead code but no removal task was completed."
|
||||
> - **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).
|
||||
>
|
||||
> 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.
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue