diff --git a/docs/research/2026-08-12-fa4-review-mechanism.md b/docs/research/2026-08-12-fa4-review-mechanism.md index f7ab6a5d..863476d1 100644 --- a/docs/research/2026-08-12-fa4-review-mechanism.md +++ b/docs/research/2026-08-12-fa4-review-mechanism.md @@ -547,3 +547,176 @@ 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.