erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

docs: FA4 fix-round narrow re-review -- CLOSED with one REOPEN (MUST-FIX 3)

Browse source 
Re-derived each disposition from the actual fix diffs (290f9b58/5499f058/
df000306/300d8189/55b17e15/1d743277/f041b09b), not the commit claims.

CLOSED (4/5 MUST-FIX, all 9 SHOULD-FIX, all 4 NIT, blast SF-1):
- MUST-FIX 1: (int)((double)pct*100.0) truncation + 6->44%/8->34% pinning
  cases + gate step corrected.
- MUST-FIX 2: intercept deleted, every type routes to the generic
  controller, type-4 dialog test added; type-1 allegiance path unaffected
  (was never intercepted).
- MUST-FIX 4: world->panel selection sync reproduces retail's found/
  fallback arms; AD-82 records the deferred generic UiTemplateListBox
  selection-model port honestly -- minimal-observable-contract, not a
  hidden gap.
- MUST-FIX 5: AD-82/AD-83 well-formed; AD-78 count corrected to 34/16.
- D6/D7/SF-8 dimming (audited from source): 34 dimmed / 16 live is
  correct, not split-the-difference. FellowshipShareLoot has NO client
  value-reader (only an editor/display surface; 0x00A2 sends shareXP
  alone; ACE authors loot server-side) -> dimmed faithful.
  FellowshipShareXP is genuinely read by the Create click -> Live right.

REOPEN (MUST-FIX 3): the 0x00A6 reconnect re-arm is placed at a pre-world
reset seam. ResetSessionTransientUi runs via the SessionDialogs reset
stage at ResetHostBeforeStart / retired-scope teardown -- both BEFORE
_inWorld=true and before command activation for the new generation -- and
SetPanelOpen requires world, so the re-declaration returns Inactive and
nothing is published, yet _pageVisible is still set true and no
post-world-entry hook re-evaluates. The new server never receives 0x00A6
and fellow vitals stay frozen -- the exact bug the fix targets. The unit
test passes only because its fake command records unconditionally.
Recommend moving the re-declaration to an in-world seam (EnteredWorld).

Totals/probe: 109/109 touched App test classes green on post-fix
binaries; live-mount probe PASS 1/1; +13/0-deletion delta and 13,285/4/0
corroborated on the touched projects and by arithmetic (not re-run
end-to-end).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-12 07:48:26 +02:00
parent f041b09b7c
commit 8bbceff594
1 changed files with 173 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

173
docs/research/2026-08-12-fa4-review-mechanism.md
View file

@ -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.