Skip to content

chore(release): cut v2.4.9 "Plumbline II" — the bus half of rung 2, and what it found the day it existed - #445

Merged
doublegate merged 3 commits into
mainfrom
chore/v2.4.9-release
Aug 23, 2026
Merged

chore(release): cut v2.4.9 "Plumbline II" — the bus half of rung 2, and what it found the day it existed#445
doublegate merged 3 commits into
mainfrom
chore/v2.4.9-release

Conversation

@doublegate

Copy link
Copy Markdown
Owner

Cuts v2.4.9 "Plumbline II" — the bus half of rung 2, and what it found the day it existed.

A plumb line does not describe vertical; it is vertical, and everything else is measured against it. Rung 1 compares seven CPU registers at instruction boundaries; this adds a reference the cycles are held against.

Both mutations v2.4.8 recorded as NOT CAUGHT are caught by it — so the release named for the read-modify-write double write can, one release later, verify one. Rung 1 grows to seven ROMs, 1663 records (RustyNES_MiSTer@715952b).

It found two real defects on its first run

Neither is visible to rung 1 by construction — CpuBootTrace carries pc, a, x, y, p, s, cycle, and neither defect changes any of them.

  • Indexed RMW skipped its dummy read — the RMW branch left pre-access cycles at addr = pc.
  • STA $xxxx,X without a page cross wrote TWICE. The comment above the line said "we stays low even for a store"; the code read we = d_ir.writes && !idx_page_cross. Same memory, cycles and registers — and on hardware a mapper register written twice is not one written once.

Divergences went 7 → 1 → 0.

Both sides must start from the same work RAM

The oracle seeds its 2 KiB from a PRNG, so a zeroed testbench disagrees on every read of unwritten memory — 5 of the original 7 divergences. <stem>.ram_init.bin is exported and loaded rather than the PRNG being reimplemented in C++, where a second copy would drift.

pc populated, deliberately not compared

Verified observation-only first (900 records differing in those two bytes and nothing else, boot trace byte-identical). Still not a gate: the two sides define it differently and agree on 45% of cycles. It labels divergences instead.

Three tests agreed with their own mutations

SLO's flags; DCP writing A (twice — the second time because A was $42 and the byte became $42, v2.4.4's TXS/TSX exactly); and one mutant that did not compile, reported as its own outcome rather than as a catch. Eight mutations, all caught, baseline control correctly NOT CAUGHT.

Also lands the logical group (AND/ORA/EOR/BIT — a hard prerequisite that was simply missing) and the undocumented opcodes (LAX, SAX, six RMW combinations, multi-byte NOPs).

Gates

fmt clean · clippy --workspace --all-targets -D warnings 0 errors · 2233 workspace tests, 0 suites failed · all four release audits green (8/8/2/3).

The current-tag rule fired again — third consecutive release it has caught this drift.

No chip crate changes, so AccuracyCoin 141/141 and nestest 0-diff hold by construction.

Written from public documentation only. No reference NES core was opened; none is present in the tree.

🤖 Generated with Claude Code

https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj

doublegate and others added 3 commits August 23, 2026 00:27
 where the oracle does

Rung 2's bus comparison needs both sides to agree about memory the
program has not written, and they did not.

`Nes::from_rom_with_power_on_seed` fills the 2 KiB of work RAM from a
seeded PRNG, so the oracle's unwritten RAM is deterministic but NOT zero.
A co-simulation testbench with flat, zeroed memory therefore disagrees on
every read of an untouched location -- and those reads are routine, not
exotic: the dummy read of an un-indexed zero-page address lands on
unwritten RAM constantly.

Rung 1 never compares memory, so this was invisible until a per-cycle bus
trace existed. It then accounted for 5 of the 7 divergences in the first
DUT-versus-oracle bus comparison, on a program rung 1 scores 358/358.

`<stem>.ram_init.bin` is captured BEFORE a single cycle runs, beside the
existing `<stem>.ram.bin` which is captured after. Both are 2 KiB; the
names are deliberately not `ram.bin` and `ram_final.bin`, because
renaming an existing golden would silently invalidate every consumer that
already reads it.

## Exported rather than reproduced

The obvious alternative is to reimplement the PRNG in the C++ testbench.
That is precisely the parallel second implementation module 90 forbids: a
copy that agrees today and drifts the first time the oracle's power-on
model changes, with the drift appearing as a CPU divergence at some
unrelated cycle. The DUT loads these bytes; it does not compute them.

