feat(accuracysnes): A5.18 — BRK is 8 cycles native, 7 in emulation

[doublegate]

The extra native cycle is the PBR push: a native BRK frame is PBR/PCH/PCL/P and an emulation frame is the same without PBR, so the difference is one stack write (8 clocks), not a 6-clock internal cycle.

Nothing returns, and that is what isolates BRK

The obvious chain is BRK into a handler that RTIs back. That measures BRK + RTI — and RTI also differs by one stack access between the modes (A5.19), so the pair would come back at two accesses per iteration and this row would be reporting its sibling's result added to its own. Deriving one from the other is weaker than measuring it.

So nothing returns. V_BRK_VEC points at the instruction after the BRK, which the runtime's jmp (V_BRK_VEC) trampoline reaches directly — there is no handler body at all, so no handler body whose cost could differ between the modes. Each frame is abandoned on the stack and S is restored after each span rather than during it.

Both BRK vectors already share that one trampoline: $FFE6 (native) and $FFFE (emulation) both land on brk_trampoline, which runtime.s documents as deliberate — in emulation $FFFE is shared between IRQ and BRK, so a separate pointer would invent a distinction the machine does not have. The consequence here is exactly what the row needs.

The repeat count was measured, not estimated

The first draft used four iterations on an estimate of ~35 dots each. It was wrong by a factor of three, and the H counter wraps at 341:

draft	native	emulation	reading
4 iterations	$FFF7	323	wrapped — a native span shorter than the emulation one, the arithmetic opposite of the assertion
3 iterations	297	291	difference exactly 6, both clear of the wrap

An iteration is really ~99 dots. The lesson is the one A5.19 already recorded and this row had to relearn: a wrapped span does not fail, it returns a plausible small number — record the two raw readings and look at them before choosing the count.

Y is the loop counter, not X: emulation forces the index registers to 8 bits, and anything that reads back differently between the two spans is a difference this row would report as BRK's.

Inject-verified

if false on the PBR push in op_brk fires the row's assertion, with A6.05 ("native pushes 4 bytes") failing alongside it as corroboration. exec.rs restored byte-clean from a pre-edit snapshot.

Verification

cargo fmt --check, workspace clippy at -D warnings, and the 56-test AccuracySNES harness suite all pass. snes9x: OK (15 known), Mesen2: OK (1 known), ares: OK (3 known), 54/54 scenes on both scene hosts, 3 reference(s) agree with the cart.

Coverage 354 → 355 of 443 (301 on-cart + 54 scenes); battery 342 tests, 100% on-cart.

🤖 Generated with Claude Code

Adds AccuracySNES assertion A5.18.

The test claims that BRK takes 8 cycles in native mode and 7 cycles in emulation mode because native mode performs one additional PBR stack write. The claim is false if that write is absent or if the measured timing difference is not 6 dots under the test setup.

The test uses three non-returning BRK executions and a shared trampoline. AccuracySNES coverage increases from 354/443 to 355/443 assertions. On-cart coverage increases from 300 to 301.

[coderabbitai]

Walkthrough

Added AccuracySNES A5.18 coverage for native and emulation-mode BRK timing. The test measures stack-access differences, registers the assertion, reports three measurements, and updates coverage documentation.

Changes

AccuracySNES BRK timing

Layer / File(s)	Summary
BRK timing measurement tests/roms/AccuracySNES/gen/src/tests/cpu.rs	Added a5_18() to measure three native-mode and three emulation-mode BRK executions. The test asserts the expected native stack-access penalty.
Test registration and measurement reporting tests/roms/AccuracySNES/gen/src/dossier.rs, crates/rustysnes-test-harness/tests/accuracysnes.rs	Registered the A5.18 assertion and added reporting slots and labels for both timings and their difference.
Coverage documentation docs/STATUS.md, docs/accuracysnes-plan.md, CHANGELOG.md	Updated AccuracySNES totals and documented the A5.18 milestone.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• doublegate/RustySNES#189: Modifies shared emulation BRK/IRQ vector handling used by the interrupt-trampoline context.
• doublegate/RustySNES#282: Adds a related AccuracySNES cross-mode CPU timing test for RTI.

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 9 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title uses a valid Conventional Commit type and scope and describes the A5.18 BRK timing change, but its subject is not in the required imperative mood.	Rewrite the subject in imperative mood, for example: "feat(accuracysnes): add A5.18 BRK timing test".

