perf(test): cheap bypass read; fix(perf): identical arms only suspect on a fresh base

crates/batten/src/perf.rs

@@ -613,6 +613,20 @@ fn take_arm(shared: &Path, dest: &Path, what: &str) -> Result<()> {
     Ok(())
 }
 
+/// Whether identical arms mean a build handed back the wrong binary.
+///
+/// ONLY WHEN THE BASE WAS BUILT IN THIS RUN (CLOUD-2107). CLOUD-2060's mechanism
+/// is two builds into the shared directory in one run, the second inheriting the
+/// first's `batten` units through cargo's mtime fingerprint. A base answered from
+/// the cache — CLOUD-2097's kept head, or an earlier lap's build for the same key
+/// — lent nothing to this run's head build, and a head that is byte-identical to
+/// it is a change that did not touch the binary: measured on CLOUD-2106's
+/// test-only lap, which this guard refused before any measurement.
+//MUTANT identical-arms-always-suspect|s@^    !null \&\& base_built_now \&\& identical$@    !null \&\& identical@|identical_arms_are_suspect_only_when_the_base_was_built_this_run
+fn arms_suspect(null: bool, base_built_now: bool, identical: bool) -> bool {
+    !null && base_built_now && identical
+}
+
 /// Whether two files hold the same bytes; an unreadable side is never "same".
 fn same_bytes(left: &Path, right: &Path) -> bool {
     match (std::fs::read(left), std::fs::read(right)) {
@@ -1120,6 +1134,7 @@ fn measure(repo: &Path, options: Options, base_sha: &str) -> Result<Vec<Record>>
     let out = out_dir(repo)?;
     let shared = arms_target_dir(&perf_dir(repo));
 
+    let mut base_built_now = false;
     let (base_bin, base_tree) = if options.null {
         // The null experiment: the same bytes as both arms. COPIED rather than
         // aliased so hyperfine sees two distinct commands and cannot
@@ -1155,6 +1170,7 @@ fn measure(repo: &Path, options: Options, base_sha: &str) -> Result<Vec<Record>>
             seed_base_target_dir(&perf, &key)?;
         }
         if !base_arm_is_built(&perf, &key) {
+            base_built_now = true;
             build(&base_tree, Some(&shared), "base", PAIR_PROFILE)?;
             // A cargo that exits 0 without leaving the binary is could-not-look,
             // never a measurement: hyperfine would report the missing path as a
@@ -1189,11 +1205,13 @@ fn measure(repo: &Path, options: Options, base_sha: &str) -> Result<Vec<Record>>
         }
     }
     // THE GUARD THE MEASURED DEFECT LACKED (CLOUD-2060): a pair whose arms are
-    // the same bytes measures nothing, and its ratio of ~1 reads as a pass. A lap
-    // reaches here only when the crate changed, so identical arms mean a build
-    // handed back the wrong binary. The null experiment copies one arm on
-    // purpose, after this.
-    if !options.null && same_bytes(&base_bin, &head_bin) {
+    // the same bytes measures nothing, and its ratio of ~1 reads as a pass. The
+    // null experiment copies one arm on purpose, after this.
+    if arms_suspect(
+        options.null,
+        base_built_now,
+        same_bytes(&base_bin, &head_bin),
+    ) {
         bail!(
             "perf-pair: the head arm is byte-identical to the base arm, so a build handed back the wrong binary. No measurement."
         );
@@ -3362,6 +3380,22 @@ mod tests {
         assert!(!switches_arm(Some("head"), "head"));
     }
 
+    /// CLOUD-2107: identical arms are a broken build only when this run built the
+    /// base into the shared directory; a cached base cannot have lent its units.
+    #[test]
+    fn identical_arms_are_suspect_only_when_the_base_was_built_this_run() {
+        assert!(arms_suspect(false, true, true), "CLOUD-2060's own case");
+        assert!(
+            !arms_suspect(false, false, true),
+            "a cached base equal to the head is an unchanged binary"
+        );
+        assert!(!arms_suspect(false, true, false));
+        assert!(
+            !arms_suspect(true, true, true),
+            "the null experiment is exempt"
+        );
+    }
+
     /// CLOUD-2060: the guard that reads two identical arms as a broken build.
     #[test]
     fn identical_arms_are_the_same_bytes() -> std::io::Result<()> {

crates/batten/tests/it/bypass_scrub.rs

@@ -87,6 +87,44 @@ fn every_row_declared_hatch_is_scrubbed() {
     }
 }
 
+/// THE CHEAP READ IS HELD TO THE FULL LOAD (CLOUD-2106).
+///
+/// `common` reads the hatches through a lenient TOML view rather than the
+/// validated load, because the load was 96% of a spawning case's own CPU. A cheap
+/// read that missed a row would scrub less, silently, so both readers are compared
+/// on a config that DOES declare a hatch — the committed one declares none today,
+/// which would make the comparison vacuous — and on the committed file itself.
+#[test]
+fn the_cheap_bypass_read_names_what_the_full_load_names() {
+    let root = common::scratch_repo("bypass-cheap-read");
+    write(
+        &root,
+        "batten.toml",
+        "version = 1\n\n[[rule]]\nid = \"no-touching\"\nkind = \"shape\"\n\
+         scope = \"mediated_call\"\nseverity = \"deny\"\npattern = \"touch guarded.txt\"\n\
+         reason = \"the fixture declares a hatch\"\nbypass_env = \"BATTEN_FIXTURE_HATCH\"\n",
+    );
+    let full = |path: &std::path::Path| -> Vec<String> {
+        let mut names: Vec<String> = batten::config::load(path)
+            .expect("the config loads")
+            .rules
+            .iter()
+            .filter_map(|rule| rule.bypass_env.clone())
+            .collect();
+        names.sort();
+        names.dedup();
+        names
+    };
+    let fixture = root.join("batten.toml");
+    assert_eq!(
+        common::bypass_env_vars_in(&fixture),
+        vec![String::from("BATTEN_FIXTURE_HATCH")]
+    );
+    assert_eq!(common::bypass_env_vars_in(&fixture), full(&fixture));
+    let committed = at_root("batten.toml");
+    assert_eq!(common::bypass_env_vars_in(&committed), full(&committed));
+}
+
 /// THE REMOVED GLOBAL HATCH OPENS NOTHING, asserted on the compiled binary.
 ///
 /// `BATTEN_HOOK_BYPASS` used to turn this refusal into an allow; the engine no

crates/batten/tests/it/common/mod.rs

@@ -325,23 +325,49 @@ fn bypass_env_vars() -> Vec<String> {
     // list of hatch names. CLOUD-1227 is explicit about why: a list "stops
     // covering the next row somebody adds, silently, in the direction that
     // weakens the suite". The signature is unchanged so no caller has to know.
-    static NAMES: std::sync::LazyLock<Vec<String>> = std::sync::LazyLock::new(|| {
-        let mut names = Vec::new();
-        if let Ok(config) = batten::config::load(&at_root("batten.toml")) {
-            names.extend(
-                config
-                    .rules
-                    .iter()
-                    .filter_map(|rule| rule.bypass_env.clone()),
-            );
-        }
-        names.sort();
-        names.dedup();
-        names
-    });
+    //
+    // AND READ CHEAPLY, BECAUSE THE MEMO IS PER PROCESS (CLOUD-2106). nextest
+    // runs one process per case, so the `LazyLock` saves nothing across the suite
+    // and every spawning case paid the whole validated load at the test binary's
+    // opt-level 0: 242M of `cli::help_leads_with_the_crate_description`'s 251M
+    // instructions. The names are still read out of the committed file; only the
+    // `[[rule]]` rows' `bypass_env` is looked at, and
+    // `the_cheap_bypass_read_names_what_the_full_load_names` holds this read to
+    // the full load's answer.
+    static NAMES: std::sync::LazyLock<Vec<String>> =
+        std::sync::LazyLock::new(|| bypass_env_vars_in(&at_root("batten.toml")));
     NAMES.clone()
 }
 
+/// `[[rule]].bypass_env` out of the config at `path`, sorted and deduplicated,
+/// without the validated load. A file that will not read or parse yields nothing,
+/// as the full load's failure did.
+///
+/// A two-field serde view rather than a `toml::Value` tree: everything but the
+/// `[[rule]]` rows' one column is skipped by the deserializer instead of built.
+//MUTANT bypass-read-drops-rows|s@^        .filter_map(\x7crule\x7c rule.bypass_env)$@        .filter_map(\x7crule\x7c rule.bypass_env.filter(\x7c_\x7c false))@|the_cheap_bypass_read_names_what_the_full_load_names
+pub(crate) fn bypass_env_vars_in(path: &Path) -> Vec<String> {
+    #[derive(serde::Deserialize)]
+    struct Rows {
+        #[serde(default)]
+        rule: Vec<Row>,
+    }
+    #[derive(serde::Deserialize)]
+    struct Row {
+        bypass_env: Option<String>,
+    }
+    let mut names: Vec<String> = fs::read_to_string(path)
+        .ok()
+        .and_then(|text| toml::from_str::<Rows>(&text).ok())
+        .map_or_else(Vec::new, |rows| rows.rule)
+        .into_iter()
+        .filter_map(|rule| rule.bypass_env)
+        .collect();
+    names.sort();
+    names.dedup();
+    names
+}
+
 /// The compiled binary, with the ambient environment scrubbed.
 ///
 /// Unconditional by design: a helper that scrubbed only where a suite