fix(review-pr): anchor findings to diff lines
The gitea posting snippet used the issue-comment endpoint, which has no path or line, so findings named `path:line` in prose instead of landing on the code. Both forges now post one COMMENT review carrying anchored findings in comments[] and loose ones in the body. land's gitea baseline missed review-comment ids, which would replay every code comment on the first hint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+11
-3
@@ -61,9 +61,11 @@ fi
|
|||||||
Gitea equivalent — issue comments plus reviews:
|
Gitea equivalent — issue comments plus reviews:
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
{ curl -sS -H "Authorization: token $GITEA_TOKEN" "$BASE/api/v1/repos/$REPO/issues/$N/comments"; \
|
{ curl -sS -H "Authorization: token $GITEA_TOKEN" "$BASE/api/v1/repos/$REPO/issues/$N/comments" | jq -r '.[]?.id'
|
||||||
curl -sS -H "Authorization: token $GITEA_TOKEN" "$BASE/api/v1/repos/$REPO/pulls/$N/reviews"; } 2>/dev/null \
|
for r in $(curl -sS -H "Authorization: token $GITEA_TOKEN" "$BASE/api/v1/repos/$REPO/pulls/$N/reviews" | jq -r '.[]?.id'); do
|
||||||
| jq -r '.[]?.id' > "$seen" || : > "$seen"
|
echo "$r"
|
||||||
|
curl -sS -H "Authorization: token $GITEA_TOKEN" "$BASE/api/v1/repos/$REPO/pulls/$N/reviews/$r/comments" | jq -r '.[]?.id'
|
||||||
|
done; } 2>/dev/null > "$seen" || : > "$seen"
|
||||||
```
|
```
|
||||||
|
|
||||||
**Request the Copilot review** (github only, once). Its comments then
|
**Request the Copilot review** (github only, once). Its comments then
|
||||||
@@ -111,6 +113,12 @@ and act on what's left:
|
|||||||
echo "$rid" >> "$seen"
|
echo "$rid" >> "$seen"
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Gitea has no reply endpoint, so that lands as a loose PR comment. To
|
||||||
|
answer a code comment inside its own thread, post a review instead
|
||||||
|
whose `comments[]` entry repeats the same `path` and `new_position` —
|
||||||
|
gitea groups code comments by position into one conversation. Record
|
||||||
|
the review id and its comment ids.
|
||||||
|
|
||||||
- **Resolve the thread** (github only — gitea has no per-thread
|
- **Resolve the thread** (github only — gitea has no per-thread
|
||||||
resolve, so a short confirming reply plus the pushed fix is the
|
resolve, so a short confirming reply plus the pushed fix is the
|
||||||
signal):
|
signal):
|
||||||
|
|||||||
+40
-15
@@ -62,43 +62,68 @@ than reviewing hunks in isolation.
|
|||||||
|
|
||||||
**Write the findings** to `<git-dir>/pr-<N>-findings.md` — in the git
|
**Write the findings** to `<git-dir>/pr-<N>-findings.md` — in the git
|
||||||
dir, not the working tree, so nothing lands in the branch under review.
|
dir, not the working tree, so nothing lands in the branch under review.
|
||||||
One finding per entry: `path:line`, what's wrong, what to do. No praise,
|
One finding per entry: `path:line`, what's wrong, what to do — and mark
|
||||||
no summary of what the PR does, no severity theatre. If you find
|
whether it anchors to a diff line or is a loose remark about the change
|
||||||
nothing, say so in one line.
|
as a whole, which decides where it goes in §2. No praise, no summary of
|
||||||
|
what the PR does, no severity theatre. If you find nothing, say so in
|
||||||
|
one line.
|
||||||
|
|
||||||
Then follow the mode: post (gitea) or report the file to the user and
|
Then follow the mode: post (gitea) or report the file to the user and
|
||||||
stop (github).
|
stop (github).
|
||||||
|
|
||||||
## 2. Posting
|
## 2. Posting
|
||||||
|
|
||||||
|
Post **one review** per pass, never a stream of separate comments. A
|
||||||
|
review carries two kinds of finding at once:
|
||||||
|
|
||||||
|
- **Anchored** — the finding is about a specific line in the diff. It
|
||||||
|
belongs in `comments[]` with a `path` and a line, so it renders on
|
||||||
|
the code.
|
||||||
|
- **Loose** — the finding is about the change as a whole, or about code
|
||||||
|
the diff doesn't touch, or it has no single line to sit on. It goes
|
||||||
|
in the review `body`.
|
||||||
|
|
||||||
|
Anchor whatever can be anchored. Writing `path:line` into prose when
|
||||||
|
the API would have put the comment on that line is the failure mode
|
||||||
|
this section exists to prevent.
|
||||||
|
|
||||||
Only after the user's go-ahead on gated repos. Record every id you post
|
Only after the user's go-ahead on gated repos. Record every id you post
|
||||||
in the same step, or the next hint reads your own review as new
|
in the same step, or the next hint reads your own review as new
|
||||||
feedback:
|
feedback:
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# gitea
|
# gitea — body is the loose findings, comments[] the anchored ones
|
||||||
rid=$(curl -sS -X POST -H "Authorization: token $GITEA_TOKEN" -H "Content-Type: application/json" \
|
# new_position = line in the new file; use old_position for a removed line
|
||||||
"$BASE/api/v1/repos/$REPO/issues/$N/comments" -d "$(jq -nc --arg body "<finding>" '{body:$body}')" | jq -r .id)
|
rid=$(jq -nc \
|
||||||
|
--arg body "<loose findings, or empty>" \
|
||||||
|
--argjson comments '[{"path":"path/to/file.ts","new_position":11,"body":"<finding>"}]' \
|
||||||
|
'{event:"COMMENT", body:$body, comments:$comments}' \
|
||||||
|
| curl -sS -X POST -H "Authorization: token $GITEA_TOKEN" -H "Content-Type: application/json" \
|
||||||
|
"$BASE/api/v1/repos/$REPO/pulls/$N/reviews" -d @- | jq -r .id)
|
||||||
echo "$rid" >> "$seen"
|
echo "$rid" >> "$seen"
|
||||||
|
curl -sS -H "Authorization: token $GITEA_TOKEN" \
|
||||||
|
"$BASE/api/v1/repos/$REPO/pulls/$N/reviews/$rid/comments" | jq -r '.[].id' >> "$seen"
|
||||||
```
|
```
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# github, after approval
|
# github, after approval — same shape, `line` instead of new_position
|
||||||
rid=$(gh api repos/<OWNER>/<REPO>/pulls/<N>/comments -f body="<finding>" \
|
jq -nc --arg body "<loose findings, or empty>" \
|
||||||
-f commit_id=<sha> -f path=<path> -F line=<line> --jq .id)
|
--argjson comments '[{"path":"path/to/file.ts","line":11,"body":"<finding>"}]' \
|
||||||
echo "$rid" >> "$seen"
|
'{event:"COMMENT", commit_id:"<sha>", body:$body, comments:$comments}' \
|
||||||
|
| gh api repos/<OWNER>/<REPO>/pulls/<N>/reviews --input - --jq .id >> "$seen"
|
||||||
|
gh api repos/<OWNER>/<REPO>/pulls/<N>/comments --jq '.[].id' >> "$seen"
|
||||||
```
|
```
|
||||||
|
|
||||||
Prefer one review with several comments over a stream of separate
|
`event: "COMMENT"` is the only event either forge should see from you.
|
||||||
comments. **Never approve and never request changes as a review
|
**Never approve and never request changes as a review decision** —
|
||||||
decision** — that's the user's call on someone else's PR, and it carries
|
that's the user's call on someone else's PR, and it carries weight your
|
||||||
weight your findings don't.
|
findings don't.
|
||||||
|
|
||||||
## 3. Handling a hint
|
## 3. Handling a hint
|
||||||
|
|
||||||
| reason | what to do |
|
| reason | what to do |
|
||||||
| --- | --- |
|
| --- | --- |
|
||||||
| `comments` | Read comments not in the seen file. Someone replying to a finding gets an answer; a new comment thread may need a fresh look at that code. Record every id you handle or post. |
|
| `comments` | Read comments not in the seen file. Someone replying to a finding gets an answer; a new comment thread may need a fresh look at that code. Reply in the thread it came from: on github, `POST /pulls/<N>/comments/<cid>/replies`; on gitea there is no reply endpoint, so post a review whose `comments[]` entry carries the same `path` and line — gitea groups code comments by position into one conversation. A loose reply goes to `POST /issues/<N>/comments`. Record every id you handle or post. |
|
||||||
| `ci` | New head SHA: the author pushed. Re-read the diff for the new commits only, and check whether your open findings are addressed. Do not investigate their CI failures — not your PR. |
|
| `ci` | New head SHA: the author pushed. Re-read the diff for the new commits only, and check whether your open findings are addressed. Do not investigate their CI failures — not your PR. |
|
||||||
| `state` | Merged or closed: write the outcome to the state file and stop. Draft flips: nothing to do. |
|
| `state` | Merged or closed: write the outcome to the state file and stop. Draft flips: nothing to do. |
|
||||||
| `conflicts` | Nothing to do. The author resolves conflicts on their own branch. |
|
| `conflicts` | Nothing to do. The author resolves conflicts on their own branch. |
|
||||||
|
|||||||
Reference in New Issue
Block a user