Skip to content

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

Open
rascani wants to merge 1 commit into
pytorch:mainfrom
rascani:codex/arm-ci-routing
Open

rascani wants to merge 1 commit 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 every changed path is inside backends/arm/ or examples/arm/, starting unrelated backend jobs. Select ciflow/trunk-arm for these diffs and retain full trunk CI for mixed paths, incomplete metadata, or PR heads without the Arm workflows.

Review the selection logic in .github/workflows/labeler.yml first, then the two workflow entrypoints. The bulk of the diff moves the existing 33 Arm, Zephyr, and Cortex-M test entries into _arm-trunk.yml, which both trunk.yml and the new arm.yml call. Existing main/release triggers, sampling, and viable/strict behavior are preserved.

Selection includes deleted files and both sides of renames, reruns after base-branch edits, and serializes label updates per PR. Existing ciflow/trunk labels are preserved so manual full-run requests remain effective; scope transitions can briefly start both runs. The implementation stays within ExecuTorch and uses the existing PyTorch bot.

Test plan

A local mocked-API harness passed 37 path/label scenarios plus transition and error checks, including base retargeting and overlapping label updates. The concurrency regression reproduces the lost-label case without serialization and preserves the current head's label with serialization. These harnesses were local validation artifacts, not additions to the repository's test suite.

Structural comparison against the current base confirmed all 33 Arm/Cortex-M test entries and all other trunk job definitions are preserved. A three-agent review of the amended patch found no outstanding actionable issues. Rebasing onto current main produced an identical patch according to git range-diff.

actionlint -shellcheck='' -pyflakes='' .github/workflows/{labeler,arm,_arm-trunk,trunk}.yml
git diff --check HEAD^ HEAD
bash -n backends/arm/test/test_github_trunk_zephyr.sh

Lintrunner was unavailable locally, and live CI was not part of this validation.

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.

❌ 2 New Failures, 1 Unclassified Failure

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

NEW FAILURES - The following jobs have failed:

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.

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.

1 participant