Enable the PyTorch PR review skill

[Gasoonjia]

Import the core PyTorch pr-review skill unchanged and route GitHub @claude review requests through it.

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

✅ You can merge normally! (2 Unrelated Failures)

As of commit 8aecaed with merge base 3faa2e4 ():

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

• Test QNN Windows MSVC build / build-qnn-windows-arm64 (gh) (trunk failure)
• Test QNN Windows MSVC build / build-qnn-windows-x64 / windows-job (gh) (trunk failure)

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

[github-actions]

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.

[huydhn]

PyTorch pr-review skill has lots PyTorch-specific references though, and I don't think they all apply to ET. Should we ask for a core ET maintainer to review what should go into that skill?

On the other hand, the workflow change LGTM

[huydhn]

@claude Review the new pr-review skill, highlight areas that PyTorch-specific and not related to ExecuTorch. Also check if there are areas in the skill where ExecuTorch could adopt the same best practices as PyTorch

[claude]

Claude finished @huydhn's task in 1m 33s —— View job

---

Review: PyTorch-specific content vs. ExecuTorch applicability

Summary: This PR imports the PyTorch pr-review skill unchanged (SKILL.md, review-checklist.md, bc-guidelines.md), registers it in CLAUDE.md, and routes @claude requests through it. Because it's imported verbatim, a large fraction of the checklist references PyTorch-core infrastructure that does not exist in ExecuTorch — a reviewer following it here would hunt for subsystems ET doesn't have and miss the ones it does. Below is the breakdown @huydhn asked for.

---

🔴 PyTorch-specific — does NOT apply to ExecuTorch

review-checklist.md → "PyTorch Infrastructure" (lines 48–162): almost the entire section is PyTorch-core-only.

• C++ Kernel Infrastructure (52–63): TensorIterator, DispatchStub, structured kernels + native_functions.yaml, AT_DISPATCH_*, TORCH_LIBRARY. ET kernels are portable/optimized/backend ops registered via YAML op-schema + kernels.yaml / selective build, not ATen codegen. The TORCH_CHECK_VALUE/_INDEX/_TYPE variants don't exist in the ET runtime (ET uses ET_CHECK / ET_KERNEL_CHECK / Error codes).
• CUDA & Device Management (65–74): C10_CUDA_CHECK, CUDAStreamGuard, recordStream, CUDA graphs, AcceleratorHooksInterface. ET is an on-device edge runtime; there is no CUDA backend. Delegation to accelerators happens through the backend/delegate interface, not c10 device guards.
• Operator Registration & Codegen (76–83): native_functions.yaml, tags.yaml, CompositeImplicitAutograd fallbacks, @register_meta, @register_fake. ET consumes an already-exported graph; meta/fake kernels matter at export time in PyTorch, not in ET's runtime kernel registration.
• Autograd (85–93): derivatives.yaml, setup_context, gradcheck, forward-mode AD, vmap. ET is inference-only — there is no autograd at all. This entire subsection is inapplicable.
• Python Utilities (95–103): __torch_function__, torch._logging, torch.backends.cudnn.flags() — PyTorch-core Python surface, not ET.
• nn Module Patterns (105–112): ModuleList, nn.init, parametrize, LazyModuleMixin, clip_grad_norm_. Authoring modules is a PyTorch concern; ET lowers exported programs.
• Dynamo / Inductor / Compile (114–122): @register_lowering, Inductor decomps, CustomGraphPass, Dynamo trace rules. ET has its own export/pass infra (exir, EdgeProgramManager, ExportedProgram passes, to_edge/to_backend) — none of these torch/_inductor / torch/_dynamo APIs.
• FX / Export (124–132): partially relevant in spirit (ET does heavy FX/export work), but the specific references (torch/fx/passes/infra/pass_base.py, make_fx, torch.export.Dim) should be re-pointed at ET's exir.pass_base.ExportPass, PassManager, and ET's dynamic-shape handling.
• Type Promotion & Dtypes (134–139): elementwise_dtypes, TensorIteratorConfig, result_type, promoteTypes — ATen-internal, not ET runtime.
• Serialization (141–145): torch.load(weights_only=...), add_safe_globals, skip_data. ET does not use pickle/torch.load for its runtime artifact — models are .pte flatbuffers. This is a genuinely important distinction: the security guidance here is aimed at the wrong format.
• Distributed (147–150) & Tensor Subclasses (152–155): DeviceMesh, MultiThreadedPG, _make_wrapper_subclass, __tensor_flatten__. Not part of the ET runtime story.
• Miscellaneous (157–162): torch._check, cpp_extension, register_package, @register_backend (Dynamo) — PyTorch-core.

