Skip to content

fix(accuracysnes): the oracle's "14 of 335" does not reproduce - #296

Merged
doublegate merged 10 commits into
mainfrom
fix/mesen-oracle-never-starts
Aug 1, 2026
Merged

fix(accuracysnes): the oracle's "14 of 335" does not reproduce#296
doublegate merged 10 commits into
mainfrom
fix/mesen-oracle-never-starts

Conversation

@doublegate

@doublegate doublegate commented Aug 1, 2026

Copy link
Copy Markdown
Owner

The battery oracle has blocked v1.29.0's scene work for several sessions, and two cart-side fixes
aimed at it moved nothing. This re-measures it with an independent instrument, and the recorded
diagnosis does not survive.

Every prior conclusion rested on mesen_crossval.lua's own read, with nothing checking that read.
scripts/accuracysnes/mesen_wram_probe.lua is a deliberately minimal second one: run N frames with
the input contract held, dump RESULTS, stop.

Three findings, two of which overturn what was written down

1. MesenCE headless is not broken. It renders the entire undisbeliever corpus
(perdot_crossval.sh, 0 skipped captures) and runs the AccuracySNES cart to exit 0. Treating
"the Mesen oracle" as one broken thing was wrong — rendering works, only the battery read does not.

Consequence: the scene work is not blocked by whatever blocks the battery. Those were being
tracked as one blocker and they are two.

2. The render channel cannot observe the battery at all. MesenCE and RustySNES produce
byte-identical colour histograms for the cart at 60 / 300 / 500 / 800 / 900 frames, with and
without the input contract held, and the picture is static throughout in both. A hang and a clean run
look the same through the framebuffer, so any diagnosis must go through WRAM.

3. The battery writes no verdict at all — not 14 of 335. At RESULTS = $7E:F000 (snesWorkRam
offset $F000, the address runtime.inc defines and mesen_crossval.lua already uses):

  • the magic reads as bytes that differ between runs — uninitialised WRAM, never written;
  • R_STATUS[0..39] are all zero, at 600 frames and again at 1500.

"Completes 14 of 335 and stops at A3.03" does not reproduce. That explains why the two cart-side
A3.03 fixes moved nothing — they were chasing a number that is not there. (Those changes were
independently worthwhile and are already merged; only the motivation was wrong.)

What the open question actually is

Not "why does it stop at A3.03" but "why does the battery never start". Different search.

Prime suspect, explicitly not yet confirmed: the input contract holds B + Start + X + R from
power-on, so a menu that starts the battery on a press edge would never see one — consistent with
the picture never changing. The next step is small and written into the plan: release the contract
buttons for a few frames, re-press, re-probe. If that starts the battery, every runner in
crossval.sh needs an edge rather than a constant hold.

Scope

Diagnosis and tooling only. No emulator or cart behaviour changes, so nothing here can move a golden.

🤖 Generated with Claude Code

This change claims that MesenCE can run the AccuracySNES battery when the runner supplies controller 1 input. It claims that the prior “14 of 335” result was an invalid over-read of an unwritten status region, not a cartridge failure. It also claims that the scene runner now validates 53 matching scenes.

The claim is false if controller 1 input is overwritten, Start is not supplied, or the WRAM result block remains unwritten. The remaining Mesen2 failure is dossier assertion A2.10, catalogue index 11, with failure code 1.

The change updates the dossier findings for the MesenCE oracle and the AccuracySNES input contract. It changes the runner diagnosis from a battery-stop or cartridge-execution problem to MesenCE input plumbing. It does not add or remove dossier assertions. The denominator remains 443 assertions. The separate battery catalogue remains 335 entries.

Every prior conclusion about the battery oracle rested on
mesen_crossval.lua's own read, with nothing independent checking that
read. Added a deliberately minimal second instrument and re-measured.

Three findings, two of which overturn what was recorded:

MesenCE headless is not broken. It renders the whole undisbeliever
corpus with 0 skipped captures and runs the AccuracySNES cart to exit
0. Calling "the Mesen oracle" one broken thing was wrong -- rendering
works, only the battery read does not, so the scene work is not blocked
by whatever blocks the battery.

The render channel cannot observe the battery at all: MesenCE and
RustySNES produce byte-identical colour histograms for the cart at
60/300/500/800/900 frames, with and without the input contract, and the
picture is static throughout in both. A hang and a clean run look the
same.

