Skip to content

Narrow Arm PR trunk CI to the shared Arm suite - #24758

Open
rascani wants to merge 3 commits into
pytorch:mainfrom
rascani:codex/arm-ci-routing
Open

rascani wants to merge 3 commits into
pytorch:mainfrom
rascani:codex/arm-ci-routing

Conversation

@rascani

@rascani rascani commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Arm PRs currently receive ciflow/trunk even when all changed paths are inside backends/arm/ or examples/arm/, starting unrelated backend jobs. Select ciflow/trunk-arm for these diffs; mixed Arm/outside diffs still request full trunk CI.

Review the inline selector in .github/workflows/labeler.yml, then the workflow entrypoints. The existing Arm, Zephyr, and Cortex-M jobs move into _arm-trunk.yml, shared by arm.yml and trunk.yml. Both entrypoints use the trunk workflow name for Dr. CI matching, with a distinct run title for narrow CI.

Selection includes deleted files and both rename paths, reruns after base retargeting, and serializes label updates per PR. Snapshot retries retain observed Arm paths for full-CI fallback when metadata remains incomplete. Unknown diffs without Arm evidence or an existing trunk request fail selection without changing CI labels, while normal module/Docker labeling continues. Older PR heads missing either Arm workflow also fall back to full CI.

Existing full labels remain sticky to preserve manual requests. Scope transitions can run both suites to completion. The original trunk triggers, non-Arm jobs, sampling, and viable/strict behavior are preserved. Routing uses the existing PyTorch bot; the policy and rollout behavior are documented in .github/arm-ci.md.

Test plan

All 49 local mocked-API scenarios passed, including failures during snapshot retries and recovery to a complete unrelated diff. These harnesses are local validation artifacts. Structural YAML comparison against base 472953c12c preserved all 33 Arm/Cortex-M entries, the original trunk triggers, and every non-Arm trunk job. The latest independent review found no actionable issues.

actionlint -shellcheck='' -pyflakes='' .github/workflows/{labeler,arm,_arm-trunk,trunk}.yml
git diff --check 472953c12c HEAD

Lintrunner was unavailable locally. Local validation does not include the live GitHub Actions runs for this update.

Authored with OpenAI Codex.

cc @digantdesai @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell

Select ciflow/trunk-arm for changes confined to the Arm backend and examples.
Keep full trunk coverage for mixed changes, uncertain metadata, older PR heads,
and existing full-run requests. Share the existing Arm and Cortex-M jobs
between both workflow entrypoints.

Recompute CI scope after PR base changes and serialize each PR's label updates.

Authored with OpenAI Codex.
@rascani
rascani requested a review from digantdesai as a code owner November 7, 2026 00:43
@pytorch-bot

pytorch-bot Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24758

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 Unclassified Failure

As of commit b3fd639 with merge base 472953c (image):

UNCLASSIFIED FAILURE - DrCI could not classify the following job because the workflow did not run on the merge base. The failure may be pre-existing on trunk or introduced by this PR:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 7, 2026
@github-actions github-actions Bot added ciflow/trunk module: arm Issues related to arm backend labels Nov 7, 2026
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@rascani

rascani commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

@claude review effort=max

@claude

claude Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

Claude finished @rascani's task in 36m 21s —— View job


Request changes: besides narrowing Arm-only PRs, this PR runs most of trunk on every PR that edits trunk.yml or the new Arm workflow files. It also lets the metadata fallback put permanent full trunk on PRs that never touch Arm, and it classifies Arm PRs that must commit regenerated Arm docs as mixed.

Detailed review

PR Review: #24758

Summary

