docs(overhaul): S4-c2 review round 1 — retail lens FAIL recorded; fix round 1 contract (packet §11, M1–M8)

The two-list/router/flush core of 048d5b12f is exact at the bytes; the
failures are at the call sites (a data-driven row 5 turned into a throw;
detail-surface input hardcoded false while the drain applies detail),
the register (the structurally empty CLIP list is undeclared; AP-237's
example is wrong; AP-34 not retired by convention), the first-for-list
quantifier, the all-KnownFailure gate, and the skipped C4 that turns out
to be bounded.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-04 06:14:53 +02:00
parent 5f3b4f98ee
commit 3813f0bbf2
2 changed files with 40 additions and 1 deletions

View file

@ -458,3 +458,42 @@ PASS/FAIL with file:line findings; two fix rounds maximum, then the ledger.
**Gates (all reproduced by the lenses, none claimed vacuously):** Release build 0 warnings; hermetic lane green; InstalledDat lane with ONLY the pre-existing #383 ×2, TowerAscent, #458 KnownFailure failures (terrace-edge must no longer be among them); all six transcript Facts green; the shader classes green. One commit on top of `2bd353234` in `s4-c1-impl`.
**Lenses (sequential, session model, narrowed to the round-2 diff):** retail-faithful (R2-3's predicate and R2-2's seeding argued from the bytes/decomp — the counter's persistence across frames at `PView::DrawCells` and the reset site), gate-honesty (every sentence in the commit body and every test doc comment is something the tests evaluate; both sequences compared together), production (no allocation/behavior change outside the test project except R2-5(a) comments).
> §10 (the implementer's S4-c2 landing note) lives in the chunk commit `048d5b12f` in worktree `s4-c2-impl` and arrives with the cherry-pick; §11 below was written on the campaign branch so the fixer reads it from here.
## 11. S4-c2 fix round 1 (lead, 2026-09-04 — after the two-lens review of `048d5b12f`; round 1 of the two the plan allows)
**Verdicts on `048d5b12f` (worktree `s4-c2-impl`, on `766f9e749`):** retail-faithful lens
FAIL (one blocking, four major, three minor — every predicate of the core port confirmed exact
against the bytes: `AddMeshToAlphaList` @0x0059C230 list select/capacity/return,
`FlushAlphaList` @0x0059D2E0 early-return `test ah,5; jp` = strict-less on BOTH counts so 2250
drains, CLIP-then-ALPHA drain, `DrawMesh` @0x0059D4A0 rows 15, the four call sites' bytes and
symbol ranges, `::flush` = 0.75f @0x820ed0, `s_AlphaDelayMask` = 0x0E @0x820d88,
`MultiPassAlpha` = 0 @0x81ef96, `alwaysDrawObjects` = 1 @0x820ed4, `SortCellExit` placed at
DrawBlock's `0x5a19e6 DrawSortCell → 0x5a1a07 FlushAlphaList` under the same gate); lead
verification agrees on every one of those and adds the C4 ruling. Production/gate lens: see
the addendum at the end of this section when it reports. The failures are at the three call
sites, the register, and the gate's CI value — the list/router/flush core stands.
**Items (each is a contract; quote decomp predicates, never paraphrase — `feedback_quote_decomp_predicates`; every new or changed pin carries a mutation text in the commit body — `feedback_every_new_pin_must_be_shown_to_fail`):**
**M1 (BLOCKING) — the particle site throws on a data-driven row.** `ParticleRenderer.TryAppendMeshDraws` (`ParticleRenderer.cs:645-663`) appends EVERY `renderData.Batches[i]` with `IndexCount > 0` and an assigned texture; `ObjectRenderBatch.Translucency` is the mesh classification (`ObjectMeshManager.cs:2253`), so `TranslucencyKind.Opaque` is an ordinary value for a `FullMesh` particle GfxObj → `MaskFromTranslucencyKind(Opaque) = 0x00``Route(..., mask 0x00, materialHasAlpha:false)` → row 5 `Immediate` → the `InvalidOperationException` at `ParticleRenderer.cs:361-372` fires in the render loop. Retail defines row 5 (`0x0059d58c RenderMeshSubset`, draw now) and row 4 (`(delayMask & 4) != 0 && material != 0 && *(material+8) != 0` → append ALPHA). Fix: (a) delete the throw at BOTH sites (`ParticleRenderer.cs:361-372`, `WbDrawDispatcher.SubmitToAlphaQueue`) — a router row is a data-driven outcome, never an invariant (`feedback_retail_dispatch_is_data_driven`); (b) for particles derive `materialHasAlpha` from retail's particle material: find where the emitter's `CMaterial` is created/`translucency` set (grep `ParticleEmitter::` / `CParticle` / `CMaterial::has_alpha` in the named pseudo-C; `has_alpha` is the field at `+0x8` DrawMesh row 4 reads) and QUOTE the rule (expected: a particle whose translucency < 1 or whose material carries alpha has `has_alpha != 0` row 4 ALPHA; a fully opaque particle material row 5 immediate); (c) implement the immediate outcome for rows 1/5 at the particle site by drawing the submission now through the existing non-deferring particle draw path (the same code `DrawOrdered`/`DrawOrderedRhi` uses for one submission) no new renderer; (d) pins: an Opaque-classified mesh-particle batch with has_alpha routes to ALPHA and is drawn at the flush; the same batch with no material alpha draws immediately and never enters a list; mutation text for each.
**M2 (MAJOR) — the CLIP list is structurally unreachable for ordinary content, undeclared.** `WbDrawDispatcher.IsOpaque(t) => t == Opaque || t == ClipMap` (`WbDrawDispatcher.cs:3567`) keeps every clip-mapped subset out of `_translucentDraws`/`SubmitWalkAlphaInstance`; retail's `ConstructMesh` gives them mask 0x08 → row 3 → CLIP, and the six captures show 4,183 of 10,556 `AM` lines on CLIP (e.g. the §10 quotes `(DrawBuilding,0,158,452)`: 158 CLIP entries acdream reports as 0). Fix (no pipeline change in this round — the alpha-to-coverage treatment of clip-mapped surfaces predates S4 and is a VisualMaster-era translucency decision): file a NEW AP row stating exactly this — clip-mapped GfxObj/scenery/building subsets draw immediately on the `OpaqueAlphaToCoverage` pipeline and never enter retail's CLIP FIFO; consequence = compositing in draw-call order instead of retail's deferred CLIP drain (depth-writing cutouts, so ordering is mostly invisible; edges differ by the older alpha-to-coverage choice); the only CLIP-list feeders left are clip-mapped mesh PARTICLES; cite `D3DPolyRender::ConstructMesh` 0x0059DFA0 / `DrawMesh` 0x0059D4A0 row 3 / `AddMeshToAlphaList` 0x0059C230 — and correct AP-236's sentence "the ONE residual AP-34 documented that this chunk does NOT fix" (false: there are two, this and EnvCell).
**M3 (MAJOR) — contract C3's detail-surface input is hardcoded false and the drain applies detail.** Retail `DrawBuilding` @0x0059f2a0: `0059f2eb curr_detail_surface = building_detail_surface; 0059f30b FlushAlphaList(0f); 0059f31d CPhysicsPart::Draw(parts,1); 0059f336 CPhysicsPart::Draw(parts,0); 0059f345 curr_detail_surface = nullptr` → with a non-null building detail EVERY shell subset takes row 1 (immediate, WITH detail); the delayed replay always passes `detailEnabled = 0` (`FlushAlphaList` @0x0059d3c7/@0x0059d45d call `RenderMeshSubset(..., 0, entry.clip, ...)`). acdream: `SubmitToAlphaQueue` passes `detailSurfaceActive:false` while building shells DO reach the alpha path (`WalkClassify.cs:310 detailCategory = entity.IsBuildingShell ? 1u : 0u`) and `DrawPreparedAlphaBatchRhi` (`WbDrawDispatcher.Rhi.cs:642-694`) runs the detail pass ON THE DRAIN — the exact state spec §4 says replay never has. Fix: (a) router input `detailSurfaceActive = entity.IsBuildingShell && RetailDetailTextureContract.ShouldRender(_buildingDetailEnabled(), _buildingDetail)` at both Wb sites (the walk site knows the entity; `DeferTransparentGroups` must carry the category per instance — it already has `DetailCategories`); (b) row 1 for a translucent building-shell instance = draw it NOW at its own turn (adaptation: retail draws the subset in place inside the mesh; the walk's opaque instances are stream-batched, so "now" = at the entity's stream mark, i.e. when Replay reaches that instance's `AlphaSubmitMark`) through a new single-instance immediate draw on the alpha source — bind the blend pipeline, one indirect draw, then the detail pass — reusing `DrawPreparedAlphaBatchRhi`'s pipeline/detail code; (c) the drain never applies detail: delete the `hasDetail`/`DrawBuildingDetailRangeRhi` branch from `DrawPreparedAlphaBatchRhi` (retail replay is `detailEnabled = 0`); (d) a register row for the "in place → at the entity's stream mark" granularity adaptation; (e) pins: a translucent building-shell instance with building detail ON draws at its turn with the detail pass and never enters a list; with detail OFF it is appended and drained WITHOUT detail; an ordinary (non-shell) translucent instance is unaffected by the detail flag; mutation texts.
**M4 (MAJOR) — `IsFirstForList` has the wrong quantifier.** Retail: `0059d4cc arg3 = 1; 0059d4d0 var_c = 1` at `DrawMesh` ENTRY, cleared after the first append to each list within THAT call (`0059d5ef if (var_4_1 == 0) var_c = 0; else arg3 = 0;`); spec §5: "Each DrawMesh invocation owns two independent first-for-this-list flags"; the captures: `new=1` on 9,685 of 10,556 `AM` lines — impossible under "first since the last drain" (`RetailAlphaQueue.TryAppend` `isFirstForList = target.Count == 0`). Each acdream append is one instance = its own DrawMesh call, so the truthful port is `IsFirstForList = true` on every append — or delete the field. Fix: choose one, fix the doc comments (`RetailAlphaEntry`, `RetailAlphaQueue.TryAppend`, `WalkAlphaDepthTrace`'s "new" comment), delete or rewrite `TryAppend_FlagsOnlyTheFirstEntrySinceTheLastDrain` (a pin of the wrong semantic), mutation text if a pin remains.
**M5 (MAJOR) — gate G-c2 has no CI signal.** All six `AlphaFlushTranscript_*` Facts are `Status=KnownFailure`, so the ONE dimension the harness proves (the (site, threshold) sequence) is excluded from the hermetic lane and expected-red in InstalledDat; a future site-sequence regression is invisible. Also the sequence contains zero `SortCellExit` entries in every pose (0 < 2250 on both sides), so the gate exercises nothing the chunk added. Fix: split over the SAME replay into `AlphaFlushSites_<pose>_MatchesRetailFrame2` (live in the lane, asserts (site, threshold) only) and `AlphaFlushCounts_<pose>_MatchesRetailFrame2` (KnownFailure). Rewrite the KnownFailure reason truthfully in the Facts' doc comments AND packet §10: the counts can never match by construction (1) this harness has no mesh content, (2) acdream appends one entry per INSTANCE where retail appends one per SUBSET per DrawMesh, (3) acdream's CLIP list is empty for ordinary content (M2) not "harness content only". Add ONE live pin that exercises the valve the chunk added: a driver test where 2250 entries sit in a list when the SortCellExit event replays and the drain fires there (and 2249 does not) `RetailAlphaQueueTests` has the boundary; this one goes through `WalkFrameDriver.Replay`'s `SortCellExit` arm; mutation text.
**M6 (MAJOR) — contract C4 (EnvCell) is smaller than claimed; do it.** Retail `DrawEnvCell` @0x0059f1c2 sets `curr_detail_surface = environment_detail_surface` (may be null) before `DrawMesh` @0x0059f212 and clears it @0x0059f21a: detail ON → row 1 immediate (acdream already matches — `EnvCellRenderer.Rhi.cs:254 detailEnabled`); detail OFF → row 3 append (mask from the cell surface: alpha-family → 0x02 ALPHA). `EnvCellRenderer.RenderTransparentOrdered(IReadOnlyList<uint>)` (`EnvCellRenderer.cs:828`) already replays a cell's transparent batch on demand, so no "new deferred-replay abstraction" is needed. Fix: an EnvCell `IRetailAlphaDrawSource` owned by `RetailPViewPassExecutor`: `WalkProductionLeafRenderer.DrawCellShell` (`RetailPViewPassExecutor.WalkLeaf.cs:262`) appends ONE token (the cellId) to ALPHA when `detailEnabled == false` (route through the router with the shell's mask so the decision is the table's, not a hand-coded branch), draws immediately when `detailEnabled == true` (today's path); `PrepareAlphaDraws` collects the cell ids in token order; `DrawPreparedAlphaBatch(first, count)` calls `RenderTransparentOrdered` on that slice. One token per cell is coarser than retail's per-subset entries — AP-236 shrinks to exactly that granularity residual (rewrite the row; drop its "substantial separate architecture change" text). Pins: detail-off cell shell is drained at the next AlphaBarrier/LandscapeFlush/pass end and never before; detail-on draws at its turn; a particle appended between two cell tokens keeps its position (adjacent-only batching); mutation texts.
**M7 (MINOR) — AP-237's only cited example is disproven.** `TranslucencyKind.cs:65-68` records cloud surface `0x08000023` as `Type=0x10114` = ADDITIVE|ALPHA|TRANSLUCENT|BASE1_CLIPMAP; `0x10114 & 0x10300 ≠ 0` → retail mask 0x02 → ALPHA — identical to the port. The abstract class (Translucent+ClipMap with NO alpha-family bit) may still exist: run a DAT scan over every Surface (`SurfaceType` bits: Translucent 0x10, Base1ClipMap 0x04, alpha family 0x00010300) and either cite a real instance (id, type, where used) or rewrite AP-237 as "no known instance in the installed DATs (scan date, count scanned)"; delete "extremely narrow… only known example is the cloud GfxObj".
**M8 (MINOR) — citations and register convention.** (a) `RenderDeviceD3D::DrawBlock` is @0x005a17c0 (symbols.json); 0x005a18d0 is its loop head — fix `RetailAlphaQueue.cs:41`, `RetailPViewPassExecutor.cs:350`, `WalkEvents.cs:159`, `WalkFrameDriver.cs:242,1479`, `RetailFrameWalk.cs:250`. (b) `RetailAlphaQueue.cs:39-50` label the CALL instructions (0x0059f30b / 0x005a1a07 / 0x005a4872 / 0x00453b8b) as "return site"; the return addresses are +5 (0059f310 / 005a1a0c / 005a4877 / 00453b90) — say "call at … (returns to …)". (c) Register: keep `~~AP-34~~` struck through with "RETIRED 2026-09-04 (S4-c2) — residuals AP-236 / AP-237 / <M2's new row>" per the register's own convention (see `~~AP-28~~`, `~~AD-23~~`), fix AP-236's lead sentence ("retired, not narrowed" contradicts its content), and update the "## 3. Approximation (AP) — N active rows" header count. (d) `docs/architecture/worldbuilder-inventory.md:420` cites AP-34 — update it (that file is added to this round's allowed list).
**Gates (real output lines in the commit body; a lens reproduces each):** Release build 0 warnings; hermetic lane green; InstalledDat lane = the four pre-existing failures + the six `AlphaFlushCounts_*` KnownFailure Facts (and NOTHING else — the six `AlphaFlushSites_*` Facts pass in the lane); shader classes; `RetailAlphaQueueTests`, `RetailAlphaMeshRouterTests`, `RetailFrameWalkTests`, `WalkFrameDriverTests`, `ParticleRenderer*Tests`, `EnvCellRenderer*Tests` classes green. Allowed files: src/AcDream.App, tests/AcDream.App.Tests, docs/architecture/retail-divergence-register.md, docs/architecture/worldbuilder-inventory.md (line 420 only), this packet (§10 correction + a "§11 result" note). One commit on top of `048d5b12f` in `s4-c2-impl`. Must NOT: reintroduce any distance; change the alpha-to-coverage classification of clip-mapped surfaces (M2 is a register row this round); touch S4-c1's punch/seal machinery; add flush sites.
**Lenses after the round:** retail-faithful (narrowed to M1/M3/M6's decomp claims and the router inputs) and production/gate-honesty (the six live Sites Facts, the valve pin, the immediate-draw paths' allocation, the register). A third round stops the chunk (plan §5).