fix(skill): strengthen batch-fix enforcement in deft-review-cycle (#250)#253
Conversation
Greptile SummaryThis PR adds batch-fix enforcement to Confidence Score: 5/5Safe to merge — all changes are documentation, skill instructions, and corresponding tests with no architectural or runtime risk. No P0 or P1 findings. The single P2 comment is a wording precision issue between the pre-commit gate (P0/P1 scope) and the anti-pattern (implied all-severity scope), which could cause agent confusion but does not break any current behavior. All tests verify the new SKILL.md rules accurately, SPECIFICATION.md and CHANGELOG.md are correctly updated. No files require special attention.
|
| Filename | Overview |
|---|---|
| skills/deft-review-cycle/SKILL.md | Added pre-commit gate requiring full Greptile review re-read before committing, plus two anti-patterns against partial fix commits — consistent with surrounding RFC2119 conventions and the existing exit condition logic. |
| tests/content/test_skills.py | Section 26 adds three assertion tests for the new SKILL.md rules; all three string checks case-insensitively match the exact phrasing in the updated skill file and will pass. |
| SPECIFICATION.md | Added t1.12.2 task entry marked [completed] with accurate acceptance criteria matching the SKILL.md changes. |
| CHANGELOG.md | New [Unreleased] Fixed entry added for the batch-fix enforcement change — follows existing changelog conventions. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: skills/deft-review-cycle/SKILL.md
Line: 87
Comment:
**Anti-pattern severity scope is broader than pre-commit gate**
The pre-commit gate (line 87) gates only on "all P0/P1 issues are addressed," but the anti-pattern at line 229 says "if Greptile flags 3 issues, **all 3** must be fixed in one commit before pushing" — with no severity qualifier. Step 6 explicitly states P2 is non-blocking, so an agent following the anti-pattern strictly would feel required to fix P2 findings before pushing even when the exit condition doesn't require it. Consider aligning the anti-pattern to match the gate's P0/P1 scope:
```
⊗ Push a fix commit that leaves any P0 or P1 finding unaddressed — if the current review surfaces P0/P1 issues, all must be fixed in one commit before pushing (P2-only findings may be deferred)
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (3): Last reviewed commit: "fix(skill): strengthen batch-fix enforce..." | Re-trigger Greptile
1e816fd to
79178b1
Compare
- Add ! pre-commit gate to Phase 2 Step 3: re-read FULL Greptile review and confirm all P0/P1 issues addressed in staged changes before committing - Add anti-pattern: push a fix commit addressing fewer findings than the current review surfaces (if 3 issues flagged, all 3 fixed in one commit) - Add anti-pattern: push after fixing a P1 without checking for additional P1/P2 findings in the same review - Add spec task t1.12.2 to SPECIFICATION.md with [completed] status - Add CHANGELOG entry under [Unreleased] Closes #250
79178b1 to
82ef2aa
Compare
Summary
Strengthen batch-fix enforcement in
skills/deft-review-cycle/SKILL.mdto prevent agents from pushing one fix commit per Greptile finding instead of batching all findings into a single commit.Root cause: agents push one fix commit per Greptile finding, causing N re-review cycles instead of 1.
Changes
!rule requiring agents to re-read the FULL current Greptile review and confirm all P0/P1 issues are addressed in staged changes before committing[completed]status[Unreleased]Related Issues
Closes #250
Checklist
/deft:change <name>-- N/A (3 files changed but all are doc/skill updates with no architectural risk)CHANGELOG.md-- added entry under[Unreleased]ROADMAP.md-- N/A (release-time update per convention)Post-Merge
masterrequiring CI status check (one-time setup, see Add GitHub Actions CI workflow for linting and tests on PRs and pushes #57)