fix(accuracysnes): E3.06 polls and accumulates; retract F1.10's "1-vs-3"

[doublegate]

Completes task #88. Two things, and the second is a retraction of #306.

E3.06: the 4-bit ceiling is gone

The row read TnOUT once at the end of the interval. TnOUT is a four-bit read-and-clear counter, so the useful range was 8..15 with the wrap one tick above — not a band that can be widened, but the instrument'''s ceiling.

It now polls timer 2 every ~32 SPC cycles (one timer-2 period), so every read returns 0, 1 or 2 and the running sum cannot lose a tick however long the interval gets.

It also asserts the ratio rather than two absolute counts — |T2 − 8·T0| ≤ 6, computed on the 65816. Absolute counts pin the poll loop'''s own cycle cost, which differs between cores for reasons that have nothing to do with the 64 kHz stage. That is precisely what made the old row fail on ares while ares''' own Timer<128>/Timer<128>/Timer<16> declarations were an exactly correct 8:1.

RustySNES and ares now report the identical 6 and 46. ARES_KNOWN_FAILURES drops 5 → 4.

A ceiling in the instrument reads exactly like a defect in the thing measured.

Retraction: F1.10 is not "1-vs-3", and its Mesen2 verdict is phase-fragile

#306 published that reading from a measurement taken at the time. Then rewriting E3.06 — an unrelated APU row — made Mesen2 pass F1.10. Three identical runs before, three identical runs after.

The rewrite changed one uploaded program'''s length, which moved the cart'''s execution phase, which moved when F1.10 samples $4212 relative to the vblank edge. So Mesen2'''s verdict on that row encodes where the cart happens to be, not only what Mesen2 models. snes9x and ares fail it stably; Mesen2 flips.

MESEN2_KNOWN_FAILURES drops 2 → 1 (F1.03 only), and the comment says not to restore an F1.10 entry — either pin the row'''s sampling point, or accept that Mesen2'''s verdict on it carries no information.

