docs: FA4 mechanism review -- reconcile with the concurrent pass at 6849b457
A second mechanism-lens review landed on the same path at6849b457while this one was in progress and was overwritten by913e35cd. Its text is recoverable from git and is now cited from a new appendix, its two unique findings are carried forward, and the two places the reviews disagree are adjudicated from primary source. Carried forward: - SF-9: AD-78's register row still says "35 of 50 rows dimmed" (the D7 addendum landed in the class doc, not the row's Where column). - N-0: the Open/Close caption does not optimistically pre-toggle; lane B feature 11 records that retail's handler pre-toggles _open_fellow locally before sending 0x0291. Adjudicated: - _ftol2 vs MathF.Round:6849b457filed it a NIT ("round and truncation agree on every table value"). That holds for the DECIMAL literals, not the stored floats -- 0x007C91D4 = 0.44999998807907104 and 0x007E72BC = 0.3499999940395355, so retail truncates 44.999998/34.999999 to 44/34 while acdream rounds to 45/35. Stays MUST-FIX 1. - D6 invite auto-response:6849b457passed it as verified-clean after confirming the code matches the plan. The binary says retail has no such client-side read on any confirmation path. Stays MUST-FIX 2. The reviews agree on the reconnect D4 hole, the missing panel-level D4 conjunction test, the leader-tint register omission, and the live-DAT probe result. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
913e35cdb5
commit
e202fbef6e
1 changed files with 46 additions and 1 deletions
|
|
@ -10,7 +10,7 @@ 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
|
`docs/research/2026-08-11-fa-fellowship-wire.md` (lane B) and
|
||||||
`-fa-panel-structure.md` (lane A) as ground truth.
|
`-fa-panel-structure.md` (lane A) as ground truth.
|
||||||
|
|
||||||
**Verdict: APPROVE-WITH-FIXES — 5 MUST-FIX, 8 SHOULD-FIX, 3 NIT.**
|
**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
|
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
|
table is a verbatim port, the even-split *table* is byte-exact, the
|
||||||
|
|
@ -442,10 +442,31 @@ in AD-78. (`FellowshipShareXP` is defensible: the Create click genuinely
|
||||||
reads it, `:325-326`.) The same "second surface" argument in
|
reads it, `:325-326`.) The same "second surface" argument in
|
||||||
`CharacterOptionsPageController`'s doc should be corrected either way.
|
`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
|
## 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
|
**N-1 — meter text vs authored child.** Retail's `UpdateFellowVitals
|
||||||
@0x0048ED60` sets meter attribute `0x69` to `cur/max` and writes the
|
@0x0048ED60` sets meter attribute `0x69` to `cur/max` and writes the
|
||||||
cur/max ints into the meter's authored **child** text elements
|
cur/max ints into the meter's authored **child** text elements
|
||||||
|
|
@ -502,3 +523,27 @@ reader from assuming atomicity.
|
||||||
lock as every other read, honest ordering caveat in its doc.
|
lock as every other read, honest ordering caveat in its doc.
|
||||||
- **`SelectionChangeSource.Social`** — additive; no exhaustive switch on
|
- **`SelectionChangeSource.Social`** — additive; no exhaustive switch on
|
||||||
the enum exists in production, so no dispatch site was missed.
|
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.
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue