Improve spec review loop in brainstorming skill#691
Open
Conversation
- Add explicit review loop after spec modifications - Ensure AI asks for user confirmation after each change - Update process flow diagram to reflect iterative review cycle - Update checklist step 8 to clarify the loop requirement This prevents the AI from proceeding to implementation without user approval after making spec changes. Users now have full control over the spec review process with multiple revision rounds.
📝 WalkthroughWalkthroughThe brainstorming skill flow is updated to implement a multi-step user review loop that requires explicit satisfaction before invoking writing-plans. The single review gate is replaced with iterative review, change application, and confirmation steps, with a critical note restricting subsequent skill invocation to only writing-plans. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
skills/brainstorming/SKILL.md (1)
135-142: Minor clarity issue: "Return to this User Review Gate" phrasing.Line 140 states "Return to this User Review Gate" but according to the flow diagram (lines 67-68), after applying user changes, the flow goes through the "Spec review loop" before returning to "Ask user to review again". The text could be clearer about this intermediate step.
Suggested clarification
2. Re-run the spec review loop if needed - 3. **Return to this User Review Gate** - ask again: "I've updated the spec based on your feedback. Please review the changes and let me know if you need any further modifications, or if we can proceed to the implementation plan." + 3. **Once the spec review passes, return to this User Review Gate** - ask again: "I've updated the spec based on your feedback. Please review the changes and let me know if you need any further modifications, or if we can proceed to the implementation plan." 4. Repeat this loop until the user explicitly approves🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/SKILL.md` around lines 135 - 142, Replace the ambiguous phrase "Return to this User Review Gate" with a clearer instruction that reflects the actual flow: after making requested changes to the spec, re-run the "Spec review loop" (as referenced earlier in the doc) and then prompt the user again with the review gate; update the step text in SKILL.md (the block that currently says "Return to this User Review Gate") to read something like "Re-run the Spec review loop, then ask the user to review the updated spec" so it explicitly names the intermediate "Spec review loop" step and the subsequent prompt.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/brainstorming/SKILL.md`:
- Around line 65-69: The diagram and text disagree about whether the spec
re-review is mandatory; make them consistent by changing the flow to be
conditional: modify the diagram edge from "Apply user changes" -> "Spec review
loop" (currently unconditional) to include a decision node or label like "[if
user requests/reviewer fails]" and update the prose phrase "Re-run the spec
review loop if needed" (the sentence containing "Re-run the spec review loop")
to spell out the trigger criteria (e.g., automated-review failure,
user-requested changes, or user opt-in) and/or note that the system will prompt
the user to re-run the automated review when those criteria are met. Ensure
references to "Apply user changes", "Spec review loop", and the sentence "Re-run
the spec review loop" are updated together so diagram and text match.
---
Nitpick comments:
In `@skills/brainstorming/SKILL.md`:
- Around line 135-142: Replace the ambiguous phrase "Return to this User Review
Gate" with a clearer instruction that reflects the actual flow: after making
requested changes to the spec, re-run the "Spec review loop" (as referenced
earlier in the doc) and then prompt the user again with the review gate; update
the step text in SKILL.md (the block that currently says "Return to this User
Review Gate") to read something like "Re-run the Spec review loop, then ask the
user to review the updated spec" so it explicitly names the intermediate "Spec
review loop" step and the subsequent prompt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e25e6881-3ad2-4dde-99e6-f72dceaf01ce
📒 Files selected for processing (1)
skills/brainstorming/SKILL.md
Comment on lines
+65
to
+69
| "Spec review passed?" -> "Ask user to review again" [label="approved"]; | ||
| "Ask user to review again" -> "User satisfied?"; | ||
| "User satisfied?" -> "Apply user changes" [label="changes requested"]; | ||
| "Apply user changes" -> "Spec review loop" [label="re-run review"]; | ||
| "User satisfied?" -> "Invoke writing-plans skill" [label="approved"]; |
There was a problem hiding this comment.
Inconsistency: Diagram mandates spec re-review, but text suggests it's optional.
The flow diagram (line 68) shows a mandatory path from "Apply user changes" back to "Spec review loop", but line 139 states "Re-run the spec review loop if needed", suggesting it's optional.
Potential issue: If a user requests changes that the automated spec reviewer doesn't approve (e.g., stylistic preferences, domain-specific terminology), the AI could get caught in a conflict trying to satisfy both the user's intent and the automated reviewer's rules.
Consider: Should the spec review re-run be:
- Always mandatory (as the diagram shows) - update line 139 to remove "if needed"
- Conditionally optional (as the text suggests) - update the diagram to show an optional path or decision node
- User-controlled - ask the user if they want to re-run the automated review after their changes
Suggested clarification approaches
Option 1: Make it clearly mandatory
- 2. Re-run the spec review loop if needed
+ 2. Re-run the spec review loop to validate the changesOption 2: Make it clearly conditional with criteria
- 2. Re-run the spec review loop if needed
+ 2. Re-run the spec review loop if the changes affect spec content (not just typos/formatting)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/brainstorming/SKILL.md` around lines 65 - 69, The diagram and text
disagree about whether the spec re-review is mandatory; make them consistent by
changing the flow to be conditional: modify the diagram edge from "Apply user
changes" -> "Spec review loop" (currently unconditional) to include a decision
node or label like "[if user requests/reviewer fails]" and update the prose
phrase "Re-run the spec review loop if needed" (the sentence containing "Re-run
the spec review loop") to spell out the trigger criteria (e.g., automated-review
failure, user-requested changes, or user opt-in) and/or note that the system
will prompt the user to re-run the automated review when those criteria are met.
Ensure references to "Apply user changes", "Spec review loop", and the sentence
"Re-run the spec review loop" are updated together so diagram and text match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request: Improve spec review loop in brainstorming skill
Problem Description
The current brainstorming skill has a workflow gap in the spec review phase:
Solution
This PR improves the spec review workflow by establishing an explicit "modify-review" loop:
Key Changes
1. Updated Checklist Step 8
ask user to review the spec file before proceedingask user to review the spec file; if changes requested, apply them and ask again; repeat until user explicitly approves2. Updated Process Flow Diagram
3. Enhanced User Review Gate Documentation
Testing & Validation
Successfully tested through 5 rounds of modify-review cycles:
Impact Scope
skills/brainstorming/SKILL.mdModified Files
skills/brainstorming/SKILL.md- Primary modificationMotivation
This change ensures users have full control over the spec review process. Previously, after requesting changes, users had no opportunity to verify the modifications before the AI moved to implementation planning. This improvement enables iterative refinement of specs with proper user oversight at each step.
Related Issues
N/A - This is a proactive improvement based on user workflow analysis.