# Campaign FA slice FA3 — MECHANISM-FAITHFULNESS review **Target:** `0a9ca2f1` (feature), `74c3d85d` (fixture + tests), `b6a25110` (gate script + ledger), `d7e1cffd` (research addendum), reviewed against `docs/plans/2026-08-11-fellowship-allegiance-campaign.md` slice FA3 + D1 + §6 and `docs/research/2026-08-11-fa-panel-structure.md` (incl. §10 + the FA3 correction addendum). **Verdict: APPROVE-WITH-FIXES** — 2 MUST-FIX (both in the user's connected-gate script, which is the artifact the user actually reads while gating), 9 SHOULD-FIX. The load-bearing tab-table correction is **CONFIRMED CORRECT** by re-derivation from the authored bytes AND independently from the installed DATs. No code defect found in the mount path. Method note: every claim below was re-derived, not taken from the commit messages. The fixture was parsed directly; the env-gated live-mount probe was run against the installed DATs; the retail anchors were re-read in `acclient_2013_pseudo_c.txt`; and one BN string operand was byte-verified in the PDB-paired binary (`check_exe_pdb.py` → `=== MATCH ===`, GUID `9e847e2f-777c-4bd9-886c-22256bb87f32`). --- ## 1. The tab-table correction — VERIFIED CORRECT (item 1) This was the load-bearing fact. It holds, three independent ways. **(a) The authored `0x2E` array, read straight out of the committed fixture** (`tests/AcDream.App.Tests/UI/Layout/fixtures/social_panel_2100006E_1000018F.json`, root `0x1000018F`, property `46` = `0x2E`, four `Kind 8` struct entries with sub-properties `48`=`0x30` button, `49`=`0x31` page, `50`=`0x32` IsDefault): | entry | P0x30 (button) | P0x31 (page) | P0x32 | |---|---|---|---| | 0 | 268436108 = `0x1000028C` | 268436113 = `0x10000291` | **true** | | 1 | 268436110 = `0x1000028E` | 268436114 = `0x10000292` | absent | | 2 | 268436754 = `0x10000512` | 268436755 = `0x10000513` | absent | | 3 | 268436795 = `0x1000053B` | 268436810 = `0x1000054A` | absent | This is **exactly** the pairing recorded in `SocialPanelController.cs:34-37`, and Allegiance (`0x1000028C`→`0x10000291`) is the sole `IsDefault` entry. **(b) Each button's own resolved caption** — from the live-mount probe run against the installed DATs during this review: ``` [socialprobe] tab button 0x1000028C (UiText) caption='Allegiance' [socialprobe] tab button 0x1000028E (UiText) caption='Fellowship' [socialprobe] tab button 0x10000512 (UiText) caption='Friends' [socialprobe] tab button 0x1000053B (UiText) caption='Squelch' ``` **(c) Each page's own `P0x57` + its `RegisterElementClass` type**, both from the fixture and re-confirmed by the probe: | page | authored `Type` | retail class (verified in pseudo-C) | `P0x57` | |---|---|---|---| | `0x10000291` | `0x1000002C` | `RegisterElementClass(0x1000002c, gmAllegianceUI::Create)` @`004911fa` | `0x1000000E` = `ToggleAllegiancePanel` (F3) | | `0x10000292` | `0x1000002D` | `RegisterElementClass(0x1000002d, gmFellowshipUI::Create)` @`0048e6aa` | `0x1000000F` = `ToggleFellowshipPanel` (F4) | | `0x10000513` | `0x10000045` | `RegisterElementClass(0x10000045, gmFriendsUI::Create)` @`0048e21a` | `0x10000118` | | `0x1000054A` | `0x10000047` | `RegisterElementClass(0x10000047, gmSquelchUI::Create)` @`0048d04a` | `0x10000124` | The class doc's further claim that `0x10000118`/`0x10000124` carry **no** retail-default keybind is also correct: neither literal appears in `keymap-default.txt` or `retail-default.keymap.txt`. **A fourth corroboration the addendum did not state, and should have.** With the corrected pairing, the authored x-order becomes Allegiance (x=0), Fellowship (x=72), Friends (x=144), Squelch (x=206) — i.e. **the authored default tab is also the left-most tab**, which is what one would expect and which the old (refuted) guess did not produce. Button widths track caption lengths under the corrected mapping too (`Friends`, the shortest caption, is the narrowest button at 62px). This same x-order is the basis of MUST-FIX 1 below. Also verified: root `P0x10000029 = 12` (the `RetailPanelCatalog.SocialPanel` byte-verification claim), root `P0x57 = 0x1000000D`, close button `0x10000290` `P0x12 = 0x1000000D`, and `0x1000000D` really is an InputMap **context** id (`ID_InputMap_ToggleChatEntry`, `src/AcDream.Core/Input/RetailActionMap.cs:161`; it appears as `Context 0x1000000D` in `keymap-default.txt`) — so the class doc's "authored but UNCONSUMED" reading is accurate, not a hand-wave. --- ## MUST-FIX ### MF-1 — the gate script carries the REFUTED x-order into the user's contract `docs/research/2026-08-12-campaign-fa-test-script.md:59-64` (step 9): > "left-to-right authored order is Friends, Allegiance, Fellowship, Squelch" and `:20-25` (step 1): > "the authored default tab — NOT Friends, despite Friends being drawn > left-most on the tab strip in some sort orders" Both are wrong. The authored geometry (fixture, corroborated by the probe's `root-child` dump) is: ``` 0x1000028C "Allegiance" x=0 72x25 0x1000028E "Fellowship" x=72 72x25 0x10000512 "Friends" x=144 62x25 0x1000053B "Squelch" x=206 70x25 0x10000290 (close X) x=276 24x25 ``` Friends is drawn **third**, not left-most. This is the same x-order guess `d7e1cffd` corrects in the research doc — it just never got corrected in the script. Step 10's caption list (`:65-68`) repeats the wrong ordering. Why this is MUST-FIX and not a nit: the script is the user's contract for the connected gate, and step 1 explicitly primes the user to expect Friends at the left edge next to a tab that opens by default. The user will see Allegiance at x=0 and has been told that is the wrong-looking case. Fix step 9, step 1's parenthetical, and step 10 to the real order — and take the opportunity to say "the default tab is also the left-most tab", which is the most reassuring form of the correction. ### MF-2 — the gate script's step 14 sends the user to a trigger that CANNOT fire, and tells them to report it if it doesn't `docs/research/2026-08-12-campaign-fa-test-script.md:100-107` (step 14): > "If some other client action causes an allegiance push to land server-side > during this session (e.g. a `@allegiance info` chat command, which already > works independently of this panel), the monarch/patron blocks should then > become VISIBLE — **report if they do not**." and the matching "What to report" bullet at `:153-155`: > "The Allegiance page's monarch/patron blocks staying hidden after a real > allegiance push has visibly landed (e.g. after `@allegiance info` shows > non-empty data in chat)." `RuntimeAllegianceState._hasProfile` is set in exactly one place — `ApplyUpdate` (`src/AcDream.Runtime/Gameplay/RuntimeAllegianceState.cs:126-142`), whose own doc comment reads *"`0x0020 AllegianceUpdate` — … the **ONLY** inbound writer of this owner's profile (see the class doc's MUST-FIX 2 correction)"*. FA2's own fix round deliberately stopped `0x027C` (the `@allegiance info` response family) from seeding this owner (`4272ad0e`, "0x027C stops seeding"; `GameEventWiring.cs:231` documents it), and the plan's FA5 row states the `0x001F` subscription that provokes `0x0020` is **FA5 scope**. FA3 sends no `0x001F`. So the script names a trigger the campaign deliberately disconnected, and then instructs the user to file a defect when it fails to work. Either drop the trigger entirely and state plainly that the blocks are expected to stay hidden for the whole of FA3 (the honest shell-slice expectation), or replace it with the only real path (a server-side allegiance change that makes ACE push `0x0020` unsolicited) and say so. The "What to report" bullet must go with it. --- ## SHOULD-FIX ### SF-1 — the F3/F4 "retail Toggle-action semantics" claim has no retail anchor (item 2) `src/AcDream.App/UI/Layout/SocialPanelController.cs:200-204`: > "lets `RetailUiRuntime.HandleInputAction` implement **retail's** Toggle-action > close-on-second-press semantics (same shape as `OpenSpellbook`'s own > page-aware toggle)" I went looking for the retail handler and could not find one. Re-derived: - **No `P0x57` consumer exists in the binary.** The only two `0x57` reads anywhere in `acclient_2013_pseudo_c.txt` are `AppraisalProfile::InqInt(arg2, 0x57, …)` @`004b2954` and `AppraisalProfile::InqFloat` @`004b3981` — unrelated. No `GetAttribute_Enum(this, 0x57, …)` site exists. This matches lane A §6.1's UNVERIFIED flag; the FA3 class doc correctly restates it for the *root's* `P0x57`, then quietly relies on a different retail claim for the toggle. - **`ClientUISystem::OnAction @0x00564B90` does not handle these actions.** For `m_InputAction > 0x7c` it handles exactly `0x1000001E`, `0x10000025` and `0x1000002B` and returns `0` for everything else — `0x1000000E`/`0x1000000F` fall through. - **`gmPanelUI` never sees the action.** `gmPanelUI::PostInit @0x004BD010` does `RegisterForGlobalMessage(this, 1)`, but `gmPanelUI::ListenToGlobalMessage` is COMDAT-folded onto `NoticeHandler::RecvNotice_DisplayWeenieError @0x004F5860`, whose body is `return;`. - No other `OnAction` implementation's range covers `0x1000000E`/`0x1000000F` (`ChatInterface` `0x10000020..0x10000119`, magic `0x10000060..0x10000105`, combat `0x1000005B..`, `CPlayerSystem` `0x1000002A..`). So retail's actual F3/F4 behaviour — and in particular what a repeat press does when the panel is already open on the *other* tab — is **not established from this decomp**. The shipped rule ("close only when already on the target tab") is acdream's own `OpenSpellbook` precedent, which is a perfectly defensible choice; it just must not be labelled as retail's. Re-word the comment to name it as precedent-following, and add the unknown to the research doc's §8 list (next to U7, which is the same gap) so FA7's closeout can decide whether it wants a register row. The gate script's "same toggle-closes-on-repeat-press shape as every other `Toggle*Panel` action" (`:26-29`) is fine as-is — it claims acdream consistency, not retail fidelity. ### SF-2 — per-frame closure allocation in the allegiance page tick `src/AcDream.App/UI/Layout/SocialAllegiancePageController.cs:108-110`: ```csharp IReadOnlyList lines = hasProfile ? NoLines : BlankLine; if (_monarchName is not null) _monarchName.LinesProvider = () => lines; if (_patronName is not null) _patronName.LinesProvider = () => lines; ``` `lines` is captured, so this allocates a display class **plus** a delegate, twice, on **every frame**, whether or not the panel is visible (`RetailUiRuntime.cs:559` ticks it unconditionally). At the profile's measured ~520 FPS that is ~2,000 allocations/second of pure garbage for a value that changes at most once per session. The Modern Runtime slices spent whole commits driving per-frame paths to 0 B; this is a new, avoidable regression of that discipline in the graphical hot loop. Fix: hoist two `static readonly Func>` fields (one returning `BlankLine`, one returning `NoLines`) and assign the cached delegate, and/or only write `LinesProvider` when the gate actually flips. The fellowship page's tick is already allocation-free — this is the only offender. ### SF-3 — the live-mount probe prints two of its headline findings without asserting them (item 7) `tests/AcDream.App.Tests/UI/Layout/SocialPanelLiveMountProbeTests.cs:117-118` computes `passupCount` and prints it, but never asserts it. The commit message nevertheless claims *"`0x10000492` **is confirmed** authored twice under the allegiance page"*. A print is not a gate: this cannot fail if a future importer change collapses or drops one of the two instances — which is exactly the regression the scoped-lookup rule exists to survive. (For the record I did confirm the fact independently: the fixture authors `0x10000492` twice, once under `0x10000255/0x10000490` and once under `0x1000025A/0x10000490`, and the probe printed `= 2`.) Add `Assert.Equal(2, passupCount)`. Likewise, the probe activates the tab behavior (`:67`) and asserts `UnresolvedEntries` is empty, but never asserts **page exclusivity** — that after `ActivateTabBehavior()` exactly one of the four page elements is `Visible`, and that it is the Allegiance page. That is the #372 class ("the panel mounts but the pages are wrong/blank") stated as an assertion rather than a hope, and it is one line. Everything else the probe asserts is real and does cover the classes it claims: non-empty tab captions (the #375 resolver class — I re-ran it and it passes), the four pages resolving, the fellowship frame pair, and the six allegiance signature elements. All pass against the installed DATs. ### SF-4 — "all three visible option checkboxes" is four `src/AcDream.App/UI/Layout/SocialFellowshipPageController.cs:19`. Frame `0x1000026B` authors **four** `Type 0x10000035` children — `0x10000270` (IgnoreFellowshipRequests), `0x10000271` (FellowshipAutoAcceptRequests), `0x10000272` (FellowshipShareXP), `0x10000273` (FellowshipShareLoot) — per lane A §3.1 and confirmed in the fixture. The gate script step 11 (`:73-82`) hedges with "a fourth, Share Loot, may also be present" when both the decomp and the committed fixture already settle it. Fix the count in both places; a gate script that is unsure about what the user will see invites noise. ### SF-5 — AD-79's pinning test does not cover the control AD-79 itself enumerates The register row names seven controls, including the Friends page's "Appear Offline"-shaped checkbox `0x1000052C`, and cites `SocialPanelControllerTests.FriendsAndSquelchActionButtons_AreClickable_ButHaveNoHandler` as what pins the INERT contract. That test (`SocialPanelControllerTests.cs:272-291`) covers only the six **buttons**. `0x1000052C` builds through `DatWidgetFactory.BuildCheckbox` as a `UiButton` (`DatWidgetFactory.cs:785-816`), so it can be added to the existing loop verbatim. Seven enumerated, six pinned is the kind of drift AD-78's own "Risk" column warns about. ### SF-6 — the two page controllers do not cite AD-79 by id `SocialFriendsPageController.cs:23-25` and `SocialSquelchPageController.cs:21-22` both say "See this commit's single register row…". AD-79's own *Where* column asserts "both classes' own doc comments cite this row". Literally true, practically not: a future reader grepping `AD-79` will not find the code. Name the row. ### SF-7 — the allegiance empty state is coarser than the contract's own source (item 4) FA3's contract line is "all four pages' **empty states**", and lane A §4.5 is what defines the allegiance one. I re-read and byte-verified that section's claims because it is what the slice implements against: - `gmAllegianceUI::UpdateMonarchData @0x00491B40` branches on `(monarchId == player_id || monarchId == 0)` — retail hides the monarch block when there is no monarch **or the monarch is you**, then sets **both** `m_pMonarchName` *and* `m_pMonarchFollowers` to the literal at `data_794358`. - **`data_794358` byte-verified** in the PDB-paired binary (`C:\Users\erikn\Downloads\acclient.exe`, image base `0x400000`, RVA `0x394358` → file offset `0x394358`): bytes `20 00 00 00` = UTF-16LE `" "`, one character. So lane A's `L" "` reading is correct and FA3's `BlankLine = [new UiText.Line(" ", …)]` (`SocialAllegiancePageController.cs:40-41`) matches it exactly — **a single space, not an empty string**, as asserted at `SocialPanelControllerTests.cs:177-178`. This half is clean. What is coarser: FA3 gates **both** blocks on `Snapshot.HasProfile` rather than per-relationship. The consequence is not hypothetical — the moment any `0x0020 AllegianceUpdate` lands, a monarch character sees an **empty, visible** monarch block that retail hides, and anyone whose patron is the monarch sees an empty patron block instead of retail's relabelled-monarch presentation. FA2 already ships the data needed to do it properly: `IRuntimeAllegianceView.TryGetMonarch` / `TryGetPatron` (`GameRuntimeGameplayViews.cs:200-204`) plus `Snapshot.MonarchGuid`; the FA3 `Callbacks` record deliberately narrows the input to `Func` (`SocialPanelController.cs:90-96`), which is what forecloses it. I am **not** calling this a MUST-FIX: FA3 is a shell slice, the class doc is honest about the simplification, and MF-2's finding means `HasProfile` is effectively always `false` for the whole FA3 gate, so nothing wrong is visible *during this gate*. But it is a contract gap that will be invisible by the time FA5 lands unless it is written down. Either widen the callback to the view and implement the real per-relationship gate now, or add an explicit FA5 acceptance line ("the monarch block hides when the monarch is you; the patron block hides when the patron is the monarch") to the plan's FA5 row so it cannot be lost. Related, and worth a line in the class doc: `Tick()` reassigns `LinesProvider` **unconditionally every frame**, so any FA5 code that sets a real monarch/patron name on `0x10000257`/`0x1000025C` will be overwritten on the next frame unless this method is changed at the same time. ### SF-8 — `Tick()` ignores `_disposed` `SocialPanelController.Dispose()` (`:224-228`) sets `_disposed` and nothing reads it. The handle passes `Controller = controller` to `RetailWindowFrame.Mount`, and `RetailUiRuntime.SocialPanelController` is never nulled, so `RetailUiRuntime.Tick`'s `SocialPanelController?.Tick()` (`RetailUiRuntime.cs:559`) will keep ticking the four page controllers after disposal. Nothing throws today (the Runtime owners outlive the panel and the J-owner snapshot getters are disposed-guarded), so this is low severity — but "tick a disposed controller" is precisely the class the J-slices spent commits eliminating. One `if (_disposed) return;` closes it. ### SF-9 — `SocialPanelRowText.FindDeepest` documents a stronger guarantee than it implements `SocialPanelRowText.cs:19-31` promises "the deepest `UiText` descendant (pre-order, last match wins)". The implementation returns the **last match in traversal order**, which equals the deepest only when the subtree is a single chain. Both real templates *are* single chains — I dumped them from the installed DATs: ``` FriendsRow 0x10000519 Type 3 (270x24) -> 0x1000051A Type 0xC -> 0x1000051F Type 0xC SquelchRow 0x10000541 Type 3 (270x24) -> 0x10000542 Type 0xC -> 0x1000054D Type 0xC ``` so the behaviour is correct today and the class doc's honesty about `gmFriendsUI`/`gmSquelchUI` being outside the campaign's decompiled scope is exactly the right posture. Just make the doc describe what the code does, or make the code track depth. --- ## 2. What verified clean Stated plainly, because most of this slice is right. **The tab-table correction (item 1).** Confirmed three independent ways (§1 above). The addendum in `d7e1cffd` is accurate, and correcting in place while leaving the original text visible follows the FA1/FA2 convention. **§6 Campaign-OP lessons compliance (item 3).** Each one re-derived: - *String resolver on every `Build`.* `MountSocialPanel` passes `resolver.Resolve` to the main `LayoutImporter.Build` (`RetailUiRuntime.cs:2611-2617`) **and** its nested `TemplateResolver` builds each row template with its own `strings.Resolve` (`:2626-2645`). Both covered — the #375 class. The probe's non-empty-caption assertion is the live check, and it passes. - *Scoped `FindDescendant` for every repeating id.* I enumerated **all** duplicate element ids in the panel subtree rather than trusting the stated one. Duplicates exist at `0x10000439`/`0x100000E9`/`0x10000215` (×4, tab button art), `0x100002CE`–`0x100002D0` (×16, button 3-slice art), `0x10000328` (×6, checkbox art), `0x00000001`/`0x10000071`/`0x10000072`/ `0x10000364`–`0x10000366` (×4, scrollbar parts), `0x10000490`/`0x10000491`/ `0x10000492` (×2, the two allegiance blocks), and — not previously called out anywhere — **`0x10000254`, `0x1000025D` and `0x1000051E` repeat ACROSS pages** (`0x10000254` under both the Friends page and the allegiance monarch block; `0x1000025D` under the Friends page and the patron block; `0x1000051E` under both Friends and Squelch). Every FA3 lookup is safe: the four page ids, the two fellowship frames, both allegiance field containers, both name texts and both ListBox ids are each **unique**, the per-page controllers scope to their page root anyway, and the single flat `layout.FindElement(0x10000290)` for the close button targets a unique id. `0x10000492` is not looked up at all in FA3, so the intra-panel duplication cannot bite yet — but the three cross-page duplicates above are worth carrying into FA5's notes, since FA5 will be resolving allegiance-page ids. - *Tab activation performed.* `controller.ActivateTabs()` at `RetailUiRuntime.cs:2662`, after `Bind` — the OP3 ordering constraint. Verified live: `UnresolvedEntries` is empty and `IsShowingAllegiance` is true after activation. - *No lazily-created fill-anchored 0×0 children (the #372 class).* The new controllers create no elements at all. The only lazy child is `UiTemplateListBox`'s viewport, which already seeds `Width`/`Height` from the ListBox's authored extent (`UiTemplateListBox.cs:152-182`, the #372 fix), and both social ListBoxes carry real authored extents (Friends `270x400`, Squelch `270x430`) at `Bind` time. - *Same-layout template-prototype skip.* Checked for this layout specifically: all four `P0x64` arrays point at **other** LayoutDescs — Friends `0x2100005D/0x10000519`, Squelch `0x21000060/0x10000541`, fellows `0x21000030/0x10000281`, vassals `0x2100002F/0x10000266`. The importer's skip is same-LAYOUT only (`LayoutImporter.cs:233-249`), and in any case `MountSocialPanel` uses the single-root `ImportInfos(dats, layoutId, rootElementId)` overload, which never enumerates top-level siblings. No parked prototypes are possible here. - *No hand-rolled scroll viewport.* Both lists ride the existing `UiScrollablePanel` through `UiTemplateListBox`, so the #371 straddling-row clip comes for free. Popup/pointer-priority rules are not applicable. **`UiTemplateListBox.Flush()`.** The doc claim "resetting `ContentHeight` to 0" is accurate: `UiScrollablePanel.ClearContent()` (`:54-61`) removes every child, clears `_baseTops`, sets `ContentHeight = 0` and resets the scroll offset. The dormancy guard (`_viewport?.`) is genuine — a never-populated box still never allocates its viewport. The shrink case the Friends/Squelch rebuild needs therefore works. **Fellowship empty-state frame swap (item 4).** Retail anchor re-read at `gmFellowshipUI::Update @0x0048F440`: when `ClientFellowshipSystem::GetFellowshipSystem()->m_pFellowship == 0` it calls `m_pInAFellowshipFrame->SetVisible(0)` then `m_pNotInAFellowshipFrame->SetVisible(1)` — exactly the two writes `SocialFellowshipPageController.Tick` makes, driven off `RuntimeFellowshipSnapshot.IsInFellowship`. Lane-A unknown **U6 is genuinely closed**: page `0x10000292` has **exactly two children**, the two frames, so a single `Visible` toggle per frame really is the whole swap. (Frame contents: `0x1000026B` = name box `0x1000026F`, Create `0x10000274`, four checkboxes; `0x10000275` = roster ListBox `0x10000279`, scrollbar, six buttons `0x1000027B`–`0x10000280`.) **Friends/Squelch read-only binding (item 5).** Bound to the J4.1 owners by reference — `RuntimeCommunicationState.Friends`/`.Squelch` are get-only properties constructed once (`RuntimeCommunicationState.cs:67-68,132-133`) and **cleared in place** at reset (`:250-251`), so holding the instance across a generation reset is correct, and `FriendsState.Clear()` bumps `Revision` (`FriendsState.cs:67-74`) so the panel rebuilds to empty on its own. No new state copies; `Snapshot()`'s array copy only runs on a revision change. The per-frame cost when nothing changed is one `Interlocked.Read` per list. The D1 INERT contract holds: checkboxes build as `UiButton` with a null `OnClick` (no latching visual state without a handler), and the six buttons are pinned null by test. **Panel exclusivity + catalog (item 6).** `SocialPanel = 12` is byte-verified (root `P0x10000029 = 12`). Registered in `Mounted` only. The toolbar seam genuinely tolerates it: `SyncToolbarWindowButtons` iterates `ToolbarPanels` only (`RetailUiRuntime.cs:809-814`), so no phantom button; `BindPanelButtons` binds by authored toolbar element, and the toolbar authors no `0x10000029 = 12` button; and the one path that *does* reach the toolbar with a non-toolbar id — `OnWindowVisibilityChanged` → `ToolbarController.SetPanelOpen(12, …)` (`:831-832`) — walks `_panelButtons`, finds nothing, and returns (`ToolbarController.cs:394-406`). This is the same path `PositiveEffects` (4), `LinkStatus` (8), `MiniGame` (9) and `Vitae` (15) already take. No crash, no greyed phantom. Both open paths keep `RetailPanelUiController` state coherent: `ToggleWindow` and `CloseWindow` both route through `TryGetPanelId` → `_panelUi` (`:708-711`, `:801-807`). **Window-frame policy.** `RetailWindowFrame.Options` for the social panel is byte-for-byte the Options panel's post-review policy (ResizeX=false, bottom-edge-only, both constrain flags, full ContentAnchors, `DrawChromeCenter = !AuthorsFullPanelCenter(rootInfo)`), differing only in `Left`/`Top`. No invented divergence in the shared main-panel cohort. The root/page height mismatch (root `300x362`, pages `300x575`) is **identical** to the already-gated Options panel's own authored shape, so it is not an FA3 property. **The live-mount probe run (item 7).** Executed with `ACDREAM_PROBE_LIVE_MOUNT=1` against the installed DATs: **1 passed, 0 failed**. Every assertion holds. Given #383 (installed-DAT vs committed-fixture drift), I also cross-checked the committed social-panel fixture against the probe's live dump on root extent, child count and order, the tab table, all four page `P0x57` values, and both allegiance blocks' geometry (including the two *differently sized* `0x10000490` instances, monarch `(210,0 90x36)` vs patron `(220,0 80x36)`). **No drift** — this fixture is faithful to the installed DAT. **Tests.** `SocialPanelControllerTests` + `RetailPanelCatalogTests`: 22 passed. Full `AcDream.App.Tests` on the committed Release binaries: **4,871 passed / 3 skipped / 0 failed** — no collateral regressions. The ledger's `+18` arithmetic checks out exactly (13 `SocialPanelControllerTests` + 4 `RetailPanelCatalogTests` + 1 probe). I did not re-run the full 13,233-test solution suite (no builds permitted in this review), so that figure is unverified but arithmetically consistent. **Register bookkeeping.** AD-79's header count is right — the table contains exactly 59 `AD-` rows, matching "59 active rows", up one from 58. The row's element enumeration is accurate against the fixture (Friends `0x10000514`/ `0x10000515`/`0x10000516` + `0x1000052C`; Squelch `0x10000547`/`0x1000054B`/ `0x1000054C`), and its honesty about `gmFriendsUI`/`gmSquelchUI` being outside the campaign's researched scope is the right framing. Only SF-5/SF-6 above. **Gate-script spot checks (item 8).** Steps 1–3 (F3 opens on Allegiance; repeat F3 closes; F4 opens directly on Fellowship), step 4 (F3 while open on Fellowship switches rather than closes) and step 5 (the authored X at `0x10000290`, a direct root child at `(276,0) 24x25`, always visible because it is not inside any page, wired to `ToggleWindow`) all match `RetailUiRuntime.OpenSocialPanel` (`:603-620`) and `SocialPanelController.Bind`'s close wiring (`:145-150`) exactly. Steps 7–8's exclusivity claims match `RetailPanelUiController.SetPanelVisibility`'s one-active-panel logic. Step 6's geometry claims match the frame options. The two claims that do **not** hold are MF-1 and MF-2. One wording nit for whoever edits the script: step 5 calls the X "top-right of the window chrome" when it is an authored panel element, not chrome — harmless, but a user hunting the chrome's own close affordance may be briefly confused. --- ## 3. Disposition APPROVE-WITH-FIXES. MF-1 and MF-2 must land before the user runs the gate — both are in `docs/research/2026-08-12-campaign-fa-test-script.md` and neither touches code. SF-1 through SF-9 are code/doc/test corrections that can ride the same fix round. Nothing here warrants a re-review of the mount path itself: the tab table, the frame swap, the string resolver, the scoped lookups, the tab activation, the catalog registration and the toolbar seam are all correct, and the live-mount probe passes against the installed DATs. --- # 4. NARROW RE-REVIEW — 2026-08-12 — **CLOSED, no reopen** Fix round `9afa05b5` / `35c40a9b` / `ae772709` / `a5553904` / `c5f73744`. Every disposition below was re-derived from the actual diffs, not from the commit messages. **All 2 MUST-FIX and all 9 SHOULD-FIX are correctly applied. No finding is reopened. No new finding rises to MUST-FIX or SHOULD-FIX.** ## 4.1 Mechanism dispositions | # | Verdict | Evidence | |---|---|---| | **MF-1** tab x-order in the gate script | **FIXED** | Steps 1, 9, 10 and the "What to report" bullet now all read **Allegiance, Fellowship, Friends, Squelch**, and step 1 states the point I asked for — "the default tab and the left-most tab are the SAME tab". Each correction carries an inline `[FA3 fix-round correction, mechanism MF-1]` note naming the earlier draft's error, matching the campaign's correct-in-place convention. | | **MF-2** `@allegiance info` false-defect route | **FIXED, and inverted correctly** | Step 14 now states the true FA3 expectation in full: `@allegiance info` still prints real chat data, the panel blocks stay hidden all gate, "report NEITHER half as a bug". The "What to report" bullet is inverted from "blocks staying hidden" to "**blocks becoming VISIBLE at ANY point** … there is no code path in FA3 that can make that happen". That is the right anomaly to point the user at, and it cites the FA2 SHA (`4272ad0e`) for why. | | **SF-1** unsupported retail claim on F3/F4 | **FIXED** | `SocialPanelController.cs:200-215` relabels it as acdream's own `OpenSpellbook`-precedent convention and reproduces all three negative findings (no `GetAttribute_Enum(this, 0x57, …)` site; `ClientUISystem::OnAction`'s `>0x7c` branch handles three unrelated ids; `gmPanelUI::ListenToGlobalMessage` COMDAT-folded onto a no-op). **U11 filed** in `fa-panel-structure.md` §8 with the same evidence and an honest "cdb trace is the only remaining way" recipe. I also checked the no-register-row argument rather than taking it: grepping the register for `Toggle*Panel` / close-on-second-press returns **nothing** — no row exists for the spellbook or any other non-toolbar toggle, so "following the existing no-row precedent" is factually true, not a convenient claim. | | **SF-2** per-frame closure allocation | **FIXED** | Two `static readonly Func<…>` fields (`BlankLineProvider`/`NoLinesProvider`, `SocialAllegiancePageController.cs:59-67`); `Tick` now selects one and does a plain field write. Both lambdas close over `static readonly` fields only, so each is a single cached instance — the tick is genuinely zero-allocation now. | | **SF-3** printed-but-unasserted probe findings | **FIXED, and better than asked** | `Assert.Equal(2, passupCount)` added; page exclusivity promoted to a real assertion that also pins **which** page (`Assert.Equal("Allegiance", visiblePageName)`) and fails loudly with both offending page names if two are visible. Re-ran live (§4.3): `Allegiance Visible=True`, other three `False`, `0x10000492 occurrences = 2`. | | **SF-4** "three checkboxes" is four | **FIXED in both places** | `SocialFellowshipPageController.cs:19-21` now says FOUR and names `0x10000270`–`0x10000273`; gate-script step 11 drops the "a fourth may also be present" hedge. | | **SF-5** AD-79 pins six of its seven controls | **FIXED** | `0x1000052C` added to the existing loop in `FriendsAndSquelchActionButtons_AreClickable_ButHaveNoHandler`, with a doc comment explaining why the checkbox slots into the button loop. Seven enumerated, seven pinned. | | **SF-6** controllers don't cite AD-79 by id | **FIXED** | Both classes now read "See register row AD-79 (`docs/architecture/retail-divergence-register.md`)". Greppable in both directions. | | **SF-7** allegiance gate coarser than retail's | **FIXED via the documented alternative** | Taken as the plan-line option I offered rather than a code change — correct call for a shell slice. The FA5 row now carries an explicit MUST: widen `Callbacks.AllegianceSnapshot` to reach `TryGetMonarch`/`TryGetPatron`, "a monarch character must NOT see an empty, visible monarch block". The class doc (`:37-49`) repeats it **and** carries the forward hazard I flagged — that FA5's name population must change `Tick`'s unconditional `LinesProvider` reassignment in the same commit. | | **SF-8** `Tick()` ignores `_disposed` | **FIXED** | `if (_disposed) return;` is the first line of `SocialPanelController.Tick`. | | **SF-9** `FindDeepest` doc overstates | **FIXED** | Rewritten to "the LAST `UiText` found by a pre-order walk", with the single-chain caveat and the live-dumped shape of both real templates. Describes what the code does. | ## 4.2 Blast dispositions — spot-verified - **Scrollbar wiring (blast MF-1).** Both controllers resolve their authored sibling scrollbar scoped to the page root and assign `scrollbar.Model = listBox.Scroll`, with a log line on miss. Verified the seam rather than assuming it: `UiTemplateListBox.Scroll => Viewport.Scroll` (`:107`) forces the lazily-created viewport, which is already extent-seeded by the #372 fix and returns one stable `UiScrollable`, so wiring before the first row exists is sound. Type `0xB` really does build as `UiScrollbar` (`DatWidgetFactory.cs:122`), the ids `0x10000518`/`0x10000543` match each ListBox's own authored `ScrollbarElementId` (probe-confirmed), and `scrollbar.Model = listBox.Scroll` is verbatim the shape `CharacterOptionsPageController:323` / `ChatOptionsPageController:293` / `ConfigOptionsPageController:397` already use. The four new tests pin both the wiring (`Assert.Same`) and real reachability (40 rows exceeding the authored extent, `HasOverflow`, scroll actually advances). - **Template cache (blast SF-2).** `rowTemplateCache` memoizes the per-template `ElementInfo?` (nulls included, so a permanent miss cannot re-walk the DAT) while still `Build`-ing a fresh widget per row under the DAT lock. I checked the soundness question this introduces — whether re-`Build`ing from one shared `ElementInfo` is safe: `Build` is the documented pure builder and mutates nothing; the only mutating entry point is `BuildFromInfos` (`LayoutImporter.cs:68-78`, a tests-only convenience that overwrites `rootInfo.Children`), which this path does not call. Safe. - **Visibility-gated rebuild (blast SF-2).** `OnShown`/`OnHidden` are real `IRetainedPanelController` members (`IRetainedPanelController.cs:12-16`) and are genuinely driven — `RetailWindowHandle.NotifyVisibility` (`:92-108`) fires them on every transition, and `AttachController`/`NotifyInitialState` correctly do NOT fire `OnShown` for a window mounted `Visible = false`. So `_visible` starts false in production exactly as the new test simulates. Fellowship/Allegiance deliberately stay ungated (no DAT access) — right split. `Friends_RevisionBumpWhileHidden_DoesNotRebuild_ButRebuildsOnShow` pins both halves, including that the bump is deferred rather than lost. - **Revision-latch ordering (blast SF-3).** `_lastRevision` now advances only when every row resolved, so a transient resolver miss retries instead of latching an empty list until the next server-side change. Checked the empty -roster case: the loop body never runs, `allRowsResolved` stays true, the revision latches — no spurious rebuild loop on an empty friends list. - **`Flush` doc (blast SF-5).** Corrected to include the scroll reset, and it goes further than the finding asked by naming the resulting UX cost (a scrolled user yanked to the top on every rebuild), explaining why it was NOT silently fixed, and flagging it forward. That is the right disposition for an unasked behavior change. - **#383 narrowing (blast SF-6).** I re-derived the timestamps rather than trusting them: `keyboard_config_21000009.json` last written at `b4edee97` (2026-08-11 09:19), `options_2100002B.json` at `e71e5a96` (2026-08-11 06:25), FA3's fixture commit `74c3d85d` at 2026-08-12 02:58 — ~17h39m and ~20h33m earlier, the previous day. "~18h and ~21h, same day" is accurate, and it does materially tighten the investigation window. The addendum correctly attributes the new fixture's no-drift confirmation and narrows the issue to the two OP-era fixtures. - **Two new gate steps (blast N-8).** Step 19 (long-roster scroll) targets exactly where the scrollbar fix bites and is honestly marked soft when the roster is too short; its arithmetic is right (Friends box `270x400` inside a `300x362` panel). Step 21 (restore-open across relaunch) is stated as expected-not-a-bug, and its premise checks out — `stateManagedVisibilityWindows` is still `[Combat, JumpPowerbar, ExternalContainer, Vendor]` (`RetailUiRuntime.cs:440-446`), so the social panel restores like every sibling main panel. ## 4.3 Gates re-run on the post-fix binaries Release binaries timestamp 2026-08-12 03:44, i.e. at/after the last code commit (`ae772709`, 03:44) — the runs below are against the fixed code. - **Live-mount probe** (`ACDREAM_PROBE_LIVE_MOUNT=1`, installed DATs): **1 passed / 0 failed.** The two promoted assertions pass with observable values — `page Allegiance … Visible=True`, Friends/Fellowship/Squelch `Visible=False`, `0x10000492 occurrences under allegiance page = 2` — and all four captions still resolve (`Allegiance`/`Fellowship`/`Friends`/`Squelch`). - **`AcDream.App.Tests`, Release, `--no-build`: 4,876 passed / 3 skipped / 0 failed**, exactly `+5` over the 4,871 I measured pre-fix-round and exactly the ledger's own per-project App figure. The `+5` reconciles: five genuinely new `[Fact]`s (two scrollbar-wiring, two long-roster reachability, one hidden-vs-shown rebuild), with the two extended-in-place tests and the two new probe assertions correctly counted as net zero. The 13,238/4/0 solution total is therefore consistent at the only project this fix round touched; I did not re-run the other eight (no builds permitted). - **Blast radius**: `git diff --stat 9e622f56..c5f73744` touches 14 files, all FA3's own plus doc-only edits to the shared `UiTemplateListBox` and the `MountSocialPanel` method. No collateral edits. ## 4.4 Carry-forward (non-blocking, for FA4/FA5 — not reopened findings) 1. **Scroll position resets on every rebuild.** Correctly deferred and documented in `UiTemplateListBox.Flush`. It bites harder in FA4, whose fellow roster rebuilds on every vitals tick rather than on a rare roster change — worth resolving there rather than deferring again. 2. **The production template cache is untested.** Both long-roster tests use `FakeRowTemplateResolver`, so `rowTemplateCache` itself has no coverage. I verified its soundness by inspection (§4.2), but a rebuild-twice assertion in the live-mount probe would cost one line and cover the real path. 3. **Hardcoded scrollbar ids.** `0x10000518`/`0x10000543` are literals when `UiTemplateListBox.ScrollbarElementId` already carries the authored value. Both match today (probe-confirmed) and this mirrors the existing Options-tab consumers, so it is a cohort-wide nit, not an FA3 defect. 4. **`allRowsResolved` retry loop.** A permanently unresolvable template makes the list rebuild every frame while the panel is open. Bounded and cheap (the cache stores the null, so no DAT walk; `Build` is skipped entirely) — recorded so it is not rediscovered as a mystery. ## 4.5 Verdict **CLOSED — no reopen.** Every mechanism finding is applied, and each was applied at the right altitude: the two MUST-FIX as script rewrites that state the verified truth rather than merely deleting the wrong claim, SF-7 as a binding FA5 acceptance line rather than scope creep into a shell slice, and SF-3 as assertions that pin more than the finding asked for. The fix round also left the two things it chose not to change (the Flush scroll reset, the coarser allegiance gate) documented and forward-flagged rather than silent, which is the disposition this project's register rules ask for. FA3's remaining obligation is unchanged: the user's connected gate, now against a script that no longer contains two instructions guaranteed to produce false defect reports.