docs: interlace needs a rule extract cannot express, and Mesen2 is no longer flaky - #326
Conversation
…no longer flaky Probed and withdrawn. Two findings. GOOD NEWS: the recorded Mesen2 nondeterminism is gone. Three consecutive Mesen2 runs and two snes9x runs of an interlace scene produced identical results, so "Mesen2 alternates between both field parities run-to-run on one build" no longer reproduces -- almost certainly the emu.setInput defect fixed earlier. "Run any field-dependent capture twice per host" can be retired as the REASON interlace is blocked. THE BLOCKER: the two hosts emit different WIDTHS for the same interlace scene. snes9x renders a single field at 256x224; Mesen2 doubles both dimensions to 512x478. Hi-res worked because both emitted 512 wide and differed only in height, which HiResEven absorbs by deriving the row step from the observed height. Here no single (xstep, ystep) pair describes both reductions, so `extract` as designed -- one declared rule per scene, every host applying the same reduction -- is insufficient. ADR 0013 records the three options and argues against the tempting one: a generic "downsample whatever you get to 256x224" rule would subsume HiResEven but give up the declared-not-inferred property, and would then silently accept a 256-wide frame for a scene that must be 512. If interlace is worth covering, a per-host reduction table is the honest shape. Docs only; the probe scene is not in the tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WalkthroughThe changelog and ADR document that interlace output is deterministic, while snes9x and Mesen2 produce incompatible framebuffer geometries. The ADR rejects generic downsampling and identifies per-host reduction rules as the future design. ChangesInterlace geometry documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
Comment |
Antigravity review (Gemini via Ultra)This documentation PR updates ADR 0013 and the project changelog to record that Mesen2 frame captures are no longer nondeterministic and to document why current Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
There was a problem hiding this comment.
🟢 Ready to approve
The changes are documentation-only and the added ADR/changelog content is internally consistent and well-formed.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Updates project documentation around AccuracySNES framebuffer “scenes” extraction rules, clarifying why interlace scenes are still blocked and noting that the previously recorded Mesen2 nondeterminism no longer reproduces.
Changes:
- Extend ADR 0013 with a new “Interlace” section describing the host-geometry mismatch and why the current
extractrule model can’t express it. - Add a matching Unreleased changelog entry summarizing the new finding and pointing readers to ADR 0013.
File summaries
| File | Description |
|---|---|
| docs/adr/0013-accuracysnes-framebuffer-oracle.md | Documents the interlace blocker (host width/height mismatch) and evaluates rule-design options. |
| CHANGELOG.md | Records the updated status of interlace scenes and links the rationale back to ADR 0013. |
Review details
- Files reviewed: 2/2 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: 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 `@CHANGELOG.md`:
- Around line 21-25: Update the HiResEven description in the changelog to
clarify that it is host-specific row handling: each host applies the declared
rule using its own validated row mapping, rather than implying the Rust consumer
derives the row step from observed height. Preserve the existing explanation of
differing host widths and reductions.
In `@docs/adr/0013-accuracysnes-framebuffer-oracle.md`:
- Around line 296-303: Expand the ADR’s extraction rules into a complete
per-host contract covering Mesen2 and snes9x: define row cropping, FIRST_ROW,
column mapping, sample dimensions, accepted input geometry, and rejection
behavior. Correct Mesen2’s 512x478 reduction to specify how 239 rows become the
required 224, document RustySNES’s current 224/239-row limitation, and reconcile
the snes9x 256x224 entry with the 256x448 geometry in libretro_crossval.c.
🪄 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: b2f3cf6e-910f-434e-b968-1372dbe9ce1b
📒 Files selected for processing (2)
CHANGELOG.mddocs/adr/0013-accuracysnes-framebuffer-oracle.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: build demo + docs
🧰 Additional context used
📓 Path-based instructions (8)
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/adr/0013-accuracysnes-framebuffer-oracle.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/adr/0013-accuracysnes-framebuffer-oracle.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/adr/0013-accuracysnes-framebuffer-oracle.mdCHANGELOG.md
docs/adr/**/*
📄 CodeRabbit inference engine (docs/adr/0004-determinism-contract.md)
docs/adr/**/*: The emulator must produce bit-identical framebuffer and audio for the same seed, ROM, and input sequence.
Power-on CPU, PPU, and SMP phase alignment must come from a seeded PRNG, and reset must preserve that alignment.
Track the SPC700 asynchronous domain using the integer relative-time accumulator defined by ADR 0001; do not use floating-point timing for this purpose.
The core must not use system time, thread scheduling, or OS RNG as sources of nondeterminism.
Use a fixed nominal 1.024 MHz SPC domain in the deterministic path; resonator drift must be excluded. Any hardware-accurate drift toggle belongs in the frontend, is off by default, and must remain outside the bit-identical path.
RTC chips such as S-RTC and SPC7110's RTC-4513 must use fixed or seeded time and must never read the host wall clock.
Rate control and run-ahead must be implemented in the frontend, never in core synthesis; netplay rollback must be frontend-orchestrated against the deterministic core.
Core code must not introduce hidden nondeterminism, including host time, thread scheduling, OS RNG, or HashMap iteration order.
Files:
docs/adr/0013-accuracysnes-framebuffer-oracle.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/adr/0013-accuracysnes-framebuffer-oracle.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/adr/0013-accuracysnes-framebuffer-oracle.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/adr/0013-accuracysnes-framebuffer-oracle.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
| The actual blocker is that the two hosts emit different **widths** for the same interlace scene: | ||
| snes9x `256x224`, Mesen2 `512x478`. Hi-res worked because both emitted 512 wide and differed only | ||
| in height, which `HiResEven` absorbs by deriving the row step from the observed height. Here no | ||
| single `(xstep, ystep)` pair describes both reductions, so `extract` as designed — one declared | ||
| rule per scene, every host applying the same reduction — is insufficient. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Describe HiResEven as host-specific row handling.
The entry says HiResEven derives its row step from the observed height. The Rust consumer uses fixed scene rows and does not observe a height-based row step. Mesen2 handles doubled rows in its own host. State that each host applies the declared HiResEven rule with its own validated row mapping.
🤖 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 21 - 25, Update the HiResEven description in the
changelog to clarify that it is host-specific row handling: each host applies
the declared rule using its own validated row mapping, rather than implying the
Rust consumer derives the row step from observed height. Preserve the existing
explanation of differing host widths and reductions.
| 1. A `Downsample` rule that reduces *whatever* the host emits to 256x224 by deriving both steps from | ||
| the observed geometry. General, and it subsumes `HiResEven` — but it gives up the | ||
| **declared-not-inferred** property that is the whole reason `extract` catches a core rendering | ||
| 256 wide when it should render 512. | ||
| 2. A per-host reduction table, so a rule can say "snes9x: as-is; Mesen2: halve both". Keeps the | ||
| declaration honest at the cost of the hosts no longer being interchangeable. | ||
| 3. Declare the expected geometry per rule *and* per host, and reject anything else — the strictest, | ||
| and the most to maintain. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'enum Extract|HiResEven|extract|visible_width|SCENE_W|SCENE_H|FIRST_ROW|SCENE_BUF_LEN' \
--glob '*.rs' --glob '*.lua' --glob '*.c' .Repository: doublegate/RustySNES
Length of output: 50377
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- ADR 0013 target section ---'
sed -n '240,315p' docs/adr/0013-accuracysnes-framebuffer-oracle.md
printf '%s\n' '--- extraction-related files ---'
git ls-files | rg 'AccuracySNES|accuracysnes|0013|scene|libretro' | head -200
printf '%s\n' '--- host extraction definitions and call sites ---'
rg -n -C 5 \
'HiResEven|hires-even|FIRST_ROW|SCENE_H|SCENE_W|line.?double|512x478|512x224|256x239|256x224|extract' \
tests scripts crates docs/adr/0013-accuracysnes-framebuffer-oracle.md \
--glob '*.rs' --glob '*.c' --glob '*.lua' --glob '*.md' --glob '*.tsv' \
| head -1000Repository: doublegate/RustySNES
Length of output: 50379
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- ADR 0013 complete extraction supplement ---'
sed -n '128,205p' docs/adr/0013-accuracysnes-framebuffer-oracle.md
sed -n '268,307p' docs/adr/0013-accuracysnes-framebuffer-oracle.md
printf '%s\n' '--- Rust extraction implementation ---'
sed -n '65,225p' crates/rustysnes-test-harness/tests/accuracysnes_scenes.rs
printf '%s\n' '--- C extraction implementation ---'
sed -n '120,265p' scripts/accuracysnes/libretro_crossval.c
printf '%s\n' '--- Lua extraction implementation ---'
sed -n '120,180p' scripts/accuracysnes/mesen_scenes.lua
printf '%s\n' '--- scene manifest extraction tags ---'
awk -F '\t' 'NF >= 4 { print NR ":" $0 }' tests/roms/AccuracySNES/build/scenes.tsv | head -120Repository: doublegate/RustySNES
Length of output: 29247
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- interlace references and Rust PPU geometry ---'
rg -n -C 4 \
'interlace|INTERLACE|interlaced|pseudo.?hires|frame_hires|visible_height|SCREEN_HEIGHT|overscan' \
docs crates/rustysnes-ppu crates/rustysnes-test-harness scripts/accuracysnes \
--glob '*.md' --glob '*.rs' --glob '*.c' --glob '*.lua' \
| head -1200
printf '%s\n' '--- exact relevant Rust PPU constants and accessors ---'
rg -n -C 8 \
'pub const (SCREEN_WIDTH|MAX_SCREEN_WIDTH|SCREEN_HEIGHT)|fn visible_height|frame_hires|interlace' \
crates/rustysnes-ppu/src --glob '*.rs' | head -500
printf '%s\n' '--- extraction-shape verifier ---'
python3 - <<'PY'
from dataclasses import dataclass
SCENE_W, SCENE_H = 256, 224
cases = {
"snes9x interlace": (256, 224, 1, 1, 0),
"Mesen2 interlace": (512, 478, 2, 2, 7),
}
for name, (w, h, xstep, ystep, first_row) in cases.items():
sample_w = (SCENE_W - 1) if w == 512 else SCENE_W
source_rows = list(range(first_row * ystep, (first_row + SCENE_H) * ystep, ystep))
source_cols = [(1 + x) * xstep for x in range(sample_w)] if w == 512 else list(range(sample_w))
print(name)
print(f" source geometry={w}x{h}")
print(f" sampled geometry={sample_w}x{len(source_rows)}")
print(f" first/last source row={source_rows[0] if source_rows else None}/{source_rows[-1] if source_rows else None}")
print(f" first/last source col={source_cols[0] if source_cols else None}/{source_cols[-1] if source_cols else None}")
assert source_rows[-1] < h
assert source_cols[-1] < w
PYRepository: doublegate/RustySNES
Length of output: 50379
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Rust PPU geometry symbols ---'
rg -n -C 6 \
'pub const (SCREEN_WIDTH|MAX_SCREEN_WIDTH|SCREEN_HEIGHT)|pub const fn visible_height|fn visible_height|SETINI|is_interlace|interlace' \
crates/rustysnes-ppu/src/lib.rs crates/rustysnes-ppu/src/render.rs crates/rustysnes-ppu/src/registers.rs \
--glob '*.rs' | head -700
printf '%s\n' '--- targeted interlace documentation ---'
rg -n -C 8 \
'interlace|field.?doubling|448|478' \
docs/ppu.md docs/accuracysnes-coverability-audit-2026-07-23.md \
docs/adr/0013-accuracysnes-framebuffer-oracle.md docs/adr/0014-per-dot-ppu-compositor.md \
--glob '*.md' | head -700
printf '%s\n' '--- verifier for the documented Mesen2 reduction ---'
python3 - <<'PY'
SCENE_W, SCENE_H = 256, 224
source_w, source_h = 512, 478
first_row, xstep, ystep, first_col = 7, 2, 2, 1
sample_w = SCENE_W - first_col
rows = [(y + first_row) * ystep for y in range(SCENE_H)]
cols = [(x + first_col) * xstep for x in range(sample_w)]
print(f"halved geometry: {source_w // xstep}x{source_h // ystep}")
print(f"cropped sample: {sample_w}x{len(rows)}")
print(f"source rows: {rows[0]}..{rows[-1]}")
print(f"source cols: {cols[0]}..{cols[-1]}")
assert source_w // xstep == 256
assert source_h // ystep == 239
assert len(rows) == SCENE_H
assert rows[-1] < source_h
assert cols[-1] < source_w
PYRepository: doublegate/RustySNES
Length of output: 28114
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf '%s\n' '--- ADR interlace section ---'
sed -n '276,305p' docs/adr/0013-accuracysnes-framebuffer-oracle.md
printf '%s\n' '--- Rust interlace output status ---'
sed -n '193,200p' docs/ppu.md
sed -n '315,321p' docs/ppu.md
sed -n '550,560p' docs/ppu.md
printf '%s\n' '--- conflicting host geometry statements ---'
sed -n '136,152p' docs/adr/0013-accuracysnes-framebuffer-oracle.md
sed -n '196,206p' scripts/accuracysnes/libretro_crossval.c
printf '%s\n' '--- deterministic geometry verifier ---'
python3 - <<'PY'
SCENE_W, SCENE_H = 256, 224
# The ADR's measured Mesen2 interlace frame and the proposed "halve both" reduction.
source_w, source_h = 512, 478
reduced_w, reduced_h = source_w // 2, source_h // 2
print(f"Mesen2 source={source_w}x{source_h}; halve-both result={reduced_w}x{reduced_h}")
assert (reduced_w, reduced_h) == (256, 239)
assert (reduced_w, reduced_h) != (SCENE_W, SCENE_H)
# Existing Mesen2 host constants crop seven leading output rows after line halving.
first_row, ystep = 7, 2
sample_rows = [(y + first_row) * ystep for y in range(SCENE_H)]
print(f"Mesen2 sampled rows={len(sample_rows)}, source range={sample_rows[0]}..{sample_rows[-1]}")
assert len(sample_rows) == SCENE_H
assert sample_rows[-1] < source_h
# RustySNES currently exposes only 224 or 239 visible rows, not interlace-doubled rows.
rust_heights = {224, 239}
print(f"RustySNES documented visible heights={sorted(rust_heights)}")
assert 478 not in rust_heights
PYRepository: doublegate/RustySNES
Length of output: 6024
Define a complete per-host extraction contract.
For Mesen2, 512x478 reduced by two produces 256x239, not 256x224. Specify the row crop, FIRST_ROW, column mapping, sample dimensions, expected geometry, and rejection behavior for each host. RustySNES currently exposes only 224 or 239 rows because interlace field doubling is not implemented. Also reconcile the ADR’s snes9x 256x224 entry with the 256x448 geometry documented in scripts/accuracysnes/libretro_crossval.c. Without this contract, hosts can hash different pixel sets under the same rule.
🤖 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/adr/0013-accuracysnes-framebuffer-oracle.md` around lines 296 - 303,
Expand the ADR’s extraction rules into a complete per-host contract covering
Mesen2 and snes9x: define row cropping, FIRST_ROW, column mapping, sample
dimensions, accepted input geometry, and rejection behavior. Correct Mesen2’s
512x478 reduction to specify how 239 rows become the required 224, document
RustySNES’s current 224/239-row limitation, and reconcile the snes9x 256x224
entry with the 256x448 geometry in libretro_crossval.c.
Source: Path instructions
Probed and withdrawn. Two findings, and the second is the blocker.
The recorded Mesen2 nondeterminism is gone
The standing note says "Mesen2 alternates between both field parities run-to-run on one build" — which is why the earlier interlace scene was withdrawn. It no longer reproduces. Three consecutive Mesen2 runs and two snes9x runs of an interlace scene produced identical results.
That was almost certainly the
emu.setInputdefect fixed earlier this cycle. The advice to "run any field-dependent capture twice per host" stays sound as general practice, but it can be retired as the reason interlace is blocked.The actual blocker: the hosts emit different widths
256x224256x239512x224512x478256x224512x478Hi-res worked because both hosts emitted 512 wide and differed only in height — which
HiResEvenabsorbs by deriving the row step from the observed height. Interlace does not: snes9x renders a single field at 256 wide while Mesen2 doubles both dimensions.HiResEven'swidth == 512assertion cannot hold for both, and no single(xstep, ystep)pair describes both reductions. Soextractas designed — one declared rule per scene, every host applying the same reduction — is insufficient here.Options, and the one to resist
Downsamplethat reduces whatever the host emits to 256×224 by deriving both steps from the observed geometry. General, subsumesHiResEven— and gives up the declared-not-inferred property, which is the whole reasonextractcatches a core rendering 256 wide when it should render 512.Option 1 is the tempting one and should be resisted on its own merits. If interlace is worth covering, option 2 is the honest shape.
Docs only; the probe scene is not in the tree. 55 scenes still match on both hosts.
🤖 Generated with Claude Code
Documents that repeated Mesen2 interlace runs are deterministic after the
emu.setInputfix. The remaining defect occurs on interlace scenes because snes9x outputs256x224and Mesen2 outputs512x478; oneHiResEvenextraction rule cannot validate both geometries.The claim is false if repeated Mesen2 runs become nondeterministic again, or if both hosts produce compatible geometry. ADR 0013 records the alternatives and favors per-host reduction rules over generic downsampling.
No dossier assertions were added or altered. The coverage denominator remains 55 scenes matching on both hosts.