fix(accuracysnes): E3.06 polls both timers, not just timer 2

[doublegate]

What

E3.06 compares timer 2's rate against timer 0's over one interval. Timer 2 was already accumulated across polls — that was the earlier fix for TnOUT's four-bit ceiling — but timer 0 was still read exactly once, at the end, on the reasoning that a handful of ticks is nowhere near its own wrap.

True about magnitude, and silent about phase: one read samples whatever sub-tick phase the previous test left the timer in. $F1's enable-raise resets a timer's stage-2 and stage-3 state (E3.02) but nothing resets stage 0, so the reading was reproducible only while nothing before it disturbed that phase.

Why now

Not hypothetical. Authoring a row that drives timer 2 hard (an SPC700 cycle-cost instrument for E2.10) broke E3.06 on the battery's second run and not its first — the residue crosses the run boundary.

The band was not the problem and was not widened to admit the new row. Weakening an existing row to make room for a new one is backwards; the row is right and its instrument was one read short.

The fix

Both counters are accumulated in the same poll loop, over 32 passes rather than 24, so neither side carries a phase.

The comparison is quantisation-honest for the first time. Each accumulated count is exact to ±1, so 8 × T0 carries ±8 and the difference ±9 — the previous ±6 was tighter than its own instrument could support, and passed only because the two phases happened to correlate. The band is now ±10 and still discriminating by a wide margin: a core running every timer from the 8 kHz stage lands some ninety ticks out.

Verification

• cargo run -p accuracysnes-gen — 343 tests, bank headroom unchanged
• cargo test -p rustysnes-test-harness --features test-roms --test accuracysnes — 56 passed
• crossval.sh — battery 309/309, 55 scenes match / 0 unblessed / 0 mismatched, 3 references agree (snes9x 15 known, Mesen2 1, ares 3 — all unchanged)

Measured across hosts, the same interval yields T0 = 12-13, T2 = 99. That one-tick spread in T0 is precisely the sensitivity ±6 could not absorb: it moves the target by 8.

🤖 Generated with Claude Code

The change claims that accuracysnes E3.06 measures timer 0 and timer 2 without dependence on residual timer phase. The defect occurs when a second test run inherits timer phase from the first run; the previous single timer 0 read can then fail.

The altered dossier assertions are slots 266 and 267. Slot 266 asserts the accumulated timer 0 count. Slot 267 asserts the timer 2 count relative to slot 266 with an expected 8:1 ratio. The tolerance changes from ±6 to ±10 to allow quantisation error while still rejecting incorrect timer implementations.

The coverage denominator did not move. Verification still reports 309/309 battery results and 55 matching scenes. The claim would be false if repeated runs still produce phase-dependent measurements, or if the wider tolerance accepts an incorrect timer implementation.

[coderabbitai]

Walkthrough

E3.06 now initializes and polls Timer 0 and Timer 2 across 32 passes. It accumulates both counters, validates the Timer 0 range and 8:1 ratio, and widens the ratio tolerance from ±6 to ±10. Harness descriptions and the changelog reflect the new measurements.

Changes

E3.06 timer accuracy

Layer / File(s)	Summary
Accumulate and validate timer counts tests/roms/AccuracySNES/gen/src/tests/apu.rs	E3.06 initializes both timers and accumulates their counts across 32 polling passes. It validates Timer 0 over the 6–20 range and checks the 8:1 ratio with a ±10-tick tolerance.
Update harness and changelog descriptions crates/rustysnes-test-harness/tests/accuracysnes.rs, CHANGELOG.md	The slot descriptions and changelog document accumulated Timer 0 and Timer 2 measurements, phase-independent polling, and the revised tolerance.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• doublegate/RustySNES#186: Introduced the E3.06 timer accuracy test revised here.
• doublegate/RustySNES#307: Modified the E3.06 timer measurement logic and related slots.
• doublegate/RustySNES#310: Modified E3.06 timer accumulation and ratio validation.

🚥 Pre-merge checks | ✅ 9 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title uses a valid Conventional Commits prefix and accurately describes the timer polling change, but the subject is not written in imperative mood.	Change “polls both timers” to an imperative subject such as “poll both timers, not just timer 2.”

✅ 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 PR diff against origin/main modifies CHANGELOG.md with an E3.06 entry and updates the AccuracySNES ROM images.
Docs-As-Spec	✅ Passed	The only crates/ change updates two test-harness report strings. It changes diagnostic labels only; E3.06 behavior changes are confined to tests/roms, not a RustySNES crate.
Accuracysnes Bookkeeping	✅ Passed	Against origin/main, only existing E3.06 was modified; no test or scene was added or removed. E3.06 remains in dossier.rs::MAP, and all applicable generated artifacts in the diff are present.
No Panic On Untrusted Input	✅ Passed	The diff adds no new .unwrap(), .expect(), or panic!() calls. E3.06 only constructs a local SPC program and reads timer counters through generated assembly.
Safety Comment On New Unsafe	✅ Passed	The HEAD^..HEAD diff adds no unsafe blocks or unsafe fn; both changed Rust files contain no unsafe constructs.

