acdream/docs/research/2026-08-12-fa3-review-mechanism.md
Erik bf07b70ef1 docs: FA3 mechanism re-review -- CLOSED, no reopen (4 carry-forwards)
Narrow re-review of the FA3 fix round (9afa05b5/35c40a9b/ae772709/
a5553904/c5f73744) against this doc's 2 MUST-FIX and 9 SHOULD-FIX. Every
disposition re-derived from the diffs, not from the commit messages. All
11 correctly applied; nothing reopened; no new MUST-FIX or SHOULD-FIX.

Dispositions worth naming: MF-1/MF-2 were fixed as script REWRITES that
state the verified truth (Allegiance is both the default and the
left-most tab; @allegiance info prints chat data AND the blocks stay
hidden, report neither) rather than deletions, and MF-2's report-bullet
is correctly inverted to "blocks becoming VISIBLE is the anomaly".
SF-3's assertions pin more than asked (which page is visible, not just
that one is). SF-7 landed as a binding FA5 acceptance line plus the
forward hazard about Tick()'s unconditional LinesProvider reassignment,
the right altitude for a shell slice.

Claims verified rather than accepted: SF-1's no-register-row precedent
(grepped -- no row exists for any Toggle*Panel close-on-second-press, so
the precedent is real); the #383 timestamp correction (b4edee97
2026-08-11 09:19, e71e5a96 06:25, 74c3d85d 2026-08-12 02:58 = ~17h39m
and ~20h33m, previous day); OnShown/OnHidden really are driven by
RetailWindowHandle.NotifyVisibility and correctly do NOT fire for a
window mounted Visible=false; UiTemplateListBox.Scroll forces the
extent-seeded viewport so pre-row scrollbar wiring is sound; and the new
template cache is safe because Build is the pure builder -- only
BuildFromInfos (tests-only) mutates the ElementInfo it is handed.

Gates re-run on the post-fix Release binaries: live-mount probe passes
against the installed DATs with the promoted assertions showing
Allegiance Visible=True (other three False) and 0x10000492 count = 2;
AcDream.App.Tests 4,876 passed / 3 skipped / 0 failed, exactly +5 over
the pre-fix 4,871 and exactly the ledger's App figure, so the 13,238/4/0
+5 reconciles at the only project this round touched. Blast radius is 14
files, all FA3's own plus doc-only edits to UiTemplateListBox and
MountSocialPanel.

Carry-forwards (non-blocking): the Flush scroll-position reset will bite
harder in FA4's per-vitals-tick roster rebuild; the production template
cache has no test (both long-roster tests use the fake resolver); the
scrollbar element ids are literals where ScrollbarElementId carries the
authored value (cohort-wide nit); and the allRowsResolved retry rebuilds
per frame on a permanently unresolvable template (bounded and cheap).

FA3's remaining obligation is unchanged: the user's connected gate,
against a script that no longer contains two instructions guaranteed to
produce false defect reports.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-12 03:52:08 +02:00

40 KiB
Raw Permalink Blame History

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 (0x1000028C0x10000291) 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:

IReadOnlyList<UiText.Line> 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<IReadOnlyList<UiText.Line>> 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<RuntimeAllegianceSnapshot> (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), 0x100002CE0x100002D0 (×16, button 3-slice art), 0x10000328 (×6, checkbox art), 0x00000001/0x10000071/0x10000072/ 0x100003640x10000366 (×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 0x1000027B0x10000280.)

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 — OnWindowVisibilityChangedToolbarController.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 13 (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 78'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 0x100002700x10000273; 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-Building 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.