# Issue #319 — independent retail-conformance review (2026-08-05) **Verdict: PASS.** Reviewed: the uncommitted working tree at base HEAD `af828a8a`, branch `claude/acdream-physics-divergence-5aa784` — `git diff HEAD` (13 files, +709/-30) plus the untracked contract `docs/research/2026-08-05-issue-319-contract.md`. Review only. No file in the tree was modified by this review except this report. Independent gates run for this review (a green suite is not evidence, but a red one is): - `dotnet build AcDream.slnx -c Release` — exit 0. - `AcDream.Runtime.Tests` Release — 1170/1170, 0 skipped. - `AcDream.App.Tests` Release — 4125 passed, 3 skipped, 0 failed. --- ## 1. The central retail claim — independently verified, and the contract ## UNDERSTATES its own anchor Every retail claim below was read at `docs/research/named-retail/acclient_2013_pseudo_c.txt` for this review. None was inherited from the contract or the investigation. | contract claim | address | verified | |---|---|---| | CreateObject's attach is GUID-only | `ACCObjectMaint::CreateObject` @0x00558870; `PhysicsDesc::get_parent_id` @0x00558a18 → `if (eax_20 != 0)` @0x00558a1f → `CObjectMaint::GetObjectA(this, eax_20)` @0x00558a2d → `get_parent_location_id` @0x00558a31 → `CPhysicsObj::set_parent(result, arg2, eax_22)` @0x00558a3e | ✓ **exact**. `GetObjectA` takes `(this, guid)`. There is no instance-sequence read, comparison, or argument anywhere in the block. | | The reverse direction is the same shape | `CObjectMaint::SetChildren` @0x00509370: `unparent_children` @0x00509379, hash-bucket walk on `PhysicsDesc::get_child_id` @0x005093b2–@0x005093ca, `GetNullObject(this, get_child_id(...), 1)` placeholder @0x005093e6, `set_parent(hash_next_1, arg2, get_child_location_id(...))` @0x005093f8 | ✓ **exact**, including the `GetNullObject` placeholder for a not-yet-constructed child. No instance field participates. | | No player branch in the relation path | `set_parent` @0x00515A90 (3-arg) and @0x00515B50 (4-arg, Frame); `enter_cell` @0x00510ED0; `change_cell` @0x00513390 | ✓. Both `set_parent` overloads operate on `CPhysicsObj*` uniformly: `add_child` → `unset_parent` → `leave_world` → `parent = edi` → `if (cell != 0) change_cell` → `UpdateChild` → `recalc_cross_cells`. The only conditional in either body is the parent's `state & 0x4000` no-draw cascade @0x00515B26. `enter_cell`'s only guard is `part_array != 0` @0x00510ED8. **No player test exists in any of them.** | | Retail's player cell is written per tick | `CPhysicsObj::SetPositionInternal` @0x00515330 | ✓ — and **stronger than the contract states**, see below. | | `update_object` early-outs on a parented object | @0x00515D40: `if ((this_3->parent != 0 \|\| (this_3->cell == 0 \|\| (this_3->state & 0x1000000) != 0))) { transient_state &= ~0x80; return; }` | ✓ **exact**. A parented child is never independently updated or re-placed. | **The contract's §1 row 5 cites the weaker half of its own best anchor.** `SetPositionInternal` @0x00515330 does not merely write the mover's own cell: in the same-cell branch it writes `this->m_position.objcell_id = objcell_id` @0x00515385, then **walks `this->children` and writes each child's cell id directly** — `*(uint32_t*)((char*)eax_2 + 0x4c) = objcell_id_1` @0x005153BD, followed by `CPartArray::SetCellID` @0x005153CC, looping @0x005153AE– @0x005153D8; the cross-cell branch delegates to `change_cell` @0x00515372. That is retail's D2 equivalent living **inside the same per-tick function** as the player's own cell write. It is a strengthening, not a contradiction: the child-equals-parent equality invariant and AP-146's "retail's player cell is never stale" both get a single anchor. (AP-142 clause (b) already cites this range; only §1's summary table understates it.) ### Conclusion on the central claim **Retail does NOT distinguish a player parent, and there is no instance-sequence check anywhere in the relation path — confirmed independently.** acdream's player/non-player split is exactly what the contract says it is: an artifact of keying committed relations by `(guid, incarnation)` against a wire message that carries only a GUID. **Late-binding to whoever currently holds the GUID is the faithful mapping**, because "the current holder of the GUID" is literally what `GetObjectA` / `GetNullObject` return. The fix shape is right. --- ## 2. Scope expansion (`OnCreateParentAccepted`) — JUSTIFIED, and the fix is ## correct there The contract's Scope line named only `EquippedChildRenderController.OnSpawn`. The implementer also fixed `OnCreateParentAccepted`. Verified: - `CreateParentUpdate` is declared at `src/AcDream.Runtime/Entities/InboundPhysicsStateController.cs:1399-1405` as `(ChildGuid, ParentGuid, ParentLocation, PlacementId, ChildInstanceSequence, ChildPositionSequence)`. Its producer, `BuildSameGenerationEvents` (`:1240-1248`), fills `ChildInstanceSequence` from `ts.Instance` where `ts` is `incoming.Physics.Timestamps` and `incoming` **is the child**. So `ChildInstanceSequence` is the CHILD's own timestamp, exactly as the implementer reports. **No parent instance sequence exists on this shape.** - The codebase already knew this: `InboundPhysicsStateController. TryApplyCreateParent`'s pre-existing doc comment (`:216-218`) reads "Unlike standalone ParentEvent it carries no parent INSTANCE_TS, so only the child's shared POSITION_TS participates in freshness." - Pre-fix, `OnCreateParentAccepted` hardcoded `ParentInstanceSequence: 0` into the identical `Relations.AcceptCreateObjectRelation` call. **Byte- identical defect.** Shipping the key fix on one producer and not the other would have left #319 half-open for every same-generation ObjDesc refresh of a player-parented child. Routing both through one `AcceptLateBoundCreateObjectRelation` (`EquippedChildRenderController.cs:836-857`) is the right shape and the right commit. --- ## 3. Findings ### R1 — MINOR (incorrect load-bearing comment). Both deferred branches are production-unreachable, not just `OnSpawn`'s. `tests/AcDream.App.Tests/Rendering/EquippedChildProjectionWithdrawalTests.cs:875-881` The test's XML doc asserts: > `CreateParentUpdate`'s producer … has no equivalent parent-addressability > precondition, so this is the reachable shape for #319's deferred branch. **Verified false.** The only production driver of `OnCreateParentAccepted` is `LiveEntityHydrationController.OnCreate`'s `result.SameGenerationEvents is { } refresh` branch (`LiveEntityHydrationController.cs:340-341` → `LiveEntityNetworkUpdateController.cs:380` → `LiveEntitySameGenerationUpdateRouter.Apply` → `OnParent` → `:400-401`). `SameGenerationCreateObjectEvents` are produced only inside `InboundPhysicsStateController.AcceptCreate` (`:70-90`), reached only from `RuntimeEntityObjectLifetime.RegisterEntityCore` — and `RegisterEntityCore`'s deferral gate runs **before** `PreviewCreateDisposition`: ``` RuntimeEntityObjectLifetime.cs:798-812 uint parentGuid = incoming.ParentGuid ?? incoming.Physics?.Parent?.Guid ?? 0u; if (beginInitialResidence && parentGuid != 0u && !Entities.TryGetActive(parentGuid, out _)) { Entities.ParentAttachments.EnqueueDeferredCreate(...); return DeferredForParent: true; } ``` The `??` chain covers both the flattened `ParentGuid` and the nested `Physics.Parent.Guid`, i.e. both shapes `OnSpawn` and `OnCreateParentAccepted` read. The App graphical host is the `beginInitialResidence: true` caller (`LiveEntityRuntime.cs:583`). So by the time either producer runs, the parent is active. The implementer's `OnSpawn` finding is correct; the extension of it to `OnCreateParentAccepted` is not. Retail address contradicted: none — this is a reachability claim about acdream, not about retail. **Not a code defect.** The deferred machinery is correct, and it is genuinely defensive: the deferral gate reads `Entities.TryGetActive` (the `_activeByGuid` map) while the late-bind reads `TryGetSnapshot` (the `_inbound._snapshots` map) — two distinct stores, so a producer-side fallback is warranted rather than an assertion. Correct behaviour: the comment should say the deferred branch is defensive against store drift and is not exercised by any production path today, and the test should be labelled as covering the mechanism rather than a reachable production shape. ### R2 — MINOR (latent bug, unreachable today by R1). The late-bind sentinel `0` is filtered as if it were a wire-named incarnation. `src/AcDream.Runtime/Entities/ParentAttachmentState.cs:828-834` (`EndGeneration`) and `:853-857` (`DeleteGeneration`), via `FilterParentCandidates` (`:1002-1018`). `DeferCreateObjectRelation` (`:412-426`) queues the relation into `_unresolvedByChild` carrying `ParentInstanceSequence: 0` plus `LateBindParentInstance = true`. The `0` is a **meaningless placeholder** — that is the whole premise of the fix. But `EndGeneration`'s retain predicate treats it as a wire-named value: ``` relation => relation.ParentInstanceSequence == replacementGeneration || PhysicsTimestampGate.IsNewer(replacementGeneration, relation.ParentInstanceSequence) ``` `IsNewer(oldStamp, newStamp)` (`AcDream.Core/Physics/PhysicsTimestampGate.cs:58-63`) returns true when `newStamp` is newer. For a player parent replaced at generation 7: `0 == 7` false, `IsNewer(7, 0)` false → **the late-bind relation is dropped**, which is the exact opposite of its own semantic ("attach to whoever currently holds the GUID" — the replacement generation is precisely whom it should attach to) and of retail's GUID-keyed blob replay (`QueueBlobForObject`, AP-132's retail half: replay on GUID (re)creation with only an addressability check). Not reachable in production today because of R1, and **not a regression** — pre-fix a CreateObject relation went straight to `_stagedByChild`, which `EndGeneration` clears unconditionally via `RemoveParentReferences` (`:822-827`). Correct behaviour when the path becomes reachable: both `FilterParentCandidates` retain predicates should keep `relation.LateBindParentInstance` unconditionally, or the deferred relation should carry a nullable incarnation rather than a `0` sentinel that collides with a legitimate creature-parent value. ### R3 — MINOR (incorrect comment, two sites; the same wrong rationale twice). `tests/AcDream.App.Tests/World/LiveEntityHydrationControllerTests.cs:149-153` and `tests/AcDream.App.Tests/World/LiveEntityPresentationControllerTests.cs:101-105`. Both comments justify the fixture setup with: > …giving it a nonzero canonical `FullCellId` via D1/D2 (exercised > elsewhere) — impossible to reach before #319's fix for a player-class > parent. Two errors: 1. **"the relation never committed for a player-class parent at all" is false.** `HasCommittedParent` is keyed by the CHILD guid (`ParentAttachmentState.cs:571-572`: `_lastAcceptedByChild.ContainsKey(childGuid)`), and pre-fix `CommitProjection` succeeded under `(playerGuid, 0)` — the contract's own §0 says so verbatim ("`TryCommitParent` never validates the sequence; the attach succeeds silently"). A player-parented child **did** have a committed parent at HEAD. What was inert at HEAD is the child's zero `FullCellId`, which is the contract's actual §3.1/§3.2 argument. 2. **The nonzero cell in both fixtures does not come from D1/D2.** The presentation fixture materialises at `0x01010001u` (`LiveEntityPresentationControllerTests.cs:690`, seeded from the spawn's own `ServerPosition` at `:744`); the hydration fixture's record carries `Cell` from its own spawn. Neither test calls `CommitAcceptedParentCellless`. The tests themselves are **valid and their sabotage claims hold** — I verified independently that `ProjectionCellId => WorldEntity is not null ? FullCellId : …` (`LiveEntityRuntime.cs:387-389`) makes a committed child's candidate cell exactly `FullCellId`, so it is 0 at HEAD and the parent's cell post-fix; the gates are therefore behavior-preserving at HEAD, exactly as the contract argued. Only the stated rationale is wrong. Correct behaviour: cite the zero-`FullCellId` inertness, not a non-existent absence of a committed relation. ### R4 — MINOR (comment made partially false by this diff; the comment itself is untouched). `src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs:1529-1531` (D1): > The committed relation (not the guid alone) resolves the parent so a stale > or superseded incarnation can never re-cell the child. After F1 a CreateObject-carried relation is, by construction, always equal to the parent's live incarnation at commit (and the new tripwire at `ParentAttachmentState.cs:651-661` enforces it). For that producer the sentence now describes a protection that is vacuous rather than active. It remains true for the `ParentEvent` producer, whose incarnation the wire names (AP-132). The diff changed the semantics of a comment it did not edit — the exact class the review brief flags. Correct behaviour: qualify the sentence to name the ParentEvent producer. ### R5 — LOW (comment misdirection). `src/AcDream.App/World/LiveEntityPresentationController.cs:228` — "the same predicate the hydration gate above uses". The hydration gate is in a different file (`LiveEntityHydrationController.OnLandblockLoaded:561`), not "above". ### R6 — OBSERVATION, partly UNVERIFIABLE (flagged rather than guessed). The tripwire converts a silent skip into a production throw. `ParentAttachmentState.CommitProjection` (`:639-661`) now throws `InvalidOperationException` when the parent is addressable and the relation's incarnation disagrees. The contract explicitly authorised this ("throw or logged refusal … pinned outcome: never a silent success"), so it is in scope. I enumerated the false-positive surface as far as source allows: - Only the **Staged** branch of `PrepareAndTryRealize` (`EquippedChildRenderController.cs:954-960`) reaches it in the graphical host; the Recovery branch does not. - That branch is gated by `ValidateParentProjection` (`:986-996`), which requires a live `LiveEntityRecord` for the parent. - A staged relation is staged either at the parent's live incarnation (`AcceptLateBoundCreateObjectRelation:846-853`), or after `Resolve`'s equality check (`ParentAttachmentState.cs:484-498`), or by the late-bind adoption (`:473-483`). - A parent generation change removes every staged/recovery/committed reference to that parent (`EndGeneration:822-827`, `DeleteGeneration:847-852`). - In the headless host (`RuntimeLiveEntitySessionController.cs:410`) relations originate only from `Enqueue(ParentEvent.Parsed)` and pass `Resolve`'s equality gate, so the tripwire is a no-op there. **What I could NOT establish:** whether `_inbound._snapshots[parent]. InstanceSequence` (the tripwire's source, reached via `TryGetSnapshot`) can ever lead `_activeByGuid[parent].Incarnation` inside a re-CreateObject transaction — `AcceptCreate` writes `_snapshots[guid] = incoming` (`InboundPhysicsStateController.cs:77`) before `RegisterEntityCore` completes the record replacement, and the two stores are documented as "related but distinct" (`:1198-1210`). I found no call into `CommitProjection` inside that window, but I did not prove the absence exhaustively. Reporting as unverified. If the connected gate ever produces this exception, the message already names #319 and the two incarnations, which is the right diagnostic. ### R7 — OBSERVATION. The hydration gate is broader than its cited retail anchor. `LiveEntityHydrationController.cs:551-562` excludes a committed child from the **entire** candidate loop, so `ProjectExact(CreateSupersessionRecovery)` (`:595`) and `ProjectExact(SpatialRecovery)` (`:614`) are also refused for it, not only the legacy `RebucketLiveEntity → CommitRebucket` branch (`:607`). This is what the contract pinned ("the candidate loop excludes records with a committed parent", §3.1) and it is behavior-preserving at HEAD (verified: a committed child's `projectionCellId` is `FullCellId` = 0 today). But the retail anchor in the comment, `update_object`'s `parent != 0` early-out @0x00515D40, speaks to **physics re-placement**, not to render-projection recovery. The broader exclusion actually rests on route 7's P4 record (a committed child is never a spatial root, joins no workset, has no shadow row) plus the fact that a child's projection is driven exclusively through `EquippedChildRenderController`'s realize/retry path. Worth one sentence in AP-142 clause (f) or the contract rather than a code change — as written the comment claims more anchoring than @0x00515D40 provides. --- ## 4. The three unwritten contract tests — NONE blocks - **§6 test 5 (residence non-event).** Not blocking. I verified §3.3's premise directly in source rather than relying on the test: `RuntimeEntityObjectLifetime.InitializeAcceptedCreateResidence:2750-2752` reads `if (canonical.FullCellId != 0u) Entities.SetFullCell(canonical, 0u, 0u);` immediately before `InitialCreateResidences.Begin`. `Begin`'s `FullCellId != 0` refusal is therefore a defensive invariant, not a reachable gate, exactly as §3.3 concluded. The zero-then-re-cell atomicity claim remains argued-not-measured, but no production change was made at that site. - **§6 test 6 (unwield classification population).** Not blocking. The contract itself recorded (from `af828a8a`) that ACE advances TELEPORT_TS at a drop and the classifier is a short-circuit OR (`RuntimeAuthoritativePositionRouteClassifier.cs:391`), so the observable label is `teleport-ts` either way. The test would pin a flavour, not a behaviour the fix changes. - **§6 test 9 (ledger convergence).** Not blocking, and I checked why: `RestoreShadow`'s refusal for a committed child is byte-identical pre- and post-fix (the `FullCellId == 0` clause already refused every committed child at HEAD). The one ledger asymmetry that exists — `_suspendedShadowOwners.Add` on `BecameHidden` (`LiveEntityPresentationController.cs:172`) with no matching removal on `BecameVisible` because `RestoreShadow` returns false — is **pre-existing and unchanged**, and converges through `Forget`/`Clear` (`:117`, `:125`). The connected §7 gate (both halves, player class mandatory) remains the real acceptance test and has not run. That is correctly recorded in ISSUES.md's #319 status line, which honestly says "FIX IMPLEMENTED, awaiting the connected acceptance gate … NOT YET COMMITTED". --- ## 5. Register and bookkeeping — verified row by row - **AP-146 (new row, `retail-divergence-register.md:174`).** Retail anchor `CPhysicsObj::SetPositionInternal` @0x00515330 **verified** — the address resolves to the `(CPhysicsObj*, CTransition const*)` overload (the 4-arg `SetPositionInternal` at @0x00515BD0 is a different function; the row cites the right one), and the cell write is unconditional across both branches (`m_position.objcell_id = …` @0x00515385 same-cell, `change_cell` @0x00515372 cross-cell). The three acdream writers, the `LocalPlayerProjectionController` landblock coarsening, and the `LiveEntityRuntime.cs:935-938` preserve are all cited. The risk column names the unresolved `ParkCollisionResidents` question **openly** rather than smoothing it. Register rule 1 is satisfied: this was a standing undocumented divergence and the diff files it. Header count 102 → 103, correct. - **AP-142 clause (f).** Every retail address in the clause (@0x00558a18/@0x00558a2d/@0x00558a3e, @0x00509370/@0x005093e6) was re-verified above and is exact. The clause states the acdream side honestly (the `(guid, incarnation)` key is acdream's, retail has none) and names the tripwire. Correct. - **AP-132 amendment.** The added sentence distinguishes the two producers correctly and does not weaken the row's own gate. Verified against the wire types: `ParentEvent.Parsed` carries `ParentInstanceSequence`; `CreateParentUpdate` carries `ChildInstanceSequence`. Correct. - **Route-7 contract §7 supersession blockquote.** Accurate — the old criterion is genuinely unfalsifiable under #319 (a zero-cell child emits no probe line, which the old criterion read as clean) and it points at the corrected positive-equality criterion. Correct. - **#320.** A faithful transcription of contract §2.2's five-item enumeration plus §9 item 2 as the mandatory first verification step, with the correct "do not implement without a fresh retail-conformance argument" gate. It inherits exactly what #319 deliberately excluded — nothing more, nothing less. --- ## 6. Errors in the CONTRACT itself Two, neither material to the verdict: 1. **§1 row 5 understates its own anchor** (see §1 of this review). `SetPositionInternal` @0x00515330 also walks `this->children` writing each child's `objcell_id` @0x005153AE–@0x005153D8 — retail's per-tick D2 equivalent in the same function. This *strengthens* the equality invariant and AP-146; citing only the mover's own write leaves the strongest available evidence on the table. 2. **§4 F4's "ISSUES.md: #319 → Recently closed with the commit SHA" was not executed** — correctly, since the connected gate has not run and the tree is uncommitted. The contract's instruction was written assuming the fix commit and the gate land together. Deviation is honest and recorded in the issue's own status line. Everything else in the contract that this review touched — §0's defect chain, §1's five rows, §2.1's four-consumer enumeration, §3.1/§3.2's wake mechanics, §3.3's residence non-event, §4 F1's pinned constraints, §5's invariants — was confirmed against source or pseudo-C.