fix(ledger): retire "lit pixels" as evidence of rendering (R-18) - #178
Conversation
Asked whether a working Super Mario 64 screenshot could be saved, I captured the scanned-out frames and, for the first time, LOOKED at them. Two titles I had been citing as rendering are producing pure noise. The lit-pixel count reports non-black scanned-out pixels, and uninitialised RDRAM is non-black. Rogue Squadron scores 68,527 of 75,840 (90%) with 4,790 distinct colours; Jet Force Gemini 69,479 (92%) with 6,203. Converted to PNG and viewed, both are RDRAM garbage with no rendered content whatever - and neither title even runs microcode, which should have been the tell. This is the retired > 1_000_000 failure again, and I repeated it in the same session I diagnosed it: a metric a broken machine satisfies exactly as easily as a working one, quoted for weeks because nobody opened the image. Twice now the number looked like progress and was measuring nothing. Video evidence now means one of exactly two things: a byte-comparison against a committed golden frame, or someone actually viewing the output. The capstone still prints the count, relabelled "non-black pixels (NOT proof of rendering)" with the reason inline, so it stays a diagnostic and cannot be misread as a pass. Actual state: no title produces a picture. Super Mario 64 scans out 27 lit pixels of 75,840 - effectively black. Ocarina of Time executes 733 RSP instructions and submits 17,900 RDP commands yet scans out an entirely black frame (0 lit, 1 colour), so the RDP-to-VI presentation path is the next thing to close. screenshots/ stays empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughR-18 documentation and commercial-boot diagnostics now distinguish non-black pixel counts from rendering evidence, prioritising microcode loading and RDP command submission while recording the remaining proof gap. ChangesR-18 evidence clarification
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
Comment |
Review follow-up. "### Corrected" was a one-off heading in a file that otherwise uses Added/Fixed/Changed 161 times between them; it is now "### Fixed". The five-line comment inside the eprintln! restated the ledger's R-18 entry verbatim, which is both clutter and a second copy to drift. Cut to one line naming the rule and pointing at R-18, where the evidence and the measurements belong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Antigravity — the five-line comment duplicates the ledger. ADOPTED. It restated R-18's entry verbatim inside an Nitpick — Both are the same small failure on my part: writing as if this entry were special enough to need its own vocabulary and its own copy of the evidence. It is not; the ledger row is the spec and the CHANGELOG has a convention. |
![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 `@crates/rustyn64-test-harness/tests/commercial_boot.rs`:
- Around line 166-169: Rename the diagnostic metric field and all references
from `lit` to `non_black_pixels`, including `BootResult.lit`, its initialization
and propagation, and the formatted output in the commercial boot harness.
Preserve the existing value and ensure it remains diagnostic-only rather than a
rendering-pass condition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1494e06c-8129-4282-9f0b-e3a460bf2562
📒 Files selected for processing (3)
CHANGELOG.mdcrates/rustyn64-test-harness/tests/commercial_boot.rsdocs/accuracy-ledger.md
The message said "NOT proof of rendering" while the value still flowed through a field called lit, so the name kept asserting what the label denied - and a name is what a future assertion reaches for. Renamed at the field, the local, and the module docs, with the reason on the field rather than only in the ledger. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Antigravity review (Gemini via Ultra)This PR updates documentation, changelogs, and integration test diagnostics to rename the "lit pixels" metric to "non-black pixels" and document that non-black scanout is uninitialized RDRAM noise rather than proof of rendering. Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
1 participant
No title produces a picture, and I had been implying otherwise
Asked whether a working Super Mario 64 screenshot could be saved, I captured the
scanned-out frames and — for the first time — looked at them.
The lit-pixel count reports non-black scanned-out pixels, and uninitialised
RDRAM is non-black. 90% lit with thousands of distinct colours sounds like a
picture; converted to PNG and viewed, both are RDRAM garbage with no rendered
content whatever. Neither title even runs microcode, which should have been the
tell.
This is the
retired > 1_000_000failure, repeated in the same sessionR-18's original root cause hid behind
retired >= 1_000_000— a threshold aNOP-sledding machine cleared just as easily as a booted game. I diagnosed that,
wrote it up as a lesson, and then spent the rest of the session quoting another
metric with exactly the same defect.
Twice the number looked like progress and was measuring nothing.
Change
a committed golden frame, or someone actually viewing the output.
of rendering)" with the reason inline, so it stays a diagnostic and cannot be
misread as a pass condition.
revisions of that row implied rendering.
Where this leaves video
screenshots/stays empty. The most informative case is Ocarina of Time: itexecutes 733 RSP instructions and submits 17,900 RDP commands, yet scans out
an entirely black frame (0 lit, 1 colour). So the microcode and the DPC seam work
while the RDP → VI presentation path does not — that is now the concrete next
target, and it is much better localised than "reach a title frame".
Also in this branch
Three R-18 hypotheses tested and refuted, recorded so they are not re-chased:
KSEG0 under 64-bit addressing; "these titles sit in the exception vector" (a
correlated capture shows
ExcCode=0,EPC=0—0x8000_0000is simply theCIC-6103/6106 entry point); and R-18's original "the VI vblank interrupt never
fires" (F-Zero X receives 1,228 VI interrupts and 3,304 IP2 assertions, comparable
to a working SM64).
Gates
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace,RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps,pre-commit run markdownlint --all-files— each run separately, exitstatus checked.
Oracle: no emulation behaviour changes (docs + one test-output relabel), so
n64-systemtest is unchanged at 90 suite-wide, Phase 1
Failed: 0, perdocs/STATUS.md.🤖 Generated with Claude Code