docs(ledger): record the provisional CI4 probe result and the tlut_en gap - #181
Conversation
Ran the probe the previous entry named. It eliminates the CI4 decoder as the cause of R-18's black frame, and turns up a separate real defect. A CI4-with-TLUT vector was authored - eight indices 0..7 against eight distinct non-zero palette entries - and replayed. RustyN64 resolves the full palette correctly: f801 07c1 003f ffff ffc1 07ff, i.e. red, green, blue, white, yellow, cyan, exactly the authored TLUT. So the colour-indexed fetch works in isolation and is not what turns Ocarina black. The vector is deliberately NOT committed. Angrylion rendered mostly 0x0001 from the same command list, which means the authored Load Tlut encoding is wrong - Angrylion loaded a near-empty palette and we were more permissive about where the entries came from. Committing that golden would pin my authoring error as the spec, which is the one thing a test-as-spec project must not do. The next probe has to verify the Load Tlut encoding against N64brew before any golden is trusted. Separately, and independent of R-18: Other Modes.en_tlut (command bit 47) is not decoded at all. Our TLUT lookup is driven purely by the tile's format field, so a CI tile with en_tlut clear still gets a palette lookup and a non-CI tile with it set does not - wrong in both directions. So the black frame is narrowed to the texture path but NOT to the CI4 decoder. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 14 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 (2)
📝 WalkthroughWalkthroughR-18 documentation now records the corrected IPL3 boot root cause, revises “lit pixels” evidence, and narrows the remaining black-frame diagnosis to CI4/TLUT handling, including undecoded ChangesR-18 accuracy diagnosis
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 8 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (8 passed)
Comment |
![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/accuracy-ledger.md`:
- Line 413: Keep the CI4/TLUT findings explicitly provisional in the R-18 entry:
retain that the isolated decoder probe is not evidence of the correct Load Tlut
encoding, and state that N64brew Load Tlut and Other Modes.en_tlut citations
must be verified before treating the behavior as settled. Apply the same
qualification to the corresponding CHANGELOG entry at CHANGELOG.md lines 37-47;
do not present CI4 or en_tlut behavior as confirmed hardware semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bb0cba60-cd3e-4af0-a0d2-c52b06412ed3
📒 Files selected for processing (2)
CHANGELOG.mddocs/accuracy-ledger.md
doublegate
changed the title
Review follow-up, all three findings adopted. The CI4 result is now recorded as provisional rather than as an elimination. The reviewer's point is sharper than my framing: because the authored Load Tlut encoding was wrong, our decoder was reading TMEM that a CORRECT load would not have produced. So the probe shows the index-to-palette mapping resolves against whatever we loaded - which makes a totally-dead decoder unlikely - but it does not establish the load path. CI4 is weakly de-prioritised, not eliminated, and the entry says so. The bit-position claim now carries its citation, and the field carries the wiki's own name. It is tlut_en, not en_tlut: N64brew Reality Display Processor/Commands 0x2F, bit 47, "tlut_en: Enables Texture Look-Up Table (TLUT) sampling", with tlut_type at bit 46 selecting RGBA16 vs IA16. I had asserted the position from memory; it happened to be right, which is not the same as having checked. PR title rewritten imperative per conventional commits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Measured, Never Tuned — ADOPTED, and it caught a real lapse. I asserted Checked it. The position was right; the name was not, and "happened to be right" is not the same as "verified":
The field is now called Title check — ADOPTED. Retitled imperative: "record the provisional CI4 probe result and the tlut_en gap", which also now describes what the PR concluded rather than the stronger claim it originally made. |
Antigravity review (Gemini via Ultra)This PR updates Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
|
Both remaining pre-merge warnings are stale: CodeRabbit last reviewed at Merging on that basis. CI 9/9, all threads resolved, no outside-diff findings. |
1 participant
The probe ran, and it eliminates the suspect
The previous entry named a CI4-with-TLUT vector as the next probe. It has been run.
A vector was authored — eight CI4 indices
0..7against eight distinct,non-zero palette entries — and replayed through RustyN64:
RustyN64 resolves the full palette correctly. The colour-indexed fetch works
in isolation and is not what turns Ocarina black.
Why the vector is deliberately NOT committed
Angrylion rendered mostly
0x0001from the same command list:That is a near-empty palette — which means my
Load Tlutencoding is wrong,not that our decoder is. Angrylion faithfully executed the command list I wrote;
we were simply more permissive about where the entries came from.
Committing that golden would pin an authoring error as the spec, which is the
one thing a test-as-spec project must not do. The next probe has to verify the
Load Tlutencoding against N64brew …/Commands before any golden is trusted.A separate real defect, found along the way
Other Modes.en_tlut(command bit 47) is not decoded at all. Our TLUT lookupis driven purely by the tile's format field, so:
en_tlutclear still gets a palette lookup (it should not), anden_tlutset does not (on hardware it should).Wrong in both directions, and independent of R-18. Recorded rather than fixed here
— fixing it needs its own vector, and the
Load Tlutencoding has to be settledfirst so that vector is trustworthy.
Net effect on R-18
The black frame is narrowed to the texture path but no longer to the CI4
decoder. That is a smaller claim than the previous entry made, and it is the
honest one.
Gates
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace(38 RDP conformance vectors still green),pre-commit run markdownlint --all-files— each run separately, exit status checked.Oracle: docs-only, no emulation behaviour change, so n64-systemtest is
unchanged at 90 suite-wide, Phase 1
Failed: 0, perdocs/STATUS.md.🤖 Generated with Claude Code