With the golden loaded, the bus comparison over `opgroup5` reports **0
divergences across 793 cycles** on address, data and access type.

The emulation core is untouched -- this reads `bus().ram_bytes()`, which
already existed for the post-run export.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
…ng else

`Observable.pc` was zero in every record the bus gate read, because
`CycleRecord.pc` is documented as "0 when `cpu-instr-cycle-trace` is not
enabled" and this crate did not enable it. So a bus divergence was a bare
cycle number, and naming the instruction meant hand-disassembling the
ROM.

Enabling a trace feature on this crate is not automatically safe, which
is the whole reason it sits outside the workspace: `irq-timing-trace`
selects a DIFFERENT `for sub_dot in 0..3` loop in
`Bus::tick_one_cpu_cycle`, and cargo's feature unification is what made
the accuracy battery validate a scheduler no user runs.

So this was measured rather than assumed. Re-exported `opgroup5` before
and after, and compared record by record:

  records differing ONLY in pc (bytes 8..9): 900
  records differing in ANY other field     : 0
  boot.bin byte-identical                  : True

Observation-only, unlike its neighbour.

**It is still not compared.** The two sides do not mean the same thing by
`pc`: this one holds the INSTRUCTION's opcode-fetch PC across every cycle
of that instruction, while the DUT exposes its live PC register, which
advances during operand fetches. Measured, they agree on 45% of cycles --
high enough to look nearly right, far too low to gate on. The sibling's
`bus_diff.py` uses it as a label instead:

  [bus diff @ cycle=40 in $C016]

which is what this field is documented for.

The emulation core is untouched; this changes only which features an
already-excluded crate asks for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
 what it found the day it existed

A plumb line does not describe vertical; it IS vertical, and everything
else is measured against it. v2.3.1 borrowed the name for measurement
replacing intuition. This borrows it one rung up: rung 1 compares seven
CPU registers at instruction boundaries, and this adds a reference the
CYCLES are held against.

  make -C tb cpu-bus-gate ROM=opgroup7 BUS_GOLDEN=... RAM_INIT=...
  All 693 overlapping cycles match on bus_addr, bus_data and bus_access

Both mutations v2.4.8 recorded as NOT CAUGHT are caught by it. The
release named for the read-modify-write double write can, one release
later, verify one.

Rung 1 grows to SEVEN ROMs, 1663 records
(147/140/286/179/358/236/317). RTL is RustyNES_MiSTer@715952b.

## Two real defects, found the day the gate existed

Neither is visible to rung 1 by construction -- CpuBootTrace carries pc,
a, x, y, p, s and cycle, and neither defect changes any of them.

INDEXED RMW SKIPPED ITS DUMMY READ. The RMW branch drove the bus only
from the access cycle onward and left everything before it at the default
`addr = pc`, so `LSR $30,X` never performed the dummy read of the
un-indexed $0030 that its plain-read counterpart does.

`STA $xxxx,X` WITHOUT A PAGE CROSS WROTE TWICE. The comment directly
above the line said "`we` stays low even for a store, which is why a
write always needs cycle 4", and the code read
`we = d_ir.writes && !idx_page_cross`. Same memory, same cycle count,
same registers -- and on hardware a mapper register written twice is not
a register written once.

The prose was right and the code was wrong. That is why nobody
re-checked it, and it is the third time this project has recorded that
shape.

Divergences went 7 -> 1 -> 0 across 793 cycles.

## Both sides must start from the same work RAM

The oracle fills its 2 KiB from a seeded PRNG, so a flat zeroed testbench
disagrees on every read of a location the program has not written -- and
the dummy read of an un-indexed zero-page address is one, constantly.
That was 5 of the original 7 divergences.

`<stem>.ram_init.bin` is captured before a single cycle runs and loaded
by the testbench. Exported rather than reimplemented: a PRNG written a
second time in C++ is a copy that drifts, and the drift would surface as
a CPU divergence at an unrelated cycle.

## `pc` is populated, and still not compared

It was zero in every record, because `CycleRecord.pc` is documented as
"0 when `cpu-instr-cycle-trace` is not enabled" and the cosim crate did
not enable it.

Enabling a trace feature there is not automatically safe -- that is why
the crate sits outside the workspace at all, since `irq-timing-trace`
selects a DIFFERENT per-dot loop. So it was measured: 900 records
differing ONLY in those two bytes, ZERO differing in any other field,
boot trace byte-identical.