✅ Passed checks (9 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Changelog Entry	✅ Passed	The full origin/main...HEAD diff includes a 22-line CHANGELOG.md entry for A5.18 and the AccuracySNES cartridge changes.
Docs-As-Spec	✅ Passed	The only crates/** edit is a test-harness constant: it adds three AccuracySNES measurement labels and changes test reporting, not emulator or public crate behavior.
Accuracysnes Bookkeeping	✅ Passed	Base-to-HEAD diff adds A5.18 and maps it to A5.18; all six required artifact paths are present, no scene changed, and plan counts increase by one.
No Panic On Untrusted Input	✅ Passed	The commit adds no .unwrap(), .expect(), or panic!() calls; added-line scans are empty and parent/current Rust token counts are identical. A5.18 constructs local test data.
Safety Comment On New Unsafe	✅ Passed	The changed Rust files add no unsafe { ... } blocks or unsafe fn; the added A5.18 code is entirely safe Rust.

---

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

[github-actions]

Antigravity review (Gemini via Ultra)

This PR adds test row A5.18 to the AccuracySNES test ROM battery to verify that BRK takes 8 cycles in native mode and 7 cycles in emulation mode due to the extra PBR stack push.

Blocking issues

None found.

Suggestions

• crates/rustysnes-test-harness/tests/accuracysnes.rs:599: Group A measurement slots (272–274 for A5.18) were appended to GROUP_E_BATCH_SLOTS. Rename the constant or separate the slots into a Group A array to avoid miscategorized harness metadata.
• tests/roms/AccuracySNES/gen/src/tests/cpu.rs:2840 / tests/roms/AccuracySNES/asm/tests_group_a.s:4226: In test_a5_18, the emulation arm performs sec; xce inside the hv_begin/hv_end measurement window while the native arm executes clc; xce. Although CLC and SEC take identical cycle counts (2 CPU cycles / 1 bus access), performing mode transitions within the timed block risks introducing mode-switch side effects into the measured differential. Move sec; xce prior to hv_begin.

Nitpicks

• tests/roms/AccuracySNES/gen/src/tests/cpu.rs:2897: Hardcoded bounds #$0004 and #$0009 are used directly in assert_a16_range rather than referencing named constants for the tolerance window.

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

[Copilot]

Pull request overview

Adds a new AccuracySNES CPU timing assertion (A5.18) to measure the cycle difference of BRK between native and emulation mode, and updates the generated ROM artifacts and documentation/counters accordingly.

Changes:

• Introduce A5.18 in the AccuracySNES generator (gen/src/tests/cpu.rs) and wire it into the test list + dossier map.
• Regenerate/update AccuracySNES artifacts (assembled Group A, source catalog, error codes) and bump documented coverage/test counts.
• Extend the harness measurement-slot reporting to include A5.18’s raw timing slots.

Reviewed changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/roms/AccuracySNES/gen/src/tests/cpu.rs	Implements the new A5.18 generator-side test logic and assertions.
tests/roms/AccuracySNES/gen/src/dossier.rs	Adds A5.18 to the dossier coverage map.
tests/roms/AccuracySNES/asm/tests_group_a.s	Regenerated assembled Group A test code including test_a5_18 and updated test tables/count.
tests/roms/AccuracySNES/SOURCE_CATALOG.tsv	Catalog updated to include the new A5.18 row and shifted indices.
tests/roms/AccuracySNES/ERROR_CODES.md	Documents the new A5.18 failure code meaning.
crates/rustysnes-test-harness/tests/accuracysnes.rs	Adds measurement-slot reporting strings for A5.18’s raw timing slots.
docs/STATUS.md	Updates the advertised AccuracySNES assertion coverage/test counts.
docs/accuracysnes-plan.md	Updates the plan summary counts for tests and assertion coverage.
docs/accuracysnes-coverage.md	Updates the coverage summary to reflect A5.18 now covered on-cart.
CHANGELOG.md	Records the addition and rationale for A5.18.

Suppressed comments (2)

tests/roms/AccuracySNES/gen/src/tests/cpu.rs:2860

• This comment says the second span abandons "four" more frames, but the loop runs three BRKs (Y = #$03). Keeping the prose accurate helps when adjusting repeat counts later.

    a.c("Put S back before the second span abandons four more frames onto it.");

crates/rustysnes-test-harness/tests/accuracysnes.rs:628

• The slot list is no longer in ascending slot order: the (266/267) entries come after (270–274). Keeping these ordered makes the printed diagnostic output easier to scan and compare across runs.

    (266, "E3.06 timer 0 ticks over the interval"),
    (
        267,
        "E3.06 timer 2 ticks over the SAME interval (TnOUT wraps at 16)",
    ),

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@tests/roms/AccuracySNES/gen/src/tests/cpu.rs`:
- Around line 2792-2797: Update the documentation comment above the relevant CPU
test to describe three abandoned frames, matching the loop’s ldy #$03 and three
BRK executions: 12 bytes in native mode and 9 bytes in emulation mode. Leave the
test implementation and the later iteration-count discussion unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9693832d-c23a-484e-82ed-deebd8930f29

📥 Commits

Reviewing files that changed from the base of the PR and between 8f19133 and a709279.

⛔ Files ignored due to path filters (6)

• docs/accuracysnes-coverage.md is excluded by !docs/accuracysnes-coverage.md and included by docs/**
• tests/roms/AccuracySNES/ERROR_CODES.md is excluded by !tests/roms/AccuracySNES/ERROR_CODES.md and included by tests/**
• tests/roms/AccuracySNES/SOURCE_CATALOG.tsv is excluded by !**/*.tsv, !tests/roms/AccuracySNES/SOURCE_CATALOG.tsv and included by tests/**
• tests/roms/AccuracySNES/asm/tests_group_a.s is excluded by !tests/roms/AccuracySNES/asm/tests_group_a.s and included by tests/**
• tests/roms/AccuracySNES/build/accuracysnes-pal.sfc is excluded by !tests/roms/AccuracySNES/build/** and included by tests/**
• tests/roms/AccuracySNES/build/accuracysnes.sfc is excluded by !tests/roms/AccuracySNES/build/** and included by tests/**

📒 Files selected for processing (6)

• CHANGELOG.md
• crates/rustysnes-test-harness/tests/accuracysnes.rs
• docs/STATUS.md
• docs/accuracysnes-plan.md
• tests/roms/AccuracySNES/gen/src/dossier.rs
• tests/roms/AccuracySNES/gen/src/tests/cpu.rs

📜 Review details

⏰ Context from checks skipped due to timeout. (5)

• GitHub Check: accuracysnes
• GitHub Check: test-light
• GitHub Check: lint
• GitHub Check: copilot-pull-request-reviewer
• GitHub Check: build demo + docs

🧰 Additional context used

📓 Path-based instructions (15)

docs/**/*.md

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Before changing a subsystem, consult docs/architecture.md, docs/STATUS.md, CONTRIBUTING.md, the relevant subsystem documentation, and applicable ADRs.

New subsystems must add documentation under docs/.

Files:

• docs/STATUS.md
• docs/accuracysnes-plan.md

**/*.{rs,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Chip-behavior changes must update both the chip implementation and the corresponding docs/<subsystem>.md documentation.

A chip change must update both the chip implementation and its corresponding docs/<chip>.md documentation in the same change.

Files:

• docs/STATUS.md
• crates/rustysnes-test-harness/tests/accuracysnes.rs
• docs/accuracysnes-plan.md
• tests/roms/AccuracySNES/gen/src/dossier.rs
• tests/roms/AccuracySNES/gen/src/tests/cpu.rs
• CHANGELOG.md

**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Do not commit or vendor the generated snesdev_wiki/ mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits: <type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevant to-dos/ sprint file.

**/*: Preserve the one-directional crate graph: chip crates must not depend on one another; rustysnes-core ties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keep docs/STATUS.md as the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNES v2.0 or engine-lineage anchors as project releases.

Files:

• docs/STATUS.md
• crates/rustysnes-test-harness/tests/accuracysnes.rs
• docs/accuracysnes-plan.md
• tests/roms/AccuracySNES/gen/src/dossier.rs
• tests/roms/AccuracySNES/gen/src/tests/cpu.rs
• CHANGELOG.md

docs/**/*

📄 CodeRabbit inference engine (docs/testing-strategy.md)

Chip crates should exceed 90% unit-test coverage, and each chip should be fuzzable in isolation.

Files:

• docs/STATUS.md
• docs/accuracysnes-plan.md

docs/**

⚙️ CodeRabbit configuration file

docs/**: Docs are the spec, not a history log. Flag claims that contradict the code, counts that
contradict the generated docs/accuracysnes-coverage.md, and any statement of coverage that
is broader than what the corresponding test actually asserts.

Files:

• docs/STATUS.md
• docs/accuracysnes-plan.md

**/*.md

⚙️ CodeRabbit configuration file

**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).

