Skip to content

Commit b9ab741

Browse files
mmckyclaude
andauthored
Serialise translation reviews to stop duplicate report comments (#7)
A single translation sync fires `opened` plus one `labeled` event per label applied, all within a couple of seconds. Each starts its own run of this workflow, and the review action's "update existing comment, else create" logic is a check-then-act with no lock: concurrent runs all see "no comment yet" and each create one. On PR #6 that produced five concurrent runs and two independent review comments with different scores (9.2 and 9.0), plus five full model calls where one was intended. Add a per-PR concurrency group so runs queue instead of racing — the first creates the comment, any later run updates it. cancel-in-progress is left false on purpose: the labels are applied in one API call so event order is not guaranteed, and cancelling would let a `labeled` event for 'automated' kill the in-flight review and then skip its own job, leaving no review at all. Also ignore `labeled` events for labels other than 'action-translation', which removes the redundant review triggered by the 'automated' label. The underlying unsynchronised upsert is tracked in QuantEcon/action-translation#96. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 819f221 commit b9ab741

1 file changed

Lines changed: 25 additions & 1 deletion

File tree

.github/workflows/review-translations.yml

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,33 @@ on:
77
pull_request:
88
types: [opened, synchronize, labeled, reopened]
99

10+
# Serialise reviews per PR.
11+
#
12+
# The sync action creates the PR and then applies its labels in a separate call,
13+
# so a single sync fires `opened` plus one `labeled` event per label, all within
14+
# a couple of seconds. Every one of those starts a full review, and the action's
15+
# "update the existing comment, else create one" logic is a check-then-act with
16+
# no lock — concurrent runs all observe "no comment yet" and each create one.
17+
# That is how PR #6 collected two independent review comments.
18+
#
19+
# cancel-in-progress is deliberately false. The labels are applied in one API
20+
# call, so event ordering is not guaranteed; if 'automated' arrived last it would
21+
# cancel the in-flight review for 'action-translation' and then skip its own job
22+
# via the filter below, leaving no review at all. Queuing instead means the first
23+
# run creates the comment and any later run updates it — one comment, always.
24+
concurrency:
25+
group: review-translations-${{ github.event.pull_request.number }}
26+
cancel-in-progress: false
27+
1028
jobs:
1129
review:
12-
if: contains(github.event.pull_request.labels.*.name, 'action-translation')
30+
# Require the 'action-translation' label, and — for `labeled` events — ignore
31+
# labels other than that one. Without the second clause the 'automated' label
32+
# fires a second, redundant review of the identical diff.
33+
if: >-
34+
contains(github.event.pull_request.labels.*.name, 'action-translation') &&
35+
(github.event.action != 'labeled' ||
36+
github.event.label.name == 'action-translation')
1337
runs-on: ubuntu-latest
1438

1539
steps:

0 commit comments

Comments
 (0)