fix(suite): a phase whose target list is empty must fail, naming the source - #2285
Merged
Conversation
…source suite.rs has carried a non-empty floor for the ORACLE since W628 and never for the TARGETS. load_expectations returns Ok(None) for a missing ledger, documented as "never an empty ledger", and the ratchet turns that into "RATCHET: FAIL -- Absence is not amnesty (T31)". Ten lines away, collect_t27 returns Ok(Vec::new()) for a directory that is not there; every phase iterates it happily, run_phase returns (0, 0), and the phase prints "0 passed, 0 failed" into a suite that then prints ALL TESTS PASSED. No phase in run_comprehensive checked files.is_empty(), and no test anywhere asserted a phase's target count. It was already live. Untracking specs/scratch (#2283) took icarus-simulate and icarus-cocotb from 155 targets to 0 and gen-verilog-yosys-smoke from 482 to 27. 281 baselines are still tracked under .trinity/icarus-baselines/specs/scratch/ -- golden files whose subject no longer exists and whose comparing phase has no targets. Nothing said a word, and the ratchet could not: its 221 ledger entries include 0 under scratch. Adds require_targets(phase, source, files) and its single-file form require_target_file. Both name where the targets were supposed to come from, because every instance of this bug looks identical from the summary line and completely different at the source. Six call sites: the post-corpus-only spec lists (phases 1-6), phase 1c's gf16 spec (which reported "skipped (spec not found)" for a conformance gate that checked nothing), phase 3b's smoke list, phases 3d and 3e, the relative "specs" root shared by the phase 6 metrics, and phase 7's catalog gate -- which sat behind a bare "if cat.is_file()" with no else and printed "gate failures: 0" followed by "the catalog gate are all clean" for a gate that never ran. collect_t27 is deliberately unchanged: specs/scratch is legitimately absent since #2283, so the walker must keep returning an empty vec. Only the call site knows whether zero is a defect or a configuration. A test pins that division of labour. Four named opt-outs, all "the phase does not run": the standalone specs_scratch binding, phase 3b without yosys, phase 3d without its flag or iverilog, phase 3e without --cocotb. No CI effect today: corpus-ratchet.yml walks 650 corpus specs from the repo root, both fixed specs are tracked, and it installs neither yosys nor iverilog. Every guard passes. Running the suite with --icarus-simulate now fails, naming specs/scratch, which is the honest outcome. Does not restore the coverage the untrack removed, and does not detect a phase that shrank without reaching zero. Not compiled locally; suite.rs was parse-checked with rustfmt only. See docs/NOW.md. Closes #2284
Contributor
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
gHashTag
enabled auto-merge (squash)
August 20, 2026 03:05
Contributor
This was referenced Aug 20, 2026
gHashTag
added a commit
that referenced
this pull request
Aug 20, 2026
…ero (#2287) #2285 gave suite.rs a floor for the TARGET LIST. Two sites of the same shape on the INPUT side were left, and both reported a clean pass when what they read was missing or unreadable. The discipline was already in the file. load_expectations returns Ok(None) for a missing ledger and never an empty one -- "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)". Neither site followed it. A. Phase 7 catalog gate asked the working directory, not the tree. require_target_file closed the silent skip in #2285, but both operands stayed RELATIVE, so what the gate examined -- and whether the suite aborted -- depended on the cwd the binary was invoked from, and the failure named a relative path that cannot distinguish a wrong cwd from a lost file. Both are now joined onto the canonicalized repo root. The Err arm also still failed open: with the file proven present, reaching it meant the gate could not READ a file that exists, yet it printed one line, left gate_fail untouched, and the block below still printed "(lexer/parser conformance and the catalog gate are all clean)". It now bails. B. A corrupt baseline was byte-identical to a clean one. load_gen_verilog_smoke_baseline collapsed six conditions into HashSet::new() behind an eprintln!: missing file, unreadable file, invalid JSON, no expected_failures key, that key not an array, a non-string entry inside it. The caller set summary.baseline_failures = 0, which feeds summary.acceptable. The tracked baseline legitimately holds "expected_failures": [], so the fail-open path produced exactly the same output as the real file -- the right answer for the wrong reason on every run to date, which is why nothing caught it. It now returns anyhow::Result<Option<HashSet<String>>>: Ok(None) is absent, Err is present-but-unparseable, Ok(Some) is present-and-valid, and only the third may produce a count. That is load_expectations' signature and its reason. The caller decides in the open, and decides FAIL -- the file is tracked, so absence is a broken path, not a configuration. 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 corrupt payloads and asserts each is an Err naming the file. No criterion moved. CATALOG_ALLOWED, catalog_gate::run, the acceptable formula and the baseline file are untouched. With cwd == repo root -- the only configuration these gates have run in -- the resolved paths are identical and no finding changes. Effect on CI today: none. corpus-ratchet.yml runs from the repo root with both files present, so every guard passes and every printed number is identical. Honesty limits: this does not restore the 482 -> 27 coverage #2283 took from phase 3b, does not detect a phase that shrank without reaching zero, and phase 3b still cannot fail CI because it never calls record(...) -- a separate defect, unfixed. Not compiled: parse-checked with rustfmt only, so types and borrows are unproven until CI's cargo build. Closes #2286. Co-authored-by: Claude <claude@anthropic.com>
This was referenced Aug 20, 2026
gHashTag
added a commit
that referenced
this pull request
Aug 20, 2026
The cli-tri `build` job has never passed. Not regressed -- never passed: 11 runs on master, 11 failures, and 0 successes across all 56 runs on every branch the workflow has ever run on. ubuntu-latest ships no yosys, and the install sat as a line inside the run block of the final step, which is ordered after `cargo test -p tri`. So fpga::tests::test_smoke_gate_json_synthetic_verify_lean -- which calls the real smoke_gate(), whose verdict ANDs in yosys_ok -- hit "[smoke-gate] SKIP: yosys not on PATH" on every run. A failed step aborts the job, so the install step had never actually executed once. Hoisted into its own named `install yosys` step placed before the test step. The inline copy installed yosys and nothing else, so it is removed in full rather than trimmed; the comment on the final step now says yosys arrives from the step above instead of claiming to install it. Every other step keeps its order and content. cli/tri/src/fpga.rs is untouched. Relaxing the assertion to tolerate a missing binary would have produced a green check that verified no synthesis at all -- the absence-is-not-a-value defect already closed in GH-2285 and GH-2287. The YAML was wrong, not the test. Checked that yosys was the only gap: smoke_gate() spawns no lake, python3 or nextpnr, theorem_matrix_ok is a hardcoded true, and the verify-lean phase is a pure-Rust synthetic fixture. Closes #2302
This was referenced Aug 20, 2026
Closed
Merged
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
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.
Closes #2284.
The defect
bootstrap/src/suite.rshas carried a non-empty floor for the oracle since W628 and has never carried one for the targets.load_expectations(suite.rs:292-301) returnsOk(None)for a missing ledger, doc-commented "A missing file isOk(None), never an empty ledger. T31 is the bug where a gate treats 'no oracle' as 'pass'", and the ratchet turns thatNoneintoRATCHET: FAIL -- Absence is not amnesty (T31).Ten lines away,
collect_t27returnsOk(Vec::new())for a directory that is not there. Every phase iterates that happily,run_phasereturns(0, 0), and the phase prints0 passed, 0 failedinto a suite that then printsALL TESTS PASSED. No phase inrun_comprehensivecheckedfiles.is_empty(), and no test anywhere asserted a phase's target count --collect_t27,icarus_regression_specsandsmoke_targetsappeared in no test in this repo.It was already live. Untracking
specs/scratch(#2283) tookicarus-simulateandicarus-cocotbfrom 155 targets to 0 andgen-verilog-yosys-smokefrom 482 to 27. 281 baselines are still tracked under.trinity/icarus-baselines/specs/scratch/-- golden files whose subject no longer exists and whose comparing phase has no targets. Nothing said a word, and the ratchet could not: its 221 ledger entries include 0 underspecs/scratch.What this changes
require_targets(phase, source, files)fails an empty list;require_target_fileis its single-file form. Both name where the targets were supposed to come from -- every instance of this bug looks identical from the summary line and completely different at the source: a wrong path, a deleted tree, a filter that matched nothing.--corpus-onlyfilterspecs_compiler,specs_only-- phases 1, 1a2-1a7, 1b, 2, 2b, 3, 4, 5, 60 passed, 0 failed, thenALL TESTS PASSEDspecs/numeric/gf16.t27GF16: skipped (spec not found),skipped=1-- a conformance gate that checked nothing, filed as a skipsmoke_targetsigla_clean_specs()would silently retire the gatesim_targets/cocotb_targetsspecsroot shared by dup-names, check-calls, lex-dropped,cc_gate,impl_statusand the phase 7 catalog gatespecs/numeric/formats_catalog.t27if cat.is_file()with noelse:gate failures: 0then(lexer/parser conformance and the catalog gate are all clean)for a gate that never ran. Its findings count intogate_fail, so this was a false greencollect_t27is deliberately unchanged.specs/scratchis legitimately absent since #2283, so the walker must keep returning an empty vec; only the call site knows whether zero is a defect or a configuration.the_floor_lives_in_the_guard_not_in_the_walkerpins that division of labour -- it is the part a future edit is most likely to "simplify" back into the bug.Four tests, sited next to
a_missing_ledger_is_none_not_an_empty_ledger, which they are the counterpart to.Opt-outs (four, each named with its reason)
All four are "the phase does not run", none is a blanket exemption:
specs_scratch(suite.rs:1585) is not guarded. chore(specs): untrack specs/scratch -- 455 files, 578 MB, 64.5% of the tracked tree #2283 untracked all 455 files underspecs/scratch/; an empty walk there is the tree as designed. It is one of two inputs to phase 3b, and the combined list is guarded.yosys. The guard is inside theyosys_available()branch. A skipped phase has no target list to be empty; it already reportsskipped=1.--icarus-simulate/--icarus-lowerable, or withoutiverilog/vvp. W641'sskipped_notealready printsSKIPPED (not run -- pass the flag to enable)rather than0.--cocotb, or the same missing tools.Phases 3c and 3c-standalone are out of scope rather than opted out: a single external
triinvocation with its own skip semantics, not a spec walk.Effect on CI: none today
The only workflow that runs the suite is
.github/workflows/corpus-ratchet.yml(t27c suite --repo-root . --ratchet --corpus-only, from the repo root). It walks 650 corpus specs;specs/numeric/gf16.t27andspecs/numeric/formats_catalog.t27are both tracked; it installs neitheryosysnoriverilog, so 3b is skipped and 3d/3e never run. Every guard passes. The change is visible the moment one of those stops being true.t27c suite --icarus-simulatenow fails locally, namingspecs/scratch. That is the intended, honest outcome: the phase has had no targets since #2283 and used to say so by saying nothing.Honesty limits (BINDING)
(path, phase)failures, not on target populations -- and inventing one here would have made this unreviewable..trinity/icarus-baselines/specs/scratch/are untouched.cargowas not run.suite.rswas parse-checked withrustfmt --edition 2021 --emit stdout(exit 0), which proves syntax and nothing about types or borrows.corpus-ratchet.yml'scargo build --release -p t27cis the first real compile -- if it fails, this PR is wrong and should not be merged.