Skip to content

Commit d45c0bc

Browse files
authored
fix: ignore superseded top-level review feedback (#120)
1 parent a64920b commit d45c0bc

3 files changed

Lines changed: 42 additions & 4 deletions

File tree

.github/scripts/check-pr-review-threads.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ def unresolved_threads(payload, min_severity: "high", include_outdated: false)
7777
def top_level_feedback(payload, min_severity: "high")
7878
threshold = SEVERITY_RANK.fetch(min_severity)
7979
pull_request = payload.dig("data", "repository", "pullRequest") || {}
80+
current_head_oid = pull_request["headRefOid"].to_s
8081
feedback = []
8182
Array(pull_request.dig("comments", "nodes")).each do |comment|
8283
next if informational_summary?(comment["body"], author: comment.dig("author", "login"))
@@ -98,6 +99,9 @@ def top_level_feedback(payload, min_severity: "high")
9899
detected = severity(review["body"])
99100
next if SEVERITY_RANK.fetch(detected) < threshold
100101

102+
review_commit_oid = review.dig("commit", "oid").to_s
103+
next if !current_head_oid.empty? && !review_commit_oid.empty? && review_commit_oid != current_head_oid
104+
101105
feedback << {
102106
kind: "pr_review",
103107
severity: detected,
@@ -148,6 +152,7 @@ def graphql_query
148152
query($owner:String!,$repo:String!,$number:Int!) {
149153
repository(owner:$owner, name:$repo) {
150154
pullRequest(number:$number) {
155+
headRefOid
151156
comments(first:100) {
152157
pageInfo {
153158
hasNextPage
@@ -171,6 +176,9 @@ def graphql_query
171176
login
172177
}
173178
body
179+
commit {
180+
oid
181+
}
174182
state
175183
url
176184
}
@@ -239,6 +247,9 @@ def reviews_page_query
239247
login
240248
}
241249
body
250+
commit {
251+
oid
252+
}
242253
state
243254
url
244255
}

.github/workflows/review-thread-guard.yml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ on:
2121
description: "evalops/.github ref to checkout for guard scripts"
2222
required: false
2323
type: string
24-
default: c02a97ba9b92c6b2ac837aab77dc3becb77f301c
2524
settle_seconds:
2625
description: "Seconds to wait before checking review feedback so bot reviews can finish before auto-merge"
2726
required: false
@@ -50,7 +49,6 @@ on:
5049
description: "evalops/.github ref to checkout for guard scripts"
5150
required: false
5251
type: string
53-
default: c02a97ba9b92c6b2ac837aab77dc3becb77f301c
5452
settle_seconds:
5553
description: "Seconds to wait before checking review feedback so bot reviews can finish before auto-merge"
5654
required: false
@@ -69,7 +67,7 @@ jobs:
6967
- uses: actions/checkout@v5
7068
with:
7169
repository: evalops/.github
72-
ref: ${{ inputs.guard_ref || 'c02a97ba9b92c6b2ac837aab77dc3becb77f301c' }}
70+
ref: ${{ inputs.guard_ref || github.workflow_sha }}
7371
path: evalops-github
7472

7573
- name: Let asynchronous review bots settle

test/check_pr_review_threads_test.rb

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,11 @@ def test_detects_top_level_pr_comment_severity_markers
8484

8585
def test_detects_top_level_review_body_severity_markers
8686
payload = payload_with(
87+
head_ref_oid: "head-sha",
8788
reviews: [
8889
{
8990
"author" => { "login" => "reviewer" },
91+
"commit" => { "oid" => "head-sha" },
9092
"state" => "COMMENTED",
9193
"body" => "**P1 Badge** paired public PR feedback is missing",
9294
"url" => "https://github.com/evalops/example/pull/1#pullrequestreview-1"
@@ -100,6 +102,32 @@ def test_detects_top_level_review_body_severity_markers
100102
assert_equal "p1", feedback.first.fetch(:severity)
101103
end
102104

105+
def test_skips_top_level_review_feedback_from_superseded_heads
106+
payload = payload_with(
107+
head_ref_oid: "new-head",
108+
reviews: [
109+
{
110+
"author" => { "login" => "chatgpt-codex-connector[bot]" },
111+
"commit" => { "oid" => "old-head" },
112+
"state" => "COMMENTED",
113+
"body" => "**P1 Badge** stale feedback already fixed on the latest head",
114+
"url" => "https://github.com/evalops/example/pull/1#pullrequestreview-1"
115+
},
116+
{
117+
"author" => { "login" => "reviewer" },
118+
"commit" => { "oid" => "new-head" },
119+
"state" => "COMMENTED",
120+
"body" => "**High Severity** latest-head feedback still blocks",
121+
"url" => "https://github.com/evalops/example/pull/1#pullrequestreview-2"
122+
}
123+
]
124+
)
125+
126+
feedback = EvalOpsReviewThreadGuard.blocking_feedback(payload, min_severity: "high")
127+
128+
assert_equal ["https://github.com/evalops/example/pull/1#pullrequestreview-2"], feedback.map { |item| item.fetch(:url) }
129+
end
130+
103131
def test_skips_informational_bot_pr_summaries
104132
payload = payload_with(
105133
comments: [
@@ -238,11 +266,12 @@ def test_fetch_connection_tail_uses_connection_specific_cursor
238266

239267
private
240268

241-
def payload_with(comments: [], reviews: [], threads: [])
269+
def payload_with(comments: [], reviews: [], threads: [], head_ref_oid: nil)
242270
{
243271
"data" => {
244272
"repository" => {
245273
"pullRequest" => {
274+
"headRefOid" => head_ref_oid,
246275
"comments" => { "nodes" => comments },
247276
"reviews" => { "nodes" => reviews },
248277
"reviewThreads" => { "nodes" => threads }

0 commit comments

Comments
 (0)