fix(writing-plans): require strict TDD task order in generated plans#629
Conversation
📝 WalkthroughWalkthroughThe writing-plans skill is updated to mandate test-driven development discipline, requiring all tasks to follow a failing-test-first sequence with validation checks before plan execution. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🤖 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/writing-plans/SKILL.md`:
- Around line 100-111: The "Task Structure Validation (MANDATORY)" section's
Allowed exceptions list diverges from the canonical TDD skill's exceptions,
causing inconsistency; update the Allowed exceptions in this SKILL.md to match
the canonical TDD exceptions (e.g., include Throwaway prototypes, Generated
code, Configuration files) or add a brief explicit rationale explaining why this
plan-writing context needs different exceptions, and also add a note clarifying
that the "STOP and rewrite" guidance is manual (or link to an automated
validation task if one exists) so readers know enforcement is not automatic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3637ef1a-5cc2-45d5-b0f4-53e44b6249f5
📒 Files selected for processing (1)
skills/writing-plans/SKILL.md
| ## Task Structure (MANDATORY) | ||
|
|
||
| Every implementation task MUST follow this exact 5-step order: | ||
|
|
||
| 1. **Step 1: Write the failing test** (include complete test code) | ||
| 2. **Step 2: Run test to verify it fails** (include exact command + expected failure) | ||
| 3. **Step 3: Write minimal implementation** (only enough to pass the new test) | ||
| 4. **Step 4: Run test to verify it passes** (include exact command + expected success) | ||
| 5. **Step 5: Commit** |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the TDD skill defines a REFACTOR step
echo "=== Searching for REFACTOR in test-driven-development SKILL.md ==="
cat skills/test-driven-development/SKILL.md | rg -B 3 -A 3 "REFACTOR|refactor"
echo ""
echo "=== Full TDD skill file (checking size first) ==="
wc -l skills/test-driven-development/SKILL.mdRepository: obra/superpowers
Length of output: 1511
🏁 Script executed:
#!/bin/bash
# Check the writing-plans SKILL.md for the full task structure and related sections
echo "=== Writing-plans SKILL.md around the task structure (lines 40-120) ==="
sed -n '40,120p' skills/writing-plans/SKILL.mdRepository: obra/superpowers
Length of output: 2417
Add the REFACTOR step to the mandatory 5-step task structure.
The canonical TDD workflow defined in skills/test-driven-development/SKILL.md includes an explicit REFACTOR phase (RED-GREEN-REFACTOR). Your current 5-step structure implements RED-GREEN-COMMIT, systematically omitting refactoring from every task in every plan.
This creates a documented divergence from established TDD discipline. Plans generated under this structure will skip code quality improvements, even though you mandate TDD compliance in the planning workflow.
Proposed fix
## Task Structure (MANDATORY)
-Every implementation task MUST follow this exact 5-step order:
+Every implementation task MUST follow this exact 6-step order:
1. **Step 1: Write the failing test** (include complete test code)
2. **Step 2: Run test to verify it fails** (include exact command + expected failure)
3. **Step 3: Write minimal implementation** (only enough to pass the new test)
4. **Step 4: Run test to verify it passes** (include exact command + expected success)
-5. **Step 5: Commit**
+5. **Step 5: Refactor** (remove duplication, improve names, keep tests green)
+6. **Step 6: Commit**Update the validation at line 102 to check for "exact 6-step TDD order" and update the template to show all 6 steps.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Task Structure (MANDATORY) | |
| Every implementation task MUST follow this exact 5-step order: | |
| 1. **Step 1: Write the failing test** (include complete test code) | |
| 2. **Step 2: Run test to verify it fails** (include exact command + expected failure) | |
| 3. **Step 3: Write minimal implementation** (only enough to pass the new test) | |
| 4. **Step 4: Run test to verify it passes** (include exact command + expected success) | |
| 5. **Step 5: Commit** | |
| ## Task Structure (MANDATORY) | |
| Every implementation task MUST follow this exact 6-step order: | |
| 1. **Step 1: Write the failing test** (include complete test code) | |
| 2. **Step 2: Run test to verify it fails** (include exact command + expected failure) | |
| 3. **Step 3: Write minimal implementation** (only enough to pass the new test) | |
| 4. **Step 4: Run test to verify it passes** (include exact command + expected success) | |
| 5. **Step 5: Refactor** (remove duplication, improve names, keep tests green) | |
| 6. **Step 6: Commit** |
| ## Task Structure Validation (MANDATORY) | ||
|
|
||
| Before saving the plan, verify every implementation task follows the exact 5-step TDD order. | ||
|
|
||
| - If a task starts with implementation work (for example: "add field", "update service", "implement method"), STOP and rewrite it into TDD order. | ||
| - If tests appear after implementation, STOP and rewrite. | ||
| - Do not hand off execution until every implementation task is TDD-compliant. | ||
|
|
||
| Allowed exceptions (must be explicitly justified in the plan): | ||
| - Pure documentation-only tasks | ||
| - Pure configuration-only tasks | ||
| - Schema/migration-only tasks (still require a separate test task to validate behavior) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Compare exception lists between TDD and writing-plans skills
echo "=== TDD skill exceptions ==="
rg -A 3 "Exceptions" skills/test-driven-development/SKILL.md
echo -e "\n=== writing-plans exceptions ==="
rg -A 5 "Allowed exceptions" skills/writing-plans/SKILL.mdRepository: obra/superpowers
Length of output: 453
Major: Exception list diverges from the canonical TDD skill.
The allowed exceptions at lines 108-111 differ significantly from those in skills/test-driven-development/SKILL.md:
TDD skill: Throwaway prototypes, Generated code, Configuration files
writing-plans: Pure documentation-only tasks, Pure configuration-only tasks, Schema/migration-only tasks
The lists have no overlap. This creates confusion about when TDD can legitimately be skipped across the two skills. Align the exception lists or explicitly document why they differ by context.
Additionally, this validation is manual guidance ("STOP and rewrite") rather than automated enforcement, so plans can still violate TDD if not followed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/writing-plans/SKILL.md` around lines 100 - 111, The "Task Structure
Validation (MANDATORY)" section's Allowed exceptions list diverges from the
canonical TDD skill's exceptions, causing inconsistency; update the Allowed
exceptions in this SKILL.md to match the canonical TDD exceptions (e.g., include
Throwaway prototypes, Generated code, Configuration files) or add a brief
explicit rationale explaining why this plan-writing context needs different
exceptions, and also add a note clarifying that the "STOP and rewrite" guidance
is manual (or link to an automated validation task if one exists) so readers
know enforcement is not automatic.
1 participant
Summary
skills/writing-plans/SKILL.mdValidation
python3static content check for required frontmatter and mandatory 5-step TDD sectionsCloses #566