From db0cde667dfea5f8514ae1edd7cc9b6362640269 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Thu, 16 Jul 2026 16:13:51 +1000 Subject: [PATCH 1/2] Pin to @v0 and adopt the upstream review-workflow template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes, both aligning this repo with action-translation's own documented conventions. Pin to @v0 on both workflows. The action's docs use @v0 throughout (19 occurrences against a single exact pin) and v0.17.0's notes recommend it directly. Exact pins were the deviation, and they cost us: v0.17.0's typography and review fixes sat unreachable behind nine stale pins while every sync kept stripping French non-breaking spaces. Development tempo is fast enough that manual bumps lag the releases that matter. The safety net is not the pin — sync output lands as a PR a human reviews before merge — and the floating tags are well maintained (v0 tracks v0.17.0; every vN.N tag tracks its latest patch). Adopt the upstream review template verbatim (docs/user/tutorials/ connect-existing.md), replacing the local variant from #7: - Add permissions: contents: read + pull-requests: write. v0.17.0's review dedupe deletes superseded comments, which needs pull-requests: write. Posting works today only because the default token happens to be permissive. - Move concurrency from workflow level to job level and flip cancel-in-progress to true. #7 argued true was unsafe, because an 'automated' labeled event would cancel the in-flight review then skip its own job, leaving none. That reasoning holds at workflow level, which is where #7 put it. At job level the group is entered only after the `if` passes, so a skipped job cancels nothing — and true is then better than false, superseding a stale in-flight review instead of queueing a redundant one. It also matches rebase-translations.yml, which has used job-level permissions + concurrency all along. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/rebase-translations.yml | 2 +- .github/workflows/review-translations.yml | 46 +++++++++++------------ 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/.github/workflows/rebase-translations.yml b/.github/workflows/rebase-translations.yml index 1fc1dd5..37f5daa 100644 --- a/.github/workflows/rebase-translations.yml +++ b/.github/workflows/rebase-translations.yml @@ -34,7 +34,7 @@ jobs: steps: - name: Rebase open translation PRs - uses: QuantEcon/action-translation@v0.16.1 + uses: QuantEcon/action-translation@v0 with: mode: rebase anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }} diff --git a/.github/workflows/review-translations.yml b/.github/workflows/review-translations.yml index 0380800..56abba2 100644 --- a/.github/workflows/review-translations.yml +++ b/.github/workflows/review-translations.yml @@ -1,47 +1,43 @@ # Review Translations — Quality check on translation PRs # When a PR is opened/updated that carries the 'action-translation' label, # this workflow runs a quality review and posts a comment. +# +# Mirrors the upstream template in action-translation +# docs/user/tutorials/connect-existing.md — keep it in step with that. name: Review Translations 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: - # 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: >- + # Ignore `labeled` events for every other label: a sync applies its labels one call at a + # time, and each one would otherwise start a full (billed) review of the same diff. + if: > contains(github.event.pull_request.labels.*.name, 'action-translation') && - (github.event.action != 'labeled' || - github.event.label.name == 'action-translation') + (github.event.action != 'labeled' || github.event.label.name == 'action-translation') runs-on: ubuntu-latest + # v0.17.0's review dedupe deletes superseded comments, which needs pull-requests: write. + permissions: + contents: read + pull-requests: write + + # One review per PR — supersede an in-flight review instead of running both. + # Job-level (not workflow-level) on purpose: the group is entered only after the `if` + # above has passed, so a `labeled` event for 'automated' skips out without cancelling + # the real review. At workflow level it would cancel first and skip second, leaving none. + concurrency: + group: review-translations-${{ github.event.pull_request.number }} + cancel-in-progress: true + steps: - uses: actions/checkout@v6 with: fetch-depth: 2 - - uses: QuantEcon/action-translation@v0.16.1 + - uses: QuantEcon/action-translation@v0 with: mode: review source-repo: QuantEcon/lecture-python-programming From 23c416f6cbaea5a0c558dd14108cc900d929c59e Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Thu, 16 Jul 2026 16:18:03 +1000 Subject: [PATCH 2/2] Correct the label-event comment: one addLabels call, one event per label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot caught this on #10. The comment claimed the sync applies labels 'one call at a time', but pr-creator.ts makes a single addLabels call with an array — it is GitHub that emits one `labeled` event per label, which is what multiplies the runs. Comment only; no behaviour change. The wording came verbatim from the upstream template (connect-existing.md), so the same inaccuracy is there and worth reporting back rather than silently diverging. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/review-translations.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/review-translations.yml b/.github/workflows/review-translations.yml index 56abba2..af37bbf 100644 --- a/.github/workflows/review-translations.yml +++ b/.github/workflows/review-translations.yml @@ -12,8 +12,9 @@ on: jobs: review: - # Ignore `labeled` events for every other label: a sync applies its labels one call at a - # time, and each one would otherwise start a full (billed) review of the same diff. + # Ignore `labeled` events for every other label: a sync adds its labels in a single + # addLabels call, but GitHub emits one `labeled` event per label, and each would + # otherwise start a full (billed) review of the same diff. if: > contains(github.event.pull_request.labels.*.name, 'action-translation') && (github.event.action != 'labeled' || github.event.label.name == 'action-translation')