PR #665 follow-ups: --force wording, retry evidence, FLAKY/FIXED overlap nf-9rb ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/667 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #665. All advisory, all below the gate floor — filed so the thread can close. Unclaimed.

1. --force announces a skip that would not have fired. tools/pr/probe_gate.py:862 gates the message on args.force and record_ok and prev_sha, but the skip itself also requires prev_sha == sha or not sweep_input_changed(...). So probe_gate.py --force on main after a merge that touched games/ prints --force: re-sweeping a tree that is already recorded (baseline abc1234) about a tree that was never swept. Cheapest fix: compute the skip predicate once and gate both arms on it.

2. The retry overwrites the evidence it tells you to file. probe_all.run_one rewrites outDir to probe_out/<name> on every run, and shared/VisualProbe.cs:691 writes <outDir>/<shot>.png — so the passing re-run replaces the failing run screenshots, which is what the FLAKY text asks the reader to open an issue about. The teed console output still carries the assertion text, so nothing is fully lost. Either rewrite the retry to probe_out/<name>-retry, or say in the FLAKY block that the shots on disk are the passing ones.

3. A baselined probe that flakes is reported twice, contradictorily. A probe already in the baseline as failing that fails-then-passes leaves sweep.failing, so compute_delta (probe_gate.py:949) puts it in delta.fixed and the report prints both ⚠ FLAKY ... not a regression ... IS a defect in the probe and ✓ FIXED since the last merge. Consider subtracting flaky from delta.fixed, or labelling that row.

4. Front-door docstrings do not mention the new surfaces. tools/pr/probe_gate.py:4-6 lists --dry-run and --show but not --force; probe_all.py opens with ./probe_all.py and Runs every tools/probes/*.json with the subset only described further down in main. Both are the first thing a session reads.

5. probe_all.main() argv wiring has no unit test. The five new tests drive sweep(..., only=[...]) directly; the positional-args-to-only path is covered only by the two real runs in the test plan. One main(["beta"]) case would pin it.

6. The unknown-name refusal fires after build_once. A drifted name costs a full dotnet build before scripts() raises. Validating the subset before the build would fail fast and keep the refusal cheap.

7. user:// is not reset between the sweep run and its retry. probe.py:157 redirects user:// to probe_out/userdata/ per clone and nothing clears it, so the retry starts from whatever the failed run left behind. Pre-existing across the sweep, but it is new that a second run of the same probe is being used as evidence about the first — a state-dependent pass would read as FLAKY.

8. A near-wipeout retries almost everything. wipeout() is failed > 0 and passed == 0, so a host fault where 33 of 34 die retries 33 probes and pays a second near-full sweep. Rare, and the conservative direction, but worth knowing the cost cliff is at exactly one passing probe.

Dependencies

None.

Comments

No comments.

Add a comment