acdream/docs/research/2026-08-04-c4-route-4b-3-architecture-review.md

C4 route 4b-3 — architecture / adversarial review (2026-08-04)

Reviewer lens: adversary. Retail conformance is a separate reviewer's scope; this review asks only "does this break something, leak something, deadlock something, or leave an entity in a bad state?"

Subject: the uncommitted working tree on claude/acdream-physics-divergence-5aa784 at base 3e002993 (git diff HEAD plus the three untracked non-contract files). Contract: 2026-08-04-c4-route-4b-3-contract.md.

Verdict: FAIL

Two MAJOR findings. Both are behaviour REGRESSIONS against 3e002993 — not pre-existing residuals — and both sit on code paths the contract's own invariant list names as load-bearing (invariant 5, the arm-count partition; invariant 6 / D3, "the hook's whole point is that no locomotion state survives a teleport"). Both are invisible to the specified two-client connected gate, because that gate teleports a player character and both defects are on the NPC/creature arm.

Everything else I checked held. Build is green (0 warnings); focused suites pass Runtime 1,125 / 0 skips and App 4,078 / 3 skips at Release with --no-build.

---

What I verified and found sound (do not churn)

• Invariant 1 (StoresAcceptedDestination). ApplyAcceptedRemoteTeleport (src/AcDream.Runtime/Session/RuntimeRemotePlacementDriveController.cs:820-848) is byte-for-byte the far arm's shape minus the StopInterpolating clear, including the currency-guarded StoreAcceptedDestinationPose. The partition is extended, not re-litigated.
• Invariant 3 (no stranded entity). The teleport arm reaches SubmitAndResolve through the same TryExecuteAcceptedRemotePosition the far arm uses; the DeferredCell arm still cancels synchronously with restoreCancelledPark: true. No new park shape, no new withdrawal site.
• Invariant 10. TickLostCellDeadlines / TryDequeueExpiredLostCell / ArmLostFamilyDeadlines still have zero production callers (4 references, all inside RuntimeSetPositionState.cs; the rest are tests). The reaper stays inert.
• Invariant 11 (route 1 unchanged). The shared builder got an overload, not a third copy, and the route-1 overload forwards committedCellId: canonical.FullCellId — its pre-slice value verbatim (RuntimeAcceptedPositionRouteRequests.cs:41-56).
• D1's plumbing is honest. PreMergeCommittedCellId = hadCanonical ? beforeCell : null is measured before RefreshSnapshot (RuntimeEntityObjectLifetime.cs:1725-1770), never re-read, and null ("no prior canonical record") makes TryBuild decline rather than fabricate a 0. RuntimeRemoteTeleportClassificationTests genuinely discriminates the fix from the shipped dead predicate — the companion test fails under the post-merge read. This is the strongest test in the diff.
• The _activePlacementOwners deletion is legitimate — see item 1 below.
• Shadow suspend/restore across the hook. RemoteTeleportHook's ReportCollisionEnd → ShadowObjects.Suspend removes the entity from _entityToCells; the arm tails' LiveEntityShadowPublisher.TryPublishRemote → ShadowPositionSynchronizer.Sync → RefreshPositionRows un-suspends it (ShadowObjectRegistry.cs:1336-1393). I specifically checked for a pose-gate that could skip the restore on a zero-distance teleport: the OnPosition-tail overload is ungated (RuntimeRemotePhysicsUpdater.cs:1175-1191); only the per-tick loop gates. TeleportRefused_…'s Assert.Single(fixture.Shadows.AllEntriesForDebug(), …) is a real proof of this, because AllEntriesForDebug enumerates _entityToCells. No invisible-and-intangible entity on this path.
• Deletion completeness. All eight wiring sites in the contract's inventory are cut, including the two the handoff missed (SessionPlayerComposition, LiveSessionResetManifest). Reset coverage is genuinely preserved: GraphicalSessionEventRoute.cs:152 calls _remotePlacementDrive.DetachRoute.
• Register bookkeeping. AD-42, AP-136, AP-137, AP-138 all updated in the diff; AP-138's Risk column does gain the teleport arm as a second producer, and AP-137 is rewritten rather than deleted. (One factual defect in the AP-137 rewrite — see A7.)

