Compare commits

..

No commits in common. "c45cc35c60224ccedef5b4224ae001269197952b" and "beeb2e1778c84b9e6dc342cf99f3be0183df4c61" have entirely different histories.

7 changed files with 90 additions and 130 deletions

View File

@ -20,7 +20,6 @@ You are assisting the user in reviewing a change.
Load and read full config from `{project-root}/_bmad/bmm/config.yaml` and resolve:
- `implementation_artifacts`
- `planning_artifacts`
- `communication_language`
## FIRST STEP

View File

@ -1,38 +0,0 @@
# Generate Review Trail
Generate a review trail from the diff and codebase context. A generated trail is lower quality than an author-produced one, but far better than none.
## Follow Global Step Rules in SKILL.md
## INSTRUCTIONS
1. Get the full diff against the appropriate baseline (same rules as Surface Area Stats in step-01).
2. Read changed files in full — not just diff hunks. Surrounding code reveals intent that hunks alone miss. If total file content exceeds ~50k tokens, read only the files with the largest diff hunks in full and use hunks for the rest.
3. If a spec exists, use its Intent section to anchor concern identification.
4. Identify 25 concerns: cohesive design intents that each explain *why* behind a cluster of changes. Prefer functional groupings and architectural boundaries over file-level splits. A single-concern change is fine — don't invent groupings.
5. For each concern, select 14 `path:line` stops — locations where the concern is most visible. Prefer entry points, decision points, and boundary crossings over mechanical changes.
6. Lead with the entry point — the highest-leverage stop a reviewer should see first. Inside each concern, order stops so each builds on the previous. End with peripherals (tests, config, types).
7. Format each stop using `path:line` per the global step rules:
```
**{Concern name}**
- {one-line framing, ≤15 words}
`src/path/to/file.ts:42`
```
When there is only one concern, omit the bold label — just list the stops directly.
## PRESENT
Output after the orientation:
```
I built a review trail for this {change_type} (no author-produced trail was found):
{generated trail}
```
Set review mode to `full-trail`. The generated trail is the Suggested Review Order for subsequent steps.
If git is unavailable or the diff cannot be retrieved, return to step-01 with: "Could not generate trail — git unavailable."

View File

