feat(feature): read ERRORS.md in Phase 0 before coding#3604
Conversation
Adds a bullet in Phase 0 step 1 to read ERRORS.md for known bugs and non-obvious pitfalls before starting implementation. Closes #3594.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ 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 |
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Pull request overview
Updates the /feature skill’s Phase 0 (scope analysis) to explicitly read ERRORS.md before any coding, surfacing known pitfalls earlier in the feature implementation workflow.
Changes:
- Adds a new first bullet under “Phase 0 — 1. Identify target module” instructing to read
ERRORS.mdbefore coding.
There was a problem hiding this comment.
Pull Request Overview
The PR successfully introduces a mandatory step to check for known pitfalls in ERRORS.md. Codacy analysis shows the code is up to standards with no new quality issues. However, there is a conflict between the requested acceptance criteria (placing the instruction in Phase 0, Step 1) and logical best practices. Placing the check in Step 3 ('Analyze flows & edge cases') is more appropriate for pitfall analysis. Furthermore, the instruction lacks a conditional existence check, which could cause agents to error out if the file is not present in a repository.
About this PR
- The requirement to read
ERRORS.mdis currently targeted for the very beginning of Phase 0. While this ensures it isn't missed, it breaks the flow of 'Identify target module'. Consider if this instruction serves the user better during the edge-case analysis phase.
Test suggestions
- Verify the 'Read ERRORS.md' instruction is present as the first bullet under the 'Identify target module' section
- Confirm that existing steps regarding module selection and scaffold-running remain intact and in their original relative order
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify the 'Read ERRORS.md' instruction is present as the first bullet under the 'Identify target module' section
2. Confirm that existing steps regarding module selection and scaffold-running remain intact and in their original relative order
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
|
|
||
| ### 1. Identify target module | ||
|
|
||
| - Read `ERRORS.md` for known bugs and non-obvious pitfalls to avoid before coding. |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: This instruction relates to implementation pitfalls rather than module selection, making its placement under 'Identify target module' inconsistent. Additionally, the command should be conditional to ensure the agent does not halt if the file is absent. Try running the following prompt in your coding agent: > Relocate the ERRORS.md check to '3. Analyze flows & edge cases' and rephrase it as: '- If ERRORS.md exists, read it for known bugs and non-obvious pitfalls to avoid before coding.'
Summary
/featureskill to readERRORS.mdbefore codingChanges
Additive only — inserts one bullet under
### 1. Identify target modulein.claude/skills/feature/SKILL.md. No existing behaviour is removed or altered.Closes #3594.
Test plan