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

Campaign OP slice OP2 rework — MECHANISM-lens FOCUSED RE-REVIEW

Date: 2026-08-11 Rework under review: b236a442 — "fix(ui): OP2 rework — dormant UiDatElement subclasses, fixed Panel/CheckboxBitfield64 mechanism" Round-1 findings being closed: docs/research/2026-08-11-op2-review-mechanism.md (verdict REJECT on df9c7a35) Mode: READ-ONLY. No build, no test run, no launch. Every claim is a quote from committed source, a decode of a committed fixture, or an extract from docs/research/named-retail/acclient_2013_pseudo_c.txt.

VERDICT: REOPEN — one MUST-FIX, narrowly scoped

All four behavioural MUST-FIXes and all five SHOULD-FIXes from round 1 are genuinely closed, with regression tests that would actually catch a relapse. This is not a redo — it is a one-item fix round.

The one thing that fails is the tail of round-1 MUST-FIX 6, and it is the same failure mode that caused the round-1 REJECT, on the same element: the commit asserts a completeness claim about 0x10000520's authored data and about retail's Refresh that the data and the decomp both refute.

• UiCheckboxBitfield64.cs:194-201's doc comment quotes UIOption_CheckboxBitfield64::Refresh @0x004859C0 as if the ANY-bit predicate were the whole function. It is not. Refresh computes both an all-set and an any-set result and uses them for two different things (§2.2 below). The all-set half drives a per-row LED media swap between the element's two authored RenderSurface DIDs — P0x10000082 = 0x06004D17 and P0x10000083 = 0x06004D19, both still present in the regenerated fixture, both still unread anywhere in src/AcDream.App (verified by grep: zero hits outside one prose mention).
• That unported half has no register row. Round-1 MUST-FIX 6 asked for exactly this ("Also missing rows for … the dropped 0x64/0x10000082/0x10000083 authored data"). 0x64 is now consumed; the two DIDs are not, and the register gained only AD-73 (tab dormancy) while TS-72 was retired.

Fix = one AP row naming the unported Refresh media branch + the unported ResizeTo(GetWidth(), CalculatePaperSize(0,-1)) tail, and a corrected doc comment that stops presenting a half-quoted Refresh as complete. The port itself belongs to OP5 with the consumer; nothing is user-reachable today.

---

1. Finding-by-finding closure check

1.1 UiTabPanel — CLOSED (all five sub-checks)

Round-1 item	Rework	Evidence
MUST-FIX 5 — cite UIElement_Panel, not the fictional UIElement_TabControl	✔	src/AcDream.App/UI/UiTabPanel.cs:8-21 names UIElement_Panel::Create @0x0046C6B7, SetupTabPageHash @0x0046C2E0, Update @0x0046BD00, OpenTab @0x0046BE20, InqTabFromPage @0x0046BEB0, ListenToElementMessage @0x0046BF90, and explicitly records the naming correction. DatWidgetFactory.cs:110-119 carries the same correction at the factory arm. Class renamed UiTabControl → UiTabPanel; the old file is deleted.
Switching semantics preserved	✔	UiTabPanel.cs:184-199 is byte-equivalent to the round-1 reading of UIElement_Panel::Update @0x0046BD00: exactly one page Visible, every tab button driven Open/Closed via RetailTabBinding.SetOpen → RetailUiStateIds.Open/Closed = retail 0xC/0xB. Nothing in this commit touched RetailTabBinding.cs or RetailUiStateIds.
SHOULD-FIX 8 — recursive descendant resolution	✔	FindDescendant (UiTabPanel.cs:208-219) is a depth-first descendant walk used for both the button (:149) and the page (:150, :191, :194), matching retail's GetChildRecursive at 0x0046C4EE / 0x0046BD00. FindDirectChild is gone.
SHOULD-FIX 9 — no invented _tabs[0] fallback	✔	UiTabPanel.cs:172-173 — if (defaultEntry is { } def) SwitchTo(def.PageElementId); with no ??=. Independently re-verified against the decomp this round: SetupTabPageHash zeroes m_OpenTabToken/m_OpenPageToken at 0x0046C312/0x0046C318, seeds ebp_1 = 0 at 0x0046C31E, only writes ebp_1/var_3c inside the 0x32-true branch at 0x0046C4DF/0x0046C4E3, and tail-calls Update(this, var_3c, ebp_1) at 0x0046C5EB. With nothing authoring 0x32, that is Update(0,0) against zeroed tokens, and Update's guard arg2 != m_OpenPageToken && arg3 != m_OpenTabToken is false → no switch. Confirmed.
SHOULD-FIX 7 — reader skips malformed entries	✔	ElementReader.cs:526 — if (buttonId == 0u || pageId == 0u) continue; with the retail filter cited inline at :520-525 (InqProperty(0x30) @0x0046C3E5, InqProperty(0x31) @0x0046C3FC). I re-read both sites: a missing 0x30 jumps label_46c510 and a missing 0x31 jumps the same, both landing on label_46c594 (next entry) without touching either hash. Match.