This PR moves the 33 Arm/Zephyr/Cortex-M trunk entries into _arm-trunk.yml and adds an Arm workflow that runs on ciflow/trunk-arm/* tags. For PRs entirely under backends/arm/ or examples/arm/, the Labeler now selects ciflow/trunk-arm instead of ciflow/trunk. Problems found:

  • trunk.yml gains pull_request triggers that the description doesn't mention.
  • The metadata fallback escalates non-Arm PRs to full trunk, permanently.
  • The two labels get inconsistent ownership rules.
  • isArm excludes the generated Arm docs that pull.yml makes many Arm PRs commit.
  • A new failure path skips actions/labeler.

Dr. CI and PyTorch bot behavior can't be checked from this repo. The one finding that depends on it is marked low confidence.

Infrastructure

  • trunk.yml now runs on every PR that edits trunk or the Arm workflows (.github/workflows/trunk.yml:12-14). Each push to such a PR now runs these without any label:

    • every job with no if:, e.g. test-models-linux-aarch64 (line 97), test-llama-runner-linux (324) and test-qnn-model (531), plus the 33-entry Arm suite;
    • the jobs gated on github.event_name == 'pull_request', e.g. test-models-macos-cpu (12 entries on macos-m1-stable, line 52) and test-models-windows (896);
    • when trunk.yml itself changes, the five jobs gated on that path (lines 240, 275, 399, 593, 737).

    Before this PR, these PRs needed ciflow/trunk. Three further consequences:

    • A PR that adds an Arm trunk matrix entry (_arm-trunk.yml + backends/arm/) is mixed, so it also gets ciflow/trunk. The pull_request run (group trunk-<N>-…) and the tag run (group trunk-ciflow/trunk/<N>-…, line 21) don't cancel each other, so most of trunk runs twice per push.
    • Listing arm.yml starts trunk, which never calls arm.yml, so the edited file is never exercised.
    • The comment at qnn-windows-msvc.yml:28-32 that lists trunk's PR paths goes stale.

    Drop the three entries. If you want PR-time validation of the Arm workflow files, count .github/workflows/arm.yml and _arm-trunk.yml as Arm paths in isArm instead; the tag run uses the PR head's copies. Fix this →

  • The metadata fallback escalates PRs that never touch Arm, and the escalation sticks (.github/workflows/labeler.yml:56-58). !complete selects ciflow/trunk whatever the paths are, so the Labeler now adds it to PRs the old rule never labeled. complete is false for:

    • an empty diff (files.length > 0, line 33);
    • more than 3000 files (listFiles stops at 3000, so the count never matches changed_files);
    • any API error or exception inside the try (github-script's retries defaults to 0);
    • a push landing between the two pulls.get calls (lines 28 and 30).

    ciflow/trunk starts all seven workflows that listen on ciflow/trunk/* (_ci-run-decision.yml:21-24). Line 45 treats any existing ciflow/trunk as a manual request, so the label then applies to every later push. One race is enough to turn off narrow CI on an Arm-only PR for good.

    Make the fallback no worse than the old rule:

    • on a head or base change, return; the serialized run for the newer event recomputes;
    • treat an empty diff as complete;
    • on a truncated list, escalate only if the files seen include an Arm path;
    • on an API error, change no labels and fail so that a re-run fixes it. Once the actions/labeler finding below is fixed, that failure no longer skips the labeler.

    Comparing base.ref instead of base.sha (line 32) is enough, since main advancing doesn't change the file list. Update the warning at line 38 and arm-ci.md:14 to match. Fix this →

  • Label ownership isn't tracked, so the two labels get opposite blanket rules (.github/workflows/labeler.yml:45, :62). Every ciflow/trunk is treated as manual and never removed. Every ciflow/trunk-arm is treated as automatic and stripped.

    • Line 45: every Arm PR open at merge time carries the ciflow/trunk that the removed .github/labeler.yml rule added. Those PRs, and any PR that was mixed once or hit the fallback above, stay on full trunk until someone removes the label by hand.
    • Line 62: a maintainer can't opt a PR outside backends/arm/ and examples/arm/ into the Arm suite. One example is a backends/cortex_m/ PR, whose trunk-level test-cortex-m-e2e/test-cortex-m-ops jobs live in this suite. The label is stripped on the next push.

    Have the script remove only labels it added, for example only when the actor of the label's latest labeled event (from issues.listEvents) is github-actions[bot]. That would also move the open Arm PRs to narrow CI without manual cleanup. At minimum, remove ciflow/trunk-arm only when escalating to ciflow/trunk. Fix this →

  • Arm PRs that commit regenerated docs can never be Arm-only (.github/workflows/labeler.yml:43). isArm is narrower than the module: arm globs in .github/labeler.yml:4-9. Two pull.yml checks fail unless regenerated files under docs/source/backends/arm-*/ are committed:

    • test-arm-backend-docgen (pull.yml:1030-1047), for the generated overview, partitioner, quantization and getting-started pages;
    • the test_pytest_ops_no_target entry of test-arm-backend-no-driver (pull.yml:1086-1100), for the *_op_support.md tables.

    So every Arm PR that changes operator support, or the sources of those generated pages, counts as mixed and gets full trunk. Yet neither trunk.yml nor _arm-trunk.yml references docs/. Add at least docs/source/backends/arm-*/ to isArm, ideally all the module: arm globs. Fix this →

  • A failure in the new step skips actions/labeler (.github/workflows/labeler.yml:59-69). addLabels (line 60) is unguarded, and line 66 rethrows non-404 errors. Either failure ends the job before actions/labeler@v5 runs, so that run adds neither module: arm nor ciflow/docker. The comment at .github/labeler.yml:11-12 notes that ciflow/docker is what rebuilds the images for a PR that changes them. Move the Arm selection into its own job, or set continue-on-error: true on it. Fix this →

