Skip to content

ci: run the full lane on stacked PRs, and refuse to report green when it did not (D18 #11) - #106

Merged
heydryft merged 2 commits into
masterfrom
ci/stacked-pr-trigger-gap
Aug 17, 2026
Merged

heydryft merged 2 commits into
masterfrom
ci/stacked-pr-trigger-gap

Conversation

@heydryft

Copy link
Copy Markdown
Contributor

The defect

ci.yml and cuda_compile_check.yaml both triggered on:

  pull_request:
    branches:
      - master

A PR whose base is another PR's branch — a stack — matches neither, so not one lane ran: no compile, no test, no rustfmt, no clippy, no MSRV, no nvcc.

The only workflow without a branch filter is analysis.yaml, which uses pull_request_target. So those PRs showed exactly one check — comment — and rendered green.

Six open PRs were in that state simultaneously: the stack #95 → #100 → #102 → #103 → #104, plus #98. Check-run census at the time: 16 checks on every master-based PR, 1 on every stacked one.

This compounds with a known gap: a stacked PR touching #[cfg(feature = "cuda")] Rust got neither the nvcc lane nor a local type-check, because cargo check on macOS structurally cannot see those arms. #99's missing qtip_grouped_tile_m re-export is exactly that hole — it survived a local check and was only caught once the nvcc lane was forced to run.

The fix

1. pull_request.branches: ['**'] on both workflows. Stacked PRs are normal in this repo and must get the full lane.

2. A ci-complete guard job. The trigger fix alone is not enough, because a branch filter is only one route to "the lanes did not run" — a skipped job, a renamed job, a matrix that expands to nothing, or an over-narrow paths: filter all reproduce it exactly. ci-complete depends on every lane, runs if: always(), and fails unless each reports success (treating skipped and cancelled as failures). It also asserts the lane count, so deleting a lane trips it rather than quietly shrinking what "green" covers.

Unlike the individual lanes, ci-complete cannot pass by not existing. That makes it the correct required status check for branch protection, and I'd suggest switching the protection rule to it.

flash_attn_compile_check.yaml is deliberately left workflow_dispatch-only — the flash-attn kernel compile is heavy and that's an intentional choice documented in the file.

Why this is filed as doctrine

Recorded as D18 instance 11 in memory/mission/KERNEL_RULES.md. Every prior D18 instance was a single code path lying about itself; this one is the verification layer being absent for a whole class of PR while presenting as a pass. Same mechanical form as the other ten: the absence of a signal was read as a specific signal — here, "no CI configured for this base branch" rendered as "checks passed."

Corollary worth keeping for tooling and agents: count the checks before trusting the conclusion. 0 failures out of 1 check is not a pass.

… it did not (D18 #11)

`ci.yml` and `cuda_compile_check.yaml` both triggered on
`pull_request: branches: [master]`. A PR whose base is another PR's branch
matched neither, so not one lane ran — no compile, test, rustfmt, clippy,
MSRV or nvcc. The only workflow without a branch filter is analysis.yaml
(`pull_request_target`), so those PRs showed exactly one check, `comment`,
and rendered green.

Six open PRs were in that state at once: the stack #95 -> #100 -> #102 ->
#103 -> #104, plus #98. Check-run census: 16 checks on master-based PRs, 1
on every stacked one.

This compounds with the macOS gap — a stacked PR touching cuda-gated Rust
got neither the nvcc lane nor a local type-check, since cargo check on
macOS cannot see those arms.

Two changes, because the trigger fix alone is not enough:

1. `pull_request.branches: ['**']` on both workflows. Stacked PRs are
   normal here and must get the full lane.
2. A `ci-complete` guard job that needs every lane, runs `if: always()`,
   and fails unless each reports success — skipped and cancelled included —
   and asserts the lane count. A branch filter is only one route to 'the
   lanes did not run'; a skipped job, a renamed job, an empty matrix or an
   over-narrow paths filter all reproduce it. Unlike the individual lanes,
   ci-complete cannot pass by not existing, which makes it the correct
   required status check for branch protection.

flash_attn_compile_check.yaml is left dispatch-only by design.
Widening the pull_request trigger multiplies this workflow across every
stacked PR, and ci.yml is 12 jobs — the largest multiplier in the repo, and
the only workflow with no concurrency group. cuda_compile_check.yaml has
carried the identical block for a while; this is the same shape on purpose.

Without it every push leaves its superseded runs sitting in a shared queue.
Measured today rather than assumed: 38 in-flight runs, of which 13 were on
SHAs that were no longer any PR's head, cancelled by hand to free capacity.
perf/qtip-grouped-gemm-arch alone held two queued CI runs, one obsolete.

The 'head_ref || run_id' idiom is load-bearing. On pull_request, head_ref is
the source branch, so a new push cancels that PR's own in-flight run. On
push, schedule and workflow_dispatch, head_ref is empty and run_id is unique
per run, so master pushes and scheduled runs are never cancelled by each
other — only PR iterations are.
@heydryft

Copy link
Copy Markdown
Contributor Author

Two additions, plus a live instance that replaces the hypothesis in the description with a dated example.

#109 — the defect, happening right now

PR #109 (perf/wgmma-descriptor-probe, base arctarget/multiarch-specialization) is a stacked PR opened today. Its checks, by trigger:

workflow event ran?
Analysis / comment pull_request_target yes — the only automatic one
CUDA compile check workflow_dispatch only because an agent dispatched it by hand, per push
Continuous integration — never ran at all

So the only check that fires on its own is the cosmetic tokei line-counter. No compile, no test, no rustfmt, no clippy, no MSRV — and the CUDA lane exists purely because someone is manually re-dispatching it on every push. Three of those hand-dispatched runs had already been superseded and were cancelled today.

Worth stating precisely, because the shorter version of this story ("#109 got only Analysis") is not quite what the API shows, and the accurate version is the stronger one: the automatic signal is one cosmetic check; everything real is a human remembering to dispatch. That race is not winnable — heads on #94, #98, #99 and #108 each moved several times within minutes while I was dispatching against them.

The concurrency group ci.yml never had

cuda_compile_check.yaml has carried one for a while. ci.yml — 12 jobs, the largest multiplier in the repo — had none, so every push left its superseded runs sitting in a shared queue. Widening the trigger in this PR multiplies that across every stacked PR, so the two changes have to ship together.

Measured today rather than assumed: 38 in-flight runs, 13 of them on SHAs that were no longer any PR's head. I cancelled those by hand to free capacity; perf/qtip-grouped-gemm-arch alone was holding two queued CI runs, one already obsolete. This block is what stops that recurring without a human sweeping.

The block is character-identical to the CUDA workflow's, deliberately. The head_ref || run_id idiom is load-bearing and worth not "simplifying" later: on pull_request, head_ref is the source branch so a new push cancels that PR's own in-flight run; on push, schedule and workflow_dispatch, head_ref is empty and run_id is unique per run, so master pushes and scheduled runs never cancel each other — only PR iterations do.

On merging this

Suggest making ci-complete the required context in the "Protect main/master" ruleset once this lands. The six currently-required contexts are the Rust lanes; ci-complete supersedes them as a single gate that, unlike any individual lane, cannot pass by not existing — which is the whole failure this PR documents.

@github-actions

Copy link
Copy Markdown
Code Metrics Report
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 Language              Files        Lines         Code     Comments       Blanks
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 C Header                  5          305          210           52           43
 CSS                       2         1181         1036           34          111
 CUDA                     72        24328        17592         4018         2718
 Dockerfile                1           39           22            8            9
 JavaScript               16         3546         2676          482          388
 Jinja2                    7          694          656            5           33
 JSON                     74         4600         4597            0            3
 Makefile                  1            6            5            0            1
 Metal Shading Lan|       33        12224         9431         1142         1651
 PowerShell                1          300          227           30           43
 Python                  143        14830        12217          797         1816
 Shell                    23         5756         3964         1412          380
 Plain Text                4         3801            0         2479         1322
 TOML                     33         1485         1292           43          150
 YAML                      3           25           23            2            0
─────────────────────────────────────────────────────────────────────────────────
 HTML                      4         2687         2604           43           40
 |- CSS                    2          543          479           37           27
 |- JavaScript             1         1233         1215           12            6
 (Total)                             4463         4298           92           73
─────────────────────────────────────────────────────────────────────────────────
 Jupyter Notebooks         4          122           83           23           16
 |- Markdown               1           60           30           22            8
 |- Python                 1          122          113            1            8
 (Total)                              304          226           46           32
─────────────────────────────────────────────────────────────────────────────────
 Markdown                188        37465            0        28743         8722
 |- BASH                  69         1614         1187          311          116
 |- C                      2           12           12            0            0
 |- CUDA                   2           84           56           16           12
 |- JSON                  18          708          708            0            0
 |- PowerShell             1            1            1            0            0
 |- Python                23         1008          787          113          108
 |- Rust                  65         2048         1713           77          258
 |- TOML                   6          207          164            0           43
 |- YAML                   4           38           33            5            0
 (Total)                            43185         4661        29265         9259
─────────────────────────────────────────────────────────────────────────────────
 Rust                    662       306982       265873        13815        27294
 |- Markdown             477        22543          471        19392         2680
 (Total)                           329525       266344        33207        29974
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 Total                  1276       450597       329477        73114        48006
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

@heydryft
heydryft merged commit 52b9a93 into master Aug 17, 2026
17 checks passed
@heydryft
heydryft deleted the ci/stacked-pr-trigger-gap branch August 17, 2026 20:59
@heydryft

Copy link
Copy Markdown
Contributor Author

Bypass record. Admin-merged without review — no second reviewer on this fork. The bypass covered the review requirement only; every required status check was green, and unusually for this session, so was the guard this PR adds.

Required contexts at merge time (the six Rust lanes, pre-switch) all success on head 71485376, and all 16 checks passed including cargo check (cuda, workspace), nvcc compile (sm_80) and nvcc compile (sm_90).

CI complete itself ran and passed before this merged, which mattered more than the rest. It was initially absent from the check list; rather than assume it was fine, I treated that as disqualifying until explained — a guard with no observed execution is not evidence, and this PR's own thesis is that an absent check must never read as a pass. The explanation was benign: ci-complete was correctly blocked on needs because the test matrix had not finished its macOS leg. Once it ran, its log read:

lane results: success success success success success success success success
All 8 lanes succeeded.

Eight lanes, matching EXPECTED_LANES. So the guard is merged with one observed green execution behind it, not on faith.

Follow-up applied: master's required status check is now ["CI complete"], replacing the six individual Rust contexts. It supersedes them because it needs all eight and, unlike any individual lane, cannot pass by not existing — an absent ci-complete leaves the PR pending rather than green. Force-push and deletion protection verified unchanged.

Transition note: branches created before this merge cannot emit a CI complete context and will sit blocked until they absorb master. The remedy is to merge master in — which produces the required context — not to bypass it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant