docs(accuracysnes): survey what is left in Group E, per row - #314
Conversation
Six rows landed in this batch at roughly one per sitting, and the next person will want to know which of the remaining 26 is cheapest. The answer is none of them, and this records WHY per row so it is not re-derived: the 21-of-32-cycle read wall (E10.02/E10.03), nothing cart-observable at all (E9.16/E10.04), reachable only through another row's mechanism (E1.11), a rate-or-ramp measurement where the sawtooth trap lives (E6.01/E6.06/E6.10/E9.07/E9.08/E7.02), genuinely racy (E3.12/E4.09/E4.10), thin on oracle (E8.11/E9.20), and one that is large but mechanical and worth planning for (E2.10, the 256-opcode sweep). "It is hard" is not usable; "here is the specific obstacle" is. Also records the two methods that did land six rows: sweep the emulator for dead config before designing anything (E3.09's subject was parsed, serialized and never read), and assert an equality against a control rather than a threshold against nothing (E3.13's control guard caught two setup faults that a threshold row would have reported as findings). 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 accuracy plan adds a Group E survey for 26 remaining rows. It documents observability, timing, race, opcode coverage, and oracle limitations. It also records methods for checking emulator configuration readers and designing controlled equality comparisons. ChangesGroup E accuracy survey
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
Comment |
There was a problem hiding this comment.
🟢 Ready to approve
Documentation-only change with consistent formatting and no apparent correctness issues.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Documents the remaining unimplemented AccuracySNES Group E rows by recording the specific obstacle for each, so future work isn’t spent re-deriving why no “quick win” rows remain.
Changes:
- Add a per-row obstacle survey for the remaining Group E items, grouping rows by why they’re hard/impractical.
- Capture two general methods that helped land the latest six Group E rows (dead-config sweeps; control-vs-threshold assertions).
File summaries
| File | Description |
|---|---|
| docs/accuracysnes-plan.md | Adds a “what’s left and why it’s hard” survey section for Group E plus two reusable method notes for future row design. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
![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 771-773: Reconcile the Group E inventory in the survey with the
generated coverage table: update the 34 and 26 counts to match the current
inventory, remove E8.01 and the other covered IDs from the current remaining
set, and include the 12 omitted uncovered IDs. If these figures are
intentionally historical, label the affected passages as historical snapshots
instead.
🪄 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: 912b4a7a-c71e-4818-b6cb-0d5f712fa812
📒 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
| Six rows landed in this batch (`E8.01`, `E9.02`, `E5.06`, `E9.09`, `E3.09`, `E3.13`) at roughly one | ||
| per sitting. The next person will want to know which of the remaining 26 is cheapest, and the | ||
| answer is **none of them** — this is the survey that establishes that, so it is not re-derived. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="docs/accuracysnes-plan.md"
sed -n '569,590p;769,823p' "$file"
echo
echo "All E-row references in the affected sections:"
sed -n '769,823p' "$file" |
grep -oE '\bE[0-9]+\.[0-9]+\b' |
sort |
uniq -cRepository: doublegate/RustySNES
Length of output: 6066
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' 'Relevant documentation files:'
git ls-files 'docs/*' | grep -E 'accuracysnes-(plan|coverage)\.md$|architecture\.md|STATUS\.md|(^|/)ADR' || true
printf '\n%s\n' 'Group E headings and row references in the plan:'
rg -n -C 2 'Group E|E[0-9]+\.[0-9]+' docs/accuracysnes-plan.md | sed -n '1,260p'
printf '\n%s\n' 'Coverage references for Group E:'
rg -n -C 2 'Group E|E[0-9]+\.[0-9]+' docs/accuracysnes-coverage.md || trueRepository: doublegate/RustySNES
Length of output: 21497
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' 'Plan and coverage status context:'
sed -n '560,590p;724,755p;769,835p' docs/accuracysnes-plan.md
sed -n '1,75p' docs/accuracysnes-coverage.md
printf '\n%s\n' 'Tracked accuracy-related files:'
git ls-files | grep -Ei 'accuracysnes|dossier|accuracy' | sed -n '1,160p'
printf '\n%s\n' 'Working-tree change summary:'
git status --short
git diff --stat -- docs/accuracysnes-plan.md docs/accuracysnes-coverage.mdRepository: doublegate/RustySNES
Length of output: 18991
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' 'Canonical Group E dossier entries:'
rg -n -C 3 '^\|? *E(1|2|3|4|5|6|7|8|9|10)\.' docs/accuracysnes-research-dossier.md | sed -n '1,260p'
printf '\n%s\n' 'Group E entries in the coverability audit:'
rg -n -C 3 'Group E|E[0-9]+\.[0-9]+' docs/accuracysnes-coverability-audit-2026-07-23.md | sed -n '1,260p'
printf '\n%s\n' 'Coverage and plan status references for the six batch IDs:'
for id in E8.01 E9.02 E5.06 E9.09 E3.09 E3.13; do
printf '\n--- %s ---\n' "$id"
rg -n -C 2 --fixed-strings "$id" docs/accuracysnes-coverage.md docs/accuracysnes-plan.md
doneRepository: doublegate/RustySNES
Length of output: 27598
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import re
from pathlib import Path
coverage = Path("docs/accuracysnes-coverage.md").read_text()
plan = Path("docs/accuracysnes-plan.md").read_text()
# Read only Group E rows from the generated coverage table.
start = coverage.index("| `E1` |")
end = coverage.index("| `F1` |")
coverage_rows = coverage[start:end]
uncovered = []
for line in coverage_rows.splitlines():
if not line.startswith("| `E"):
continue
cells = [cell.strip() for cell in line.strip("|").split("|")]
if len(cells) < 5 or cells[4] in ("—", ""):
continue
uncovered.extend(re.findall(r"E\d+\.\d+", cells[4]))
# Read only the survey body, excluding the heading and the following E8.01 section.
survey_start = plan.index("### Survey of what is left in Group E")
survey_end = plan.index("### `E8.01`", survey_start)
survey = plan[survey_start:survey_end]
survey_ids = sorted(set(re.findall(r"E\d+\.\d+", survey)))
print("Generated coverage uncovered IDs:", len(uncovered))
print(" ".join(uncovered))
print("Unique survey IDs:", len(survey_ids))
print(" ".join(survey_ids))
print("Uncovered IDs omitted from survey:", len(set(uncovered) - set(survey_ids)))
print(" ".join(sorted(set(uncovered) - set(survey_ids))))
print("Survey IDs that coverage marks covered:", len(set(survey_ids) - set(uncovered)))
print(" ".join(sorted(set(survey_ids) - set(uncovered))))
PYRepository: doublegate/RustySNES
Length of output: 749
Reconcile the Group E inventory with generated coverage. The coverage table lists 31 uncovered IDs, but the survey names 25 IDs, including six covered rows (E3.09, E3.13, E5.06, E8.01, E9.02, E9.09). It therefore lists only 19 current uncovered rows and omits 12. Align the 34 and 26 counts and remove E8.01 from the current remaining set, or label those passages as historical snapshots.
🤖 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 771 - 773, Reconcile the Group E
inventory in the survey with the generated coverage table: update the 34 and 26
counts to match the current inventory, remove E8.01 and the other covered IDs
from the current remaining set, and include the 12 omitted uncovered IDs. If
these figures are intentionally historical, label the affected passages as
historical snapshots instead.
Sources: Path instructions, Learnings
…ssessive Bot review, both accepted. The survey says OUTX/ENVX without saying which side of the port boundary they are on, and this project has registers by those names on both -- now spelled as the DSP's $x9/$x8/$7C with a note that the whole section means DSP registers, never $2140-$2143. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Both accepted.
|
Antigravity review (Gemini via Ultra)This PR adds a technical survey section to Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
Six Group E rows landed in this batch —
E8.01,E9.02,E5.06,E9.09,E3.09,E3.13— at roughly one per sitting. The next person will reasonably ask which of the remaining 26 is cheapest.The answer is none of them, and this records why per row, so the survey is not re-derived. "It is hard" is not usable; "here is the specific obstacle" is.
E8.06wall)E10.02,E10.03E9.16,E10.04E1.11(needsE3.13's shadow trick)E6.01,E6.06,E6.10,E9.07,E9.08,E7.02E3.12,E4.09,E4.10E2.10, the 256-opcode SPC700 cycle sweepE8.11,E9.20E2.10is the one worth scheduling: it needs every opcode encodable ingen/src/spc.rs, which deliberately carries only opcodes a committed test exercises. Its difficulty is volume rather than design, which makes it the only remaining row whose cost is predictable.The two methods that did land six rows
Sweep the emulator for dead config before designing anything.
E3.09's subject —$F0's SMP wait selectors — was fully parsed intoIo::external_wait/Io::internal_wait, serialized, restored, and never read. A sweep for struct fields written-but-not-read found it, and a second pass foundVoice.keyonin the same state. Grep a setting's readers, not its definition.Assert an equality against a control, never a threshold against nothing.
E3.13compares a voice decoding the RAM shadow under$00F0-$00FFagainst a voice decoding an identical copy in ordinary RAM;E9.09compares a wrapped echo entry against an unwrapped one. Both would otherwise have been "is it non-zero" rows — andE3.13's control guard failed twice on setup faults (KOFleft set by a previous program, thenKONpulsed rather than held). A threshold row would have reported those as findings.Docs only; no code, ROM, or coverage change.
🤖 Generated with Claude Code
Adds a Group E dossier survey for the 26 remaining rows. It asserts that no row is a quick addition, that
E2.10is the only row with predictable cost, and that six completed rows used dead-configuration searches or control comparisons. It names the blockers as unavailable timing observables, unobservable behavior, dependent mechanisms, phase-sensitive measurements, genuine races, weak or ambiguous oracles, and missing SPC700 opcode coverage. It also identifies the DSP registers inE10.03asENDX($7C),VxOUTX($x9), andVxENVX($x8), distinct from the$2140-$2143ports.The claim is false if any surveyed row can be scored without its listed prerequisite, if a row other than
E2.10has predictable implementation cost, or if the documented methods do not account for the six completed rows. The coverage denominator does not move because the change adds documentation only.