chore: finish the pytest-boundary rename, correct CLAUDE.md against measurement, and fix #89's fresh-clone build - #115
Merged
Merged
Conversation
…st measurement Two deferred follow-ups, both left with reasons recorded by earlier lanes. PR #111 renamed tests/test_*.py -> tests/rig_*.py because those files wear pytest's discovery convention while collecting zero tests, so a bare `pytest` overstates coverage. It explicitly deferred the identical problem in tools/uci/, whose blast radius runs through CLAUDE.md. This is that change: six scripts renamed, following #111's precedent exactly. `norecursedirs` already closed the default-invocation path, but the rename is what holds from an arbitrary working directory, since `testpaths` only applies at the rootdir. Both halves are now pinned by the guard rather than one. tools/test_pytest_boundary.py grows a RIG_DIRS tuple covering both directories and a new test_norecursedirs_covers_every_rig_dir. Probed in both directions: a stray tools/uci/test_*.py fails test_rig_dirs_hold_no_pytest_named_files, and dropping tools/uci from norecursedirs fails the new check. Bare `pytest` is now 31 passed / exit 0; `pytest tests/` and `pytest tools/uci/` both exit 5 with an explanation naming the right README. New tools/uci/README.md mirrors tests/README.md. Reference sweep covers CLAUDE.md, README.md, conftest.py, pytest.ini, tests/README.md, both phase_f docs, tools/https_e2e/, tools/package/listener/, and the cross-invocations (rig_https_print_body and rig_https_local_p384 both delegate to rig_https_local; import-checked after the rename). Zero references exist in c64-test-harness or any other sibling c64-* repo, and no open issue or PR in the org names any of the six. README was audited claim-by-claim in #104; CLAUDE.md never was. Every item below was confirmed by running a command, not by reading: - The ip65 blob section contradicted itself and the Makefile. It claimed `ip65-blob` is phony with "no rule connecting the two" and that a fresh clone dies at the `.incbin`. Measured: deleting the blob and running plain `make` rebuilds it byte-identically (6,951 B, cf1a5ff7...) and links the usual 47,105 B PRG. The real fresh-clone blocker is the missing ip65 .lib archives, which fail at the blob's ld65 step. `make ip65-blob` is not a required step. Fixed here and in README. - "the committed blob" / `touch ip65-build/ip65-c64.bin` advice: the blob is gitignored, so on a fresh clone there is nothing to touch. - Fence macro: 17 bytes per site, not 14 (counted from the macro body). 26 sites, not 24 — 11 write + 14 read + 1 pre-loop settle at net.s:199. - uci_drain_resp/uci_drain_status: 22 call sites in net.s, not 13. All 22 still `bcs` out, so that half was right. - CIA1 TOD read order is HOUR -> TENTHS; MIN and SEC are never read and have no equate in uci_cmd.s. - UCI memory table: NET_CODE is $2000-$3B65 and NET_BSS_TAIL $3B66-$41FF (cfg grew NET_CODE by $40 for the C64U WiFi iface-fallback loop). UCI_BSS_REGION is size 0, so quoting a 512 B span contradicted its own label and overlapped NET_BSS_TAIL. - src/exports.s no longer exports ip65_init/ip65_process; those moved to src/net/ip65/exports.s. Named the real backend-agnostic set instead. - KEEP_DEBUG_ON_PASS is the env var; UCI_DEBUG_KEEP_ON_PASS is only the Python variable name, so the documented spelling did nothing. - rig_https_local_p384.py defaults to 90 minutes (5400 s), not 30. - C64_SKIP_BUILD: 15 scripts, not 14 (ran the recipe the file itself quotes). - EMBED_P256_OVERLAY's stated mutual exclusions have no $(error) guard: one is a silent auto-disable, the other is unguarded. Only the USE_NISTCURVES_ONCHIP exclusions are hard errors. - "All in-tree VICE-driven tests go through default_vice_config()" is false: 8 suites do, 15 other files still build ViceConfig directly — including run_all_tests.py, which hand-spells the REU flags. - Smoke-test list is 8 entries, not 7, and the 97/97 total predates test_finished_verify.py joining it. - pytest counts 30 -> 31; src/boot.s:107-114 -> 110-117. Wall-clock figures are labelled rather than re-measured, per instruction: a pin-vs-commit table resolves 2ceb5b1 / f0127a0 / cb6eab4 to libs/nistcurves v0.6.0 (verified with git ls-tree), every table states the pin it was taken at, and "at HEAD" captions that no longer refer to HEAD are gone. No hardware was used and no benchmark was re-run. Deliberately NOT included: the x25519 sibling failure. It is measured and understood (both backends exit 2 on a duplicate reu_mul_tables_init export, not the overflow CLAUDE.md records), but PR #113 rewrites the same spans and fixes the underlying collision, so the prose belongs there. Details handed to that lane. docs/library-ingestion-architecture.md's two stale references are owned and taken by the contract lane. Python and docs only — no .s, no .cfg, no Makefile, so no PRG changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… correction Self-correction on top of the previous commit, caught in review before merge. ## What I got wrong That commit claimed plain `make` already rebuilt the ip65 blob in the right order and that `make ip65-blob` "was never a required step". I had measured it: delete the blob, `make clean`, `make`, watch it rebuild byte-identically. The measurement was real. The conclusion was wrong, and shipping it would have re-blocked the next fresh clone -- exactly what issue #89 was filed to prevent. The confound is worth knowing, because it will fool anyone else measuring here. **ca65 does not resolve `.incbin` only relative to the including source file**, whatever the comment in ip65_blob.s said. It also tries the path relative to the current directory, and `../../../` from a repo root escapes three levels ABOVE the checkout. A git worktree lives at `<repo>/.claude/worktrees/<name>/` -- exactly three levels down. So my worktree, with its own blob deleted, silently assembled the PARENT checkout's blob and reported success. Reproduced deliberately in a scratch tree (`x/y/z/src/net/ip65/ip65_blob.s`, no `x/y/z/ip65-build/`, blob planted at the top): ca65 exit 0. The original CLAUDE.md text was right about the observable behaviour and right about the error message. It was wrong only about the mechanism -- it blamed a phony target with "no rule connecting the two". ## What is actually broken, and the fix There IS a rule, and $(IP65_BIN) has always been a prerequisite of the PRG. What was missing is an edge from the *object* that consumes the blob: make cannot see through `.incbin`, so from a clean build/ it was free to assemble ip65_blob.o before the blob rule ran -- and did. build/net/ip65/ip65_blob.o: $(IP65_BIN) Fixed at the source rather than documented around, so the workaround step goes away instead of needing a carefully worded warning. ## Evidence Genuine `git clone` from GitHub into a scratch dir, submodule init, `make ip65-libs`, then plain `make`: before: src/net/ip65/ip65_blob.s(22): Error: Cannot open include file '../../../ip65-build/ip65-c64.bin': No such file or directory make: *** [build/net/ip65/ip65_blob.o] Error 1 after: exit 0; blob 6,951 B cf1a5ff7...; PRG 47,105 B d522e684... Output-neutral -- the ip65 PRG hashes d522e684... with and without the edge, reproduced in both the fresh clone and this worktree. Full evidence bar, since this touches the Makefile: make clean && make -> exit 0, 47,105 B, d522e684... make clean && make BACKEND=uci -> exit 0, 62,977 B, 66e37037... python3 tools/test_ecdsa_kat_oracle.py -> 6/6 (3 valid, 3 invalid CAVP) python3 tools/test_x509.py -> 11/11 ## Docs CLAUDE.md and README now describe the real mechanism, credit that the original text was right about the failure and wrong only about the why, and both warn against measuring blob behaviour in a nested worktree. ip65_blob.s's own comment asserted the false source-relative rule right above the `.incbin`; corrected, since this commit proves it wrong and changes the adjacent build rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e and limits Closes the gap the previous commits left open. I had declined to write in a v0.9.1 figure because I could not corroborate one from the repo. It exists -- it lives in the release-prep session's logs rather than in the tree, which is why no amount of grepping here would have found it. The coordinator supplied all three points with per-row provenance. 48 MHz UCI, REU profile, U64E at 10.43.23.81, handshake + GET against the local listener, measured C64-side from run_prg, all PASS with server-side evidence: libs/nistcurves c64-side vs v0.6.0 v0.6.0 80.8 s -- v0.9.1 82.1 s +1.6% v0.10.1 82.4 s +2.0% Recorded with the two caveats that matter more than the numbers, because three fresh datapoints in a file full of v0.6.0 tables is exactly the kind of thing that gets read as a refresh: - n=1 per row. The +0.4% v0.9.1 -> v0.10.1 step is indistinguishable from run-to-run variation and is explicitly not presented as a measured effect. - This is ONE point of the sweep. Every other clock (1/8/16/20 MHz), every onchip and comb row, and every ip65 and C64U figure is still v0.6.0-era. The section header, the U64E sweep table and the packaging validation record all now say so in place. The +1.6% at v0.9.1 is consistent in sign and size with the FIPS 186-5 public-key validation gate v0.7.0 added -- an on-curve check on a point taken from an attacker-supplied certificate, so it is a regression worth paying rather than one to chase. Provenance is stated inline (bench/summary.txt, rel_e2e_uci48.log, e2e_wave.log) together with the fact that those logs are not in this repo, so a future reader knows the rows are not reproducible from a clean checkout and should re-run bench_ecdsa_u64e.py to confirm them. Independent corroboration of the v0.9.1 -> v0.10.1 "no timing change expected" claim: rebasing onto #113 moved the pins to nistcurves v0.10.1 / x25519 v0.11.0, and both PRGs hash exactly as they did at v0.9.1 -- ip65 d522e684... (47,105 B), UCI 66e37037... (62,977 B). Rebased onto merged master (a6cf205). One conflict, in README's USE_X25519_SIBLING known-issue bullet: resolved in favour of #113's version, which is measured at the new pins and belongs to that lane. Pin-vs-commit table extended with the a6cf205 row. Evidence bar re-run at the new pins, not carried over: make clean && make -> exit 0, 47,105 B, d522e684... make clean && make BACKEND=uci -> exit 0, 62,977 B, 66e37037... python3 tools/test_ecdsa_kat_oracle.py -> 6/6 (3 valid, 3 invalid CAVP) python3 tools/test_x509.py -> 11/11 Boundary guard and pytest count unchanged by the rebase: 5 checks pass, bare pytest 31 passed exit 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JC-000force-pushed
the
chore/deferred-followups
branch
from
August 15, 2026 13:47
c10ce19 to
08686ceCompare
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 freeto 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.
Two deferred follow-ups, each left behind by an earlier lane with its reason recorded. Plus one Makefile fix that fell out of Job 2 and is arguably the most useful thing here.
Rebased onto merged master (
a6cf205, #113). One conflict — README'sUSE_X25519_SIBLINGknown-issue bullet — resolved in favour of #113's version, which is measured at the new pins and belongs to that lane.Job 1 — finish the pytest-boundary rename
PR #111 renamed
tests/test_*.py->tests/rig_*.pyand explicitly deferred the identical problem intools/uci/: rig scripts wearing pytest's discovery convention while collecting zero tests.norecursedirsclosed the default-invocation path; the rename is the honest fix, becausetestpathsonly applies at the rootdir while a rename holds from any working directory.Six files renamed with
git mv(rename detection intact):boot_check.py,phase2_check.py,phase3_tcp_echo.pyandbench_ecdsa_u64e.pynever carried the prefix and were not renamed.Guard extended, and probed in both directions
tools/test_pytest_boundary.pygains aRIG_DIRS = ("tests", "tools/uci")tuple and a new fourth invariant. The rename and the config entry are both load-bearing, so both are now pinned — previously only one was.Probe A — a
test_*.pyreappears intools/uci/:Probe B —
tools/ucidrops out ofnorecursedirs:Both probes reverted afterwards.
Measured before/after
pytest(repo root)pytest tests/pytest tools/uci/pytest toolsReference sweep
grep -rnfor the six stems over the tree (excluding.git,libs,ip65,build) returns onlydocs/library-ingestion-architecture.md:354and:445, which the contract lane owns and has confirmed it is taking. Everything else updated here:CLAUDE.md,README.md,conftest.py,pytest.ini,tests/README.md, bothdocs/phase_f_part1_*.md,tools/https_e2e/{ensure_certs.py,certs/README},tools/package/listener/{listener.py,README.md},tools/uci/{_memory_policy.py,bench_ecdsa_u64e.py}and six in-script self-references.Delegation checked live rather than assumed — both wrappers still resolve after the rename:
Sibling repos:#111 checked only for
tests/. Checked here across the wholec64-test-harnesstree and all five other siblingc64-*repos: zero hits. No open issue or PR in the org names any of the six.New
tools/uci/README.mdmirrorstests/README.md.The Makefile fix — issue #89's fresh-clone failure, fixed at the source
This started as a documentation correction and turned into a real defect, so it is worth reading even if you skip the rest.
src/net/ip65/ip65_blob.spulls the prebuilt ip65 image in with a ca65.incbin, which make's dependency graph cannot see.$(IP65_BIN)was a prerequisite of the PRG, but nothing ordered it before the object that consumes it — so from a cleanbuild/make assembledip65_blob.ofirst and died. From a genuinely fresh clone, aftermake ip65-libs:One line states the missing edge:
Same fresh clone, same commands, with the fix:
47,105 B, no intermediate step. The fix changes no output bytes — the ip65 PRG hashes
d522e684…with and without it, and the hash reproduces across the fresh clone and this worktree.How I got this wrong first, and the trap that did it
My first pass claimed plain
makealready rebuilt the blob correctly and thatmake ip65-blobwas never needed. I had measured it — deleting the blob,make clean,make, and watching it rebuild byte-identically. The measurement was real and the conclusion was wrong.ca65 does not resolve
.incbinonly relative to the including source file, whatever the comment inip65_blob.ssaid. It also tries the path relative to the current directory, and../../../from a repo root escapes three levels above the checkout. A git worktree lives at<repo>/.claude/worktrees/<name>/— exactly three levels down. So my worktree, with its own blob deleted, silently assembled the parent checkout's blob and reported success.Reproduced deliberately in a scratch tree to be sure it was the mechanism and not a coincidence:
The coordinator caught the wrong conclusion before it merged. Both the CLAUDE.md and README wording and the source comment now state the real mechanism and warn not to measure blob behaviour in a nested worktree.
Job 2 — CLAUDE.md staleness sweep
README was audited claim-by-claim in #104; CLAUDE.md never was. Every item below was confirmed by running a command.
uci_regs.incnet.s:199)grep -n uci_fence src/net/uci/*.sgrep -c 'jsr uci_drain_resp|jsr uci_drain_status' src/net/uci/net.s(all 22 stillbcsout)uci_cmd.sdefines no$DC09/$DC0Aequate at allNET_CODE$2000-$3B25$2000-$3B65start = $2000, size = $1B66NET_BSS_TAIL$3B26-$3FFF$3B66-$41FFstart = $3B66, size = $069AUCI_BSS_REGION$4000-$41FF"zero-size"$4000(size 0)size = $0000— the span contradicted its own labelsrc/exports.sexportsip65_init,ip65_processsrc/net/ip65/exports.sUCI_DEBUG_KEEP_ON_PASSKEEP_DEBUG_ON_PASSos.environ.get("KEEP_DEBUG_ON_PASS", "0")— the documented spelling did nothingos.environ.setdefaultatrig_https_local_p384.py:73-74C64_SKIP_BUILDscriptsDEBUG_CAPTURE=0disablesos.environ.get("DEBUG_CAPTURE", "1")EMBED_P256_OVERLAYexclusions$(error)guard: one silent auto-disable, one unguarded$(error)s areMakefile:91/94/97/245default_vice_config()uptakeViceConfigdirectly, incl.run_all_tests.pygrep -lnboth waystest_finished_verify.pyrun_all_tests.pydispatches 11 and includes neither it nortest_chained_hmacpytestsrc/boot.s:107-114grep -n '^\s*\.import net_' src/boot.sPlus the ip65-blob section above, and the stale
touch ip65-build/ip65-c64.bin"committed blob is reused" advice — there is no committed blob, so on a fresh clone there is nothing to touch.Wall-clock: labelled, plus the one point that has been re-measured
No benchmark was re-run by me and no hardware was touched. Every figure now states its pin:
git ls-tree <sha> libs/nistcurves+git tag --points-at:2ceb5b1,f0127a0andcb6eab4are all v0.6.0;3a43f61is v0.9.1/v0.10.0;a6cf205(chore(libs): nistcurves v0.9.1 -> v0.10.1, x25519 v0.10.0 -> v0.11.0 (lib-contract phase-3 wave) #113, current master) is v0.10.1 / v0.11.0.@ 2ceb5b1now sayv0.6.0. The ip65/VICE table, the post-fix(tls): per-backend post-ServerHello drain budget — restores UCI handshake wall-clock #74 table and the packaging validation record all state their pin. "all three v0.6.0 profiles at HEAD" no longer claims HEAD.cc182f1is unreachable after the 2026-05 upstream history rebuild).The v0.9.1 datapoint I originally declined to write in now exists in the file. I could not corroborate it from the repo because it lives in the release-prep session's logs, not the tree — which is why grepping was never going to find it. Supplied with per-row provenance and recorded as a clearly-bounded subsection, 48 MHz UCI REU profile, U64E, handshake + GET measured C64-side from
run_prg, all PASS with server-side evidence:libs/nistcurvesCarried with the two caveats that matter more than the numbers, since three fresh rows in a file full of v0.6.0 tables invite exactly the wrong reading:
The +1.6% at v0.9.1 matches the FIPS 186-5 public-key validation gate v0.7.0 added — an on-curve check on a point taken from an attacker-supplied certificate, so a regression worth paying. Provenance (
bench/summary.txt,rel_e2e_uci48.log,e2e_wave.log) is named inline along with the fact that those logs are not in this repo, so nobody mistakes the rows for something reproducible from a clean checkout.Independent corroboration of the "no timing change expected at v0.10.1" reasoning: rebasing onto #113 moved the pins, and both PRGs hash exactly as they did at v0.9.1 — ip65
d522e684…, UCI66e37037….Found and deliberately left alone
The x25519 sibling failure. Measured: at 3a43f61 both backends exit 2 with
ld65: Error: Duplicate external identifier: 'reu_mul_tables_init'— not the overflow CLAUDE.md records. PR chore(libs): nistcurves v0.9.1 -> v0.10.1, x25519 v0.10.0 -> v0.11.0 (lib-contract phase-3 wave) #113 rewrites the same CLAUDE.md and README spans and fixes the underlying collision, so I backed my prose out entirely to avoid a conflict and handed that lane the derivation. Their measurement supersedes mine: at the wave pins UCI links, and of the three recorded overflows two vanish and ip65'sX25519_RODATAgrows.One clause of my handoff was too strong and chore(libs): nistcurves v0.9.1 -> v0.10.1, x25519 v0.10.0 -> v0.11.0 (lib-contract phase-3 wave) #113 corrected it before pasting: I wrote that nistcurves v0.6.0 "does not export
reu_mul_tables_initat all", generalising from a grep of one file. It does export it, atsrc/main.s:254. The load-bearing fact is one step further in —main.ois deliberately excluded from everylib-*archive target, and c64-https links archives only, so no archive carried the symbol at v0.6.0. Same conclusion (libs/x25519 v0.6.0 -> v0.10.0, and USE_X25519_SIBLING=1 links + verifies for the first time #102's evidence was honest on its own branch), sturdier mechanism. Recorded here because it is the same single-file-grep overreach this PR fixes elsewhere.docs/library-ingestion-architecture.md— two stale script references; owned and taken by the contract lane.src/net/uci/uci_regs.inc's own "~14 bytes" comment — same error as the CLAUDE.md one fixed here, but out of this lane's scope. (ip65_blob.s's comment was fixed, because this PR proves it wrong and changes the adjacent build rule.)unknown_policy=WARNin the MemoryPolicy migration — a code change, not a doc one.Evidence
The Makefile change puts this over the build-evidence bar:
Re-run at the post-rebase pins (nistcurves v0.10.1 / x25519 v0.11.0), not carried over from the earlier run:
Both PRG hashes are unchanged by this branch and unchanged across the wave bump; the Makefile edge is an ordering constraint and the
ip65_blob.schange is comment-only. Boundary guard: 5 checks pass, barepytest31 passed exit 0.🤖 Generated with Claude Code