Repository navigation
ci(book): gate the dialect page's catalog stamp against catalog.json - #1106
Merged
Merged
Conversation
Closes NVIDIA#1090. `mlir-dialects.md` states its own freshness rule: At catalog SHA-256 `26a9cc13` (the stamp in every `ops/generated/` file header), the dialect holds 570 operations across 42 modules ... If the header stamp no longer starts with `26a9cc13`, the counts on this page predate the catalog you are reading. Nothing enforced that rule, so the rule that would expose stale counts was itself unchecked. The page failed its own test for weeks before NVIDIA#1052 (560 printed against 570 in the tree), and **it is failing again right now**: the catalog hashes to `00372dfe`, five hand-written ops and five catalog entries after that refresh landed. Two hand audits in two batches is the pattern this closes. ## The gate One file hashed, one page grepped, no cargo and no network. The hash is the one `cuda-intrinsics-gen` stamps into every generated file -- `sha256_bytes(catalog_json.as_bytes())` over the file as committed -- so a plain `sha256sum` reproduces it, and the page prints its first eight digits. Three checks rather than one, so each failure says which thing moved: - The generated sources must stamp the hash `catalog.json` actually has. Failing here means the generated outputs are stale, and the message says to run `cuda-intrinsics-gen -- generate` -- not to edit the book. - The page must print exactly one distinct stamp. It names it twice, once stating it and once in the rule that tells a reader what a mismatch means, so updating one and not the other leaves the rule contradicting the statement above it. - The stamps must agree, and the failure prints both sides plus the two commands that re-derive the counts the stamp guards. It refuses to report success from a check that found nothing: a page reworded so no stamp is left fails with that stated, rather than passing vacuously. That path needed the `|| true` on the capture rather than on a later pipeline -- under `pipefail` a grep matching nothing aborted the script before the diagnostic, turning a reworded page into a silent non-zero exit. ## The refresh that makes it pass Five numbers and the stamp, all re-derived from the tree: | claim | was | now | |---|--:|--:| | catalog stamp (x2) | `26a9cc13` | `00372dfe` | | total operations | 570 | 575 | | hand-written operations | 21 | 26 | | hand-written `wgmma` row | 9 | 14 | | catalog entries | 1010 | 1015 | Generated stays 549 across 35 modules and every area row is unchanged, because NVIDIA#1085's five new catalog entries are aliases onto existing structural ops -- exactly what the page's "several intrinsics can share one structural op" sentence predicts. The +5 hand-written are all `wgmma`, from NVIDIA#1075, NVIDIA#1077 and NVIDIA#1079. The `wgmma` row's description moves with its count, as in NVIDIA#1052: it said "the m64n64k16 bf16/f16 shapes", and the module now holds four shape/type pairs -- `m64n64k16` bf16 and f16, `m64n128k16` bf16 (NVIDIA#1079), `m64n64k8` tf32 (NVIDIA#1077). ## Verification - Four mutations, each failing with its own message: a stale stamp, only one of the two stamps updated, the page reworded so none is found, and an edited `catalog.json`. The healthy tree passes. - Every count re-checked against the tree: 26 + 549 = 575, both tables sum to their stated totals (7 rows to 26, 11 rows to 549), and each still lists every module on disk. - All eleven guard scripts pass, `check-spdx-headers.sh` accepts the new file, `cuda-intrinsics-gen -- check` reports outputs current, the workflow parses with all nine jobs on `ubuntu-latest`, and the book builds clean under `sphinx-build -W`. `just check-guards` should list this script too, one line beside the other ten. Its recipe is being restructured in NVIDIA#1102, so that line is left for a follow-up rather than conflicting; no guard enforces that list, so CI is unaffected either way. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
Collaborator
|
nihalpasham
approved these changes
Aug 21, 2026
vyncint
added a commit
to vyncint/cuda-oxide
that referenced
this pull request
Aug 23, 2026
Every guard under `scripts/` is wired twice: a step in a workflow, and a line in the Justfile's `check-guards` recipe. The Justfile states that as a contract -- "`check-guards` covers the status-guard, naming-guard and cargo-deny workflows in full" -- and nothing enforced it. `check-book-catalog-stamp.sh` landed in NVIDIA#1106 wired into `status-guard.yml` and never added to `check-guards`, so twelve of the thirteen guards ran locally and "in full" was false. The first version of this check searched both files for each script's name. NVIDIA#1133's review was right that a text search is not enforcement evidence, and gave the counterexamples: the same text inside a `run:` block that only echoes it, a step behind `if: false`, or a reusable workflow nothing calls. All three left the search green while no pull request ran the guard. This reads structure. **Workflows are a graph.** The YAML is parsed, and reachability computed: a workflow is an entry point only if it triggers on `pull_request` or `pull_request_target`; a reusable workflow counts only if a reachable job `uses:` it, resolved to a fixpoint; a job or step proven `if: false` is dropped; and anything that may `continue-on-error` is not credited, because a swallowed failure is not enforcement. **A `run:` script is parsed into commands**, and a guard counts only in command position. `echo "bash scripts/x.sh"` runs `echo`. Leading `VAR=value` assignments and wrappers (`bash`, `sh`, `env`, `.`, `source`, `exec`, `time`, `sudo`) are stepped over, comment lines are dropped, and `&&`, `||`, `;`, `|` split a line so a guard buried mid-pipeline still counts. **The Justfile gets the same treatment**, as the review asked. Recipes and their dependencies are a graph, walked from `check`, so a guard moved into a recipe `check` does not depend on stops counting. A body line prefixed with `-` is just's `continue-on-error` and does not enforce. The same command-position rule applies, so a recipe that echoes an invocation is not running it. Reachability is conservative in one direction only, on purpose. An `if:` that is not literally `false` is treated as reachable, because `github.event_name == 'pull_request'` cannot be evaluated statically -- the guard never calls a live step dead. It is strict the other way: a step that may be skipped, or may swallow its failure, is not coverage. One subtlety worth recording, since it decides whether any of this works: YAML 1.1 reads a bare `on` as the boolean true, so PyYAML hands back `{True: ...}` for `on:` written unquoted, which is how all eleven workflows here write it. `document.get("on")` is `None` on every one of them. Both keys are looked up, and a self-test refuses if no workflow appears to trigger on `pull_request` -- the shape that would otherwise make the reachable set silently empty. Verified against every case, nine that must fail and seven that must refuse: | the guard is defeated by | caught as | |---|---| | the `run:` text only echoed | executed by `just check` but by no workflow | | a step behind `if: false` | same | | only in a reusable workflow nothing calls | same | | a step with `continue-on-error: true` | same | | a job with `continue-on-error: true` | same | | moved to a workflow with no `pull_request` trigger | same | | a Justfile line prefixed with `-` | executed by a workflow but not by `just check` | | moved to a recipe `check` does not reach | same | | the Justfile echoing it | same | | broken input | refusal | |---|---| | nothing triggers on `pull_request` | parse self-test failed, naming the `on:` case | | the `check` recipe renamed | parse self-test failed | | the `check-guards` recipe renamed | parse self-test failed | | `check` no longer depends on `check-guards` | not reachable from `check` | | a job calls a workflow without `workflow_call` | it would never run | | a job calls a workflow that does not exist | named | | a workflow file is not valid YAML | named, with the parser error | | no PyYAML available | refuses with the install command | Unmutated: "OK: all 14 guard scripts are executed by a pull-request workflow (11 reachable of 11) and by `just check` (8 reachable recipes)", exit 1 on failure, all fourteen guards still pass individually, 0.09s, no cargo, no network, no toolchain. The interpreter is probed rather than assumed: the first `python3` on PATH is not always the one with PyYAML -- where Homebrew shadows the system Python it is not -- so `python3`, `/usr/bin/python3` and `python` are tried in turn and a missing parser is a refusal with the install command, never a pass. Note for review: NVIDIA#1102 also edits the Justfile, in the `test-cuda` recipe around line 105. The `check-guards` change here is around line 235, so they do not overlap textually. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
vyncint
added a commit
to vyncint/cuda-oxide
that referenced
this pull request
Aug 24, 2026
Every guard under `scripts/` is wired twice: a step in a workflow, and a line in the Justfile's `check-guards` recipe. The Justfile states that as a contract -- "`check-guards` covers the status-guard, naming-guard and cargo-deny workflows in full" -- and nothing enforced it. `check-book-catalog-stamp.sh` landed in NVIDIA#1106 wired into `status-guard.yml` and never added to `check-guards`, so twelve of the thirteen guards ran locally and "in full" was false. ## Before -> after The first version of this check searched both files for each script's name. NVIDIA#1133's review was right that a text search is not enforcement evidence: before: grep finds "check-book-catalog-stamp.sh" in status-guard.yml -> OK: all 14 guard scripts run in both places after: the step is `echo "bash scripts/check-book-catalog-stamp.sh"` -> error: these guards are executed by `just check` but by no pull-request workflow: check-book-catalog-stamp.sh Same for a step behind `if: false`, a step or job with `continue-on-error`, a guard in a reusable workflow nothing calls, a guard moved to a workflow with no `pull_request` trigger, a Justfile line prefixed with `-`, a recipe `check` never reaches, and a Justfile line that echoes the invocation. Nine ways to defeat the old check; all nine now fail. ## How Workflow YAML is parsed and reachability computed as a graph: an entry point must trigger on `pull_request`/`pull_request_target`; a reusable workflow counts only if a reachable job `uses:` it, to a fixpoint; a job or step proven `if: false` is dropped; anything that may `continue-on-error` is not credited. A `run:` script is split into commands and a guard counts only in command position, stepping over `VAR=value` prefixes and the `bash`/`sh`/`env`/`.`/ `source`/`exec`/`time`/`sudo` wrappers. The Justfile gets the same treatment: recipes and dependencies are a graph walked from `check`, and a `-` prefix is just's `continue-on-error`. Conservative in one direction only, on purpose: an `if:` that is not literally `false` is treated as reachable, because `github.event_name == 'pull_request'` cannot be evaluated statically. The guard never calls a live step dead. It is strict the other way -- a step that may be skipped, or may swallow its failure, is not coverage. ## No third-party parser The YAML is read by a strict-subset reader in the script rather than PyYAML. PyYAML is **not** in the `actions/runner-images` preinstalled manifest, so depending on it would risk the job failing on merge, and no other guard in this family needs a third-party package -- on a repo that already works around macOS's bash 3.2 and BSD grep for the same reason. The reader covers exactly what these files use: block mappings and sequences, plain and quoted scalars, literal `|` and folded `>` block scalars with chomping indicators, flow sequences, comments, one leading document marker. Anything else -- a tab, an anchor, an alias, a tag, a flow mapping, a second document -- is a refusal, never a guess. That is what separates it from the text search: it cannot silently misread, only decline to read. Folding is implemented rather than approximated because `book.yml` folds one command across two lines; treating it as literal would invent a second command whose first token is `cuda-oxide-book`. Where PyYAML *is* installed the script uses it as an **oracle**: it compares its own parse against PyYAML's on every workflow, over exactly the fields the walk reads, and refuses on any disagreement. Locally that reports "agrees with PyYAML on all 11 workflows". So the reader is checked, not trusted, without being required. One subtlety the whole walk depends on: YAML 1.1 reads a bare `on` as the boolean true, so `on:` written unquoted is the key `True`, and `document.get("on")` is `None` on all eleven files. Both keys are looked up. ## Tests The nine defeats above were originally verified by hand-mutating this repository, which is neither repeatable nor visible in CI. They are now fixtures the guard asserts about itself on every run, alongside the controls that catch a rule grown too strict (a live step, a guard mid-pipeline, a called reusable workflow, `@`-suppressed Justfile lines) and four reader refusals. Each was confirmed to bite by breaking the implementation it guards: | implementation broken | self-test that failed | |---|---| | step `if`/`continue-on-error` check removed | a step behind `if: false` was counted | | job `continue-on-error` check removed | a job with continue-on-error was counted | | command position -> any token | the invocation only echoed was counted | | reusable-workflow fixpoint removed | a called reusable workflow was not counted | | Justfile `-` skip removed | a `-` prefixed script path was counted | | Justfile reachability walk removed | a recipe `check` does not reach was counted | | reader's tab refusal removed | the reader accepted a tab | | reader's flow-mapping refusal removed | the reader accepted a flow mapping | Unmutated: all fourteen guards pass individually, 0.02s, needs only python3 -- no cargo, no network, no third-party package. Note for review: at 835 lines this is the largest guard in the family by some margin, and that is what structural parsing costs. Say so if you would rather have the dependency and a shorter script. NVIDIA#1102 also edits the Justfile, in the `test-cuda` recipe around line 105; the `check-guards` change here is around line 235, so they do not overlap. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1090.
mlir-dialects.mdstates its own freshness rule:Nothing enforced that rule, so the rule that would expose stale counts was itself unchecked. The page failed its own test for weeks before #1052 (560 printed against 570 in the tree), and it is failing again right now: the catalog hashes to
00372dfe, five hand-written ops and five catalog entries after that refresh landed. Two hand audits in two batches is the pattern this closes.The gate
One file hashed, one page grepped — no cargo, no build, no network. The hash is the one
cuda-intrinsics-genstamps into every generated file (sha256_bytes(catalog_json.as_bytes())over the file as committed), so a plainsha256sumreproduces it, and the page prints its first eight digits.Three checks rather than one, so each failure says which thing moved:
catalog.jsonactually has. Failing here means the generated outputs are stale, and the message says to runcuda-intrinsics-gen -- generate— not to edit the book.It refuses to report success from a check that found nothing: a page reworded so no stamp survives fails with that stated. That path needed
|| trueon the capture rather than on a later pipeline — underpipefaila grep matching nothing aborted the script before the diagnostic, turning a reworded page into a silent non-zero exit. I only found that because the mutation produced no message.The refresh that makes it pass
26a9cc1300372dfewgmmarowGenerated stays 549 across 35 modules and every area row is unchanged, because #1085's five new catalog entries are aliases onto existing structural ops — exactly what the page's "several intrinsics can share one structural op" sentence predicts. The +5 hand-written are all
wgmma, from #1075, #1077 and #1079.The
wgmmarow's description moves with its count, as in #1052: it said "the m64n64k16 bf16/f16 shapes", and the module now holds four shape/type pairs —m64n64k16bf16 and f16,m64n128k16bf16 (#1079),m64n64k8tf32 (#1077).Verification
catalog.json. The healthy tree passes.check-spdx-headers.shaccepts the new file,cuda-intrinsics-gen -- checkreports outputs current, the workflow parses with all nine jobs onubuntu-latest, and the book builds clean undersphinx-build -W.One deferral
just check-guardsshould list this script too, one line beside the other ten. Its recipe is being restructured in #1102, so that line is left for a follow-up rather than conflicting. No guard enforces that list, so CI is unaffected either way.