Files:

• docs/STATUS.md
• docs/accuracysnes-plan.md
• CHANGELOG.md

crates/**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

crates/**/*.rs: Preserve the master-clock lockstep timing model.
rustysnes-core::Bus owns mutable machine state, and the CPU borrows &mut Bus.
Preserve determinism: seed, ROM, and input must produce bit-identical output.
Treat test ROMs as the behavioral specification; when documentation disagrees with passing ROM behavior, update the documentation.
Keep unsafe confined to existing allowed areas, namely frontend and FFI code, and document every unsafe block with a // SAFETY: comment.

Files:

• crates/rustysnes-test-harness/tests/accuracysnes.rs

**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.rs: Use Rust edition 2024 and the toolchain pinned in rust-toolchain.toml (Rust 1.96).
Run cargo fmt --all --check; Rust code must remain rustfmt-compliant.
Run Clippy with cargo clippy --workspace --all-targets -- -D warnings; warnings must not remain.
New public Rust items must have rustdoc because missing_docs is a workspace lint.
Do not run cargo clippy --all-features; scripting and script-wasm are mutually exclusive. Use explicit per-feature jobs instead.

**/*.rs: Do not introduce .unwrap(), .expect(), or panic!() on untrusted external input—such as ROM/save-state bytes, netplay messages, Lua or scripting input, or user-supplied paths—outside #[cfg(test)] code. Use typed errors at those boundaries; locally constructed values or values immediately protected by a checked invariant are allowed.
Every new unsafe { ... } block or unsafe fn must have an adjacent // SAFETY: comment naming the relied-on invariant and its guarantor. Unsafe code outside the frontend and FFI shims should additionally be questioned because unsafe_code is a workspace lint.

**/*.rs: Use Rust edition 2024 with the pinned 1.96 toolchain; satisfy workspace pedantic, nursery, missing_docs, and unsafe_code warnings because CI runs with -D warnings. Document every public item.
Keep unsafe code restricted to the frontend and FFI, and include a // SAFETY: justification for each use.
Keep hot paths allocation-free.
Treat rustysnes_core::Bus as the owner of mutable emulator state; the CPU borrows &mut Bus.
Use the master clock at 21477270 Hz as the timing master; advance the scheduler in lockstep and run other chips on their divisors.
Maintain determinism: seed, ROM, and input must produce bit-identical audio/video; frontend rate control must not alter emulation results.
When implementing hardware behavior, pin and run the failing test ROM first; treat test ROMs as the specification.

