fix(ci): drop relative-timing perf tests from the required PR/merge_group gate#6433
Merged
Merged
Conversation
…roup gate The required macos gate flakes intermittently because the Studio runs up to `macos_vm_cap` (2, Apple's guest limit) concurrent build VMs, and the relative-timing / CPU-budget / benchmark tests — labels `performance`, `bench`, `quality-lab` (e.g. heritage-performance's "Representative chain stays within the shipping CPU budget", asserting a ratio <= 2.0x1.05 vs an in-run baseline) — tolerate steady load but NOT the load VARIANCE a sibling VM's bursty compile creates. Whenever 2 gate builds run at once, the ratio inflates past threshold and the test flakes; a different test fails each run. Re-running makes it worse (adds Studio load). A perf/ratio test cannot be a required gate on a runner that (correctly) runs 2 concurrent VMs. Exclude those labels from the PR + merge_group `label_exclude` (both the non-Windows and Windows ctest invocations). They still run on `push` (informational) and belong in a dedicated cap=1 nightly/perf lane (follow-up). This makes both PR-head and merge_group builds reliable, which is the prerequisite for PRs to enter and clear the merge queue. Documents the failure mode + fix in the ci skill and the versioning guide's CI-cadence section. Reversible DIAL: drop the labels from the exclude to re-gate. Version-Bump: skip reason="ci workflow + skill/doc only; no versioned surface" Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017CdzQZrrtCETcDYH9MABhR
danielraffel
force-pushed
the
fix/perf-tests-off-required-pr-gate
branch
from
July 21, 2026 16:31
cd65287 to
ee9c1a7
Compare
This was referenced Jul 21, 2026
danielraffel
added a commit
that referenced
this pull request
Jul 23, 2026
…required gate The `heritage-performance::Representative chain stays within the shipping CPU budget` ratio test has flaked the required macOS gate on unrelated PRs all along — it is a CPU-budget ratio assertion that cannot be reliable on a shared runner, which is exactly why #6433 added it to the label-excluded set (`ctest -LE "…|performance|quality-lab"`). That exclusion never worked. Root cause (verified against the generated *_tests.cmake): catch_discover_tests serialises a multi-value LABELS list UNQUOTED into set_tests_properties — `PROPERTIES LABELS a b c … RUN_SERIAL TRUE …` — so ctest parses it as name/value PAIRS. Only the FIRST label survives (`LABELS=a`); the rest, and RUN_SERIAL / TIMEOUT, are silently mis-bound. These tests led with `audio`, so `-LE performance|quality-lab` matched nothing and the ratio test ran on every PR and merge_group, flaking under load. Minimal, forward-compatible fix: lead with `quality-lab` so the surviving label is an exclusion label. The test now leaves the required PR/merge_group gate (via `-LE quality-lab`) while still running on push (event-gated `label_exclude`). If the underlying truncation is ever fixed, all labels are present and the test is still excluded — so this is safe either way. Verified: after rebuild both `heritage-performance::` tests surface LABEL `quality-lab` and are excluded. This is a REPO-WIDE bug — dozens of tests carry a slow/bench/quality-lab label that is not first and are therefore silently NOT excluded from the required gate (slower, flakier than intended). The proper fix is a catch_discover_tests label-serialisation change; it needs a full-build validation pass and is tracked in the handoff. This commit only unblocks the one test that actively flakes. Version-Bump: skip reason="test-registration label order; no runtime surface" Skill-Update: skip skill=playback reason="test label-order fix; no workflow change" Skill-Update: skip skill=timeline reason="test label-order fix; no workflow change"
danielraffel
added a commit
that referenced
this pull request
Jul 23, 2026
…required gate The `heritage-performance::Representative chain stays within the shipping CPU budget` ratio test has flaked the required macOS gate on unrelated PRs all along — it is a CPU-budget ratio assertion that cannot be reliable on a shared runner, which is exactly why #6433 added it to the label-excluded set (`ctest -LE "…|performance|quality-lab"`). That exclusion never worked. Root cause (verified against the generated *_tests.cmake): catch_discover_tests serialises a multi-value LABELS list UNQUOTED into set_tests_properties — `PROPERTIES LABELS a b c … RUN_SERIAL TRUE …` — so ctest parses it as name/value PAIRS. Only the FIRST label survives (`LABELS=a`); the rest, and RUN_SERIAL / TIMEOUT, are silently mis-bound. These tests led with `audio`, so `-LE performance|quality-lab` matched nothing and the ratio test ran on every PR and merge_group, flaking under load. Minimal, forward-compatible fix: lead with `quality-lab` so the surviving label is an exclusion label. The test now leaves the required PR/merge_group gate (via `-LE quality-lab`) while still running on push (event-gated `label_exclude`). If the underlying truncation is ever fixed, all labels are present and the test is still excluded — so this is safe either way. Verified: after rebuild both `heritage-performance::` tests surface LABEL `quality-lab` and are excluded. This is a REPO-WIDE bug — dozens of tests carry a slow/bench/quality-lab label that is not first and are therefore silently NOT excluded from the required gate (slower, flakier than intended). The proper fix is a catch_discover_tests label-serialisation change; it needs a full-build validation pass and is tracked in the handoff. This commit only unblocks the one test that actively flakes. Version-Bump: skip reason="test-registration label order; no runtime surface" Skill-Update: skip skill=playback reason="test label-order fix; no workflow change" Skill-Update: skip skill=timeline reason="test label-order fix; no workflow change"
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
1 participant
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.
Excludes
performance|bench|quality-lablabels from the required PR/merge_group macos ctest (build.yml + .shipyard/config.toml). These relative-timing/CPU-budget/benchmark tests flake under the Studio's cap=2 concurrent-VM load variance and can't be a required gate on a cap=2 runner. Keystone for reliable PR builds → merge-queue entry. Self-bootstraps (its own build excludes them). Version-Bump: skip. See planning/org-flip-status.md §A.🔎 Provenance
claudem5~/Code/pulp-option-b3239e1c5-c3ff-434a-ac47-7db21d930bceResume
Jump to this tab
Relaunch (any agent)
Restore URL — https://claude.ai/code/session_017CdzQZrrtCETcDYH9MABhR
stamped 2026-07-21 16:19 UTC