The battery writes no verdict at all -- not 14. At RESULTS = $7E:F000
the magic reads as bytes that differ between runs (uninitialised WRAM,
never written) and R_STATUS[0..39] are all zero at 600 and at 1500
frames. That explains why two cart-side A3.03 fixes moved nothing: they
were chasing a number that is not there.

The question is now why the battery never STARTS. Prime suspect,
recorded and not yet confirmed: the input contract holds B+Start+X+R
from power-on, so a menu starting on a press edge never sees one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 1, 2026 01:30
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

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

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9f4e3613-82e1-4ee8-847d-192d76850d6d

📥 Commits

Reviewing files that changed from the base of the PR and between b40a261 and 0c2bb2f.

📒 Files selected for processing (1)
  • docs/accuracysnes-plan.md

Walkthrough

The PR updates MesenCE controller input handling, adds Lua probes for input and WRAM diagnostics, and records revised AccuracySNES and emulator-validation findings.

Changes

AccuracySNES MesenCE input diagnostics

Layer / File(s) Summary
MesenCE input handling
scripts/accuracysnes/mesen_crossval.lua, scripts/accuracysnes/mesen_scenes.lua
The cross-validation script sends input only to controller 1. The scene script supplies Start and held buttons through the input-poll callback.
Diagnostic probe scripts
scripts/accuracysnes/mesen_pad_probe.lua, scripts/accuracysnes/mesen_pad_held_probe.lua, scripts/accuracysnes/mesen_edge_probe.lua, scripts/accuracysnes/mesen_wram_probe.lua, scripts/accuracysnes/mesen_status_index_probe.lua
The new probes vary input timing, sample controller registers and WRAM, write configurable result files, and stop emulation after the target frame.
Validation findings and investigation record
docs/accuracysnes-plan.md, CHANGELOG.md
The documentation records the corrected MesenCE battery diagnosis, rejected input hypotheses, remaining runner-input investigation, and the Mesen2 A2.10 disagreement.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 9 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title describes the main change and uses a valid Conventional Commits prefix, but its subject is not written in the required imperative mood. Rewrite the subject in imperative mood, such as "fix(accuracysnes): correct the unreproducible oracle result".
✅ 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 PR diff from its base includes CHANGELOG.md with two AccuracySNES entries documenting the changes.
Docs-As-Spec ✅ Passed Against merge-base 10841d8, the PR changes only CHANGELOG.md, docs/accuracysnes-plan.md, and scripts/accuracysnes; no crates/rustysnes-/ path changes exist.
Accuracysnes Bookkeeping ✅ Passed The complete diff from origin/main changes no files under tests/roms/AccuracySNES/gen/src/, so the test/scene bookkeeping checks are not applicable.
No Panic On Untrusted Input ✅ Passed The PR changes only Markdown and Lua files; the complete diff adds no .unwrap(), .expect(), or panic!() calls, and no Rust file changes exist.
Safety Comment On New Unsafe ✅ Passed The PR changes only Lua and Markdown files; the complete diff adds no unsafe { ... }, unsafe fn, or Rust files requiring a // SAFETY: comment.

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

doublegate and others added 2 commits July 31, 2026 21:31
Applying the input contract from frame 0, 30 or 120 -- the latter two
manufacturing a rising edge -- all give the same result at 1200 frames:
magic never reads ACSN, R_DONE never reads $A5, R_STATUS holds exactly
24 non-zero bytes. The battery is not waiting for a press edge.

Committing the probe so the next attempt does not re-run a settled
negative.

The live lead is the 24, not the zeros. It is constant across runs
while the magic bytes vary, so it is not uninitialised WRAM -- that
would vary too -- and indices 0-39 read zero, so those bytes sit at
index >= 40. Twenty-four deterministic bytes in an array that should be
untouched is the next thing to explain.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The CHANGELOG carried it but the plan section still named the edge as
an unconfirmed suspect, which would have sent the next session to
re-run a settled negative.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR revisits a blocking AccuracySNES “Mesen oracle” diagnosis by adding an independent MesenCE WRAM probe and updating project documentation/changelog to reflect that the previously recorded “14 of 335” result does not reproduce. It’s focused on diagnosis/tooling rather than changing emulator or cartridge behavior.

Changes:

  • Add mesen_wram_probe.lua, a minimal MesenCE --testRunner script to dump the AccuracySNES RESULTS WRAM block after N frames.
  • Document the new measurement and revised conclusions in docs/accuracysnes-plan.md.
  • Record the updated diagnosis/tooling in CHANGELOG.md.

Reviewed changes

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

