| name | therock-pr-quality |
| description | TheRock build-repo supplements to the ROCm PR quality base skill. Use for TheRock PR author, review, or pre-merge gating where the change touches the superbuild, submodules/patches, artifact descriptors, or reusable CI workflows. Adds and tightens base rules; never relaxes a base MUST. |
| argument-hint | [author | review | pre-merge] [PR URL | branch:<name> | local] |
| extends | rocm-pr-quality |
| allowed-tools | Bash, Read, Grep, Glob, Task, WebFetch |
TheRock PR Quality (overlay)
Dependency (mandatory — do this first)
Read and apply the rocm-pr-quality base skill before anything below. It lives in this same repo at
skills/rocm-pr-quality/ (SKILL.md + reference.md). The supplements here only ADD rules or
TIGHTEN thresholds; they never relax a base MUST-rule. On any conflict, the base MUST-rule wins.
This overlay is for the build-repo concerns that the library overlays (e.g. hipblaslt-pr-quality)
do not cover: the superbuild itself, submodules and patches, artifact descriptors, and the reusable
CI workflows that every downstream build depends on.
Scope
Apply this overlay (in addition to the base) when a PR touches any of:
- The superbuild / build system: root
CMakeLists.txt, cmake/, THEROCK_ENABLE_* options,
component CMakeLists.txt under math-libs/, ml-libs/, comm-libs/, compiler/, etc.
- Submodules and patches:
.gitmodules (root and nested), third-party/, patch sets applied to
subprojects.
- Artifact descriptors:
artifact-*.toml, BUILD_TOPOLOGY.toml, and the artifact
fetch/install tooling (build_tools/**, install_rocm_from_artifacts.py).
- Reusable CI:
.github/workflows/** (especially reusable workflow_call workflows and their
callers) and build_tools/github_actions/**.
Changes outside these areas follow the base bar only.
Canonical TheRock references
Consult and cite these when reviewing build-repo PRs:
- Build system overview:
docs/development/build_system.md.
- Style guides:
docs/development/style_guides/ —
python,
cmake,
bash,
github_actions.
- Formatting:
.pre-commit-config.yaml (run pre-commit run --all-files).
Cite the specific guide section for a style finding.
PR-policy gate
The source of truth for TheRock's contributing policies is the repo's
CONTRIBUTING.md — follow it first. In practice those policies are enforced
by an automated gate (therock_pr_bot), which is the hoop every PR must actually clear before it can
be reviewed. Treat that gate as authoritative: conform the PR to it before author or pre-merge
sign-off, it overrides this skill's waivers and self-evident exemptions, and the skill never works
around it. This overlay only points at the gate; it does not restate the gate's rules, so if the gate
is later corrected to match CONTRIBUTING.md, the overlay needs no change.
When you advise the author to do something solely to clear the gate that is not stated in
CONTRIBUTING.md, say so explicitly — name it as a gate requirement, not a guide requirement — so the
author knows where it came from (for example: "the bot requires a resolving ISSUE ID / JIRA ID
line even though the contributing guide doesn't, so add one"). skills/therock_pr_bot/FAQ.md explains
how to clear a specific failure.
Supplements
Adds — change classes (bind to TheRock paths)
On top of the base classes, tag TheRock PRs with:
submodule-bump — advancing a submodule pointer (and/or its patch set).superbuild-cmake — changes to superbuild options / component wiring.artifact-descriptor — changes to artifact-*.toml / BUILD_TOPOLOGY.toml.reusable-ci — changes to a reusable workflow or its callers.dependency-add — adding a new third-party dependency or subproject.
Adds — submodules & patches review checks
- A submodule pointer bump is a real change: the PR must say what moved and why (the target
commit/range and its purpose), and link the upstream PR/commit. An unexplained pointer move is an
M5 gap.
- Pointer bumps and patch-set edits must be intentional and isolated — flag an incidental
pointer move bundled into an unrelated PR (the "undeclared submodule drift" smell).
- Patches under a subproject must still apply cleanly against the new pointer; a patch that no longer
applies (or is now upstreamed and redundant) is
BLOCKING.
- New/changed patches need a one-line rationale and, where it exists, a link to the upstreaming
effort so the patch can later be dropped.
Adds — superbuild vs sub-project CMake
- Distinguish superbuild-level changes (root
CMakeLists.txt, cmake/, THEROCK_ENABLE_*) from
sub-project CMake; a change in the wrong layer is a maintainability finding.
- A new
THEROCK_ENABLE_* (or similar) option needs a sane default, a help string, and a note on
how it interacts with existing components. See docs/development/build_system.md and the CMake
style guide for conventions.
- Enabling a component by default (or changing a default) is a behavior change → treat as the base
heuristic/default-selection class (safe default + flag/tracker).
Adds — artifact descriptor checks
- No duplicate component ownership: a given component/file should be claimed by exactly one
artifact-*.toml. Flag two descriptors claiming the same thing.
- TOML components must match what the build actually provides — the components listed line up with
the
therock_provide_artifact() (or equivalent) calls that produce them.
- Stale-descriptor-after-split: when a component is split/moved, the old descriptor must be
updated, not left pointing at the pre-split layout.
BUILD_TOPOLOGY.toml stays consistent with the descriptors and component graph (no orphaned or
dangling entries).- Changes to artifact fetch/install flags (
install_rocm_from_artifacts.py and friends) keep the
documented flags and defaults in sync.
Adds — reusable CI workflow wiring
- When a reusable (
workflow_call) workflow's interface changes, all callers are updated in the
same PR; a caller left on the old interface is BLOCKING.
- Reusable-workflow inputs are read via
inputs.*, not github.event.inputs.* (that only exists for
workflow_dispatch); flag the mismatch.
runs-on for self-hosted/specialized runners is pinned to the intended label set, not a generic
default that will mis-route the job.- Multi-checkout wiring (TheRock + a submodule/sibling repo) is correct: paths, fetch depth, and
submodule flags are set for what the job actually needs.
- No complex inline
bash embedded in YAML — non-trivial logic belongs in a script under
build_tools/ (per the GitHub Actions style guide), where it is testable and lintable.
- Any script a workflow calls has its runtime dependencies declared/available on the runner.
Adds — dependency / subproject additions
A dependency-add PR must state: the build-time and binary-size impact (with a before/after
metric where feasible), the license and its compatibility, and the maintenance owner. A new
dependency with none of these is IMPORTANT at minimum, BLOCKING if it lands on a default/shipping
path.
Tightens — stale-base on high-coupling build files
Make the base pre-merge stale-base check concrete for TheRock. High-coupling files:
root CMakeLists.txt, cmake/**, .gitmodules, artifact-*.toml, BUILD_TOPOLOGY.toml, and
reusable workflows under .github/workflows/. Overlap with the base branch on any of these since the
PR diverged → mandatory rebase + re-run (base default is strong-recommend), because these break
combinations that neither PR's own CI can see.
What the overlay cannot do
Drop the regression/test obligations (M1/M2), allow disabling tests to green CI (M3), or skip work
tracking/linking (M4/M5). Those are base MUSTs; this overlay can only make them stricter or bind them
to TheRock build paths.
