From 3db563a14467b5ae8eced093b867a37af4debeb2 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Thu, 16 Jul 2026 10:07:09 +1000 Subject: [PATCH] Serialise translation reviews to stop duplicate report comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .github/workflows/review-translations.yml | 26 ++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/.github/workflows/review-translations.yml b/.github/workflows/review-translations.yml index f4b6eb0..0380800 100644 --- a/.github/workflows/review-translations.yml +++ b/.github/workflows/review-translations.yml @@ -7,9 +7,33 @@ on: pull_request: types: [opened, synchronize, labeled, reopened] +# Serialise reviews per PR. +# +# The sync action creates the PR and then applies its labels in a separate call, +# so a single sync fires `opened` plus one `labeled` event per label, all within +# a couple of seconds. Every one of those starts a full review, and the action's +# "update the existing comment, else create one" logic is a check-then-act with +# no lock — concurrent runs all observe "no comment yet" and each create one. +# That is how PR #6 collected two independent review comments. +# +# cancel-in-progress is deliberately false. The labels are applied in one API +# call, so event ordering is not guaranteed; if 'automated' arrived last it would +# cancel the in-flight review for 'action-translation' and then skip its own job +# via the filter below, leaving no review at all. Queuing instead means the first +# run creates the comment and any later run updates it — one comment, always. +concurrency: + group: review-translations-${{ github.event.pull_request.number }} + cancel-in-progress: false + jobs: review: - if: contains(github.event.pull_request.labels.*.name, 'action-translation') + # Require the 'action-translation' label, and — for `labeled` events — ignore + # labels other than that one. Without the second clause the 'automated' label + # fires a second, redundant review of the identical diff. + if: >- + contains(github.event.pull_request.labels.*.name, 'action-translation') && + (github.event.action != 'labeled' || + github.event.label.name == 'action-translation') runs-on: ubuntu-latest steps: