rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

fix: supersede ALL old reviews, not just the most recent #43

Merged
rodin merged 2 commits from fix/supersede-all-old-reviews into main 2026-05-02 20:35:23 +00:00
Conversation 6 Commits 2 Files Changed 3
+12 -7
2 changed files with 12 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit a0fd882b0d - Show all commits
cmd/review-bot/main.go
+8 -6
View File
@@ -320,17 +320,15 @@ func main() {
// 2. Then supersede old review with link to the new one
// Order matters: post first so we have the new review's URL for the supersede message.
var oldReviews []gitea.Review
var sharedTokenMode bool
if *reviewerName != "" {
existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
if err != nil {
slog.Warn("could not list existing reviews", "pr", prNumber, "error", err)
} else {
sonnet-review-bot commented 2026-05-02 20:29:35 +00:00
Outdated
Review
Copy Link
Copy Source

[NIT] sharedTokenMode is only used to gate collection of oldReviews and to log a warning; the variable itself isn't used later. You could simplify by inlining the condition or removing the variable to reduce local state.

**[NIT]** sharedTokenMode is only used to gate collection of oldReviews and to log a warning; the variable itself isn't used later. You could simplify by inlining the condition or removing the variable to reduce local state.
sharedTokenMode = hasSharedToken(existingReviews, sentinel)
if !sharedTokenMode {
oldReviews = findAllOwnReviews(existingReviews, sentinel)
} else {
if hasSharedToken(existingReviews, sentinel) {
slog.Warn("shared token mode: skipping supersede to avoid clobbering sibling review")
} else {
oldReviews = findAllOwnReviews(existingReviews, sentinel)
}
}
}
@@ -378,13 +376,14 @@ func main() {
slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err)
continue
}
resolved := 0
resolved, failed := 0, 0
for _, c := range oldComments {
if c.ID == 0 {
continue
}
if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err)
sonnet-review-bot commented 2026-05-02 20:29:35 +00:00
Outdated
Review
Copy Link
Copy Source

[NIT] When resolving old inline comments, failures are only logged at debug level per comment and no summary of failures is emitted. Consider aggregating and logging a warning if some resolutions failed to aid observability.

**[NIT]** When resolving old inline comments, failures are only logged at debug level per comment and no summary of failures is emitted. Consider aggregating and logging a warning if some resolutions failed to aid observability.
failed++
} else {
resolved++
}
@@ -392,6 +391,9 @@ func main() {
if resolved > 0 {
slog.Info("resolved old inline comments", "review_id", oldReview.ID, "count", resolved, "pr", prNumber)
}
if failed > 0 {
slog.Warn("some inline comments could not be resolved", "review_id", oldReview.ID, "failed", failed, "pr", prNumber)
}
}
}
1
gitea/client.go
+4 -1
View File
1
 @@ -442,6 +442,9 @@ func (c *Client) GetTimelineReviewCommentIDForReview(ctx context.Context, owner,
}
var review struct {
Body string `json:"body"`
User struct {
Login string `json:"login"`
} `json:"user"`
}
if err := json.Unmarshal(body, &review); err != nil {
return 0, fmt.Errorf("parse review %d: %w", reviewID, err)
3
 @@ -474,7 +477,7 @@ func (c *Client) GetTimelineReviewCommentIDForReview(ctx context.Context, owner,
return 0, fmt.Errorf("parse timeline (page %d): %w", page, err)
}
for _, ev := range events {
if ev.Type == "review" && strings.HasPrefix(ev.Body, matchPrefix) {
if ev.Type == "review" && ev.User.Login == review.User.Login && strings.HasPrefix(ev.Body, matchPrefix) {
return ev.ID, nil
}
}