acdream/docs/research/2026-08-04-c4-route-4b-2-delta-review-findings.md
Erik 7f1c1f5aa6 feat(physics): C4 route 4b-2 — remote far snap through the canonical placement
Flips the SetPositionSimple classification (contact, PlayerDistance >= 96 m) for
remotes onto 4b-1's drive controller and deletes both legacy far blocks, both
duplicated 96f/4f constant pairs, and both `?? Vector3.Zero` fabrications. The
4 m constant now exists exactly once. Teleport and cell-less stay legacy for
4b-3.

Retail: MoveOrTeleport @0x00516330's far branch runs StopInterpolating
@0x005163CB before SetPositionSimple @0x005163D9 and returns 1 @0x005163E8
regardless — the SetPositionError is discarded — so HandleReceivedPosition arms
ConstrainTo @0x00454272 post-move on commit AND on failure. The x87 parity
decode at @0x00516393-@0x0051639E puts exactly 96.0 on the far branch.
SetPositionSimple @0x005162B0 builds flags 0x1012 at @0x005162C4.

Non-commit outcomes still advance the body, because retail's SetPositionInternal
@0x00515BD0 commits the destination via store_position @0x00515CE2 when no cell
resolves. The partition is by STAGE, not heuristic, enforced by an exhaustive
switch: Refused/Contention/NotApplicable/RejectedPreparation store (the placement
never executed); Committed/Deferred/RejectedByPlacement do not (the engine ran
and refused, matching retail's non-storing returns @0x00515CB2 and @0x00515CD5).
Without this a refused far snap froze the remote with an emptied queue.

Also fixes a shipped defect this route made live: ParkDeferred's quiescence parks
withdrew the entity (InWorld=false, clock suspended, residency removed) and were
never restorable, while Forget(restoreCancelledPark: true) runs for every
accepted Position on every entity. The restorable decision now lives inside
ParkDeferred AFTER SnapToCell, reading body.CellPosition.ObjCellId — the value
RestoreParkWithdrawal actually restores at — against every live quiescence
rather than one minimum-OperationId token. The three pre-snap fields are hoisted
into locals because SnapToCell ends with InWorld = true. ParkCollisionResidents
passes restorableOnCancel: false explicitly; the plain unplaceable park is
provably unchanged. RestoreParkWithdrawal re-tests the prefix at restore time so
a retained route-2 park cannot re-admit into a prefix that began quiescing
during the park.

CanAttemptDestination is retained as an OPTIMISATION only, with the two Core
predicates it cannot reproduce written down at the pre-flight, plus the two
properties that depend on it staying there.

Four fix rounds and eight Opus reviews. The slice was fully green at 10,990,
10,997 and 11,004 while containing real defects — a frozen remote pinned as
correct by its own test, a fallback that over-wrote on the exact retail paths
that decline to store, and a park guard incomplete on two independent axes.

Register: AP-137 (leftover classifications take AP-87's catch-up; states the
cell-less enqueue-vs-place delta deferred to 4b-3, that RejectedData is applied
anyway, and the headless divergence), AP-138 (the refusable far placement),
AP-136 narrowed to match the relocation. #309's acceptance steps rewritten —
step 5 previously asserted a recovery the code does not perform — and gated on a
new ACDREAM_PROBE_PARK=1 signal so the check cannot pass while broken.

Suite 11,009 passed / 4 skipped / 0 failed against a measured 10,968 baseline.
The 10,973 figure recorded earlier was wrong and is corrected here.

Connected gate outstanding: the two-client far-snap walk and #309.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-04 07:55:56 +02:00

14 KiB

C4 route 4b-2 — delta review FAIL (round 2 correction), 2026-08-04

Both delta Opus reviews returned FAIL on the fix round. Original slice is WIP dfd27896; the fix round is uncommitted on top of it.

The fix round did real work and most of it is verified correct — see "Do not churn" at the end. Two defects block, and one of them is worse than the defect the round was fixing.

MAJOR A — the store_position fallback fires on the two retail paths that decline to store

The round's load-bearing claim (RuntimeRemotePlacementDriveController.cs:549-553 and AP-138 part 1) is that retail's non-storing returns "are reached only after a cell resolved and the transition ran — states this controller's non-commit outcomes never represent, because they all mean the placement never executed."

The second clause is false for Rejected.

Retail, CPhysicsObj::SetPositionInternal @0x00515BD0:

  • CheckPositionInternal == 0 @0x00515C85 → handle_all_collisions @0x00515CC2 → return ((eax_14 - eax_14) & 2) + 2 @0x00515CD5 → 2 or 4. No store.
  • sphere_path.curr_cell == 0 @0x00515C8F → return 3 @0x00515CB2. No store.

acdream ports that enum literally — PhysicsSetPosition.cs:12-20 (NoValidPosition = 2, NoCell = 3, Collided = 4, doc-commented against acclient.h enum 491). The producing chain: PhysicsEngine.cs:1548-1575 / :1580-1584RuntimeSetPositionState.cs:3034-3039 (if (!result.IsSuccessful) return Outcome(Rejected, …)), which is after _physics.Engine.SetPosition(...) at :2983RuntimeRemotePlacementDriveController.cs:775-780 default:Rejected:578-579 stores.

So a far snap whose destination the engine's own sweep refuses — a remote server-snapped into geometry, or into a cell the sphere path cannot resolve — now teleports the canonical body into that refused destination. Retail leaves the object where it was. Reachable in the ordinary case: CanAttemptDestination passing means collision is published, which is exactly when the engine runs.

Worse sub-case: RuntimeSetPositionState.cs:3057-3081 returns Cancelled after CommitCanonical succeeded ("retail lets that physical commit land regardless"). Cancelled also falls into default:Rejected → the fallback overwrites the just-settled body (contact plane, step-down) with the raw unresolved destination. RemoteMotion.Body is record.PhysicsBody in production (RuntimePhysicsState.cs:1039, sole construction site).

Zero of the five new non-commit tests cover Rejected. The 10-row discrimination table has no row for the one outcome where the fallback is wrong.

The required partition

  • Store (placement genuinely never executed → retail @0x00515C1D store_position): Refused, Contention, NotApplicable, and Rejected arising from a preparation failure (InvalidData/RejectedAuthority, :770-773) which never reached the engine.
  • Do not store (engine ran and refused → retail @0x00515CB2 / @0x00515CD5): Rejected arising from outcome.Status, and Cancelled-after-commit, which must not overwrite a settled pose.

The status enum is too coarse to express this. Widen it or thread the originating outcome; do not approximate it with a heuristic.

MAJOR B — the quiescence pre-flight is incomplete on BOTH sides

Both reviewers found this independently by different routes. CanAttemptDestination (RuntimeRemotePlacementDriveController.cs:911-915) tests one prefix, and IsCollisionPrefixQuiescing (RuntimeSetPositionState.cs:827-829) masks to that one prefix, called with the destination only.

Core's predicates are broader in two independent ways:

  1. Source landblockPlacementTouchesPrefix (:3836-3841) also matches request.CurrentCellId, populated from operation.Record.FullCellId whenever body.InWorld (:2884-2888). A far snap out of a quiescing landblock passes the pre-flight and parks at :2946/:2974. This is the likelier shape — streaming retires and republishes continuously, and a remote at >=96 m sits near the window edge where retirement happens.
  2. Swept neighbourResultTouchesPrefix (:3843-3858) scans every QueriedCellIds entry. That footprint provably spans neighbour landblocks: PhysicsEngine.cs:1347 binds it to the transition's candidate array, materialized at :1384; CellArray.Add mirrors every id (CellArray.cs:43-52); CellTransit.FindCellSet (TransitionTypes.cs:3521-3527) reaches AddAllOutsideCells (CellTransit.cs:918, :985); and AddOutsideCell (CellTransit.cs:375-384) states explicitly there is no same-block filter — neighbour cells come out with the neighbour's prefix, added whenever the sphere is within its radius of a boundary (AddAllOutsideCells:337-350).

Either route reaches ParkDeferred with restorableOnCancel defaulting false (:4276), so CancelToken's restoreCancelledPark: true finds ParkWithdrawal.Captured false (:3399-3410, :4285) and RestoreParkWithdrawal never runs. The remote is left InWorld = false, Active cleared, clock suspended, FullCellId = 0, spatial projection withdrawn (:4294-4311), with the only operation that could wake it destroyed. The fallback pose write does not help — it writes a pose to a withdrawn body, which merely makes the stranding invisible to a position assertion.

A pre-flight cannot close this. The sweep footprint does not exist until the sweep has run. This is the same structural conclusion 4b-1's review reached, and SubmitAndResolve's own comment at :768-770 cites TryGetBlockingQuiescence by name as the residual — recording the hole while the code claims to close it.

MAJOR C — even a would-COMMIT placement is converted to a non-restorable park

:3012's check is result.IsSuccessful && TryGetBlockingQuiescence(...), and IsSuccessful is Error == Ok (PhysicsSetPosition.cs:151) — true for a fully committed result. It sits ahead of the restorable result.IsDeferred park at :3050-3054. So a healthy, resident, about-to-commit far snap near a seam is rewritten to DeferredCell at :3017-3020 and parked non-restorably. The claim that "the restorable engine-result park stays reachable" holds only when no quiescing prefix is touched; the quiescence branch pre-empts it.

The pinned fix — at the source, as the campaign already decided

AP-136's blanket "every quiescence/retirement park is non-restorable" is over-broad for the two SubmitPreparedPlacementCore parks. Its stated reason — re-admitting a spatial root into a retiring prefix would block the retirement — is about ParkCollisionResidents (:3683), where the entity's own cell is retiring and IsAffectedCollisionResident (:3722-3738) / HasOldPrefixPlacementDebt (:3809-3834) would pin the prefix forever.

It does not hold at :2974/:3025: RestoreParkWithdrawal restores residency at body.CellPosition.ObjCellId (:3475) — the destination cell — while the blocking prefix is the merely-swept neighbour or the departed source. Restoring there re-admits nothing into the retiring prefix.

Make :2974/:3025 restorable when the restore cell's prefix differs from the blocking quiescence prefix. Keep CanAttemptDestination as an optimisation — it uses Core's own read-only predicate, is exact within its subset, adds no timer/retry/flag, and strictly shrinks the reachable set. It must simply stop being the correctness mechanism. Update AP-136 and AP-138(2) to match.

MAJOR D — StoreAcceptedDestinationPose writes through a possibly-superseded incarnation

CancelToken publishes its receipt synchronously (RuntimeRemotePlacementDriveController.cs:925-930), and this same diff's own doc (LiveEntityNetworkUpdateController.cs:1005-1018) states a caller "MUST re-validate position ownership … on EVERY placement status, before writing anything else for the packet". :576-580 calls StoreAcceptedDestinationPose immediately after that dispatch with no currency check. RestoreParkWithdrawal does guard (RuntimeSetPositionState.cs:3468-3469). The App-side re-validation happens only after the seam returns — too late.

This is the exact rule the R5 fix in this same diff introduced. One IsCurrent(record) test closes it.

MINOR

  • N1 — Advance()'s window-drop path is asymmetric. :692-696 calls CancelToken without StoreAcceptedDestinationPose, so a retained retry whose destination leaves the window reproduces a smaller version of the freeze this round exists to fix. Its comment at :685 still claims the entry point "keeps its last committed pose" — no longer true.
  • N2 — post-sweep quiescence park overwrites a settled pose. At :3017 the parked result is the committed one, whose Position is spherePath.CurPos (PhysicsEngine.cs:1605) — collision-settled. ParkDeferred:4290 snaps there; StoreAcceptedDestinationPose:627-630 then writes the raw destination over it. Latent while MAJOR B stands; live the moment the park becomes restorable.
  • N3 — Advance() reopens the CurrentCellId condition. :690-702 re-checks only CanAttemptDestination(destination). Non-Position rebucket paths (RemoteTeleportController.cs:532, DatLiveEntityProjectionMaterializer.cs:777, EquippedChildRenderController.cs:408) can move record.FullCellId to a third, quiescing landblock in between.
  • N4 — five stale comments, inside the round meant to end the disease. :777-778 ("Rejected/Cancelled — … so the body never moved") is the one whose correctness would have exposed MAJOR A. Also :685 (N1), :91-93 (Rejected's doc omits the engine-refusal producer), :2144-2151 (doesn't mention the guard now in front of it), :2183-2187 (provenance claim fine, value claim false). And LiveEntityNetworkUpdateController.cs:1078-1079 cites :1886/:2130 where the actual early returns are :1977/:2241 — wrong citations in the fix for the stale-comment finding.
  • N5 — NotApplicable via record.PhysicsBody is null (:436) still runs the fallback, writing a RemoteMotion body that in that state is not the canonical one (RemoteMotion.cs:271).
  • N6 — teleport_hook enumeration incomplete. @0x00514ED0 also calls report_collision_end(this, 1) @0x00514F31, omitted from RuntimeRemoteFarSnapPosition.cs:454-457 and AP-137.
  • N7 — a test assertion is order-dependent. RemotePlacementLedger_ConvergesAcrossGuidReuse_WithoutTeardown:708-711Assert.Equal(0, drive.PendingCount) is zero only because the preceding CaptureOwnership() invoked CountLivePending, which mutates _pending. Swap the asserts and it fails. Also omits AssertConverged.

Required tests

  • Rejected from an engine refusal: assert the body does not move.
  • Cancelled-after-commit: assert the settled pose survives.
  • Rejected from a preparation failure: assert the body does advance.
  • Quiescing source landblock: far-snap out of it, assert body.InWorld && record.ObjectClock.IsActive && record.FullCellId != 0.
  • Quiescing neighbour: far-snap to a point one sphere radius inside a landblock seam with the neighbour quiescing, same assertions.
  • Advance()'s window-drop path: assert the pose advances.

Do not churn — independently verified correct

  • The retail decode of SetPositionInternal's storing branch (@0x00515CDA → @0x00515CE2 → @0x00515CF2 → @0x00515CF7 → return 0 @0x00515D07). Exact.
  • R2's deferral to 4b-3 is the right call, and both reviewers agree. teleport_hook @0x00514ED0 runs before the placement @0x00516420, so a pose-only half-port would strand a live moveto, stick, and leash. RuntimeTeleportHookPhase.BeforePositionOperation exists (RuntimeAuthoritativePositionRouteClassifier.cs:410) and is recorded-and-dropped for the remote arm — a representable delta with the data model already present.
  • The carried SEND_POSITION_EVENT_SPF = 0x1000 question is SETTLED: the flag is inert. @0x00515330 takes (CPhysicsObj*, CTransition const*) — no struct; @0x00515BD0 reads only bit 5; @0x00516040 tests only bits 8/9; a tree-wide search finds no bit-12 test on a SetPositionStruct.
  • Pose parity and the decoy. TryGetWorldFrameOffset is the source of ShadowWorldOffsetX/Y; StoreAcceptedDestinationPose:627-635 composes identically to RuntimeSetPositionMoverPreparer.TryBuild (RuntimeSetPositionMoverPreparation.cs:155-168). worldPos is not an input — the decoy retains its discriminating power.
  • The status is genuinely no longer discarded; both arms thread RemoteContactRouting(Arm, Placement).
  • Register bookkeeping: grep -c "^| AP-" = 96, header updated, no duplicates, nothing retired kept a row.
  • R5's ordering is identical in both arms, and its "not constructible" claim holds — though it is the standard signal the guard belongs behind a testable Runtime seam rather than duplicated at two App call sites. Name that for 4b-3.
  • The StopInterpolating "unobservable" claim is true: TryExecuteAcceptedRemotePosition never receives remote, and no placement or projection path reads or writes the queue. Stating it beats a false pin.
  • R7's Assert.True(remote.Interp.IsActive) genuinely closes its hole. R8's two park tests provoke their states honestly.
  • AP-136's scoping was read correctly and was not retrofitted (untouched in the diff).

Gate

Complete Release suite. Baseline 10,968 / 4 / 0 at 1b631f12 (App 4088, Runtime 1073), measured. The fix round measured 10,997 / 4 / 0 while containing every defect above. Known flakes: #302 (PortalProjectionTests…, App.Tests) and #308 (NakEmissionTests.LossSoak_…, Core.Net.Tests). Do not conflate.