acdream/docs/research/2026-08-04-c4-route-5-architecture-review-round2.md

C4 route 5 — architecture / adversarial DELTA review, round 2 (2026-08-04)

Verdict: FAIL. Three findings: two MAJOR, one MAJOR-coverage. Both MAJORs are narrow and cheap to close (one expression, one call, one assertion, one test), and one of them (B2) is a new defect introduced by the fix round, in the same class as the finding it was fixing.

Round 1 raised 5 MAJOR + 6 MINOR. Eight are closed well, one is closed for the teleport half only (B1), one is closed but with a regression attached (B2), one is declined acceptably (A11). Detailed disposition in §3.

Scope: the uncommitted working tree, 1,626 insertions / 398 deletions across 10 files, against HEAD 30d3d114. Independent gates run for this review: Release build green (0 warnings / 0 errors); focused Runtime suites 59/59; focused App suites (ProjectileControllerTests + LiveEntityNetworkOnPositionCollapseMatrixTests) 64/64. A green suite is not evidence — every claim below is traced to source.

---

1. New MAJOR findings

B1 — A3's fix covers only the teleport branch; the adopted-body FAR snap still leaves the interpolation queue armed, and AP-141 now asserts the opposite as retail-faithful

Severity: MAJOR (half-closed fix + a register row asserting behaviour the code does not have — the defect class the contract lists by name).

LiveEntityNetworkUpdateController.cs:2151-2159 runs the six-action hook only on SetPosition:

if (route.Disposition
        is RuntimeAuthoritativePositionDisposition.SetPosition
    && acceptedPositionCanonical.RemoteMotion is RemoteMotion adoptedRemote)
{
    RunRemoteTeleportHook(...);
}

Retail's far branch is not covered by teleport_hook — it has its own StopInterpolating, guarded on position_manager != 0 (@0x005163C9-@0x005163CB), and the remote far arm ports it exactly (ApplyAcceptedRemoteFarSnap: if (route.StopInterpolating) remote.Interp.Clear();). The projectile far arm ports nothing.

For the adopted-body shape — the one A3/R2 established as real and which this round now has a passing test for (MissileAdoptedBody_TeleportCommit_UnConstrainsAndClearsInterpQueue) — the missile does have a PositionManager and a populated Interp queue, so retail's guard passes and retail would clear it.

Concrete scenario: an ordinary remote with a live Interp queue gets the Missile bit from a State packet; TryBind's shared-body branch adopts the same body; the next accepted Position classifies far (>=96 m), not teleport. The projectile arm places the body at the destination and returns. The surviving RemoteMotion is still in _spatialRemotes with a stale waypoint, and the remote stepper drags the freshly-placed missile back toward it — the exact symptom A3 described, on the branch the fix did not reach. The MissileAdoptedBody_… test uses teleportSequence: 5 (teleport); there is no adopted-body far test.

Compounding this, AP-141 (docs/architecture/retail-divergence-register.md) now asserts the false justification unconditionally:

"The far branch's StopInterpolating skip is NOT part of this divergence — retail's own position_manager != 0 guard @0x005163C9 already skips it for a never-interpolated missile, so acdream's identical skip is retail-faithful by consequence."

That justification is conditioned on "a never-interpolated missile", which the adopted-body case is not. The row therefore claims retail-faithfulness for a shape where the code is not retail-faithful. This is the recurring class the route-5 contract names in its predecessor list ("a register row asserting behaviour the code does not have") and which AP-136 was previously amended for.

Fix direction: on the SetPositionSimple arm, when record.RemoteMotion is RemoteMotion adopted && route.StopInterpolating, clear adopted.Interp before the placement — mirroring ApplyAcceptedRemoteFarSnap's one line and retail's own ordering (@0x005163CB before @0x005163D9). Then narrow AP-141's far-branch sentence to the bare-missile case it is actually true of. Add the adopted-body far test alongside the teleport one.

---

B2 — R6 traded A1's wrong-cell symptom for a new one: on every STORED outcome the render entity is now positioned at the destination but parented to the cell it left

