acdream/docs/research/2026-08-11-op2-rereview-blast.md

# 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,
  `df9c7a35` → `b236a442`, 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 drawn** — `OP2ReworkBlastRadiusConformanceTests.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`) | **skipped** — `ConsumesDatChildren => 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.Resolve` → `IncorporateChildren`,
`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**

`FindDirectChild` → `FindDescendant`, 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` | `0x100000AA` … `0x100005C3` | 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 `DatElementId`s 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 `DatElementId`s 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.