-
Notifications
You must be signed in to change notification settings - Fork 114
fix(ci): prevent suggestion over-application and duplicate comments #1557
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -103,8 +103,28 @@ gh api "repos/{owner}/{repo}/actions/runs?branch=main&status=completed&per_page= | |
| If you cannot verify, say "I haven't confirmed whether these failures are | ||
| pre-existing." | ||
|
|
||
| ## Applying GitHub Suggestions | ||
|
|
||
| When a reviewer posts a GitHub suggestion (`suggestion` code block), apply it | ||
| with exact scope — change only the lines the suggestion covers. Do not | ||
| reinterpret the suggestion's intent or extend it to adjacent lines. | ||
|
|
||
| If the suggestion seems incomplete or you think more lines should change, apply | ||
| the literal suggestion first and note the potential improvement in your reply. | ||
|
|
||
| ## Replying to Comments | ||
|
|
||
| Before posting a comment, check for recent bot comments on the same | ||
| PR/issue to avoid duplicates from concurrent runs: | ||
|
|
||
| ```bash | ||
| gh api repos/{owner}/{repo}/issues/{number}/comments \ | ||
| --jq '.[] | select(.user.login == "{bot_login}") | {id, created_at, body: (.body | .[0:100])}' | ||
| ``` | ||
|
|
||
| If you already commented on the same topic within the last 5 minutes, do not | ||
| post again. | ||
|
Comment on lines
+117
to
+126
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to what extent does this duplicate existing guidance?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The review-pr skill has dedup logic, but it's specific to flaky test issue comments (editing an existing bot comment on a tracking issue). This one targets a different scenario: concurrent That said, if you think the pattern is general enough that skills should just know to check, happy to drop this and rely on convention. |
||
|
|
||
| Reply in context rather than creating new top-level comments: | ||
|
|
||
| - **Inline review comments** (`#discussion_r`): Reply in the review thread: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issues API returns comments oldest-first by default. Without
--paginate, only the first 30 are returned — on a busy PR, a bot comment from 2 minutes ago won't appear. Consider fetching only the tail: