Skip to content

fix(cgp): six wall-clock assertions, three of them in the REQUIRED check - #3253

Closed
noahgift wants to merge 2 commits into
mainfrom
PMAT-1098-cgp-timing-gate
Closed

noahgift wants to merge 2 commits into
mainfrom
PMAT-1098-cgp-timing-gate

Conversation

@noahgift

Copy link
Copy Markdown
Contributor
FALSIFY-CGP-061 FAILED: doctor took 2665ms

Measured during a full --lib --tests sweep — the box was running 98,621 tests in parallel.
2665 against a 2500 ms limit is a 6.6% miss that says nothing about cgp doctor and everything
about the load. It is the last of the 37 failures that sweep found, and the one blocking
#3249's hard-failing nightly.

Raising the limit is the "clever ratio rewrite" that once blocked all nine open PRs. The fix is to
stop asserting duration where duration cannot be measured.

Fixed one, found six

The test that failed was the entry point, not the scope:

site limit
tests/falsify.rs:457 doctor < 2500 ms ← the one that failed
tests/falsify.rs:583 profile < 31 s same file, simply not unlucky yet
tests/integration.rs:39 doctor < 30 s
src/doctor.rs:406 checks < 2 s in src/
src/analysis/diff.rs:344 < 100 ms in src/
src/metrics/export.rs:70 < 100 ms in src/

Those last three are #[cfg(test)] units, so they run in --workspace --libthe required
workspace-test, on every PR and every merge group.
A 100 ms assertion on a shared runner is a
coin flip, and two of them have been sitting in a required check.

scripts/check_no_timing_in_required.sh exists for exactly this; its header says "policy is what
failed the other eleven times."
But its registry covers check_*.sh guards. It has never been
able to see a Rust target, so nobody has ever counted these.

timing-gate

Off by default, enabled by nothing — the same registry idea, for Rust. Arm it where duration is
measurable: a dedicated unloaded runner at release time. Not in CI's default set, and not in
#3249's nightly full surface.

Gated, not deleted — proved by mutation rather than asserted:

limit → 0, WITH --features timing-gate    FAILED (1 failed)   the assertion is live
limit → 0, WITHOUT the feature            ok                  compiled out
restored                                  ok

A second defect in the same row

assert!(stdout.contains("RTX 4090") || stdout.contains("GPU"));

That asserts the runner's hardware — the class fixed in #3210 across four sibling rows — and
passes here only because this box happens to have a 4090. What CGP-061 owes is that doctor
reached a verdict about the device and printed it, so that is what it now asserts: non-empty
output, and a decision among GPU / CUDA / device / no GPU / not available.

Verified: aprender-cgp 121 lib + 3 falsify pass, clippy -D warnings clean.

Not fixed here

The same regex over the workspace finds ~70 duration assertions in crates/*/src/ and 36 more
in crates/*/tests/, across 12 crates — aprender-serve 9, aprender-core 5, aprender-test-lib
3, and so on. This PR fixes the one crate that actually failed; the sweep is its own ticket.

Refs #3239

no-close: #3239 stays OPEN — this clears the last of its 37 measured failures, but the tiering
decision it describes is #3249's.

🤖 Generated with Claude Code

    FALSIFY-CGP-061 FAILED: doctor took 2665ms

Measured during a full `--lib --tests` sweep of the workspace — the box was
running 98,621 tests in parallel. 2665 against a 2500 ms limit is a 6.6% miss
that says nothing about `cgp doctor` and everything about the load. It is the
LAST of the 37 failures that sweep found, and the one blocking #3249's
hard-failing nightly.

Raising the limit is the "clever ratio rewrite" that once blocked all nine open
PRs. The fix is to stop asserting duration where duration cannot be measured.

FIXED ONE, FOUND SIX. The test that failed was the entry point, not the scope:

  tests/falsify.rs:457    doctor       < 2500 ms   <- the one that failed
  tests/falsify.rs:583    profile      < 31 s      <- same file, simply not unlucky yet
  tests/integration.rs:39 doctor       < 30 s
  src/doctor.rs:406       checks       < 2 s       }
  src/analysis/diff.rs:344             < 100 ms    } IN src/ -- these are #[cfg(test)]
  src/metrics/export.rs:70             < 100 ms    } units, so they run in --workspace
                                                   } --lib: the REQUIRED workspace-test,
                                                   } on every PR and every merge group

A 100 ms assertion on a shared runner is a coin flip, and two of them have been
sitting in a required check. `scripts/check_no_timing_in_required.sh` exists for
exactly this — its header says "policy is what failed the other eleven times" —
but its registry covers `check_*.sh` GUARDS. It has never been able to see a Rust
target, so nobody has ever counted these.

`timing-gate`, off by default, is that registry for Rust: nothing enables it, so
a timing row cannot reach a required check by accident. Arm it where duration is
measurable — a dedicated unloaded runner at release time — not in CI's default
set and not in #3249's nightly full surface.

GATED, NOT DELETED, proved by mutation rather than asserted:

  limit -> 0, WITH --features timing-gate      FAILED (1 failed)  the assertion is live
  limit -> 0, WITHOUT the feature              ok                 compiled out
  restored                                     ok

Also in CGP-061, a second defect in the same row:

  assert!(stdout.contains("RTX 4090") || stdout.contains("GPU"));

That asserts the RUNNER's hardware — the class fixed in #3210 across four sibling
rows — and passes here only because this box happens to have a 4090. What
CGP-061 owes is that `doctor` reached a VERDICT about the device and printed it,
so that is what it now asserts: non-empty output, and a decision among
GPU/CUDA/device/no GPU/not available.

aprender-cgp: 121 lib + 3 falsify pass, clippy -D warnings clean.

Not fixed here, filed separately: the same regex over the workspace finds ~70
duration assertions in `crates/*/src/` and 36 more in `crates/*/tests/`, across
12 crates. This commit fixes the one crate that actually failed.

Refs #3239

Pmat-Ticket: PMAT-1098

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 14, 2026

Copy link
Copy Markdown

§13.11 rung 1 — quorum shadow verdict

S13-SHADOW pr=3253 head=39dc1da26d5613fd7b9723d13d3c88d3b5340176 verdict=REFUSE class=Q1 arm_rc=1

Shadow mode: this records a verdict and merges nothing. A refusal
to arm is not a block (§13 adds zero rows to §7) — the pull request is
exactly as green as it was.

@noahgift noahgift added this to the 0.68.0 milestone Sep 14, 2026
@noahgift
noahgift added this pull request to the merge queue Sep 14, 2026
@noahgift
noahgift removed this pull request from the merge queue due to a manual request Sep 14, 2026
@noahgift
noahgift added this pull request to the merge queue Sep 14, 2026
@noahgift
noahgift removed this pull request from the merge queue due to a manual request Sep 14, 2026
@noahgift

Copy link
Copy Markdown
Contributor Author

Superseded by #3295, which carries this change unmodified along with eight other
collision-free CI/guard PRs as a single merge-queue entry.

Not abandoned and not rejected: the commits are merged into
PMAT-3231-batch-1-ci-guards verbatim (clean=9 conflicted=0), and this branch is
retained — reopening is one click if #3295 does not land.

Why: a successful merge_group CI run is median 78 min with max_entries_to_merge = 1,
so nine entries is ~12 h of queue for 623 insertions. As one entry it is 78 minutes.
Root cause and the full five-whys: #3294.

@noahgift noahgift closed this Sep 15, 2026
@noahgift noahgift mentioned this pull request Sep 16, 2026
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,424 of 1,469 integration targets run only when a PR touches their own crate — four of tonight's defects trace to this

1 participant