This is the same trap for the third time in one session, after E8.01'''s two rejected drafts and the scene field gate: a verdict that encodes an uncontrolled phase looks stable until something unrelated moves.

Verification

• cargo fmt --check, cargo clippy --workspace --exclude rustysnes-android --all-targets -- -D warnings — clean.
• cargo test -p rustysnes-test-harness --test accuracysnes --test accuracysnes_scenes --features test-roms — 58 passed, 0 failed.
• crossval.sh with all three: snes9x: OK (14), Mesen2: OK (1), ares: OK (4), 54 scenes match on both, 3 reference(s) agree with the cart.
• Mesen2 failing set measured three times before and three times after the E3.06 change.

🤖 Generated with Claude Code

E3.06 now claims that timer 2 maintains an approximately 8:1 read-count ratio with timer 0 on the 65816. The claim is false if timer-2 polling loses counts or if |T2 − 8·T0| > 6.

E3.06 was altered. F1.10 was retracted from the Mesen2 failure interpretation because its result depends on the cart execution phase. The AccuracySNES coverage denominator did not move. The known-failure counts changed from 5 to 4 for ares and from 2 to 1 for Mesen2.

[coderabbitai]

Walkthrough

The PR rewrites E3.06 to accumulate timer-2 counts across repeated polls and validate an 8:1 ratio. It updates ares and Mesen2 known-failure counts and documents F1.10 as phase-sensitive.

Changes

AccuracySNES validation updates

Layer / File(s)	Summary
E3.06 timer measurement and ratio validation tests/roms/AccuracySNES/gen/src/tests/apu.rs	e3_06 polls timer 2 for 24 passes, accumulates its read-and-clear counts, and compares the result with eight times timer 0 within six ticks.
Cross-validation and changelog records scripts/accuracysnes/crossval.sh, CHANGELOG.md	The ares known-failure count decreases to 4 after E3.06 passes. The Mesen2 count decreases to 1, and F1.10 is documented as phase-sensitive.

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

Possibly related PRs

• doublegate/RustySNES#186: Directly revises the E3.06 timer-2 test and related records.
• doublegate/RustySNES#307: Related to the E3.06 timer-count measurement and assertion.
• doublegate/RustySNES#306: Related to F1.10 failure attribution and documentation.

🚥 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 describes the changes, but its subject is not consistently written in the imperative mood.	Rewrite the subject in imperative mood, for example: "fix(accuracysnes): poll and accumulate E3.06; retract F1.10 conclusion"

✅ Passed checks (9 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 diff from base d75a8ed includes a CHANGELOG.md hunk documenting the E3.06 and F1.10 AccuracySNES changes.
Docs-As-Spec	✅ Passed	Against origin/main, no files under crates/ or docs/ changed; the PR changes AccuracySNES test instrumentation and cross-validation accounting, not RustySNES crate behavior.
Accuracysnes Bookkeeping	✅ Passed	Against origin/main, E3.06 exists in both revisions and no test or scene is added or removed; it remains mapped in dossier.rs::MAP, with coverage and plan unchanged.
No Panic On Untrusted Input	✅ Passed	The PR diff adds 0 .unwrap(), .expect(), or panic!() calls. E3.06 only constructs local SPC program data and uses typed builder operations.
Safety Comment On New Unsafe	✅ Passed	The PR patch adds no unsafe tokens or blocks. The only changed Rust file contains no unsafe or SAFETY: markers, so no new safety comment is required.

---

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

[github-actions]

Antigravity review (Gemini via Ultra)

This PR rewrites test E3.06 in AccuracySNES to poll and accumulate SPC700 timer 2 ticks and assert an 8:1 tick ratio against timer 0, while retracting the F1.10 known-failure entry after identifying phase-sensitivity in Mesen2's test results.

Blocking issues

None found.

Suggestions

• apu.rs:L8099: cmp_a_imm(0x18) uses hardcoded 0x18 (24 passes). Add a comment or named constant explaining that 24 iterations at ~32 SPC cycles per loop yield ~768 cycles, ensuring timer 0 ticks ~6 times while staying well below wrap bounds.
• tests_group_a.s:L23000: sta f:$7E01F0 executes in 16-bit accumulator mode (.a16), writing to both $7E01F0 and $7E01F1. Ensure bytes $7E01F0..$7E01F1 are strictly reserved as scratchpad RAM in the test memory layout to prevent clobbering adjacent test state.

Nitpicks

• crossval.sh:L198: Inline date 2026-08-01 adds maintenance churn; reference commit hashes or PR numbers instead.

_{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 8088-8089: The shared timer-2 period description is incorrect:
update the comment near the APU test’s polling logic to state that timer 2 ticks
approximately every 16 cycles and each roughly 32-cycle poll spans about two
periods; also update CHANGELOG.md at lines 17-17 to replace “one timer-2 period”
with “about two timer-2 periods.”

🪄 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: 64724c02-32cf-472f-b33d-431bf1c09212

📥 Commits

Reviewing files that changed from the base of the PR and between d75a8ed and 47d8e96.

⛔ 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
• scripts/accuracysnes/crossval.sh
• tests/roms/AccuracySNES/gen/src/tests/apu.rs

📜 Review details

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

• GitHub Check: accuracysnes
• GitHub Check: test-light
• GitHub Check: lint
• GitHub Check: copilot-pull-request-reviewer

🧰 Additional context used

📓 Path-based instructions (10)

**/*

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

• scripts/accuracysnes/crossval.sh
• tests/roms/AccuracySNES/gen/src/tests/apu.rs
• CHANGELOG.md

scripts/accuracysnes/**

⚙️ CodeRabbit configuration file

scripts/accuracysnes/**: The cross-validation harness: the same AccuracySNES image is run on snes9x (through a
libretro host in C) and on Mesen2 (through its test runner and a Lua script), and their
verdicts are compared with the cart's. Its integrity is the whole argument for the
battery, so flag anything that could make a reference appear to agree — a verdict parsed
loosely, a missing-file path that degrades to success, a scene comparison that skips
rather than fails when the golden is absent. A known reference divergence belongs in
SNES9X_KNOWN_FAILURES with a source citation, never in a widened match.

Files:

• scripts/accuracysnes/crossval.sh

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

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

• tests/roms/AccuracySNES/gen/src/tests/apu.rs
• CHANGELOG.md

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

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

• 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 (3)

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

8075-8086: LGTM!

Also applies to: 8090-8106, 8121-8158

scripts/accuracysnes/crossval.sh (1)

194-218: LGTM!

Also applies to: 244-267

CHANGELOG.md (1)

14-16: LGTM!

Also applies to: 18-44

[coderabbitai]

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

Correct the shared timer-2 period description.

The 24-poll E3.06 measurement records T2=46. Therefore, each approximately 32-cycle poll spans about two timer-2 periods, not one.

• tests/roms/AccuracySNES/gen/src/tests/apu.rs#L8088-L8089: State that timer 2 ticks approximately every 16 cycles and that each poll spans about two periods.
• CHANGELOG.md#L17-L17: Replace “one timer-2 period” with “about two timer-2 periods.”

As per path instructions, “Docs are the spec, not a changelog.”

📍 Affects 2 files

• tests/roms/AccuracySNES/gen/src/tests/apu.rs#L8088-L8089 (this comment)
• CHANGELOG.md#L17-L17

🤖 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 8088 - 8089, The
shared timer-2 period description is incorrect: update the comment near the APU
test’s polling logic to state that timer 2 ticks approximately every 16 cycles
and each roughly 32-cycle poll spans about two periods; also update CHANGELOG.md
at lines 17-17 to replace “one timer-2 period” with “about two timer-2 periods.”

Source: Path instructions

[Copilot]

Pull request overview

Updates the AccuracySNES APU timer test E3.06 to avoid the 4-bit TnOUT ceiling by polling/accumulating timer-2 output and asserting a ratio (|T2 − 8·T0| ≤ 6) instead of absolute counts, and retracts/clarifies prior F1.10 claims by updating cross-validation documentation and counts.

Changes:

• Rewrite E3.06’s measurement approach to poll T2OUT, accumulate ticks, and assert a ratio-based bound.
• Regenerate the emitted ASM and error-code documentation to match the new E3.06 logic and messages.
• Update cross-validation notes/counts and changelog narrative around the F1.10 retraction and ares known-failures.

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/roms/AccuracySNES/gen/src/tests/apu.rs	Reworks E3.06 SPC700 program to poll/accumulate T2OUT and updates the 65816-side assertions/messages.
tests/roms/AccuracySNES/ERROR_CODES.md	Regenerated error text for E3.06 to match the updated assertions/messages.
tests/roms/AccuracySNES/asm/tests_group_a.s	Regenerated emitted ROM ASM reflecting the new E3.06 program length and ratio-based checking.
scripts/accuracysnes/crossval.sh	Updates Mesen2/ares known-failures commentary and constants in the cross-validation gate script.
CHANGELOG.md	Documents the E3.06 rewrite and F1.10 retraction in the Unreleased notes.

Suppressed comments (1)

CHANGELOG.md:48

• This changelog entry still references ARES_KNOWN_FAILURES=5 and lists E3.06 as part of the ares $F1 timer-2 reset bug rationale, but crossval.sh now sets ARES_KNOWN_FAILURES=4 and E3.06 was removed from the ares-known-failures list. Update the changelog text to match the current gate constants.

- **ares is wired into `crossval.sh` as a third reference.** `cross-validation: 3 reference(s) agree
  with the cart`, with `ARES_KNOWN_FAILURES=5` carrying the same per-row rationale the other two
  constants do: `C7.05`/`C7.10`/`F1.10` are rows snes9x already fails (ares corroborating it, not
  standing alone), and `E3.06`/`E8.02` cite the ares `$F1` timer-2 reset bug.