refactor(skill): flatten advanced-elicitation by inlining workflow#2076
refactor(skill): flatten advanced-elicitation by inlining workflow#2076
Conversation
…to SKILL.md Merge workflow.md content directly into SKILL.md and delete the now-redundant workflow file. The frontmatter `agent_party` variable moves into SKILL.md; all relative file references (`./methods.csv`) remain valid. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR consolidates the advanced elicitation workflow from a separate Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/core-skills/bmad-advanced-elicitation/SKILL.md (1)
30-30: Consider simplifying "return back" to just "return".Lines 30 and 101 use "return back," which is stylistically redundant. The phrase works but can be streamlined.
✨ Proposed simplification
Line 30:
-3. Return the enhanced version back when user selects 'x' to proceed and return back +3. Return the enhanced version when user selects 'x' to proceedLine 101:
-- Return the fully enhanced content back to the invoking skill +- Return the fully enhanced content to the invoking skillAlso applies to: 101-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core-skills/bmad-advanced-elicitation/SKILL.md` at line 30, The phrase "return back" in SKILL.md is stylistically redundant; update both occurrences (the sentence "Return the enhanced version back when user selects 'x' to proceed and return back" and the occurrence around line 101) to use "return" (e.g., "Return the enhanced version when the user selects 'x' to proceed") so the wording is concise and consistent; search for the exact phrase "return back" in SKILL.md and replace with "return" while preserving surrounding context and punctuation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/core-skills/bmad-advanced-elicitation/SKILL.md`:
- Line 30: The phrase "return back" in SKILL.md is stylistically redundant;
update both occurrences (the sentence "Return the enhanced version back when
user selects 'x' to proceed and return back" and the occurrence around line 101)
to use "return" (e.g., "Return the enhanced version when the user selects 'x' to
proceed") so the wording is concise and consistent; search for the exact phrase
"return back" in SKILL.md and replace with "return" while preserving surrounding
context and punctuation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 44e737bd-eea3-4096-8729-8fa63c596dfe
📒 Files selected for processing (2)
src/core-skills/bmad-advanced-elicitation/SKILL.mdsrc/core-skills/bmad-advanced-elicitation/workflow.md
💤 Files with no reviewable changes (1)
- src/core-skills/bmad-advanced-elicitation/workflow.md
🤖 Augment PR SummarySummary: This PR refactors the Changes:
Technical Notes: Relative references (e.g., 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| - DO NOT skip steps or change the sequence | ||
| - HALT immediately when halt-conditions are met | ||
| - Each action within a step is a REQUIRED action to complete that step | ||
| - Sections outside flow (validation, output, critical-context) provide essential context - review and apply throughout execution |
There was a problem hiding this comment.
This line references “validation, output, critical-context” sections, but those sections don’t exist in this SKILL.md; that can confuse execution because it asks the agent to apply missing guidance.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| ``` | ||
| **Advanced Elicitation Options** | ||
| _If party mode is active, agents will join in._ | ||
| Choose a number (1-5), [r] to Reshuffle, [a] List All, or [x] to Proceed: |
There was a problem hiding this comment.
After presenting this menu, consider adding an explicit HALT/“wait for user response” instruction; otherwise the agent may continue execution without actually waiting for a selection.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| - Display the enhanced version showing what the method revealed or improved | ||
| - **CRITICAL:** Ask the user if they would like to apply the changes to the doc (y/n/other) and HALT to await response. | ||
| - **CRITICAL:** ONLY if Yes, apply the changes. IF No, discard your memory of the proposed changes. If any other reply, try best to follow the instructions given by the user. | ||
| - **CRITICAL:** Re-present the same 1-5,r,x prompt to allow additional elicitations |
There was a problem hiding this comment.
This says to re-present the “1-5,r,x” prompt, but the menu also offers [a] List All; the mismatch can cause the agent to stop offering the full set of options after the first cycle.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| **Case a (List All):** | ||
|
|
||
| - List all methods with their descriptions from the CSV in a compact table | ||
| - Allow user to select any method by name or number from the full list |
There was a problem hiding this comment.
In the “List All” path, users can select any method “by name or number from the full list”, but the response handling only defines numbered selection for 1–5; clarifying how to handle numbers beyond 5 would prevent ambiguous behavior.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| **Case: Multiple Numbers:** | ||
|
|
||
| - Execute methods in sequence on the content, then re-offer choices |
There was a problem hiding this comment.
“Execute methods in sequence” for multiple-number input seems to bypass the earlier per-method confirm-and-HALT requirement; without clarifying the confirmation behavior here, the skill may apply multiple changes without the intended user acceptance gate.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
…SKILL.md Inline workflow.md content directly into SKILL.md for: editorial-review-prose, editorial-review-structure, help, index-docs, review-adversarial-general, review-edge-case-hunter, and shard-doc. Deletes the now-redundant workflow.md files. No behavioral change — same pattern as advanced-elicitation in PR #2076. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…SKILL.md Inline workflow.md content directly into SKILL.md for: editorial-review-prose, editorial-review-structure, help, index-docs, review-adversarial-general, review-edge-case-hunter, and shard-doc. Deletes the now-redundant workflow.md files. No behavioral change — same pattern as advanced-elicitation in PR #2076. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…SKILL.md (#2077) Inline workflow.md content directly into SKILL.md for: editorial-review-prose, editorial-review-structure, help, index-docs, review-adversarial-general, review-edge-case-hunter, and shard-doc. Deletes the now-redundant workflow.md files. No behavioral change — same pattern as advanced-elicitation in PR #2076. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
workflow.mdcontent directly intoSKILL.mdfor thebmad-advanced-elicitationskillagent_partyvariable) into the single fileworkflow.mdAll relative references (
./methods.csv) remain valid. No behavioral change.Test plan
npm run qualitypasses (all checks green)/bmad-advanced-elicitationand verify it loads correctly🤖 Generated with Claude Code