From 8098cee464248881a9b29e165fa97a1d5e361103 Mon Sep 17 00:00:00 2001 From: Noah Gift Date: Mon, 14 Sep 2026 10:19:47 +0200 Subject: [PATCH] fix(cgp): six wall-clock assertions, three of them in the REQUIRED check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- crates/aprender-cgp/Cargo.toml | 24 +++++++++++++ crates/aprender-cgp/src/analysis/diff.rs | 4 +++ crates/aprender-cgp/src/doctor.rs | 5 +++ crates/aprender-cgp/src/metrics/export.rs | 4 +++ crates/aprender-cgp/tests/falsify.rs | 43 ++++++++++++++++++++--- crates/aprender-cgp/tests/integration.rs | 4 +++ 6 files changed, 80 insertions(+), 4 deletions(-) diff --git a/crates/aprender-cgp/Cargo.toml b/crates/aprender-cgp/Cargo.toml index 8ad5c7d3b9..7dfd399f40 100644 --- a/crates/aprender-cgp/Cargo.toml +++ b/crates/aprender-cgp/Cargo.toml @@ -41,6 +41,30 @@ trueno-gpu = { path = "../aprender-gpu", version = "0.67.0", features = ["cuda"] [features] default = [] cuda = ["dep:trueno-gpu"] +# A DURATION ASSERTION IS NOT A CORRECTNESS ASSERTION (#3239, and this repo has +# paid for it eleven times — see scripts/check_no_timing_in_required.sh, whose +# registry exists because "policy is what failed the other eleven times"). +# +# tests/falsify.rs carries two: FALSIFY-CGP-061 (`cgp doctor` < 2 s) and +# FALSIFY-CGP-060 (`cgp profile compare` < 30 s). Both are real product limits +# and neither can be asserted on a shared CI box. Measured 2026-09-14 during a +# full --lib --tests sweep of the workspace: +# +# FALSIFY-CGP-061 FAILED: doctor took 2665ms (limit 2500, over by 6.6%) +# +# The box was running 98,621 tests in parallel. That number measures the LOAD, +# not `cgp doctor`. 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. +# +# check_no_timing_in_required.sh keeps exactly this rule for shell guards, with +# a declared registry so "adding a timing gate without deciding its placement is +# impossible". This feature is that registry for Rust targets: nothing enables +# it, so a timing row cannot reach a required check by accident. +# +# Arm it where duration IS measurable -- a dedicated, unloaded runner (perf-solo), +# at release time. Not in CI's default set, and not in the nightly full surface. +timing-gate = [] [dev-dependencies] proptest = "1.9" diff --git a/crates/aprender-cgp/src/analysis/diff.rs b/crates/aprender-cgp/src/analysis/diff.rs index 5a00e8c4ca..df878afc3c 100644 --- a/crates/aprender-cgp/src/analysis/diff.rs +++ b/crates/aprender-cgp/src/analysis/diff.rs @@ -338,10 +338,14 @@ mod tests { } let elapsed = start.elapsed(); // 100 diffs should take << 100ms + // DURATION ONLY UNDER `timing-gate` (#3239) — see this crate's Cargo.toml. + #[cfg(feature = "timing-gate")] assert!( elapsed.as_millis() < 100, "100 diffs took {}ms", elapsed.as_millis() ); + #[cfg(not(feature = "timing-gate"))] + let _ = elapsed; } } diff --git a/crates/aprender-cgp/src/doctor.rs b/crates/aprender-cgp/src/doctor.rs index bb3724b1da..6084924990 100644 --- a/crates/aprender-cgp/src/doctor.rs +++ b/crates/aprender-cgp/src/doctor.rs @@ -400,6 +400,11 @@ mod tests { let _ = detect_gpu(); let _ = check_binary("nonexistent", &[], parse_generic_version); let elapsed = start.elapsed(); + // DURATION ONLY UNDER `timing-gate` (#3239). A shared, saturated runner + // cannot measure elapsed time; see this crate's Cargo.toml. + #[cfg(feature = "timing-gate")] assert!(elapsed.as_secs() < 2, "doctor checks took {:?}", elapsed); + #[cfg(not(feature = "timing-gate"))] + let _ = elapsed; } } diff --git a/crates/aprender-cgp/src/metrics/export.rs b/crates/aprender-cgp/src/metrics/export.rs index ab87b55bc7..ef50993ce2 100644 --- a/crates/aprender-cgp/src/metrics/export.rs +++ b/crates/aprender-cgp/src/metrics/export.rs @@ -64,10 +64,14 @@ mod tests { let _ = load_json(&path).unwrap(); let _ = load_json(&path).unwrap(); let elapsed = start.elapsed(); + // DURATION ONLY UNDER `timing-gate` (#3239) — see this crate's Cargo.toml. + #[cfg(feature = "timing-gate")] assert!( elapsed.as_millis() < 100, "Loading two JSONs took {}ms (should be <100ms)", elapsed.as_millis() ); + #[cfg(not(feature = "timing-gate"))] + let _ = elapsed; } } diff --git a/crates/aprender-cgp/tests/falsify.rs b/crates/aprender-cgp/tests/falsify.rs index f2d274448f..843a7b4046 100644 --- a/crates/aprender-cgp/tests/falsify.rs +++ b/crates/aprender-cgp/tests/falsify.rs @@ -447,16 +447,45 @@ fn falsify_cgp_061_doctor_speed_real() { let elapsed = start.elapsed(); assert!(output.status.success()); - // Allow 500ms subprocess overhead on top of the 2s spec limit + + // DURATION ONLY UNDER `timing-gate`. Measured 2026-09-14 on a box running + // 98,621 tests in parallel: 2665 ms against this 2500 ms limit -- a 6.6% + // miss that says nothing about `cgp doctor` and everything about the load. + // See this crate's Cargo.toml for why the feature exists. + #[cfg(feature = "timing-gate")] assert!( elapsed.as_millis() < 2500, "FALSIFY-CGP-061 FAILED: doctor took {}ms", elapsed.as_millis() ); + #[cfg(not(feature = "timing-gate"))] + let _ = elapsed; let stdout = String::from_utf8_lossy(&output.stdout); - // Verify it detected the GPU - assert!(stdout.contains("RTX 4090") || stdout.contains("GPU")); + // THE STATUS MUST BE DECIDED -- not that this box has a 4090. + // `contains("RTX 4090") || contains("GPU")` asserts the RUNNER's hardware, + // the class fixed in #3210 across four sibling rows. It passes here only + // because this box happens to have one. What CGP-061 actually owes is that + // `doctor` reached a verdict about the device and printed it. + assert!( + !stdout.trim().is_empty(), + "FALSIFY-CGP-061 FAILED: doctor printed nothing -- no verdict at all" + ); + let decided = [ + "GPU", + "CUDA", + "device", + "Device", + "no GPU", + "not available", + "unavailable", + ] + .iter() + .any(|k| stdout.contains(k)); + assert!( + decided, + "FALSIFY-CGP-061 FAILED: doctor reached no decision about the device.\nstdout: {stdout}" + ); } // ══════════════════════════════════════════════════════════════════════ @@ -545,12 +574,18 @@ fn falsify_cgp_060_profile_speed() { let elapsed = start.elapsed(); assert!(output.status.success()); - // 30s spec limit + 500ms subprocess overhead + + // DURATION ONLY UNDER `timing-gate`, for the same reason as CGP-061 above. + // 30 s is a real product limit and a shared runner cannot measure it; this + // row has simply not been unlucky yet, which is not the same as correct. + #[cfg(feature = "timing-gate")] assert!( elapsed.as_secs() < 31, "FALSIFY-CGP-060 FAILED: profile took {}s (limit: 30s)", elapsed.as_secs() ); + #[cfg(not(feature = "timing-gate"))] + let _ = elapsed; } // ══════════════════════════════════════════════════════════════════════ diff --git a/crates/aprender-cgp/tests/integration.rs b/crates/aprender-cgp/tests/integration.rs index 0ba6dde32d..f97b6b29d9 100644 --- a/crates/aprender-cgp/tests/integration.rs +++ b/crates/aprender-cgp/tests/integration.rs @@ -33,11 +33,15 @@ fn test_doctor_completes() { assert!(stdout.contains("cgp System Check"), "Missing header"); assert!(stdout.contains("CPU"), "Missing CPU detection"); // Doctor should complete in <2s (FALSIFY-CGP-061), allow 30s for compilation + // DURATION ONLY UNDER `timing-gate` (#3239) — see this crate's Cargo.toml. + #[cfg(feature = "timing-gate")] assert!( elapsed.as_secs() < 30, "cgp doctor took too long: {:?}", elapsed ); + #[cfg(not(feature = "timing-gate"))] + let _ = elapsed; } /// cgp roofline --target cuda must show RTX 4090 data.