-
-
Notifications
You must be signed in to change notification settings - Fork 2
chore(release): cut v2.4.1 "Fabric", carrying the never-tagged v2.4.0 #430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| RustyNES **v2.4.1 "Fabric"** opens the **v2.4.1 → v2.5.0 "Fabric"** line: a new NES core written in SystemVerilog from public hardware documentation, in a sibling repository, with this emulator as its **verification oracle**. The line is named for what it builds — and RustyNES's role in it is the warp: the fixed threads held under tension that everything else is woven against. That is the one role this emulator is uniquely equipped for. | ||
|
|
||
| **It also ships v2.4.0 "Concordance", which never received its own tag.** That work merged to `main` and the release was never cut, so this release carries both. Where a section below belongs to v2.4.0 it says so. | ||
|
|
||
| **The accuracy contract is verified, not asserted.** `rustynes-core` changes in both halves, so AccuracyCoin was re-run rather than reasoned about: **141/141 (100.00%)** on the authoritative RAM decoder, nestest 0-diff. The framebuffer decoder reports 120 and remains known-buggy. | ||
|
|
||
| ## RustyNES is not being ported to FPGA, and cannot be | ||
|
|
||
| A MiSTer core is SystemVerilog compiled by Quartus 17.0.2 into a Cyclone V bitstream. Rust does not become a bitstream, and high-level synthesis of a cycle-accurate emulator's control flow does not produce usable hardware. What is buildable is a **new implementation** verified against this one, and `crates/rustynes-cosim` is the boundary between them: a narrow C ABI a Verilator testbench links, plus the safe Rust API a golden-export binary uses. | ||
|
|
||
| The reference firewall extends to HDL accordingly. `NES_MiSTer` and `fpganes` `rtl/` are **strict black boxes** — instantiable as opaque modules to compare *outputs*, never readable as source. Anything unimplementable from documentation escalates to an ADR **before** any source is opened. | ||
|
|
||
| **Replay, not lockstep**, because `Nes` exposes `run_frame()` and `step_instruction()` and nothing finer. Cycle-lockstep would mean new core API on the hot path and would gain nothing: the determinism contract already makes a pre-recorded trace *exactly* the trace a lockstep run produces. **No DPI-C**, because it puts `` `ifdef SIMULATION `` guards into RTL that must also pass Quartus — the construct that lets a simulated netlist drift from the synthesised one. **Hash first, capture on divergence**, because a 4200-frame AccuracyCoin run is ~125 M cycles, which as per-cycle CSV is ~7.5 GB against 244 KB of 4096-cycle checkpoints. | ||
|
|
||
| The golden framebuffer is exported **pre-palette** so a palette difference cannot masquerade as a rendering difference — the failure mode v2.3.8 "Parallax" was built to prevent. | ||
|
|
||
| **v2.5.0 is scoped to "the 6502 rung closes"**, not a finished core. A from-scratch cycle-accurate NES core is 7–13 months FTE; PPU, APU and MiSTer integration are the v2.6–v2.9 programme. Two risks are accepted in writing: `NES_MiSTer` scores 121/125 on AccuracyCoin where *real Famicom AV hardware also scores ~121/125*, so there is no published accuracy headroom and the core **may be declined as a duplicate**; and **the oracle can be wrong**, since 141/141 is not "matches silicon". Every rung is labelled by whether it has an *independent* oracle. Full reasoning: ADR 0037, `docs/mister.md`, `to-dos/plans/v2.5.0-fabric-plan.md`. | ||
|
|
||
| ## The first `run_frame()` after power-on advances zero cycles | ||
|
|
||
| The PPU is constructed at dot 340 of the pre-render line, so the seven-cycle reset sequence ticks past the frame wrap and leaves `frame_complete` latched. The first call consumes that latch and returns having stepped nothing. Measured, not inferred: frame 0 advances the cycle counter by **0**, frames 1..3 by ~29,780 each. | ||
|
|
||
| Every existing caller runs thousands of frames, so one lost frame is invisible to them. It stops being invisible the moment a **count** is recorded beside the output — `--frames 60` through a bare loop emits a 59-frame golden under a manifest claiming 60, which is a provenance record wrong in the one direction that matters. `Oracle::advance_frames` gates on the frame **counter**, with a jam bail-out, and the quirk is pinned by a test that names it. | ||
|
|
||
| ## The accuracy battery was validating a scheduler no user runs | ||
|
|
||
| The most serious finding in the release, and it was found by review rather than by a gate. | ||
|
|
||
| `rustynes-cosim` enables `cpu-boot-trace` and `irq-timing-trace` on `rustynes-core`, and cargo unifies features across a workspace build. As a workspace member it made `cargo build --workspace` compile the core **once** with the union — measured through `--message-format=json`, not inferred: | ||
|
|
||
| ```text | ||
| ['cpu-boot-trace', 'debug-hooks', 'default', 'hd-pack', 'irq-timing-trace', 'std'] | ||
| ``` | ||
|
|
||
| `irq-timing-trace` is not an inert branch: it selects a **different** `for sub_dot in 0..3` loop in `Bus::tick_one_cpu_cycle`. There are two, under opposite `cfg`s. So CI's `cargo test --workspace --release --features test-roms` — the accuracy battery — was validating the instrumented scheduler rather than the shipped one. The same shape as the v2.3.4 defect where the coverage harness tested a load path no user runs. | ||
|
|
||
| **The measured cost was +1.24% / +1.39% / +1.89%** across the three `full_frame` benches, *below* this project's own 3% adoption bar, and it never touched the shipped binary (`-p rustynes-frontend` does not select this crate) or the perf gate (`cargo bench -p rustynes-core`). That number is published precisely because it shows performance was never the argument. A gate pointed at the wrong code path is wrong at any percentage. | ||
|
|
||
| The crate is now in `[workspace] exclude`. Both costs of that are closed mechanically rather than trusted: `cosim_manifest_audit.rs` asserts every duplicated field and lint still equals the workspace's **and** that the crate is still excluded — four mutations, all caught — and CI gains explicit `fmt`, `clippy` and `test` steps, since `--all`/`--workspace` no longer reach it. The clippy step earned its place on its first run, reporting a `must_use_candidate` that `--workspace` had never surfaced. | ||
|
|
||
| ## Every path that persists user data now writes atomically and durably (v2.4.0) | ||
|
|
||
| The seven-property write sequence v2.3.9 built for `Config::save_to` is extracted into `crate::atomic_write` and adopted everywhere. The plan named three call sites; there were **four**, and the fourth is the instructive one. | ||
|
|
||
| `save_state.rs` matters most and was named last: **a truncated save state is a user's game progress**, a worse loss than a truncated config, and it was still using the bare `fs::write` the config path had already been fixed for. It is also the path most likely to be written under load — rewind capture, run-ahead and netplay rollback all produce save states. | ||
|
|
||
| `per_game.rs` was not in the plan at all, because it *looks* correct: it writes a sibling temp file and renames, so a sweep for `fs::write`-onto-a-target clears it. It held two of seven. No `fsync`, so the rename could commit a directory entry pointing at bytes that never reached the medium; and a **fixed** scratch name shared across every process and concurrent call — the exact failure the mechanism exists to prevent, reintroduced by the mechanism. A partially-correct implementation is harder to spot than an absent one. | ||
|
|
||
| Review then found **four more places the module reported success it had not earned**, each with the same shape — an error discarded under a comment explaining the rest of the operation: | ||
|
|
||
| - **`set_permissions` swallowed.** The mode applied is the mode the target *already had*, so a failure replaces a `0600` file with one at the umask default — **wider than what it replaced** — and reports success. | ||
| - **The parent-directory `fsync` swallowed**, along with the `File::open` feeding it, so the entire durability barrier could be a no-op while the module's own table claimed "yes". `EIO` — the exact condition it exists to detect — passed as success. It now propagates, excepting only the errnos that mean *this filesystem has no barrier* (`EINVAL`, `ENOTSUP`/`EOPNOTSUPP`, and `EBADF` on some network mounts). | ||
| - **The occupied-scratch retry was one attempt**, justified by "the counter cannot repeat a name within a process" — true, and beside the point, because the collision comes from a *previous* process whose pid was reused. Now a bounded loop. | ||
| - **On exhaustion, the cleanup deleted a file this process had not created** — an orphan, or a scratch file a colliding instance was actively writing. A failed save took another process's in-progress data with it. | ||
|
|
||
| **A `const fn` that only failed on Windows.** `is_transient_rename_error` was `const` and called `io::Error::kind`, which is not — `E0015`, behind `#[cfg(windows)]`, so it compiled clean on Linux. PR runs here are Linux-only and the full matrix runs on `main`, so it would have turned `main` red *after* merge. The fix is not just dropping `const`: the predicate moved into an always-compiled function reached through `cfg!(windows) && …`, so restoring the `const` now fails **on Linux** with the same error. The defect class moved from "invisible until another platform builds it" to "fails the PR". | ||
|
|
||
| Also here: the cheat save reports its failure in the panel instead of printing to a `stderr` nobody reads on a windowed build; symlink chains resolve to their end, bounded at Linux's own `MAXSYMLINKS` of 40; and the rename retry budget outlasts a Windows Defender scan lock — adopted on an **unverified** timing claim, recorded as unverified, because the cost is a longer stall on a save that is already failing. | ||
|
|
||
| ## A timeline generation counter, and the telemetry that reads it (v2.4.0) | ||
|
|
||
| v2.3.9 cleared stale debug telemetry on a ROM change and recorded that it could not clear it on a save-state load: of the four ways the emulator jumps timeline, only one is reachable from a patchable frontend call site. `Nes::timeline_generation()` is a session-local counter that any consumer can compare against its last-seen value — and it can see a restore to a *later* state, which the previous last-seen-`cycle()` heuristic could not. | ||
|
|
||
| It is **not** part of the save state, and the reasoning is now on the field with a test pinning it. Serializing it would break two things silently: loading the same slot twice would restore the same generation twice, so a consumer would miss the second load; and a value from another session means nothing in this one. Because it lives outside the snapshot, `snapshot_schema_audit` cannot see it — the property that makes the design correct also means nothing mechanical would notice the reasoning being invalidated. | ||
|
|
||
| ## Release anchors, and a claim v2.3.9 made that v2.3.9 did not ship | ||
|
|
||
| `release_anchor_audit.rs` pins **15 anchors across 10 documents** against `[workspace.package] version`, fails closed when a marker matches nothing, and asserts the CHANGELOG header parses the way `release-auto.yml` parses it. | ||
|
|
||
| It is described in **v2.3.9's release notes and is not in the v2.3.9 tag.** It merged in #427, after the tag was cut. The audit exists because eight documents had reached six different answers about the current version; that a release note could describe work the release does not contain is the same failure one level up, and it ships here. | ||
|
|
||
| ## Under the surface | ||
|
|
||
| - **The review bot destroyed its own record.** It posted a fresh comment each round and deleted the previous one, so a round nobody read before the next push was gone with nothing indicating it had existed — observed on PR #428, where two consecutive rounds each raised a blocking issue, one a data-loss defect, and only the second survived. It now edits one comment per PR, folding earlier rounds into a bounded, sentinel-delimited archive, and issues no `DELETE` at all. | ||
| - **A dot in a ROM name truncated every golden filename.** `Path::with_extension` replaces after the *last* dot, so `Super Mario Bros. 3` yielded `Super Mario Bros.ram.bin`. NES filenames contain dots routinely. | ||
| - **No CI invocation had ever enabled `cpu-boot-trace` or `irq-timing-trace` for clippy**, so those two `rustynes-core` modules had never passed the lint gate. Six pre-existing findings, all fixed. `--workspace --all-targets` reads as exhaustive and covers each crate's *default* feature set only. | ||
| - **The rung-0 gate is shown able to fail**, not merely to pass: the null-DUT self-diff reports `All 5464 aligned records match` and exits 0, and a one-bit corruption is caught at cycle 561, PC `$C419`, naming the field and both values. | ||
| - **`actions/checkout` is SHA-pinned** on the self-hosted reviewer runner — the maintainer's own machine, where a compromised tag executes rather than in a disposable VM. | ||
| - Empty `hd_packs` / `shader_presets` tables no longer write into an untouched config, correcting a false byte-identity claim v2.3.9 had corrected in prose only. | ||
|
|
||
| ## Verification | ||
|
|
||
| - **AccuracyCoin 141/141 (100.00%, RAM decoder)** and **nestest 0-diff**, re-run on the shipped path after the workspace exclusion rather than reasoned about. | ||
| - **129 test suites, 2236 tests, 0 failures** — 126 suites / 2222 in the workspace plus the excluded crate's 3 / 14, which CI runs explicitly. | ||
| - Full gate matrix: `fmt`, workspace clippy, the four frontend feature combinations, **both** wasm32 combinations, the `no_std` thumbv7em build, and rustdoc with warnings as errors. | ||
| - Every new assertion in this release was demonstrated to fail by mutation before being trusted — including one caught only as a **hang**, and one whose first mutation pass was rejected because the mutants had failed to *compile*, which proves nothing. | ||
|
|
||
| ## Upgrade notes | ||
|
|
||
| No format epochs change. Save states (`.rns`), movies (`.rnm`) and netplay replays are unchanged from v2.3.9, and the shipped emulation core is byte-identical on the default configuration. `crates/rustynes-cosim` is additive, outside the default build, and cannot be reached by an ordinary `cargo build`. | ||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,13 @@ cycle-accurate core later replaced. | |
|
|
||
| ## [Unreleased] | ||
|
|
||
| ## [2.4.1] - 2026-08-20 - "Fabric" (RustyNES as the oracle a new implementation is verified against) | ||
|
|
||
| This release also carries **v2.4.0 "Concordance"**, which merged to `main` and was never | ||
| tagged; entries below marked *(v2.4.0 item)* belong to it. Two further entries — the | ||
| standing release-anchor audit and the deferred-backlog sweep — belong to neither, having | ||
| landed between the two, and are called out where they appear. | ||
|
Comment on lines
+19
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Use one release classification for
As per coding guidelines, release notes and changelog text must accurately distinguish v2.4.0, v2.4.1, and intervening work. 🧰 Tools🪛 LanguageTool[style] ~20-~20: Replacing this phrase with a shorter alternative might make your text sound more refined. (BELONG_TO_PRP) 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| ### Added | ||
|
|
||
| - **`rustynes-cosim` — RustyNES as a co-simulation oracle for an FPGA | ||
|
|
@@ -221,7 +228,7 @@ cycle-accurate core later replaced. | |
| that diagnostic. | ||
|
|
||
| - **A standing release-anchor audit — the drift v2.3.9 corrected by hand cannot | ||
| recur silently.** `crates/rustynes-test-harness/tests/release_anchor_audit.rs` | ||
| recur silently.** (Landed between v2.4.0 and v2.4.1; part of neither.) `crates/rustynes-test-harness/tests/release_anchor_audit.rs` | ||
| pins **15 anchors across 10 documents** against `[workspace.package] version`: | ||
| the README badge and Current Release section, `docs/STATUS.md`, both `AGENTS.md` | ||
| anchors plus its "never claim a later version" guard, `VERSION-PLAN.md` (header | ||
|
|
@@ -292,7 +299,8 @@ cycle-accurate core later replaced. | |
| sync; the next is **v2.5.0**. A **licence change overrides and syncs | ||
| immediately** — the rule that incident produced in the first place. | ||
|
|
||
| - **`to-dos/DEFERRED-AND-CARRYOVER-FEATURES.md` swept entry by entry**, against | ||
| - **`to-dos/DEFERRED-AND-CARRYOVER-FEATURES.md` swept entry by entry** (landed | ||
| between v2.4.0 and v2.4.1; part of neither), against | ||
| `main` @ `fdfb2c04`. Eleven entries struck, each carrying its evidence inline — | ||
| a file that exists, a workflow line number, a test that says so — rather than a | ||
| bare tick, so a closure can be disagreed with. | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use one precise release-scope statement across all release documents.
The v2.4.1 cut changes only the two manifests. The carried v2.4.0 work changed
rustynes-core. Replace the “changed in both halves” wording at every site and report byte-identical output separately from verification results..github/release-notes/v2.4.1.md#L5-L5: identify v2.4.0 as the implementation-changing half.AGENTS.md#L44-L44: apply the same scope distinction to repository instructions.README.md#L697-L704: correct the current-release narrative.docs/STATUS.md#L21-L27: correct the authoritative status block.📍 Affects 4 files
.github/release-notes/v2.4.1.md#L5-L5(this comment)AGENTS.md#L44-L44README.md#L697-L704docs/STATUS.md#L21-L27🤖 Prompt for AI Agents
Source: Learnings