---

MAJOR findings

A1 (MAJOR) — the NPC arm silently stops arming ConstrainTo for every airborne-body packet; the code documents the opposite

File: src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs:1290-1311 (ToConstraintArm), consumed at :2643.

ToConstraintArm maps RemoteContactArm.AirborneSnap through its _ arm to RuntimeRemoteAcceptedPositionArm.AirborneNoOperation — the one value TryArmConstraintAfterOperation never arms — and justifies it at :1296-1297 with:

"There is no Runtime analogue of AirborneSnap (a caller reaching it always returns before any arming call) … production never calls this with that arm."

That claim is false for the NPC arm. After npcRouting = ApplyRemoteContactRouting(…) at :2600, the only early return is the currency guard at :2619-2626, which is gated on FarSnapPlacement or TeleportPlacement. Control then falls out of the if (!snapSuppressedByStick || isTeleportRoute) block straight into TryArmConstraintAfterOperation(ToConstraintArm(npcRouting.Arm), rmState) at :2641-2644. AirborneSnap is returned by ApplyRemoteContactRouting's free-flight carve-out at :1123-1150 for any non-teleport classification whenever !remote.Body.InContact.

Concrete failure scenario. A creature remote (guid not 0x50xxxxxx) is knocked off a ledge or jumps, so its canonical body has no contact plane (Body.InContact == false). ACE broadcasts a wire-grounded UpdatePosition (IsGrounded: true) at 5-10 Hz. The classifier returns Interpolate (playerDistance < 96 m). Routing takes the airborne carve-out → AirborneSnap → ToConstraintArm → AirborneNoOperation → no arm.

At 3e002993 the same packet armed: the post-operation site was passed the ROUTE (earlyRemoteRoute), and OwnsAfterOperationConstraint(Interpolate) was true with ConstrainAfterRouting true. So the arm count for this row goes 1 → 0, which is the "never zero times" half of contract invariant 5 and a yes row of D4's partition table turning into a no. Retail's near branch returns 1 @0x005163BE, so HandleReceivedPosition arms unconditionally @0x00454272 — the divergence is in the wrong direction. The same 1→0 applies to a far (SetPositionSimple) classification and to a null/Rejected* classification whenever the NPC's body is out of contact.

Observable consequence: a creature that is knocked airborne on the first accepted Position after spawn never sets IsConstrained at all, so the ConstraintManager brake and IsFullyConstrained() (which gates jump_is_allowed) stay dead for it; a creature already armed loses the per-packet re-anchor (ConstraintPosOffset reset to 0) for the whole airborne interval.

Why no test caught it. Proof obligation 3 asked for "a test that counts arming calls per packet across the partition table's rows". What was delivered (RuntimeRemoteSteadyStatePositionTests.TryArmConstraintAfterOperation_MatchesTheCompletePartition) tests the Runtime predicate given an already-chosen arm value. Nothing tests ToConstraintArm, and nothing counts arms per packet through OnPosition. The mapping — the only new code in the arming path — is 100 % uncovered.

Fix direction. AirborneSnap is not AirborneNoOperation: retail's airborne no-op is arg4 == 0 (the WIRE contact bit, return 0), while AirborneSnap is acdream's carve-out keyed on the BODY's contact for a packet whose wire bit said grounded — retail returns nonzero for that packet and arms. Either map AirborneSnap to an arming value, or (better) make ToConstraintArm total with an explicit AirborneSnap case plus a _ => throw, and add the missing end-to-end arm-count test across the D4 rows including the body-airborne ones.

---

A2 (MAJOR) — a teleported NPC gets a ~1,000 m/s synthesized ServerVelocity and an animation cycle planned from it

File: src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs:2526-2543 (synthesis) and :2683-2709 (consumption).

The NPC arm synthesizes a locomotion velocity for any packet that carries no wire velocity:

