ci: make python-suite reportable on stacked PRs and in a merge queue (#14747, #14353)

[mrveiss]

Closes #14747
Refs #14353, #10691, #13162, #14673, #16087, #15937, #14054

Thinking Path

The backend Python suite gates nothing, and the gap has been filed five times in 71 days — #10691 (29 Jun), #13162 (31 Jul), #14353 (16 Aug), #14673 (21 Aug), #16087 (8 Sep). All five open; only one pair was cross-linked. Consolidated on #14353.

Audited per context against live branch protection: no required check runs the backend suite. Unit & Integration Tests is published by frontend-test.yml:98 and every one of its run: steps is npm with working-directory: autobot-frontend. Four named test files (startup-import-smoke.yml:111,126,146) and one marker-filtered migrations directory (migration-gate.yml:138) are reachable — a documented carve-out, chosen as "smallest-blast-radius" rather than closing the gap. A regression in autobot-backend/services/ is invisible to all ten.

Making python-suite the eleventh is a branch-protection change and not a PR. This is the code half, and it is two triggers on one file.

What Changed

Stacked pull requests (#14747). ci.yml carried pull_request: branches: [ main, Dev_new_gui ]. A stacked PR — base is another feature branch — matches neither, so the workflow never triggered and python-suite never ran. Measured on #14641: zero CI/CD runs, while the page showed green from the workflows that do trigger.

The complement shim cannot rescue it. On a stacked PR with Python changes the shim correctly skips (needs.changes.outputs.python != 'true'), so neither side publishes. Harmless while the context is unrequired; a permanent hard block the moment it is.

Merge queue (#15937 / #14054). Ten of the required contexts' workflows already carry merge_group:. ci.yml did not, so python-suite could never report inside a queue.

What I got wrong first, and how it was caught

Both tests were originally written as parametrized rules over every workflow publishing a required context. A direct scan of all fourteen refused both:

pull_request.branches   13 workflows carry it — the norm, not the defect
merge_group missing     the 4 complement shims, deliberately

The second is worse than over-broad. I had already added merge_group: to python-required-context.yml, reasoning that both publishers of a context should match. #15937 states as an acceptance criterion that shims stay pull_request-only, and this very test file notes the shim "cannot see" merge_group because it is not one of its triggers. Inside a queue ci.yml always triggers and its python-suite job skips on a Python-free group, which reports the context. The shim is not needed and must not gain it. Reverted.

So both assertions are scoped to ci.yml, with the reason at the assertion — plus a counterpart pinning that no shim has gained the trigger, because the fix is one edit away from breaking the rule it depends on.

Verification

ci.yml   merge_group present ........... True
ci.yml   pull_request.branches absent ... True
backend-required-context.yml      merge_group absent: True
code-quality-required-context.yml merge_group absent: True
python-required-context.yml       merge_group absent: True

Both workflow files parse under yaml.safe_load, and the trigger sets are ['merge_group','pull_request','push'] and ['pull_request'] respectively.

Model Used

Claude Opus 5 (1M context).

Single-issue rationale

Closes #14747 and delivers the code prerequisite for #14353; those are one change to one trigger block, not two. The remaining half of #14353 is a branch-protection setting, which no PR can carry. The other three duplicates (#10691, #13162, #14673) describe the same gap and close on the settings change, not on this.

---

Not in scope: requiring python-suite, which needs the repository owner. And whether the other thirteen workflows should drop their branches: filter — a far larger question, and the measurement above is the argument for treating it separately rather than sweeping it in here.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 55a69d78-2ffc-4d7c-a74b-dcb963a2b112

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

[mrveiss]

Review: approved. The scoping you asked me to check hardest is correct, and the test guarding your own reverted mistake is the best thing here.

The scoping check

The tests are ci.yml-only, and read that way. :183 is explicit:

doc = yaml.safe_load((WORKFLOW_DIR / "ci.yml").read_text(encoding="utf-8"))

Both assertions hang off that single document, and both failure messages name ci.yml by hand rather than interpolating a path — so they cannot be reused against another workflow by accident. Nothing here reads as a repo-wide invariant. If it had, the 13 workflows carrying a branches: filter would have made it a mass failure rather than a wrong test.

The counterpart test is the standout

def test_the_shims_have_not_gained_a_merge_group_trigger():
    """The counterpart, because the fix above is one edit away from breaking it."""

You added merge_group to a shim, a direct scan showed it violated #15937, you reverted — and then pinned it so the next person reasoning identically is stopped by a test rather than by review. Recording why the reasoning was attractive ("both publishers should match") is what makes it work; a bare assertion would be re-argued.

That is the strongest available outcome from an error: not a fix, and not a comment, but a test that fails for the reason you nearly shipped. It also means the two tests are genuinely complementary — one requires a trigger, the other forbids it on adjacent files, so neither can be satisfied by a blanket edit.

Verification note

I am holding rather than merging, and the hold has a known end. My gate reports RUNNING (1 unfinished) — not a check-run, but a pending commit status, ci-dispatch-watchdog, outstanding since 06:43:18Z. Zero check-runs are unfinished, which is why it reads as green from the checks tab.

I verified it resolves rather than assuming: it is success on the tested heads of #16094 (817aad7ddc), #16085 (ed7ade70c9) and #16081 (eb76cbd73f). So it completes normally and this will clear.

I checked those on each PR's headRefOid rather than its merge commit — for a squash merge those are different objects, and the merge commit was never tested, so it legitimately carries no statuses. That distinction cost me a wrong conclusion about notify earlier tonight; it is not costing another.

Verdict

Approved, merging as soon as the watchdog reports. Zero unresolved threads.

[mrveiss]

Review — one finding, at d45f6fb8bf. The complement pair now has asymmetric triggers.

The branches: removal and its reasoning check out, and the inline comments are
unusually good — the #14747 note explaining why there is no filter is the
kind that survives someone "tidying" it back.

The finding: merge_group is added to one half of a guarded complement

This PR's own comment states the premise:

the two workflows that publish python-suite — this one and
python-required-context.yml — were the only ones without it

merge_group is added to ci.yml. python-required-context.yml at this head
still reads:

on:
  pull_request:

So the pair is now asymmetric: ci.yml runs on merge_group, the shim does
not.

That matters because the two are complements by construction, and
required_context_complements_test.py asserts exactly that:

assert "needs.changes.outputs.python == 'true'" in real
assert shim.strip() == "needs.changes.outputs.python != 'true'"

But a complement only holds while both workflows run. In a merge group with
no Python changes:

• ci.yml's python-suite has if: !cancelled() && needs.changes.outputs.python == 'true' → does not run;
• the shim, which exists precisely to publish the context in that case, is not
  triggered at all.

Neither half covers the merge group. The condition-complement is intact and the
trigger-complement is not.

What I can and cannot establish from here

Established: the trigger asymmetry, and that the guard checks conditions and
job names but not triggers — so it cannot see this. That is the part I would
fix regardless of the outcome below, because the guard's whole subject is "these
two stay complementary" and it is blind to the axis this PR just changed.

Not established: whether the result is a block or a silent pass. A job
skipped by if: normally still publishes a check-run with conclusion skipped,
and per #16087 a skipped required context satisfies the requirement — so the
likely outcome is a merge group proceeding with no Python testing rather than
hanging. That is milder than a block and worse in a different way, and I cannot
test which without a merge queue.

Either way it defeats the stated purpose: this PR exists so python-suite can
report inside a merge queue, and in the no-Python-changes case nothing reports
it.

Suggested

Two lines: add merge_group: to the shim, and add a test asserting the two
workflows' trigger sets match, alongside the existing condition assertions.
The second is the one that matters — without it the next trigger edit to either
side reintroduces this, and the guard will still be green.

Not blocking, and worth saying why

Everything here is about the merge-queue path, which is not enabled yet (#15937
is the settings half). So this cannot bite today. But the PR's argument is that
the queue cannot be enabled until python-suite reports there — and with the
shim untriggered, that is only true for PRs that touch Python. Landing this as
is would satisfy the prerequisite for half the cases and leave the other half to
be discovered when someone turns the queue on.

Verified sound

• Removing branches: [main, Dev_new_gui] is right and the measurement is
  specific: PR feat(document): extract PDF tables with pdfplumber (#14232) #14641 with zero CI/CD runs. Stacked PRs are exactly the
  population it excluded.
• The !cancelled() reasoning on the aggregate, and the explicit "Fail if any
  shard failed" re-assertion, are correct — without that step a red suite leaves
  a green context, which is the failure the whole file is about.
• Closing ci: stacked PRs get no Python testing, and will hard-block once python-suite is required #14747 while explicitly not closing ci: the backend Python suite gates nothing — python-suite is not a required check #14353, ci: backend unit tests not enforced — stale assertion merged via #10623 undetected #10691, ci: Python unit-test suite is not a required check — red tests merged to Dev_new_gui twice #13162 and
  ci: the inventory placeholder guard runs only in python-suite, which is not required, so the defect can be reintroduced green #14673 — because their remaining half is a branch-protection setting no PR can
  carry — is the right call and rare. Most PRs would have closed all five.