fix(skills): apply triage fixes to code-review step files
Address findings from adversarial review: clarify spec_file as path variable (F4), add file list construction rule (F8), HALT on unparseable diffs (F9), differentiate empty findings handling (F5), merge evidence during dedup instead of dropping (F6), and fix NEXT step paths (F1). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
5de8a78c6a
commit
17cd19f07b
|
|
@ -25,14 +25,15 @@ review_mode: '' # set at runtime: "full" or "no-spec"
|
|||
2. Construct `{diff_output}` from the chosen source.
|
||||
- For **branch diff**: verify the base branch exists before running `git diff`. If it does not exist, HALT and ask the user for a valid branch.
|
||||
- For **commit range**: verify the range resolves. If it does not, HALT and ask the user for a valid range.
|
||||
- For **provided diff**: validate the content is non-empty and parseable.
|
||||
- For **provided diff**: validate the content is non-empty and parseable as a unified diff. If it is not parseable, HALT and ask the user to provide a valid diff.
|
||||
- For **file list**: validate each path exists in the working tree. Construct `{diff_output}` by running `git diff HEAD -- <path1> <path2> ...`. If the diff is empty (files have no uncommitted changes), ask the user whether to review the full file contents or to specify a different baseline.
|
||||
- After constructing `{diff_output}`, verify it is non-empty regardless of source type. If empty, HALT and tell the user there is nothing to review.
|
||||
|
||||
3. Ask the user: **Is there a spec or story file that provides context for these changes?**
|
||||
- If yes: load it as `{spec_file}`, set `{review_mode}` = `"full"`.
|
||||
- If yes: set `{spec_file}` to the path provided, verify the file exists and is readable, then set `{review_mode}` = `"full"`.
|
||||
- If no: set `{review_mode}` = `"no-spec"`.
|
||||
|
||||
4. If `{review_mode}` = `"full"` and `{spec_file}` has a `context` field in its frontmatter listing additional docs, load each referenced document. Warn the user about any docs that cannot be found.
|
||||
4. If `{review_mode}` = `"full"` and the file at `{spec_file}` has a `context` field in its frontmatter listing additional docs, load each referenced document. Warn the user about any docs that cannot be found.
|
||||
|
||||
5. Sanity check: if `{diff_output}` exceeds approximately 3000 lines, warn the user and offer to chunk the review by file group.
|
||||
- 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.
|
||||
|
|
@ -45,4 +46,4 @@ Present a summary before proceeding: diff stats (files changed, lines added/remo
|
|||
|
||||
## NEXT
|
||||
|
||||
Read fully and follow `./steps/step-02-review.md`
|
||||
Read fully and follow `./step-02-review.md`
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@ description: 'Launch parallel adversarial review layers and collect findings.'
|
|||
|
||||
- **Edge Case Hunter** -- Invoke the `bmad-review-edge-case-hunter` skill in a subagent. Pass `content` = `{diff_output}`. This subagent has read access to the project.
|
||||
|
||||
- **Acceptance Auditor** (only if `{review_mode}` = `"full"`) -- A subagent that receives `{diff_output}`, `{spec_file}` content, and any loaded context docs. Its prompt:
|
||||
- **Acceptance Auditor** (only if `{review_mode}` = `"full"`) -- A subagent that 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.
|
||||
|
||||
2. **Subagent failure handling**: If any subagent fails, times out, or returns empty results, note the failed layer and proceed with findings from the remaining layers. Report the failure to the user in the next step.
|
||||
|
|
@ -37,4 +37,4 @@ description: 'Launch parallel adversarial review layers and collect findings.'
|
|||
|
||||
## NEXT
|
||||
|
||||
Read fully and follow `./steps/step-03-triage.md`
|
||||
Read fully and follow `./step-03-triage.md`
|
||||
|
|
|
|||
|
|
@ -26,7 +26,10 @@ description: 'Normalize, deduplicate, and classify all review findings into acti
|
|||
- `detail` -- full description
|
||||
- `location` -- file and line reference (if available)
|
||||
|
||||
2. **Deduplicate.** If two findings describe the same issue, keep the one with more specificity (prefer edge-case JSON with location over adversarial prose). Note merged sources on the surviving finding.
|
||||
2. **Deduplicate.** If two or more findings describe the same issue, merge them into one:
|
||||
- Use the most specific finding as the base (prefer edge-case JSON with location over adversarial prose).
|
||||
- Append any unique detail, reasoning, or location references from the other finding(s) into the surviving `detail` field.
|
||||
- Set `source` to the merged sources (e.g., `blind+edge`).
|
||||
|
||||
3. **Classify** each finding into exactly one bucket:
|
||||
- **intent_gap** -- The spec/intent is incomplete; cannot resolve from existing information. Only possible if `{review_mode}` = `"full"`.
|
||||
|
|
@ -46,4 +49,4 @@ description: 'Normalize, deduplicate, and classify all review findings into acti
|
|||
|
||||
## NEXT
|
||||
|
||||
Read fully and follow `./steps/step-04-present.md`
|
||||
Read fully and follow `./step-04-present.md`
|
||||
|
|
|
|||
Loading…
Reference in New Issue