acdream/docs/research/2026-08-12-fa4-review-blast.md

Campaign FA slice FA4 — BLAST-RADIUS review

Reviewer lens: the axes the implementer may not have traversed — enum leaks, existing-consumer regressions, interface-implementer coverage, generation-seam consistency, confirmation-ordering, dimming bookkeeping, suite accounting, register/ledger/gate-script/digest consistency, and the FA2/FA3 live paths' survival.

Target commits (branch claude/latest-commits-cb0c8f):

• 357d2032 feat(runtime): fellowship roster enumeration + Social selection source
• 5bdd0528 feat(ui): fellowship page fully live
• 38f08314 docs(fa4): register rows AD-80/AD-81, AD-78 addendum, gate script, ledger

Verdict: APPROVE-WITH-FIXES (1 SHOULD-FIX, 1 minor observation). Every code-level blast axis verified clean. The single fix is a stale count in a binding doc (the divergence register). No MUST-FIX, no REJECT.

---

VERDICT DETAIL

SHOULD-FIX 1 — the AD-78 register row still says "35 of 50 rows dimmed" (stale post-D7)

docs/architecture/retail-divergence-register.md, the AD-78 row's file-path column, still reads verbatim:

CharacterOptionsPageController.cs (35 of 50 rows dimmed — RowSpec.StoreOnly, …)

FA4 D7 flipped four rows (IgnoreFellowshipRequests, FellowshipAutoAcceptRequests, FellowshipShareXP, FellowshipShareLoot) from StoreOnly to Live, so the true count is now 31 of 50 dimmed / 19 live. The code (CharacterOptionsPageController.Groups), the class-doc derivation table (CharacterOptionsPageController.cs doc comment, updated in 5bdd0528), and the conformance test (CharacterOptionsPageControllerTests, now asserts 31/19) were all updated. The register row was not — it carries no D7/FA4 addendum and no updated count (confirmed: the only of 50 token in the AD-78 row is 35 of 50).

Why this matters and isn't a nit: the register is CLAUDE.md's "single auditable list of every KNOWN place acdream's runtime behavior can deviate from retail," and its discipline requires the commit that changes a deviation to update its row in the same commit. AD-78's own "Risk" column explicitly warns that dim/live classification drift "in the OPPOSITE direction it was built to prevent" is "a real defect, not a rendering nit." A wrong count inside the register row that governs the convention is exactly that drift, internal to the register itself. The 38f08314 commit message asserts "AD-78's derivation table gains its D7 addendum," but that addendum landed in the class doc (5bdd0528), not in the register row.

Fix: update the AD-78 register row's CharacterOptionsPageController.cs parenthetical to "31 of 50 rows dimmed" (and, optionally, a one-clause D7 note naming the four un-dimmed ids), mirroring the class-doc change.

Observation A (minor, non-blocking) — GetMembers is the first allocating member on IRuntimeFellowshipView, and the controller re-materializes it per vitals tick

RuntimeFellowshipState.FellowshipView.GetMembers() materializes a fresh RuntimeFellowMemberSnapshot[] under the gate on every call — correct and race-safe, and the interface doc is honest that it is "a materialized array (not a lazy iterator)." But it is the first allocating member on that view: Snapshot is a value-returned record struct and TryGetMember is an out-param, both 0-alloc. On the consumer side, SocialFellowshipPageController.RefreshRoster additionally wraps the result in new List<>(_bindings.Members()) (SocialFellowshipPageController.cs:549) and runs on every revision bump — and the controller's own doc (lines 19-36) states the revision bumps on every 0x02C0 vitals refresh. So while the fellowship panel is open with N members, each incoming vitals tick costs two bounded (N ≤ 9) allocations on the UI thread.

This is off the physics/render hot path, panel-gated, and bounded — it does not threaten the Slice-I/J frame budgets, so it does not block approval. Noting it because (a) lens 3 asked for the allocation profile explicitly, and (b) the per-tick List copy is avoidable: the diff at RefreshRoster only needs a count + guid-membership comparison, which can walk the IEnumerable directly without materializing a List (the array from GetMembers is already a fresh snapshot). A future tidy, not a fix this slice owes.

---

VERIFIED CLEAN, per lens

Lens 1 — SelectionChangeSource.Social enum leak: CLEAN

