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

# Campaign FA slice FA4 — MECHANISM-FAITHFULNESS review

**Reviewer lens:** mechanism faithfulness (does the shipped code do what
retail does, for the reasons retail does it). Sibling lens: regression /
blast radius (separate doc).

**Target:** `357d2032` (Runtime), `5bdd0528` (App + tests), `38f08314`
(docs) against `docs/plans/2026-08-11-fellowship-allegiance-campaign.md`
slice FA4 + D4/D5/D6/D7 + the four FA3 re-review carry-forwards, with
`docs/research/2026-08-11-fa-fellowship-wire.md` (lane B) and
`-fa-panel-structure.md` (lane A) as ground truth.

**Verdict: APPROVE-WITH-FIXES — 5 MUST-FIX, 9 SHOULD-FIX, 4 NIT.**

The slice is real work and most of it is faithful: the button-enable
table is a verbatim port, the even-split *table* is byte-exact, the
roster diff genuinely never rebuilds on a vitals tick, the create-flow
gating matches retail's "the button IS the refusal mechanism", and the
live-DAT probe (re-run by this review, below) substantiates every
element-resolution claim in the ledger. The MUST-FIXes are not polish:
two of them are behaviors retail's binary demonstrably does *not* have
(D6's invite intercept, D5's rounding), one is a mechanism retail *does*
have and this slice did not port (world→panel selection sync), one is a
lifecycle hole that silently disables the very stream D4 exists to turn
on, and one is the register rule.

---

## 0. What this review re-derived (not trusted)

| Claim | How it was checked | Result |
|---|---|---|
| Live-DAT mount/element/string claims in the FA4 ledger row | Re-ran `ACDREAM_PROBE_LIVE_MOUNT=1 dotnet test --no-build -c Release --filter SocialPanelLiveMountProbeTests` against the installed DATs | **CONFIRMED.** `0x1000026F` builds as `UiField`; all 11 buttons/checkboxes as `UiButton`; ListBox `0x10000279` templates=1 `0x21000030/0x10000281`, scrollbar `0x1000027A`; all 5 row fields at the right widget types; labels resolve ("Ignore Fellowship Requests", "Automatically Accept Fellowship Requests", "Share Fellowship Experience and Luminance", "Share Fellowship Loot"); captions resolve `'Open'`/`'Close'`; production `Bind()` console output **empty** |
| The frame-containment "structural finding" | Same probe dump | **CONFIRMED and EXTENDED** — `0x1000026B` children are `0x10000272,0x10000273,0x10000274,0x1000026C..0x10000271`: the Create button, the name field **and all four checkboxes** live in the NOT-in-fellowship frame. See SHOULD-FIX 8 for the consequence the ledger missed |
| `GetEvenSplitXPPctg` table | Byte-read `.rdata` of the PDB-paired `acclient.exe` at the ten VAs lane B §7.2 names | **CONFIRMED verbatim**, including `0x007E72B8 = 0.31111109256744385` and default `0.0` |
| How retail converts that float to the displayed integer | Byte-decoded `0x0048ECC9..0x0048ECD8` + the callee at `0x005DE394` | **REFUTES the port** — see MUST-FIX 1 |
| Whether retail's client reads the two fellowship option bits on the invite path | Read `gmFellowshipUI::RecvNotice_FellowshipRequest @0x00490880`, `MakeFellowRequestDialog @0x00490620`, `ClientUISystem::Handle_Character__ConfirmationRequest @0x005640A0` in full + swept every `IgnoreFellowshipRequests`/`FellowshipAutoAcceptRequests` occurrence in the 2013 pseudo-C | **REFUTES D6's invite half** — see MUST-FIX 2 |
| The 93 tests in the five touched test classes | `dotnet test --no-build -c Release --filter` | 93 passed / 0 failed |

---

## MUST-FIX 1 — D5's percentage conversion rounds where retail truncates: 6- and 8-fellow rows display the wrong number

`SocialFellowshipPageController.cs:650`

```csharp
return $"{member.Level}  {(int)MathF.Round(pct * 100f)}%";
```

The class doc and the commit message both claim "The percentage itself IS
retail's exact number". The *table* is exact (byte-verified this review).
The **conversion is not**.

Retail, byte-decoded from the PDB-paired binary at
`gmFellowshipUI::UpdateFellowStats`:

```
0048ECC9  d9 44 24 10           fld   dword [esp+0x10]      ; pct (float)
0048ECCD  d8 0d 70 51 7a 00     fmul  dword [0x007A5170]    ; = 100.0f  (byte-read)
0048ECD3  e8 bc f6 14 00        call  0x005DE394            ; _ftol2
```

`0x005DE394` is MSVC's `_ftol2` (`55 8b ec 83 ec 20 83 e4 f0 d9 c0 d9 54
24 18 df 7c 24 10 df 6c 24 10 …` — the fld/fst/fistp/fild + correction
dance whose whole purpose is to turn the FPU's round-to-nearest `fistp`
into **C truncation toward zero**). The product is formed on the x87
stack from two floats, so it is exact; `_ftol2` then chops it.

Per-entry comparison (float bits read from the binary; acdream column is
the shipped expression):

| n | retail float | retail `trunc(pct×100)` | acdream `(int)MathF.Round(pct*100f)` |
|--:|---|--:|--:|
| 1 | 1.0 | 100 | 100 |
| 2 | 0.75 | 75 | 75 |
| 3 | 0.6000000238418579 | 60 | 60 |
| 4 | 0.550000011920929 | 55 | 55 |
| 5 | 0.5 | 50 | 50 |
| **6** | **0.44999998807907104** | **44** | **45** ❌ |
| 7 | 0.4000000059604645 | 40 | 40 |
| **8** | **0.3499999940395355** | **34** | **35** ❌ |
| 9 | 0.31111109256744385 | 31 | 31 |
| 10 | 0.2800000011920929 | 28 | 28 |

Two of the ten roster sizes are wrong. Note the failure is not fixed by
swapping `MathF.Round` for a cast: `0.45f * 100f` **already rounds up to
exactly `45.0f`** in single precision, so `(int)(pct * 100f)` still
yields 45. The product must be formed in a wider type, exactly as
retail's x87 does:

```csharp
return $"{member.Level}  {(int)((double)pct * 100.0)}%";
```

(`(double)0.44999998807907104 * 100.0 = 44.999998807907104` → 44 ✓;
`0.3499999940395355 * 100.0 = 34.99999940395355` → 34 ✓; every other row
unchanged.)

**Also fix in the same commit:**
- `SocialFellowshipPageControllerTests.FormatStatsText_MatchesD5Rules`
  exercises only n=1/n=9/proportional — neither failing size is pinned.
  Add `[InlineData(true, true, 6, "12  44%")]` and
  `[InlineData(true, true, 8, "12  34%")]`.
- The gate script's §FA4 step 21 currently presents the 9-member ACE gap
  (AD-80) as the *only* percentage divergence a tester should tolerate,
  which would prime them to accept the wrong 45% at six fellows as
  correct. Same class as FA3's own mechanism MF-1/MF-2 script
  corrections.
- AD-80's row text says acdream "renders retail's own byte-decoded XP-share
  table verbatim" — true of the table, false of the rendered integer until
  this lands.

---

## MUST-FIX 2 — D6's invite intercept is a client-side mechanism retail does not have, and it can silently swallow real invites

`RetailUiRuntime.cs:665-693`

```csharp
if (request.Type == (uint)GameEvents.ConfirmationType.Fellowship
    && TryAutoRespondToFellowshipInvite(request))
    return true;