serverVelocity = (worldPos - rmState.LastServerPos) / (float)elapsed;   // :2532
… rmState.ServerVelocity = authoritativeVelocity; HasServerVelocity = true;

rmState.LastServerPos is only advanced at :2681, i.e. after routing, so at :2532 it still holds the pre-teleport position. For an admin teleport the delta is the whole teleport distance over one packet interval — e.g. 192 m / 0.15 s ≈ 1,280 m/s. The tail then calls RemoteServerControlledVelocityCycle.Apply(update.Guid, ae, rmState, rmState.ServerVelocity) at :2704-2709, which runs ServerControlledLocomotion.PlanFromVelocity and Sequencer.SetCycle(style, plan.Motion, plan.SpeedMod).

At 3e002993 this was unreachable for a teleport: the remotePlacementRequired block sat in the SHARED section, above the IsPlayerGuid split and above the NPC synth-velocity code, and it always returned. The diff deleted that block and routed the teleport through the NPC tail instead, so the synthesis now runs on every NPC teleport.

Concrete failure scenario. @teleto a drudge 200 m away while an acdream client observes it. The teleport arm places it correctly, then the same packet installs ServerVelocity ≈ 1.3 km/s and calls SetCycle(Run, speedMod≈huge). RemoteServerControlledVelocityCycle.Apply's three guards do not help: the body is grounded after the commit (rm.Airborne == false), the guid is not a player guid, and rm.MoveTo was just set to Invalid by the teleport hook's own CancelMoveTo — so the hook actively removes the last thing that would have suppressed this. The creature stands at the destination playing a run/sprint cycle at an absurd speed multiplier until the stale-velocity watchdog fires (ServerControlledVelocityStaleSeconds = 0.60) or the next packet's small delta replaces it.

This is directly contrary to teleport_hook's purpose (@0x00514ED0 exists so that no locomotion state survives a teleport) and is exactly the "stands with correct animation" clause of the contract's live gate — which cannot see it, because the gate teleports a player character and RemoteServerControlledVelocityCycle.Apply early-returns for 0x50xxxxxx guids. The player-remote arm is accidentally immune because its own teleport block returns at :2185, before the player synth-velocity at :2432-2443.

Fix direction. The teleport arm must suppress the synthesized velocity for its own packet on the NPC arm too — either by advancing rmState.LastServerPos to the destination before the synthesis, or (cleaner, and symmetric with the player arm) by giving the NPC teleport its own tail that skips the synth/cycle step. Whichever is chosen, add a test that asserts the NPC sequencer's cycle is unchanged across a teleport packet; today nothing in the tree looks at the animation layer for this arm.

---

MINOR findings

A3 (MINOR) — the stress test's teleport step now drives zero production code

File: tests/AcDream.App.Tests/World/LiveEntityLifecycleStressTests.cs:710-730.

The contract's deletion inventory says LiveEntityLifecycleStressTests "constructs the controller and calls TryApply — its scenario must be re-expressed against the canonical teleport arm, not dropped." BeginDeferredTeleport is now four hand-written field assignments (_remote.Body.Position = destination; _remote.CellId = …; Entity.SetPosition(…); Entity.ParentCellId = …). It exercises no placement, no hook, no routing. RepeatedRetailRecallMotion_HiddenTeleportUnhide_… (:252-296) would pass identically if the entire teleport arm were deleted. The Hidden/DeferredShadowRestore half of the scenario still discriminates, so this is a coverage loss rather than a false pass — but the mid-teleport Hidden/UnHide interaction the fixture exists for is no longer tested against the mechanism that now performs the teleport.

A4 (MINOR) — TeleportRefused_… does not discriminate the teleport arm from UnroutedCatchUp

File: tests/AcDream.App.Tests/Physics/LiveEntityNetworkRemoteTeleportPresentationTests.cs:157-206.