@ -4,71 +4,56 @@ Display: `[Orientation] → Walkthrough → Detail Pass → Testing`
## Follow Global Step Rules in SKILL.md
- The conversation context before this skill was triggered IS your starting point — not a blank slate.
- The user may have already mentioned a spec file, a commit, a branch, a PR, or described the change. Use what they gave you.
- For PR references, resolve to a commit or branch using whatever tools are available (e.g., `gh pr view` for GitHub). If resolution fails, ask for a SHA or branch.
## FIND THE CHANGE
The conversation context before this skill was triggered IS your starting point — not a blank slate. Check in this order — stop as soon as the change is identified:
1. **Explicit argument**
Did the user pass a PR, commit SHA, branch, or spec file this message?
- PR reference → resolve to branch/commit via `gh pr view`. If resolution fails, ask for a SHA or branch.
- Spec file, commit, or branch → use directly.
2. **Recent conversation**
Do the last few messages reveal what change the user wants reviewed? Look for spec paths, commit refs, branches, PRs, or descriptions of a change. Use the same routing as above.
3. **Sprint tracking**
Check for a sprint status file (`*sprint-status*`) in `{implementation_artifacts}` or `{planning_artifacts}`. If found, scan for stories with status `review`:
- Exactly one → suggest it and confirm with the user.
- Multiple → present as numbered options.
- None → fall through.
4. **Current git state**
Check current branch and HEAD. Confirm: "I see HEAD is `<short-sha>` on `<branch>` — is this the change you want to review?"
5. **Ask**
If none of the above identified a change, ask:
1. **What did the user already tell you?** Look at the conversation for a spec path, commit ref, branch, PR, or description of the change.
2. **Fill in the other half.** If they gave a spec, look for `baseline_commit` in its frontmatter. If they gave a commit, check `{implementation_artifacts}` for a spec whose `baseline_commit` matches. If they gave both, use both.
3. **If they gave nothing**, check current branch and HEAD. Confirm: "I see HEAD is `<short-sha>` on `<branch>` — is this the change you want to review?"
4. **If you still can't identify a change**, ask:
- What changed and why?
- Which commit, branch, or PR should I look at?
- Do you have a spec, bug report, or anything else that explains what this change is supposed to do?
If after 3 exchanges you still can't identify a change, HALT.
Never ask extra questions beyond what the cascade prescribes. If a step above already identified the change, skip the remaining steps.
## ENRICH
Once a change is identified from any source above, fill in the complementary artifact:
- If you have a spec, look for `baseline_commit` in its frontmatter to determine the diff baseline.
- If you have a commit or branch, check `{implementation_artifacts}` for a spec whose `baseline_commit` is an ancestor of that commit/branch (i.e., the spec describes work done on top of that baseline).
- If you found both a spec and a commit/branch, use both.
## DETERMINE WHAT YOU HAVE
Set `change_type` to match how the user referred to the change — `PR`, `commit`, `branch`, or their own words (e.g. `auth refactor`). Default to `change` if ambiguous.
Set `change_type` based on how the user referred to the change:
Set `review_mode` — pick the first match:
| User referenced | `change_type` |
|---|---|
| A pull request | `PR` |
| A commit | `commit` |
| A branch | `branch` |
| A description (e.g. "the auth refactor") | use their words (e.g. `auth refactor`) |
| Nothing specific / ambiguous | `change` |
1. **`full-trail`** — ENRICH found a spec with a `## Suggested Review Order` section. Intent source: spec's Intent section.
2. **`spec-only`** — ENRICH found a spec but it has no Suggested Review Order. Intent source: spec's Intent section.
3. **`bare-commit`** — no spec found. Intent source: commit message. If the commit message is terse (under 10 words), scan the diff for the primary change pattern and draft a one-sentence intent. Confirm with the user before proceeding.
| What you have | Review Mode | Intent Source |
|---|---|---|
| Spec with `## Suggested Review Order` | `full-trail` | Spec's Intent section |
| Spec without trail | `spec-only` | Spec's Intent section |
| Commit ref, no spec | `bare-commit` | Commit message |
**Terse commit messages** (under 10 words): Scan the diff for the primary change pattern and draft a one-sentence intent. Confirm with the user before proceeding.
## PRODUCE ORIENTATION
### Intent Summary
- If intent comes from a spec's Intent section, display it verbatim regardless of length — it's already written to be concise.
- For other sources (commit messages, bug reports, user description): if ≤200 tokens, display verbatim. If longer, distill to ≤200 tokens. Link to the full source when one exists (e.g. a file path or URL).
- For other sources (commit messages, bug reports, user description): if ≤200 tokens, display verbatim. If longer, distill to ≤200 tokens and link to the full source.
- Format: `> **Intent:** {summary}`
### Surface Area Stats
Best-effort stats from `git diff --stat`. Try these baselines in order:
Compute from `git diff --stat` against the appropriate baseline:
1. `baseline_commit` from the spec's frontmatter.
2. Branch merge-base against `main` (or the default branch).
3. `HEAD~1..HEAD` (latest commit only — tell the user).
4. If git is unavailable or all of the above fail, skip stats and note: "Could not compute stats."
- **With spec**: Use `baseline_commit` from frontmatter. If missing, diff `HEAD~1..HEAD` and tell the user stats reflect only the latest commit.
- **Bare commit**: Diff against parent (`commit~1..commit`). For merge commits, use `--first-parent`.
Display as:
@ -76,13 +61,13 @@ Display as:
N files changed · M modules touched · ~L lines of logic · B boundary crossings · P new public interfaces
```
- **Files changed**: from `git diff --stat`.
- **Modules touched**: distinct top-level directories with changes.
- **Files changed**: from `git diff --stat`
- **Modules touched**: distinct top-level directories with changes
- **Lines of logic**: added/modified lines excluding blanks, imports, formatting. `~` because approximate.
- **Boundary crossings**: changes spanning more than one top-level module. `0` if single module.
- **New public interfaces**: new exports, endpoints, public methods. `0` if none.
Omit any metric you cannot compute rather than guessing.
If git is unavailable or a command fails, show what you can and note what's missing.
### Present
@ -96,7 +81,38 @@ Omit any metric you cannot compute rather than guessing.
## FALLBACK TRAIL GENERATION
If review mode is not `full-trail`, read fully and follow `./generate-trail.md` to build one from the diff. Then return here and continue to NEXT.
**Skip this section if review mode is `full-trail`.**
When no Suggested Review Order exists, generate one from the diff and codebase context. A generated trail is lower quality than an author-produced one, but far better than none.
1. Get the full diff against the appropriate baseline (same rules as Surface Area Stats).
2. Read changed files in full — not just diff hunks. Surrounding code reveals intent that hunks alone miss. If total file content exceeds ~50k tokens, read only the files with the largest diff hunks in full and use hunks for the rest.
3. If a spec exists, use its Intent section to anchor concern identification.
4. Identify 25 concerns: cohesive design intents that each explain *why* behind a cluster of changes. Prefer functional groupings and architectural boundaries over file-level splits. A single-concern change is fine — don't invent groupings.
5. For each concern, select 14 `path:line` stops — locations where the concern is most visible. Prefer entry points, decision points, and boundary crossings over mechanical changes.
6. Lead with the entry point — the highest-leverage stop a reviewer should see first. Inside each concern, order stops so each builds on the previous. End with peripherals (tests, config, types).
7. Format each stop using `path:line` per the global step rules:
```
**{Concern name}**
- {one-line framing, ≤15 words}
`src/path/to/file.ts:42`
```
When there is only one concern, omit the bold label — just list the stops directly.
Present after the orientation output:
```
I built a review trail for this {change_type} (no author-produced trail was found):
{generated trail}
```
Set review mode to `full-trail`. The generated trail is the Suggested Review Order for subsequent steps.
If git is unavailable or the diff cannot be retrieved, skip this section and proceed with the original review mode. Note: "Could not generate trail — git unavailable."
## NEXT

View File

@ -73,17 +73,12 @@ Take your time — click through the stops, read the diff, trace the logic. Whil
- "party mode on whether this schema migration is safe"
- or just ask anything
When you're ready, say **next** and I'll surface the highest-risk spots.
When you are done with the walkthrough:
- **"detail pass"** — I'll surface the highest-risk spots and run code review
- **"testing"** — I'll suggest how to observe the behavior
- **"I've seen enough"** — tell me what to do about this {change_type} and we wrap up
```
## EARLY EXIT
If at any point the human signals they want to make a decision about this {change_type} (e.g., "let's ship it", "this needs a rethink", "I'm done reviewing", or anything suggesting they're ready to decide), confirm their intent:
- If they want to **approve and ship** → read fully and follow `./step-05-wrapup.md`
- If they want to **reject and rework** → read fully and follow `./step-05-wrapup.md`
- If you misread them → acknowledge and continue the current step.
## NEXT
Default: read fully and follow `./step-03-detail-pass.md`
When the human signals readiness for the next phase, read fully and follow `./step-03-detail-pass.md`

View File

@ -76,17 +76,10 @@ End the message with:
You've seen the design and the risk landscape. From here:
- **"dig into [area]"** — I'll deep-dive that specific area with correctness focus
- **"next"** — I'll suggest how to observe the behavior
- **"testing"** — I'll suggest how to observe the behavior
- **"I've seen enough"** — tell me what to do about this {change_type} and we wrap up
```
## EARLY EXIT
If at any point the human signals they want to make a decision about this {change_type} (e.g., "let's ship it", "this needs a rethink", "I'm done reviewing", or anything suggesting they're ready to decide), confirm their intent:
- If they want to **approve and ship** → read fully and follow `./step-05-wrapup.md`
- If they want to **reject and rework** → read fully and follow `./step-05-wrapup.md`
- If you misread them → acknowledge and continue the current step.
## TARGETED RE-REVIEW
When the human says "dig into [area]" (e.g., "dig into the auth changes", "dig into the schema migration"):
@ -103,4 +96,4 @@ The human can trigger multiple targeted re-reviews. Each time, present new findi
## NEXT
Read fully and follow `./step-04-testing.md`
When the human signals readiness for testing, read fully and follow `./step-04-testing.md`

View File

@ -66,9 +66,26 @@ End the message with:
```
---
You've seen the change and how to verify it. When you're ready to make a call, just say so.
You've seen the change and how to verify it. From here:
- **"I've seen enough"** — we wrap up this {change_type}
- or just ask anything
```
## NEXT
## WRAP-UP
When the human signals they're ready to make a decision about this {change_type}, read fully and follow `./step-05-wrapup.md`
When the human says "I've seen enough" or signals they're done:
```
---
Review complete. What's the call on this {change_type}?
- **Approve** — ship it
- **Request changes** — I'll help draft feedback
- **Discuss** — something's still on your mind
```
Act on their decision:
- **Approve**: Acknowledge briefly. If reviewing a PR, offer to approve via `gh pr review --approve` — but confirm with the human before executing, since this is a visible action on a shared resource.
- **Request changes**: Ask what needs changing. Help draft specific, actionable feedback tied to `path:line` locations.
- **Discuss**: Open conversation — answer questions, explore concerns, dig into any aspect.

View File

@ -1,22 +0,0 @@
# Step 5: Wrap-Up
Display: `Orientation → Walkthrough → Detail Pass → Testing → [Wrap-Up]`
## Follow Global Step Rules in SKILL.md
## PROMPT FOR DECISION
```
---
Review complete. What's the call on this {change_type}?
- **Approve** — ship it (I can help with interactive patching first if needed)
- **Rework** — back to the drawing board (revert, revise the spec, try a different approach)
- **Discuss** — something's still on your mind
```
## ACT ON DECISION
- **Approve**: Acknowledge briefly. If the human wants to patch something before shipping, help apply the fix interactively. If reviewing a PR, offer to approve via `gh pr review --approve` — but confirm with the human before executing, since this is a visible action on a shared resource.
- **Rework**: Ask what went wrong — was it the approach, the spec, or the implementation? Help the human decide on next steps (revert commit, open an issue, revise the spec, etc.). Help draft specific, actionable feedback tied to `path:line` locations if the change is a PR from someone else.
- **Discuss**: Open conversation — answer questions, explore concerns, dig into any aspect. After discussion, return to the decision prompt above.