Round-1 SHOULD-FIX 3's "a silently-empty table and a wholly-unresolvable table are indistinguishable" is also addressed: UnresolvedEntries (:118) plus a [D.2b] diagnostic line (:154-157).

Two retail behaviours in SetupTabPageHash remain unported and are now newly visible because the rework reads that function properly — both inert on today's data, both NOTE (§3).

1.2 UiCheckboxBitfield64 — CLOSED on all three named sub-checks

(a) Rows built from the authored template via AddItemFromTemplateList(0). ✔ UiCheckboxBitfield64.AddChild (:140-180) resolves Templates[0] through TemplateResolver, finds 0x10000219 by recursive descendant search (FindCheckboxRecursive, :182-192), and stamps label/checked/click onto that widget. The synthesized ElementInfo{Type=1} and the invented RowHeight = 14f are deleted; BuildCheckboxBitfield64 is deleted from the factory and the arm is now 0x10000044u => new UiCheckboxBitfield64(info.TemplateList) (DatWidgetFactory.cs:162). Row geometry now comes from row.Height (:173-176) — the authored 272×20.

This matches retail CreateChildren @0x00485DF0 as I re-read it this round: AddItemFromTemplateList(&this->vtable, 0, nullptr) per row → GetChildRecursive(row, 0x10000219) → DynamicCast(1) → SetAttribute_Int(cb, 0x10000084, i) → SetStringInfo → SetTooltip.

Fixture-verified independently (I decoded options_2100002B.json myself, not the commit message): 0x10000520 is T=0x10000044, 272×100, zero children, empty StateMedia, and authors exactly TemplateList = [{0x2100002B, 0x10000521}]; 0x10000521 is T=3, 272×20, one child 0x10000219. The conformance test pins that exact pair (OptionsPanelLayoutConformanceTests …Bitfield64Template_BuildsAsEmptyUiCheckboxBitfield64).

(b) IsSet is retail's ANY-bit predicate. ✔ :200-201 — (CurrentLow & lowMask) != 0 || (CurrentHigh & highMask) != 0. Matches Refresh @0x00485A11: ebx = ((eax_2 | ecx_2) != 0) ? 0 : 1 → SetAttribute_Bool(cb, 0xE, ebx == 0).

(c) A multi-bit test that actually discriminates. ✔ UiCheckboxBitfield64_MultiBitMask_IsSet_UsesAnyBitNotAllBits seeds current 0x00000040 against Combat's real two-bit mask 0x00600040 and asserts row.Selected == true. Under the old all-bits predicate IsSet returns false and Assert.True(row!.Selected) fails — the test genuinely discriminates, unlike the single-bit Error case it replaced (which is retained and now also pins that the row resolves to TemplateCheckboxElementId). It further pins that clicking clears both mask bits, i.e. the AND-NOT half.

ToggleRow (:209-224) is unchanged and remains correct: turnOn = !IsSet(...) derives from the bitfield, not from Selected, so a ToggleBehavior self-toggle on the resolved UiButton cannot double-flip it.

1.3 TS-72 retirement — PARTIALLY CLOSED (the MUST-FIX)