Every assertion in this test (body.Position == destination + offset, Entity.Position == body.Position, IsSpatiallyVisible, one shadow entry at body.Position) is also satisfied if the packet had classified null and taken UnroutedCatchUp → ApplyInterpolate, because the body-to-target distance is >192 m so AP-87's bodyToTarget > 4 m branch hard-places the body at exactly the same wire pose and the same NPC tail then syncs entity and shadow. The test does catch removal of the store_position fallback, so it is not worthless, but the sibling commit test is the only one whose expected value (+ FootSphereCenterLift) can only come from a canonical placement. Adding one Assert.Equal(0u, …)-style discriminator (or asserting the hook ran, as the routing-seam tests do) would close it.

A5 (MINOR) — the landing block's new comment states something false

File: src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs:2266-2272.

"A landing packet always classifies Interpolate (the teleport/cell-less arm dispatches earlier and returns before this block can be reached — D5)."

The teleport half is true; "always classifies Interpolate" is not. The landing block is reached for a wire-grounded, body-out-of-contact packet with ANY non-teleport, non-airborne-no-op classification — including SetPositionSimple (>= 96 m), null (login window), RejectedAuthority, and RejectedData. The hard-coded RuntimeRemoteAcceptedPositionArm.NearInterpolate at :2273 still produces the correct arm count for all of those (every one of them is an "arms" row), so this is a comment defect, not a behaviour defect — but process rule 6 is explicit, and this is exactly the shape ("a comment asserting behaviour the code no longer has") that six consecutive slices have shipped.

A6 (MINOR) — new per-packet allocations on the remote hot path

File: src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs:2148-2153, :2370-2374, :2609-2613, and :1080-1088.

runTeleportHook: () => RunRemoteTeleportHook(update.Guid, entity.Id, () => IsCurrentPositionOwner(entity)) captures update, entity and this, so a display-class + delegate pair is allocated for every remote accepted Position, not only for teleports (the pre-slice tree allocated the inner lambda only when remoteHardTeleport was true). Separately, teleportStatus.ToString() at :1087 is evaluated at the call site, so the probe's own ProbeRemoteTeleportEnabled self-guard does not prevent the string allocation. Given Slice I's "0 B/resolve" discipline this is worth a Func<bool> cached per controller, or hoisting the probe behind if (PhysicsDiagnostics.ProbeRemoteTeleportEnabled) at the call site.

A7 (MINOR) — D2's "unifies the player and NPC arms" is not delivered, and AP-137 now claims it was

Files: src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs:2186-2210; docs/architecture/retail-divergence-register.md (AP-137 row, the "D2 — the wire-airborne leftover shape" sentence).

The retail return-0 shape is implemented only inside the IsPlayerGuid block. On the NPC arm a wire-airborne packet with a null/Rejected* classification still falls through the synth-velocity block into ApplyRemoteContactRouting, which either hard-snaps the body (AirborneSnap, if the body is also out of contact) or enqueues/places it (UnroutedCatchUp → ApplyInterpolate, if the body is in contact) and then syncs the render entity and publishes the collision shadow. That is not "AP-135's two bookkeeping writes only, no body/queue/render write, no leash arm, return". The register row's claim that this "unifies player and NPC remotes on one behaviour" is therefore factually wrong as shipped, which is a register-rule-1 problem in its own right. (This is NOT a regression against 3e002993 — the NPC arm behaved this way before — so the fix can legitimately be "scope the row to the player arm" rather than "implement the NPC half".)

A8 (MINOR) — dead local

src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs:1727: bool wasCellless = hadCanonical && beforeCell == 0u; has no remaining reader (only the comment at :1759 mentions it). No compiler warning fires because the initializer is not a constant. Delete it or the comment will outlive the variable's meaning.

A9 (MINOR) — proof obligation 1 is not stated anywhere in the tree