Backward Compatibility

  • Arm check names change, and PRs and main no longer share them (.github/workflows/trunk.yml:90-92, .github/workflows/arm.yml:1,15).

    • On main, the 33 checks gain an Arm suite / segment: trunk / test-arm-backend-… (…) becomes trunk / Arm suite / test-arm-backend-… (…), and the same for test-cortex-m-*. This breaks their HUD history.
    • Narrowed PRs report them as Arm / Arm suite / …, a name that never runs on main.

    Low confidence, since Dr. CI's source isn't in this repo: if Dr. CI matches PR failures to merge-base jobs by name, an Arm job that is already broken on main will show as NEW FAILURE instead of BROKEN TRUNK on Arm-only PRs. Worth confirming before merge.

Testing

  • .github/workflows/labeler.yml:21-68: the routing policy is ~50 lines of inline JS with no committed tests; the 37-scenario harness from the test plan stayed local. pull_request_target runs the base-branch copy, so the script first runs for real on every PR right after merge. Moving it into .github/scripts/ with the harness next to it would keep those scenarios checkable.

Performance

  • .github/arm-ci.md:24-25: "a scope transition can briefly start both runs" understates the cost. Take a push that turns an Arm-only PR mixed, for example regenerated op-support docs landing in a follow-up push. An Arm run started for the new head before the label is removed is never cancelled, because its group arm-refs/tags/ciflow/trunk-arm/<N>-false gets no successor. The 33-entry suite then runs to completion twice for that head, once there and once inside trunk. Cancel the in-flight Arm run when downgrading, or document it as a full duplicate run.

Specific Comments

  • .github/workflows/arm.yml:1: name: Arm sits next to the existing test-backend-arm.yml (Test ARM Backend), and checks render as Arm / Arm suite / …. Naming it after the label (trunk-arm) would make the two easier to tell apart.
  • .github/arm-ci.md:4: the hard-coded "33 … test entries" will go stale with the next matrix edit.
  • .github/arm-ci.md:25-29: "This change requires no test-infra deployment" and the older-heads paragraph read as PR-description notes, not lasting docs.
  • .github/arm-ci.md: nothing links to this file. CI policy in this repo lives in workflow header comments (e.g. _ci-run-decision.yml), so it would be easier to find at the top of labeler.yml.

Recommendation

Request Changes

Both of these change CI well beyond Arm PRs and need fixing before merge:

  • the trunk.yml trigger expansion, which runs most of trunk on every PR that edits trunk.yml or the Arm workflow files;
  • the metadata fallback, which leaves non-Arm PRs on full trunk permanently after a single API error or race.

Fix the isArm docs gap as well. Without it, Arm PRs that change operator support or generated pages won't get narrow CI.

Review tasks
  • Load /pr-review skill and repo guidelines
  • Gather PR diff and context
  • Independent review panel (3 reviewers): labeler selection logic, workflow entrypoints / _arm-trunk.yml, probot + docs + end-to-end flow
  • Consolidate findings
  • Fact-check each finding against the code. Dropped:
    • skipped edited runs cancelling pending runs: the job if is evaluated before job-level concurrency;
    • the ^ciflow/trunk regex in trymerge*.py: defined but never used;
    • the merge bot re-adding ciflow/trunk: there is no merge workflow in this repo.
  • Post final review

This was a review only; no changes were pushed.

@zingo

zingo commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

Nice! This will ease the CI testing a lot!
Is the plan to gave a family of trunk jobs for differnt backens?

@rascani

rascani commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Nice! This will ease the CI testing a lot! Is the plan to gave a family of trunk jobs for differnt backens?

I hadn't planned on expanding this to other backends quite yet, but worth considering if this works out well. I think this still needs some work before its ready.

Retry changing PR snapshots and limit full-CI fallback to observed Arm changes or existing CI requests. Keep ordinary labeling running when Arm selection fails, restore the original trunk PR triggers, and align shared job names for Dr. CI.

Authored with OpenAI Codex.
Retain Arm evidence when a changing PR snapshot triggers another read and that read fails. Use the retained evidence only for incomplete-diff fallback; complete snapshots still determine current CI scope.

Authored with OpenAI Codex.
@rascani

rascani commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

@claude review effort=max

@claude

claude Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Claude finished @rascani's task in 41m 17s —— View job


PR Review: #24758 (re-review)

Request Changes: the selector treats every ciflow/trunk label as a manual request it never removes, including the ones it adds itself and the ones the old labeler rule added, and it counts the regenerated Arm docs that pull.yml requires as non-Arm paths, so Arm PRs open at rollout and Arm PRs that change operator support stay on full trunk for good.

Detailed review

Summary

