Skip to content

Commit 37e78f2

Browse files
mmckyclaude
andauthored
Serialise translation reviews to stop duplicate report comments (#134)
Ports the fix from QuantEcon/lecture-python-programming.fr#7. This repo has the identical trigger list and no concurrency guard, and the bug is already reproducing here: PR #133 carries two "Translation Quality Review" comments, scoring 9.2/10 and 8.8/10, both posted at 23:51:37. 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 #133 three runs fired; two logged `Posted review comment` and one logged `Updated existing review comment`, leaving two independent reports of the same diff and three 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 77708bd commit 37e78f2

1 file changed

Lines changed: 26 additions & 1 deletion

File tree

.github/workflows/review-translations.yml

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,34 @@ 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+
# See QuantEcon/lecture-python-programming.fr#6, which collected two review
18+
# comments this way, and QuantEcon/action-translation#96 for the upstream bug.
19+
#
20+
# cancel-in-progress is deliberately false. The labels are applied in one API
21+
# call, so event ordering is not guaranteed; if 'automated' arrived last it would
22+
# cancel the in-flight review for 'action-translation' and then skip its own job
23+
# via the filter below, leaving no review at all. Queuing instead means the first
24+
# run creates the comment and any later run updates it — one comment, always.
25+
concurrency:
26+
group: review-translations-${{ github.event.pull_request.number }}
27+
cancel-in-progress: false
28+
1029
jobs:
1130
review:
12-
if: contains(github.event.pull_request.labels.*.name, 'action-translation')
31+
# Require the 'action-translation' label, and — for `labeled` events — ignore
32+
# labels other than that one. Without the second clause the 'automated' label
33+
# fires a second, redundant review of the identical diff.
34+
if: >-
35+
contains(github.event.pull_request.labels.*.name, 'action-translation') &&
36+
(github.event.action != 'labeled' ||
37+
github.event.label.name == 'action-translation')
1338
runs-on: ubuntu-latest
1439

1540
steps:

0 commit comments

Comments
 (0)