fix(rigs): wire rig_https_banner.py through prepare_device — repairs red master after #225 + #227 - #228
Merged
Merged
Conversation
…ession) master is red. #227 gave this rig a turbo probe; #225's KNOWN_UNPREPPED exempted this rig from device prep because it had no device-state handling at all. Both true, both green alone, and the composition reintroduced #187's degrade-toward-the-write in the one file the contract had excused. #225's two idiom guards scan every tools/uci/*.py and do not honour the exemption, so they caught it — which is the guards working, a PR later than either author could have seen it. The fix is the follow-up both PR bodies already said was owed: * the inline `get_config_category(CAT_U64_SPECIFIC)` probe and its degrade are gone, replaced by `prepare_device(..., turbo_mhz=48, artifact_dir=prep_dir)` under the DeviceLock after `enable_uci`, matching the five existing call sites. An unreadable turbo state now aborts (C64_FORCE_TURBO_WRITE=1 overrides) instead of writing blind. * `preflight_reu` added behind it, in that order. This was owed before and matters more now: the rig is a completeness oracle with a 900 s default budget, so a comb PRG on a REU-disabled device — the documented factory default — costs far more before failing than when the rig only read a banner. * 48 MHz, asserted rather than inherited, matching the rig's default. The clock is load-bearing here specifically: a 1 MHz device turns a slow fetch into a false TRUNCATED, and a comb boot at 1 MHz needs ~36 min against C64_INIT_WAIT, which is #212's own failure. * a per-run directory for device_state.json, since this rig runs no bus capture and so never had one — `bench_ecdsa_u64e.py`'s capture-off branch without the branch. `_create_run_dir`/`_prune_old_run_dirs` are imported from `rig_https_local` as `rig_https_wiki.py` already does, rather than copied a third time. * `KNOWN_UNPREPPED` is now empty, with the episode recorded in its place: the entry covered PREP and not PROBING, and an entry there is a statement about a file at a moment. The one policy NOT folded in is the `ValueError` from an unsupported TURBO_MHZ. `prepare_device` calls the harness's `set_turbo_mhz` uncaught, so that escapes as a ValueError rather than DevicePrepError and would be a traceback at exit 1 on all six call sites. It is caught here and mapped to this rig's fatal code, with a comment saying it belongs in `_device_prep` instead — reported rather than edited, since that module is another lane's. Red-green, on this branch's tree: with master's rig in place the suite reports 3 failed / 27 passed (both idiom guards, plus the prep guard once the exemption is emptied); with the fix, 30 passed. Full `pytest`: 207 passed, 0 failed, up from master's 205/2. `tools/test_http_body_ checks_unit.py` 10/10 and `tools/mutate_http_body_checks.py` 27/27 are unchanged, and the mutation runner still restores src/http.s cleanly. No PRG bytes change: `git diff master -- src cfg Makefile` is empty and uci-onchip rebuilds to a3452651179e22d3328cda4298d2dfbf0b03f1d40747203c d3436dbd6ccc3a92. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Fixes a red master.
2 failed, 28 passed→207 passed, 0 failed.What broke, and why no single review could see it
Master's
rig_https_banner.pyhad no turbo concept. #227 gave it one —client.get_config_category(CAT_U64_SPECIFIC)and a"probe failed … writing anyway"degrade. #225's two idiom guards scan everytools/uci/*.pyfor exactly those and, unliketest_every_crypto_rig_prepares_before_it_checks, do not honourKNOWN_UNPREPPED.So #227 independently reintroduced the precise #187 defect that #225 exists to eliminate, in the one rig #225 had exempted from prep on the grounds that it had no device-state handling — a premise that stopped being true one PR later.
Both PRs were green alone. Both PR bodies named this integration as owed. Neither tree contained the other's code, so the defect existed only in the composition and appeared in no diff. It was found by trial-merging all five branches before merge, not by review.
The fix
The inline probe and its degrade are gone.
rig_https_banner.pynow callsprepare_device(client, LABELS_PATH, turbo_mhz=TURBO_MHZ, artifact_dir=prep_dir)under the DeviceLock afterenable_uci, withpreflight_reubehind it in that order — matching the five existing call sites. 48 MHz asserted, matching the rig's own default.KNOWN_UNPREPPEDis now empty.preflight_reuwas owed independently: #227 promoted this rig to a completeness oracle with a 900 s budget, so a comb PRG on a REU-disabled device (the documented default) is an expensive way to fail.artifact_dir: this rig runs no bus capture and so never had a run dir. It always creates one —bench_ecdsa_u64e.py's capture-off branch without the branch — importing_create_run_dir/_prune_old_run_dirsfromrig_https_localasrig_https_wiki.pyalready does, rather than making a third copy.One residual gap in the shared module, deliberately not fixed here
prepare_devicecalls the harness'sset_turbo_mhzuncaught (_device_prep.py:502), so an unsupportedTURBO_MHZescapes asValueError, notDevicePrepError— its docstring documents two raise cases and this is a third. On all six call sites that is a traceback at exit 1. This PR catches it at the call site and maps it to the rig's fatal 2, with a comment saying it belongs in_device_prepso all six agree.Left for
_device_prep's owner on purpose: this whole episode is what happens when two lanes edit one seam. It is exception-type mapping, not a second copy of the probe or the failure policy, sotest_rigs_do_not_hand_roll_the_turbo_probeis not being routed around.Red-green
With master's rig restored into this branch's tree:
Three, not the two master shows — emptying
KNOWN_UNPREPPEDalso reddens the prep guard against the unwired rig, so the exemption's removal is itself covered.Verification
pytest: 207 passed, 0 failed, verified independently on a trial-merge of this branch into master.git diff master -- src cfg Makefileempty; uci-onchip rebuilds toa3452651179e22d3328cda4298d2dfbf0b03f1d40747203cd3436dbd6ccc3a92(the post-UCI: three net_tcp_send timeout exits return without DATA_ACC, leaving the FPGA out of idle (plausible $86 source) #194/fix(uci): abort the transaction on net_tcp_send timeout exits (#194) #223 hash).test_http_body_checks_unit.py10/10;mutate_http_body_checks.py27/27,KNOWN_EQUIVALENTempty,src/http.srestored cleanly after the run.Worth knowing for anyone who sees a red
test_uci_timeout_recovery.py: it executes shipped bytes and is build-state dependent liketest_uci_data_acc.py. A stale pre-#223build/gives 3 failures that are not a regression;make clean && make BACKEND=uci …clears them.Follow-up worth filing:
KNOWN_UNPREPPEDentries are unfalsifiable claimsThe guards are correct as designed and should not honour the exemption — unlike prep, which a rig may legitimately not have done yet, a hand-rolled probe is a second copy of a policy that already has one home, and no state of a rig makes that right. Exempting it would have turned a loud composition failure into a silent one.
The real gap is that an exemption's premise is never re-checked. This entry said the rig "writes no turbo and no REU"; that became false one PR later and the exemption stayed, with a stated reason no longer true of the file. Two cheap fixes, preferring the first: invalidate an entry whose premise is falsified (a
KNOWN_UNPREPPEDrig mentioningset_turbo_mhz,CPU Speedor any REU item is no longer "a rig with no device-state handling", so the exemption fails rather than applies); or carry the file's hash so it expires on any change. The first encodes the actual premise instead of "this file has not changed".🤖 Generated with Claude Code