Still not a gate. The oracle holds the INSTRUCTION's opcode-fetch PC
across every cycle of that instruction; the DUT exposes its live PC
register, which advances during operand fetches. They agree on 45% of
cycles -- high enough to look nearly right, far too low to gate on. It
labels divergences instead.

## Documented opcodes in the undocumented release

AND, ORA, EOR and BIT were simply not implemented, and they are a hard
prerequisite: SLO is ASL then ORA, RLA is ROL then AND, SRE is LSR then
EOR. Recorded as dependency-driven scope rather than quietly widened.

BIT is the trap -- N and V from the MEMORY byte, Z from `A & M`, no
register written -- and it agrees with an AND-path implementation on all
three flags whenever the operand's top bits match the result's, which is
true of the obvious test operand $FF.

## Three tests agreed with their own mutations

SLO's flags: NOT CAUGHT, because A = $05 made the OR and the shifted byte
share sign and non-zeroness. DCP writing A: NOT CAUGHT twice -- first
because the next instruction overwrote A, then because A was $42 and the
byte became $42, so the wrong value equalled the right one, which is
v2.4.4's TXS/TSX exactly. And RRA's overflow mutation BUILD-FAILED, which
proves nothing and is reported as its own outcome.

Eight mutations, all caught, baseline control correctly NOT CAUGHT.

## Gates

  cargo fmt --all --check                             clean
  cargo clippy --workspace --all-targets -D warnings  0 errors
  cargo test --workspace                              2233 passed, 0 suites failed
  release_anchor_audit / _state_prose / _notes_render / libretro_info
                                                      8 / 8 / 2 / 3

The current-tag rule fired again on this cut, third consecutive release:

  to-dos/ROADMAP.md:62  calls v2.4.8 the current tag,
                        but the workspace is at 2.4.9

No crate under rustynes-{cpu,ppu,apu,mappers,core} changes, so
AccuracyCoin 141/141 (100.00%, RAM decoder) and nestest 0-diff hold BY
CONSTRUCTION.

The libretro .info display_version moves to v2.4.9 in this repository's
copy only; per the cadence rule the next upstream sync is v2.5.0.

Written from public documentation only. No reference NES core was opened;
none is present in the tree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
Copilot AI lite review requested due to automatic review settings August 23, 2026 04:36
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Your free Security trial is over. An organization admin can activate billing to continue.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@doublegate, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 7 minutes

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4b6c0f4b-acb2-41fe-96f8-24201baa7c4b

📥 Commits

Reviewing files that changed from the base of the PR and between d7c6722 and 4da25aa.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock, !Cargo.lock
  • crates/rustynes-cosim/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • .github/release-notes/v2.4.9.md
  • AGENTS.md
  • ARCHITECTURE.md
  • CHANGELOG.md
  • Cargo.toml
  • OVERVIEW.md
  • README.md
  • ROADMAP.md
  • SECURITY.md
  • SUPPORT.md
  • VERSION-PLAN.md
  • crates/rustynes-cosim/Cargo.toml
  • crates/rustynes-cosim/src/bin/nes_golden_export.rs
  • crates/rustynes-libretro/rustynes_libretro.info
  • docs/STATUS.md
  • docs/mister.md
  • to-dos/ROADMAP.md
  • to-dos/plans/v2.5.0-fabric-plan.md

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions

Copy link
Copy Markdown

Antigravity review (Gemini via Ultra)

Cuts the v2.4.9 release by bumping versions in manifests and documentation, while extending the co-simulation exporter to output the initial RAM state and enable CPU instruction cycle tracing.

Blocking issues

None found.

Suggestions

  • crates/rustynes-cosim/src/bin/nes_golden_export.rs: Consider moving the existing assert_eq!(..., RAM_LEN) check above the new ram_init export to ensure the RAM length is validated before any RAM files are written to disk. The change as-is is trivial, but validates the array size earlier.

Nitpicks

None.

Automated first-pass review by agy on a self-hosted runner -- not a human review.

@doublegate

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@doublegate

Copy link
Copy Markdown
Owner Author

Ceremony note for this PR, recorded before merge.