• No exhaustive switch/pattern-match on SelectionChangeSource exists anywhere in src/ (grep for case SelectionChangeSource. / switch … SelectionChangeSource / is SelectionChangeSource. → zero matches). Adding a value cannot silently mis-route.
• Every SelectionTransition consumer either discards the transition (RuntimeActionState, ExternalContainerController, PaperdollController, InventoryController, SpellbookWindowController, ToolbarController, SpellcastingUiController take SelectionTransition _; SelectedObjectController and VendorUiController do _ = transition;) or reads only SelectedObjectId/Reason, never Source:
  • RuntimeCombatTargetState.OnSelectionChanged reads SelectedObjectId/Reason only — a Social-sourced selection cannot mis-target combat.
  • AppraisalUiController.HandleSelectionChanged reacts to any non-null SelectedObjectId (window-visible-gated) — a fellow-row click behaves like any other world selection, which is the intended "clicking a fellow selects them in the world" mechanism (SelectFellow → Select(guid, Social)).
• Plugin-facing SelectionChangedEvent carries no Source, so Social is invisible to plugins. No mis-targeting surface.

Lens 2 — UiTemplateListBox.FlushPreservingScroll vs Flush + existing consumers: CLEAN

• Additive. Flush()'s body is unchanged (_viewport?.ClearContent()); only its doc comment expanded. FlushPreservingScroll is new.
• Only one caller. FlushPreservingScroll is called solely by SocialFellowshipPageController.RebuildRoster. The Options/Keyboard/Chat controllers never call Flush or FlushPreservingScroll at all — their ListBoxes are built once (AddItemFromTemplateList/AddPrebuiltRow) and never torn down — so the new method has zero effect on them. Only Friends/Squelch/Fellowship touch these methods.
• Shrink-below-offset math is correct. FlushPreservingScroll saves Scroll.ScrollY, calls ClearContent() (which zeroes the panel's ContentHeight and Scroll.SetScrollY(0) but leaves Scroll.ContentHeight/ ViewHeight at their stale values — those are only refreshed in LayoutScrollableChildren), then re-SetScrollY(saved) — clamped against the still-stale extents, so saved (already ≤ old MaxScroll) is preserved exactly. On the next OnDraw, LayoutScrollableChildren runs before base.OnDraw and sets Scroll.ContentHeight = ContentHeight (the fresh, possibly-smaller height) then SetScrollY(ScrollY), which re-clamps to [0, max(0, freshHeight - view)]. So a roster that shrinks below the old offset is clamped to the valid range before any row is painted — the "clamp now, correct at next layout" contract the doc claims, verified against UiScrollable.SetScrollY/MaxScroll and UiScrollablePanel.LayoutScrollableChildren.
• #371 clip/cull untouched. The straddling-row intersection cull lives in LayoutScrollableChildren; FlushPreservingScroll changes only the offset, not the cull.

Lens 3 — IRuntimeFellowshipView.GetMembers() implementer/double coverage: CLEAN (see Observation A for allocation)

• Single concrete implementer: RuntimeFellowshipState's nested FellowshipView (: IRuntimeFellowshipView, the only :-implements in the tree). It returns a materialized array — never null, an empty array when not in a fellowship (test GetMembers_EmptyWhenNotInAFellowship pins this). Build-green proves no other implementer went stale.
• Test doubles are property-return null, not interface impls: the two doubles (GameplayInputCommandControllerTests, InteractionUiRuntimeSourcesTests) expose IRuntimeFellowshipView Fellowship as => throw/=> null! on the host interface — they do not implement IRuntimeFellowshipView, so the new member cannot break them, and the suite passing shows neither is exercised through GetMembers.
• Production wiring never yields null: InteractionRetainedUiComposition binds () => d.Runtime.Fellowship.GetMembers(), and GameRuntime.Fellowship → FellowshipOwner.View is the concrete impl. No headless/bot view returns null here.

Lens 4 — DeferredGameRuntimeStateCommands.Fellowship* seam + adapter consistency: CLEAN

• All seven wrappers route through Invoke((commands, generation) => commands.Fellowship.X(generation, …)). Invoke captures _commands and _view.Generation together under _gate and returns RuntimeCommandStatus.Inactive when deactivated or unbound — the identical generation-race-safe shape as AddShortcut/ Advance. A displaced session (reconnect mid-click) cannot receive a stale-generation fellowship action.
• No auto-fire. Commands fire only from explicit button OnClick handlers and SetPageVisible transitions. SetPanelOpen (0x00A6) is caller-idempotent (only on an actual _pageVisible flip) and a no-op while disconnected (returns Inactive).
• Adapter consistency: the underlying IRuntimeFellowshipCommands (all seven methods, incl. SetOpen/SetPanelOpen) predates FA4 — it was created at FA2 (369729f0) and is unchanged in these commits. Both host adapters (DirectGameRuntimeCommandAdapter, CurrentGameRuntimeCommandAdapter) already implement it (build-green; they call the interface directly, needing no graphical wrapper). FA4 adds only the App-side DeferredGameRuntimeStateCommands façade.

Lens 5 — TryAutoRespondToFellowshipInvite interception ordering: CLEAN

• The interception guard is request.Type == (uint)ConfirmationType.Fellowship(4) && TryAutoRespondToFellowshipInvite(request). The && short-circuits on type, so any non-fellowship confirmation never enters the interceptor and falls through to _gameplayConfirmationController?.HandleRequest.
• ConfirmationType enum: SwearAllegiance = 1, AlterSkill = 2, AlterAttribute = 3, Fellowship = 4, CraftInteraction = 5, Augmentation = 6, YesNo = 7. Allegiance is type 1, distinct from 4 — it passes through untouched. FA5's allegiance confirmations reach the generic GameplayConfirmationController unchanged (that controller already retains the type-1/type-4 response tuple per its own doc).
• Within a type-4 request: if neither IgnoreFellowshipRequests nor FellowshipAutoAcceptRequests is set, TryAutoRespondToFellowshipInvite returns false → falls through to the generic dialog exactly as before. No swallowing.
• Accept semantics correct: SendResponse(type, contextId, autoAccept); the third bool is accepted (verified at GameplayConfirmationController.cs:97, _sendResponse(_serverType, _serverContext, accepted) where accepted = GetBoolean(ConfirmationResult)). So autoAccept=true → accept, ignore-only → decline. The Ignore↔AutoAccept mutual exclusion (RuntimeCharacterState.TrySetOption) guarantees at most one is set.

Lens 6 — the four un-dimmed rows + AD-78 conformance count: CLEAN (code); see SHOULD-FIX 1 (register)

• Exactly four RowSpec entries flipped StoreOnly→Live (IgnoreFellowshipRequests, FellowshipAutoAcceptRequests, FellowshipShareXP, FellowshipShareLoot); the other two Grouping-section rows (IgnoreAllegianceRequests, DisplayAllegianceLogonNotifications) stay dimmed. Test updated to 31 store-only / 19 live (50 total), internally consistent.
• No other dimming test perturbed. ConfigOptionsPageController, KeyboardConfigController, ChatOptionsPageController and their tests are untouched by FA4 (the only match was a doc-comment reference to ConfigOptions' OP6 precedent). Their dimming tests pass on the post-FA4 Release binaries (91 passed).

Lens 7 — suite accounting +34: RECONCILES EXACTLY

Test file	Δ	how counted
RuntimeFellowshipStateTests.cs	+4	4 GetMembers_* [Fact] (commit 357d2032)
RowTemplateResolverTests.cs	+3	3 [Fact] (new file)
UiTemplateListBoxFlushPreservingScrollTests.cs	+4	4 [Fact] (new file)
SocialFellowshipPageControllerTests.cs	+23	20 [Fact] + 1 [Theory]×3 [InlineData] = 23 discovered tests (new file)
CharacterOptionsPageControllerTests.cs	0	no [Fact]/[Theory] added/removed; one existing test's expected counts changed
SocialPanelControllerTests.cs	0	no [Fact]/[Theory] added/removed; in-place Callbacks-shape updates
SocialPanelLiveMountProbeTests.cs	0	assertions added inside the existing single env-gated [Fact]

4+3+4+23 = +34, matching 13,238 → 13,272. The ledger's per-file arithmetic is accurate (the "+23" resolves as 20 Facts + 3 Theory cases — a naïve attribute count of 21 would look off by 2; it is not).

Lens 8 — register/ledger/gate-script/digest bookkeeping

• Register AD-80/AD-81 well-formed: both are new rows filed at FA4 (AD-79 was the prior max, from FA3), correctly incrementing the AD section header 59 → 61 active rows (+2). AD-80 correctly frames the XP-share display gap as ACE-vs-retail (acdream renders the byte-verified retail table faithfully), AD-81 the two unported text-composition primitives. Both cite SocialFellowshipPageController.cs at the right members. Well-formed.
• AD-78 addendum: landed in the class doc, NOT in the register row → SHOULD-FIX 1 above.
• Gate script §FA4 honest: the [SOLO]/[TWO-CLIENT] markings are accurate (create/caption-swap/button-enable/D5 are solo; roster/recruit/ dismiss/leader-handoff/invite-dialog need a second character and are marked [TWO-CLIENT] with an explicit FA6-bot-gate deferral that says not to treat an unrun two-client step as a failure). The FA3 now-stale "INERT" claims in steps 11-12 are corrected in place with [FA4 correction] pointers, not left wrong. Spot-checked step expectations against code: step 3 (leader-of-solo → Quit/Disband/Open enabled, Leader/Dismiss disabled) matches RefreshButtonStates; step 15 (closed shows "Open", flips to "Close") matches RefreshOpenCaption; steps 21-22 (100% solo even-split, 0% share-off) match FormatStatsText/ EvenSplitPercent.
• Plan ledger FA4 row accurate: CODE-COMPLETE with both SHAs, the reconciled +34, the live-DAT summary, and the four scoped deferrals. Consistent with the diffs.
• project_settings_options_digest.md references AD-78 dimming but states no numeric dimmed count, so lens 8's conditional ("updated if it states a dimmed count") does not apply — no update owed there.

Lens 9 — @allegiance info + FA2/FA3 live paths on post-FA4 binaries: GREEN

Targeted dotnet test --no-build -c Release against the current Release build (verified newer than every source .cs, tree clean):

• App SocialFellowshipPageController/RowTemplateResolver/UiTemplateListBoxFlushPreservingScroll/CharacterOptionsPageController — 75 passed
• App SocialPanelController/SelectionInteractionController/SelectedObjectController — 92 passed
• App ClientCommandController (the live @allegiance info surface) — 26 passed
• Runtime RuntimeFellowshipState/RuntimeAllegianceState/allegiance-info — 37 passed
• Runtime GameEventWiring/LiveSessionEventRouter/Allegiance — 35 passed
• Core.Net Allegiance/GameEventWiring — 115 passed
• App ConfigOptions/KeyboardConfig/ChatOptions (unrelated dimming) — 91 passed

Zero failures across all targeted runs.

---

Additional blast surfaces checked, all clean

• SocialRuntimeBindings grew 4 → 14 params: only one construction site (InteractionRetainedUiComposition.cs:886), updated. No test helper constructs it. Build-green.
• SocialPanelController.Callbacks FellowshipSnapshot: → Fellowship:: only production site is RetailUiRuntime.MountSocialPanel (updated); test sites updated. Build-green.
• using AcDream.Runtime; added to RetailUiRuntime.cs: no ambiguous type surfaced (build-green); the file already fully-qualified the Runtime types it used.
• RowTemplateResolver extraction (carry-forward 2): the cache semantics are byte-identical to the old MountSocialPanel inline closure — caches the imported ElementInfo (including a null miss, so a permanently-unresolvable template is not retried), rebuilds a fresh UiElement on every call. Sharing one instance across Friends/Squelch/Fellowship is safe: distinct template-id keys, no collision. The revision-latch carry-forward 4 holds — _lastRosterRevision advances in Tick before RefreshRoster, so a partial resolver failure waits for the next real membership change (a count mismatch), not a per-frame retry.
• Friends/Squelch scrollbar carry-forward 3: both controllers now resolve the scrollbar via listBox.ScrollbarElementId (DAT 0x72) scoped to pageRoot, replacing a hardcoded literal — the same fix Fellowship's own Bind uses. Covered by the FA3 fixture tests (Friends_/Squelch_ScrollbarModel_IsWiredToListBoxScroll, *_LongRoster_IsReachableViaScrollbar), green in the SocialPanel run above. (The installed-DAT-vs-fixture drift risk for 0x72 is the pre-existing #383, not FA4-introduced.)
• Cross-surface option writes: the fellowship checkboxes write through the same SetSingleCharacterOptionRuntimeCmd command bus the Character tab uses, and RefreshCheckboxSelections re-seeds every tick — so the Ignore↔AutoAccept mutual exclusion (TrySetOption) is enforced once, at the runtime layer, for both surfaces. No double-write divergence.