This PR moves the Arm, Zephyr, and Cortex-M trunk jobs into a shared _arm-trunk.yml, adds an arm.yml entry point on ciflow/trunk-arm/* tags, and replaces the labeler's ciflow/trunk rule for Arm paths with a script that applies ciflow/trunk-arm when a PR only touches backends/arm/ and examples/arm/. The label-ownership problem from the previous review is still open. Together with the narrow path rule, it keeps many Arm PRs on full trunk, and the only way back is for a maintainer to remove the label. There are also smaller problems with very large PRs, with arm.yml's workflow_dispatch trigger, and with test coverage.

Infrastructure

1. ciflow/trunk is never removed, even when the Labeler added it (.github/workflows/labeler.yml:69). This is still open from the previous review.

  • L69 treats any ciflow/trunk as a manual request. The Labeler adds that label itself in three cases: a mixed diff (L80), incomplete metadata (L67), and any getContent error, including a 5xx after retries or a 403 rate limit (L76-78). The rule this PR removes from .github/labeler.yml added the same label to every Arm PR, also as github-actions[bot].

  • As a result, every Arm PR that is open at merge stays on full trunk until a maintainer removes the label by hand (arm-ci.md:35-37). An Arm-only PR that is mixed for one push, or hits one API error, stays on full trunk for good.

  • The reverse also happens. If a maintainer adds ciflow/trunk-arm by hand, the next push removes it whenever selected !== ARM (L86-92). That includes PRs with no Arm paths, such as a backends/cortex_m/ PR, even though the Cortex-M trunk jobs are defined in _arm-trunk.yml.

  • Fix: decide ownership from the actor of the label's latest labeled event (issues.listEvents). If the actor is github-actions[bot], the Labeler added the label and may remove it. Any other actor means a manual request. With ownership tracked, a PR that falls back to full trunk at L67, L76-78, or L80 can return to narrow CI on a later push, and the manual step in arm-ci.md:35-37 is no longer needed.

    Fix this →

2. isArm leaves out the Arm docs that Arm PRs must regenerate (.github/workflows/labeler.yml:26).

  • In pull.yml, test-arm-backend-docgen fails unless the docgen outputs are committed (L1042). test-arm-backend-no-driver runs git diff --exit-code on the generated *_op_support.md tables under docs/source/backends/arm-vgf/ and docs/source/backends/arm-ethos-u/ (L1093-1096).

  • So every Arm PR that changes operator support also commits files outside backends/arm/. The Labeler then counts it as mixed and adds ciflow/trunk, which stays (finding 1).

  • A typical sequence:

    1. The first push gets ciflow/trunk-arm.
    2. The docgen check fails.
    3. The author pushes the regenerated docs.

    On that last push, the bot can re-tag ciflow/trunk-arm/<N> while the Labeler adds ciflow/trunk, so the Arm suite runs twice on the same head (arm-ci.md:31-33).

  • arm-ci.md:27-28 excludes generated docs without giving a reason. No ciflow/trunk/* workflow reads docs/, so sending those PRs to narrow CI loses no coverage.

  • Fix: make isArm match the module: arm doc globs from .github/labeler.yml:6-9, or at least the docs/source/backends/arm- prefix.

    Fix this →

3. A PR with more than 3000 files is never treated as complete (.github/workflows/labeler.yml:45).

  • pulls.listFiles returns at most 3000 files, so files.length === pr.changed_files is always false for a larger PR.

  • A non-Arm PR without CI labels then fails the Labeler check (L62-64). Re-running the Labeler, as the error message suggests, fails the same way. An Arm PR gets ciflow/trunk.

  • Fix: when pr.changed_files > 3000, warn instead of calling core.setFailed.

    Fix this →

4. arm.yml is named trunk and also accepts workflow_dispatch (.github/workflows/arm.yml:2, :9). Low confidence, because this depends on how the viable/strict updater handles several runs with the same workflow name.

  • update-viablestrict.yml:27 requires a workflow named trunk, and the updater reads one row per run (viable-strict-gate.yml:19-22). If an Arm-only dispatch on a main commit fails, it could block that commit from viable/strict even when the full trunk.yml run passes.

  • Fix: drop workflow_dispatch from arm.yml, since dispatching trunk.yml already runs the suite. Also mention viable/strict in the arm.yml:1 comment.

    Fix this →

Testing

  • The selector has no committed tests (.github/workflows/labeler.yml:23-92). It is about 70 lines of branching JavaScript that will run on every PR. Because the workflow uses pull_request_target, the copy from main runs, so this script first runs for real after merge. The 49 mocked-API scenarios in the test plan were not committed. Fix: move the script to .github/scripts/ and commit those scenarios next to it.

    Fix this →

  • arm.yml and the label-to-tag path have never run. On this PR, the Arm jobs ran from trunk.yml through ciflow/trunk. The bot most likely reads ciflow_push_tags from the default branch, so adding ciflow/trunk-arm won't create a tag before merge. Fix: before merging, have a maintainer push the tag ciflow/trunk-arm/24758 at the PR head and link the run. A tag-push run uses the workflow files from the tagged commit, so it exercises the new trigger, the concurrency group, and the Arm suite / … job names that Dr. CI matches.

Code Quality

  • The retry loop repeats what the concurrency queue already does (.github/workflows/labeler.yml:34-51).

    • A head change fires synchronize, and a retarget fires edited with changes.base (L13). Both queue behind the labeler-<PR> group (L14-16). So if the PR changes mid-read, the step can return without touching labels and leave the decision to the queued run.
    • That removes the loop, observedArm, and the reason bookkeeping. With the current code, three changed reads in a row either add ciflow/trunk based on an older head's Arm paths (L41-42) or fail the check.
    • Comparing base.sha (L44) also triggers a retry when main moves, even though that doesn't change the PR's file list. Comparing base.ref is enough.

    Fix this →

Specific Comments

  • .github/arm-ci.md:12 - "retries transient API errors twice" is only partly true. By default, actions/github-script does not retry 400, 401, 403, 404, or 422 responses, so 403 secondary rate limits are not retried.
  • .github/arm-ci.md:26-27 - ciflow/trunk-arm is also removed whenever the selection is ciflow/trunk, not only when the PR has no Arm paths.
  • .github/workflows/labeler.yml:66 - the warning says "Requesting full trunk CI" even when ciflow/trunk is already present and nothing changes.
  • .github/workflows/labeler.yml:72-75 - arm.yml and _arm-trunk.yml are added together, so checking for one of them is enough.
  • backends/arm/test/test_github_trunk_zephyr.sh:9 - "(ignoring the conda commands)" is stale, because the Zephyr job in _arm-trunk.yml has no conda commands.

Fix these →

Recommendation

Request Changes. Findings 1 and 2 keep narrow CI from reaching Arm PRs that are open at rollout or that change operator support. Once a PR falls back to full trunk, it can't go back without a maintainer removing the label. Finding 3 is a one-line fix.

@digantdesai
digantdesai requested a review from zingo November 11, 2026 02:55
@digantdesai

Copy link
Copy Markdown
Contributor

Is the plan to gave a family of trunk jobs for differnt backens?

This is from the CI Efficiency push :)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: arm Issues related to arm backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants