Skip to content

Commit c5d62b3

Browse files
fix(review-pr): add verification checklist for multi-line suggestions (#1608)
## Summary - Replaces the vague "minimize the range" instruction for multi-line suggestions with a concrete 4-step verification checklist - Addresses the root cause of #1607: the bot constructs suggestion ranges that are wider than the replacement text, silently deleting correct code when applied ## Root cause analysis Investigated all three known cases: 1. **#1310** (PR #1309): `start_line: 18, line: 22` — 5-line range replaced with fewer lines, deleting the `git-wt` fallback branch 2. **#1326** (PR #1322): `start_line: 289, line: 291` — range included a closing ` ``` ` fence that GitHub consumed as the suggestion's own delimiter 3. **#1598**: `start_line: 93, line: 127` — 35-line range replaced with a shorter rewrite, deleting the `gh issue view` code block Common thread: the bot sets a line range covering more content than it intends to replace. The replacement text is shorter, so the surplus lines are silently deleted. The existing instruction ("minimize the range") didn't give the bot a way to verify correctness. ## Fix The new checklist requires the bot to: 1. **Read the exact lines** in the range before posting 2. **Verify no silent deletions** — every line must appear in the replacement or be intentionally removed 3. **Cap range at ~10 lines** — larger suggestions should be split or pushed as commits 4. **Avoid spanning markdown fences** — prevents delimiter collision ## Test plan - [ ] Review a PR with a multi-line suggestion opportunity and verify the bot follows the checklist - [ ] Verify the bot splits or commits changes larger than ~10 lines instead of using a suggestion 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent eec328c commit c5d62b3

1 file changed

Lines changed: 16 additions & 2 deletions

File tree

.claude/skills/review-pr/SKILL.md

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,22 @@ array indices to object keys, which GitHub rejects.
276276
- If a review has both suggestions and prose observations, put the suggestions
277277
as inline comments and the prose in the review body.
278278
- Multi-line suggestions: set `start_line` and `line` to define the range.
279-
**Minimize the range** — only include lines that actually need changing. A
280-
range that's too wide can delete correct code adjacent to the fix.
279+
GitHub **replaces** every line in that range with the suggestion content — any
280+
line in the range that isn't reproduced in the replacement is **deleted**.
281+
282+
**Before posting any multi-line suggestion, verify it:**
283+
284+
1. **Read the exact lines** `start_line` through `line` from the diff hunk.
285+
2. **Diff mentally**: every line in that range must either appear (possibly
286+
modified) in the replacement text, or be a line you intend to delete. If
287+
any line would be silently dropped, **shrink the range** or include the
288+
line in the replacement.
289+
3. **Cap the range at ~10 lines.** Larger suggestions are error-prone and hard
290+
to review. For changes spanning more than 10 lines, split into multiple
291+
suggestions or push a fix commit instead.
292+
4. **Never span markdown fences.** If the range includes a `` ``` `` line,
293+
GitHub's suggestion parser may consume it as a delimiter, corrupting the
294+
result. Either shrink the range to avoid the fence or push a commit.
281295

282296
### 5. Monitor CI
283297

0 commit comments

Comments
 (0)