```

**(a) No retail anchor exists.** Read in full this review:

- `ClientUISystem::Handle_Character__ConfirmationRequest @0x005640A0` —
  a bare 7-way jump table; `case 4` is one call to
  `CM_Fellowship::SendNotice_FellowshipRequest`. No option read.
- `gmFellowshipUI::RecvNotice_FellowshipRequest @0x00490880` — copies the
  string, tail-calls `MakeFellowRequestDialog`. No option read.
- `gmFellowshipUI::MakeFellowRequestDialog @0x00490620` — its **only**
  guard is `if (this->m_fellowRequestContext == 0)` (`@0x00490635`), the
  one-dialog-at-a-time rule lane B §2.4 already documents. No option read.
- A whole-file sweep of `IgnoreFellowshipRequests` /
  `FellowshipAutoAcceptRequests` in `acclient_2013_pseudo_c.txt` returns
  **only**: the two `PlayerOptionPage` label registrations
  (`@0x004a0b56`/`@0x004a0d16`), an input-action toggle pair
  (`@0x00561fbf`/`@0x005621ef`), `CPlayerModule::OnChanged`'s mutual
  exclusion (`@0x0059a971`/`@0x0059a987`), and the `PlayerModule`
  get/set switch arms (`@0x005d3aca`/`@0x005d3f11`/`@0x005d3c06`/
  `@0x005d4031`). **Zero reads on any confirmation path.**

Retail's client shows the dialog unconditionally. This is the same shape
as FA1's D9 and FA2's D2: the plan asserted a mechanism, primary source
refutes it.

**(b) The code comment cites ACE server source as a retail anchor.**
`RetailUiRuntime.cs:669` reads "retail's `Fellowship.cs:121`-equivalent
client-side mirror". `Fellowship.cs` is `ACE.Server`. Lane B's own rows
for these two options say the opposite of what the comment implies —
feature 5: "**Store only.** ACE is the consumer; acdream has nothing to
do beyond sending the bit (already correct)"; feature 6: "**Store
only**".

**(c) Against a correct ACE it is dead code; against a drifting one it is
harmful.** Per lane B, ACE refuses the recruit outright when the target
has `IgnoreFellowshipRequests` (`Player_Fellowship.cs:98-102` →
`FellowshipIgnoringRequests 0x0417` to the *recruiter*) and auto-joins
without a confirmation when the target has `FellowshipAutoAcceptRequests`
(`Fellowship.cs:121` → `AddConfirmedMember(…, true)`). In both cases the
target's client never receives a type-4 request, so the intercept never
fires. It only fires when the client's copy of the bit disagrees with the
server's — and then it does the wrong thing.

**(d) That disagreement is the default state, not an edge case.**
`CharacterOptionTable.cs:113` gives `IgnoreFellowshipRequests` a client
default of **`true`**. A player who has never touched the option, on any
server whose own default differs or that does not implement the
server-side filter, will have every fellowship invite auto-declined with
**no dialog and no chat line** — indistinguishable from a broken client.
The FA3 gate script's §FA4 steps 18/19 cannot detect this: they pass
identically whether the intercept exists or not, because ACE filters
first.

**Recommended disposition (matching the FA1-D9 / FA2-D2 precedent):**
delete `TryAutoRespondToFellowshipInvite` and its call site, revert the
`IgnoreFellowshipRequests` / `FellowshipAutoAcceptRequests` rows in
`CharacterOptionsPageController.Groups` to `StoreOnly` (they return to
OP1's classification, which was right), and record the finding as a D6
addendum in the plan: *the invite-receive half of D6 is refuted; both
bits are server-consumed, acdream's job is to send them, and retail's own
client shows the dialog unconditionally.* If instead the user wants the
auto-respond behavior kept as a deliberate quality-of-life divergence,
it needs (i) an explicit register row, (ii) the ACE-as-retail citation
corrected, and (iii) an answer for the default-on `Ignore` bit — but that
is a user decision, not an implementer one.

---

## MUST-FIX 3 — D4 never re-declares `0x00A6` after a generation reset, so a reconnect leaves the vitals stream dark for the whole new session

`SocialFellowshipPageController.cs:453-458` +
`SocialPanelController.cs:250-268`

`SetPageVisible` is edge-triggered on `_pageVisible`, and
`FellowshipSetPanelOpen` is a no-op while disconnected (returns
`Inactive` without sending). Enumerating every transition:

| Transition | Behavior | Verdict |
|---|---|---|
| Panel opens with Fellowship active | `OnShown` → `_visible=true` → `SetPageVisible(true)` → sends `1` | ✓ |
| Panel opens on another tab | conjunction false, `_pageVisible` already false, no send | ✓ (retail's widget isn't visible either) |
| Switch **to** Fellowship while open | `ActivePageChanged` → sends `1` | ✓ |
| Switch **away** | sends `0` | ✓ |
| Close window while on Fellowship | `OnHidden` → sends `0` | ✓ |
| `ActivateTabs()` at mount (default = Allegiance, `Visible=false`) | conjunction false, no spurious send; ordering is safe because `ActivateTabs` runs before `RetailWindowFrame.Mount` (`RetailUiRuntime.cs:2775`) | ✓ |
| Logout | nothing sent; ACE's `LogOut_Inner` quits the fellowship anyway (lane B feature 21) and the flag is per-session server-side | ✓ — no close is owed |
| **Disconnect mid-open, then reconnect** | `_pageVisible` stays `true` across the generation reset; no visibility edge occurs on the new generation; **`0x00A6` is never sent again** | ❌ |

`RetailUiRuntime` is process-lifetime (`GameWindow.cs:1020`, published
once behind a "already owns interaction/UI state" guard), and
`ResetSessionTransientUi` (`RetailUiRuntime.cs:731-736`) only resets the
dialog controller, the appraisal controller and the Examination window —
it does not touch the social panel. `RetailWindowLayoutPersistence`'s
`RestoreAll()` on the new `EnteredWorld` will not produce an edge either,
because `RetailWindowHandle.NotifyVisibility` early-returns when
`_notifiedVisible == visible`.

Observable result: after any in-process reconnect with the panel left
open on Fellowship, every fellow's health/stamina/mana freezes for the
rest of the session (lane B §4.5: `Fellowship.OnVitalUpdate` sends
`0x02C0` only to fellows whose `FellowshipPanelOpen` is true, and that
flag is set only by `0x00A6`).

**Fix:** clear the declaration latch at the existing session-reset seam
so the next `Tick`/visibility evaluation re-declares — e.g. a
`SocialPanelController.ResetSessionDeclaration()` that resets
`_fellowship`'s `_pageVisible` to `false` and then re-runs
`UpdateFellowshipPageVisibility()`, called from
`ResetSessionTransientUi`. That is a one-line addition at a seam that
already runs on every generation reset (`LiveSessionRuntimeFactory.cs:182`
→ `LiveSessionResetBindings.SessionDialogs`) — not a timer, not a poll.

**Honesty note on the retail comparison:** `gmFellowshipUI` has no
`OnEndCharacterSession` (verified — its method list is
Create/PostInit/Update/UpdateButtons/UpdateFellow*/RecvNotice_*/
ListenToElementMessage/OnVisibilityChanged/MakeFellowRequestDialog and
nothing else), and its `PostInit` does **not** send `Event_UpdateRequest`
(lane A §6.3 — only the allegiance panel does). Retail's re-declaration
therefore comes from `OnVisibilityChanged` firing when the in-game UI
state is torn down/restored around character select; that inference is
not byte-established here. The *defect* stands regardless of how retail
gets it right: acdream reaches a state where the stream it just turned on
is off and nothing will ever turn it back on.

**Also:** the gate script §FA4 has no reconnect step, so the owed
connected gate cannot catch this. Add one after step 8.

---

## MUST-FIX 4 — `gmFellowshipUI::UpdateFellowSelection` is not ported: selecting a fellow in the WORLD leaves Dismiss/Leader disabled, and no row ever shows as selected

The slice ports one direction of retail's two-directional selection
coupling. `SelectFellow` (`SocialFellowshipPageController.cs:676-680`)
correctly does panel → world (`SelectionChangeSource.Social`, the FA4
Runtime commit). The reverse arm is missing:

`gmFellowshipUI::UpdateFellowSelection @0x0048F0F0`, reached from
`RecvNotice_SelectionChanged @0x0048F1C0` (and from `Update`
`@0x0048F6E9`), walks the list box on every world-selection change:

```
selectedID = ACCWeenieObject::selectedID
for each row:
    if row.GetAttribute_InstanceID(0x1000000D) == selectedID:
        m_iidSelectedFellow = selectedID
        SetSelectedItem(listBox, row, 0)          // and return
    else if that id == m_iidSelectedFellow:
        remember row as the fallback