The contract requires the ParkCollisionResidents-overlap-throw unreachability argument (with 4b-1's B2 caveat about HasOldPrefixPlacementDebt being a stall rather than a throw) to be stated "in the contract-conformance section of the implementation commit". The work is uncommitted, so I cannot verify it; nothing in the diff carries the statement. The underlying property does hold by reading — the teleport arm adds packets to the same one-operation-per-key machinery and opens no new operation shape — but the statement is still owed.

A10 (MINOR, partly unverified) — the visibility-edge protection _activePlacementOwners provided has no replacement

I independently traced the deletion the implementer reported (item 1 of the task). Their conclusion is right, their stated reasoning is not. _activePlacementOwners was NOT write-never at 3e002993: the writer chain was LiveEntityNetworkUpdateController (remotePlacementRequired → _remoteTeleportController.BeginPlacement) → RemoteTeleportController.BeginPlacement → RemoteTeleportPlacementPresentation.Begin → LiveEntityPresentationController.BeginAuthoritativePlacement (RemoteShadowPlacementSynchronizer.cs:48 at HEAD). It becomes write-never because this slice deletes that chain, which is exactly what the contract's deletion-inventory row said. git grep over src + tests at HEAD confirms no other production writer. So deleting the set and its four IsPlacementActive gates changes nothing for any surviving path, and the four gates degenerating to "always act" is correct.

What is not replaced is the protection: during the old multi-frame pending placement, an intervening Hidden/UnHide edge was prevented from restoring a shadow at an unresolved pose. The new teleport is synchronous inside OnPosition, which mostly removes the window — but the canonical placement publishes its receipt synchronously and the projection sink can raise a visibility edge from inside it. I could not construct a reachable Hidden/UnHide-during-placement path in the new design and am flagging this as unverified, not as a defect.

---

Judgment on the three reported items

1. _activePlacementOwners deletion — sound, reasoning wrong. See A10. The set had a real production writer at HEAD; it is the slice's own deletions that kill it. The four consumers' degeneration is behaviour-neutral for every surviving path. Not a defect. The commit message should say "its only writer is deleted by this commit", not "the predicate was permanently false".

2. The two parallel OnPosition copies — a genuine structural hazard, and it has already produced a defect. This is not a stylistic complaint: A2 is precisely the two copies disagreeing. The player copy's teleport block returns at :2185 before its synth-velocity at :2432; the NPC copy has no teleport block at all and falls through its synth-velocity at :2526 on the way to routing. A1 is the second instance — the player copy cannot produce AirborneSnap (its landing block returns first) while the NPC copy can, so the shared ToConstraintArm was written against the player copy's reality and is wrong for the NPC one. The implementer's own report that "a sabotage of the wrong copy left both tests green" is the same signal. The duplication should be collapsed, or at minimum every arm tail should be one shared helper that both branches call; shipping a third slice against these two copies without that is how the next one of these lands.

3. Declining D7 (consolidating the post-placement currency guard into a Runtime seam) — acceptable, and not the source of either MAJOR. The guard is now duplicated at three App call sites (:2155-2161, :2393-2399, :2619-2626) instead of two. I checked all three: the predicate is character-identical (!IsCurrentPositionOwner(entity) || !ReferenceEquals(positionRecord.RemoteMotionRuntime, rmState)), and all three sit BEFORE their arming call, preserving the R5 invariant. The widened arm predicate (FarSnapPlacement or TeleportPlacement) is applied consistently at the two that need it; the teleport-specific site is unconditional, which is strictly stronger. So the duplication is not itself a correctness hazard today. It is, however, the same duplication-of-invariant pattern as item 2, and a fourth copy (route 5's projectile arm) is next in the queue — consolidating it is now overdue rather than optional.

---

Gate status observed

• dotnet build AcDream.slnx -c Release → succeeded, 0 warnings, 0 errors.
• dotnet test tests/AcDream.Runtime.Tests -c Release --no-build → 1,125 passed / 0 skipped / 0 failed.
• dotnet test tests/AcDream.App.Tests -c Release --no-build → 4,078 passed / 3 skipped / 0 failed.
• Complete-solution Release suite: not run by this reviewer (the contract's own gate; the new figure still has to be measured and recorded, not inherited from 11,027).
• Two-client connected teleport gate: not run, and note the finding above that neither MAJOR is observable through it as specified — A1 and A2 both need a creature teleport (@teleto a drudge/mosswart into view), not a second player character.