diff --git a/bootstrap/src/suite.rs b/bootstrap/src/suite.rs index 415fc63fb2..538d4445f9 100644 --- a/bootstrap/src/suite.rs +++ b/bootstrap/src/suite.rs @@ -84,43 +84,70 @@ fn tri_exe(repo: &Path) -> anyhow::Result { ); } -/// Load the set of spec paths that are documented as pre-existing -/// `gen-verilog` yosys smoke failures. If the baseline file is missing, the -/// set is empty and the suite summary falls back to a strict `acceptable == -/// passed` interpretation. -fn load_gen_verilog_smoke_baseline(repo: &Path) -> HashSet { - let path = repo - .join("docs") +/// The `gen-verilog-yosys-smoke` baseline file. Always resolved against the +/// repo root, never against the process cwd. +fn gen_verilog_smoke_baseline_path(repo: &Path) -> PathBuf { + repo.join("docs") .join("reports") - .join("gen_verilog_smoke_baseline.json"); - let raw = match fs::read_to_string(&path) { - Ok(r) => r, - Err(e) => { - eprintln!( - "[suite] baseline file not readable ({}); using empty baseline", - e - ); - return HashSet::new(); - } - }; - let json: serde_json::Value = match serde_json::from_str(&raw) { - Ok(j) => j, - Err(e) => { - eprintln!( - "[suite] baseline file invalid JSON ({}); using empty baseline", - e - ); - return HashSet::new(); - } - }; - json.get("expected_failures") + .join("gen_verilog_smoke_baseline.json") +} + +/// Load the set of spec paths documented as pre-existing `gen-verilog` yosys +/// smoke failures. +/// +/// **Three outcomes, kept distinct.** `Ok(None)` is *file absent*; `Err` is +/// *file present but unreadable or unparseable*; `Ok(Some(set))` is *file +/// present and valid* -- and only that last case may produce a count. This +/// mirrors [`load_expectations`], which returns `Ok(None)` rather than an +/// empty ledger because T31 is the bug where a gate treats "no oracle" as +/// "pass". +/// +/// W644: this loader collapsed all three outcomes into `HashSet::new()` behind +/// an `eprintln!`. The caller then set `summary.baseline_failures = 0`, which +/// feeds `summary.acceptable` -- so an absent or corrupt baseline was +/// bit-for-bit indistinguishable from the real one, which legitimately holds +/// zero expected failures today. The empty set was the *right* answer for the +/// wrong reason, which is why nothing ever noticed. Absence is not amnesty +/// (T31); a caller must decide what absence means, in the open. +/// +/// A present file whose `expected_failures` is missing, is not an array, or +/// holds a non-string entry is *unparseable*, not empty: each of those silently +/// shrank the baseline, which is the same defect wearing a valid-JSON hat. +fn load_gen_verilog_smoke_baseline(repo: &Path) -> anyhow::Result>> { + let path = gen_verilog_smoke_baseline_path(repo); + if !path.exists() { + return Ok(None); + } + let raw = fs::read_to_string(&path) + .with_context(|| format!("reading smoke baseline {}", path.display()))?; + let json: serde_json::Value = serde_json::from_str(&raw) + .with_context(|| format!("parsing smoke baseline {}", path.display()))?; + let arr = json + .get("expected_failures") .and_then(|v| v.as_array()) - .map(|arr| { - arr.iter() - .filter_map(|v| v.as_str().map(|s| s.to_string())) - .collect() - }) - .unwrap_or_default() + .ok_or_else(|| { + anyhow::anyhow!( + "smoke baseline {} has no `expected_failures` array.\n\ + A baseline that cannot be read is not an empty baseline -- \ + absence is not amnesty (T31).", + path.display() + ) + })?; + let mut set = HashSet::new(); + for (i, v) in arr.iter().enumerate() { + let s = v.as_str().ok_or_else(|| { + anyhow::anyhow!( + "smoke baseline {}: `expected_failures[{}]` is {}, not a string.\n\ + Dropping it would silently shrink the baseline -- absence is \ + not amnesty (T31).", + path.display(), + i, + v + ) + })?; + set.insert(s.to_string()); + } + Ok(Some(set)) } fn rel_arg(repo: &Path, file: &Path) -> anyhow::Result { @@ -1867,7 +1894,21 @@ pub fn run_comprehensive(repo_root: &Path, opts: SuiteOptions) -> anyhow::Result println!("--- Phase 3b: Gen Verilog Yosys Smoke ---"); let mut p3b_fail = 0usize; let mut p3b_skipped = 0usize; - let baseline = load_gen_verilog_smoke_baseline(&repo); + // W644: the caller must decide what absence means, and it decides FAIL. + // `gen_verilog_smoke_baseline.json` is tracked, so a missing file is a + // broken path or a lost file -- never a reason to report `BASELINE + // FAILURES: 0`, which is also what the real, legitimately-empty baseline + // reports. `?` on the Err arm covers the present-but-unparseable case. + let baseline = match load_gen_verilog_smoke_baseline(&repo)? { + Some(b) => b, + None => anyhow::bail!( + "gen-verilog-yosys-smoke baseline is MISSING: {} does not exist.\n\ + `baseline_failures` feeds `acceptable`, so an absent baseline would \ + report 0 -- identical to a baseline that is genuinely clean. \ + Absence is not amnesty (T31).", + gen_verilog_smoke_baseline_path(&repo).display() + ), + }; let (p3bp, p3bf, p3b_known_failures) = if yosys_available() { let mut smoke_targets = specs_scratch.clone(); for rel in igla_clean_specs() { @@ -2378,14 +2419,23 @@ pub fn run_comprehensive(repo_root: &Path, opts: SuiteOptions) -> anyhow::Result // name -- an allowance that is visible, counted, and will stop applying // the moment somebody settles it. const CATALOG_ALLOWED: &[&str] = &["gfternary"]; - let cat = std::path::Path::new("specs/numeric/formats_catalog.t27"); // W643: the worst instance of the shape. This is a GATE -- its findings // count into `gate_fail` -- and it sat behind a bare `if cat.is_file()` // with no `else`, so a missing catalog printed `gate failures: 0` and // then announced the catalog gate as clean. `formats_catalog.t27` is // tracked; absence is a broken path, not a configuration. - require_target_file("catalog gate (phase 7)", cat)?; - match crate::catalog_gate::run(cat, std::path::Path::new("specs")) { + // + // W644: both operands were still RELATIVE, so what this gate examined -- + // and, after W643, whether the whole suite aborted -- depended on the + // cwd the binary happened to be invoked from. `repo` is already + // canonicalized, so under the only configuration this gate has ever run + // in (cwd == repo root) these resolve to exactly the same two paths and + // not one finding changes; resolving them only removes the cwd from the + // answer, and makes `require_target_file` name an absolute path. + let cat = repo.join("specs/numeric/formats_catalog.t27"); + let specs_root = repo.join("specs"); + require_target_file("catalog gate (phase 7)", &cat)?; + match crate::catalog_gate::run(&cat, &specs_root) { Ok(r) => { let unexpected: Vec<_> = r .findings @@ -2404,7 +2454,20 @@ pub fn run_comprehensive(repo_root: &Path, opts: SuiteOptions) -> anyhow::Result println!(" FAIL [{}] {}: {}", f.check, f.id, f.detail); } } - Err(e) => println!(" catalog gate: could not run ({})", e), + // W644: the other half of the same fail-open. `require_target_file` + // above proves the catalog exists, so reaching this arm means the + // gate could not READ a file that is right there -- and printing a + // line then falling through left `gate_fail` untouched and the + // "all clean" banner below intact. A gate that could not run is not + // a gate that passed. Absence is not amnesty (T31), and neither is + // an unreadable input. + Err(e) => anyhow::bail!( + "catalog gate (phase 7) could not run: {} ({}).\n\ + The file exists -- this is an unreadable or corrupt input, not \ + a clean gate. Absence is not amnesty (T31).", + cat.display(), + e + ), } println!(" gate failures: {}", gate_fail); if gate_fail == 0 { @@ -3377,13 +3440,88 @@ mod tests { r#"{"expected_failures": ["specs/a.t27", "specs/b.t27"]}"#, ) .unwrap(); - let set = load_gen_verilog_smoke_baseline(&tmp); + let set = load_gen_verilog_smoke_baseline(&tmp) + .expect("a present, valid baseline must load") + .expect("a present baseline is Some, not None"); assert!(set.contains("specs/a.t27")); assert!(set.contains("specs/b.t27")); assert_eq!(set.len(), 2); let _ = std::fs::remove_dir_all(&tmp); } + // W644: the counterpart, for the BASELINE, of + // `a_missing_ledger_is_none_not_an_empty_ledger` above. That test has kept + // absence out of the oracle since W628. Nothing kept absence out of the + // baseline, and the baseline is the harder case to spot: the tracked file + // legitimately holds zero expected failures, so the fail-open empty set was + // observationally identical to the correct answer on every run to date. + + #[test] + fn a_missing_baseline_is_none_not_an_empty_baseline() { + // T31: `baseline_failures` feeds `acceptable`. An empty set for a file + // that is not there means "nothing was expected to fail" -- a claim the + // loader has no evidence for. None means "the caller decides". + let tmp = + std::env::temp_dir().join(format!("t27_no_such_baseline_644_{}", std::process::id())); + let _ = std::fs::remove_dir_all(&tmp); + std::fs::create_dir_all(&tmp).unwrap(); + assert!( + super::load_gen_verilog_smoke_baseline(&tmp) + .expect("an absent baseline is not an error, it is None") + .is_none(), + "a missing baseline must be None, never an empty set" + ); + let _ = std::fs::remove_dir_all(&tmp); + } + + #[test] + fn a_corrupt_baseline_is_an_error_not_an_empty_baseline() { + // The three cases must stay distinguishable: absent (None), corrupt + // (Err), valid (Some). Only the third may produce a count. Each payload + // below used to yield `HashSet::new()` and a silent `BASELINE FAILURES: + // 0` -- indistinguishable from the genuinely clean baseline this repo + // actually ships. + let corrupt: &[(&str, &str)] = &[ + ("not json at all", "{ not json"), + ("no expected_failures key", r#"{"branch": "wave-loop-455"}"#), + ( + "expected_failures not an array", + r#"{"expected_failures": 7}"#, + ), + ( + "a non-string entry silently dropped", + r#"{"expected_failures": ["specs/a.t27", 7]}"#, + ), + ]; + for (i, (what, payload)) in corrupt.iter().enumerate() { + let tmp = std::env::temp_dir().join(format!( + "t27_corrupt_baseline_644_{}_{}", + std::process::id(), + i + )); + let _ = std::fs::remove_dir_all(&tmp); + let docs = tmp.join("docs").join("reports"); + std::fs::create_dir_all(&docs).unwrap(); + std::fs::write(docs.join("gen_verilog_smoke_baseline.json"), payload).unwrap(); + let got = super::load_gen_verilog_smoke_baseline(&tmp); + assert!( + got.is_err(), + "{} must be an error, not an empty baseline", + what + ); + // The message must name the file, or a reader cannot tell a corrupt + // baseline from a wrong repo root. + let msg = format!("{:#}", got.unwrap_err()); + assert!( + msg.contains("gen_verilog_smoke_baseline.json"), + "{}: names the file: {}", + what, + msg + ); + let _ = std::fs::remove_dir_all(&tmp); + } + } + fn make_fake_tri_script(report_path: &Path, passed: bool) -> PathBuf { let script_dir = report_path .parent() diff --git a/docs/NOW.md b/docs/NOW.md index 9dec98753c..e52770b3f8 100644 --- a/docs/NOW.md +++ b/docs/NOW.md @@ -1,3 +1,153 @@ +# NOW -- an absent or corrupt gate input now fails instead of counting as zero (2026-08-20) + +Last updated: 2026-08-20 + +## fix(suite): absence and corruption are not a clean pass (the input side of #2285) + +#2285 gave `bootstrap/src/suite.rs` a floor for the **target list**: a phase with +nothing to check now fails, naming where the targets were supposed to come from. +Two sites of the same shape on the **input** side were left, and both still +reported a clean pass when what they read was missing or unreadable. + +The discipline was already in the file and neither site followed it. +`load_expectations` returns `Ok(None)` for a missing ledger, never an empty one, +because *"T31 is the bug where a gate treats 'no oracle' as 'pass'"*, and the +ratchet turns that `None` into `RATCHET: FAIL -- Absence is not amnesty (T31)`. + +### A. The phase 7 catalog gate asked the working directory, not the tree + +`require_target_file` from #2285 closed the silent skip -- the bare +`if cat.is_file()` with no `else` that printed `gate failures: 0` for a gate +that never ran. It did not close two things behind it. + +**Both operands were relative.** `Path::new("specs/numeric/formats_catalog.t27")` +and `Path::new("specs")` resolve against the process cwd, so what this gate +examined -- and, after #2285, whether the whole suite aborted -- depended on +where the binary happened to be invoked. The failure message named a relative +path, which cannot tell a wrong cwd from a lost file. Both are now joined onto +the already-canonicalized `repo`, so the diagnostic names an absolute path and +the answer no longer depends on the caller's shell. + +**The `Err` arm still failed open.** `require_target_file` proves the file is +there, so reaching `Err(e) => println!(" catalog gate: could not run ({})", e)` +meant the gate could not *read* a file that exists. It printed one line, left +`gate_fail` untouched, and eleven lines later the block still printed +`(lexer/parser conformance and the catalog gate are all clean)`. A gate that +could not run was announced as a gate that passed. It is now a `bail!`. + +### B. A corrupt baseline was byte-identical to a clean one + +`load_gen_verilog_smoke_baseline` collapsed **six** distinct conditions into +`HashSet::new()` behind an `eprintln!`: file missing, file unreadable, invalid +JSON, no `expected_failures` key, that key not an array, and a non-string entry +inside the array. The caller then did + +```rust +summary.baseline_failures = baseline.len(); // 0 +``` + +and `baseline_failures` feeds `summary.acceptable`: + +```rust +summary.acceptable = summary.passed + || (summary.known_failures.iter().cloned() + .collect::>().len() == summary.baseline_failures + && total_fail == summary.known_failures.len()); +``` + +What made this one invisible: `docs/reports/gen_verilog_smoke_baseline.json` +legitimately holds `"expected_failures": []` today -- Wave Loop 455 cleared the +tuple-return, let-destructuring and ROM lowering gaps, and the file is kept as a +schema artifact. **So the fail-open path produced exactly the same output as the +real file.** It has been giving the right answer for the wrong reason on every +run since, which is why nothing ever caught it. An empty set was never evidence; +it was the absence of evidence wearing the shape of evidence. + +The loader now returns `anyhow::Result>>` -- `Ok(None)` +is *absent*, `Err` is *present but unreadable or unparseable*, `Ok(Some(set))` +is *present and valid*, and only the third may produce a count. That is +`load_expectations`' signature and `load_expectations`' reason. The caller +decides what absence means, in the open, and it decides FAIL: the file is +tracked, so a missing one is a broken path, not a configuration. + +### What changed + +| site | before | after | +|---|---|---| +| `load_gen_verilog_smoke_baseline` | `HashSet`; six conditions -> empty set + `eprintln!` | `anyhow::Result>>`; absent / corrupt / valid stay distinct | +| its caller, phase 3b | `summary.baseline_failures = 0` for an absent or corrupt file | `?` on corrupt, `bail!` naming the absolute path on absent | +| phase 7 catalog target | relative to cwd | `repo.join(...)`, so `require_target_file` names an absolute path | +| phase 7 specs root | relative to cwd | `repo.join("specs")` | +| phase 7 `Err` arm | `println!` and fall through with `gate_fail` unchanged | `bail!` naming the file and the error | + +Three tests. `a_missing_baseline_is_none_not_an_empty_baseline` is the +counterpart, for the baseline, of `a_missing_ledger_is_none_not_an_empty_ledger` +-- which has kept absence out of the **oracle** since W628 while nothing kept it +out of the **baseline**. `a_corrupt_baseline_is_an_error_not_an_empty_baseline` +runs four payloads (not JSON, no key, key not an array, a non-string entry) and +asserts each is an `Err` whose message names the file, because a reader who +cannot see the filename cannot tell a corrupt baseline from a wrong repo root. +The existing `test_load_gen_verilog_smoke_baseline` is updated for the new +signature and now also asserts a present file is `Some`, not `None`. + +### No criterion moved + +`CATALOG_ALLOWED` is untouched, so `gfternary` is still allowed by name. +`catalog_gate::run` is untouched. The `acceptable` formula is untouched. The +baseline file is untouched. Under the only configuration in which these gates +have ever run -- cwd == repo root -- `repo.join("specs")` and `Path::new("specs")` +resolve to the same directory and not one finding changes; resolving them only +removes the cwd from the answer. + +### Effect on CI today: none + +The only workflow that runs the suite is +`.github/workflows/corpus-ratchet.yml`, which invokes +`t27c suite --repo-root . --ratchet --corpus-only` from the repo root. +`docs/reports/gen_verilog_smoke_baseline.json` and +`specs/numeric/formats_catalog.t27` are both tracked and both present, and the +cwd is the repo root, so every new guard passes and every printed number is +identical. + +The change is visible the moment one of them stops being true. Unlike the phase +3b failure **count**, these guards are CI-reachable: they `bail!` out of +`run_comprehensive` before the ratchet block, so they fail the job regardless of +`--ratchet` bypassing `total_failures`. + +### Honesty limits (BINDING) + +- **It does not restore the coverage the scratch untrack took.** #2283 took + phase 3b from 482 targets to 27. It stays at 27. Nothing here re-creates a + test subject; this change is about what happens when an input is missing, not + about what the inputs cover. +- **It does not detect a phase that shrank without reaching zero.** 482 -> 27 is + a 94.4% loss and passes every guard in this file, #2285's included. A ratchet + on target *counts* has no existing mechanism to extend -- the expectations + ledger keys on `(path, phase)` failures, not on target populations. +- **Phase 3b still cannot fail CI, because it never calls `record(...)`.** + Thirteen `record(...)` calls build the ledger the ratchet gates on; + `gen-verilog-yosys-smoke` is not one of them, so its failures never enter the + ratchet, and with `--ratchet` the exit code is the ratchet verdict alone -- + `total_fail`, which does include `p3b_fail`, is bypassed entirely. CI also + installs no yosys, so the phase is skipped there in any case. **That is a + separate defect and this change does not fix it.** What is fixed is narrower: + the baseline that phase 3b reports against can no longer be absent or corrupt + without saying so. +- **The phase 6 guard's label is now slightly wide.** #2285's + `require_targets("phase 6 integrity metrics + phase 7 catalog gate", ...)` + still guards the relative `specs` walk the phase 6 metrics use, and is left + exactly as it is; phase 7 simply no longer depends on it. The five phase 6 + metrics remain cwd-relative. They are reporting-only and excluded from + `total_fail`, so they are out of scope here rather than fixed. +- **Not compiled.** The crate was not built for this change; disk headroom on + the authoring machine was 3.1 GiB. `suite.rs` was parse-checked with + `rustfmt --edition 2021` (no diagnostics on stderr), which proves the file is + valid Rust syntax and proves nothing about types or borrows. CI's + `cargo build --release -p t27c` in `corpus-ratchet.yml` is the first real + compile. + +Closes #2286. + # NOW -- a suite phase with no targets now fails instead of passing (2026-08-20) Last updated: 2026-08-20