review-checklist.md → Testing (164–199): the principles transfer, but nearly every named tool is PyTorch-core: OpInfo/op_db, ModuleInfo, torch.testing._internal.common_utils.TestCase, instantiate_device_type_tests, @ops, make_tensor, common_dtype helpers, largeTensorTest. ET tests use pytest + gtest and ET-specific harnesses; a reviewer should not demand OpInfo entries here.

review-checklist.md → Security → CI/CD (203–211): the "non-ephemeral runners / no repo secrets / reverse shell" guidance is a description of PyTorch's specific CI security posture. ET's CI security model (this very workflow uses secrets: inherit) may differ and should be validated by an ET infra owner rather than imported as fact.

review-checklist.md → Security → PyTorch API (213–220) & Performance → Device (267–282): torch.load/TorchScript/model_dump, torch.profiler, torch.utils.benchmark.Timer, MPS. ET has its own profiling (ETDump/Inspector, the /profile skill) — pointing reviewers at torch.profiler is actively misleading.

bc-guidelines.md:

• Python version claim (lines ~/"3.10–3.14", "last 5 versions") and free-threaded CPython support are PyTorch's support matrix, not ET's — must be verified against ET's actual supported versions.
• The "Public API definition" (links to the pytorch/pytorch wiki) and the __module__ starting with "torch." rule are PyTorch-specific. ET's public surface is executorch.* and, critically, includes .pte schema / runtime ABI / backend delegate contract BC — none of which the imported doc covers.
• All examples reference torch/nn.Module/forward.

SKILL.md:

• Review philosophy example (line ~"missing Composite dispatch key breaks every out-of-tree backend… TensorIterator… type promotion") is PyTorch-specific.
• "Files to Reference" (bottom) points at torch/testing/_internal/common_utils.py, opinfo/core.py, native_functions.yaml, derivatives.yaml, tags.yaml — none of these paths exist in this repo. This list should point at ET equivalents (CLAUDE.md ✅ which exists, CONTRIBUTING.md, ET kernel YAMLs, exir pass infra, backend partitioner/delegate docs).
• Default base branch is written as main in Local-CLI/Local-Branch modes and example URLs use pytorch/pytorch — fine for the GitHub Actions path (which correctly uses origin/<baseBranch>), but the local-mode examples are PyTorch-flavored.

---

🟢 PyTorch best practices ExecuTorch should adopt

These are backend/framework-agnostic and are genuinely stronger than what ET's CLAUDE.md currently encodes:

1. "Only report problems / no praise" review discipline (SKILL.md §Review Philosophy) — a high-signal review format worth adopting wholesale.
2. Consolidate-then-fact-check workflow (SKILL.md Steps 4–5): dedupe findings by root-cause/fix/file:line, then one verification sub-agent per surviving finding. This is a good, model-agnostic anti-hallucination pattern.
3. "One finding, one section" precedence (SKILL.md Output Format) — clean way to avoid the same issue appearing in three buckets.
4. BC discipline as a first-class review axis — ET arguably needs this more than the imported doc provides: .pte backward compatibility, delegate/AOT-vs-runtime version skew, and operator-schema stability are real ET pain points. Adopt the structure of bc-guidelines.md but rewrite the content for ET's artifact/ABI contracts.
5. Testing rigor principles (checklist Testing §Test Quality, 195–199): regression test required for every bug fix; assertRaisesRegex over bare assertRaises; prefer xfail over skip so coverage isn't silently lost; weakref over getrefcount. All of these apply directly to ET's pytest suites.
6. Thread-safety / RAII lock-guard and free-threaded-Python guidance (checklist 222–257): ET's C++ runtime and multi-threaded backends benefit from the same C++ concurrency review items (data races, RAII guards, atomic ordering). The CPython-C-API-under-NoGIL items apply anywhere ET has pybind/C-extension code.
7. "Investigate, don't guess" + parallel sub-agents (SKILL.md) — good general reviewer methodology.

---

Recommendation

I agree with @huydhn's instinct. The workflow routing change (claude-code.yml) and CLAUDE.md entry are fine. But importing the checklist unchanged means ~60–70% of review-checklist.md and the specifics of bc-guidelines.md / SKILL.md's "Files to Reference" describe subsystems that don't exist in ExecuTorch (autograd, CUDA, Inductor/Dynamo, native_functions.yaml, torch.load, OpInfo). A reviewer following it verbatim would waste effort on inapplicable items and — more importantly — miss ET-specific load-bearing concerns: .pte/ABI BC, delegate/partitioner contracts, memory planning, selective build, ETDump profiling, and the backend op-registration model.

Suggested path: land the workflow + skill scaffold, then fast-follow with an ET-authored pass that (a) strips the PyTorch-core infra/autograd/serialization sections, (b) rewrites "Infrastructure" around ET backends/delegates/exir passes, (c) rewrites bc-guidelines.md around .pte/schema/ABI BC, and (d) fixes the non-existent paths in "Files to Reference." Given the PyTorch-specific footprint, I'd echo @huydhn: get a core ET maintainer to own the checklist content before treating it as the canonical review rubric.

I did not make code changes — this was a review-only request.
· branch claude/pr-review-skill

[JakeStevens]

do we need to update this for ExecuTorch vs PyTorch Core?

[JakeStevens]

in general seems like haven't been updated for PyTorch -> ExecuTorch

BC on serialized stuff probably should be called out explicitly

<edit: lol i just looked at the code changes to start, looks like this concern has been covered>

[izaitsevfb]

infra looks good, but as claude mentioned, the prompt has some pytorch-specific residuals, see below

[izaitsevfb]

please remove pytorch-specific sections.

---

🟡 Both tests the backward-compatibility rules give for "is this a public API" are keyed to PyTorch, so neither one selects an ExecuTorch API. (ai-generated section)

Step 3 of the review procedure sends the reviewer to bc-guidelines.md to judge backward compatibility, and that file carries PyTorch's public-API definition verbatim. Its structural test requires the module reported by __module__ to start with "torch."; its fallback test is whether the function is documented on pytorch.org/docs. This project's own APIs live in executorch.* modules and are documented elsewhere, so neither test reaches them, and a reviewer applying the rules literally can conclude there is no public API at stake on a change that does break callers. The file is also silent on the surfaces that carry compatibility risk in an on-device runtime: the serialized program schema in schema/program.fbs, the C++ runtime headers under runtime/, and the backend delegate interface. Changing the module prefix and naming those surfaces would make the step select something.

[izaitsevfb]

make sure that references are correct.

---

🟡 The imported skill sends the reviewer to five reference files that are not in this repository. (ai-generated section)

SKILL.md closes with a "Files to Reference" list and tells the reviewer to read those files rather than rely on memory. Five of the seven are PyTorch core paths — torch/testing/_internal/common_utils.py, torch/testing/_internal/opinfo/core.py, aten/src/ATen/native/native_functions.yaml, tools/autograd/derivatives.yaml, aten/src/ATen/native/tags.yaml — and none of them exist here; there is no aten/ or torch/ tree in this repository at all. review-checklist.md's "PyTorch Infrastructure" section has the same shape: where a change touches its scope it asks the reviewer to stop and investigate whether a listed ATen or autograd mechanism should have been used, and several of those items turn on editing native_functions.yaml or derivatives.yaml, which a change to this repository cannot do. ExecuTorch declares its operators in kernels/*/functions.yaml instead. A reviewer following the list can dead-end on unreadable paths, or ask an author to adopt infrastructure this project does not carry — in a comment posted on a public pull request. Retargeting the reference list at this repository's own files would keep the rest of the import intact.

Measured: Checked against the full recursive git tree at the reviewed commit; there is no aten/ or torch/ directory..