docs(accuracysnes): record why E8.06 is not reachable from the cart - #301
Conversation
Authored, injected and withdrawn the same day. The design was sound -- two KON writes 32 cycles apart, GAIN $DF so ENVX climbs 2 per sample, difference the two voices: 4 for the two-sample lead hardware gives, 2 for the one-sample lead a per-sample poll would give, with both hardware outcomes accepted so no uncontrolled phase decides the verdict. What killed it is the read window. Each voice's ENVX is latched at its own tick of the DSP's 32-tick schedule, so a reading taken between two voices' updates is one whole sample wrong -- exactly the size of the signal. Bracketing the voice-6 read with two voice-7 reads and requiring them to agree does not fit: three dsp_read_to calls span 39 cycles of a 32-cycle sample, and even landing each reading in its own register spans 21. Injecting the named bug trips the guard rather than the row's own code on both versions, so the scored assertion could not be demonstrated. The reusable finding: two voices' ENVX cannot be compared at single-sample resolution from the cart. The loop is already as tight as the instruction set allows, so a future row needs a different observable rather than a faster one. E8.06's swallow half is not separately assertable either -- at any gap both poll rates swallow or spare the first voice depending on phase. Withdrawal verified byte-clean: the ROM returned to checksum $DA9E, its value before the row was added. The MOV Y,dp / MOV X,dp emitters went with it, since this file carries only opcodes a committed test exercises. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WalkthroughDocuments the attempted and withdrawn E8.06 test. It records the two-voice ChangesAccuracySNES test documentation
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
Comment |
Antigravity review (Gemini via Ultra)This documentation-only PR records the design rationale, hardware read-window limitations, and withdrawal of test row Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
There was a problem hiding this comment.
Pull request overview
Documents why AccuracySNES row E8.06 was attempted and then withdrawn, capturing the key technical limitation (DSP ENVX latch timing/read window) so future work doesn’t repeat the same dead-end, while keeping the ROM byte-identical (checksum unchanged).
Changes:
- Add a new subsection for
E8.06describing the intended design, the failure mode, and the reusable takeaway. - Record the “withdrawal verified byte-clean” rationale (ROM checksum returned to the prior value).
| **Attempted 2026-08-01 and withdrawn the same day.** The row reads: *"collapse case `KON=$01` / | ||
| `KON=$02` → usually only voice 2, and if both, voice 1 is **2 samples ahead**, proving the 16 kHz | ||
| rate."* It is `E8.01`'s mechanism asked the other way round, and the setup was already built, so it | ||
| looked like the cheapest row left in Group E. | ||
|
|
![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 `@docs/accuracysnes-plan.md`:
- Around line 296-301: Annotate the E8.06 dossier entry in
docs/accuracysnes-plan.md as [NOT CART-MEASURABLE], preserving its existing
assertion and explaining the limitation with links to §E8.06 and ticket T-04-E.
Do not modify the generated coverage file, which already records E8.06 as
uncovered.
🪄 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: 913bfc2d-9333-4d64-9488-8a4d4f636383
📒 Files selected for processing (1)
docs/accuracysnes-plan.md
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: build demo + docs
- GitHub Check: review
🧰 Additional context used
📓 Path-based instructions (6)
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>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
docs/accuracysnes-plan.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/accuracysnes-plan.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 generateddocs/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
🧠 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
| ### `E8.06` — authored, injected, and withdrawn: two voices cannot be compared at one sample | ||
|
|
||
| **Attempted 2026-08-01 and withdrawn the same day.** The row reads: *"collapse case `KON=$01` / | ||
| `KON=$02` → usually only voice 2, and if both, voice 1 is **2 samples ahead**, proving the 16 kHz | ||
| rate."* It is `E8.01`'s mechanism asked the other way round, and the setup was already built, so it | ||
| looked like the cheapest row left in Group E. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'E8\.06|NOT CART-MEASURABLE' docs to-dosRepository: doublegate/RustySNES
Length of output: 9101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- required documentation context ---'
for f in docs/architecture.md docs/STATUS.md CONTRIBUTING.md; do
if [ -f "$f" ]; then
printf '\n### %s\n' "$f"
sed -n '1,220p' "$f"
fi
done
printf '%s\n' '--- E8.06 and coverage conventions ---'
rg -n -C 8 'E8\.06|NOT CART-MEASURABLE|coverage|T-06|roadmap' \
docs/accuracysnes-plan.md docs/accuracysnes-research-dossier.md \
docs/accuracysnes-coverage.md to-dos
printf '%s\n' '--- candidate subsystem and ADR documentation ---'
rg -l -i 'AccuracySNES|SPC|DSP|audio|E8' docs adr* 2>/dev/null | head -80Repository: doublegate/RustySNES
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- plan section ---'
sed -n '280,345p' docs/accuracysnes-plan.md
printf '%s\n' '--- dossier E8 section and related status conventions ---'
sed -n '1278,1310p' docs/accuracysnes-research-dossier.md
rg -n -C 3 '\[NOT CART-MEASURABLE' docs to-dos
printf '%s\n' '--- coverage generator and dossier parsing ---'
rg -n -C 5 'accuracysnes-coverage|NOT CART-MEASURABLE|research-dossier|Uncovered|uncovered' \
tests scripts docs to-dos \
-g '*.rs' -g '*.py' -g '*.sh' -g '*.md' -g '*.toml'
printf '%s\n' '--- E8.06 ticket references ---'
rg -n -C 5 'E8\.06|T-[A-Z0-9-]+' docs/accuracysnes-plan.md \
docs/accuracysnes-research-dossier.md to-dos | \
grep -E 'E8\.06|T-[A-Z0-9-]+' | head -120Repository: doublegate/RustySNES
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact roadmap E-ticket rows ---'
sed -n '448,470p' to-dos/ROADMAP.md
printf '%s\n' '--- all exact E8.06 references ---'
rg -n 'E8\.06' . \
-g '!snesdev_wiki/**' \
-g '!target/**'
printf '%s\n' '--- coverage parser and report implementation ---'
fd -t f . tests/roms/AccuracySNES/gen/src | sort
rg -n -C 8 'fn parse_enumeration|parse_enumeration|coverage_report|covered|uncovered' \
tests/roms/AccuracySNES/gen/src
printf '%s\n' '--- status-marker grammar ---'
rg -n -C 4 'NOT CART-MEASURABLE|NOT.*MEASURABLE|status marker|annotation' \
tests/roms/AccuracySNES docs to-dos \
-g '*.rs' -g '*.md' -g '*.toml'Repository: doublegate/RustySNES
Length of output: 37167
Annotate E8.06 in the dossier as not cart-measurable.
Keep the assertion unchanged and add a [NOT CART-MEASURABLE] explanation linking docs/accuracysnes-plan.md §E8.06 and ticket T-04-E. Do not edit the generated coverage file; it already lists E8.06 as uncovered.
🤖 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/accuracysnes-plan.md` around lines 296 - 301, Annotate the E8.06 dossier
entry in docs/accuracysnes-plan.md as [NOT CART-MEASURABLE], preserving its
existing assertion and explaining the limitation with links to §E8.06 and ticket
T-04-E. Do not modify the generated coverage file, which already records E8.06
as uncovered.
Sources: Path instructions, Learnings
Authored, injected and withdrawn the same day. Docs-only — the ROM is byte-identical (checksum
$DA9Ebefore and after), which is the withdrawal check this project uses.The design was sound. Two
KONwrites exactly 32 SPC cycles apart,GAIN = $DFsoENVXclimbs 2 per output sample, then difference the two voices: 4 for the two-sample lead hardware gives, 2 for the one-sample lead a per-sample poll would give. Both hardware outcomes are accepted, so no uncontrolled phase decides the verdict — the lesson fromE8.01's two rejected drafts.What killed it is the read window. Each voice's
ENVXis latched at its own tick of the DSP's 32-tick schedule, so a reading taken between two voices' updates is one whole sample wrong — exactly the size of the signal. Bracketing the voice-6 read with two voice-7 reads and requiring them to agree does not fit:dsp_read_tox3MOV Y,dp/MOV X,dp)Injecting the named bug (
KONexamined every sample) trips the guard, not the row's own code, on both versions. A row whose named injection fires the guard rather than the assertion has not been demonstrated.The reusable finding: two voices'
ENVXcannot be compared at single-sample resolution from the cart. The loop is already as tight as the instruction set allows, so a future row needs a different observable rather than a faster one.E8.06's swallow half is not separately assertable either — at any gap both poll rates swallow or spare the first voice depending on phase, so the lead is what carries the claim.The two new SPC700 emitters went with it: this file carries only opcodes a committed test exercises, and an unexercised encoding is an unverified one.
🤖 Generated with Claude Code
Documents the attempted and withdrawn AccuracySNES dossier assertion E8.06.
The proposed two-voice
ENVXcomparison is not reliable at single-sample resolution because DSP read timing spans too much of the sample window. The named bug triggers the guard before the assertion on both hardware versions.The coverage denominator does not move. The ROM remains byte-identical and retains checksum
$DA9E.The claim is false if a future observable permits reliable cart-level single-sample envelope comparison, or if the timing and injection results are disproven.