fix(seals): refresh 116 stale generated-code hashes, and check them from now on - #3416
Merged
Conversation
…rom now on Closes #3415 A seal records four `gen_hash_*` fields -- the hashes of what each backend emitted. Nothing checked them. Measured on master: 109 stale gen_hash_rust, 7 stale gen_hash_zig, 0 C, 0 Verilog, and 0 stale spec_hash. All 116 were left by five merged backend repairs -- #3401, #3403, #3405, #3407, #3411 -- every one of them mine. None touched a spec, so spec_hash stayed correct and every coverage and staleness check in the repository stayed green while a sixth of the Rust seals described output the compiler no longer produces. The existing checks cover the other half. check_seal_coverage.py asks whether a seal describes a spec that exists, unchanged at SOURCE. `Seal Staleness Warning` is about the NMSE manifest and FROZEN_HASH, unrelated to .trinity/seals, and exits 0 by design. `t27c seal --verify` answers this question exactly -- exit 1 with a precise MISMATCH line, exit 0 on a current seal, both used as controls here -- and nothing called it across the corpus. The refresh took three attempts and each failure was informative. `--save` fixed 116 -> 58, not 0, because it writes to .trinity/seals/<module>.json, one name, while 1313 seals cover 728 distinct specs and 547 specs carry more than one seal file. The remaining 58 duplicates are rewritten in place. `sealed_at` is left alone in those: rewriting a hash the tool itself just computed is not a fresh certification event, and moving the timestamp would claim one. gen_hash=none is a different debt and is counted apart -- 169 seals record it for at least one backend, and --save refuses to overwrite them ("4 of 4 backends rejected it"). Conflating the two is how the 116 stayed invisible. tools/check_seal_currency.py now asks the question. Its --self-check plants a wrong hash on a scratch tree and requires exactly that seal to be reported, because a zero from a check that cannot see is indistinguishable from a healthy zero. It exits 2 when t27c is absent rather than 0: a check that could not run has not passed. Filed, not guessed: some duplicate seals are not named after a module at all. specs/tri/utils/logger.t27 carries `"[]const u8".json`, `utils_"[]const u8".json` and `utils_TriLogger.json` -- a seal named after a type string. After this: 1318 seals scanned, 1055 current, 94 whose spec is gone, 169 sealed with none, 0 stale. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gHashTag
enabled auto-merge (squash)
September 7, 2026 21:11
Contributor
Contributor
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
gHashTag
added a commit
that referenced
this pull request
Sep 7, 2026
…ns by case (#3419) Closes #3418 Two defects in one place, both found by being bitten rather than by review. A spec can own more than one seal file and `--save` wrote one. Measured over .trinity/seals: 1313 seals name 728 distinct specs, and 547 specs carry more than one -- 501 of them the same module under two names, `<Module>.json` beside `<dir>_<Module>.json`, left by a naming scheme that changed. Refreshing 116 stale seals through this command in #3416 reached 58 and stopped for exactly that reason; editing four specs in #3417 produced ten stale seals where --save would have fixed four. The duplicates are not deletable: math_compare.rs opens .trinity/seals/PellisFormulas.json by its bare name and that file is one of a pair. So the tool maintains them. Only hashes and the timestamp are rewritten; each file keeps its own `module`, because the file is NAMED after that field and overwriting it would leave a seal whose name and contents disagree. Second defect, invisible on macOS: seal_file_path derives `ar_restraint.json` where `ar_Restraint.json` is tracked. 751 filenames match the derived name exactly, 4 differ only by case, 558 use the older scheme. A case-insensitive filesystem resolves the pair to one file; the case-sensitive one CI runs on would grow a SECOND seal for the same spec, and neither would look wrong. An existing file now wins the lookup. The test for that half CANNOT FAIL on this machine and says so. Mutation proved it -- making the lookup case-sensitive leaves the test green here, because creating `Aa` then `aA` yields one file. It probes the filesystem and skips loudly rather than reporting a pass it did not earn; it discriminates on Linux, where CI runs. The other test does kill its mutant here. The child runs with its own working directory rather than set_current_dir, which is per-process and would race every other test in this binary under the default parallel runner. Left alone and filed: 93 specs carry a seal whose name is not an identifier -- specs/tri/utils/logger.t27 has `"[]const u8".json`, a seal named after a type string. compiler.rs is untouched, so FROZEN_HASH is unchanged. Co-authored-by: lab <lab@example.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
gHashTag
added a commit
that referenced
this pull request
Sep 7, 2026
…s did (#3421) Closes #3420 rings/ring-090-rust says in its own doc comment that it is "faithful to the spec" specs/fpga/simulator.t27. A differential harness driving every shared function from both modules on the same inputs -- 14 u32 values crossed with 3 name strings -- disagreed on 126 of 1190 cases. sim_time_ns 4/10 2_000_000_000 -> hand 4294967295, spec 2820130816 sim_time_us 1/10 -> hand 4294967, spec 2820130 sim_time_ms 1/10 -> hand 4294, spec 2820 cycles_for_time_ns 0/10 The hand-written model widens to u64 and then saturates. The spec widened and then narrowed with a bare `as u32`, which wraps: at 2_000_000_000 cycles on the default 100 MHz clock, 20_000_000_000 ns becomes 2_820_130_816. The spec states the assumption that fails, in a comment on the line above: "Widen the intermediate to u64 and narrow the (small) result back." It is not always small. With the guard, the harness reports 1190 of 1190 agreeing, and its control confirms it can see a difference. Context for why this was worth looking for. Ten of the seventeen rings/* crates name a spec that exists in this repository, and the overlap between them is wildly uneven: ring-090 22 hand items, 21 shared, 0 spec-only <- 15/16 signatures identical ring-097 18 hand items, 11 shared, 0 spec-only ring-099 9 hand items, 4 shared, 0 spec-only ring-098 12 hand items, 5 shared, 2 spec-only ring-088 3 hand items, 0 shared <- names gf16.t27, unrelated ring-101 5 hand items, 0 shared <- same ring-090 is the closest pair in the repository and it had still drifted in BEHAVIOUR, which a comparison of names and signatures alone could never show. Two tests are added to the spec in its own language rather than around it: the saturating case and an exact case below the ceiling. Both reach the Zig output. The seal was refreshed by `t27c seal --save`, which reported "and 1 other seal file(s) naming the same spec" and updated both fpga_Simulator.json and Simulator.json -- the repair from #3419 on its first real case, after check_seal_currency.py from #3416 named the stale hashes. Co-authored-by: lab <lab@example.com> Co-authored-by: Claude Opus 5 <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
1 participant
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.
Closes #3415
A seal records four
gen_hash_*fields — the hashes of what each backend emitted. Nothing checked them.Measured on master, before this
gen_hash_rustgen_hash_ziggen_hash_cgen_hash_verilogspec_hashAll 116 were left by five merged backend repairs — #3401, #3403, #3405, #3407, #3411 — every one of them mine. None of them touched a spec, so
spec_hashstayed correct and every coverage and staleness check in the repository stayed green while a sixth of the Rust seals described output the compiler no longer produces.What the existing checks cover, and what they do not
check_seal_coverage.py— does a seal describe a spec that exists, unchanged at source? That isspec_pathandspec_hash.Seal Staleness Warning— the NMSE manifest seal,sha256(compiler.rs)againstFROZEN_HASH. Unrelated to.trinity/seals, and always exits 0 by design.t27c seal --verify— answers exactly this, per spec. Nothing called it across the corpus.Controls:
--verifyexits 1 onspecs/ar/restraint.t27withgen_hash_rust: MISMATCH (saved=7d50c681…, current=e60d0df7…)and MATCH on the other three; it exits 0 onspecs/fpga/testbench/apb_bridge_tb.t27.The refresh took three attempts, and each failure was informative
t27c seal --savefixed 116 → 58, not 0. It writes to.trinity/seals/<module>.json— one name — and:The remaining 58 duplicates are rewritten in place.
sealed_atis left alone there: rewriting a hash the tool itself just computed is not a fresh certification event, and moving the timestamp would claim one.Two things kept apart on purpose
gen_hash=noneis a different debt. 169 seals record it for at least one backend, and--saverefuses to overwrite them ("4 of 4 backends rejected it"). Conflating them with real mismatches is how the 116 stayed invisible. That 169 independently reproduces the figure an earlier audit reported fornone == noneseals.Filed, not guessed: some duplicate seals are not named after a module at all —
A seal named after a type string. Which name is canonical is a decision.
The check
tools/check_seal_currency.py. Its--self-checkplants a wrong hash on a scratch tree and requires exactly that seal to be reported and the good one to read current — because a zero from a check that cannot see is indistinguishable from a healthy zero. It exits 2 whent27cis absent rather than 0: a check that could not run has not passed.Not wired into CI here — that is a cost decision about running four backends over 651 specs on every PR, and it is the owners.