acdream/docs/research/2026-08-11-op2-rereview-blast.md
Erik 5242de9f15 fix(ui): OP2 re-review closure (coordinator pass) — AP-195, tooltip port, zero-children pin
Closes the mechanism lens's REOPEN (one MUST-FIX) and both lenses' small
residuals on the OP2 rework (b236a442); the blast lens re-review was
CLOSED outright. Fable-direct per the two-failure escalation rule.

- AP-195 filed: UIOption_CheckboxBitfield64 ports HALF of Refresh
  @0x004859C0 — the ANY-set checkbox predicate is exact, but the ALL-set
  LED media swap (P0x10000082=0x06004D17 / P0x10000083=0x06004D19) and
  the ListBox self-sizing tail (ResizeTo/CalculatePaperSize — the block
  IS a UIElement_ListBox in retail) are unported, and the block's row
  stacking is a second divergent implementation beside UiTemplateListBox.
  All due at OP5 before the Chat tab's connected gate; the IsSet doc
  comment now names both halves instead of quoting only the ported one.
- Row tooltips: UiButton gains settable TooltipText surfaced through the
  shared GetTooltipText hover pipeline (UiCatalogSlot's pattern);
  UiCheckboxBitfield64.AddChild applies the row tooltip retail stamps in
  CreateChildren @0x00485DF0, and documents that the 0x10000084 row-index
  attribute stamp is deliberately replaced by the typed mask closure.
- AD-73 addendum: UiTemplateListBox.ConsumesDatChildren=true is inert
  only while no authored Type-5 element carries children — that premise
  is now conformance-PINNED across all 32 fixtures (a future DAT
  regeneration surfacing an authored child fails the build instead of
  silently dropping it).
- Plan doc: OP2's contract names UiTabPanel.cs (retail UIElement_Panel),
  not the fictional-class-named UiTabControl.cs; ledger records OP1 and
  OP2 both CLOSED.

Full Release suite: 12,871 passed / 4 skipped / 0 failed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-11 01:26:29 +02:00

27 KiB
Raw Permalink Blame History

OP2 rework re-review — regression / blast-radius lens

Rework under review: b236a442 "fix(ui): OP2 rework — dormant UiDatElement subclasses, fixed Panel/CheckboxBitfield64 mechanism" Original (REJECTed): df9c7a35; round-1 findings docs/research/2026-08-11-op2-review-blast.md Reviewer lens: what OP2 breaks OUTSIDE its own scope (Options panel). Date: 2026-08-11 Verdict: CLOSED — all three MUST-FIXes and all three SHOULD-FIXes are discharged, verified against the committed data rather than against the commit message. Four residuals, none blocking: 1 SHOULD-FIX (an unpinned premise), 3 NOTEs.


0. Executive summary

The rework replaces "widen the factory arm and hope" with a dormancy model: UiDatElement is unsealed (src/AcDream.App/UI/Layout/UiDatElement.cs:43), and both behavioral widgets now derive from it — UiTabPanel : UiDatElement (src/AcDream.App/UI/UiTabPanel.cs:79) and UiTemplateListBox : UiDatElement (src/AcDream.App/UI/UiTemplateListBox.cs:81). The Type-8 and Type-5 factory arms are unconditional again (src/AcDream.App/UI/Layout/DatWidgetFactory.cs:106, :119), which is now safe rather than lucky, because an un-activated instance inherits the exact UiDatElement media draw, ClickThrough = true default, and IUiDatStateful propagation the pre-OP2 fallback provided, and performs zero import-time work (UiTabPanel.cs:126-128, the empty OnChildrenAttached).

That is the right shape. It removes the class of defect instead of narrowing a guard, and it does so without the "gate on the Options LayoutDesc ids" workaround round 1 offered as one option.

All three MUST-FIXes verified closed against primary data, not against the commit message:

  • The five Type-8 elements are behaviorally identical to pre-OP2, and the six new pins assert the failure modes (media file id non-zero, ClickThrough true, ActivePageElementId == 0, child count 16, all children Visible), not just widget class names.
  • The ten Type-5 elements gain no viewport child. Verified independently: all 16 Type-5 elements across all 32 fixtures have zero children, so the one genuine remaining delta (ConsumesDatChildren => true) drops nothing today.
  • All 32 fixtures were regenerated and committed. Verified by structural diff: all 1,102 nodes across all 32 files carry the five new fields, and the regeneration changed no pre-existing field value in any of the 27 previously stale fixtures — zero drift, in all 27, not just the 5 spot-checks requested.

Two of round 1's own findings were factually wrong and the rework absorbed one of them without correcting the record. See §5. This matters because SHOULD-FIX 2's stated rationale (base-vs-incorporated id mismatch) is refuted by the raw dat data, and MUST-FIX 1(d)'s "mitigating: it is luck, three of four panels don't resolve" was the inverse of the truth — every tab-table id resolves as a direct child, so df9c7a35 was strictly worse than round 1 assessed. The rework's dormancy fix is correct either way; only the reasoning under SHOULD-FIX 2 needs correcting.


1. Method

  • Parsed all 32 committed fixtures under tests/AcDream.App.Tests/UI/Layout/fixtures/ at b236a442, walked every ElementInfo node (1,102 total), and recorded Type, child ids, raw dat properties 0x2E/0x64/0x72, StateMedia, and new-field presence.
  • Structural deep-diff of every one of the 27 pre-existing fixtures, df9c7a35b236a442, with the five new field names stripped, to isolate "did regeneration change anything OLD?"
  • Cross-checked the raw 0x2E struct members (0x30/0x31/0x32) against the projected TabTable field, per element, to settle the base-vs-incorporated-id question with data.
  • Read the widget/factory/importer sources and every new or changed test.
  • No build, no test run, no launch (read-only mandate).

2. MUST-FIX closure

MUST-FIX 1 — five Type-8 elements re-classed — CLOSED

Every one of the five is now behaviorally identical to pre-OP2. The mechanism is inheritance, not per-element special-casing: UiTabPanel adds no OnDraw override, no ClickThrough write, no ConsumesDatChildren override, and its IUiChildrenAttachedListener.OnChildrenAttached body is empty (UiTabPanel.cs:126-128).

Element Round-1 defect Status Evidence
vendor 0x1000008D lost DirectState backdrop fill fixed fixture StateMedia[""] = (100687632, DrawMode 1); UiDatElement ctor picks "" (no DefaultStateName, no Normal) → ActiveMedia() returns it → UiDatElement.OnDraw (UiDatElement.cs:208) draws it
character 0x10000227 ClickThrough true→false; lost state propagation fixed ClickThrough = true set in UiDatElement.cs:126, never overwritten; TrySetRetailState is non-virtual (UiDatElement.cs:86) so a subclass cannot alter propagation
spellbook 0x100002A8 same fixed same
vendor 0x100000B8 same fixed same
combat 0x100000A2 import-time takeover (8-entry table, no controller re-binder) fixed OnChildrenAttached is empty; ActivateTabBehavior has no caller anywhere in src/

Do the new pins assert failure modes rather than class names? Yes, on three of the four axes, with one weaker:

  • Media drawnOP2ReworkBlastRadiusConformanceTests.cs:48-49: (uint file, int _) = backdrop.ActiveMedia(); Assert.NotEqual(0u, file); This is the actual draw input, not a class name.
  • Click-through:47, :67, :90, :106, :155: Assert.True(...ClickThrough) on all five Type-8 elements plus the representative Type-5. This is the exact field UiElement.HitTest gates on.
  • No visibility flip / no takeover:129-131: Assert.Equal(0u, host.ActivePageElementId), Assert.Equal(16, host.Children.Count), Assert.All(host.Children, child => Assert.True(child.Visible)). That is the observable failure mode (a hidden page), not a class name.
  • State propagation:91, :107: Assert.IsAssignableFrom<IUiDatStateful>(root). This is a capability assertion, not a behavior assertion: it does not build a PassToChildren state and check a child received it. Acceptable-as-is because TrySetRetailState/ActiveRetailStateId are non-virtual on UiDatElement (:86, :69), so a subclass structurally cannot diverge — but it is the one pin of the four that would still pass if the mechanism broke in the base class. Recorded as NOTE 1, not a residual defect.

Combat's specific claim ("no controller rebind") re-verified: grep of src/AcDream.App/UI/Layout/CombatUiController.cs for 0x100000A2 / 0x100000A3 returns nothing.

MUST-FIX 2 — ten Type-5 elements — CLOSED

The viewport is now created lazily inside the Viewport property (UiTemplateListBox.cs:147-162), reached only from Scroll and from a successful AddItemFromTemplateList (:182). DatWidgetFactory leaves TemplateResolver null (:106 passes no resolver), and AddItemFromTemplateList returns before touching Viewport when the resolver is null (:175-176) or the index is out of range (:174). So no pre-existing Type-5 element can allocate or attach the viewport.

Child structure verified identical to the old UiDatElement build — with one qualifier the class doc names and I confirmed:

old UiDatElement new dormant UiTemplateListBox
viewport child none none (lazy)
authored dat children built and attached (LayoutImporter.cs:129-136) skippedConsumesDatChildren => true (UiTemplateListBox.cs:145)

That second row is the only place the "behaviorally identical" claim is not literally true. It is harmless today, and I verified it directly rather than taking the doc's word: across all 32 fixtures, all 16 Type-5 elements have zero Children. Incorporation (LayoutImporter.ResolveIncorporateChildren, LayoutImporter.cs:352) happens in the ImportInfos phase, so the committed fixture child tables are the final production child tables — this is not a fixture-vs-production gap. Nothing is dropped, and no id disappears from byId.

The premise being unpinned by any test is SHOULD-FIX A below.

EffectsUiControllerTests' updated assertion is correct, not loosened. The diff changes Assert.IsType<UiDatElement>Assert.IsType<UiTemplateListBox> (exact-type, xUnit IsType is not assignability), and the line immediately after is unchanged and is the load-bearing one: UiItemList list = Assert.IsType<UiItemList>(Assert.Single(listHost.Children)); Assert.Single is exactly the assertion that fails if a viewport is injected alongside the controller's own UiItemList. The pin got stronger in effect, not weaker.

MUST-FIX 3 — harness blindness — CLOSED

Three independent checks, all measured:

  1. All 32 fixtures regenerated and committed. git show --stat b236a442 lists all 32 fixture files as modified; 27 of them carry substantial line counts (e.g. character_2100002E.json +1001, combat_21000073.json +741).

  2. All 27 previously-stale fixtures now carry the five new fields. Verified node-by-node, not file-by-file: 1,102 / 1,102 nodes across all 32 files carry TabTable, TemplateList, ScrollbarElementId, Outline, and OutlineColor. Zero nodes missing any of the five.

  3. The regeneration introduced ONLY the expected field additions. The task asked for a spot-diff of at least 5 of the 27; I ran the structural deep-diff on all 27, stripping only the five new field names and comparing every remaining key/value recursively:

    character_2100002E.json           : 0 non-new-field differences  CLEAN
    combat_21000073.json              : 0 non-new-field differences  CLEAN
    spellbook_21000034.json           : 0 non-new-field differences  CLEAN
    vendor_21000012_100000B7.json     : 0 non-new-field differences  CLEAN
    effects_positive_2100001B.json    : 0 non-new-field differences  CLEAN
    … and the other 22, all CLEAN (0 differences each)
    

    No pre-existing value drifted. The hazard the task named — "a changed pre-existing value would mean the generator's output changed for OLD fields" — did not occur anywhere. (chat_2100006f.json, the one fixture that already carried Outline/OutlineColor from CH6a, is included in that sweep with the same result on its other fields.)

  4. The suite genuinely exercises the new-shape fixtures. ElementInfo's three new members are public fields (ElementReader.cs:192, :199, :207) and FixtureLoader deserializes with IncludeFields = true (FixtureLoader.cs:14-17), so the fixture path now reads real projected values. Concretely: OP2ReworkBlastRadiusConformanceTests.cs:64 asserts host.Tabs.Count == 3 and :127 asserts == 8 — values that were empty under the stale fixtures. The structural false negative is gone.

  5. The reader itself is now covered independently of the fixtures. The remaining structural gap round 1 named — ApplyCanonicalLegacyProjection is never called on the fixture path (LayoutImporter.cs:465, ElementReader.cs:410 are still its only call sites) — is addressed by five new tests in ElementReaderTests.cs that drive ApplyCanonicalLegacyProjection directly from synthetic raw property bags. A reader regression now fails a test even without regeneration. That is the right seam.


3. SHOULD-FIX closure

SHOULD-FIX 1 — unconditional viewport injection — CLOSED

Lazy Viewport property (UiTemplateListBox.cs:147-162), pinned in both directions: OptionsPanelLayoutConformanceTests asserts Assert.Empty(listBox.Children) after a failed/never-attempted add, and Assert.Single(listBox.Children) after the one successful add. ConsumesDatChildren => true also resolves round 1's "authored children become siblings of the viewport" trap for OP3+ — authored children are now never built for a Type-5 element, so the sibling-vs-row ambiguity cannot arise.

SHOULD-FIX 2 — base-vs-incorporated tab-table ids — CLOSED, on a corrected premise

FindDirectChildFindDescendant, a pre-order DFS of the whole subtree (UiTabPanel.cs:208-219), correctly cited to retail GetChildRecursive.

Round 1's stated rationale was factually wrong. I read the raw 0x2E struct members straight out of the fixtures and compared them to the projected TabTable and to the actual built child ids:

Host raw 0x2E page ids projected TabTable actual direct children
vendor 0x100000B8 0x100000BC, 0x100000C4, 0x100000CD identical contains all three
combat 0x100000A2 0x100000AA0x100005C3 identical contains all eight
character 0x10000227 0x1000022B, 0x1000022C, 0x10000539 identical contains all three
spellbook 0x100002A8 0x100002AC, 0x100002AD identical contains both

Round 1 reported vendor's table as naming 0x1000007C/0x10000084/0x1000008D and character's third entry as 0x100003F8→0x100003F9. Neither appears anywhere in the dat property. Since the deep-diff proves the raw properties are byte-identical between df9c7a35 and b236a442, this was a round-1 mis-read, not a data change.

Measured consequence: every tab-table id occurs exactly once, at depth 1, in every one of the five Type-8 hosts — so FindDescendant returns the same element FindDirectChild did. No behavior change today.

Does the recursion introduce NEW ambiguity? Yes, latently — and the codebase already documents the duplicate-id hazard the task asked about (UiDatElement.cs:60-62: "footer state groups that appear once per tab page but share the same dat id"). Two facts:

  • Duplicate DatElementIds are rampant inside these very subtrees: character 0x10000227's 133-node subtree has 39 duplicated ids (0x10000244 ×6, 0x1000023D ×2, …); combat has four ids at ×8; spellbook has 0x10000328 ×13; options has 0x100002CE/CF/D0 ×16.
  • FindDescendant is pre-order: it fully recurses into child n's subtree before testing child n+1. So a deep match under an earlier sibling beats a direct-child match on a later sibling — a strictly larger ambiguity surface than FindDirectChild had.

Safe today only because no tab-table id collides with a duplicated id. Recorded as NOTE 2, with the OP3+ trigger named.

SHOULD-FIX 3 — silent no-resolve — CLOSED

UnresolvedEntries (UiTabPanel.cs:118) plus a per-miss Console.WriteLine carrying the host id, both entry ids, and which side missed (:154-157). Detectable enough for this lens: it is a public, testable collection, not only a log line, and OptionsPanelLayoutConformanceTests pins Assert.Empty(tabControl.UnresolvedEntries) after activation. A future regression that breaks resolution is now an assertable state change rather than a silent no-op.


4. New blast radius introduced by the rework

Scanned the rework diff for what it adds to the exposure surface.

(a) Unsealing UiDatElement — no invariant becomes mutable by subclasses. Info is protected readonly (UiDatElement.cs:57), so the reference cannot be reseated. The class's identity-carrying members are non-virtual: ElementId, ActiveRetailStateId (:69), TrySetRetailState (:86), ActiveMedia (:150). The only overridable surface a subclass inherits is UiElement's own (OnDraw, OnHitTest, ConsumesDatChildren, OnEvent, HandlesClick) — and of those, exactly one is exercised (UiTemplateListBox.ConsumesDatChildren, §2). OnDraw is protected override and not sealed, so a future subclass could silently kill authored-media drawing; neither current subclass does. NOTE 3.

(b) Unsealing widens every is/as UiDatElement site in the tree. Nine production sites now match Type-5/Type-8 widgets where they previously fell through: AppraisalUiController.cs:176, :224; CharacterStatController.cs:1582, :1585, :1799, :1839; SelectedObjectController.cs:152; CreatureAppraisalRows.cs:312; RetailTabBinding.cs:21; SpellcastingUiController.cs:658; WindowChromeController.cs:41. Every one of them is additionally gated on a specific ElementId/role, and I checked the ids involved (InscriptionBackgroundId = 0x10000137, SpellIconId = 0x1000015F, FooterStateAId/BId = 0x10000240/41, OverlayId = 0x100001A0) against the complete Type-5/Type-8 id set — no overlap. The widening is also benign in direction: the behavior these sites reach for (OnClick, MediaVisible, ClickThrough) is inherited unchanged. Clean.

The inverse — tests using exact-type Assert.IsType<UiDatElement> — is the sharper edge, since xUnit's IsType rejects subclasses. Ten such sites exist; EffectsUiControllerTests.cs:136 was the one that needed updating and was updated. The remaining nine target ids outside the Type-5/8 set.

(c) The factory arms being unconditional again. Correct now, for the reason the comments give. Verified there is no arm collision: the switch is a single info.Type switch (DatWidgetFactory.cs:76), and the four UIOption_* class ids (0x10000036/37/38/44) are distinct Type values from 5 and 8 — the 0x10000044 template element 0x10000520 is Type 0x10000044, not Type 5, so it cannot be captured by the Type-5 arm.

(d) BuildCheckboxBitfield64 deleted; 0x10000044u => new UiCheckboxBitfield64(info.TemplateList) (DatWidgetFactory.cs:162) no longer seeds SpriteResolve/LabelFont. Zero blast radius — class 0x10000044 occurs in exactly one fixture (options_2100002B.json, element 0x10000520), in a panel that is not yet shipped. Rows now carry their own art via the resolved template. Noted for completeness only.

(e) Fixture regeneration's effect on other conformance suites. No test pins a fixture value that regeneration changed, because regeneration changed no pre-existing value (§2, MUST-FIX 3.3, all 27 files). The only way a suite could have broken is by exact-type assertion on a re-classed widget — one site, updated.

(f) The RuntimeCollisionReportingStateTests allocation flake — not caused by this rework, and it is exactly ISSUES #250's class. tests/AcDream.Runtime.Tests/Physics/RuntimeCollisionReportingStateTests.cs:2359-2384 (WarmedSteadyContactRefreshDoesNotAllocate) has the precise shape #250 named as its root cause: one warmup call, then a 10,000-iteration loop written inline inside the measured window, asserted Assert.Equal(0, allocated). #250's own table lists that shape (UiDatFontTests: "1 call" warmup / "a 10,000-iteration loop written inline") and diagnoses it as on-stack replacement compiling on the measuring thread inside the window.

The #250 fix moved every strict-zero site in AcDream.App.Tests onto tests/AcDream.App.Tests/ZeroAllocationProbe.cs. It never swept the other test assemblies: AcDream.Runtime.Tests still has three files with raw GC.GetAllocatedBytesForCurrentThread() sites (Physics/RuntimeCollisionReportingStateTests.cs, Physics/RuntimePhysicsStateTests.cs, Physics/RuntimeSetPositionStateTests.cs, plus Gameplay/RuntimeLocalPlayerMovementStateTests.cs), as do AcDream.Headless.Tests and AcDream.Core.Tests. #250's own closing lesson applies verbatim: "'Not observed failing' only ever meant 'not yet observed'."

Could the rework have affected it? No causal path. git show --stat b236a442 touches only docs/, src/AcDream.App/UI/**, and tests/AcDream.App.Tests/UI/**. Zero files under src/AcDream.Runtime/ or tests/AcDream.Runtime.Tests/. Test assemblies run in separate processes, so the larger App fixtures cannot raise GC pressure inside the Runtime test process.


5. Corrections to the round-1 report (record-keeping)

Two round-1 claims are refuted by the committed data. Both are recorded here so the next reader does not re-derive them:

  1. "On 3 of the 4 panels the tab table's ids do not match the resolved child ids, so FindDirectChild misses and SwitchTo is inert — mitigating, and it is luck." False. All 16 tab-table ids across the four pre-existing hosts resolve as direct children (§3, SHOULD-FIX 2 table). df9c7a35 was therefore worse than round 1 assessed: all four panels would have had a live import-time switcher, and combat — with no controller re-binder — would have hidden seven of its eight stance pages on import. This strengthens, not weakens, the case for the dormancy fix that landed.
  2. SHOULD-FIX 2's premise, "property 0x2E carries base-layout ids that LayoutImporter.IncorporateChildren remaps." False; the raw struct members equal the incorporated child ids exactly. The FindDescendant change is still defensible on its own merits (retail uses GetChildRecursive, and OP3+ cross-layout page mounts genuinely nest), but it was justified by a diagnosis that does not hold, and it carries the real cost measured in §3.

This is the docs/research/2026-08-05-c4-closeout-handoff.md pattern in miniature — inferring a fact you can observe. The observation cost one script.


6. Residuals

SHOULD-FIX A — ConsumesDatChildren => true rests on a measured-but-unpinned premise

UiTemplateListBox.cs:145 is the single remaining behavioral delta versus the pre-OP2 UiDatElement fallback: authored dat children of a Type-5 element are no longer built. The class doc states the premise correctly (UiTemplateListBox.cs:28-32: "None of the ten pre-existing elements author real dat children of their own"), and I verified it — all 16 Type-5 elements, all 32 fixtures, zero children.

No test pins it. Worse, the closest pin cannot distinguish the two outcomes: OP2ReworkBlastRadiusConformanceTests.cs:153 asserts Assert.Empty(host.Children) on effects 0x10000123, which passes both when no viewport was injected (intended) and when authored children were silently consumed (the failure). A future dat/importer change that gives any of the ten a child would be dropped with the whole suite green.

Suggested pin (cheap): assert info.Children.Count == 0 on the ElementInfo side for the ten ids, in the same test file, so the premise is asserted where it is relied upon. Contrast the Type-8 side, which does have the right pin — Assert.Equal(16, host.Children.Count) at :130.

Also, only one of the ten Type-5 elements is pinned at all; the other nine (0x1000023D, 0x10000532, 0x10000123 negative, 0x10000149, 0x1000032D, 0x10000335, 0x10000174, 0x10000464) rely on class-level reasoning. That is defensible for the dormancy properties, which are class-level; it is not defensible for the child-count premise, which is per-element.

NOTE 1 — the state-propagation pin is a capability assertion

OP2ReworkBlastRadiusConformanceTests.cs:91, :107 assert IsAssignableFrom<IUiDatStateful>, not that a PassToChildren state actually reached a child. Sound in practice because TrySetRetailState is non-virtual (UiDatElement.cs:86), so no subclass can diverge — but it is the one MUST-FIX-1 axis whose pin would survive the mechanism breaking in the base class.

NOTE 2 — FindDescendant pre-order first-match in a duplicate-id tree

UiTabPanel.cs:208-219 recurses into an earlier sibling's entire subtree before testing the next sibling, in trees where duplicate DatElementIds are common (up to ×16 in options_2100002B.json). Harmless today — every tab id is unique and at depth 1 — but the OP3+ trigger is concrete: the Options page slots (0x10000211/0x10000212/0x10000213/0x1000050C) mount cross-layout content that already re-uses ids present elsewhere in the same tree. If it bites, prefer breadth-first, or search direct children first and fall back to descendants.

Two smaller items in the same class, both unreachable today (nothing calls ActivateTabBehavior):

  • UiTabPanel.cs:161 binds the click even when only the page failed to resolve, so the button would switch to a page that does not exist — and SwitchTo (:184-199) would then hide every page and close every tab, i.e. a blank panel.
  • Conversely, the new ReadTabTable zero-id filter (ElementReader.cs:520-527) usefully protects SwitchTo's ActivePageElementId == 0 sentinel from ever being a real page id. Good interaction, worth keeping in mind if that filter is ever relaxed.

NOTE 3 — unsealing leaves OnDraw overridable

UiDatElement.OnDraw (:208) is protected override, not sealed. A future UiDatElement subclass that overrides it silently loses authored-media drawing — which is precisely the df9c7a35 vendor-backdrop defect, reachable again by a different route. Sealing OnDraw, TrySetRetailState's surface is already safe (non-virtual), would close it. Also: UiTabPanel still implements IUiChildrenAttachedListener with an empty body (:126-128) purely to document "deliberately does nothing" — harmless, but it is a live interface registration that a reader may mistake for an active hook.

NOTE 4 — blast-radius sweeps remain bounded by the 32 fixtures

Both round 1 and this re-review measure the 27 layouts the fixture generator covers (RetailLayoutFixtureGenerator.cs:19-37, :95-184). Production imports at least a few LayoutDescs with no fixture — 0x21000008 (ExternalContainerController, mounted at RetailUiRuntime.cs:2177/:2223), 0x21000037 (shared UIItem catalog), 0x21000043 (vendor popup catalog), 0x21000011 (SpewBox). With dormancy in place the only unmeasured risk in those is narrow and specific: a Type-5 element carrying authored dat children (Type-8 is now fully identical to the old fallback, with no ConsumesDatChildren override, so it carries no residual risk at all). 0x21000008's two lists build as UiItemList (ExternalContainerController.cs:80-81 uses a RequiredList helper), i.e. class 0x10000031, not Type 5 — so the highest-traffic unfixtured layout is clear. Recorded so the boundary of the claim is explicit, not because a defect is suspected.


7. Verdict

CLOSED.

The rework fixes the class rather than the instances: dormancy makes an unactivated Type-8/Type-5 widget indistinguishable from the pre-OP2 UiDatElement fallback by construction, so the unconditional factory arms are now correct instead of accidentally survivable. All three MUST-FIXes are discharged and — critically for a lens whose round-1 complaint was that the harness could not see anything — the closure is verifiable from committed data: 1,102/1,102 fixture nodes carry the new fields, 27/27 regenerated fixtures show zero drift in pre-existing values, all 16 Type-5 elements carry zero children, and the new pins assert media file ids, ClickThrough, child counts, and page visibility rather than widget class names.

Residuals are one SHOULD-FIX (pin the ConsumesDatChildren premise where it is relied upon) and four NOTEs, none of which changes shipped behavior. The RuntimeCollisionReportingStateTests flake is ISSUES #250's class in an assembly #250's fix never swept, with no causal path to this commit.

The one thing worth carrying forward beyond OP2: round 1 got two facts wrong that one fixture-parsing script settles, and one of them became the stated rationale for a code change that shipped. §5 corrects the record.