docs: CH2 review REJECT — findings doc + ledger (renders-never + table transcription)

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

docs/plans/2026-08-09-chat-parity-campaign.md

@@ -102,7 +102,7 @@ implementer per slice against a pinned contract (per
 |---|---|---|---|---|
 | R1–R4 research | `see docs/research/2026-08-09-chat-retail-*` | — | — | — |
 | CH1 colors | `172c6f9a` | 11,835 passed / 4 skipped / 0 failed | APPROVE-WITH-FIXES; fixed `34d8a3c0` | pending |
-| CH2 interface text | `77c8296e` | 11,890 passed / 4 skipped / 0 failed | pending | pending |
+| CH2 interface text | `77c8296e` | 11,890 passed / 4 skipped / 0 failed | REJECT (`docs/research/2026-08-09-ch2-review-findings.md`); rework in progress | pending |
 | CH3 side channels | — | — | — | — |
 | CH4 commands | — | — | — | — |
 | CH5 closeout | — | — | — | — |

docs/research/2026-08-09-ch2-review-findings.md

@@ -0,0 +1,163 @@
+# CH2 review findings — `77c8296e` (REJECT; rework list)
+
+Opus dual-lens review, 2026-08-09. Method: all 339 `case` bodies of
+`HandleFailureEvent @0x00571990` extracted, every `push imm32` string
+reference in VA `0x571990–0x575480` swept from the PDB-paired binary
+(`check_exe_pdb.py` → MATCH), diffed against the landed table,
+cross-checked against ACE's `WeenieError.cs`/`WeenieErrorWithString.cs`
+doc comments. The two oracles agreed on every discrepancy.
+
+## BLOCKER 1 — SpewBox never renders; pending queue unbounded
+
+`SpewBoxController.cs:75` constructs the `UiText` with `Visible = false`;
+`:94` (inside `ComputeLines`, the `LinesProvider`) is the only site that
+sets it true. `LinesProvider` is only invoked from `UiText.OnDraw`
+(`UiText.cs:407/429/451/472`), reached only through
+`UiElement.DrawSelfAndChildren`, whose first statement is
+`if (!Visible) return;` (`UiElement.cs:460`; `DrawOverlays` gates
+identically at `:511`). Start-invisible → provider never called →
+never visible → **no SpewBox line ever draws**. Consequence 2:
+`SpewBoxVM.Lines` is the sole caller of `SpewBoxState.Tick`, so
+`_pending` (`SpewBoxState.cs:66`, uncapped `Queue<string>`) never drains
+— an unbounded per-session leak on a hot path. The controller tests
+(`SpewBoxControllerTests.cs:28,43`) invoke `text.LinesProvider!()`
+directly, bypassing the `Visible` gate, so they cannot see it.
+
+**Fix:** retail drives `gmSpewBoxUI::Update` from the UI tick (global
+message 3, `UIElementManager::UseTime @0x0045CFD0`), not from drawing.
+Give `SpewBoxController` an explicit per-frame `Tick(double now)` wired
+from the retained-UI host loop (the existing per-UI-tick seam — see
+`IUiGlobalTimeListener` / wherever retained controllers receive frame
+time); it calls `_vm.Lines(now)`, caches the result, sets
+`_text.Visible` there; `LinesProvider` returns the cache (exactly
+`PortalWaitNoticeController`'s external-push shape). Tests: drive the
+frame hook (not the provider); assert the pending queue drains without
+a draw pass.
+
+## BLOCKER 2 — table: 5 missing ids, ~22 wrong strings, 0x4F8 resolvable
+
+Transcription by "enumerate `case` labels, read the pseudo-C's ~33-char
+inline preview" is structurally unsound: it misses ids dispatched by
+`else if (arg2 == N)` and merges siblings whose literals collide in the
+preview. Re-derive mechanically: sweep `push imm32` operands in
+`0x571990–0x575480`, read each UTF-16LE literal to NUL, attribute per
+case block by disassembly (BN's line addresses shift up to ~0x20 bytes);
+cross-check every row against ACE's enum comments.
+
+### 2a. Five missing ids (else-if dispatch)
+
+| id | retail literal | type |
+|---|---|---|
+| `0x04F` | `You fail to affect %s because $s cannot be harmed!` | Magic (7) |
+| `0x3EE` | `The container is closed!` | ClientLocal (0x1A) |
+| `0x408` | `Your spell cannot be cast inside` | ClientLocal (0x1A) |
+| `0x48A` | `You must be a monarch to purchase this dwelling.` | Default (0) |
+| `0x4E8` | `The %s cannot be used while on a hook, use the '@house hooks on' command to make the hook openable.` | Default (0) |
+
+`0x43` is correctly absent (no text — only `AbortAutomaticAttack`).
+`0x3EE`/`0x408` are live on local ACE (`Player_Inventory.cs:883,955`,
+`Player_Magic.cs:513`). The landed test
+`Format_0x004F_FallsBackToHex_NoRetailCaseExists` pins the 0x04F
+regression and must be replaced.
+
+### 2b. Wrong strings (landed line → correct retail text)
+
+| line | id | correct retail string |
+|---|---|---|
+| `:166` | `0x051` | `You fail to affect %s because you are not a player killer!` |
+| `:168` | `0x053` | `You fail to affect %s because you are not the same sort of player killer as %s!` |
+| `:169` | `0x054` | `You fail to affect %s because you are acting across a house boundary!` |
+| `:218` | `0x466` | `You must purchase Asheron's Call: Dark Majesty to interact with that portal.` |
+| `:251` | `0x4A3` | `You must have linked with a portal in order to recall to it!` |
+| `:268` | `0x4B5` | `You must specify a character to query.` |
+| `:306` | `0x4E0` | `…certain level of skill. Your attributes cannot be transferred…` (transfer wording, not skill-lowering) |
+| `:328` | `0x4F7` | `%s fails to affect you because you are not a player killer!` |
+| `:401` | `0x544` | `…attempting to remove %s as an allegiance officer.` |
+| `:411` | `0x54E` | `The hook does not contain a usable item. You cannot open the hook because you do not own the house to which it belongs.` |
+| `:415` | `0x552` | `…Throne of Destiny to use this function.` |
+| `:416` | `0x553` | `…Throne of Destiny to use this item.` |
+| `:417` | `0x554` | `…Throne of Destiny to use this portal.` |
+| `:418` | `0x555` | `…Throne of Destiny to access this quest.` |
+| `:460` | `0x57F` | `Your allegiance chat privileges have been temporarily removed by %s. Until they are restored, you may not view or speak in the allegiance chat channel.` |
+| `:463` | `0x582` | `Your allegiance chat privileges have been restored by %s.` |
+
+(`0x581` at `:462` is correct.) For the `0x552`–`0x555` family and
+`0x4E0` the reviewer gave shapes — the fixer's binary sweep provides the
+exact full texts; every row must come from the swept literal, not this
+table's abbreviations. All routing TYPES were verified correct — only
+strings change.
+
+### 2c. 0x4F8 resolves — delete the exclusion
+
+`data_7d2ee8` = `" fails to affect you because you are not the same sort
+of player killer as "`, `data_7d2f80` = `"!\n"`; block stages
+`arg3 + literal + arg3 + "!\n"` through three `operator+` calls
+(`0x57475e/0x574765/0x574776`); type 7 (Magic) at the block's
+`AddTextToScroll (0x00573ba5)`. Row:
+`[0x4F8u] = new("%s fails to affect you because you are not the same sort of player killer as %s!", RetailLogTextType.Magic)`
+(both `%s` take the same param — replace-all already does this). Delete
+`Resolve_0x4F8_IsDeliberatelyExcluded_FallsBackToHex`; the class-doc
+claim about 0x4F7's "dangling" literal is a misattribution of 0x4F8's
+first operand — fix it. New pinned count: **344** (338 + 5 + 0x4F8).
+
+## SHOULD-FIX
+
+1. **`ResetSpewBox()` is dead code** (`RuntimeCommunicationState.cs:135`
+   — no caller; `RuntimeGenerationReset.cs:318` only resets
+   ChatIdentity). Fold `SpewBox.Reset()` into the ChatIdentity stage (same
+   lifetime boundary) or add a stage; add a reset test.
+2. **`AddText` trims the wrong end + invents an empty-drop**
+   (`RuntimeCommunicationState.cs:172-176`). Retail's
+   `AddTextToScroll` calls `trim(&str, 1, 1, ws)` at `0x00563ce3` —
+   BOTH ends (the trailing-only trim in research §3.1 is
+   `gmSpewBoxUI::Update`'s separate call). Retail also broadcasts empty
+   strings deliberately (`s_NullBuffer` at type 7). Fix: `text.Trim()`;
+   remove the empty early-return (and its pinning test) — or keep it
+   with a register row.
+3. **`ShowWeenieError` bypasses the chokepoint** —
+   `LiveSessionRuntimeFactory.cs:341` still routes through
+   `ChatLog.OnWeenieError` (hardcoded type 0x00, comment cites retired
+   AP-176). `0x0561` belongs in the SpewBox and lands green in chat.
+   Fix: route through `Communication.AddText(Resolve(code, param))` and
+   delete `ChatLog.OnWeenieError` — or restore an AP row.
+4. **Unmapped-id fallback is unregistered divergence**
+   (`WeenieErrorMessages.cs:129-134` emits `WeenieError 0xNNNN` into
+   chat). Retail's switch has no default — unhandled ids produce NO
+   text. Fix: match retail (silence toward the player), keep the hex id
+   as a diagnostics log line only; flip/delete the fallback tests — or
+   file a register row.
+
+## NITS
+
+1. `SpewBoxController.cs:34,42` — "AP-TBD" ×2 → AP-177 (lifetime),
+   AP-178 (position/colour).
+2. Three stale "the split lands with CH2" comments (`ChatVM.cs:122`,
+   `HeadlessGameplayOperations.cs:231`, `LiveSessionRuntimeFactory.cs:336`)
+   + `AddText`'s `windowId` accepted but never consumed: retail's §2.3
+   dual-destination (0x1A + non-zero windowId → SpewBox AND the
+   originating chat window, ~40 sites) is unimplemented. File a register
+   row for it and update the comments (implementing it is CH4/CH5 scope
+   at the earliest).
+3. `SpewBoxLayoutDumpDiagnostic` almost certainly never consulted
+   `dats.Local` (`client_local_English.dat` — the file research §8.1
+   names); also the `0x10000012` probe treats a layout enum as a dat id.
+   Extend the sweep to `dats.Local` before trusting AP-178's wording; if
+   still negative, keep the row as-is.
+4. `WeenieErrorMessages.cs:36-41` — says "19 ids", lists 18.
+5. `SpewBoxState` doc overstates "exactly one frame" decoupling —
+   same-call `Tick`+`Snapshot` displays same-frame; retail is 0–1. Fix
+   wording.
+6. `GameEventWiring.cs:261,276` apply `IsSilentClientControlStatus` to
+   0x028A/0x028B but the UseDone path (`:596-601`) does not — harmless
+   today; align or comment.
+
+## CONFIRMED-OK (do not touch)
+
+Core placement of `SpewBoxState` (canonical instance owned by
+`RuntimeCommunicationState`, UI borrows); `0x47` silent (DoJump's jump
+table has exactly 4 targets; `CommenceJump`'s fallback unreachable —
+brief's §4.2 reading was wrong); all 338 routing TYPES verified against
+the decomp with zero mismatches; all 11 `ClientTextRefusals` byte-exact;
+jump-family constant sharing; AP-177/178/179 content; placeholder
+honesty; architecture/layering; the `ClientLocal`-never-reaches-
+transcript test.