The retirement itself is legitimate. TS-72's stated unknown was the click-toggle bit math; that is now decomp-confirmed (ListenToElementMessage @0x00485AE0 → BitUtils::SetBitsOnOrOff @0x004857D0: arg3 != 0 → OR-in both halves, else AND-NOT both halves). The retirement note at docs/architecture/retail-divergence-register.md:350 states plainly that the toggle half was always right and the checked-state half was wrong and is now fixed. Accurate, and written per register convention (struck row retained with its original text; the section header's active count moved 41 → 40).

What was swept: the note ends "nothing about its own click/checked mechanism remains genuinely unverified". Verified is not the same as ported. Reading Refresh in full (§2.2) shows a decomp-confirmed, unported presentation half of the checked mechanism, on the same element whose "authors nothing" claim caused round 1's REJECT, with the two authored DIDs it consumes still sitting unread in the fixture. That gap has no row anywhere in the register.

AD-73 is correct as far as it goes but is scoped to UiTabPanel only (§1.4).

1.4 The dormancy shape — CLOSED, with one undocumented shape change

Overrides cannot leak pre-activation. I enumerated every member both subclasses add or override:

	UiTabPanel	UiTemplateListBox
base	UiDatElement (:79)	UiDatElement (:81)
OnDraw	not overridden → UiDatElement's authored-media draw	not overridden
ClickThrough	base ctor sets true (UiDatElement.cs:138)	same
IUiDatStateful	inherited unchanged	inherited unchanged
ConsumesDatChildren	not overridden → false, children recurse	overridden true (:145) ← the one change
AddChild	not overridden	not overridden
import-time hook	OnChildrenAttached is an empty body (:126-128)	none
side-effecting members	ActivateTabBehavior() / SwitchTo() — explicit calls only	AddItemFromTemplateList(); Scroll getter materialises the viewport (:107)

UiDatElement itself changed only sealed → public class (:43) and _info → protected readonly ElementInfo Info (:57) plus mechanical renames. No behavioural edit in the base. ✔

The one shape change: UiTemplateListBox.ConsumesDatChildren => true is not identical to the pre-OP2 fallback, which recursed into dat children. I verified the class doc's justifying claim myself by walking every committed fixture: every Type-5 element in the corpus has zero authored children — character 0x1000023D / 0x10000532, effects_positive/effects_negative 0x10000123, examine 0x10000149 / 0x10000335 / 0x1000032D, mini_game 0x10000174, spellbook 0x10000464, and the three Options ListBoxes 0x100001FA / 0x10000200 / 0x1000050D. So the change is inert today and the claim holds. It is a latent divergence (a future Type-5 that authors children loses them silently), and it is not mentioned in AD-73 or anywhere in the register — NOTE, §3.

Tab-table entry counts survived the new 0x30/0x31 skip — character 3, combat 8, spellbook 2, vendor 3, options 4, i.e. the same counts round 1 recorded. The filter is inert on real data, exactly as retail's is.

AD-73 states the adaptation truthfully. Register :108. I checked each claim:

• "Retail … unconditionally activate the authored default page for ANY instance that carries a tab table" — correct, and I confirmed the trigger this round: SetupTabPageHash has exactly one caller, UIElement_Panel::OnSetAttribute @0x0046C6DC, fired when property 0x2E is applied. Per-instance, unconditional.
• Oracle citations SetupTabPageHash @0x0046C2E0, Update @0x0046BD00, OpenTab @0x0046BE20 — all three verified present at those addresses in acclient_2013_pseudo_c.txt.
• The four named hosts and their controller ownership match the fixture decode (character 0x10000227 tab=3, spellbook 0x100002A8 tab=2, vendor 0x100000B8 tab=3, combat 0x100000A2 tab=8, plus the tab-table-less vendor backdrop 0x1000008D).
• The Risk column names the real failure mode (a forgotten ActivateTabBehavior() leaves every page slot visible at once, silently). Accurate.

AD-73 covers only UiTabPanel. UiTemplateListBox's dormancy (lazy viewport + ConsumesDatChildren) rides on the same adaptation and is unregistered — NOTE, §3.

1.5 Reader-level tests — CLOSED