Files:

• crates/rustysnes-test-harness/tests/accuracysnes.rs
• tests/roms/AccuracySNES/gen/src/dossier.rs
• tests/roms/AccuracySNES/gen/src/tests/cpu.rs

crates/rustysnes-*/**/*

📄 CodeRabbit inference engine (Custom checks)

For the full pull request diff against its base branch, any observable behavior change under crates/rustysnes-<chip>/ must be accompanied by an edit to the matching docs/<chip>.md; a crate change passes without documentation only when it does not alter observable behavior, with the non-behavioral change stated explicitly.

Files:

• crates/rustysnes-test-harness/tests/accuracysnes.rs

**/*.{rs,toml}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{rs,toml}: Additive features must be default-off so shipped/native, no_std, and wasm builds remain byte-identical.
Never use or configure --all-features; validate opt-in feature combinations individually as required by the project recipe.

Files:

• crates/rustysnes-test-harness/tests/accuracysnes.rs
• tests/roms/AccuracySNES/gen/src/dossier.rs
• tests/roms/AccuracySNES/gen/src/tests/cpu.rs

crates/**

⚙️ CodeRabbit configuration file

crates/**: Emulator core. Hot paths are allocation-free; unsafe requires a // SAFETY: comment
naming the invariant. Any change to save-stated fields needs a FORMAT_VERSION bump and a
docs/adr/0006 bump-log entry. Behavior changes must update the matching docs/<chip>.md
in the same change.

Files:

• crates/rustysnes-test-harness/tests/accuracysnes.rs

tests/roms/AccuracySNES/gen/src/**/*