SetSelectedItem(listBox, fallbackOrNull, 0)
UpdateButtons(this)
```

So in retail, clicking a fellow **in the 3D world** (or reaching them via
any other selection origin) selects their panel row and enables
Dismiss/Assign-Leader. In acdream `_selectedFellowGuid` is written only
by the row name-text `OnClick`
(`SocialFellowshipPageController.cs:599,676`), so the world→panel arm
does nothing and those two buttons stay greyed.

Two further consequences of the same gap:

- **No visual selection at all.** There is no `SetSelectedItem`
  equivalent — nothing in the roster ever indicates which fellow is
  selected. The user clicks a name and gets no feedback until they notice
  two buttons un-greying.
- **The plan's FA4 contract line is not met.** §3's FA4 row reads
  "roster rows (adds `UiTemplateListBox` Flush/selection/row-instance-id
  — lane A sized this)". `Flush` pre-dated FA4 (FA3); this slice added
  `FlushPreservingScroll` only. **No selection model and no per-row
  instance-id were added** — retail's row identity key is
  `SetAttribute_InstanceID(row, 0x1000000D, fellowIid)` (lane A §3.1 row
  table, `ghidra@0x0048F440:95`), the exact primitive both
  `ListenToElementMessage` and `UpdateFellowSelection` read. The ledger's
  four-item "Contradictions/deferrals" list does not mention this.

Minimum fix: give `UiTemplateListBox` a row-instance-id + selected-row
concept (or, if that is deferred, say so explicitly in the ledger and
file the register row), and subscribe the controller to
`SelectionState.Changed` — or simply re-derive `_selectedFellowGuid` from
`Selection.SelectedObjectId` in `RefreshButtonStates` when the selected
world object is a member — so the world→panel arm exists.

---

## MUST-FIX 5 — three shipped deviations have no register row (CLAUDE.md's register rule, plan §5)

The register rule is binding and explicit: *"Any commit that introduces a
deviation adds its register row IN THE SAME COMMIT… A deviation found
without a row is a bug twice over."* `38f08314` files AD-80 (D5 display
vs ACE) and AD-81 (StringInfo/FormatName) and amends AD-78. Missing:

1. **The D6 client-side invite intercept** — a client behavior retail
   does not have (MUST-FIX 2). If it survives review at all it needs its
   own row; if it is deleted, the row is not owed.
2. **The gold leader-name tint** (`LeaderNameColor` `(1, 0.84, 0, 1)`,
   `SocialFellowshipPageController.cs:110,614`). This is an invented
   user-visible color with no DAT or decomp anchor — the class doc says
   so in as many words ("a minimal, clearly-adaptive visual cue rather
   than inventing a DAT mechanism that was not found"). That is precisely
   an AD row: a reviewer comparing a retail screenshot will see a color
   retail never paints. Same class as AD-78's own rationale.
3. **Row selection restricted to the name text.** Retail's list box
   selects on the *row* (`ListenToElementMessage` message `3`/`0x42`,
   lane B §2.8). acdream binds `OnClick` to the row's name `UiText` only,
   so clicking the stats text, any meter, or row whitespace does nothing.
   Documented in `SelectFellow`'s doc comment; not registered.

Separately, the ledger's decision to keep the **Recruit "is the target a
player" gate** as "an inline comment, not a register row" is the wrong
call by the same rule. Retail *disables* the button
(`UpdateButtons`, lane B §2.8: "selected world object **is a player**,
not already a fellow, and `!IsFull`"); acdream enables it and lets the
server refuse. That is a divergence with a user-visible symptom (a lit
Recruit button that does nothing when a chest is selected), and the
justifying comment — "exactly like retail's own silent no-op" — is
inexact, because retail's click handler is unreachable when the button is
disabled.

---

## SHOULD-FIX

**SF-1 — `RefreshFellowshipName` reintroduces the per-frame `LinesProvider`
closure allocation that FA3's fix round removed from the sibling page one
commit earlier.**
`SocialFellowshipPageController.cs:489-493` assigns
`() => [new UiText.Line(name, MemberNameColor)]` on **every** `Tick`
while in a fellowship — a display-class + delegate allocation per frame,
and `SocialPanelController.Tick` calls the fellowship page
unconditionally, so it allocates while the panel is hidden too.
`35c40a9b` fixed exactly this in `SocialAllegiancePageController`
(hoisted `BlankLineProvider`/`NoLinesProvider` statics, doc: "zero
allocation while idle") in response to FA3 mechanism SF-2 / blast SF-4.
Cache the provider and only reassign when `name` actually changes.

**SF-2 — the commit message's carry-forward-4 claim does not describe the
code.** The message says "The Fellowship roster path never advances its
revision latch on a partial resolver failure until the NEXT real
membership change — never a per-frame retry loop."
`SocialFellowshipPageController.cs:433-437` advances `_lastRosterRevision`
**unconditionally**, before `RefreshRoster` runs. What actually happens
on a permanently-unbuildable template: `RebuildRoster` `continue`s past
the failed row (`:580-586`), leaving `_rows.Count < members.Count`, so
`membershipChanged` is true on **every** subsequent revision bump — i.e.
a full `FlushPreservingScroll` + N `LayoutImporter.Build` calls under the
shared DAT lock on every incoming `0x02C0` vitals tick, not "until the
next real membership change". `RowTemplateResolver` bounds the *import*
(a null import is cached, `RowTemplateResolverTests` pins it) but not the
*build*. Either make the claim accurate or bound the rebuild (e.g. latch
a per-generation "row template is unbuildable" flag).

**SF-3 — Recruit's "already a fellow" test reads render rows, not
membership.** `SocialFellowshipPageController.cs:527` uses
`!_rows.ContainsKey(id)`. Retail uses `Fellowship::IsFellow`. If a row
failed to build (SF-2's path), an existing fellow passes `targetValid`
and Recruit lights up for someone already in the fellowship. Use the
member set.

**SF-4 — `SocialPanelController.Dispose` does not unsubscribe
`ActivePageChanged`.** `:140` subscribes a `this`-capturing lambda;
`:305-309` only sets `_disposed`. `Tick()` guards on `_disposed`
(`:295`); the event handler does not, so a tab switch after disposal
still reaches `SetPageVisible` → a Runtime command. Add the unsubscribe
(or a `_disposed` check in `UpdateFellowshipPageVisibility`).

**SF-5 — test naming/coverage gaps on the slice's headline mechanisms.**
- `Tick_MembershipChange_RebuildsRoster_ButPreservesScrollPosition`
  (`SocialFellowshipPageControllerTests.cs:244`) asserts only
  `Children.Count == 2`; its own comment admits it cannot observe the
  preserved offset. Rename it or build a roster tall enough to scroll.
  (The widget-level `UiTemplateListBoxFlushPreservingScrollTests` **do**
  cover the shrink case correctly — re-derived: `ClearContent` zeroes
  `UiScrollablePanel.ContentHeight` but not `UiScrollable.ContentHeight`,
  so the restore clamps against the stale height, and
  `LayoutScrollableChildren` re-clamps against the fresh height at the
  head of the next `OnDraw`, before anything paints. That is sound.)
- **D4's actual conjunction is untested.** The only D4 test is
  `SetPageVisible_SendsPanelOpen_OnlyOnATransition` on the *page*
  controller. Nothing tests `SocialPanelController`'s
  `_visible && IsShowingFellowship` logic, the `ActivePageChanged`
  subscription, or `OnShown`/`OnHidden` — the mechanism the slice is
  named for. `SocialPanelControllerTests` gained a bindings helper and
  zero new tests.
- No controller-level test for a member **leaving** (the shrink rebuild),
  and none for `_selectedFellowGuid` being cleared when the selected
  fellow departs (`:568-569`).

**SF-6 — the live-mount probe prints the strings it is cited for but does
not assert them.** `SocialPanelLiveMountProbeTests.cs:185-201` writes the
four checkbox labels and the two captions to the console with no
assertion, yet the ledger's live-DAT paragraph cites them as verified.
This is the same finding FA3's own mechanism SF-3 raised ("printed but
never asserted — deserves a real assertion, not just a hope") for
`0x10000492` and page exclusivity. Assert non-null (and, for the two
captions, the exact `"Open"`/`"Close"`).

**SF-7 — gate-script §FA4 defects (the FA3 MF-1/MF-2 class).**
- Step 3 contradicts itself: "Only the Quit button should be enabled …
  Disband and Open should ALSO be enabled." A tester following the first
  clause reports correct behavior as a defect.
- Steps 18/19 claim to gate D6, but per lane B ACE filters both bits
  server-side and never sends the confirmation, so both steps pass
  identically with or without the client intercept. They cannot fail.
- No reconnect step (MUST-FIX 3) and no six/eight-fellow percentage step
  (MUST-FIX 1) — the two defects most likely to reach a user are both
  outside the script's reach.

**SF-8 — D7's `FellowshipShareLoot` un-dim does not meet AD-78's own
definition of "Live".** AD-78 defines the dimmed set as rows that
"persist and, where auto-save, send the wire bit, but drive nothing
observable client-side". `FellowshipShareLoot`'s stated new consumer is
"a SECOND live checkbox surface on the fellowship page itself" — but a
second editor of the same stored value is not a consumer, nothing in
acdream reads the bit (`FormatStatsText` uses `snapshot.ShareXp` only,
and the `0x00A2` Create builder carries `shareXP` alone), and the live
DAT dump confirms that checkbox (`0x10000273`) is a child of the
NOT-in-fellowship frame `0x1000026B`, i.e. **invisible whenever you are
in a fellowship**. Re-dim it, or state the widened definition explicitly
in AD-78. (`FellowshipShareXP` is defensible: the Create click genuinely
reads it, `:325-326`.) The same "second surface" argument in
`CharacterOptionsPageController`'s doc should be corrected either way.

**SF-9 — AD-78's own row still says "35 of 50 rows dimmed".** (Carried
from the superseded review, verified independently.) `38f08314`'s message
claims "AD-78's derivation table gains its D7 addendum", but the addendum
landed in `CharacterOptionsPageController`'s **class doc** (in
`5bdd0528`); the register row's own "Where" column still reads
`CharacterOptionsPageController.cs (35 of 50 rows dimmed …)` while the
post-FA4 count is 31 of 50. The register is "the single auditable list" —
a stale count in an active row is exactly the drift AD-78's own "Risk"
column warns about. (Note this interacts with SHOULD-FIX 8 and MUST-FIX
2: if either lands, the correct number is not 31 either. Fix the count
last.)

---

## NIT

**N-0 — the Open/Close caption does not optimistically pre-toggle.**
(Carried from the superseded review, verified.) Lane B feature 11 records
that retail's Open button handler "client pre-toggles its own
`_open_fellow`" before `Event_ChangeFellowOpeness` — i.e. the caption
flips immediately and the server echo confirms it. `RefreshOpenCaption`
(`:495-508`) keys purely off server-authoritative `snapshot.IsOpen`, so
the caption flips only when the `0x02BE` echo lands. Convergent, and
invisible on localhost; not byte-faithful.

**N-1 — meter text vs authored child.** Retail's `UpdateFellowVitals
@0x0048ED60` sets meter attribute `0x69` to `cur/max` and writes the
cur/max ints into the meter's authored **child** text elements
(`0x10000286` under health, `0x10000288` under stamina). acdream uses
`UiMeter.Fill`/`UiMeter.Label` because `UiMeter.ConsumesDatChildren` is
true — a pre-existing widget decision, not FA4's, but worth one line in
`SetVitals`'s doc so the next reader does not go hunting for the
unbound `0x10000286`/`0x10000288`/`0x1000028A`.

**N-2 — `max > 0` guard.** `SetVitals` (`:661`) returns `0f` when
`max == 0`; retail divides unconditionally. The guard is the right
engineering call; just say so, since "verbatim port" is claimed nearby.

**N-3 — snapshot/roster read atomicity.** `Tick` takes
`_bindings.Snapshot()` and `_bindings.Members()` under two separate
acquisitions of `RuntimeFellowshipState._gate`, so `Revision` /
`MemberCount` can momentarily disagree with the returned roster. It is
self-correcting on the next revision bump and harmless in practice
(single-threaded tick today), but a one-line comment would stop a future
reader from assuming atomicity.

---

## What is faithful (checked, no action)

- **Button-enable table** (`RefreshButtonStates`, `:510-537`) matches lane
  B §2.8 exactly, including the non-obvious Recruit rule
  (`isLeader || snapshot.IsOpen`) and Quit-always-enabled.
- **Open/Close caption reads as the ACTION, not the state** (`:495-508`),
  resolved once at `Bind` and swapped from cache — correct, and correctly
  refuses to invent English when the strings do not resolve.
- **Create gating** — empty/whitespace name disables the button and the
  click double-checks (`:322-327`, `:460-465`); no invented error text,
  matching lane B §2.2's "the button IS the refusal mechanism".
- **Quit/Disband share `0x00A3` with the flag flipped**, and both route
  through `IRuntimeFellowshipCommands.Quit` so the retail pre-quit
  `0x0290` leader hand-off in `RequiresLeaderHandoffBeforeQuit` is not
  bypassed — the reason the commit gives for not calling `WorldSession`
  directly is correct and load-bearing.
- **The roster diff never rebuilds on a vitals tick** — the FA3
  carry-forward's exact hazard. Verified by construction (set comparison
  at `:551-560`, in-place `UpdateRow` otherwise) and by
  `Tick_SameMemberSet_UpdatesRowsInPlace_NoRebuild`'s `Assert.Same` on
  the row instance.
- **`FlushPreservingScroll` shrink semantics** — re-derived against
  `UiScrollablePanel`/`UiScrollable`; correct, and pinned by
  `FlushPreservingScroll_ClampsToTheNewShorterContent`.
- **`EvenSplitPercentTable` and the `>= 9` full check** — byte-verified
  against the binary, including the out-of-range `0.0` default matching
  retail's unsigned `ja` fallthrough for n=0.
- **`RowTemplateResolver`** — clean extraction, caches the import (miss
  included) and rebuilds per row; carry-forward 2 satisfied.
- **`IRuntimeFellowshipView.GetMembers`** — materialized under the same
  lock as every other read, honest ordering caveat in its doc.
- **`SelectionChangeSource.Social`** — additive; no exhaustive switch on
  the enum exists in production, so no dispatch site was missed.

---

## Appendix — collision with a concurrent review at `6849b457`

A second mechanism-lens pass landed on this same path at `6849b457`
(2026-08-12 04:59:35) while this one was in progress; this document
replaces it. **Its text is not lost — read it with
`git show 6849b457:docs/research/2026-08-12-fa4-review-mechanism.md`.**
Its two unique findings are carried forward above (SF-9, N-0). The two
reviews disagree in exactly two places, both adjudicated here from
primary source:

| Topic | `6849b457` | This review | Why |
|---|---|---|---|
| `MathF.Round` vs `_ftol2` | **NIT 6, "non-issue"** — "For every value in the even-split table (×100 = 100/75/60/55/50/45/40/35/31.11/28) round and truncation agree, so there is no observable difference on any reachable input" | **MUST-FIX 1** | That reasoning uses the DECIMAL literals. The stored constants are floats: `0x007C91D4 = 0.44999998807907104` and `0x007E72BC = 0.3499999940395355` (byte-read from the PDB-paired binary this review), so retail's `×100` products are `44.999998…` and `34.999999…` and `_ftol2` chops them to **44** and **34**. Two of ten roster sizes differ. This is precisely the campaign's §6 rule ("BN literal-0 operands are byte-verified before use") applied one level deeper — to the `.rdata` float, not the decompiler's rendering of it |
| D6 invite auto-response | **"✓ verified clean"** — describes the intercept's shape and Runtime's mutual exclusion, then passes it | **MUST-FIX 2** | The review verified that the code does what the PLAN says. Mechanism faithfulness asks whether RETAIL does it. `Handle_Character__ConfirmationRequest @0x005640A0`, `RecvNotice_FellowshipRequest @0x00490880` and `MakeFellowRequestDialog @0x00490620` were read in full here: no option read on any confirmation path, and a whole-file sweep of both accessors finds reads only in chargen, the input-action toggle, `OnChanged`'s mutual exclusion, and the `PlayerModule` get/set switch |

The two reviews AGREE on D4-not-re-armed-across-reconnect (its SHOULD-FIX
3, raised here to MUST-FIX 3 because it silently disables the stream the
slice exists to enable), on the missing panel-level D4 conjunction test
(its SHOULD-FIX 4 = SF-5 here), on the leader-gold-tint register omission
(its MUST-FIX 1 = part of MUST-FIX 5 here), and on the live-DAT probe
result.

---

## Narrow re-review of the fix round — 2026-08-12

**Fix commits:** `290f9b58` / `5499f058` / `df000306` / `300d8189` /
`55b17e15` / `1d743277` / `f041b09b`.

**Verdict: CLOSED with ONE REOPEN — MUST-FIX 3.** Dispositions re-derived
from the actual diffs (not the commit claims), the D6/D7/SF-8 dimming
arithmetic audited from primary source, totals reconciled on the touched
projects, and the live-mount probe re-run on the post-fix binaries. Four
of five MUST-FIX, all 9 SHOULD-FIX, all 4 NIT, and blast SF-1 are
correctly applied. **MUST-FIX 3 is REOPENED:** the latch-clear it ships is
necessary but its re-declaration never reaches the wire in the real
reconnect path, so the fellow-vitals-freeze defect it targets persists.

### MUST-FIX 1 — CLOSED
`FormatStatsText` now returns `$"…{(int)((double)pct * 100.0)}%"`
(`SocialFellowshipPageController.cs:790`). The two failing sizes are
pinned: `[InlineData(true, true, 6, "12  44%")]` and
`[InlineData(true, true, 8, "12  34%")]` added to
`FormatStatsText_MatchesD5Rules`. Gate-script step 23 corrected to "the
panel must show 44% and 34% respectively — NOT 45% or 35%". AD-80's
"renders retail's byte-decoded table verbatim" claim is now true of the
rendered integer, not just the table — no further AD-80 edit owed.

### MUST-FIX 2 — CLOSED
`TryAutoRespondToFellowshipInvite` deleted; `HandleConfirmationRequest`
is now `=> _gameplayConfirmationController?.HandleRequest(request) == true`
(`RetailUiRuntime.cs:678`), routing every type through the generic
controller exactly as retail `Handle_Character__ConfirmationRequest
@0x005640A0` does. New test
`FellowshipInviteRequest_Type4_OpensDialog_MessageVerbatim_AndSendsAcceptOnClose`
proves the type-4 dialog renders the server message verbatim (not
`" Continue?"`-suffixed) and sends accept through the generic path.
**No allegiance (type-1) path broke:** the deleted intercept keyed only on
`ConfirmationType.Fellowship` (type 4) — type 1 always fell through to the
generic controller both before and after the fix (verified against the
pre-fix `5bdd0528` source). The `IgnoreFellowshipRequests` /
`FellowshipAutoAcceptRequests` rows revert with the dimming reversal below.

### MUST-FIX 3 — REOPEN (latch-clear correct, re-declaration swallowed)
The fix adds `SocialPanelController.ResetSessionDeclaration()` (clears the
edge-latch via `SocialFellowshipPageController.ResetPageVisibleLatch()`,
then re-evaluates the D4 conjunction) and wires it into
`RetailUiRuntime.ResetSessionTransientUi()`. The direction is right; the
**timing is wrong**, and the unit test cannot see it because its fake
command records unconditionally.

Traced from source:
- `ResetSessionTransientUi` runs only via the `SessionDialogs` reset stage
  (`LiveSessionRuntimeFactory.cs:182` -> `LiveSessionResetManifest`).
- Both `ResetSessionState` call sites run that stage **before the new
  session is in-world**: `ResetHostBeforeStart`
  (`LiveSessionController.cs:733-751`, run inside `StartCore` *before*
  `_inWorld = true` at `:642` and before `binding.ActivateCommands()`),
  and the retired-scope teardown (`:231`, which tears down the OLD scope).
- `SetPanelOpen` requires world:
  `Validate(expectedGeneration, requireWorld: true)`
  (`CurrentGameRuntimeCommandAdapter.cs:929`), and the deferred App seam is
  not even bound to the new generation until `ActivateCommands`. So during
  the reset the send returns `Inactive`/rejected and **nothing is
  published**.
- But `ResetSessionDeclaration` -> `SetPageVisible(true)` still sets
  `_pageVisible = true` (the transition fires the no-op command). No
  post-world-entry hook re-evaluates the conjunction (the `EnteredWorld`
  bindings — RestoreLayout/SyncToolbar/etc. — never touch the social
  panel's visibility, and the panel is not a state-managed-visibility
  window so layout restore produces no OnShown edge; `Tick` never calls
  `SetPageVisible`).

Net: after an in-process reconnect with the Fellowship page left open,
`_pageVisible` ends `true` but `0x00A6` was never sent to the new server —
the exact vitals-freeze this MUST-FIX targets (lane B §4.5) persists. The
fix relocated the latch reset without achieving the observable outcome.
The gate script's new reconnect step (step 9) *will* catch this at the
owed connected gate — so this is not silently hidden — but a green
automated gate here is not evidence the wire send happens, and the
fixer's "resolved" claim is not substantiated in the real path.

**Recommended direction:** re-declare while in-world on the new session,
not during the pre-world reset — e.g. re-run the D4 conjunction from the
`EnteredWorld` seam (`LiveSessionRuntimeFactory.cs:158-164`) after clearing
the latch, or make `SetPageVisible` refuse to advance `_pageVisible` when
the command result is not `Accepted` (paired with an in-world evaluation
trigger). Either is a one-seam change; the current placement cannot work
because the reset is a pre-connect, local-state-clearing phase.

### MUST-FIX 4 — CLOSED (deferral is minimal-observable-contract, audited)
`SyncSelectionFromWorld`/`SetSelectedFellow` reproduce retail
`UpdateFellowSelection @0x0048F0F0`: the "found" arm (a world selection
matching a member becomes the panel selection, enabling Dismiss/Leader and
tinting the row) and the "fallback" arm (a non-member world selection
leaves the existing panel selection untouched; `RefreshRoster` clears it
only when that fellow leaves the roster). Called every `Tick` while in a
fellowship, mirroring retail's per-`Update` placement. The generic
`UiTemplateListBox` selection-model + `0x1000000D` row-instance-id port is
**not** done — but AD-82(4) records that honestly as owed, and the
OBSERVABLE contract (button-enable + a highlight) is met. This is a
scoped, declared page-local reimplementation, not a hidden gap: the only
observable divergences (a blue selection tint retail never paints; only
the name text is a click target) are both named in AD-82.

### MUST-FIX 5 — CLOSED
AD-82 (invented leader/selection tints, name-text-only click target,
page-local world->panel sync with the generic port recorded as owed) and
AD-83 (Recruit "is-a-player" enable-gate omission) are well-formed:
correct file/symbol cites, honest "Risk" columns, real decomp anchors
(`UpdateFellowSelection @0x0048F0F0`, `ListenToElementMessage @0x004901C0`,
`UpdateButtons` §2.8). The prior "inline comment, not a register row" call
on the Recruit gate is corrected. AD-78's stale "35 of 50" is fixed to
"34 of 50 / 16 live" and self-flags the two-campaign register lag as an
instance of the exact risk that row warns about.

### D6/D7/SF-8 dimming arithmetic — CLOSED and CORRECT (audited from source)
Landed at **34 of 50 dimmed / 16 live** — one net un-dim from the pre-FA4
baseline (`FellowshipShareXP` only), not "splitting the difference".
Verified from primary source, not the commit message:
- **`FellowshipShareLoot` -> dimmed is faithful.** Grep of `src/` finds
  exactly two live references (`SocialFellowshipPageController.cs:437`
  binds the page checkbox; `:578` seeds it from the value) — both an
  *editor/display* surface, neither a *reader of the value*. The `0x00A2`
  Create builder carries `shareXP` alone (`SocialActions.cs:139-141`);
  nothing in acdream reads `FellowshipShareLoot` to drive a wire message
  or client behavior (ACE authors loot-sharing and its chat lines
  server-side). A second editor of a stored value is not a consumer — this
  is precisely AD-78's own store-only definition.
- **`FellowshipShareXP` -> Live is right.** The Create-button `OnClick`
  (`SocialFellowshipPageController.cs:377`) reads
  `CurrentCharacterOption(FellowshipShareXP)` and passes it as the sent
  `shareXP` bit — a genuine client-side read of the value that changes
  what the wire carries (lane B feature 1: "shareXP comes from option
  0x0F"). That is the exact asymmetry that separates it from ShareLoot.
- `IgnoreFellowshipRequests`/`FellowshipAutoAcceptRequests` -> dimmed:
  their only claimed consumer was the deleted intercept; both are pure
  server-side filters. Correct.
Conformance test updated to `Assert.Equal(34, …)` / `Assert.Equal(16, …)`
with the three ids added back to `ExpectedStoreOnlyIds`.

### SHOULD-FIX / NIT — all applied (spot-verified)
SF-1 (`_fellowshipNameLinesProvider` cached, reassigned only on a real
name change — `:583`); SF-2/SF-3 (`_memberGuids` is the source of truth
for the membership diff and Recruit's already-a-fellow test, `:626`/`:657`
— no longer `_rows.Count`); SF-4 (`Dispose` unsubscribes the stored
`_onActivePageChanged`); SF-5 (the D4 panel-level conjunction is now
tested by `FellowshipPageVisible_Declares0x00A6_OnlyWhenWindowShownANDFellowshipActive`,
plus a member-leaves shrink test and a selection-clear test); SF-6 (the
probe now `Assert`s the four labels non-empty and the captions exactly
`"Open"`/`"Close"`); SF-7 (gate step 3 self-contradiction reconciled);
SF-8 (see dimming above); SF-9 / N-0 / N-1 / N-2 / N-3 (AD-78 count,
optimistic caption pre-toggle, meter-child + `max>0` + snapshot-atomicity
doc notes) all present.

### Totals and probe
`ACDREAM_PROBE_LIVE_MOUNT=1` re-run on the post-fix binaries: PASS 1/1 —
the four checkbox labels resolve, captions resolve `Open`/`Close`, tab
captions Allegiance/Fellowship/Friends/Squelch. The six touched App test
classes pass **109/109** on the post-fix Release binaries (rebuilt
07:36). The ledger's **+13 / 0-deletion** delta reconciles by direct count
(8 in `SocialFellowshipPageControllerTests` = 2 new `[InlineData]` + 6
`[Fact]`; 4 `[Fact]` in `SocialPanelControllerTests`; 1 `[Fact]` in
`GameplayConfirmationControllerTests`; the two count-only changes are
net-0). The full-suite **13,285/4/0** claim is corroborated on the touched
projects and by arithmetic (13,272 + 13); it was not re-run end-to-end
here.

### Disposition
One REOPEN (MUST-FIX 3 — re-declaration placed at a pre-world reset seam,
so `0x00A6` never reaches the reconnected server). Everything else CLOSED,
with the dimming arithmetic audited correct at 34/16. Recommend a focused
MUST-FIX-3-only re-fix (move the re-declaration to an in-world seam); no
re-review of the other findings is owed.

---

## MF-3 REOPEN re-fix re-review — 2026-08-12 (commit `04161def`)

**Verdict: CLOSED.** The seam ordering the whole fix rests on holds — the
`EnteredWorld` re-declaration is genuinely post-world, the inverse of the
pre-world `ResetSessionState`/`SessionDialogs` stage the original fix used.
The widget-level root I named is addressed, the tests model the world gate
(not an unconditional fake), and the full suite reconciles.

### The two halves, verified in the diff
1. **Widget-level (the root I named).**
   `SocialFellowshipPageController.SetPageVisible` now advances the latch
   ONLY on `Accepted`:
   `if (_bindings.SetPanelOpen(visible).Status == RuntimeCommandStatus.Accepted) _pageVisible = visible;`
   A dropped/Inactive publish (the pre-world reconnect state) leaves
   `_pageVisible` untouched, so the in-world attempt is not deduplicated
   away by the `if (_pageVisible == visible) return` guard. The normal
   in-world case is unchanged (Accepted → latch advances as before), and a
   persistently non-Accepted result retries at most once per generation
   (EnteredWorld fires once per generation) — no retry loop.
2. **Lifecycle split.** `ResetSessionDeclaration` (pre-world) now ONLY
   clears the latch (`=> _fellowship?.ResetPageVisibleLatch();`) — it no
   longer calls `UpdateFellowshipPageVisibility`, so it makes no dropped
   pre-world declaration. The new `RedeclareAfterWorldEntry()`
   (`=> UpdateFellowshipPageVisibility();`) does the re-evaluation, exposed
   as `RetailUiRuntime.RedeclareSocialPanelAfterWorldEntry` and composed
   into `LiveSessionRuntimeFactory`'s `EnteredWorld.RestoreLayout` delegate
   (RestoreLayout first, then redeclare).

### Seam ordering — traced, and it holds (the load-bearing claim)
The fix is correct only if `EnteredWorld` runs after `_inWorld = true` and
after the command seam is active. It does. In `LiveSessionController.StartCore`:
- `ResetHostBeforeStart` (`:555`) → `host.ResetSessionState` →
  `LiveSessionLifecycleHost.ResetSessionState` (`:47`) → `_bindings.Reset`
  → the reset manifest's `SessionDialogs` stage → `ResetSessionTransientUi`
  → `ResetSessionDeclaration` (clears the latch). **PRE-world** — `_inWorld`
  is still false here.
- `binding.ActivateCommands()` (`:639`) — the deferred command seam binds
  for the new generation.
- `_inWorld = true` (`:642`).
- `host.ApplyEnteredWorld(selection)` (`:644`) →
  `LiveSessionLifecycleHost.ApplyEnteredWorld` (`:57`) → `_bindings.Entered`
  → `LiveSessionHost.ApplyEnteredWorld` (`:212`) →
  `_enteredWorld.RestoreLayout()` (`:216`) → the re-fixed delegate →
  `RedeclareSocialPanelAfterWorldEntry` → `RedeclareAfterWorldEntry` →
  `UpdateFellowshipPageVisibility` → `SetPageVisible(true)`. **POST-world**
  — `_inWorld` is true and commands are active, so
  `SetPanelOpen`'s `Validate(requireWorld: true)` passes → **Accepted →
  `0x00A6` published** → latch advances. Fellow vitals resume on the fresh
  server.

This is the exact inverse of the pre-world `SessionDialogs` stage traced in
the REOPEN, and it is idempotent: the social panel is not a
state-managed-visibility window, so `RestoreLayout` does not re-show it and
no `OnShown` edge fires — `RedeclareAfterWorldEntry` is therefore
load-bearing (not redundant) in the reconnect-with-panel-open case, while
still a safe no-op if some other path had already re-declared (latch
already set).

### Tests genuinely exercise the world gate (and would fail against pre-fix)
The fake `SetPanelOpen` now returns `AcceptedResult` when its
`inWorld`/`PanelOpenInWorld` flag is true and `InactiveResult` when false —
a real world-gate model, not an unconditional recorder.
- `SetPageVisible_DoesNotLatch_WhenDeclarationDropped_SoItRetriesInWorld`:
  sets the flag false, `SetPageVisible(true)` records the dropped attempt;
  flips the flag true, `SetPageVisible(true)` AGAIN records a send. Against
  the pre-fix unconditional `_pageVisible = visible`, the second call would
  early-return (deduplicated) and record nothing — the `Assert.Contains`
  would fail. So this pins the widget-level root.
- `Reconnect_ReDeclares0x00A6_AfterWorldEntry_NotDuringPreWorldReset`:
  after the in-world declare, sets in-world false, calls
  `ResetSessionDeclaration()` and asserts `DoesNotContain(...set-panel-open...)`
  — this is exactly the assertion that fails if the pre-world declaration is
  reintroduced (matching the coordinator's RED-verification); then sets
  in-world true, calls `RedeclareAfterWorldEntry()` and asserts the send
  lands.
- `Reconnect_StaysSilent_WhenFellowshipPageIsNotActuallyOpen`: default tab
  Allegiance; after reset + post-world redeclare, the D4 conjunction is
  false, so no send — the counterpart holds.

### Build currency and totals
The current App test binary is post-`04161def`: the three new tests
reference `SocialPanelController.RedeclareAfterWorldEntry`, a method that
exists only in the fixed source — the project would not compile against
pre-fix source, so a resolving+passing run proves the binary reflects the
fix. Ran on it: the three new tests 3/3, and the three touched classes
(`SocialFellowshipPageControllerTests` / `SocialPanelControllerTests` /
`GameplayConfirmationControllerTests`) 58/58. The **13,286/4/0** full-suite
claim reconciles by direct count: net **+1** test from the prior 13,285
(the one new widget `[Fact]`; the two panel tests were renamed in place,
net 0), 0 deletions. Not re-run end-to-end here.

### Disposition
MF-3 is CLOSED. All five MUST-FIX, all 9 SHOULD-FIX, all 4 NIT, and blast
SF-1 are now correctly applied; the D6/D7/SF-8 dimming stands at the
audited-correct 34/16. The FA4 fix round is fully resolved on the
mechanism lens; only the user's connected gate (several steps
`[TWO-CLIENT]`, deferrable to FA6) remains owed.