Skip to content

smoke-gate verdict ANDs dry_run_sweep_ok, which nothing ever sets — cli-tri build cannot pass #2304

Description

@gHashTag

Summary

cli-tri / build has never been green. #2303 fixed one of its two causes (yosys was installed after the tests that needed it). The second cause is still on master: the smoke-gate verdict ANDs a flag that nothing ever sets.

In cli/tri/src/fpga.rs, dry_run_sweep_ok has exactly two mentions in the whole file:

6098:    let mut dry_run_sweep_ok = false;
6333:        && dry_run_sweep_ok

Declared false, read in the final conjunction, never assigned true. The dry-run sweep phase runs, succeeds, and prints its OK line — but nothing records that it did, so passed is false for every possible input.

Unlike its siblings, this conjunct is unguarded. verify_lean_ok and validate_lean_standalone_ok are read as (!run_verify_lean || verify_lean_ok), so they are vacuously true when the phase is not requested. && dry_run_sweep_ok has no such escape, so it fails the gate unconditionally.

Evidence from the post-#2303 master run

Run 32352361519, job 96374085002 — every phase reports OK and the gate still fails:

[smoke-gate] dry-run sweep report OK (8 variants)
[smoke-gate] verify-lean OK (source=synthetic, theorems present)
[smoke-gate] yosys synthesis OK
[smoke-gate] complete (passed: false)

thread 'fpga::tests::test_smoke_gate_json_synthetic_verify_lean' panicked at cli/tri/src/fpga.rs:9977:9:
smoke-gate synthetic verify-lean path failed: Err(smoke-gate did not pass all phases)

test result: FAILED. 155 passed; 1 failed; 0 ignored

yosys synthesis OK confirms #2303 landed and worked. dry_run_sweep_ok is the only remaining false conjunct, and that one test is the only failure in the job.

The compiler has been reporting it for all 12 runs

warning: variable does not need to be mutable
    --> cli/tri/src/fpga.rs:6098:9
     |
6098 |     let mut dry_run_sweep_ok = false;
     |         ----^^^^^^^^^^^^^^^^

A mut that is never mutated is the signature of exactly this defect. Warnings are not denied in this job, so it never turned the build red on its own — it was just printed and scrolled past, 12 times.

Prior art — this is a known shape, second occurrence

Line ~6167 documents the identical defect for the sibling flag verify_lean_ok, restored from 494e659d8:

run_verify_lean stayed a parameter and verify_lean_ok stayed a declaration nothing ever set, so the smoke gate could not pass whenever a caller asked for that phase — which is precisely what its own regression test asks for. Restored from 494e659, the commit that wrote it. Same shape as the seven definitions lost in #2228.

dry_run_sweep_ok was lost in the same batch merge and missed on that pass.

The report entry is missing too

The same success path is also the only place that would write report["dry_run_sweep"]. Today it is written only on failure (line 6148); on success it stays null from the initializer at 5967. So even with the flag set, the test's next assertion still fails:

for key in ["bit_config", "dry_run_sweep", "verify_lean"] {
    let phase = report.get(key).expect("missing phase");
    assert!(phase.is_object(), "phase {} should be populated: {}", key, phase);

verify_lean sets flag and report at its success point. Mirroring it fixes both halves.

Fix

Mirror verify_lean_ok exactly at the point where the sweep's success is established — after the variant-count bail, before the OK line:

dry_run_sweep_ok = true;
report["dry_run_sweep"] = serde_json::json!({ "status": "ok", ... });
println!("[smoke-gate] dry-run sweep report OK ({} variants)", variant_count);

Not in scope, and explicitly not done: weakening the conjunction. Dropping && dry_run_sweep_ok would turn the check green by verifying less, which is the same absence-is-not-a-value failure #2285, #2287 and #2302 each closed.

Known follow-up, not fixed here

validate_lean_standalone_ok (line 6100) has the same two-mention shape — and worse, the entire validate_lean_standalone phase body is absent from smoke_gate(); the flag has no success point to attach to. It is currently masked because its conjunct is guarded and both of its tests skip when lake is not on PATH. It will bite the moment a runner has lake. Filing separately.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions