Skip to content

Commit 3b460fb

Browse files
mmckyclaude
andauthored
Migrate translation workflows to @v0, adopt the review template, enable -W (#137)
* Migrate translation workflows to @v0, adopt the review template, enable -W Moves review and rebase off the exact `@v0.16.1` pin onto the floating `@v0` tag, per the pin policy settled in QuantEcon/project-translation#9, and aligns the review workflow with the upstream template in action-translation docs/user/tutorials/connect-existing.md. Two substantive changes beyond the pin: **Concurrency moves from workflow level to job level**, and `cancel-in-progress` from false to true. The previous comment argued that job-level `true` was unsafe because an 'automated' label event would cancel the in-flight review for 'action-translation' and then skip its own job via the filter, leaving no review at all. That was reasoned from the GitHub docs and never tested, and production disproved it: across roughly 15 live opportunities on lecture-intro.zh-cn the group was entered only after the `if` passed, the 'automated' event skipped without cancelling, and a review was posted every time. The stale rationale is replaced rather than left in place. **A `permissions` block is added** — v0.17.0's review dedupe deletes superseded comments, which requires `pull-requests: write`. The pin was two releases behind, which matters here: v0.18.0 carries the fix for a review-mode defect where a model response missing a criterion score became NaN and rendered as an automatic FAIL on otherwise clean PRs. See QuantEcon/action-translation#102. Separately, `ci.yml` gains `-W` on the notebook build. This repo's publish.yml already builds strict, so the PR gate was weaker than the publish gate — a corrupted sync could merge green and then break the published site. Coverage is 26 of 26 lectures with a `_toc.yml` identical to source, so there are no untranslated-lecture references for the flag to trip on. The strict build is the last line of defence against the silent-corruption class in QuantEcon/action-translation#118 and #119, both of which surfaced only because a downstream repo built with -n -W. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Rebase: also match resync/* branches (action-translation v0.18.1) The rebase workflow only fired for `translation-sync-*` branches, which the Action's sync mode creates. The CLI's `translate forward --github` creates `resync/{stem}` branches, so merging one resync PR never rebased its siblings — during a drift-recovery wave that leaves a stack of open PRs whose bases go stale with every merge. Fixed engine-side in action-translation v0.18.1, but the action-side half is not sufficient on its own: this `if` gates whether the job runs at all, and it runs before the action does. Both layers must list both prefixes or the result is a job that never starts. Brings this file in step with the upstream template as of that release. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 444349c commit 3b460fb

3 files changed

Lines changed: 42 additions & 32 deletions

File tree

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ jobs:
6060
- name: Build HTML
6161
shell: bash -l {0}
6262
run: |
63-
jb build lectures --path-output ./ -n --keep-going
63+
jb build lectures --path-output ./ -n -W --keep-going
6464
- name: Upload Execution Reports
6565
uses: actions/upload-artifact@v7
6666
if: failure()

.github/workflows/rebase-translations.yml

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
# Rebase Translation PRs
22
#
33
# Install this workflow in the TARGET (translated) repository.
4-
# When a translation-sync PR is merged, this workflow automatically
5-
# rebases other open translation-sync PRs against the updated main branch.
4+
# When a translation PR is merged, this workflow automatically rebases the
5+
# other open translation PRs against the updated main branch. It covers both
6+
# kinds this tool creates: `translation-sync-*` branches from the Action's sync
7+
# mode, and `resync/*` branches from the CLI's `forward --github`.
68
#
79
# This eliminates merge conflicts caused by multiple upstream PRs
810
# modifying the same files. See: https://github.com/QuantEcon/action-translation/issues/63
@@ -17,10 +19,16 @@ on:
1719

1820
jobs:
1921
rebase:
20-
# Only run when a translation-sync PR is merged
22+
# Only run when a translation PR is merged. Both prefixes must be listed:
23+
# sync mode creates `translation-sync-*`, while the CLI's `forward --github`
24+
# creates `resync/*`, and a wave of resync PRs goes stale the same way.
25+
# Keep this in step with `isTranslationBranch` in the action's src/branch-naming.ts
26+
# — this `if` decides whether the job runs, that predicate decides which open PRs
27+
# it then rebases, so a prefix matching only one of them is a no-op run.
2128
if: >
2229
github.event.pull_request.merged == true &&
23-
startsWith(github.event.pull_request.head.ref, 'translation-sync-')
30+
(startsWith(github.event.pull_request.head.ref, 'translation-sync-') ||
31+
startsWith(github.event.pull_request.head.ref, 'resync/'))
2432
runs-on: ubuntu-latest
2533

2634
permissions:
@@ -34,7 +42,7 @@ jobs:
3442

3543
steps:
3644
- name: Rebase open translation PRs
37-
uses: QuantEcon/action-translation@v0.16.1
45+
uses: QuantEcon/action-translation@v0
3846
with:
3947
mode: rebase
4048
anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }}

.github/workflows/review-translations.yml

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,50 @@
11
# Review Translations — Quality check on translation PRs
22
# When a PR is opened/updated that carries the 'action-translation' label,
33
# this workflow runs a quality review and posts a comment.
4+
#
5+
# Mirrors the upstream template in action-translation
6+
# docs/user/tutorials/connect-existing.md — keep it in step with that.
47
name: Review Translations
58

69
on:
710
pull_request:
811
types: [opened, synchronize, labeled, reopened]
912

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-
2913
jobs:
3014
review:
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: >-
15+
# Ignore `labeled` events for every other label: a sync adds its labels in a single
16+
# addLabels call, but GitHub emits one `labeled` event per label, and each would
17+
# otherwise start a full (billed) review of the same diff.
18+
if: >
3519
contains(github.event.pull_request.labels.*.name, 'action-translation') &&
36-
(github.event.action != 'labeled' ||
37-
github.event.label.name == 'action-translation')
20+
(github.event.action != 'labeled' || github.event.label.name == 'action-translation')
3821
runs-on: ubuntu-latest
3922

23+
# v0.17.0's review dedupe deletes superseded comments, which needs pull-requests: write.
24+
permissions:
25+
contents: read
26+
pull-requests: write
27+
28+
# One review per PR — supersede an in-flight review instead of running both.
29+
# Job-level (not workflow-level) on purpose: the group is entered only after the `if`
30+
# above has passed, so a `labeled` event for 'automated' skips out without cancelling
31+
# the real review. At workflow level it would cancel first and skip second, leaving none.
32+
#
33+
# This supersedes the workflow-level `cancel-in-progress: false` previously used here,
34+
# whose rationale — that an 'automated' label event would cancel the real review and
35+
# then skip its own job, leaving none — was reasoned from the docs and never tested.
36+
# Production disproved it: across ~15 live opportunities on lecture-intro.zh-cn the
37+
# 'automated' event skipped without cancelling and a review was posted every time.
38+
concurrency:
39+
group: review-translations-${{ github.event.pull_request.number }}
40+
cancel-in-progress: true
41+
4042
steps:
4143
- uses: actions/checkout@v7
4244
with:
4345
fetch-depth: 2
4446

45-
- uses: QuantEcon/action-translation@v0.16.1
47+
- uses: QuantEcon/action-translation@v0
4648
with:
4749
mode: review
4850
source-repo: QuantEcon/lecture-python-programming

0 commit comments

Comments
 (0)