Skip to content

docs(accuracysnes): correct the C7.05/C7.06 verdict — no eval-line gap - #284

Merged
doublegate merged 2 commits into
mainfrom
fix/range-over-eval-line
Jul 31, 2026
Merged

docs(accuracysnes): correct the C7.05/C7.06 verdict — no eval-line gap#284
doublegate merged 2 commits into
mainfrom
fix/range-over-eval-line

Conversation

@doublegate

Copy link
Copy Markdown
Owner

Corrects a claim I landed in #283.

What was wrong

#283's assessment table said C7.05/C7.06 were blocked because "RustySNES sets range_over one
line late per-dot (two batched) against MesenCE"
. That is false.

Re-measured on main with the committed probe:

$ cargo run -p rustysnes-test-harness --bin probe_213e -- scripts/probes/eval-line-213e/probe.sfc
RUSTY range_over first-set scanline=Some(100) time_over=None

100 is MesenCE's value. The one-line lag belonged to the batch compositor, and per-dot is now
the only one. scripts/probes/eval-line-213e/README.md had both figures written down all along:

build scanline
MesenCE (oracle) 100
RustySNES per-dot 100 (matches)
RustySNES batch (retired) 101

So this was a misreading of a stale note, not a measurement that moved. The check cost one command;
I should have run it before writing the row.

What actually blocks those rows

Sampling resolution, not correctness. A polling loop is several dots per iteration and would pass
whatever the true position is — the F1.09 vacuity shape.

The probe itself demonstrates the technique that works: an H-IRQ at a chosen HTIME, latching
$2137/$213F and reading $213E. So the rows should be written as a bracket — one sample just
below the expected H = OAM.INDEX*2 expecting clear, one just above expecting set. That is the same
shape D2.01 already uses, and it is a two-point assertion rather than a dot-exact one, which is
what the instrument can actually support.

Two traps from the probe's own README are carried into the note, since both cost a debugging pass
there: HTIME=300 pushes the IRQ-service latency into the V-counter increment and skews every
reading by a line, and the $213F read is required to reset the OPHCT/OPVCT second-read flip-flop or
odd scanlines are corrupted.

time_over reading None in that output is correct, incidentally: range evaluation caps at 32
sprites, so the probe's 40 8×8 sprites yield 32 tiles, under the 34-tile limit. The probe does not
exercise time_over at all.

Docs and CHANGELOG only.

🤖 Generated with Claude Code

The assessment landed in #283 said RustySNES sets range_over a line late
against MesenCE. Re-measured on main with scripts/probes/eval-line-213e:
it first reads set at scanline 100, which is MesenCE's value.

The one-line lag belonged to the BATCH compositor, and per-dot is now the
only one -- the probe's own README already recorded both figures, so this
was a misreading rather than a measurement that changed.

What actually remains for those rows is sampling resolution, not
correctness. A polling loop is several dots per iteration and would pass
whatever the true position is (the F1.09 vacuity shape). The probe
demonstrates the technique that works -- an H-IRQ at a chosen HTIME,
latching $2137/$213F and reading $213E -- so the rows should be written
as a bracket: one sample below the expected H = OAM.INDEX*2 expecting
clear, one above expecting set, the same shape D2.01 uses.

Both of the probe's recorded traps are carried into the note: HTIME=300
pushes IRQ latency into the V-counter increment and skews every reading
by a line, and the $213F read is required to reset the OPHCT/OPVCT
second-read flip-flop.

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

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 14 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: c8e9a435-4546-4215-ace7-f3451e3efd06

📥 Commits

Reviewing files that changed from the base of the PR and between f004f4e and 98f8761.

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

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

Raised in review: the C7.05/C7.06 cell had grown past 1,000 characters,
which is unreadable in a plain-text editor and makes every future diff
noisy. The A5.18 cell had the same problem at 744.

Both now carry a one-line verdict and point at a subsection below the
table, where the probe invocation, the two sampling traps, and the
wrap-detection tell have room to be stated properly. Also records that
probe.sfc is gitignored, so run.sh must build it first.

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

Copy link
Copy Markdown
Owner Author

Review adjudication

No blocking issues. Both suggestions applied; the nitpick found something real and is also applied.

The table cell is over 1,000 characters. Applied, and the same treatment given to the A5.18
cell, which had the same problem at 744. Both now carry a one-line verdict and point at a subsection
below the table. That is better than a pure length fix: the probe invocation, the two sampling traps
and the wrap-detection tell all get room to be stated properly instead of compressed into a cell.
The file's pre-existing cells run ~400 characters, so the table is back in line with its own
convention.

Check the edited CHANGELOG line is in an unreleased section. Verified — ## [Unreleased] is at
line 10 and the edited line is 16. Nothing finalized was rewritten; the entry was added in #283,
which is itself unreleased.

Verify scripts/probes/eval-line-213e is tracked. Good catch, and the answer is partly:

$ git ls-files scripts/probes/eval-line-213e/
.gitignore  README.md  probe.cfg  probe.s  probe_mesen.lua  run.sh
$ cat scripts/probes/eval-line-213e/.gitignore
probe.o
probe.sfc

The sources and the runner are tracked; the built ROM is gitignored. So "re-measure with the
probe" was not a complete instruction — a developer has to build it first, and that needs cc65.
The subsection now gives both commands:

bash scripts/probes/eval-line-213e/run.sh        # builds probe.sfc (needs cc65)
cargo run -p rustysnes-test-harness --bin probe_213e -- scripts/probes/eval-line-213e/probe.sfc

Docs only; no generator or ROM change, so the committed cart is untouched.

@github-actions

Copy link
Copy Markdown

Antigravity review (Gemini via Ultra)

This PR updates docs/accuracysnes-plan.md and CHANGELOG.md to clarify that C7.05/C7.06 are reachable via an H-IRQ bracket rather than blocked by an evaluation line offset bug, while adding documented rationale for both A5.18 and C7.05/C7.06.

Blocking issues

None found.

Suggestions

  • docs/accuracysnes-plan.md:L469: The A5.18 summary row in the table was shortened, but omitting the explicit mention of dot-wrap/quantization error makes the table row less self-contained compared to the adjacent entries.
  • CHANGELOG.md:L16: The changelog entry was updated to reflect the C7.05/C7.06 verdict change, but ignores the updated rationale details for A5.18 which were also modified in docs/accuracysnes-plan.md.

Nitpicks

  • docs/accuracysnes-plan.md:L506: The code snippet references binary probe_213e and probe.sfc, but there are no check-in updates or verification steps provided ensuring scripts/probes/eval-line-213e/run.sh runs cleanly in CI or standard dev setups.

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

@doublegate
doublegate merged commit cbdbb16 into main Jul 31, 2026
13 checks passed
@doublegate
doublegate deleted the fix/range-over-eval-line branch July 31, 2026 19:23
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.

1 participant