Skip to content

Pin to @v0 and adopt the upstream review-workflow template#10

Merged
mmcky merged 2 commits into
mainfrom
chore/pin-v0-and-adopt-review-template
Jul 16, 2026
Merged

Pin to @v0 and adopt the upstream review-workflow template#10
mmcky merged 2 commits into
mainfrom
chore/pin-v0-and-adopt-review-template

Conversation

@mmcky

@mmcky mmcky commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

What

Pins both workflows to @v0, and replaces the local review-workflow variant from #7 with the upstream template verbatim.

Why @v0

The action's own docs use @v0 throughout — 19 occurrences against a single exact pin — and v0.17.0's release notes recommend it directly: "bump workflow pins to @v0.17.0 (or use @v0, which now tracks this release)." Our exact pins were the deviation, not the convention.

They also cost us this week. v0.17.0's typography fix (#99) and review-dedupe fix (#98) sat unreachable behind nine stale pins while every sync kept stripping French non-breaking spaces. Development tempo is fast enough that hand-bumping lags exactly the releases worth having.

The residual risk — 0.x minors are allowed to break, and the changelog confirms they carry behaviour changes — is real but mis-framed. The safety net is not the pin: sync output lands as a PR a human reviews before merge, so a bad release yields a bad PR, not bad content. The floating tags are also well maintained (v0 → v0.17.0; every vN.N tracks its latest patch).

Why the template swap

#7 (this repo, today) Upstream template
permissions absent contents: read, pull-requests: write
concurrency level workflow job
cancel-in-progress false true

permissions is the part that matters now. v0.17.0's dedupe deletes superseded comments, which requires pull-requests: write. Comment posting works today only because this repo's default GITHUB_TOKEN happens to be permissive — that is luck, not design, and it breaks silently if org defaults ever tighten.

On cancel-in-progress: #7 argued true was unsafe, on the grounds that an automated labeled event arriving last would cancel the in-flight action-translation review and then skip its own job via the label filter, leaving no review at all. That reasoning is correct at workflow level — which is where #7 put the block, since cancellation happens at queue time, before the job-level if is evaluated.

The upstream template puts concurrency at the job level, where the group is entered only after the if has passed. A skipped job never enters the group and cancels nothing, so true is safe there — and strictly better than false, because it supersedes a stale in-flight review instead of queueing a redundant one. This also matches rebase-translations.yml, which has used job-level permissions + concurrency since it was seeded; #7's review workflow was the odd one out.

Verification

Both files parse; the action inputs are unchanged (source-repo, source-language: en, target-language: fr, docs-folder, same secrets). Confirmed post-change: no workflow-level concurrency remains, job-level group is review-translations-${{ github.event.pull_request.number }} with cancel-in-progress: true, and permissions are contents: read + pull-requests: write.

Worth flagging honestly: the job-level skip-vs-cancel semantics are reasoned from the GitHub docs and upstream's deliberate template comment, not from a live test. The first sync after this merges is the real check — it should produce exactly one review comment and exactly one billed review run.

🤖 Generated with Claude Code

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) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 16, 2026 06:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the repository’s translation automation workflows to follow the upstream QuantEcon/action-translation guidance by switching to the floating @v0 tag and aligning the review workflow structure (permissions + job-level concurrency) with the upstream template.

Changes:

  • Pin QuantEcon/action-translation usage in both workflows to @v0.
  • Replace workflow-level concurrency in review-translations.yml with job-level concurrency and cancel-in-progress: true.
  • Add explicit job permissions in review-translations.yml to support comment deduping behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/review-translations.yml Moves concurrency to the job level, adds explicit permissions, and updates the action pin to @v0.
.github/workflows/rebase-translations.yml Updates the action pin to @v0 for rebase mode.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +15 to +16
# 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.

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) <noreply@anthropic.com>
@mmcky mmcky merged commit e641d96 into main Jul 16, 2026
1 check passed
@mmcky mmcky deleted the chore/pin-v0-and-adopt-review-template branch July 16, 2026 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants