Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/rebase-translations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
46 changes: 21 additions & 25 deletions .github/workflows/review-translations.yml
Original file line number Diff line number Diff line change
@@ -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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 23c416f — correct catch, and the comment now states the real mechanism: a single addLabels call with an array of labels, with GitHub emitting one labeled event per label. Verified against pr-creator.ts:198, which does exactly that.

Worth noting the wording was copied verbatim from the upstream template (action-translation docs/user/tutorials/connect-existing.md:411-412), so the same inaccuracy is upstream — reporting it back rather than letting this repo silently diverge.

One thing your comment sharpens but does not settle: on the sync that prompted all this (#6), GitHub recorded four labeled events for two labels, while the action logged a single successful addLabels call and no retry. So one-event-per-label is right, but something applied the labels twice. Still unexplained, and tracked upstream alongside QuantEcon/action-translation#92 — the retry loop wrapping that call is precisely the non-idempotent pattern #92 warns about.

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
Expand Down