From 1c2292265b7d95f60e0045cc16d6546103552b4f Mon Sep 17 00:00:00 2001 From: Rodin Date: Fri, 1 May 2026 22:17:36 -0700 Subject: [PATCH] feat: skip re-posting when review is unchanged (preserve threads) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before posting, compare the new review body+event against the existing review with the same sentinel. If identical, skip entirely — this preserves conversation threads on inline comments and avoids re-notifying reviewers for findings they already know about. Only re-posts when findings actually change (fixed, new, or different). Tests: 6 cases covering identical, different body, different state, stale reviews, and different sentinels. --- cmd/review-bot/main.go | 29 ++++++++++++++ cmd/review-bot/main_test.go | 79 +++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index ac1a6d1..55b6281 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -257,6 +257,15 @@ func main() { log.Printf("Attaching %d inline comments", len(inlineComments)) } + // Check if existing review is unchanged — skip to preserve conversation threads + if *updateExisting && *reviewerName != "" { + existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber) + if err == nil && reviewUnchanged(existingReviews, reviewBody, event, sentinel) { + log.Printf("Review unchanged from previous run; skipping to preserve threads") + return + } + } + log.Printf("Posting review (event=%s)...", event) posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, inlineComments) if err != nil { @@ -465,3 +474,23 @@ func shouldEscalate(reviews []gitea.Review, postedID int64, postedLogin, ownSent } return false } + +// reviewUnchanged checks if an existing review with the same sentinel +// already has identical body and state. Returns true if a re-post would +// produce the same result (skip to preserve conversation threads). +func reviewUnchanged(reviews []gitea.Review, newBody, newEvent, sentinel string) bool { + for _, r := range reviews { + if r.Stale { + continue + } + if !strings.Contains(r.Body, sentinel) { + continue + } + // Compare state (map APPROVED back from Gitea's representation) + existingEvent := r.State + if existingEvent == r.State && existingEvent == newEvent && r.Body == newBody { + return true + } + } + return false +} diff --git a/cmd/review-bot/main_test.go b/cmd/review-bot/main_test.go index 38304c5..11ff8e2 100644 --- a/cmd/review-bot/main_test.go +++ b/cmd/review-bot/main_test.go @@ -148,3 +148,82 @@ func TestShouldEscalate(t *testing.T) { }) } } + +func TestReviewUnchanged(t *testing.T) { + tests := []struct { + name string + existing []gitea.Review + newBody string + newEvent string + sentinel string + want bool + }{ + { + name: "no existing review", + existing: nil, + newBody: "new review", + newEvent: "APPROVED", + sentinel: "", + want: false, + }, + { + name: "identical body and state", + existing: []gitea.Review{ + makeReview(100, "bot", "APPROVED", false, "same body\n"), + }, + newBody: "same body\n", + newEvent: "APPROVED", + sentinel: "", + want: true, + }, + { + name: "same body but different state", + existing: []gitea.Review{ + makeReview(100, "bot", "APPROVED", false, "body\n"), + }, + newBody: "body\n", + newEvent: "REQUEST_CHANGES", + sentinel: "", + want: false, + }, + { + name: "different body same state", + existing: []gitea.Review{ + makeReview(100, "bot", "APPROVED", false, "old body\n"), + }, + newBody: "new body\n", + newEvent: "APPROVED", + sentinel: "", + want: false, + }, + { + name: "stale review with same body (should still post)", + existing: []gitea.Review{ + makeReview(100, "bot", "APPROVED", true, "same\n"), + }, + newBody: "same\n", + newEvent: "APPROVED", + sentinel: "", + want: false, + }, + { + name: "different sentinel (not our review)", + existing: []gitea.Review{ + makeReview(100, "bot", "APPROVED", false, "body\n"), + }, + newBody: "body\n", + newEvent: "APPROVED", + sentinel: "", + want: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := reviewUnchanged(tc.existing, tc.newBody, tc.newEvent, tc.sentinel) + if got != tc.want { + t.Errorf("reviewUnchanged() = %v, want %v", got, tc.want) + } + }) + } +}