---

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

[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/apu.rs`:
- Around line 8347-8358: The phase explanation is overstated: accumulation does
not remove Timer 0 phase. In tests/roms/AccuracySNES/gen/src/tests/apu.rs lines
8347-8358, update the comment to state that accumulation prevents four-bit
output loss while bounding phase quantization, with the remaining finite-window
error absorbed by the ±10 ratio tolerance; make the corresponding wording change
in CHANGELOG.md lines 28-34 so the documentation matches the emitted program’s
validated behavior.

🪄 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: 01c2ffda-583c-4250-a354-02178c2d4c62

📥 Commits

Reviewing files that changed from the base of the PR and between b167fe9 and 9cd07fe.

⛔ Files ignored due to path filters (4)

• tests/roms/AccuracySNES/ERROR_CODES.md is excluded by !tests/roms/AccuracySNES/ERROR_CODES.md 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 (3)

• CHANGELOG.md
• crates/rustysnes-test-harness/tests/accuracysnes.rs
• tests/roms/AccuracySNES/gen/src/tests/apu.rs

📜 Review details

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

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

🧰 Additional context used

📓 Path-based instructions (12)

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/tests/apu.rs

**/*.{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:

• crates/rustysnes-test-harness/tests/accuracysnes.rs
• tests/roms/AccuracySNES/gen/src/tests/apu.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:

• crates/rustysnes-test-harness/tests/accuracysnes.rs
• tests/roms/AccuracySNES/gen/src/tests/apu.rs
• CHANGELOG.md

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/tests/apu.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/tests/apu.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/tests/apu.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/tests/apu.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

**/*.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:

• CHANGELOG.md

🧠 Learnings (3)

📚 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/tests/apu.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/apu.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/apu.rs

🔇 Additional comments (2)

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

8335-8335: LGTM!

Also applies to: 8359-8378, 8391-8412, 8434-8442

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

629-635: LGTM!

[Copilot]

Pull request overview

Updates the AccuracySNES APU timer-rate row E3.06 so it no longer depends on a single end-of-interval T0OUT sample (which can be phase-sensitive across battery runs), by accumulating both timer 0 and timer 2 counts inside the same poll loop.

Changes:

• Accumulate both T0OUT and T2OUT in the poll loop (and extend the loop to 32 passes) so the ratio check is phase-robust.
• Widen the allowed ratio error band to match the instrument’s quantisation bounds, and update the emitted failure text accordingly.
• Regenerate/update emitted artifacts (ERROR_CODES.md, asm/tests_group_a.s), plus adjust harness reporting text and the changelog entry.

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/roms/AccuracySNES/gen/src/tests/apu.rs	Implements the E3.06 polling/accumulation change and updates the assertions/messages.
tests/roms/AccuracySNES/ERROR_CODES.md	Updates the generated error-code message text for the new ±10 band wording.
tests/roms/AccuracySNES/asm/tests_group_a.s	Regenerated assembly output reflecting the new APU program and updated checks/messages.
crates/rustysnes-test-harness/tests/accuracysnes.rs	Updates the Group E batch measurement descriptions for slots 266/267.
CHANGELOG.md	Documents the behavioral/robustness correction for E3.06 in Unreleased “Fixed”.

[github-actions]

Antigravity review (Gemini via Ultra)

This PR updates the E3.06 AccuracySNES test ROM generator, assembly, and compiled binaries to accumulate both timer 0 and timer 2 counts in a 32-pass poll loop, widening the pass tolerance band from ±6 to ±10 to prevent test flakiness caused by un-reset stage-0 clock divider phases across test runs.

Blocking issues

None found.

Suggestions

• crates/rustysnes-test-harness/tests/accuracysnes.rs:631 vs tests/roms/AccuracySNES/gen/src/tests/apu.rs:8404: The description for slot 266 in GROUP_E_BATCH_SLOTS ("E3.06 timer 0 ticks, accumulated across polls (expect ~13)") does not match the string recorded in the generator ("E3.06 timer 0 ticks, ACCUMULATED across polls (no end-of-interval phase)"). Keep slot names consistent across the test harness and the test generator.

Nitpicks

• tests/roms/AccuracySNES/gen/src/tests/apu.rs:8448: a.assert_a16_range(0, 20, ...) generates cmp #$0000 followed by bcs. Since A is an unsigned 16-bit value masked with $00FF, it is guaranteed to be >= 0, rendering the lower-bound comparison and jump redundant in the emitted 65816 assembly.

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

[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/apu.rs`:
- Around line 8451-8454: Correct the diagnostic text in the timer 2 versus timer
0 assertion to state that an 8 kHz core produces approximately one-eighth of the
expected count, replacing the incorrect “seventh” wording. Preserve the
surrounding explanation and slot references.

🪄 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: 1ce98a5f-ed28-412f-88ba-cd3da8563b43

📥 Commits

Reviewing files that changed from the base of the PR and between 9cd07fe and 97e08da.

⛔ Files ignored due to path filters (4)

• tests/roms/AccuracySNES/ERROR_CODES.md is excluded by !tests/roms/AccuracySNES/ERROR_CODES.md 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 (3)

• CHANGELOG.md
• crates/rustysnes-test-harness/tests/accuracysnes.rs
• tests/roms/AccuracySNES/gen/src/tests/apu.rs

📜 Review details

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

• GitHub Check: accuracysnes
• GitHub Check: lint
• GitHub Check: test-light
• GitHub Check: build demo + docs
• GitHub Check: review

🧰 Additional context used

📓 Path-based instructions (12)

**/*.{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:

• CHANGELOG.md
• crates/rustysnes-test-harness/tests/accuracysnes.rs
• tests/roms/AccuracySNES/gen/src/tests/apu.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

**/*

📄 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:

• CHANGELOG.md
• crates/rustysnes-test-harness/tests/accuracysnes.rs
• tests/roms/AccuracySNES/gen/src/tests/apu.rs

**/*.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:

• 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/tests/apu.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/tests/apu.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/tests/apu.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/tests/apu.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/tests/apu.rs

🧠 Learnings (3)

📚 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/tests/apu.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/apu.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/apu.rs

🔇 Additional comments (7)

tests/roms/AccuracySNES/gen/src/tests/apu.rs (5)

8332-8340: Describe bounded phase quantization, not phase removal.

crates/rustysnes-apu/src/lib.rs clears stage3 on a timer read. It does not reset stage0. The initial phase can still change a finite accumulated count by one tick. Polling both timers prevents TnOUT wrap loss, but it does not make the measurement phase-independent.

Replace “removes the phase,” “makes the row reproducible,” and “no end-of-interval phase” with wording that states the remaining phase quantization is bounded by the ±10 tolerance.

Proposed wording

-    // Accumulating both removes the phase from both sides.
+    // Accumulating both prevents four-bit output loss; residual phase
+    // quantization is bounded by the ratio tolerance.

This is the same unresolved finding from the previous review. As per path instructions, AccuracySNES test prose must describe only behavior validated by the emitted program.

Also applies to: 8380-8393, 8403-8411

Source: Path instructions

---

8309-8320: LGTM!

---

8341-8370: LGTM!

Also applies to: 8394-8400

---

8414-8424: LGTM!

---

8446-8450: LGTM!

CHANGELOG.md (1)

28-34: State bounded phase quantization in the changelog.

Accumulation removes four-bit TnOUT output loss, but stage0 remains phase-bearing. A finite measurement can still vary by one tick. Replace “neither side carries a phase” with wording that says accumulation bounds phase quantization and that the ±10 band absorbs the remaining error.

Proposed wording

-  Both counters are now accumulated in the same poll loop, over 32 passes rather than 24, so neither
-  side carries a phase.
+  Both counters are now accumulated in the same poll loop, over 32 passes rather than 24, so four-bit
+  output loss is avoided and the remaining phase quantization is bounded by the ±10 band.

This is the same unresolved finding from the previous review. As per path instructions, changelog prose must match the implemented behavior.

Source: Path instructions

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

629-635: LGTM!

[coderabbitai]

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

Correct the ratio fraction in the diagnostic.

When both timers run from the 8 kHz stage, T2 is approximately T0. The expected value is 8 * T0, so that result is about one-eighth of the expected count, not a seventh. Correct the diagnostic.

Proposed wording

-         a seventh of the expected count, which is some ninety ticks out.
+         one-eighth of the expected count, which is some ninety ticks out.

As per path instructions, AccuracySNES test prose must match the behavior that the emitted program validates.

📝 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

	"timer 2 did not count within ten ticks of eight times timer 0 over the same interval, so \
	it is not running from the 64 kHz stage — a core running every timer at 8 kHz lands near \
	a seventh of the expected count. The two raw counts are in slots 266 and 267",
	a seventh of the expected count, which is some ninety ticks out. The two raw counts are \
	in slots 266 and 267",
	"timer 2 did not count within ten ticks of eight times timer 0 over the same interval, so \
	it is not running from the 64 kHz stage — a core running every timer at 8 kHz lands near \
	one-eighth of the expected count, which is some ninety ticks out. The two raw counts are \
	in slots 266 and 267",

🤖 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/apu.rs` around lines 8451 - 8454,
Correct the diagnostic text in the timer 2 versus timer 0 assertion to state
that an 8 kHz core produces approximately one-eighth of the expected count,
replacing the incorrect “seventh” wording. Preserve the surrounding explanation
and slot references.

Source: Path instructions