[NO-MERGE] evidence: independently reproduce ULX3S LSC-1 bitstream - #117
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fb99409ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| require(r.get("base_commit") == BASE, "base commit") | ||
| source = r.get("build_source", {}) | ||
| require(source == {"commit": SOURCE, "tree": "87462c2c698c43318df8b9a4db78c0c6ca9de251", "inputs_match_revision": True}, "source identity") | ||
| tree = subprocess.check_output(["git", "rev-parse", f"{SOURCE}^{{tree}}"], cwd=ROOT, text=True).strip() |
There was a problem hiding this comment.
In a clean checkout of this commit, fde1b885a56b98391833f4632676f14d1e3e2f9c is neither reachable from a ref nor present as a Git object, so this rev-parse raises CalledProcessError. Consequently the standalone verifier exits 1 and all eight newly added tests fail before validating their receipts. Preserve a reachable source object or validate the archived source manifest without depending on an unavailable detached-worktree commit.
Useful? React with 👍 / 👎.
| require(run.get("raw_receipt_sha256") == RAW_RECEIPTS[run["id"]], "raw receipt hashes") | ||
| require(run.get("artifacts") == ARTIFACTS, "run artifact hashes") |
There was a problem hiding this comment.
For reviewers relying on this verifier to substantiate the two clean builds, this check only compares receipt fields with constants added in the same commit. A repo-wide tracked-file search finds no per-run clean-build-* logs or artifacts to hash, and the recorded Yosys/nextpnr digests do not match the sole pre-existing archived logs. Thus verify() can report success without inspecting any output from either purported reproduction run; archive and hash the actual per-run receipts and artifacts before accepting the reproduction conclusion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9ce903a2c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| require(r.get("base_commit") == BASE, "base commit") | ||
| source = r.get("build_source", {}) | ||
| require(source == {"commit": SOURCE, "tree": SOURCE_TREE, "inputs_match_revision": True}, "source identity") | ||
| require(git("rev-parse", f"{SOURCE}^{{tree}}") == SOURCE_TREE, "source tree object") |
There was a problem hiding this comment.
Fresh evidence shows that the replacement anchor adc3e2c5b86fb08e1b0225573486ae08af4ac194 is still absent from this reviewed commit's ancestry: 8605446 has sole parent 5610ea2, while the anchor is an intermediate commit on a separate history. In a clean checkout containing only the reviewed commit and its ancestors—such as after a squash merge or deletion of the PR branch—this rev-parse raises CalledProcessError, causing the newly added workflow-check verifier to fail before checking the archived runs; the packet-evidence verifier now depends on the same unavailable object. Use a reachable ancestor (the recorded build inputs are also present at 5610ea2) or preserve the manifest without requiring this detached object.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebf087429a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "malformed source manifest") | ||
| require(fields[1] not in entries, f"duplicate source input {fields[1]}") | ||
| entries[fields[1]] = fields[0] | ||
| require(len(entries) == 25, "complete source manifest") |
There was a problem hiding this comment.
Checking only len(entries) == 25 allows any 25 files from the base tree to pass as a complete source manifest. I replaced asic_core/rtl/gf128_mul_bitstream.sv with a correctly hashed README.md, updated the receipt hash, and verify() still returned success, so evidence can omit an RTL, constraint, or build-script input while retaining the reproduction conclusion. Require the entry-name set to equal the packet recipe's known inputs, as the existing PACKET_BUILD_INPUTS check does in the physical-evidence verifier.
Useful? React with 👍 / 👎.
| subprocess.run(["git", "clone", "--bare", "--shared", str(source), str(staging)], | ||
| check=True, stdout=subprocess.DEVNULL) |
There was a problem hiding this comment.
When make check runs from a blob-filtered checkout, this shared bare clone references the source object store but does not inherit its promisor remote, so the later --no-local clone fails while packing an absent historical blob (git clone -h describes --shared as “setup as shared repository”). I reproduced the new test failing with fatal: bad tree object before either verifier ran; the executable-model CI job currently uses fetch-depth: 0, so this affects partial/local checkouts rather than that full-clone lane. Build the staging repository without --shared, or otherwise preserve the partial-clone fetch configuration.
Useful? React with 👍 / 👎.
|
Successor verification for
The change only repairs test-fixture object materialization before independent bare clones and updates the tracked checksum. It does not broaden the evidence claim beyond host-side authored-RTL synthesis/logical-netlist/P&R reproduction; it adds no physical-board, E2E, LSC-1µ, LSC1-06+, universal-correspondence, or host fetch/memory claim. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d7032338a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def materialize_promised_objects(source: Path) -> None: | ||
| """Make an independent clone safe to serve without lazy fetching.""" | ||
| promisor = subprocess.run( | ||
| ["git", "-C", str(source), "config", "--get", "remote.origin.promisor"], |
There was a problem hiding this comment.
In a valid blob-filtered checkout whose promisor remote is not named origin (for example, git clone --origin upstream --filter=blob:none ...), this probe returns nonzero and skips materialization. Under the GIT_NO_LAZY_FETCH=1 context modeled later in this test, the subsequent git clone --bare --no-local then fails while packing a missing promised blob. Fresh evidence after the prior --shared fix is that this reproduces with remote.upstream.promisor=true; git clone -h documents that --origin <name> can use a name other than origin. Discover and fetch the configured promisor remote instead of hard-coding origin.
Useful? React with 👍 / 👎.
| for key in ("host_fetch", "memory_expansion", "physical_hardware", "end_to_end", "universal_or_unbounded", "later_roadmap_work"): | ||
| require(scope.get(key) is False, f"scope boundary {key}") | ||
| layers = r.get("evidence_layers", {}) | ||
| require(set(layers) == {"executable_model", "lean", "authored_rtl", "netlist", "place_and_route", "physical_hardware"}, "evidence layer separation") |
There was a problem hiding this comment.
When verify() is called with a supplied receipt, retaining these six keys while changing their values is accepted. For example, I changed physical_hardware to claim board validation and place_and_route to say it was not exercised, and the verifier still reported success. Because these values define what each evidence layer actually establishes, checking only the key set permits an internally contradictory or scope-inflated receipt; compare the complete expected mapping or validate every value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a8c0e5aac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| d6c7dd33771eb5386c9fc00e1003edce1d90b4a25625400eb8f0eccc57efc7e9 ./test/test_lsc1_fpga_packet_evidence.py | ||
| 2dd4a7cbfcb2cf765e3068526fd1f68c5ab738d752ff6b60c0fa4260cab87219 ./test/test_lsc1_host_authored_rtl_boundary.py | ||
| e482475415e3a56c90db896fac31f7faff94a4a36f344fe6630222da88bcf07c ./test/test_lsc1_scalar_status_workflow.py | ||
| e0bd89e19ffae189a4a458fc9368a9f5eaa0ce7c44cfabf53a706d933e731178 ./test/test_lsc1_ulx3s_reproduction.py |
There was a problem hiding this comment.
The newly added test currently hashes to 16417a203b31e19b4c4d3c84a75a26c6ac271c15c1b48ceb13e016ab254296a1, not the recorded e0bd89e... value. Consequently tools/generate_checksums.py --check reports that SHA256SUMS is stale, and every standard make check (and therefore make package) fails when it reaches the checksum-check dependency.
Useful? React with 👍 / 👎.
1 participant
Summary
fde1b885a56b98391833f4632676f14d1e3e2f9c.bit,.config, and.svfoutputs are byte-identical to each other and the immutable archive on exact base5610ea221dd82b5749690043e8c3665b2be9ced8Reproduction result
Both isolated builds exited 0 and nextpnr reported
Program finished normally.Both complete source manifests bind all build inputs tofde1b885…. Both timing receipts report 15.32 MHz achieved and PASS at the required 10.00 MHz core clock.Artifact SHA-256 values reproduced in both runs:
.bit:226514183384b875821426b8c4d338508d8cff08cd12cb8e39c9162db37e3b9e.config:0737bbafae6704139ad9de2669a4e43f64d02644ce7cb9fc7bef336b28b7bf6b.svf:0ad6510adeb1afbc3b20bdbc96528211a2d2e38ebcee398fdeb82acb3b1ad0eeValidation
./fpga/ulx3s/build_packet_uart.shexecutions — exit 0sha256sum -c SHA256SUMS— exit 0 for all three artifacts.bit,.config,.svfpython3 tools/verify_lsc1_ulx3s_reproduction.py— exit 0python3 -m pytest -q test/test_lsc1_ulx3s_reproduction.py test/test_lsc1_fpga_packet_evidence.py test/test_ulx3s_packet_build_recipe.py— exit 0; 22 passed, 34 subtests passedmake -C test/packet_frontend sim— exit 0make lean— exit 0; both Lean builds succeeded and all configured mutation families were killedgit diff --cached --checkbefore commit — exit 0An exploratory
make -C test/packet_frontend clean allexited 2 because that Makefile has noalltarget; the valid repository targetsimwas then run and passed.Boundaries
This is host-build reproducibility evidence only for the full LSC-1 host-prepared packet endpoint. It adds no host fetch or memory expansion and covers no later roadmap work. Executable-model, Lean, authored-RTL, generated-netlist, place-and-route, and physical-hardware evidence remain separate. No board was attached, programmed, or observed. This makes no physical, universal/unbounded, or end-to-end claim.
Review only: intentionally NO-MERGE. Do not merge this PR.