Antigravity: no blocking issues, one suggestion — move the assert_eq!(..., RAM_LEN) above the new ram_init write. Correct, and sharper than reported: that assert covers the post-run ram buffer, so ram_init had no length check at all. A co-simulation testbench mirrors those 2 KiB through $1FFF, so a short buffer would place the bytes wrongly rather than fail — surfacing as a bus divergence at an unrelated address, which is the most expensive way to learn about a length mismatch.

Fixed in a8583cfe, on the v2.5.0 branch rather than here. Same handling as the non-blocking suggestion on #442: the finding is a hardening improvement with no behavioural change, and restarting a ~40-minute matrix on a PR one job from green to carry it is not a good trade. Stating where it landed so it is not left implicit.

CodeRabbit did not review this PR. Rate-limited — one included review per hour under Fair Usage, and the @coderabbitai review request returned that notice rather than a review. Copilot's attempt errored out. So the automated review this PR actually received was Antigravity's alone, and the finding above is all of it. Recorded because "all threads resolved" would otherwise read as three-bot coverage when it was one.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@doublegate Thanks for the clear record.

The ram_init length assertion must run before the testbench writes ram_init into mirrored RAM. The existing post-run ram assertion did not validate that input buffer.

Commit a8583cfe contains the hardening change outside PR #445. The stated scope decision is reasonable because the change has no behavioral effect on this release.

The rate-limit result means CodeRabbit performed no review for PR #445. I will not represent this PR as having a CodeRabbit review.


✏️ Learnings added
Learnt from: doublegate
URL: https://github.com/doublegate/RustyNES/pull/445

Timestamp: 2026-08-23T05:04:49.784Z
Learning: In the RustyNES co-simulation testbench, validate `ram_init.len() == RAM_LEN` before copying `ram_init` into mirrored 2 KiB RAM. A later assertion on the post-run `ram` buffer does not validate `ram_init`; a short input can misalign mirrored RAM and cause bus divergences at unrelated addresses.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

@doublegate
doublegate merged commit 163e7b0 into main Aug 23, 2026
29 of 30 checks passed
@doublegate
doublegate deleted the chore/v2.4.9-release branch August 23, 2026 05:05
doublegate added a commit that referenced this pull request Aug 23, 2026
CodeRabbit reviewed this PR (rate limit permitting, which it did not on
the previous two) and raised seven. Four are real and fixed here; two are
checkably wrong and are refuted with the evidence rather than skipped.

## Applied

**"Within noise" is not a pass/fail criterion.** Correct, and it landed
while I was in the middle of failing to apply it: measuring ADR 0038's
precondition produced +1.3%, -1.4%, +1.7%, -1.0% across four workloads,
and I had no threshold to judge that against. Mixed signs are the
signature of code layout, not cost -- but "signature of" is not a gate.

Constraint 2 is now two checks, and the first is DECISIVE rather than
statistical:

  2a  cargo expand -p rustynes-core --lib | grep -c inject_   MUST be 0
  2b  full_frame vs a `main` worktree baseline, same runner,
      one session, 100 samples, all four workloads within +/-1.0%

2a is a proof: the fields and branches are `#[cfg]`-gated, so a default
build emits none of them. **Measured on this implementation: 0.** 2b
corroborates. If 2a is 0 and 2b exceeds the band, the finding is the
measurement environment, because code that does not exist cannot cost
anything -- and saying so in advance stops that from becoming an argument
after the fact.

**Two "nestest 0-diff" claims mean different things.** Constraint 3's is
RustyNES's own, full-run, and true -- re-verified in both configurations
for this commit. The DUT's is bounded at cycle 27396 by its first $2002
read. The ADR now says which is which, and states that nothing in it
extends the bounded one. The reviewer took the wrong one, which is
evidence the wording invited it.

**docs/mister.md carried two incompatible current states.** A new status
section at the top and, ninety lines down, "the undocumented opcodes are
not delivered" and "rung 2 has not begun" -- both false since v2.4.9. The
older text is labelled historical rather than deleted, because the
reasoning still explains why the rung was scoped that way.

**SECURITY.md's matrix said 2.4.x was the current line** while its own
first line said v2.5.0. `bump_release.py` moves the prose anchor and not
the table. 2.5.x is now current and 2.4.x is Partial.

