docs: FA5 blast-radius review -- APPROVE-WITH-FIXES (1 SHOULD-FIX, 2 NIT)

Suite-accounting SHOULD-FIX: the FA5 ledger/commit cite FA4's INTERMEDIATE
13,285 figure as the baseline and claim +11 net, but FA4 CLOSED at 13,286
(its 'Final full suite' figure) and the real net is +10 (verified per-file
[Fact] counts: SocialPanelControllerTests 22->31, Confirmation 4->5, probe
1->1) -- the ledger's own itemization already sums to +10, contradicting
its +11 headline. End figure 13,296/4/0 is itself correct; documentation
fix only.

Verified clean: all three Callbacks/Bindings construction sites pass the
widened Allegiance binding; every production accessor fed from a real seam;
the 0x001F and 0x00A6 toggles are independent edge-triggered latches with
no cross-talk (38 Fellowship tests green); ResolveWorldObjectName reuses
the Toolbar's ClientObjectTable read and ShowConfirmation is a pre-existing
shared method with no Fellowship collision; @allegiance info/0x027C path
untouched (52 Core.Net + 16 Runtime allegiance tests green); FA5 makes zero
Runtime changes; register 63->66 rows accurate (AD-84/85/86 + AD-82
addendum); NUL-fix correct and no residual control bytes in any of the 11
touched files.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

docs/research/2026-08-12-fa5-review-blast.md