tests/AcDream.App.Tests/UI/Layout/ElementReaderTests.cs:354-481 adds five tests that build a synthetic ElementInfo, stuff a raw UiPropertyValue into States[DirectStateId].Properties.Values[0x2E/0x64/0x72], and call ElementReader.ApplyCanonicalLegacyProjection(info) directly.

Would they catch what the fixture tests miss? Yes, decisively. The fixture tests deserialize ElementInfo.TabTable/TemplateList/ScrollbarElementId as already- projected JSON fields; LayoutImporter.Build never re-runs the projection at load, so deleting the body of ReadTabTable would not fail one fixture test. These five drive the reader functions themselves. Coverage:

• ReadTabTable_DecodesButtonPageDefaultInAuthoredOrder — struct-member ids, authored order, 0x32 present/absent.
• ReadTabTable_SkipsEntriesMissingButtonOrPage — the new retail filter, with a well-formed entry after two malformed ones, so an early-break regression fails too.
• ReadTemplateList_DecodesLayoutDidAndElementIdInAuthoredOrder — 0x63/0x62 pair, order.
• …ScrollbarLinkage_DecodesEnumProperty / …AbsentPropertyStaysZero — the 0x72 value and its absent case.

Gap (NOTE, not a miss of the round-1 finding): none of the five exercises ReadStructMemberId's Enum/DataId/Integer kind coercion — every test uses Enum. A regression narrowing that coercion would still slip through.

The blast-radius pins (OP2ReworkBlastRadiusConformanceTests.cs) are the other half of the answer: six built-widget tests across the five pre-existing Type-8 elements and one representative Type-5, asserting BehaviorActive == false, ActivePageElementId == 0, ClickThrough == true, non-zero ActiveMedia() on the vendor backdrop, 16 children still present and all Visible on combat, and Empty(Children) + ContentHeight == 0 on the effects list. Those are exactly the properties the dormancy model rests on.

---

2. New mechanism infidelity introduced or newly exposed by the rework

2.1 MUST-FIX — Refresh's all-set branch is unported, its two authored DIDs are unread, and neither the doc comment nor the register says so

UIOption_CheckboxBitfield64::Refresh @0x004859C0, in full, per row:

lowHit  = m_current.low  & mask.low;                        // eax_2 @0x004859F9
highHit = m_current.high & mask.high;                       // ecx_2 @0x004859FB
allSet  = (lowHit == mask.low && highHit == mask.high);     // var_d_1 @0x004859FF..0x00485A0A
ebx     = ((lowHit | highHit) != 0) ? 0 : 1;                // anySet @0x00485A11

row = UIElement_ListBox::GetItem(this, i);                  // @0x00485A21
cb  = UIElement::GetChild(row, 0x10000219);                 // @0x00485A35
UIElement::SetAttribute_Bool(cb, 0xE, ebx == 0);            // checked <=> ANY set   ← ported ✔

if (ebx == 0) {                                             // @0x00485A51 — only when ANY set
    led = UIElement::GetFirstChildElement(cb);              // @0x00485A5A  (0x10000328)
    if (led) {
        if (allSet && GetAttribute_DataID(this, 0x10000082, &v))   // @0x00485A69/0x00485A7C
            UIElement::SetMediaImageForState(led, v, 1, 6);        // @0x00485AA7
        else if (GetAttribute_DataID(this, 0x10000083, &v))        // @0x00485A9A
            UIElement::SetMediaImageForState(led, v, 1, 6);
    }
}

Retail therefore renders three row states, not two: unchecked, checked-partial (0x10000083), checked-complete (0x10000082). Eight of the thirteen Chat filter masks are multi-bit (Gameplay 0x83912021, Combat 0x00600040, Magic 0x00020080, AreaSpeech 0x00001004, Tells 0x00000018, Allegiance 0x00040C00, …), so the partial state is reachable in ordinary play, not a corner case.

Evidence of the gap:

• src/AcDream.App/UI/UiCheckboxBitfield64.cs:194-201 quotes Refresh as checked = (lowHit | highHit) != 0 and stops there — the allSet computation and the entire media branch are absent from both the code and the citation.
• RefreshRowVisuals (:226-230) writes only row.Toggle.Selected.
• grep -i '0x10000082\|0x10000083\|10000328\|0x10000084' src/AcDream.App/**/*.cs returns exactly one hit, a prose mention at UiCheckboxBitfield64.cs:28. The two DIDs are read by no code path; ElementReader does not project them onto ElementInfo either.
• They are unambiguously authored: decoded from options_2100002B.json, element 0x10000520 DirectState carries P0x10000082 = DataID(0x06004D17) and P0x10000083 = DataID(0x06004D19) alongside P0x64.
• docs/architecture/retail-divergence-register.md gained AD-73 only. No AP/TS row names this.

Adjacent and in the same fix: retail's CreateChildren tail is ResizeTo(GetWidth(), CalculatePaperSize(0, -1)) at 0x00485E87/0x00485E97 — the block sizes itself to its rows because it is a non-scrolling UIElement_ListBox nested inside the Chat page's scroller. UiCheckboxBitfield64 tracks _contentHeight (:53, :174-175) but never applies it to Height, so the block stays at its authored 272×100 while thirteen 20 px rows need 260 px. Also unported, also unrowed.

Required: one AP row covering both (unported Refresh media branch + unported ResizeTo/CalculatePaperSize), naming 0x10000082/0x10000083/0x10000328 and pointing at OP5 as the consumer gate; and a corrected IsSet doc comment that quotes Refresh completely instead of half. The port itself can land at OP5 with the consumer — nothing user-reachable observes it today.

2.2 SHOULD-FIX — AddChild drops retail's tooltip and per-row index stamp

CreateChildren @0x00485E50/@0x00485E65 calls SetStringInfo(cb, siLabel) and SetTooltip(cb, siTooltip); @0x00485E3E stamps SetAttribute_Int(cb, 0x10000084, i). acdream sets checkbox.Label (:169) and stores tooltip in the Row record (:178) but never applies it; there is no settable tooltip on UiElement (only a virtual GetTooltipText() returning null, UiElement.cs:454), so this needs a one-line override or a settable property. The 0x10000084 index stamp is correctly replaced by a mask closure (:171) — equivalent and better typed; call that out in the doc rather than leaving it silently absent.

2.3 SHOULD-FIX — the ListBox mechanism is now implemented twice, divergently

UIOption_CheckboxBitfield64 is a UIElement_ListBox in retail (it calls AddItemFromTemplateList/GetItem/CalculatePaperSize/ListenToElementMessage on itself; PostInit tail-calls UIElement_Scrollable::PostInit). The rework's class doc argues against reusing UiTemplateListBox because "retail's block does not itself scroll" — an assertion with no citation, and one the CalculatePaperSize/ResizeTo tail partly contradicts (the block does do ListBox self-sizing). The result is two independent row-stacking implementations that already differ: UiTemplateListBox stacks into a lazily-created UiScrollablePanel (UiTemplateListBox.cs:172-187), UiCheckboxBitfield64 stacks directly onto itself (:173-176). Not a defect today; it is the seam where §2.1's ResizeTo gap and any future ListBox fix will diverge. Either reuse the class or record why in the register alongside §2.1's row.

---

3. NOTES

