chore(release): v0.3.0 "Microcode" — the LLE RSP (Phase 2 close) - #54
Merged
Conversation
Phase 2 close. Both exit criteria are met and on main (RSP category n64-systemtest Failed: 0, and libdragon's rdpq microcode emitting an RDP command list). This prepares the v0.3.0 tag: - Bump the workspace + all crate versions 0.2.0 -> 0.3.0 (Cargo.lock too). - CHANGELOG: rename [Unreleased] -> [0.3.0] "Microcode" (2026-07-22) with a full Phase 2 summary (PRs #35-#53: the SU, the VU, the vector load/store family, VZERO, BREAK-in-delay-slot, the DPC registers, the RSP->RDP seam, and the real-microcode boot harness); fresh [Unreleased] points at v0.4.0. - README: status Phase 1-in-progress -> Phase 2 complete (v0.3.0); update the badges, the highlights/compatibility tables, the current-release and roadmap sections, and the citation. - docs/STATUS.md: current release -> v0.3.0; Phases 1-2 complete. - to-dos/VERSION-PLAN.md: correct the stale "never cut a release" note. Pre-release gate green: fmt, clippy (-D warnings), cargo test --workspace, rustdoc (-D warnings), no_std, markdownlint (changed files). Versions consistent across Cargo/README/CHANGELOG. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe project version was advanced to v0.3.0, and release documentation was updated to mark Phases 0–2 complete. The changelog documents Phase 2 evidence, LLE RSP, the RSP-to-RDP seam, real microcode boot coverage, and remaining later-phase work. ChangesRelease alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
Comment |
Antigravity review (Gemini via Ultra)This PR performs the v0.3.0 release bump, updating workspace and crate version numbers from Blocking issuesNone found. SuggestionsNone found. Nitpicks
Automated first-pass review by |
Owner
Author
|
Adjudicating the Antigravity review: Nitpick — CHANGELOG exit-criteria table omits runner path links: REJECTED, with reason. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 36-53: Add exact provenance for the hardware behavior and
constants described in the changelog, including the 12-bit PC wrapping, correct
misaligned accesses, VZERO semantics, and 0x37 encoding. Cite the specific
manual or wiki pages, or label each item as measured/inferred and document the
measurement method and results in docs/accuracy-ledger.md; apply the same
treatment to the referenced lines 82–85.
In `@README.md`:
- Around line 11-12: Keep all v0.3.0 release claims pending until the tag
exists: in README.md lines 11-12, 25-37, 375-406, and 496-504; docs/STATUS.md
lines 6-9 and 23-34; to-dos/VERSION-PLAN.md lines 10-15; and CHANGELOG.md lines
12-33, 34-66, and 67-91, remove or defer version badges, completion/status
claims, current-tag and release references, citations, and release details so
none present v0.3.0 as tagged, current, or published before the corresponding
tag is created.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ca9fc176-2ca1-4bd3-bc79-90accb6aa9d2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock
📒 Files selected for processing (15)
CHANGELOG.mdCargo.tomlREADME.mdcrates/rustyn64-audio/Cargo.tomlcrates/rustyn64-cart/Cargo.tomlcrates/rustyn64-cheevos/Cargo.tomlcrates/rustyn64-core/Cargo.tomlcrates/rustyn64-cpu/Cargo.tomlcrates/rustyn64-frontend/Cargo.tomlcrates/rustyn64-netplay/Cargo.tomlcrates/rustyn64-rdp/Cargo.tomlcrates/rustyn64-rsp/Cargo.tomlcrates/rustyn64-test-harness/Cargo.tomldocs/STATUS.mdto-dos/VERSION-PLAN.md
CodeRabbit review of #54 (adopted): the release commit declared v0.3.0 as already "cut and tagged" / "the tag matches the tree", but the v0.3.0 tag is created only on merge. Reword STATUS.md + README to state that this commit *is* the v0.3.0 release, tagged from it on merge -- accurate for the tagged commit without asserting a tag that does not yet exist. The version bump itself (Cargo/badge/CHANGELOG heading) is required in the tagged commit and stays. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Update the version-reference token (now at v0.2.0 -> v0.3.0) in the MC-PROJECT block and normalize a trailing double-blank line (markdownlint MD012). The larger "Current state" narrative in AGENTS.md is maintained separately by the maintainer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (2)
25-32: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not describe COP1 as complete while the authoritative status marks it partial.
docs/STATUS.mdLines 121-139 record unimplementedBC1F/BC1T, missingSQRT, and 37 remaining COP1 assertions. Retain the verified gate result, but label COP1/VR4300 completion consistently with that documented status.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 25 - 32, Update the COP1 and VR4300 status wording in README.md at lines 25-32, 90, and 501-504 to match docs/STATUS.md: retain the verified gate result, but do not describe COP1 as complete; identify its remaining partial or unimplemented functionality, including BC1F/BC1T, SQRT, and the remaining assertions, and ensure all three README locations use consistent completion language.Source: Path instructions
381-384: 📐 Maintainability & Code Quality | 🔵 TrivialRun the pinned markdownlint hook locally.
.pre-commit-config.yamlpinsmarkdownlinttov0.49.1, andAGENTS.mdsays markdownlint is pre-commit only here;pre-commit run markdownlint --all-filesneeds to be part of the change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 381 - 384, Run the pinned markdownlint hook with pre-commit before finalizing the change: README.md lines 381-384 and docs/STATUS.md lines 24-35 require validation, with any reported markdown issues fixed while preserving their existing test documentation.Sources: Path instructions, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@README.md`:
- Around line 25-32: Update the COP1 and VR4300 status wording in README.md at
lines 25-32, 90, and 501-504 to match docs/STATUS.md: retain the verified gate
result, but do not describe COP1 as complete; identify its remaining partial or
unimplemented functionality, including BC1F/BC1T, SQRT, and the remaining
assertions, and ensure all three README locations use consistent completion
language.
- Around line 381-384: Run the pinned markdownlint hook with pre-commit before
finalizing the change: README.md lines 381-384 and docs/STATUS.md lines 24-35
require validation, with any reported markdown issues fixed while preserving
their existing test documentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1f4a3fb9-d5a2-48e5-a959-0459bcb823e0
📒 Files selected for processing (2)
README.mddocs/STATUS.md
Rewrite the "Current state" section to reflect both phases complete: Phase 1 (v0.2.0) and Phase 2 (v0.3.0, the LLE RSP + real microcode emitting an RDP command list). The RSP now executes (SU + VU + SP DMA + the COP0 c8-c15 -> DPC seam); only the RDP rasteriser and AI remain LLE-shaped stubs. CLAUDE.md is a symlink to this file, so both update together. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
doublegate
added a commit
that referenced
this pull request
Jul 30, 2026
…ep 1) (#206) * perf(frontend): port the triple-buffer present handoff from RustyNES/RustySNES First step on the user-reported GUI stall: menu clicks taking 15-45s, one presented frame every 30-60s, SM64 showing a frame then hanging. DIAGNOSED, not guessed. The emu thread (emu_thread.rs:70-80) holds the emu mutex across its ENTIRE `coordinator.step()` -- not "under a brief lock" as its own comment claims -- and when it falls behind it sets `next = now` and re-acquires immediately with no sleep and no yield. Since the core is ~6.5x slower than real time (measured: 462s of emulated time took 2985s wall in the corpus census), it is ALWAYS behind, so it holds the lock ~100% of the time in a tight loop. `app.rs:144 snapshot()` needs that same lock every UI frame just to clone the framebuffer, so the UI starves on an unfair mutex. DO NOT REINVENT -- the user pointed out the sibling projects already solve this, and they do. `RustySNES/.../present_buffer.rs` (itself ported from RustyNES) is exactly this handoff, and its module doc names RustyN64's bug verbatim: copying the framebuffer out from under the emu mutex "would serialize the present against the emulation thread's whole run_frame (which the emu-thread feature exists specifically to decouple)". RustyN64 shipped `emu-thread` WITHOUT the handoff, so it does the thing that module documents as wrong. Ported here, same author throughout so it is licence-clean. Triple-buffer SPSC: producer writes `back` and publishes by swapping back<->ready; consumer swaps front<->ready. Producer and consumer touch disjoint slots (the packed index's three 2-bit fields stay a permutation of {0,1,2}), and the small dedicated mutex is held only for one memcpy -- never across emulation. N64-specific adaptations: - FB_LEN is the PAL worst case (720x576) and is a SIZING HINT ONLY; the N64's scan-out is variable (625x237 typical NTSC) so slots resize per publish rather than zero-padding a small frame up to the maximum. - Dims travel WITH their bytes through the same lock. Not hypothetical here: ledger R-18 records a single-point sample catching Super Mario 64 with `H_VIDEO = 0` mid-VI-reprogram, which is exactly the mismatch a separately queried dims read would produce. - The status-bar readings (frame count, master_ticks, loaded, paused) are published beside the frame as plain atomics, because they were the OTHER thing `snapshot()` took the emu lock for -- decoupling only the bytes would have left the starvation in place for the sake of a status line. Determinism (ADR 0004) is unaffected: this moves WHERE already-produced deterministic bytes are copied. No emulated state, no audio. Six tests, each aimed at a way the index could be wrong rather than at coverage: a publish/take round trip; a take with nothing new preserving the previous frame; two publishes between takes yielding the NEWEST (a mis-packed index would return stale bytes); dims surviving a size change; the index staying a permutation of {0,1,2} over 32 cycles; and reset clearing the published state. NOT YET WIRED -- this lands the primitive with its tests so the wiring change (emu_thread publishes, app.rs stops taking the emu lock, and the missing yield when behind) is reviewable on its own. Tasks #54-#57 track the rest, including RustyNES's perf_log.rs (the instrumentation task #55 needs before optimizing anything) and resampler.rs (the audio hiccup). Gates: fmt, clippy -D warnings, cargo test --workspace, rustdoc -D warnings, check_en_us -- all green. * fix(en-us): the gate could not see NEW files, and gave a false PASS Found while correcting a doc error in present_buffer.rs: the en-US gate reported PASS on commit 25b5212, and that commit contains a `licence`. ROOT CAUSE: the gate enumerated files with `git ls-files`, which lists only TRACKED files. A brand-new file is untracked until it is staged, so it was invisible to the check for exactly as long as it took to `git add` it -- and a new file is precisely where a new violation arrives. The pre-commit hook would not have caught it until the NEXT commit, by which point the bad spelling is already on main. My own gate, blind in the one place it most needed to see. FIX: enumerate tracked files PLUS `git ls-files --others --exclude-standard` (untracked but not ignored). `--exclude-standard` keeps the original property that scratch files under a gitignored path never fail the gate. Verified three ways, because only the set of three shows it is right: 1. clean tree -> passes 2. an UNTRACKED new file containing "colour" -> now CAUGHT (was invisible) 3. a file under the gitignored target/ -> still correctly exempt Also in this commit, both from the wiring survey of the frontend: - CORRECTED A FACTUAL ERROR in present_buffer.rs's module doc: it referred to `EmuApp::snapshot()` twice. There is no `EmuApp` in this crate -- the winit struct is `App` (app.rs:59). The name came in with the port from RustySNES and I did not check it against this codebase. A doc that names a type which does not exist sends the next reader grepping for nothing. - RECORDED A MEASUREMENT that strengthens the case for the handoff: `Frame::blank` sizes `rgba` at `FB_MAX_W * FB_MAX_H * 4` = 1,228,800 bytes and `produce_frame` never resizes it, so `snapshot()`'s `frame.rgba.clone()` copied the ENTIRE backing store every UI frame whatever the active resolution. The UI was not just waiting on the mutex, it was doing a 1.2 MB memcpy while holding it. Publishing the `w * h * 4` prefix instead is 592,500 bytes at 625x237 and 307,200 at 320x240 -- under a quarter the copy, and off the emu mutex entirely. Gates: fmt, clippy -D warnings, cargo test, rustdoc -D warnings, markdownlint, check_no_roms, check_en_us -- all green. * fix(frontend): reset() must clear the handoff under its own lock Adopted from the Antigravity review of #206, and it is a violation of this module's own stated contract: the SPSC section claims the mutex guards the index, the has_new flag, the bytes and the dims *together*, while reset() cleared has_new BEFORE acquiring the lock and never touched the dims. The window is real, not theoretical -- reset() runs on the UI thread while the emu thread publishes. A publish landing between the store and the lock set has_new = true and then had its bytes wiped, so the next take_into returned Some(dims) over a zero-length buffer. The present path slices out[..w * h * 4] from exactly that pair, which panics. Two independent halves, so the invariant does not rest on locking alone: - has_new is now cleared inside the lock, making the state unreachable; - the dims are cleared, making "nonzero dims, no bytes" unrepresentable. generation stays outside the lock on purpose: clippy's significant_drop_tightening is right that this is the present hot path, and the counter is a diagnostic. The residual race is now documented with its bound -- one UI frame of the previous image instead of black, and never a slice, because has_new and the dims are under the lock. Tests (the review's nitpick, and the mutation check for the fix): - reset_clears_the_dims_so_none_can_be_paired_with_empty_bytes -- verified red against the old reset with exactly the stale (2, 2) the review predicted; - status_round_trips_and_reset_zeroes_only_the_counters -- pins that reset leaves rom_loaded/paused to the caller, so a closed ROM must republish them. Also records that status() can tear ACROSS its four independent atomics, not only lag by a frame, and that a seqlock -- not stronger orderings -- is the fix if a consistent (frames, ticks) pair is ever needed. Gates: fmt, clippy -D warnings, cargo test --workspace, rustdoc -D warnings, check_en_us.sh -- all green. * refactor(frontend): name the two slot-index transitions Adopted from the second Antigravity review of #206. `pack` takes three positional slot ids in field order, so the calls the transitions needed -- `pack(front, back, ready)` to publish and `pack(ready, front, back)` to take -- read like transpositions of pack's own parameters. The hazard is that such a slip is SILENT: every permutation of {0,1,2} satisfies the disjointness invariant, so `the_slot_index_stays_a_permutation` keeps passing while the wrong slot is handed over. This project has already paid for one transposed positional-argument call this cycle (the TEX_BLOCK base/step swap), so the concern is not hypothetical. swap_ready_back / swap_front_ready are const fns over the same `pack`, so the bit layout is unchanged and the port stays structurally comparable to the RustySNES original. Pinned by the_named_transitions_swap_exactly_one_pair, which asserts each transition moves exactly one pair and leaves the third field alone -- mutation-checked by transposing swap_ready_back's first two arguments, which takes that test and five others red. Gates: fmt, clippy -D warnings, cargo test --workspace, rustdoc -D warnings, check_en_us.sh -- all green. * fix: reset() must reach its documented empty state, and the en-US gate must fail closed Both findings from the CodeRabbit review of #206, both adopted. reset() left rom_loaded and paused untouched -------------------------------------------- The doc said "reset to the empty state" and the body did not reach it, which is this repo's recurring comment-versus-code split. This REVERSES a choice made one commit earlier -- and pinned in a test -- that the two flags were the caller's to republish. The review's reasoning is better: the failure directions are not symmetric. Clearing them costs at most one UI frame of "no ROM" right after a load, which the next publish corrects. NOT clearing them leaves a closed ROM reading as loaded until something publishes again, and with the emu-thread feature off nothing ever does. A stale positive outlives a stale negative, so the gate clears all four and status() now equals PresentStatus::default() after a reset. The test asserts that whole equality rather than field-by-field. The en-US gate could still report PASS over zero files ------------------------------------------------------ `mapfile -d '' -t files < <(producer)` discards the producer's exit status, and `set -euo pipefail` cannot see inside a process substitution -- so a failing `git ls-files` produced an empty array and the script then reported success over nothing. MEASURED against the previous commit's script with a stub `git` whose `ls-files` exits 128: en-US check passed: 0 files (tracked + untracked), no en-GB or malformed spellings. exit=0 This is the SECOND false PASS in this one file -- the first was the missing `--others`, which hid new files. Same lesson each time: a gate whose failure mode is a silent pass is worse than no gate. The listing now goes through a file so each `git ls-files` is a simple command `set -e` can see, and an empty result is treated as a broken gate rather than a clean tree. The grep filters stay status-tolerant on purpose (`grep -v` exits 1 when it selects nothing, which is not an error), so the emptiness check is what carries the guarantee. Verified three ways: normal run passes over 444 files; a `git` that fails exits 128; a `git` that succeeds while returning nothing exits 1 with an explicit "the file listing is empty" diagnostic. Gates: fmt, clippy -D warnings, cargo test --workspace, rustdoc -D warnings, check_en_us.sh, markdownlint -- all green. * fix(frontend): enforce that published bytes and dims agree Adopted from the third Antigravity review of #206. `publish` established the (bytes, dims) pair that this whole type exists to keep consistent, and checked nothing -- so an inconsistent pair was invisible here and would surface far away as an out-of-bounds slice in whatever presents `out[..w * h * 4]`. It found a real bug immediately: publishing_twice_between_takes_yields_the_newest offered 8 bytes for (4, 1), a 16-byte frame. That test had been passing since the port, and an eyeball audit of the nine publish call sites in this file missed it -- which is the argument for the assert rather than against it. Kept as a debug_assert, not a release branch. The release alternatives are to drop the frame silently or to clamp the geometry, and both hide the caller's bug while producing a wrong picture. The release-mode defense belongs at the consumer, which will bound its upload by the bytes it actually received when the present path is wired up. The expected length is computed with saturating_mul so the check itself cannot overflow on a 32-bit usize. The should_panic test is #[cfg(debug_assertions)] -- without that gate it would fail under `cargo test --release`, since the assert compiles out. Verified: the release run passes with the test absent rather than failing. Gates: fmt, clippy -D warnings, cargo test --workspace, rustdoc -D warnings, check_en_us.sh -- all green.
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.
Release: v0.3.0 "Microcode" — Phase 2 close (the LLE RSP)
Both of Phase 2's exit criteria are met and already on
main:Failed: 0(RSP category)cargo test -p rustyn64-test-harness --release --test systemtest -- --ignoredcargo test -p rustyn64-test-harness --test microcodeThis PR is the phase-close prep for the
v0.3.0tag (docs + version bumps only —no behavior change):
Cargo.lock.[Unreleased]→[0.3.0] "Microcode"(2026-07-22) with the fullPhase 2 summary (PRs Close Phase 1's four real gaps, and give the RSP its memory (T-21-001) #35–feat(rsp): emit RDP commands from the rdpq microcode via the DPC seam (Phase 2 criterion 2) #53); fresh
[Unreleased]points at v0.4.0.compatibility tables, current-release + roadmap sections, and the citation.
.gitignoreneeds no changes (no new artifacts; the microcode blob + symbol mapwere already committed under
crates/rustyn64-test-harness/microcode/).Gates run locally
fmt, clippy (
-D warnings),cargo test --workspace, rustdoc (-D warnings),the
thumbv7em-none-eabihfno_std build, and markdownlint on the changed.mdfiles — all green. Versions consistent across Cargo / README / CHANGELOG.
After this merges,
v0.3.0will be tagged onmainwith annotated release notes.🤖 Generated with Claude Code