File Description
scripts/accuracysnes/mesen_wram_probe.lua New minimal MesenCE probe script to dump the results/status region from WRAM for independent verification.
docs/accuracysnes-plan.md Adds a plan section documenting the re-measurement and updated hypothesis (“battery never starts”).
CHANGELOG.md Adds a changelog entry summarizing the new probe and overturned diagnosis.

Comment on lines +18 to +20
local TARGET = tonumber(os.getenv("MCE_FRAMES") or "900")
local RES = os.getenv("MCE_RESULT") or "/tmp/wram.txt"
local frames = 0
local function onEndFrame()
frames = frames + 1
if frames < TARGET then return end
local f = io.open(RES, "w")
Comment on lines +37 to +45
local nonzero, first_zero = 0, -1
local bytes = {}
for i = 0, 359 do
local b = emu.read(0xF020 + i, emu.memType.snesWorkRam)
if b ~= 0 then nonzero = nonzero + 1 elseif first_zero < 0 then first_zero = i end
if i < 40 then bytes[#bytes+1] = string.format("%02x", b) end
end
f:write(string.format("status nonzero=%d first_zero_index=%d\n", nonzero, first_zero))
f:write("first40=" .. table.concat(bytes, " ") .. "\n")
doublegate and others added 4 commits July 31, 2026 21:34
…menu

The 24 non-zero bytes I called a lead were my own over-read -- they sit
at indices 336-359, past the end of a 335-entry array. Retracted.

The real signal is the reverse of what I reported. R_STATUS[0..334] is
ZERO while the surrounding WRAM is RANDOM, and MesenCE powers on with
randomised WRAM -- so an all-zero status array is not the power-on
state. Something zeroed it, and that something is runtime.s's own clear
loop. The cart's init runs and reaches the pre-battery menu.

So the precise statement is not "the battery never starts" but "the
cart never leaves its menu", whose exit condition the source names
immediately below the clear loop: wait for Start.

Also ruled out this round: declaring --snes.port1.type=SnesController
explicitly alongside port 2.

Next suspect, which fits every observation: the menu reads the pad
through the automatic joypad read ($4218). If --testRunner does not
drive auto-read the way emu.setInput implies, Start is held host-side
and reads zero cart-side -- invisible in the framebuffer, and
consistent with init running while the menu never exits.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reading a non-zero COUNT without reading the indices is what let an
over-read masquerade as a lead for a whole round. This probe prints the
indices, and its header records what that showed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Auto-joypad: at frame 300 NMITIMEN reads $42 -- bit 0 clear, auto-read
disabled -- so $4218-$421F reading 0000 is correct rather than a fault.
The cart says so itself at runtime.s:9, "read MANUALLY through $4016,
not via auto-joypad", and line 570 stz NMITIMEN. My own previous
suspect, killed by reading the register instead of theorising.

Software edge: the menu computes V_PAD_NEW = HELD & ~LAST and seeds
both on entry, so a held button yields NEW = 0 forever -- which looked
like the answer. Pressing Start at frame 300 or 600 of a 2000-frame run
changes nothing either.

What remains is harness-side: emu.setInput from an inputPolled callback
does not appear to reach the cart's manual $4016 strobe read under
--testRunner. Every host-side way of supplying the press has been tried
and the cart observes none, while its init demonstrably runs. The
in-repo harness and the snes9x libretro driver both work, so this is
MesenCE-runner input plumbing, not contract design.

Next step recorded: a minimal ROM that only reads $4016 and stores it,
testing the channel in isolation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The battery now runs to completion under MesenCE: magic ACSN, R_DONE
$A5, 335/335 status bytes written, and crossval.sh reports "Mesen2: 1
failing test(s)" where it previously timed out.

Root cause: in this MesenCE build's --testRunner, emu.setInput's port
argument does not select a controller. Indices 0, 1 and 2 all land on
controller 1, verified through the cart's own V_PAD_HELD. The script
called setInput twice, so the second call -- intended for port 2 --
overwrote port 1 and the cart saw PAD2_CONTRACT ($60A0) on controller
1. $60A0 has no Start; the pre-battery menu waits for Start. So the
cart booted, ran init, cleared R_STATUS, reached the menu and sat there
forever behind a static picture.

Every recorded symptom follows from that line. The "completes 14 of 335
and stops at A3.03" figure was an all-zero status array misread, and
two cart-side A3.03 fixes were aimed at a layer that was never at
fault.

Stated rather than hidden: port 2 cannot be driven from Lua in this
build, so PAD2_CONTRACT-dependent rows are not cross-validated by this
runner. The in-repo harness and the snes9x libretro driver drive both
ports, so those rows stay covered elsewhere.

Still open, and now a separate problem: Mesen2 reports no rendered
scenes -- the run does not reach the scene loop.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@doublegate

Copy link
Copy Markdown
Owner Author

The oracle is fixed, and it is already arbitrating

First run with the fix — the battery completes under MesenCE and immediately produces a verdict this
project has never had:

magic='ACSN'  done=a5  count=335  passed=300  failed=1  status_written=335
crossval.sh → "Mesen2: 1 failing test(s)"     (previously: timeout, no verdict at all)

The single Mesen2 disagreement is catalogue index 11, failure code 1. That is a real,
newly-visible accuracy signal and it wants its own investigation — deliberately not folded into
this PR, which stays diagnosis + tooling so it can land without touching behaviour.

Worth applying this project's own heuristic to it before anyone chases it: three emulators failing
identically usually means a broken test, RustySNES failing alone usually means a real bug. This is
RustySNES-alone under Mesen2 while snes9x reports only its 14 known divergences — so check which
before investigating.

Also still open, and now genuinely separate

Mesen2: no scenes reported at all — the run did not reach the scene loop. The battery half works;
the scene half does not yet. That is a different problem from the one this PR closes, and the scene
work it gates should be scoped against it rather than against "the oracle is broken".

🤖 Generated with Claude Code

"Mesen2: no scenes reported at all -- the run did not reach the scene
loop" read as a scene-loop problem and was an input problem, the same
class as the battery one.

mesen_scenes.lua had no setInput anywhere. With nothing held the cart
never leaves its pre-battery menu -- that menu waits for Start -- so it
never runs the battery and never reaches the scene loop that follows.

One port-1 call added, with the same warning as mesen_crossval.lua
about not adding a port-2 call: in this build the port argument does
not select a controller, so a second call would overwrite this one with
PAD2_CONTRACT, which has no Start.

Both halves of the oracle now arbitrate:

  snes9x: 53 scene(s) match, 0 unblessed, 0 mismatched
  Mesen2: 53 scene(s) match, 0 unblessed, 0 mismatched

That is the ADR 0013 gate the v1.29.0 scene work has been waiting
behind -- a golden can now be blessed from a render both references
agree on.

What remains is a finding rather than an oracle failure: Mesen2 reports
1 failing test, catalogue index 11 (A2.10), while snes9x reports only
its 14 known divergences.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
crossval.sh now reports Mesen2: 1 failing test -- catalogue index 11,
cart A2.10 "PEI does not page-wrap". RustySNES passes it and so does
snes9x (it is not among snes9x's 14 known divergences), so this is
2-vs-1 with Mesen2 as the outlier -- the inverse of the shape the
standing heuristic warns about.

Deliberately NOT recorded as a known-good Mesen2 divergence yet. That
needs a third opinion from ares or bsnes, because a harness bug
upstream of an implementation produces this exact signature and one
already did in this project once. Until then crossval.sh's
DISAGREEMENT verdict is correct and should stay.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@CHANGELOG.md`:
- Around line 38-41: Update the fenced output block in CHANGELOG.md to include a
language identifier, using text or console, while preserving its existing
contents.

In `@scripts/accuracysnes/mesen_pad_probe.lua`:
- Around line 4-8: The probe scripts currently overwrite controller 1 with a
second input call targeting port 2; update
scripts/accuracysnes/mesen_pad_probe.lua lines 4-8,
scripts/accuracysnes/mesen_pad_held_probe.lua lines 5-10,
scripts/accuracysnes/mesen_edge_probe.lua lines 18-23,
scripts/accuracysnes/mesen_wram_probe.lua lines 21-25, and
scripts/accuracysnes/mesen_status_index_probe.lua lines 16-20 to use only the
controller-1 input contract, then re-measure edge cases where requested. In
docs/accuracysnes-plan.md lines 652-722, replace the superseded manual $4016
diagnosis with the confirmed controller-1 overwrite root cause and final battery
result. In CHANGELOG.md lines 52-99, remove or rewrite the contradictory interim
diagnosis; no direct change is needed to lines 14-50.
🪄 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: 45f40dc0-87c0-44d6-962a-6e0ea92b9baf

📥 Commits

Reviewing files that changed from the base of the PR and between 10841d8 and b40a261.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • docs/accuracysnes-plan.md
  • scripts/accuracysnes/mesen_crossval.lua
  • scripts/accuracysnes/mesen_edge_probe.lua
  • scripts/accuracysnes/mesen_pad_held_probe.lua
  • scripts/accuracysnes/mesen_pad_probe.lua
  • scripts/accuracysnes/mesen_scenes.lua
  • scripts/accuracysnes/mesen_status_index_probe.lua
  • scripts/accuracysnes/mesen_wram_probe.lua
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: lint
  • GitHub Check: accuracysnes
  • GitHub Check: test-light
  • GitHub Check: build demo + docs
🧰 Additional context used
📓 Path-based instructions (8)
**/*

📄 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/mesen_scenes.lua
  • scripts/accuracysnes/mesen_pad_held_probe.lua
  • scripts/accuracysnes/mesen_wram_probe.lua
  • scripts/accuracysnes/mesen_crossval.lua
  • scripts/accuracysnes/mesen_pad_probe.lua
  • scripts/accuracysnes/mesen_status_index_probe.lua
  • docs/accuracysnes-plan.md
  • CHANGELOG.md
  • scripts/accuracysnes/mesen_edge_probe.lua
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/mesen_scenes.lua
  • scripts/accuracysnes/mesen_pad_held_probe.lua
  • scripts/accuracysnes/mesen_wram_probe.lua
  • scripts/accuracysnes/mesen_crossval.lua
  • scripts/accuracysnes/mesen_pad_probe.lua
  • scripts/accuracysnes/mesen_status_index_probe.lua
  • scripts/accuracysnes/mesen_edge_probe.lua
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/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/accuracysnes-plan.md
  • 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/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/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/accuracysnes-plan.md
  • CHANGELOG.md
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 (2)
📚 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
🪛 markdownlint-cli2 (0.23.1)
CHANGELOG.md

[warning] 38-38: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (2)
scripts/accuracysnes/mesen_crossval.lua (1)

92-112: LGTM!

scripts/accuracysnes/mesen_scenes.lua (1)

157-173: LGTM!

Comment thread CHANGELOG.md
Comment on lines +38 to +41
```
snes9x: 53 scene(s) match, 0 unblessed, 0 mismatched
Mesen2: 53 scene(s) match, 0 unblessed, 0 mismatched
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Add a language identifier to the fenced output block.

markdownlint-cli2 reports MD040 for this fence. Use text or console so the documentation lint gate passes.

🧰 Tools
🪛 markdownlint-cli2 (0.23.1)

[warning] 38-38: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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 `@CHANGELOG.md` around lines 38 - 41, Update the fenced output block in
CHANGELOG.md to include a language identifier, using text or console, while
preserving its existing contents.

Source: Linters/SAST tools

Comment on lines +4 to +8
local function onInput()
emu.setInput({ b=true, start=true, x=true, r=true, y=false, select=false, a=false, l=false,
up=false, down=false, left=false, right=false }, 0)
emu.setInput({ y=true, select=true, a=true, l=true, b=false, start=false, x=false, r=false,
up=false, down=false, left=false, right=false }, 1)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Remove the stale duplicate-input diagnosis and make every probe use one controller-1 input call.

In this runner, the second emu.setInput(..., 1) call overwrites controller 1 with start=false. Any probe that runs these lines cannot leave the menu. Its output cannot refute press-edge, auto-joypad, or manual $4016 input hypotheses.

  • scripts/accuracysnes/mesen_pad_probe.lua#L4-L8: remove the port-2 call or make it an explicit routing experiment that does not support normal input conclusions.
  • scripts/accuracysnes/mesen_pad_held_probe.lua#L5-L10: retain only the controller-1 contract.
  • scripts/accuracysnes/mesen_edge_probe.lua#L18-L23: retain only the controller-1 contract, then re-measure the press-edge cases.
  • scripts/accuracysnes/mesen_wram_probe.lua#L21-L25: retain only the controller-1 contract before collecting WRAM evidence.
  • scripts/accuracysnes/mesen_status_index_probe.lua#L16-L20: retain only the controller-1 contract before collecting status evidence.
  • docs/accuracysnes-plan.md#L652-L722: replace the superseded “manual $4016 input does not work” conclusion with the confirmed controller-1 overwrite root cause and final battery result.
  • CHANGELOG.md#L52-L99: remove or rewrite the contradictory interim diagnosis. Lines 14-50 already state the resolved behavior.
📍 Affects 7 files
  • scripts/accuracysnes/mesen_pad_probe.lua#L4-L8 (this comment)
  • scripts/accuracysnes/mesen_pad_held_probe.lua#L5-L10
  • scripts/accuracysnes/mesen_edge_probe.lua#L18-L23
  • scripts/accuracysnes/mesen_wram_probe.lua#L21-L25
  • scripts/accuracysnes/mesen_status_index_probe.lua#L16-L20
  • docs/accuracysnes-plan.md#L652-L722
  • CHANGELOG.md#L52-L99
🤖 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 `@scripts/accuracysnes/mesen_pad_probe.lua` around lines 4 - 8, The probe
scripts currently overwrite controller 1 with a second input call targeting port
2; update scripts/accuracysnes/mesen_pad_probe.lua lines 4-8,
scripts/accuracysnes/mesen_pad_held_probe.lua lines 5-10,
scripts/accuracysnes/mesen_edge_probe.lua lines 18-23,
scripts/accuracysnes/mesen_wram_probe.lua lines 21-25, and
scripts/accuracysnes/mesen_status_index_probe.lua lines 16-20 to use only the
controller-1 input contract, then re-measure edge cases where requested. In
docs/accuracysnes-plan.md lines 652-722, replace the superseded manual $4016
diagnosis with the confirmed controller-1 overwrite root cause and final battery
result. In CHANGELOG.md lines 52-99, remove or rewrite the contradictory interim
diagnosis; no direct change is needed to lines 14-50.

Two material corrections, made now that the oracle arbitrates.

"34 rows need new SPC700 emitters" overstates it. apu.rs already has
dsp_write and dsp_read_to, plus apu_upload/release_to_ipl, so any row
whose observable is a DSP register needs no new opcode encoding at all.
The emitter gap is real only for timer/GAIN/echo-coefficient opcodes.

Pick DSP-observable rows first, because those are the ones the working
oracle can adjudicate. Batch chosen: E8.01, E9.02, E5.06 -- the first
two errata-flagged, so the expected behaviour is stated rather than
inferred from an implementation.

E1.11 is explicitly NOT first: inspected and set aside as likely
vacuous, since TSET1's second read is a dummy and on a timer-output
target the first read has already cleared the counter, with no chance
of an increment between two reads six cycles apart. Design the
observable before writing that row.

Settle A2.10 before authoring any of them -- authoring a batch while
one reference disagreement is unexplained risks misattributing a new
failure to the wrong cause.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Antigravity review (Gemini via Ultra)

Fixes the AccuracySNES Mesen2 test harness by removing a port 2 emu.setInput call that was overwriting controller 1 input, while adding diagnostic Lua probes and updating test documentation.

Blocking issues

  • Committed probe scripts reintroduce the fixed bug: scripts/accuracysnes/mesen_edge_probe.lua (L18–23), scripts/accuracysnes/mesen_pad_held_probe.lua (L5–10), scripts/accuracysnes/mesen_status_index_probe.lua (L16–21), and scripts/accuracysnes/mesen_wram_probe.lua (L21–26) all call emu.setInput(..., 0) followed immediately by emu.setInput(..., 1). As documented in mesen_crossval.lua, index 1 overwrites controller 1 with $60A0 (no Start button). These newly added probes still contain the exact bug being fixed and will produce invalid test output.
  • Duplicate event callback registration in mesen_scenes.lua: scripts/accuracysnes/mesen_scenes.lua (L174) registers emu.addEventCallback(onFrame, emu.eventType.endFrame) at the bottom of the script. onFrame was already registered earlier in the file, causing onFrame to run twice per frame and corrupting frame accounting.

Suggestions

  • CHANGELOG.md (L14–99): Remove the ~80 lines of lab notes, debugging narratives, and retracted hypotheses ("RETRACTED: the '24 non-zero bytes' was my own over-read"). The changelog should state user/developer-facing changes concisely; internal debugging logs belong in design docs or issue threads.
  • scripts/accuracysnes/mesen_edge_probe.lua (L7–12): Remove outdated header comments referencing the retracted "24 non-zero bytes" reading to avoid conflicting with mesen_status_index_probe.lua.

Nitpicks

  • Probe scripts hardcode temporary file paths (/tmp/edge.txt, /tmp/held.txt, etc.) without cleanup or environment defaults fallback.
  • docs/accuracysnes-plan.md reads like an unedited stream of consciousness rather than curated roadmap documentation.

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

@doublegate
doublegate merged commit e41c9a5 into main Aug 1, 2026
16 checks passed
@doublegate
doublegate deleted the fix/mesen-oracle-never-starts branch August 1, 2026 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants