test(fds): wire TakuikaNinja's FDS hardware-verification probes#309
Merged
Conversation
The user surfaced NESdev-scene news about newly-published FDS test ROMs from TakuikaNinja covering $4023 register behavior and nametable mirroring. Investigation confirmed RustyNES already correctly models and independently unit-tests both ($4023 master I/O-enable logic at fds.rs:2187-2211, clear_4023_stops_and_acks; mirroring via the FDS control-byte bit 3, deliberately not subject to the game_database.txt mirroring-override hazard since FDS has zero DB entries) -- so this is regression insurance against a second, hardware-verified oracle, not a fix for a functional gap. Sourced and verified all four of TakuikaNinja's published probes directly against their GitHub release assets (not the unreliable auto-summarized wiki text): FDS-Mirroring-Test ($4025.D3 write, $4030.D3 read, $4023.D0=0 nametable-arrangement reset), FDS-4023-Test ($4020-$409F register readback while toggling $4023 bits 0/1), FDS-Audio-Registers (audio register readback while toggling $4023.D1 during wavetable playback), and FDS-4030D1-Addr (DRAM-refresh-watchdog IRQ timing via $4030.D1 -- explicitly under-research upstream and not modeled by most current FDS emulators, RustyNES included). None of the four carries an explicit permissive license (no LICENSE file, nothing stated in any README), which falls short of the bar tests/roms/ (committed) requires. Staged them gitignored under tests/roms/external/fds-takuikaninja/ instead, mirroring the existing commercial-ROM convention -- verified gitignored via `git check-ignore`, nothing redistributed via git. Added crates/rustynes-test-harness/tests/fds_takuikaninja.rs: four RUSTYNES_FDS_BIOS-gated smoke tests (mirroring the existing fds_irq_tests_with_real_bios pattern in fds.rs) that skip cleanly with a printed notice when the BIOS or the specific probe disk is absent, so CI stays clean by default. Verified both paths locally: the skip path (no BIOS) and the real-BIOS path (against the already-staged local disksys.rom) both construct, run 600 frames, and complete without panicking against all four real hardware-verification disk images. Deliberately assert only construction + a bounded run complete -- these ROMs render pass/fail state as on-screen text and register dumps, and decoding that programmatically is a follow-up once the exact screen layout is confirmed against real hardware captures; asserting a specific outcome now would claim precision this harness does not actually verify. Documented the new tests/roms/external/fds-takuikaninja/ subdirectory in tests/roms/external/README.md (a gitignored, untracked local file -- not part of this commit) and added a new docs/accuracy-ledger.md row tracking the $4030.D1 DRAM-watchdog residual as an honest, tracked-not-asserted gap rather than a silently-passed claim. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds BIOS/ROM-gated regression smoke tests for newly published TakuikaNinja FDS hardware-verification probes, and records the $4030.D1 DRAM-refresh-watchdog IRQ as a tracked (not yet asserted) residual in the accuracy ledger.
Changes:
- Add
rustynes-test-harnessintegration tests that load/run four external (gitignored) FDS probe disks under a real BIOS, skipping cleanly when prerequisites are absent. - Extend
docs/accuracy-ledger.mdwith an explicit “tracked not asserted” row for the$4030.D1watchdog IRQ behavior gap. - Document the above under
[Unreleased]inCHANGELOG.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/accuracy-ledger.md | Adds a new ledger row tracking the $4030.D1 watchdog IRQ as an honest residual, linked to the new gated probe test. |
| crates/rustynes-test-harness/tests/fds_takuikaninja.rs | Introduces four test-roms + RUSTYNES_FDS_BIOS-gated smoke tests that run external FDS probe disks for a bounded number of frames. |
| CHANGELOG.md | Adds an [Unreleased] “Added” entry describing the new gated probe wiring and its licensing/staging constraints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request integrates TakuikaNinja FDS hardware-verification probes as gated, gitignored smoke tests in crates/rustynes-test-harness/tests/fds_takuikaninja.rs, documents this addition in CHANGELOG.md, and updates docs/accuracy-ledger.md to track the unimplemented DRAM-refresh-watchdog IRQ ($4030.D1). Feedback was provided to refactor the file path construction in the test runner using std::path::PathBuf instead of raw string formatting with forward slashes to prevent cross-platform path resolution issues on Windows.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
copilot-pull-request-reviewer caught a grammar error in the CHANGELOG
entry ("behavior these probe is" -- fixed to "behaviors these probes
exercise are"). gemini-code-assist caught a cross-platform path hazard
in fds_takuikaninja.rs: the disk path was built via string
concat!/format! with a hardcoded forward slash, which is a portability
smell on Windows (part of this project's CI matrix). Switched to
PathBuf::from(...).join(...).join(...), using .display() in the skip
eprintln! since PathBuf doesn't implement Display directly. Re-verified
both the skip path and the real-BIOS path locally after the change --
unchanged behavior, still 4/4 passing either way.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
5 tasks
doublegate
added a commit
that referenced
this pull request
Jul 15, 2026
Bumps the workspace version 2.2.0 -> 2.2.1 to ship everything merged to main since the v2.2.0 "Capstone" tag: PR #304 (Game Genie re-key research tooling archived to scripts/gg/), PR #305 (six Dependabot bumps consolidated with zero source changes -- pollster 0.4->1.0, wide 0.7->1.5, tungstenite+tokio-tungstenite 0.29->0.30, bytemuck/cc patch, actions/setup-python v5->v6), PR #306/#307 (their CHANGELOG records), PR #308 (the 2A03-revision DMA-divergence probe archived to scripts/probes/), and PR #309 (four gitignored, RUSTYNES_FDS_BIOS-gated smoke tests against TakuikaNinja's FDS $4023/mirroring/audio-register/ DRAM-watchdog hardware-verification probes). This is a pure housekeeping patch: zero accuracy, feature, or core changes. The deterministic #![no_std] chip stack, save-state/TAS/ netplay-replay formats, and every golden vector are untouched -- AccuracyCoin holds 141/141 (100.00%), unchanged from v2.2.0. Version bump scope (workspace Cargo.toml `[workspace.package] version` is the single source of truth; every crate inherits via `version.workspace = true`, confirmed via `cargo clippy --workspace` reporting v2.2.1 for all 18 crates): - Cargo.toml / Cargo.lock: 2.2.0 -> 2.2.1 - CHANGELOG.md: renamed [Unreleased] -> [2.2.1] - 2026-07-15, added a fresh empty [Unreleased] above it - docs/STATUS.md, README.md (badge + "Current Release" section), AGENTS.md (canonical -- CLAUDE.md/GEMINI.md are symlinks to it): updated "current release" narrative to v2.2.1, reframing v2.2.0 "Capstone" as the prior release this patch sits on top of - .github/release-notes/v2.2.1.md: authored for release-auto.yml's override path (preferred over the CHANGELOG-section fallback) Deliberately NOT touched: the Android app's versionCode/versionName and the iOS app's MARKETING_VERSION -- these track their own host-only release cadence (frozen at their v2.0.x readiness points) and are decoupled by design from the desktop/core version until the joint mobile store launch at v2.3.0. Also not touched: to-dos/ ROADMAP.md's "Current release" line, which was already stale (citing v2.1.0) before this patch and is a pre-existing narrative-staleness issue orthogonal to this release's scope. Full gate green: cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings (all 18 crates confirm v2.2.1), cargo test --workspace (exit 0, zero failures), markdownlint on every touched doc. Co-authored-by: Claude Opus 4.8 <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
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.
Summary
Follows up on NESdev-scene news about newly-published FDS test ROMs from
TakuikaNinja covering
$4023register behavior and nametable mirroring.Investigation confirmed RustyNES already correctly models and independently
unit-tests both (
$4023master I/O-enable logic atfds.rs:2187-2211,clear_4023_stops_and_acks; mirroring via the FDS control-byte bit 3,deliberately not subject to the
game_database.txtmirroring-overridehazard since FDS has zero DB entries) — so this is regression insurance
against a second, hardware-verified oracle, not a fix for a functional gap.
Sourced and verified all four of TakuikaNinja's published probes directly
against their GitHub release assets:
$4025.D3write,$4030.D3read,$4023.D0=0nametable-arrangement reset.
$4020-$409Fregister readback while toggling$4023bits 0/1.
$4023.D1during wavetable playback.$4030.D1,explicitly under-research upstream and not modeled by most current FDS
emulators, RustyNES included.
Licensing note
None of the four carries an explicit permissive license (no
LICENSEfile,nothing stated in any README), which falls short of the bar
tests/roms/(committed) requires. Staged them gitignored under
tests/roms/external/fds-takuikaninja/instead, mirroring the existingcommercial-ROM convention — verified via
git check-ignore, nothingredistributed via git.
Changes
crates/rustynes-test-harness/tests/fds_takuikaninja.rs: fourRUSTYNES_FDS_BIOS-gated smoke tests (mirroring the existingfds_irq_tests_with_real_biospattern) that skip cleanly with a printednotice when the BIOS or the specific probe disk is absent — CI stays
clean by default. Deliberately assert only construction + a bounded run
complete without panicking; these ROMs render pass/fail state as
on-screen text/register dumps, and decoding that programmatically is a
follow-up once the exact screen layout is confirmed against real
hardware captures.
docs/accuracy-ledger.md: new row tracking the$4030.D1DRAM-watchdogresidual as an honest, tracked-not-asserted gap.
CHANGELOG.md:[Unreleased]entry.Test plan
RUSTYNES_FDS_BIOSset): all 4 testsprint a skip notice and pass cleanly.
disksys.rom): all 4 tests construct, run 600 frames, and completewithout panicking against the real hardware-verification disk images.
cargo fmt --all --check/cargo clippy -p rustynes-test-harness --all-targets --features test-roms -- -D warnings— green.markdownlint(pre-commit hook) — green.