@@ -0,0 +1,215 @@
+# Campaign FA slice FA5 — BLAST-RADIUS review
+
+**Reviewer lens:** blast radius — the axes the implementer may not have
+traversed (construction-site fan-out, cross-subsystem toggle interaction,
+shared-seam collisions, suite-accounting, register/ledger/gate-script
+bookkeeping, and the NUL-byte class).
+**Target commits:** `7ed79eaf` (code), `bc29a1db` (register/gate-script/
+ledger), `7e394cbf` (NUL-byte repair) on `claude/latest-commits-cb0c8f`.
+**Date:** 2026-08-12.
+
+## VERDICT: APPROVE-WITH-FIXES
+
+One SHOULD-FIX, documentation-only (suite-accounting arithmetic). Every
+functional axis in scope verified clean; the shipped end-state figure
+(13,296/4/0) is itself correct — the error is only in the delta narrative.
+Two NITs (coverage observations). No REJECT-class findings.
+
+---
+
+## SHOULD-FIX 1 — the ledger/commit cite the wrong FA4 baseline and a +11 delta that contradicts their own itemization (actual net is +10)
+
+**Where:** `docs/plans/2026-08-11-fellowship-allegiance-campaign.md` FA5
+ledger row (line 314) and commit `bc29a1db`'s message; echoed in
+`7ed79eaf`'s commit message ("up from FA4's 13,285/4/0 baseline").
+
+**The claim:** "Baseline **13,285**/4/0 (13,289 total, FA4's own ledger
+figure) → FA5 13,296/4/0 (13,300 total), directly measured, **+11 tests
+net**".
+
+**Why it's wrong — three independent proofs:**
+
+1. **FA4 did not close at 13,285.** The FA4 ledger row (line 313) carries
+   TWO figures: an intermediate `13,285 passed / 4 skipped / 0 failed
+   (13,289 total), directly measured` (the FA4 *pre-fix-round* code
+   measurement) and, after FA4's own MUST-FIX/MF-3-REOPEN fix rounds,
+   **`Final full suite 13,286/4/0`**. The FA4-CLOSE baseline is **13,286**,
+   not 13,285 — the implementer grabbed the earlier of the two figures in
+   FA4's own row.
+
+2. **The real net is +10, verified by counting `[Fact]`/`[Theory]` per
+   file before (`f5bd3e56`) vs after (`7ed79eaf`):**
+   - `SocialPanelControllerTests.cs` 22 → 31 = **+9** (10 added, 1 removed
+     — `Allegiance_HasProfile_ShowsBothBlocks` confirmed present at
+     `f5bd3e56`, absent at `7ed79eaf`)
+   - `GameplayConfirmationControllerTests.cs` 4 → 5 = **+1**
+   - `SocialPanelLiveMountProbeTests.cs` 1 → 1 = **+0** (extended in place
+     inside its single env-gated `[Fact]`)
+   - Total **+10**.
+
+3. **The ledger contradicts itself.** Its own itemization reads "nets +9 …
+   +1 … net 0 new tests" = **+10**, directly under the "+11 tests net"
+   headline. `13,286 + 10 = 13,296` — which is exactly the (correct)
+   measured end figure. `13,285 + 11 = 13,296` is arithmetically self-
+   consistent but built on the wrong baseline and a delta one higher than
+   the itemization supports.
+
+**Blast:** none functional — 13,296/4/0 is the right end figure and the
+suite is green (targeted re-runs below all pass). This is a bookkeeping
+error that will mislead the next slice's baseline pick if left uncorrected.
+
+**Fix:** in the FA5 ledger row and (if re-touched) the closeout, change
+"13,285/4/0 … FA4's own ledger figure" → **13,286/4/0 (FA4's *Final full
+suite* close figure)** and "+11 tests net" → **+10 tests net**. No code or
+test change.
+
+---
+
+## NIT 1 — the combined tab-switch-while-shown toggle pairing has no dedicated test
+
+Concern #3's headline scenario (window shown, switch Allegiance↔Fellowship,
+verify BOTH subscriptions flip correctly) is covered only by reasoning +
+two separate axis tests (`AllegiancePageVisible_...OnWindowShown...`
+exercises the visibility axis; `ShowFellowship_SwitchesTheActiveTab`
+exercises the tab axis with the window still closed). The logic is
+correct (see concern #3 below), but a single test that shows the window,
+switches tabs, and asserts `allegiance-set-subscription:False` +
+`fellowship-set-panel-open:True` in one go would pin the no-cross-talk
+property directly. Optional.
+
+## NIT 2 — the FA5 live-mount probe assertions run in no automated context here
+
+`SocialPanelLiveMountProbeTests.ProbeLiveMountShapes` returns early unless
+`ACDREAM_PROBE_LIVE_MOUNT=1` (line 26-27) AND live DATs are installed
+(absent in this worktree). Its new FA5 assertions (scoped `0x10000492`
+dual-resolution, the vassal row template, the checkbox, the five
+confirmation/label strings, a full production `Bind()` pass) therefore
+compile but are never exercised by the suite — it passes trivially, adding
+0 tests and 0 skips. The implementer disclosed this honestly and tied it
+to the owed connected gate; recording it here only so the next reviewer
+does not mistake "37 App tests green" for "the live-mount probe ran".
+
+---
+
+## What verified CLEAN
+
+### Concern #1 — every `Callbacks`/`Bindings` construction site passes the new binding
+The record field changed from `Func<RuntimeAllegianceSnapshot>
+AllegianceSnapshot` to `SocialAllegiancePageController.Bindings Allegiance`.
+A grep for construction sites finds exactly three, all passing the widened
+binding — a defaulting site is impossible (positional record; the compiler
+would reject a missing arg):
+- **Production:** `RetailUiRuntime.cs:2779` `Allegiance: new …Bindings(...)`
+  — all 17 fields supplied.
+- **Test builder:** `SocialPanelControllerTests.cs:122` `MakeCallbacks` →
+  `MakeAllegianceBindings` (line 87) — all 17 fields.
+- **Probe builder:** `SocialPanelLiveMountProbeTests.cs:379` — all 17
+  fields (test doubles, fine).
+
+### Concern #2 — every `Bindings` accessor fed from a real production seam
+`RetailUiRuntime.cs:2798-2821` feeds all 17 fields; none null/no-op in
+production. `Monarch/Patron/Member/Vassals` ← `IRuntimeAllegianceView`
+(`GameRuntimeGameplayViews.cs:208`, real out-param accessors);
+`Swear/Break/Kick/SetUpdateSubscription` ←
+`DeferredGameRuntimeStateCommands` → `commands.Allegiance.*`;
+`ResolveWorldObjectName` ← `ClientObjectTable.Get()?.GetAppropriateName()`;
+`ShowConfirmation` ← `RetailUiRuntime.ShowConfirmation` →
+`DialogFactory.MakeConfirmation`. The `SocialRuntimeBindings` record grew
+8 new fields appended at the end (`RetailUiRuntime.cs:279-286`); the
+composition site (`InteractionRetainedUiComposition.cs:886-922`) uses
+positional args for the pre-existing 14 and named args for the 8 new ones,
+in matching order — well-formed.
+
+### Concern #3 — the 0x001F and 0x00A6 toggles share the conjunction with no cross-talk
+Both are independent edge-triggered latches on separate page controllers
+(`SocialAllegiancePageController.SetPageVisible` guards
+`if (_subscribed == visible) return;`; Fellowship's `SetPageVisible` is the
+same shape). `SocialPanelController._onActivePageChanged`, `OnShown`, and
+`OnHidden` each drive BOTH `UpdateFellowshipPageVisibility()` and
+`UpdateAllegiancePageVisibility()`. The two conjunctions
+(`_visible && IsShowingFellowship`, `_visible && IsShowingAllegiance`) are
+mutually exclusive on the active-tab axis, so:
+- Switch → Allegiance (window shown): Allegiance ON, Fellowship OFF.
+- Switch → Fellowship (window shown): Fellowship ON, Allegiance OFF.
+- Window hidden: both OFF.
+No shared mutable state between the two latches; no missed toggle.
+`RedeclareAfterWorldEntry` deliberately treats them differently — Fellowship
+stays conjunction-gated (`UpdateFellowshipPageVisibility`), Allegiance is
+UNCONDITIONAL (`_allegiance?.RedeclareAfterWorldEntry()`), matching retail's
+differing arm semantics (PlayerDescReceived always sends 1) and
+side-stepping the FA4 MF-3-REOPEN bug class. The 38 Fellowship App tests
+pass — no regression from the folded-in second toggle.
+
+### Concern #4 — ResolveWorldObjectName / ShowConfirmation seams
+`ResolveWorldObjectName` (`guid => _bindings.Inventory.Objects.Get(guid)?.
+GetAppropriateName()`) is the byte-identical read pattern the Toolbar's
+`ResolveName` uses (`InteractionRetainedUiComposition.cs:725`,
+`d.Inventory.Objects.Get(guid)?.GetAppropriateName()`) — a pure
+`ClientObjectTable` read, no registration, no lifetime coupling.
+`ShowConfirmation` is a **pre-existing shared method**
+(`RetailUiRuntime.cs:726`) already used by the Toolbar (line 1545) and the
+end-session paths (2102/2131); FA5 adds one more caller, each producing a
+fresh `DialogFactory.MakeConfirmation` dialog. **No collision with
+Fellowship:** the FA4 Fellowship `Bindings` has no `ShowConfirmation` field
+at all (production mount lines 2781-2797) — Fellowship commands send
+directly, so there is nothing to collide with. The server-driven
+accept-swear (ConfirmationType 1) routes through
+`GameplayConfirmationController`, which shares the same single
+`RetailDialogFactory` — but that is a pre-existing multi-consumer design
+(item/skill-training/end-session confirmations already share it); FA5
+introduces no new shared mutable state.
+
+### Concern #5 — the @allegiance info / 0x027C live path is untouched
+FA5's three commits touch **no** `AcDream.Core.Net` files (diff stat
+confirms: only App/Composition, App/UI, tests, docs). The parser
+(`ClientCommandResponses.cs`, `GameEventWiring.cs`) was last touched by FA2
+(`4272ad0e` = "0x027C stops seeding", the MF-2 correction). Targeted
+re-runs (Release, `--no-build`): **52 Core.Net allegiance tests pass**
+(incl. the 0x027C AllegianceInfoResponse parse path), **16 Runtime
+allegiance tests pass**.
+
+### Concern #6 — FA5 only CONSUMES RuntimeAllegianceState.SetUpdateSubscription
+FA5's code commit touches zero `src/AcDream.Runtime/` files (the "Runtime"
+strings in its stat are `RetailUiRuntime.cs`/`InteractionUiRuntimeSources.cs`,
+both App). `SetUpdateSubscription` (plus Swear/Break/Kick) exists on
+`IRuntimeAllegianceCommands` (`GameRuntimeCommands.cs:367`) and **both**
+host adapters — `DirectGameRuntimeCommandAdapter.cs:1043` and
+`CurrentGameRuntimeCommandAdapter.cs:1031` — all shipped by FA2.
+
+### Concern #8 — register / gate-script bookkeeping
+- **Register:** AD-84 (Swear IsPlayer gate, mirrors AD-83), AD-85 (unported
+  StringInfo gap, mirrors AD-81), AD-86 (ACE's seven zeroed fields) are
+  well-formed with file:line anchors and retail citations; the AD-82
+  addendum is correctly appended to the existing row (no new row for it).
+  Section-2 header `63 → 66 active rows`; **actual table row count is
+  exactly 66** — accurate.
+- **Gate-script §FA5** (lines 519-739): mirrors §FA4's structure; the CF-1
+  correction note (0x001F not 0x027B/0x027C), the SF-7 per-relationship
+  steps, the reconnect MF-3-REOPEN analogue, the AD-85 numeric-only honesty,
+  and the ACE-zeroed-field honesty are present; `[SOLO]`/`[TWO-CLIENT]`
+  markings are honest (swear/populated-roster/break/kick/offline-dimming
+  genuinely need a second client; subscription/empty-state are solo).
+- **Ledger FA5 row:** substantively accurate except the baseline/delta error
+  (SHOULD-FIX 1).
+
+### Concern #9 — the NUL-byte class
+`7e394cbf` correctly restored the two spaces: `BlankSentinel`'s bytes are
+now `22 20 62 6C 61 6E 6B 20 22` = `" blank "` (0x20 on both sides of
+"blank"); the pre-fix `7ed79eaf` version had exactly 2 NUL bytes.
+**Independent byte-level sweep of ALL 11 FA5-touched files** (5 source, 3
+test, 3 doc): **0 NUL, 0 sub-0x20 control bytes (excluding TAB/LF/CR), 0
+0x7F** — the implementer's "swept … found none elsewhere" claim is verified.
+
+---
+
+## Targeted test evidence (Release, `--no-build`; artifacts fresh — App.Tests.dll built 08:47 after the 08:43 source edit)
+
+| Filter | Result |
+|---|---|
+| Core.Net `~Allegiance` | 52 passed / 0 failed / 0 skipped |
+| Runtime `~Allegiance` | 16 passed / 0 failed / 0 skipped |
+| App `~SocialPanelController\|~GameplayConfirmationController\|~SocialPanelLiveMountProbe` | 37 passed / 0 failed / 0 skipped |
+| App `~Fellowship` (regression) | 38 passed / 0 failed / 0 skipped |
+
+The full-suite figure was reconciled analytically (13,286 close + 10 net =
+13,296), not re-measured — the review charter forbids a full build.