Severity: MAJOR (regression introduced by this fix round).

ProjectileController.SyncPresentationFromResolvedBody:530 changed from record.FullCellId to:

entity.ParentCellId = runtime.Body.CellPosition.ObjCellId;

Trace the three outcome classes:

outcome	body.Position after	body.CellPosition.ObjCellId	record.FullCellId
Committed	resolved destination	result cell (SnapToCell)	result cell (CommitCanonical's SetFullCell)
Stored (Refused/Contention/RejectedPreparation/NotApplicable)	accepted destination, resolved in the WIRE cell's frame	source cell — StoreAcceptedDestinationPose never writes the cell	wire cell — the merge's RefreshDerivedState → SetFullCell
Deferred/RejectedByPlacement	ack does not run (A1 gate)	—	—

On a committed outcome the two sources are identical, so R6 is a no-op there — and MissileTeleportCommit_… asserts both, confirming the equality rather than discriminating between the sources.

On a stored outcome they diverge, and R6 picked the wrong one. StoreAcceptedDestinationPose:1123-1132 computes accepted.PositionX + worldOffset(accepted.LandblockId) — the world point of the wire cell-local coordinates — and record.FullCellId is that same wire cell. So record.FullCellId is the cell the stored position belongs to; body.CellPosition.ObjCellId is the cell the body left.

The inconsistency is visible inside this fix round: on the same stored packet, Runtime's own SyncProjectilePresentation:1043-1051 publishes the shadow row with record.FullCellId (wire cell) paired with the new position, while App publishes the render entity with the source cell paired with the same new position. Two presentation surfaces, two different cells, one body.

Concrete failure: a missile crosses a landblock boundary at the streaming edge; the destination landblock is not yet in the collision service window, so CanAttemptDestination refuses. The body stores to the destination. The render entity is placed there but entity.ParentCellId is the source cell — and ParentCellId is precisely what RetailPViewRenderer.cs:925/945 uses for the indoor/outdoor stage split and viewcone.SphereVisibleInCell(e.ParentCellId!.Value, …). The arrow is visibility-tested against a cell it is no longer in: culled, or drawn in the wrong stage.

The R6 doc comment's own retail citation argues against its choice — it states that retail's store_position @0x00515CE2 "writes the object's whole Position including objcell_id". If retail's store wrote the cell, the body's cell after a store would be the wire cell; record.FullCellId is the faithful stand-in for that, and the body's stale cell is the acdream residual (AP-138), not the truth.

Fix direction: revert ParentCellId to record.FullCellId (round 1's value) and keep the A1 status gate, which is what actually fixed the no-op case. That pairing is correct on all three outcome classes and agrees with Runtime's own shadow publication. If the body's stale cell is considered the truth, then StoreAcceptedDestinationPose must write the cell too — but that is AP-138 scope and would change the remote arms as well.

---

B3 — the one branch where SyncProjectilePresentation is the sole writer — spatial+visible on the STORE path — is still unasserted at both layers, and it is exactly where B2 and the surviving A1 sabotage hide

Severity: MAJOR (coverage; one assertion + one test wide).

I independently reproduced the implementer's "2 of 5" trace and confirm it:

new test	discriminates a gutted SyncProjectilePresentation?	why
TeleportCommit_…ForceEndsCollision… (shadow assertion)	No	the shared pipeline's ShadowObjects.CommitSetPosition publishes the same row
FarCommit_… (shadow assertion)	No	same
TeleportCommit_ReenteringWorldReactivatesBody	No	RuntimeSetPositionState.cs:2983-2985 sets EnteringWorldFromCelllessResidence |= !body.InWorld || FullCellId == 0 at prepare time (before SnapToCell), and :4979-4984 then sets Active + LastUpdateTime itself
TeleportCommit_HiddenSuspendsShadowStaysInWorld	Yes	nothing else suspends on Hidden
Refused_NonSpatialDeactivatesAndSuspends	Yes	the Refused path never reaches the engine

The implementer's trace is accurate and was reported honestly. But the table also shows why only 2 discriminate: on the committed path the shared pipeline independently produces the same observable, so SyncProjectilePresentation's spatial+visible branch is only load-bearing on the store path — and no test asserts it there. Refused_StillAdvancesPoseNoParkPredictionInvalidated is spatial+visible+Refused and asserts body.Position, prediction, body.InWorld (vacuous — AttachBody's SnapToCell already set it) and ObjectClock.IsActive, but not the shadow row.

The same hole exists at the App layer: the collapse matrix now has teleport commit, far commit, near no-op, airborne no-op, null classification, unbound fall-through, and adopted-body teleport — but no store/refused scenario. That is why the reported A1 sabotage survived: with R6 reading the body's own cell, an unconditional ack on a no-op outcome writes nothing observable (position and cell both unchanged), so the gate is invisible on the no-op path. It is visible on the store path — which is untested.

Both gaps are cheap:

• Runtime: add Assert.Equal(body.Position, shadowEntry.Position) (using the file's existing AllEntriesForDebug pattern) to Refused_StillAdvancesPoseNoParkPredictionInvalidated. Gutting the sync then leaves the shadow at the spawn pose and the test fails.
• App: add a MissileFarRefused_… scenario to the collapse matrix. The fixture already supports it with no new machinery — PublishDestinationCollision() and ServiceWindow.Allow(DestinationLandblock) are separate opt-in calls, so simply omitting Allow yields Refused from CanAttemptDestination. Assert entity.Position == body.Position and entity.ParentCellId against the wire cell. That single test catches B2 and gives the A1 gate a discriminating home.

---

2. New MINOR findings

B4 — the App hook call allocates a per-packet closure, contradicting its own #315-pattern claim

LiveEntityNetworkUpdateController.cs:2155-2158 passes () => _liveEntities.IsCurrentPositionAuthority(acceptedPositionRecord, acceptedPositionAuthorityVersion) — a fresh display class + delegate on every teleport-classified adopted-body missile packet. The comment immediately above claims it uses "the SAME ordered hook seam and per-packet currency check the remote teleport arm already uses (RunRemoteTeleportHook, #315 pattern)". The remote arm's #315 pattern is precisely the opposite: _remoteArmCallbacks cached delegates over scratch fields (:1474-1492), introduced by the collapse's second commit to remove per-packet closures from this exact method. D-P4 also states the wiring must be "without per-packet closures". Narrow reach (SetPosition + RemoteMotion present), so MINOR — but the comment asserts compliance the code does not have.

B5 — the retained-retry arm invalidates prediction on an outcome that writes nothing

RuntimeRemotePlacementDriveController.Advance:1265-1268: the else branch calls pendingProjectile?.InvalidatePrediction() and then SubmitAndResolve. When SubmitAndResolve returns Contention (retryable → re-parked into _pending), Advance performs no StoreAcceptedDestinationPose — so nothing was written, yet the prediction version advanced. Contract invariant 4 pins "prediction invalidation accompanies every body write on this route… The no-op dispositions invalidate nothing." Effect is one aborted in-flight quantum per re-park; small, but it is the invariant's stated shape violated on one arm only. The entry point does not have this problem (Contention there always stores).

---

3. Round-1 findings — disposition

#	status	verification
A1 (unconditional ack)	CLOSED, gate correct — but see B2	:2170-2179 gates on placementStatus is not null and not Deferred and not RejectedByPlacement, an exact set-equality mirror of the Runtime gate (:975-983 plus the null returns). Verified by enumerating every RuntimeRemotePlacementExecutionStatus producer.
A2 (Missile bit without a bound projectile)	CLOSED, well	The discriminator became conjunctive at the classifier (RuntimeEntityObjectLifetime.cs:673-677) with the App's null-route fallback mirroring it exactly (:2122-2131). Retail justification is sound and better than my suggested fix: retail places every non-player object unconditionally, so an unbindable missile taking the ordinary remote path is the faithful behaviour, not a fallback. Blast radius enumerated below. MissileUnbound_FallsThroughToRemoteTail_TracksInsteadOfFreezing is discriminating (revert the conjunct and the body never moves).
A3 (teleport hook)	PARTIALLY closed — see B1	Teleport branch wired through the existing RemoteTeleportHook bundle with a real per-packet currency guard; the adopted-body test is genuinely discriminating (removing the call leaves Constraint.IsConstrained true and Interp.IsActive true). Far branch not covered.
A4 (sync untested)	PARTIALLY closed — see B3	2 of 5 new tests discriminate; the store-path spatial+visible branch remains unasserted.
A5 (no App missile coverage)	CLOSED, well	7 tests on the shared collapse fixture. Fixture-regression check below.
A6 (wasInWorld read after placement)	CLOSED (behaviourally inert, correctly so)	wasInWorld is now captured before dispatch at both call sites (entry point :942, retry :1226) and threaded as a parameter. Note it has no observable effect: on commit, EnteringWorldFromCelllessResidence already re-activates from the same pre-SnapToCell condition; on store, body.InWorld is unchanged so before == after. Correct and trap-removing either way.
A7 (hidden LastUpdateTime)	CLOSED	Restored at :1055. Clock basis verified equivalent: UpdateFrameOrchestrator.CurrentScriptTime => _runtime.SimulationTimeSeconds, i.e. _physicsScriptGameTime and _clock.SimulationTimeSeconds are the same clock, so no mixed-basis write into body.LastUpdateTime.
A8 (silent shadow skip)	CLOSED	else { physics.ThrowIfWorldFrameUnreachable(record.FullCellId); } at :1053. Verified non-throwing during the legitimate pre-Create window (RuntimePhysicsState.cs:596-606 returns early unless the local-player Create was observed with a zero frame).
A9 (local player not fenced)	CLOSED	update.Guid != _playerServerGuid && added to the fallback (:2125).
A10 (OwnsFarSnap doc)	CLOSED	Doc-only change; the predicate body is byte-identical (verified — the diff adds only comment lines). No effect on the far arm the collapse and 4b-2 gated.
A11 (stale cutover inventory)	DECLINED — acceptable	docs/research/2026-08-02-cutover-route-inventory.md is a dated research/planning record, and the project's documentation rules treat those as historical. D-P7's grep was scoped to src/ and docs/architecture/, both of which are clean. Reasonable call; the risk is a future reader treating a dated inventory as current, which the date already signals.
_lastFiniteGameTime	CLOSED	SyncPresentationFromResolvedBody:521-522 restores the rebase under double.IsFinite, matching the deleted method's semantics (it also skipped the assignment on a non-finite clock).

A2's fix — blast radius, enumerated

Everything that now depends on the conjunctive kind derivation, and its status:

1. RuntimeAcceptedPositionRouteRequests.TryBuild → RuntimeAuthoritativePositionRouteClassifier — re-verified that the classifier's only kind branch past ValidEntityKind is LocalPlayer, so Projectile and Remote remain disposition-, flag-, StopInterpolating- and ConstrainPhase-identical. Kind changes the OperationKind only.
2. RuntimeRemotePlacementDriveController.OwnsPlacement — admits both; single production reader (TryExecuteAcceptedRemotePosition:640).
3. RuntimeRemoteFarSnapPosition.OwnsFarSnap / RuntimeRemoteTeleportPosition.OwnsTeleportPlacement / ApplyAcceptedRemoteFarSnap / ApplyAcceptedRemoteTeleport / TryArmConstraintAfterOperation — still RemoteAuthoritative-gated. An unbound missile now legitimately reaches them (it is classified Remote), which is the intended pre-route-5 behaviour; a bound one never can. The throwing guards stay unreachable for projectiles.
4. App isMissilePacket — derived from route.OperationKind whenever a route exists, so it cannot disagree with the classifier; the null-route arm restates the identical conjunct.
5. Kind is now derived from mutable binding state, so it can flip between packets without the Missile bit changing. The one place that stores a kind across packets is _pending[key].Route; Advance re-validates pending.Record.Projectile and body identity before syncing (:1214-1224), and TryPrepareAndSubmitAuthoredPlacement compares route.OperationKind against the operation.Kind stamped at the same Begin. Consistent — this was handled deliberately (R3), not by luck.
6. First-entry admission, the residence Create half, and the continuation executor all pass explicit kinds and are untouched.

I found no dependent that broke.

A5's fix — did the shared fixture regress for its original dual-guid purpose?

No. Verified line by line:

• The remote construction is moved verbatim into an else branch; the RemoteMotion + Shadows.Register(… Cylinder, cylHeight: 1.835f …) block is byte-identical to the original.
• baseState evaluates to exactly PhysicsStateFlags.ReportCollisions when isMissile: false, so both RawState and PhysicsState are unchanged for every pre-existing test (all of which use the default isMissile: false).
• Remote stays null! for missile fixtures; no missile test dereferences it. The two tests that do use fixture.Remote (MissileUnbound_…, MissileAdoptedBody_…) both construct with isMissile: false — deliberately, because both scenarios are remote-shaped records that acquire the Missile bit later. That is the right modelling.
• The new Projectile property is additive.
• All pre-existing dual-guid tests in the file pass in the 64/64 run.

MissileAdoptedBody_… is worth calling out as a genuinely good test: it builds the adopted-body shape through the production seam (BindProjectile over the record's canonical body, which SetRemoteMotion had already adopted from the component), and its two discriminating assertions (Constraint.IsConstrained, Interp.IsActive) fail if the hook call is removed.

---

4. The two reported residuals — judgment

Residual 1 — A1's fix has no App-level regression test

Judgment: the stated gap is acceptable; the surrounding gap is not.

The narrow claim is correct and I verified it: Deferred requires a DeferredCell park (collision-generation quiescence) and RejectedByPlacement requires the engine's own sweep to refuse, and no sibling remote test in that file constructs either — the fixture has no machinery for it. The gate is a literal set-equality mirror of a Runtime gate whose behaviour is exercised by Refused/no-op/rejected tests, so code review is a defensible verification for those two statuses specifically.

But the reason the sabotage survived is not that Deferred is unreachable — it is that R6 made the ack idempotent on the no-op path, so the gate has no observable effect on any scenario the matrix currently contains. The status the gate matters for that is trivially constructible is the store path, and the fixture already supports it (omit ServiceWindow.Allow). That test is required (B3), and it also catches B2. So: accept the Deferred/RejectedByPlacement carve-out, reject the absence of any store-path App scenario.

Residual 2 — only 2 of 5 A4 tests discriminate

Judgment: 2 is not sufficient for invariant 2, but the shortfall is one assertion, not a suite.

The trace is accurate (I reproduced all five verdicts independently, including the non-obvious EnteringWorldFromCelllessResidence mechanism behind the reactivation test). Keeping the redundant assertions as correct facts is the right call — they are true, cheap, and they pin the shared pipeline's contribution.

The shortfall is specific: the spatial+visible branch is redundant only on the committed path. On the store path SyncProjectilePresentation is the sole writer of the shadow row, and no test asserts it. Invariant 2 is "#312's layer" — an entity left rendered a packet behind its body — and the store path is exactly the outcome class where that can happen without the shared pipeline noticing. One Assert.Equal(body.Position, shadowEntry.Position) in the existing Refused_… test converts the third branch from redundant to discriminating and closes invariant 2 at the Runtime layer. Combined with the App store scenario from B3, invariant 2 is then covered at both layers by discriminating assertions.

---

5. What would make this PASS

1. B2: entity.ParentCellId = record.FullCellId (keep the A1 gate). One expression.
2. B1: clear the adopted Interp queue on the SetPositionSimple arm when route.StopInterpolating; narrow AP-141's far-branch sentence to the bare-missile case. One call + one clause + one test.
3. B3: one shadow assertion in Refused_StillAdvancesPoseNoParkPredictionInvalidated; one MissileFarRefused_… App scenario asserting position and ParentCellId (which is the B2 regression test).
4. B4/B5: cached delegate for the hook currency check; move the retry arm's InvalidatePrediction to the paths that actually write.

Nothing here needs new machinery, a new fixture, or a design change.