Repository navigation
docs(accuracysnes): adjudicate ares' divergences; two corrections to my own claims - #305
Conversation
…my own claims Three of ares' five failures are rows snes9x ALREADY fails, which the tally alone does not show and which changes what each one means: C7.10 and F1.10 become 2-vs-2 (RustySNES + Mesen2 against snes9x + ares), C7.05 is 2-vs-2 with the two dissenters failing on different codes, and only E8.02 and E3.06 are ares-only. ares is corroborating snes9x more than it is standing alone. Still not wired into crossval.sh -- an ARES_KNOWN_FAILURES constant encoding unadjudicated disagreements would be worse than no third reference. CORRECTION 1, to the ares README and PR #304: F1.10 is NOT a PAD2_CONTRACT row and this host's port detection is not the suspect. f1_require_contract reads $4016 only -- port 1 -- and F1.10 code 2 means "$4212 read busy at the very start of the vblank line", which does not involve controller state. The claim was inherited from crossval.sh's Mesen2 grouping rather than checked. CORRECTION 2, to crossval.sh itself: its Mesen2 known-failure comment names idx279 F1.03 and idx286 F1.10, and in the current catalogue those indices are F1.01 and F1.08 -- an index moves whenever a test is added ahead of it. Keyed on rows now, with the drift noted. That same comment attributes Mesen2's F1.10 failure to the port-2 limitation while the snes9x block a few lines above says Mesen2 PASSES F1.10; both cannot be true, and it is marked as doubted rather than quietly rewritten, because resolving it needs Mesen2's failing set read at DONE and mapped through SOURCE_CATALOG.tsv. F1.10 now deserves a hard look on its own: fullsnes puts the automatic read's start ~dot 32.5-95.5 into the first vblank line, snes9x fails the row, ares fails it, and if the Mesen2 attribution is right then RustySNES passes ALONE on a row it passes only because of a deliberate fix. The heuristic that says RustySNES failing alone means a real bug should say the same about RustySNES passing alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WalkthroughThe PR updates AccuracySNES documentation with detailed ares and snes9x disagreement results, revises the unresolved ChangesAccuracySNES documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
Comment |
Antigravity review (Gemini via Ultra)This PR updates documentation and shell script comments across Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
There was a problem hiding this comment.
Pull request overview
Refines the AccuracySNES cross-validation documentation around ares/snes9x/Mesen2 divergences, correcting earlier misattribution of F1.10 and making the Mesen2 “known failures” commentary resilient to catalogue index drift.
Changes:
- Update
crossval.sh’s Mesen2 known-failure notes to be keyed by row name (not moving catalogue indices) and explicitly flag theF1.10attribution as doubtful. - Revise the ares host README to reflect that 3/5 ares divergences are corroborated by snes9x (changing the interpretation from “ares-alone” to several 2-vs-2 splits) and correct the prior
F1.10/PAD2_CONTRACTclaim. - Mirror the above clarifications in the changelog for discoverability.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| scripts/accuracysnes/crossval.sh | Comment-only clarification: use stable row identifiers instead of catalogue indices; mark F1.10 attribution as doubtful. |
| scripts/accuracysnes/ares_host/README.md | Documentation correction and reframing of the five ares divergences, including the F1.10 attribution fix. |
| CHANGELOG.md | Changelog entry updated to reflect the same adjudication/attribution corrections and index-drift warning. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@CHANGELOG.md`:
- Around line 27-38: Revise the CHANGELOG discussion of F1.10 so the Mesen2
failure remains explicitly conditional rather than established. Update the
statement that identifies E8.02 and E3.06 as the only ares-only rows to
acknowledge that F1.10 is also not ares-only if Mesen2’s recorded failure is
confirmed, while preserving the later attribution doubt.
🪄 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: b74a340e-6e84-4d57-8822-0495d89b9549
📒 Files selected for processing (3)
CHANGELOG.mdscripts/accuracysnes/ares_host/README.mdscripts/accuracysnes/crossval.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: test-light
- GitHub Check: lint
- GitHub Check: accuracysnes
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{rs,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Chip-behavior changes must update both the chip implementation and the corresponding
docs/<subsystem>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
scripts/accuracysnes/ares_host/README.mdCHANGELOG.md
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Do not commit or vendor the generatedsnesdev_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 relevantto-dos/sprint file.
**/*: Preserve the one-directional crate graph: chip crates must not depend on one another;rustysnes-coreties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keepdocs/STATUS.mdas the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNESv2.0orengine-lineageanchors as project releases.
Files:
scripts/accuracysnes/ares_host/README.mdscripts/accuracysnes/crossval.shCHANGELOG.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_FAILURESwith a source citation, never in a widened match.
Files:
scripts/accuracysnes/ares_host/README.mdscripts/accuracysnes/crossval.sh
**/*.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:
scripts/accuracysnes/ares_host/README.mdCHANGELOG.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.mdwhen 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
🔇 Additional comments (3)
scripts/accuracysnes/ares_host/README.md (1)
34-61: LGTM!CHANGELOG.md (1)
814-824: LGTM!scripts/accuracysnes/crossval.sh (1)
174-194: LGTM!
| **Five rows where ares disagrees with the cart** — and **three of them are rows snes9x already | ||
| fails**, which the tally alone does not show. `C7.10` and `F1.10` become **2-vs-2** (RustySNES + | ||
| Mesen2 against snes9x + ares); `C7.05` is 2-vs-2 with the two dissenters failing on *different* | ||
| codes; only `E8.02` and `E3.06` are ares-only. So ares is corroborating snes9x more than it is | ||
| standing alone. Not wired into `crossval.sh` — an `ARES_KNOWN_FAILURES` constant encoding | ||
| unadjudicated disagreements would be worse than no third reference. | ||
|
|
||
| **`F1.10` deserves a hard look.** fullsnes puts the automatic read's start ~dot 32.5-95.5 into the | ||
| first vblank line rather than at the vblank edge. snes9x fails that row, ares fails it, and | ||
| `crossval.sh` records Mesen2 failing it too — which would leave **RustySNES passing alone**, on a | ||
| row it passes only because of a deliberate fix. The standing heuristic says RustySNES failing | ||
| alone means a real bug; it should say the same about RustySNES *passing* alone. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Keep the F1.10 result explicitly conditional.
This entry states that only E8.02 and E3.06 are ares-only, then treats Mesen2's F1.10 failure as established. The later entry from Line 819 through Line 824 marks that attribution as doubtful. If Mesen2 really fails F1.10, that row is not ares-only.
Proposed wording
- codes; only `E8.02` and `E3.06` are ares-only. So ares is corroborating snes9x more than it is
+ codes; only `E8.02` and `E3.06` are ares-only in the measured set; `F1.10` remains unresolved.
+ So ares is corroborating snes9x more than it is
- `crossval.sh` records Mesen2 failing it too — which would leave **RustySNES passing alone**, on a row
+ `crossval.sh` attributes a failure to Mesen2 — if that attribution is correct, **RustySNES would be
+ passing alone**, on a rowAs per path instructions, **/*.md: “Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes rather than style nits.”
📝 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.
| **Five rows where ares disagrees with the cart** — and **three of them are rows snes9x already | |
| fails**, which the tally alone does not show. `C7.10` and `F1.10` become **2-vs-2** (RustySNES + | |
| Mesen2 against snes9x + ares); `C7.05` is 2-vs-2 with the two dissenters failing on *different* | |
| codes; only `E8.02` and `E3.06` are ares-only. So ares is corroborating snes9x more than it is | |
| standing alone. Not wired into `crossval.sh` — an `ARES_KNOWN_FAILURES` constant encoding | |
| unadjudicated disagreements would be worse than no third reference. | |
| **`F1.10` deserves a hard look.** fullsnes puts the automatic read's start ~dot 32.5-95.5 into the | |
| first vblank line rather than at the vblank edge. snes9x fails that row, ares fails it, and | |
| `crossval.sh` records Mesen2 failing it too — which would leave **RustySNES passing alone**, on a | |
| row it passes only because of a deliberate fix. The standing heuristic says RustySNES failing | |
| alone means a real bug; it should say the same about RustySNES *passing* alone. | |
| **Five rows where ares disagrees with the cart** — and **three of them are rows snes9x already | |
| fails**, which the tally alone does not show. `C7.10` and `F1.10` become **2-vs-2** (RustySNES + | |
| Mesen2 against snes9x + ares); `C7.05` is 2-vs-2 with the two dissenters failing on *different* | |
| codes; only `E8.02` and `E3.06` are ares-only in the measured set; `F1.10` remains unresolved. | |
| So ares is corroborating snes9x more than it is | |
| standing alone. Not wired into `crossval.sh` — an `ARES_KNOWN_FAILURES` constant encoding | |
| unadjudicated disagreements would be worse than no third reference. | |
| **`F1.10` deserves a hard look.** fullsnes puts the automatic read's start ~dot 32.5-95.5 into the | |
| first vblank line rather than at the vblank edge. snes9x fails that row, ares fails it, and | |
| `crossval.sh` attributes a failure to Mesen2 — if that attribution is correct, **RustySNES would be | |
| passing alone**, on a row it passes only because of a deliberate fix. The standing heuristic says RustySNES failing | |
| alone means a real bug; it should say the same about RustySNES *passing* alone. |
🤖 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 27 - 38, Revise the CHANGELOG discussion of F1.10
so the Mesen2 failure remains explicitly conditional rather than established.
Update the statement that identifies E8.02 and E3.06 as the only ares-only rows
to acknowledge that F1.10 is also not ares-only if Mesen2’s recorded failure is
confirmed, while preserving the later attribution doubt.
Source: Path instructions
Follow-up to #304, which established the ares host and reported five divergences without adjudicating them.
Three of the five are rows snes9x already fails
The tally alone does not show this, and it changes what each row means:
C7.05C7.10F1.10E8.02E3.06So ares is corroborating snes9x on three rows rather than standing alone. Still not wired into
crossval.sh: anARES_KNOWN_FAILURESconstant encoding unadjudicated disagreements would be worse than no third reference.Correction 1 — to my own README and to PR #304
I wrote that
F1.10is aPAD2_CONTRACTrow and that this host'''s port detection should be suspected first. That is wrong.f1_require_contractreads$4016only — port 1 — andF1.10code 2 means "$4212read busy at the very start of the vblank line", which does not involve controller state at all. I inherited the claim fromcrossval.sh'''s Mesen2 grouping instead of checking it.Correction 2 — to
crossval.shIts
MESEN2_KNOWN_FAILUREScomment namesidx279 F1.03andidx286 F1.10. In the current catalogue those indices areF1.01andF1.08— an index moves whenever a test is added ahead of it. Now keyed on row names, with the drift stated: an index in a comment is a fact with a shelf life.That same comment attributes Mesen2'''s
F1.10failure to the port-2 limitation, while the snes9x block a few lines above says Mesen2 passesF1.10. Both cannot be true. Marked as doubted rather than quietly rewritten — resolving it needs Mesen2'''s failing set read atDONEand mapped throughSOURCE_CATALOG.tsv, a measurement nobody has taken since the catalogue grew.And
F1.10now deserves a hard lookfullsnes puts the automatic read'''s start ~dot 32.5–95.5 into the first vblank line rather than at the vblank edge. snes9x fails that row; ares fails it; and if the Mesen2 attribution is right, Mesen2 fails it too — leaving RustySNES passing alone, on a row it passes only because of a deliberate fix. This project'''s heuristic says RustySNES failing alone means a real bug. It should say the same about RustySNES passing alone.
Verification
REF_PROJ=$PWD/ref-proj bash scripts/accuracysnes/crossval.sh— unchanged:snes9x: OK (14 known),Mesen2: OK (2 known), 54 scenes match on both,2 reference(s) agree with the cart.bash -nclean. Docs and comments only.🤖 Generated with Claude Code
Summary
This change updates AccuracySNES dossier assertions from ares comparison results.
E8.02andE3.06remain ares-only. This claim is false if fresh comparison results change either the overlap or the ares-only rows.F1.10reads$4016, while code 2 concerns$4212busy status at vblank start. The previousPAD2_CONTRACTattribution is false.F1.10attribution remains unresolved because recorded behavior conflicts. The claim is false if fresh measurement confirms one attribution.F1.10for investigation because RustySNES may pass the row alone despite the deliberate fix. This claim is false if isolated testing shows RustySNES fails or the fix is not causal.