1. UiTemplateListBox.ConsumesDatChildren => true (:145) is the one place the dormant subclass is not byte-identical to the UiDatElement fallback. Inert today (all ten pre-existing Type-5 elements have zero authored children — verified across every committed fixture), unregistered, and silent if a future Type-5 authors children. One sentence in AD-73 or its own row.
2. UiTemplateListBox.Scroll (:107) is a side-effecting getter — reading it materialises and attaches the viewport, breaking dormancy without any explicit activation call. ContentHeight (:111) and LineHeight (:116-124) are both null-safe; Scroll is the odd one out. Nothing reads it today.
3. AD-73 scopes dormancy to UiTabPanel only. UiTemplateListBox's lazy viewport is the same adaptation against retail's always-scrollable ListBox and rides uncovered.
4. SetupTabPageHash's early return is unported. At 0x0046C4EE/0x0046C4F5 retail does GetChildRecursive(this, tabId) and, on a miss, returns outright — abandoning the remaining entries and never calling Update. UiTabPanel logs the miss, keeps iterating, and still performs the default switch (:149-173). Inert on today's data (every options entry resolves) and arguably the better diagnostic, but it is a divergence the round-1 SHOULD-FIX 3 response introduced.
5. tab != page and MasterPropertyId == 0x2F filters still unimplemented (0x0046C3A6, 0x0046C46B). Retail adds the hash entries either way and only gates the default-flag read and SetMouseVisible(1) on tab != page. Harmless today. Carried from round-1 NOTE 12.
6. SetMouseVisible(1) per tab button (0x0046C4FB) and BroadcastElementMessage(this, 0x2C, …) after each Update remain unported. OP3 needs the second for research §3.6's apply/revert semantics. Carried from round-1 NOTE 14.
7. 0x71 (horizontal scrollbar) still unread; ScrollbarElementId is specifically the vertical link (UIElement_Scrollable::OnSetAttribute @0x00474630) and is still not named as such. Carried from round-1 NOTE 13.
8. SaveCurrentValue/RestoreSavedValue/Changed (@0x004856A0–@0x00485700) are unported on UiCheckboxBitfield64 — the Apply/Cancel triad OP5 needs. Not introduced here; SetDefaultValue/RestoreDefaultValue shipped in OP2 as-is.
9. SpriteResolve and LabelFont are now dead on UiCheckboxBitfield64 — the factory no longer sets SpriteResolve (the row subtree carries its own resolver) and LabelFont's own doc says it is unused (:80-82). Harmless; delete at OP5.
10. The campaign plan is stale. docs/plans/2026-08-10-options-panel-campaign.md:181 still says "Create src/AcDream.App/UI/UiTabControl.cs" — the exact fictional-class name this round corrected — and still cites "lane A §5" for the switching contract when the tab table is §1.3 and its host §1.4 (round-1 NOTE 18, unaddressed). OP3/OP8 will read that line.
11. Reader tests use only Enum-kind struct members, so ReadStructMemberId's Enum/DataId/Integer coercion tolerance is unpinned.
12. IUiChildrenAttachedListener now has exactly one implementer with an empty body (UiTabPanel.cs:126-128). Still the right hook for OP3's activation plumbing; just no longer load-bearing in this commit.
13. Research §1.5's "0x10000521 … UNKNOWN consumer" / §10.5 U10 are now closed by the fixture + CreateChildren, and the research doc still says UNKNOWN. Carried from round-1 NOTE 17.

---

4. What I re-derived independently this round

Everything below was read from primary source, not taken from the commit message:

Claim	Source
SetupTabPageHash has exactly one caller, OnSetAttribute on property 0x2E	acclient_2013_pseudo_c.txt 0x0046C6C0–0x0046C6DC (grep: 2 hits total for the symbol)
Zeroed tokens + Update(0,0) ⇒ no default switch	0x0046C312, 0x0046C318, 0x0046C31E, 0x0046C4DF, 0x0046C5EB; Update guard 0x0046BD00
Missing 0x30/0x31 skips the entry	0x0046C3E5, 0x0046C3FC → label_46c510 → label_46c594
Refresh computes all-set AND any-set, and uses all-set for the LED media swap	0x004859F9–0x00485AA7 (full function read)
SetBitsOnOrOff = OR-on / AND-NOT-off	0x004857D0–0x0048580B
CreateChildren = per-row AddItemFromTemplateList(0) + GetChildRecursive(0x10000219) + index/label/tooltip + ResizeTo(CalculatePaperSize)	0x00485DF0–0x00485E97
0x10000520 authors P0x64 = [{0x2100002B, 0x10000521}], P0x10000082 = 0x06004D17, P0x10000083 = 0x06004D19, zero children, empty StateMedia	decode of committed options_2100002B.json
0x10000521 is T=3, 272×20, one child 0x10000219	same
Every Type-5 element in every committed fixture has zero authored children	walk of all 32 committed fixtures
Tab-table entry counts unchanged post-regeneration (character 3 / combat 8 / spellbook 2 / vendor 3 / options 4)	same walk
Vendor 0x1000008D is T=8 with no tab table (the backdrop)	same walk

Fixture decodes used a throwaway reader in the scratchpad. Nothing in the repository was modified except this document.