**Two files named v2.5.0 meant different releases.**
`docs/release-notes/v2.5.0.md` is the ENGINE-LINEAGE v2.5.0 (2026-06-11),
which shipped inside RustyNES v1.0.0; `.github/release-notes/v2.5.0.md`
is this release. Renaming the older one would destroy documented lineage,
so it gains a banner saying what it is. This is the exact confusion
AGENTS.md has a standing rule about, now visible at the file rather than
only in the rule.

## Refuted

**"Update the mapper description to 172."** It is **174**, and has been
since v2.3.4 added 176/2, 154 and 243. README states 174 in five places
and `AGENTS.md` agrees. `libretro_info_audit.rs` pins the file against the
workspace manifest, so a wrong count here would fail a gate. Not changed.

**"Replace 'current tag' with 'current release' -- no tag exists."** True
at PR time and true of every release cut in this project: `release-auto`
creates the tag when the version goes green on `main`, minutes after
merge. Refuted on #445 for the same reason and refuted the same way here,
because `release_state_prose_audit`'s fourth rule keys on that literal
phrase and is the gate that has caught this line drifting on four
consecutive releases. Rewording it would leave that rule matching zero
claims across the corpus -- policing nothing, silently.

Gates: release_anchor_audit / _state_prose / _notes_render all green;
markdownlint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
doublegate added a commit that referenced this pull request Aug 23, 2026
…tes it cannot reach (#446)

* docs(adr): ADR 0038 -- a test-only interrupt-injection API, and the
 contract it amends

Rung 2's interrupt-injection sweep has no oracle. The DUT half shipped in
v2.5.0 -- nmi_n/irq_n, the /NMI edge latch, level-sampled /IRQ, the
hijack decided at the push, delayed-I falling out of where the poll sits,
and testbench controls to assert either pin at a chosen cycle -- and none
of it can be compared against anything, because co-simulation needs the
same stimulus on both sides and `rustynes-core` has no way to receive it.

Searched rather than assumed: no injection entry point exists, and the
interrupts the oracle does produce come from hardware this rung does not
model -- /IRQ from the APU frame counter or a mapper, /NMI from the PPU.

## What the ADR decides, and what it costs

Four options are recorded with why each was rejected, because the
rejected ones are the argument. Doing nothing leaves the RTL's most
error-prone region resting on a reading of the documentation, in a
programme whose premise is that a reading of the documentation is not
evidence. Waiting for rungs 3 and 4 inverts the ladder: rung 3 would have
to be trusted before rung 2 could be checked, and a divergence would then
be ambiguous between the CPU and the PPU that produced the interrupt.
Comparing against a third emulator moves the problem somewhere harder.

So: a feature-gated, default-off injection API, under SIX constraints
that are the substance of the decision rather than a footnote. The two
that matter most are stated as preconditions of merging, not follow-ups:

  * zero hot-path cost when off, asserted by MEASUREMENT;
  * byte-identical default output verified with the feature ABSENT and
    again PRESENT-BUT-UNUSED.

The second is a distinct case for a specific reason. `irq-timing-trace`
selects a DIFFERENT per-dot loop merely by being compiled in, and that
one reached the accuracy battery itself -- the exact failure this
constraint exists to prevent.

If either check fails the decision is VOID, not "revisit", and the
fallback is written down. A decision whose failure mode is undefined
tends to get argued into acceptance after the fact.

## The contract is amended where it is stated, in both places

ADR 0037's hard contract and the Fabric plan's both said "the emulation
core is untouched" and both now point at 0038. Amending one and leaving
the other is how the original claim survives its own revision -- and this
project has already published a false claim assembled from two true
statements nobody re-read together.

The honest restatement is "untouched in the default build, with one
default-off test feature", which is WEAKER than what eight releases of
notes have said, and the ADR says so under Consequences rather than
burying it: that claim's value came from being unqualified.

## What this does NOT reach

nestest 0-diff and the 5 M-cycle bus window stay blocked. Both need a
PPU and are rung 3. Nothing here closes them, and v2.5.0's notes continue
to say so.

No code changes in this commit. The API is not implemented here; this
records the decision to allow it and the conditions under which it may
land.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj

* fix(cosim): length-check the power-on RAM before writing it

The review round on #445. Antigravity reported no blocking issues and one
suggestion -- move the existing `assert_eq!(..., RAM_LEN)` above the new
`ram_init` write so the length is validated before anything reaches disk.

Correct, and sharper than reported: that assert covers the POST-RUN `ram`
buffer, so `ram_init` had no length check at all. It is now checked, on
itself, before the write.

Why it matters more than an ordering nicety: a co-simulation testbench
mirrors these 2 KiB through $1FFF, so a short buffer would place the
bytes WRONGLY rather than fail -- surfacing as a bus divergence at an
unrelated address, which is the most expensive way to learn about a
length mismatch.

## CodeRabbit did not review this PR

Rate-limited again -- one included review per hour, and the earlier
`@coderabbitai review` on this PR returned the same Fair-Usage notice
that v2.4.8's did. Recorded rather than left implicit: the ceremony on
#445 covered TWO bots, not three, and Copilot's own attempt errored out.
So the only substantive automated review this PR received is
Antigravity's, and this commit is its one finding.

Gates: fmt clean; clippy on the excluded crate 0 errors; the exporter
builds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj

* chore(release): cut v2.5.0 "Rungwork" -- the 6502 rung, and the two gates
 it cannot reach

Rungwork is the scaffolding you stand on to build the next thing, and the
honest name for this release: the rung it was scoped to close does not
fully close, for reasons that are structural rather than defects.

REAL: nine opcode-group ROMs at 2115 records; 4537 cycles of per-cycle
bus equality across all nine; 27,388 cycles of nestest at 8571
instructions; `pc` agreeing on 3551 of 3551 cycles and now a COMPARED
field. RTL is RustyNES_MiSTer@27171cd.

NOT: nestest 0-diff over the whole run, the 5 M-cycle window, and the
interrupt-injection sweep.

## Interrupts

nmi_n and irq_n with the distinction that matters -- /NMI EDGE-sensitive
and latching, /IRQ level-sampled. Implementing NMI as a level is the
classic error and is invisible on any test whose NMI stays asserted.

Delayed-I falls out of WHERE the poll sits rather than needing a special
case: it reads P before the retiring instruction changes it. The hijack
is decided at the push, not at entry -- deciding at entry makes it
impossible and nothing else changes, which is why it needs its own test.
Reset does not latch a pending NMI from an already-low line.

## Five defects, all found by the bus gate

RTS read the incremented address on its final cycle. AM_IZX's `default`
arm caught cycle 1, driving an effective address before the pointer byte
was read. AM_IZY tested `idx_page_cross` rather than `izy_cross` -- a
quantity with no meaning for a mode that adds Y to the FETCHED pointer --
and took five cycles where hardware takes six. AM_IZY also wrote at the
unfixed address when the index did not carry.

And `build()` stamped over every ROM's interrupt vectors, so opgroup8's
handlers were unreachable: BRK "worked" by falling through to the
program's first instruction, and BOTH SIDES WOULD HAVE AGREED, because
both read the same wrong ROM. The fill byte is NOP, not an impossible
value, so "was this written?" could not be answered by inspecting the
byte -- the assembler now returns the set of offsets the program wrote.

## The two gates that do not close

NESTEST STOPS AT A PERIPHERAL. It reads $2002 at cycle 27396. BOTH SIDES
ADDRESS $2002; only the data differs, because the oracle has a PPU and
this rung's testbench is flat memory. Rung 3 by design, and the 5 M-cycle
window hits the same wall sooner. The gate is bounded at 27396 and stated
as a number, because a gate whose extent moves silently is not a gate.

THE SWEEP HAS NO ORACLE. It needs NMI and IRQ asserted at chosen cycles
on BOTH sides, and rustynes-core exposes no injection API -- searched,
not assumed. So the pins, the hijack and delayed-I are IMPLEMENTED AND
NOT ORACLE-VERIFIED. BRK is verified, because a software interrupt needs
no pin and exercises the same seven-cycle sequence.

Saying this rather than shipping the number is the point. "The 6502 rung
closes" was this release's own scope, and a rung reported closed on a
gate it never ran is the exact failure this programme was built to catch.

## ADR 0038, and a contract amended in both places

A test-only, default-off injection API, six constraints, two of them
preconditions of merging: zero hot-path cost asserted by MEASUREMENT, and
byte-identical output verified with the feature ABSENT and again
PRESENT-BUT-UNUSED. That second case is distinct because `irq-timing-trace`
selects a different per-dot loop merely by being compiled in, and that one
reached the accuracy battery itself. If either fails the decision is VOID,
not "revisit", and the fallback is written down.

ADR 0037's hard contract and the Fabric plan both said "the emulation
core is untouched" and both now point at 0038. The honest restatement is
"untouched in the default build, with one default-off test feature" --
weaker than eight releases of notes have said, and recorded as a cost.

## No upstream sync

The cadence is amended: the next libretro/RetroArch sync waits for the
MiSTer core to be COMPLETE, at whatever vX.Y.0 that lands on. A lagging
display_version understates what shipped; a leading one overstates it.

## Gates

  cargo fmt --all --check                             clean
  cargo clippy --workspace --all-targets -D warnings  0 errors
  cargo test --workspace                              2233 passed, 0 suites failed
  release audits (anchor/prose/notes/libretro)        8 / 8 / 2 / 3

The current-tag rule fired again, fourth consecutive release.

No crate under rustynes-{cpu,ppu,apu,mappers,core} changes, so
AccuracyCoin 141/141 (100.00%, RAM decoder) and nestest 0-diff hold BY
CONSTRUCTION.

Written from public documentation only. No reference NES core was opened;
none is present in the tree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj

* docs: the v2.5.0 review round -- four findings applied, two refuted

CodeRabbit reviewed this PR (rate limit permitting, which it did not on
the previous two) and raised seven. Four are real and fixed here; two are
checkably wrong and are refuted with the evidence rather than skipped.

## Applied

**"Within noise" is not a pass/fail criterion.** Correct, and it landed
while I was in the middle of failing to apply it: measuring ADR 0038's
precondition produced +1.3%, -1.4%, +1.7%, -1.0% across four workloads,
and I had no threshold to judge that against. Mixed signs are the
signature of code layout, not cost -- but "signature of" is not a gate.

Constraint 2 is now two checks, and the first is DECISIVE rather than
statistical:

  2a  cargo expand -p rustynes-core --lib | grep -c inject_   MUST be 0
  2b  full_frame vs a `main` worktree baseline, same runner,
      one session, 100 samples, all four workloads within +/-1.0%

2a is a proof: the fields and branches are `#[cfg]`-gated, so a default
build emits none of them. **Measured on this implementation: 0.** 2b
corroborates. If 2a is 0 and 2b exceeds the band, the finding is the
measurement environment, because code that does not exist cannot cost
anything -- and saying so in advance stops that from becoming an argument
after the fact.

**Two "nestest 0-diff" claims mean different things.** Constraint 3's is
RustyNES's own, full-run, and true -- re-verified in both configurations
for this commit. The DUT's is bounded at cycle 27396 by its first $2002
read. The ADR now says which is which, and states that nothing in it
extends the bounded one. The reviewer took the wrong one, which is
evidence the wording invited it.

**docs/mister.md carried two incompatible current states.** A new status
section at the top and, ninety lines down, "the undocumented opcodes are
not delivered" and "rung 2 has not begun" -- both false since v2.4.9. The
older text is labelled historical rather than deleted, because the
reasoning still explains why the rung was scoped that way.

**SECURITY.md's matrix said 2.4.x was the current line** while its own
first line said v2.5.0. `bump_release.py` moves the prose anchor and not
the table. 2.5.x is now current and 2.4.x is Partial.

**Two files named v2.5.0 meant different releases.**
`docs/release-notes/v2.5.0.md` is the ENGINE-LINEAGE v2.5.0 (2026-06-11),
which shipped inside RustyNES v1.0.0; `.github/release-notes/v2.5.0.md`
is this release. Renaming the older one would destroy documented lineage,
so it gains a banner saying what it is. This is the exact confusion
AGENTS.md has a standing rule about, now visible at the file rather than
only in the rule.

## Refuted

**"Update the mapper description to 172."** It is **174**, and has been
since v2.3.4 added 176/2, 154 and 243. README states 174 in five places
and `AGENTS.md` agrees. `libretro_info_audit.rs` pins the file against the
workspace manifest, so a wrong count here would fail a gate. Not changed.

**"Replace 'current tag' with 'current release' -- no tag exists."** True
at PR time and true of every release cut in this project: `release-auto`
creates the tag when the version goes green on `main`, minutes after
merge. Refuted on #445 for the same reason and refuted the same way here,
because `release_state_prose_audit`'s fourth rule keys on that literal
phrase and is the gate that has caught this line drifting on four
consecutive releases. Rewording it would leave that rule matching zero
claims across the corpus -- policing nothing, silently.

Gates: release_anchor_audit / _state_prose / _notes_render all green;
markdownlint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants