docs(ledger): narrow the mirrored-text defect; two suspects cleared (R-18) - #193
Conversation
…R-18) Checked before guessing, and both came back clean: the Set Tile (0x35) decode of mirror_s is correct (bit 8, matching N64brew), and mask_coupled implements the mirror fold and its neighbour-diff seam. So the flip is not a mis-decoded tile flag. Texture Rectangle Flip (0x25) is separately unimplemented in COPY mode -- it bails on flip || tile.size != 2 || color_image_size != 2 (R-8) -- but that cannot be the explanation alone, because a bail-out renders nothing and the mirrored logos do appear. Something is applying a flip on a path that renders. The negative control constrains it further: WCW/nWo Revenge's THQ logo is correctly oriented while WCW vs. nWo World Tour's banner is flipped, so the defect is per-draw, not global. Next step is an Angrylion vector rather than a patch -- a texture rectangle with mirror_s set, and a second with the 0x25 opcode -- which distinguishes "we mirror when we should not" from "we fail to mirror and the game pre-flips". Guessing between those without the oracle is how three earlier localisations in this row went wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 9 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)
Comment |
Antigravity review (Gemini via Ultra)This PR updates Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
Adjudication — Antigravity reviewNo blocking issues. Both suggestions concern the same thing, and my answer has changed. Extract the R-18 investigation log — accepted in principle, tracked, not done hereThis is the second time it has been raised independently (also on #188), and the row is now ~40 KB in a single Markdown table cell. Two reviewers arriving at it separately is evidence rather than opinion, and I am not going to reject it a third time with the same reasoning. What I said before still holds as far as it goes: R-18 is append-only by design, and that property is what makes it trustworthy — this PR is itself an example, since it records two cleared suspects, and an earlier one records a retraction that only reads correctly because the wrong claim sits beside the measurement that killed it. Extracting must preserve every superseded claim verbatim, not summarise it. But that is an argument about how to extract, not whether. What I am still not willing to do is migrate R-18 alone, which would leave the ledger inconsistent about where provenance lives — some residuals inline, one behind a link. The right shape is: move all long residuals to That is a self-contained refactor with a real risk of dropping content, and it should not ride along with a docs change about a texture defect. Tracked as its own task; it will get its own PR. Hyperlink the inline
|
CodeRabbit caught a real document defect. The R-18 row contained unescaped pipes inside inline code spans -- `flip || tile.size != 2 || color_image_size != 2`, introduced in #193, plus a "WHITE | WHITE" in the mirror result added in this branch. Markdown code spans do NOT protect pipes inside a table, so those split the row into extra cells; MD056 reports "Expected: 5; Actual: 9; Too many cells, extra data will be missing" -- meaning content at the end of the row was being dropped from the rendered table. That is the ledger silently losing the very provenance it exists to hold, in a row I have spent this session appending measurements to. Fixed by escaping the operators as \|\| and rephrasing the mirror result to avoid the separator entirely. markdownlint is green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sis (R-13) (#194) * feat(rdp): pin mirror_s against Angrylion -- and refute my own hypothesis (R-13) Authored a vector instead of guessing a patch, and it settled the question against the hypothesis it was written to confirm. tex_tri_mirror_s_16 (V40) renders four distinct 16-bit texels with mask_s = 2 and mirror_s = 1 across EIGHT columns, so the fold is unmistakable. Angrylion emits RED GREEN BLUE WHITE | WHITE BLUE GREEN RED, and RustyN64 matches it byte-for-byte. Eight columns are load-bearing. A four-column render samples only the forward half, where mirrored and non-mirrored output are identical -- the same converging-paths trap that let an earlier CI4 vector declare eight palette entries while exercising six. Mutation-checked by replacing the fold (s ^ -wrap) with a plain mask, which names the vector directly: "vectors diverged from the Angrylion oracle: [rdp-conformance/tex_tri_mirror_s_16]". Note the first mutation attempt looked like the test was insensitive; it was a filter artefact -- the vector-replay assertion lives in default_battery_matches_the_oracle, not in the tests matched by a "conformance" filter. So the mirrored text in GoldenEye 007 and WCW vs. nWo is NOT the S-axis mirror path, and the note recording that as the likely cause is superseded. Third suspect cleared by measurement here, after the Set Tile decode and mask_coupled. Texture Rectangle Flip (0x25) is unimplemented in COPY mode but bails rather than rendering flipped, so it is not it either; the live candidates are a negative DsDx and the triangle edge/winding path. The vector is committed regardless of the refutation -- it is permanent coverage for a path that had none. Battery is now 54 probes (41 RDP + 13 VI); STATUS updated. Gates: fmt, clippy -D warnings, cargo test --workspace, rustdoc -D warnings, markdownlint -- all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(ledger): escape the pipes breaking the R-18 table row CodeRabbit caught a real document defect. The R-18 row contained unescaped pipes inside inline code spans -- `flip || tile.size != 2 || color_image_size != 2`, introduced in #193, plus a "WHITE | WHITE" in the mirror result added in this branch. Markdown code spans do NOT protect pipes inside a table, so those split the row into extra cells; MD056 reports "Expected: 5; Actual: 9; Too many cells, extra data will be missing" -- meaning content at the end of the row was being dropped from the rendered table. That is the ledger silently losing the very provenance it exists to hold, in a row I have spent this session appending measurements to. Fixed by escaping the operators as \|\| and rephrasing the mirror result to avoid the separator entirely. markdownlint is green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: clear two contradictions the mirror refutation left behind CodeRabbit caught both, and both are the same failure: a conclusion updated in one place and left standing in another. screenshots/README.md exonerated the S-axis mirror path in one paragraph and then, four lines later, still concluded "the defect is in how a particular tile's mirror_s is resolved". Rewritten so the per-draw observation and the oracle result read as one argument, ending on the actual live candidates -- a negative DsDx, or the triangle edge/winding path. docs/STATUS.md said "53 committed Angrylion vectors" at line 216 while the rows this branch updated say 54, and lines 262-266 listed both "an RDP-driven real-ROM frame" and "the accuracy battery" as not yet true. Both are true now: the battery executes at 54 probes / 100%, and Super Mario 64 renders its title screen through the full LLE path. Corrected, and the correction says which claims it supersedes rather than quietly replacing them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Docs-only. Narrows the mirrored-text defect found by inspecting the screenshot captures, and clears two suspects by checking rather than guessing.
Cleared
Set Tile(0x35)mirror_sdecode is correct — bit 8 of the low word,(lo >> 8) & 1, matching N64brew …/Commands §0x35.mask_coupledimplements the mirror fold (s ^ -wrap) and its neighbour-diff seam handling.So the flip is not a mis-decoded tile flag.
Not the explanation either
Texture Rectangle Flip(0x25) is unimplemented in COPY mode —texture_rectangle_copybails onflip || tile.size != 2 || color_image_size != 2(R-8). But a bail-out renders nothing, and the mirrored logos do appear. So something is applying a flip on a path that renders.The negative control constrains it
Per-draw, not global.
Next step: a vector, not a patch
An Angrylion vector with
mirror_sset, plus a second using the 0x25 opcode, compared byte-for-byte. That distinguishes "we mirror when we should not" from "we fail to mirror and the game pre-flips" — two hypotheses with opposite fixes.Guessing between them without the oracle is precisely how three earlier localisations in this row went wrong (the exception vector, the presentation path, the CI4 decoder), so this lands as a narrowed lead with its evidence rather than a speculative change.
Gates
markdownlint — green. Docs only; no behaviour change.