📄 CodeRabbit inference engine (Custom checks)

For the full pull request diff against its base branch, when a test or scene is added or removed under tests/roms/AccuracySNES/gen/src/, verify its dossier.rs::MAP entry, all required regenerated artifacts, and matching count changes in docs/accuracysnes-plan.md. Artifact presence must be judged from the path-filter exclusion list, not from unreadable contents.

Files:

• tests/roms/AccuracySNES/gen/src/dossier.rs
• tests/roms/AccuracySNES/gen/src/tests/cpu.rs

tests/roms/AccuracySNES/gen/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Rebuild AccuracySNES after any change to gen/ or asm/; never hand-edit generated asm/tests_group_a.s or asm/scenes.s.

Files:

• tests/roms/AccuracySNES/gen/src/dossier.rs
• tests/roms/AccuracySNES/gen/src/tests/cpu.rs

tests/roms/AccuracySNES/gen/src/**

⚙️ CodeRabbit configuration file

tests/roms/AccuracySNES/gen/src/**: This generates a hardware-accuracy test cartridge. Judge each test by whether it can
distinguish the behavior it names from the alternatives, not by whether it passes.

Flag, specifically:

• Vacuity. An assertion whose expected value is also what a broken or absent
  implementation produces (zero, "unchanged", "not $FF") needs a paired control assertion
  that would fail on that implementation. Say which alternative goes uncaught.
• Overstated doc comments. The prose above a test is a claim about what it validates.
  If it names behaviors the emitted program does not exercise, or asserts a rationale that
  is not true of the code, that is a defect even though the test passes.
• Shared state. OAM, CGRAM, VRAM and the S-DSP registers are not reset between tests. A
  test that does not establish its own starting conditions may be measuring the previous
  one; look for an earlier test that leaves the relevant state dirty.
• Timing-marginal reads. Reading a register a few cycles after disturbing it, or
  asserting on a value that is still moving, produces a verdict that flips when unrelated
  code shifts. Prefer a settle, a disarm, or a provably stationary value.
• Scanline geometry. Line 0 is a blanking line; the V counter's low byte aliases on a
  312-line PAL frame; the visible height is 224 or 239 depending on overscan. Constants
  derived from any of these deserve a second look.
• Duplicate coverage. dossier.rs::MAP must not claim an assertion another test already
  implements. There is a build gate for this, but flag it in review too.

Files:

• tests/roms/AccuracySNES/gen/src/dossier.rs
• tests/roms/AccuracySNES/gen/src/tests/cpu.rs

CHANGELOG.md

📄 CodeRabbit inference engine (CONTRIBUTING.md)

User-visible changes must be recorded under the [Unreleased] section.

For the full pull request diff against its base branch, modify CHANGELOG.md when user-visible behavior changes, including emulator output, frontend features, CLI flags, public APIs, or AccuracySNES cartridge contents. Do not require it for purely internal changes, tests, comments, or CI configuration.

Files:

• CHANGELOG.md

🧠 Learnings (6)

📚 Learning: 2026-07-21T01:34:22.909Z

Learnt from: doublegate
Repo: doublegate/RustySNES PR: 189
File: docs/accuracysnes-plan.md:0-0
Timestamp: 2026-07-21T01:34:22.909Z
Learning: When reviewing the AccuracySNES documentation in docs/accuracysnes-*.md (notably docs/accuracysnes-plan.md vs the generated docs/accuracysnes-coverage.md), treat the reported metrics as intentionally non-equivalent: the battery test count and dossier assertion coverage are not interchangeable. Do not infer one count/coverage from the other during review (e.g., one test may contain multiple assertions, and multiple tests may contribute to a single assertion/row such as E6.02).

Applied to files:

• docs/accuracysnes-plan.md

📚 Learning: 2026-07-21T05:22:58.848Z

Learnt from: doublegate
Repo: doublegate/RustySNES PR: 197
File: docs/accuracysnes-plan.md:598-600
Timestamp: 2026-07-21T05:22:58.848Z
Learning: In the AccuracySNES documentation under `docs/`, when an assertion exists in the research dossier but cannot be measured/verified by the current cartridge timing test, distinguish the dossier assertion from test measurability: keep the original hardware assertion (and any contribution to the coverage denominator) intact, withdraw/stop using the specific test coverage only if the sources cannot decompose the required CPU-cycle timing into bus vs internal components, and mark the row as not measurable using the `[NOT CART-MEASURABLE ...]` annotation with links to the corresponding plan section (e.g., `docs/accuracysnes-plan.md` §A5.20) and the related roadmap/ticket (e.g., `to-dos/ROADMAP.md` ticket `T-06-A`). Ensure the documentation/coverage reporting treats the row as uncovered rather than removing or redefining the assertion.

Applied to files:

• docs/accuracysnes-plan.md

📚 Learning: 2026-07-21T02:10:49.581Z

Learnt from: doublegate
Repo: doublegate/RustySNES PR: 190
File: tests/roms/AccuracySNES/gen/src/tests/ppu.rs:0-0
Timestamp: 2026-07-21T02:10:49.581Z
Learning: For any AccuracySNES tests that perform a runtime measurement investigation using writes to $2137 and $4201, do not reuse measurement/slot indices that may already be owned by another test. Before using a slot, verify it is unused (e.g., via an on-cart probe/readback that confirms the slot contains no prior test result). Then independently record $213F immediately before and immediately after each $2137/$4201 operation, so the test can attribute changes to its own operation and avoid cross-test interference.

Applied to files:

• tests/roms/AccuracySNES/gen/src/dossier.rs
• tests/roms/AccuracySNES/gen/src/tests/cpu.rs

📚 Learning: 2026-07-21T05:32:09.180Z

Learnt from: doublegate
Repo: doublegate/RustySNES PR: 197
File: tests/roms/AccuracySNES/gen/src/tests/cpu.rs:0-0
Timestamp: 2026-07-21T05:32:09.180Z
Learning: For SNES LoROM CPU accuracy tests covering JMP (a,X) and JSR (a,X) pointer-bank wrapping, do not place the tested effective pointer addresses below $8000 in LoROM banks $00-$3F or $80-$BF. In RustySNES, those banks’ $0000-$1FFF map to the same 8 KiB WRAM (so different bank forms alias identically), while $2000-$7FFF is mirrored I/O/open-bus; this can make pointer-bank assertions vacuous. Instead, use ROM-resident pointers at linker-controlled, bank-distinct addresses and include a paired control/target case that distinguishes the carried-bank behavior (e.g., for the triggering form ($FFFE,X) with X=$1002, avoid a situation that produces wrapped $00:1000 aliasing with an incorrectly carried $01:1000).

Applied to files:

• tests/roms/AccuracySNES/gen/src/tests/cpu.rs

📚 Learning: 2026-07-21T06:21:34.629Z

Learnt from: doublegate
Repo: doublegate/RustySNES PR: 198
File: docs/accuracysnes-plan.md:0-0
Timestamp: 2026-07-21T06:21:34.629Z
Learning: When reviewing slot allocation for SNES/ROM measurement channels in generator-driven Rust code, account for slots assigned via generation-time computed writers (e.g., slots derived from formulas like `slot_base = 8 + index * 2`) rather than only literal `record(...)` calls. Trace the computed writer’s full emitted slot range and verify there are no collisions with other opcodes/channels that may use different slot ranges after earlier conflict resolution.

Applied to files:

• tests/roms/AccuracySNES/gen/src/tests/cpu.rs

📚 Learning: 2026-07-22T06:12:00.703Z

Learnt from: doublegate
Repo: doublegate/RustySNES PR: 201
File: tests/roms/AccuracySNES/gen/src/tests/apu.rs:1508-1538
Timestamp: 2026-07-22T06:12:00.703Z
Learning: When working on APU/voice timing in these AccuracySNES test sources, treat NTSC vs PAL differences as a first-class constraint. In particular, do not change `Voice::settle` (or any settling/code-size/timing logic it affects) unless you cross-validate against both regions (NTSC and PAL), because timing/polling phase shifts can cause regressions that may appear as PAL-only test failures. For ROM-specific expectations like `E7.13`, keep the intentionally chosen ENVX range (e.g., `0x68..=0x7C`) unless you re-validate that the absorbed post-KON timing variation still matches across both regions.

Applied to files:

• tests/roms/AccuracySNES/gen/src/tests/cpu.rs

🪛 LanguageTool

docs/STATUS.md

[style] ~244-~244: Try using a descriptive adverb here.
Context: ...cs/adr/0013), kept as separate columns on purpose (docs/accuracysnes-coverage.md`, regen...

(ON_PURPOSE_DELIBERATELY)

🔇 Additional comments (6)

tests/roms/AccuracySNES/gen/src/tests/cpu.rs (1)

98-98: LGTM!

tests/roms/AccuracySNES/gen/src/dossier.rs (1)

86-86: LGTM!

crates/rustysnes-test-harness/tests/accuracysnes.rs (1)

599-623: LGTM!

docs/STATUS.md (1)

26-27: LGTM!

Also applies to: 244-244

docs/accuracysnes-plan.md (1)

16-17: LGTM!

CHANGELOG.md (1)

14-35: LGTM!

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Stale iteration count in the doc comment.

Line 2795 states "four of them is 16 bytes native and 12 emulation." The loop uses ldy #$03 and executes brk 3 times, not 4. Three native frames are 12 bytes (3 × 4), and three emulation frames are 9 bytes (3 × 3), not 16 and 12.

This number is a leftover from the original 4-iteration draft. The doc later corrects the iteration count in "the first draft used four iterations... wrapped... Three iterations measure 297 native against 291 emulation," but this earlier paragraph still names the wrong count.

As per path instructions, "The prose above a test is a claim about what it validates. If it names behaviors the emitted program does not exercise... that is a defect even though the test passes."

📝 Proposed fix

-/// abandoned on the stack; four of them is 16 bytes native and 12 emulation, and `S` is put back
+/// abandoned on the stack; three of them is 12 bytes native and 9 emulation, and `S` is put back
 /// after each span rather than during it.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	/// So nothing returns. `V_BRK_VEC` is pointed at the instruction *after* the `BRK`, which the
	/// runtime's `jmp (V_BRK_VEC)` trampoline reaches directly — there is no handler body at all, and
	/// therefore no handler body whose cost could differ between the modes. Each frame is simply
	/// abandoned on the stack; four of them is 16 bytes native and 12 emulation, and `S` is put back
	/// after each span rather than during it.
	///
	/// So nothing returns. `V_BRK_VEC` is pointed at the instruction *after* the `BRK`, which the
	/// runtime's `jmp (V_BRK_VEC)` trampoline reaches directly — there is no handler body at all, and
	/// therefore no handler body whose cost could differ between the modes. Each frame is simply
	/// abandoned on the stack; three of them is 12 bytes native and 9 emulation, and `S` is put back
	/// after each span rather than during it.
	///

🤖 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 `@tests/roms/AccuracySNES/gen/src/tests/cpu.rs` around lines 2792 - 2797,
Update the documentation comment above the relevant CPU test to describe three
abandoned frames, matching the loop’s ldy #$03 and three BRK executions: 12
bytes in native mode and 9 bytes in emulation mode. Leave the test
implementation and the later iteration-count discussion unchanged.

Source: Path instructions