docs: sync coverage figures to the generated report (351/443) - #311
Conversation
The generated `docs/accuracysnes-coverage.md` reads 297 on-cart + 54 scene-only = 351; the hand-maintained figures in STATUS.md and accuracysnes-plan.md still said 350 (297 + 53). PR #302's "unchanged at 350" claim was wrong and the CHANGELOG says so. Three other hand-maintained lines had drifted behind this cycle's work and are corrected in the same pass: - the reference list — Mesen2, snes9x and ares, three independent references as of v1.29.0, with MesenCE named separately as the per-dot compositor's blueprint rather than as a cross-validation reference; - Mesen2's known-failure rationale — F1.03 alone (it clocks both ports out of one latch, so it needs the port-2 input the Lua runner cannot drive), not "two PAD2_CONTRACT rows"; - the scene tally — 54 declared, all matching on both scene hosts. STATUS.md also gains a paragraph on the headless ares host: why a third reference was needed (the provenance rule counts ares and bsnes as one, so several rows sat at 2-vs-1 with no tiebreaker) and the defect it found in ares rather than in this emulator. No code or ROM change; the battery and its pass rate are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (6)docs/**/*.md📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{rs,md}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/**/*📄 CodeRabbit inference engine (docs/testing-strategy.md)
Files:
docs/**⚙️ CodeRabbit configuration file
Files:
**/*.md⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (2)📚 Learning: 2026-07-21T01:34:22.909ZApplied to files:
📚 Learning: 2026-07-21T05:22:58.848ZApplied to files:
🔇 Additional comments (1)
WalkthroughThe PR reconciles AccuracySNES documentation with 351 covered assertions, 54 rendered scenes, ares cross-validation, and revised Mesen2 failure reporting. It adds a changelog entry. It does not change test or coverage behavior. ChangesAccuracySNES documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
Comment |
There was a problem hiding this comment.
Pull request overview
Reconciles hand-maintained AccuracySNES coverage and reference-host documentation with the generated docs/accuracysnes-coverage.md report (which now totals 351/443), and records the correction explicitly in the changelog.
Changes:
- Update AccuracySNES coverage figures from 350 → 351 of 443 and 53 → 54 scene-only rows in
docs/STATUS.mdanddocs/accuracysnes-plan.md. - Refresh cross-validation/reference-host documentation to reflect Mesen2 + snes9x + ares (with MesenCE called out separately), and clarify Mesen2’s known-failure rationale (
F1.03only). - Add an Unreleased changelog entry documenting the correction rather than silently editing the numbers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/STATUS.md | Synces the headline AccuracySNES coverage numbers and adds a short note on the headless ares host rationale. |
| docs/accuracysnes-plan.md | Updates the plan’s “where the battery stands” table and related narrative to match the generated coverage and current reference-host set. |
| CHANGELOG.md | Adds an explicit Unreleased correction note about the 351/443 figure and related doc reconciliations. |
| ### Fixed | ||
|
|
||
| - **CORRECTION: the battery covers 351 of 443 assertions, not 350.** The `E5.06` entry below says | ||
| coverage was "350 either way"; the regenerated `docs/accuracysnes-coverage.md` reads **297 on-cart |
There was a problem hiding this comment.
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 `@docs/STATUS.md`:
- Around line 219-225: Align the documentation with the currently executable
validation path by removing unsupported ares claims unless a runnable host and
driver are added. In docs/STATUS.md lines 219-225 remove the headless-host and
defect claims; in docs/STATUS.md line 244 remove ares from the cross-validation
list and replace “independent references” with provenance-accurate wording; in
docs/accuracysnes-plan.md lines 20-24 and 746-751 remove ares-specific and
all-host results; and in CHANGELOG.md lines 18-21 document ares only once its
host and driver are shipped.
- Around line 26-27: Update the status wording around the “351 of 443” figure to
identify it as dossier assertion coverage, not the battery test count; preserve
the distinction between the 338 battery tests and 351 dossier assertions.
🪄 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: cfa176b2-88f1-4513-b815-7a19728a4122
📒 Files selected for processing (3)
CHANGELOG.mddocs/STATUS.mddocs/accuracysnes-plan.md
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: build demo + docs
🧰 Additional context used
📓 Path-based instructions (7)
docs/**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Before changing a subsystem, consult
docs/architecture.md,docs/STATUS.md,CONTRIBUTING.md, the relevant subsystem documentation, and applicable ADRs.New subsystems must add documentation under
docs/.
Files:
docs/STATUS.mddocs/accuracysnes-plan.md
**/*.{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:
docs/STATUS.mddocs/accuracysnes-plan.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:
docs/STATUS.mddocs/accuracysnes-plan.mdCHANGELOG.md
docs/**/*
📄 CodeRabbit inference engine (docs/testing-strategy.md)
Chip crates should exceed 90% unit-test coverage, and each chip should be fuzzable in isolation.
Files:
docs/STATUS.mddocs/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 generateddocs/accuracysnes-coverage.md, and any statement of coverage that
is broader than what the corresponding test actually asserts.
Files:
docs/STATUS.mddocs/accuracysnes-plan.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).
Files:
docs/STATUS.mddocs/accuracysnes-plan.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
🧠 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
🪛 LanguageTool
docs/STATUS.md
[style] ~244-~244: Try using a descriptive adverb here.
Context: ...cs/adr/0013), kept as separate columns on purpose (docs/accuracysnes-coverage.md`, regen...
(ON_PURPOSE_DELIBERATELY)
🔇 Additional comments (2)
docs/accuracysnes-plan.md (1)
17-19: LGTM!CHANGELOG.md (1)
12-17: LGTM!
| results + per-test B-skip + a Select WRAM debug viewer) with its battery at **351 of 443** dossier | ||
| assertions (297 on-cart + 54 rendered scenes). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use dossier coverage terminology.
Line 26-27 calls 351/443 the battery size. The plan distinguishes 338 battery tests from 351 dossier assertions. State that the dossier coverage is 351/443 to prevent readers from treating 351 as the test count.
Proposed wording
-results + per-test B-skip + a Select WRAM debug viewer) with its battery at **351 of 443** dossier
-assertions (297 on-cart + 54 rendered scenes).
+results + per-test B-skip + a Select WRAM debug viewer) with dossier coverage at **351 of 443**
+assertions (297 on-cart + 54 rendered scenes).Based on learnings: battery test count and dossier assertion coverage are intentionally non-equivalent.
As per path instructions: Docs are the spec, not a history log.
📝 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.
| results + per-test B-skip + a Select WRAM debug viewer) with its battery at **351 of 443** dossier | |
| assertions (297 on-cart + 54 rendered scenes). | |
| results + per-test B-skip + a Select WRAM debug viewer) with dossier coverage at **351 of 443** | |
| assertions (297 on-cart + 54 rendered scenes). |
🤖 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 `@docs/STATUS.md` around lines 26 - 27, Update the status wording around the
“351 of 443” figure to identify it as dossier assertion coverage, not the
battery test count; preserve the distinction between the 338 battery tests and
351 dossier assertions.
Sources: Path instructions, Learnings
| As of `v1.29.0` a **headless ares host** (`scripts/accuracysnes/ares_host/`) runs the battery as a | ||
| third reference, which matters because this project's provenance rule counts ares and bsnes as one: | ||
| several rows sat at 2-vs-1 with no way to break the tie. It also produced the first defect | ||
| AccuracySNES has found in a *reference* emulator rather than in this one — ares' `$F1` handler | ||
| negates the timer-2 counter reset, so any later `$F1` write zeroes `T2OUT`, while its own timers 0 | ||
| and 1 use the un-negated form. | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Align all ares claims with the executable validation path.
The supplied scripts/accuracysnes/crossval.sh explicitly excludes ares and states that no headless or memory-dump path exists. The changed documentation and changelog therefore report results that the shown validation path cannot produce. Update the host, driver, and citations together. If the ares claims remain, use “three reference emulators” unless the provenance rule supports “independent.”
docs/STATUS.md#L219-L225: add the ares host to the executable validation path, or remove the headless-host and defect claims.docs/STATUS.md#L244-L244: remove ares from the cross-validation list until the runner produces the result, and replace “independent references” unless justified by the provenance model.docs/accuracysnes-plan.md#L20-L24: do not citescripts/accuracysnes/crossval.shfor ares divergences until the script runs ares.docs/accuracysnes-plan.md#L746-L751: reportares: OK (4 known)and all-host scene results only after the repository contains that runnable validation path.CHANGELOG.md#L18-L21: document ares as a reference only when the corresponding host and driver are shipped.
As per path instructions: Docs are the spec, not a history log.
📍 Affects 3 files
docs/STATUS.md#L219-L225(this comment)docs/STATUS.md#L244-L244docs/accuracysnes-plan.md#L20-L24docs/accuracysnes-plan.md#L746-L751CHANGELOG.md#L18-L21
🤖 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 `@docs/STATUS.md` around lines 219 - 225, Align the documentation with the
currently executable validation path by removing unsupported ares claims unless
a runnable host and driver are added. In docs/STATUS.md lines 219-225 remove the
headless-host and defect claims; in docs/STATUS.md line 244 remove ares from the
cross-validation list and replace “independent references” with
provenance-accurate wording; in docs/accuracysnes-plan.md lines 20-24 and
746-751 remove ares-specific and all-host results; and in CHANGELOG.md lines
18-21 document ares only once its host and driver are shipped.
Source: Path instructions
Caught by the bot review, which flagged the internal contradiction between the updated table at the top and this line further down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Antigravity review (Gemini via Ultra)This documentation PR synchronizes hand-maintained coverage stats, scene tallies, and reference emulator test results across Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
What
The generated
docs/accuracysnes-coverage.mdreads 297 on-cart + 54 scene-only = 351 of 443. The hand-maintained figures indocs/STATUS.mdanddocs/accuracysnes-plan.mdstill said 350 (297 + 53). PR #302's "coverage unchanged at 350" claim was wrong, and the CHANGELOG entry here says so rather than quietly editing the number.The generated report is the authority precisely because the hand-maintained ones drift. This is the periodic reconciliation, not a coverage change — no code, no ROM, no battery change.
Three other lines that had drifted behind this cycle
v1.29.0— with MesenCE named separately as the per-dot compositor's blueprint and exact-frame oracle rather than as a cross-validation reference. It never was one; it was listed as though it were.F1.03alone, and why: it clocks both ports out of one latch, so it needs the port-2 input the Lua runner cannot drive. The old text said "twoPAD2_CONTRACTrows", which was wrong on both the count and, forF1.10, the cause.Added to STATUS.md
A paragraph on the headless ares host (
scripts/accuracysnes/ares_host/): why a third reference was needed at all — the provenance rule counts ares and bsnes as one, so several rows sat at 2-vs-1 with no way to break the tie — and the defect it produced in ares rather than in this emulator ($F1's timer-2 counter reset is negated, so any later$F1write zeroesT2OUT, while ares' own timers 0 and 1 use the un-negated form).Verification
Current cross-validation, unchanged by this PR:
snes9x: OK (14 known),Mesen2: OK (1 known),ares: OK (4 known), 54/54 scenes on both scene hosts,3 reference(s) agree with the cart.🤖 Generated with Claude Code
This change claims that the documentation now matches the generated AccuracySNES report: 297 on-cart assertions plus 54 scene-only assertions, for 351 of 443 total assertions. It alters the coverage tally, the 54/54 scene tally, Mesen2’s known failure at F1.03, and the reference-emulator validation records. The coverage denominator remains 443.
The claim is false if the regenerated report does not show 351 covered assertions, if fewer than 54 scenes match on both hosts, or if the recorded Mesen2, snes9x, and ares validation results do not reproduce. The ares timer defect applies when the headless host processes timer 2 at
$F1.