erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

docs: C4 route 4b-3 contract and both dual-review rounds

Browse source 
The pinned contract plus the four review documents behind 6dc7ba51:
round 1 (retail FAIL 3 MAJOR / architecture FAIL 2 MAJOR) and round 2
(both PASS on the fixed diff).

Process findings worth carrying into the remaining routes:

1. The contract's 13 explicit "what must REMAIN true" invariants were the
   fix for 4b-2's round-1 defect (a contract that said what must change but
   not what must stay). They did NOT prevent a round-1 FAIL here. What the
   round-1 MAJORs actually shared was a STRUCTURAL cause the invariant list
   could not express: two parallel inline copies of the same routing tail.
   An invariant list constrains behaviour; it cannot see duplication.

2. Both reviews independently found the same NPC synth-velocity defect
   (retail R3 = architecture A2). Independent convergence on one finding is
   the strongest signal this process produces — weight it accordingly.

3. The specified two-client gate could not have observed three of the four
   MAJORs: it teleported a player character, and all three live on the NPC
   arm. Caught by both reviewers before the gate ran, not after. Check that
   a gate can structurally see the defect class it is gating.

4. A reviewer named a symbol that does not exist (RuntimeCollisionReportingState
   .ForceEnd is a private helper, not the public entry point). The implementer
   silently substituted the correct one (LeaveWorld) while reporting "no
   disagreements with either review". Verify implementer claims against source
   even when they report full agreement.

5. A new test asserted only what must NOT happen, so an emptied
   ApplyWireAirborneLeftoverBookkeeping passed every test in the tree. Caught
   in round 2 and closed with positive assertions, sabotage-verified. Negative
   assertions alone cannot detect a deleted write.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-04 16:00:26 +02:00
parent 6dc7ba51ee
commit 8c269ad1a6
5 changed files with 2162 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

299
docs/research/2026-08-04-c4-route-4b-3-architecture-review-round2.md Normal file
View file

@ -0,0 +1,299 @@
# C4 route 4b-3 — architecture / adversarial DELTA review, round 2 (2026-08-04)
Reviewer lens: **adversary**. Round 1's findings are
[`2026-08-04-c4-route-4b-3-architecture-review.md`](2026-08-04-c4-route-4b-3-architecture-review.md);
the retail lane's are
[`2026-08-04-c4-route-4b-3-retail-review.md`](2026-08-04-c4-route-4b-3-retail-review.md).
Subject: the same uncommitted working tree on
`claude/acdream-physics-divergence-5aa784` at base `3e002993`, after the round-1
fix pass. This review re-attacks the **fixes**, not the whole slice — the
round-1 "verified sound" list is carried forward except where a fix touched it.
## Verdict: **PASS**
Both round-1 MAJORs (A1, A2) are fixed, and I verified each fix produces the
retail-correct outcome rather than merely silencing the symptom. The retail
lane's R1/R2/R3 are fixed. **The `RunRemoteArmTail` /
`ApplyWireAirborneLeftoverBookkeeping` extraction — the thing most likely to
introduce a new defect — is behaviour-identical at all five call sites**; I
walked each one against the pre-fix code and found no reordering, no dropped
step, and no widened or narrowed guard. Six MINORs below, none blocking.
Build `dotnet build AcDream.slnx -c Release`: **succeeded, 0 warnings**.
Focused suites at Release `--no-build`: **Runtime 1,125 / 0 skips**,
**App 4,081 / 3 skips** (up 3 from round 1's 4,078 — the three new App tests).
---
## Part 1 — the extraction, call site by call site
The blast radius of the fix pass is small and I confirmed it: only
`LiveEntityNetworkUpdateController.cs`, one new method on `LiveEntityRuntime.cs`,
the App teleport test file, and three docs carry round-2 markers.
### `RunRemoteArmTail` — 3 call sites
Signature returns `RemoteContactRouting?`; `null` means "the currency guard
tripped, write nothing further". The guard inside it is
`(Arm is FarSnapPlacement or TeleportPlacement) && (!isCurrentPositionOwner() || !ReferenceEquals(positionRecord.RemoteMotionRuntime, remote))`.
| site | pre-fix sequence | post-fix | verdict |
|---|---|---|---|
| player teleport dispatch (`:2333`) | routing → **unconditional** guard → return | routing → **arm-gated** guard → return | **identical.** `ApplyRemoteContactRouting` returns `TeleportPlacement` iff `OwnsTeleportPlacement(route)`, which the caller has already evaluated on the *same* `earlyRemoteRoute` with the same pure static predicate. The arm at this site is provably always `TeleportPlacement`, so arm-gating the guard cannot weaken it. |
| player grounded routing (`:2566`) | routing → arm-gated guard → return | same, via helper | **identical**, including the R5 guard-before-arm order (`TryArmConstraintAfterOperation` is still the first statement after the null check, `:2596`). |
| NPC dispatch (`:2818`) | routing → arm-gated guard → `return` from `OnPosition` | routing → helper returns `null` → `if (npcRoutingResult is null) return;` inside the same `if (!snapSuppressedByStick \|\| isTeleportRoute)` block | **identical**, same lexical position, same effect. |
Specific things I checked for and did **not** find:
- **`willBeDrTicked` moved inside the helper.** The player grounded site used to
hoist it into a local (`HEAD:2317`). I grepped every use in both the HEAD and
current file: its only consumer was ever the routing call itself. Nothing
downstream reads it. No loss.
- **Delegate identity.** `runTeleportHook` now closes over `canonical`/`remote`
and reuses the caller's `isCurrentPositionOwner` for both the hook's per-step
currency checks and the post-routing guard. Previously two separately
allocated but semantically identical lambdas. No change in what is tested.
- **Guard-before-arm (R5) at all three sites.** Held. The helper cannot arm — it
has no access to `ToConstraintArm` and its doc says so — and all three callers
arm immediately after the null check.
- **The NPC sticky-suppressed path still arms.** `npcArm` is initialised to
`UnroutedCatchUp` and `TryArmConstraintAfterOperation` sits *outside* the
`if (!snapSuppressedByStick || isTeleportRoute)` block, exactly as at HEAD.
The extraction did not pull it inside.
### The LANDING TRANSITION block — confirmed untouched and non-conflicting
`:2445-2532` still gates on `!rmState.Body.InContact`, still `return`s, and
still arms with its own hard-coded `NearInterpolate`. It therefore cannot
double-run with the shared tail, and — importantly — it is what makes
`playerArm` provably never `AirborneSnap` at the grounded-routing site
(the free-flight case has already returned). The only edit is its comment,
which round 1 flagged as false (A5) and which is now correct **and complete**:
it enumerates `SetPositionSimple` / `null` / `Rejected*` as classifications
that also reach the block, and states why `NearInterpolate` is the right arming
value for every one of them.
### `ApplyWireAirborneLeftoverBookkeeping` — 2 call sites
The helper writes exactly three fields: `remote.CellId`, `remote.LastServerPos`,
`remote.LastServerPosTime`.
- **NPC site (`:2717-2722`)** — new, and the R1 fix. Correctly gated
`!update.IsGrounded && !isTeleportRoute`, placed after the
`IsAirborneNoOperation` return (so `NoPositionOperation` never reaches it)
and before the synth-velocity block, the sticky probe, and routing. I
confirmed the gate is exhaustive: for `RuntimeAcceptedPositionSource.PositionEvent`
a wire-airborne packet can only classify `NoPositionOperation` (handled above),
`null`, or `Rejected*` — the classifier's `effectiveContact` widening applies
only to `SameIncarnationCreate`, which this path never carries.
- **Player site (`:2390-2395`)** — the helper adds a `remote.CellId` write the
hand-rolled block did not have. **I chased this specifically as the classic
extraction hazard and it is benign**:
`RuntimePhysicsState.CommitCanonicalCell:2145` short-circuits on
`fullCellId == record.FullCellId` *before* `SetFullCell` and before raising
`CellCommitted`, and the player arm wrote the identical `p.LandblockId` ~180
lines earlier with no placement in between. The redundant write is a true
no-op — no spurious rebucket event, no version bump.
---
## Part 2 — the individual fixes
### A1 — `ToConstraintArm(AirborneSnap) → NearInterpolate`, switch made total
Correct, and correct for the right reason: `AirborneSnap` is acdream's
*body*-contact carve-out for a packet whose **wire** contact bit said grounded,
so retail's `MoveOrTeleport` returns nonzero and `HandleReceivedPosition` arms
@0x00454272. Mapping it to an arming value restores the 1-arm count and makes
the App-layer partition exactly the contract's D4 table.
**The throwing default cannot be reached in production.** `RemoteContactArm`
has exactly five members (`AirborneSnap`, `SteadyStateInterpolate`,
`FarSnapPlacement`, `TeleportPlacement`, `UnroutedCatchUp`) and all five have
explicit cases; `default(RemoteContactArm)` is `AirborneSnap` (value 0), which
is handled. The only two producers of the argument are
`ApplyRemoteContactRouting`'s returns and the NPC arm's
`RemoteContactArm.UnroutedCatchUp` initialiser. Reaching `_` requires an
out-of-range cast, which nothing performs. The throw is a genuine
"can't-happen" guard, not a live hazard — this was the right call over a silent
zero-arm fallback.
**The test genuinely discriminates.**
`NpcAirborneSnap_LandingPacket_StillArmsTheLeash` asserts
`host.PositionManager.Constraint` goes from `null` to non-null across the
packet. I verified `PositionManager.Constraint` is lazily created *only* inside
`ConstrainTo` (`PositionManager.cs:27-28`, `:62`), so its presence is direct
proof of an arm rather than an inference. Under the pre-fix mapping the packet
arms zero times and `Constraint` stays null — the test fails.
### A2 / R3 — NPC synth-velocity and cycle-apply gated on `!isTeleportRoute`
Correct. `isTeleportRoute` is hoisted above both the D2 check and the velocity
block, and both the install (`:2741`) and the `RemoteServerControlledVelocityCycle.Apply`
call (`:2886`) carry it. The fix's own claim that leaving `ServerVelocity`
stale is safe holds up: its only consumers are the now-excluded cycle-apply and
`RuntimeRemotePhysicsUpdater`'s stale-velocity watchdog, which only *zeroes*.
Two incidental changes in the same block are behaviour-neutral and I checked
them rather than assumed: the `!IsPlayerGuid(update.Guid)` guards were dropped
from the synth condition and the `else if` collapsed to `else`. Both were
already dead — the whole NPC section sits after `if (IsPlayerGuid(update.Guid)) { … }`
and every path inside that block `return`s (the last statement before its
closing brace at `:2666` is `return;`), so the section is unreachable for
player guids.
`NpcTeleport_DoesNotInstallASynthesizedVelocity` seeds `LastServerPos`/
`LastServerPosTime` so a broken implementation has a real distance and interval
to synthesise from. It fails pre-fix (`HasServerVelocity` would be `true`).
### R1 — the D2 shape on the NPC arm
Correct and now genuinely shared. The AP-137 sentence *"unifies player and NPC
remotes on one behaviour"* is true against the code as of this round — I
re-read the row and the two call sites.
`NullClassifiedNpc_WireAirbornePacket_WritesOnlyBookkeepingNoBodyOrShadow`
discriminates: pre-fix the packet reaches `ApplyRemoteContactRouting`'s
free-flight carve-out and hard-snaps `Body.Position = worldPos`, which the
test's `Assert.Equal(spawnBodyPose, …)` catches.
### R2 — the teleport hook's collision-end action → `ForceEndCollisionReporting`
**This was the fix I was asked to attack hardest, and it does not carry any of
this family's failure modes.** `RuntimeCollisionReportingState.LeaveWorld` is
named for the *collision-reporting* state machine's own leave transaction, not
the entity lifecycle's. I traced it end to end:
- `LeaveWorld:870-887` → `_admissionBlocked.Add(key)` → `ForceEnd:1435-1454` →
`EndExpiredObjectCollisions(force: true)` → `TrimEmptyOwner`. **The only
mutations are the owner's collision table, the reverse-owner index, and the
report queue.** Nothing writes `body.InWorld`, `TransientStateFlags.Active`,
`record.ObjectClock`, `FullCellId`, spatial residency, or visibility.
**Invariant 3 is not at risk.**
- `_admissionBlocked` is released in a `finally`, so the subsequent placement's
`TryPrepareSetPositionBatch:230-235` — which *does* refuse while a key is in
`_leaving` or `_admissionBlocked` — is not blocked by the hook that ran
immediately before it.
- `_mutationRevision` is bumped by `LeaveWorld`, but the placement's batch
captures `expectedMutation` *after* the hook (`:238`) and the
prepare→install window is entirely inside the placement, so the bump cannot
invalidate the teleport's own batch.
- `Publish` swallows observer exceptions (`:1489-1494`), and `_leaving` guards
re-entrancy, so a force-end cannot escape as a throw into the packet path.
- `TrimEmptyOwner` runs on the exit path, so `CaptureOwnership`'s owner count
cannot leak an empty entry — no ledger regression.
Dropping the old `ShadowObjects.Suspend` is also safe and is arguably the
better half of the fix: the far-snap arm (shipped, user-accepted) has never
suspended the shadow before its own placement, so the teleport arm is now
*consistent* with it rather than uniquely different, and the arm tail's
`TryPublishRemote` → `SyncRemoteShadowToBody` (ungated at this call site) still
re-seeds the registry at the resolved pose. See MINOR B2 for the one thing
about R2 that should be stated rather than left implied.
---
## MINOR findings
### B1 (MINOR) — contract test-plan item 11 is still unwritten, and it is now the *only* thing guarding the #42 class
`NullClassifiedNpc_WireAirbornePacket_…` asserts what must NOT happen (no body
write, no shadow republish) but never asserts what MUST happen: that
`ApplyWireAirborneLeftoverBookkeeping` actually wrote `remote.CellId` and
`LastServerPos`/`LastServerPosTime`. A regression that emptied the helper body
would pass every test in the tree. Those writes are exactly AP-135 — the
free-fall sweep gates on `rm.CellId != 0` and without it "an airborne remote
falls through the floor" (#42), and the first grounded packet after an arc
would synthesise its velocity across the whole jump. Two `Assert.Equal`s on the
existing test close it.
### B2 (MINOR) — R2's fix is correct plumbing, but its *observable* half still goes nowhere; say so before someone records it as closed
`IRuntimeCollisionReportObserver` has **zero production implementations** —
`git grep OnCollisionReport` returns the interface, the dispatch loop, and four
test doubles. `CollisionReports.Subscribe` has no production caller. So the
bidirectional `DoCollisionEnd` notification retail fires at @0x00514620 still
reaches no gameplay consumer; what the fix actually delivers today is the
**table clear** (real, and it does prevent the later ~1 s stale-end and any
mis-attribution) plus a correctly-wired channel for when a consumer appears.
The hook's new doc doesn't overclaim, but the retail review's phrasing
("neither side receives the immediate `ObjectCollisionEnd`") will read as
closed. State the residual — it is a pre-existing gap in the collision-report
plumbing, wider than this slice, and it deserves one sentence rather than a
silent inheritance.
### B3 (MINOR, disclosed) — no test for R2
Judged **acceptable but worth filing**. A test would assert against a channel
nothing production reads (B2), so its value is pinning the retail *mapping*,
not a behaviour. The cheap version is one assertion that the teleporting
owner's collision table is empty after an `OnPosition` teleport packet — that
would fail if a future edit reverted the action to `ShadowObjects.Suspend`,
which is the actual regression risk. Not blocking.
### B4 (MINOR, disclosed) — the per-packet `runTeleportHook` closure
Two delegates + two display classes per remote accepted Position (the outer
`runTeleportHook` lambda and the caller's `isCurrentPositionOwner`), allocated
whether or not the packet is a teleport. The implementer's stated blocker —
that `ApplyRemoteContactRouting`'s `Func<bool>` parameter is injected by
existing tests — is real but the parameter is `internal`, so it is a
refactor cost, not a compatibility one. Judged **acceptable to defer**: this is
the per-packet network path (5-10 Hz per remote), not Slice I's per-frame
resolve path where the 0-B target applies, and the objects are gen0-transient.
File it with the probe-family cleanup rather than churning the seam now.
### B5 (MINOR, disclosed) — the stress test's teleport step
Round 1's A3 stands as written and is judged **acceptable to leave**. The
scenario it covers (Hidden → DeferredShadowRestore → landblock churn → UnHide)
no longer has any teleport-specific machinery to interact with, because this
slice deletes `_activePlacementOwners` and the placement is now synchronous
inside `OnPosition`. The hand-built step is honestly documented as reproducing
the arm tail's observable state, and the arm tail itself is directly covered by
`LiveEntityNetworkRemoteTeleportPresentationTests`. The residual loss — the
*interaction* between a mid-teleport Hidden edge and the placement — is a
degenerate case now, not an untested one.
### B6 (MINOR, carried) — proof obligation 1 is still unstated
Unchanged from round 1's A9. The contract requires the
`ParkCollisionResidents`-overlap-throw unreachability argument (with 4b-1's B2
caveat that the guarded property is
`TryAcquireCollisionPrefixMutationPermission`'s `HasOldPrefixPlacementDebt`
refusal — a stall, not a throw) in the implementation commit's
contract-conformance section. The work is still uncommitted so I cannot verify
it; the property itself holds by reading (the teleport arm adds packets to the
same one-operation-per-key machinery and opens no new operation shape).
---
## Round-1 MINORs — spot-check
Verified fixed, not re-derived: **A4** (`TeleportRefused_…` now arms sticky
before the packet and asserts `GetStickyObjectId() == 0` afterward — a real
discriminator, since only the teleport arm's hook calls `UnStick`, so the
`UnroutedCatchUp` look-alike I identified can no longer pass it); **A5** (the
landing-block comment is now correct *and* complete); **A7** (R1, above);
**A8** (`wasCellless` deleted). **A10** was accepted and restated correctly.
The retail lane's **R4** (the prologue comment's `CommittedCellId` claim) and
**R5** (`acdream-architecture.md` / `code-structure.md` still describing the
deleted classes) are both fixed in the diff.
---
## Gate status observed
- `dotnet build AcDream.slnx -c Release` → **succeeded, 0 warnings, 0 errors**.
- `tests/AcDream.Runtime.Tests -c Release --no-build` → **1,125 / 0 / 0**.
- `tests/AcDream.App.Tests -c Release --no-build` → **4,081 / 3 skipped / 0**.
- Complete Release suite: **still not run and not recorded.** The contract is
explicit that the new figure must be measured, not inherited from 11,027.
Outstanding.
- Two-client connected teleport gate: outstanding. **It must use an NPC
target** (`@teleto` a creature into view), not a second player character —
both round-1 MAJORs lived on the NPC arm, and
`RemoteServerControlledVelocityCycle.Apply` early-returns for `0x50xxxxxx`
guids, so a player-remote teleport exercises neither fix. Confirm at least
one `[remote-teleport]` line per teleport with the expected `cause`.

386
docs/research/2026-08-04-c4-route-4b-3-architecture-review.md Normal file
View file

@ -0,0 +1,386 @@
# 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`](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 `return`ed. 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.

676
docs/research/2026-08-04-c4-route-4b-3-contract.md Normal file
View file

@ -0,0 +1,676 @@
# C4 route 4b-3 — remote teleport + cell-less: pinned contract (2026-08-04)
Split rationale:
[`2026-08-04-c4-route-4b-scoping-and-split.md`](2026-08-04-c4-route-4b-scoping-and-split.md).
4b-1 (infrastructure) landed at `2e8e09ac`; 4b-2 (far snap) landed at
`7f1c1f5a` after four fix rounds; the shared-core park restore it forced is at
`634bc551` and inside 4b-2's rounds 3/4. Read the whole findings chain before
implementing — every defect class it names reappears here at larger scale:
[round 1](2026-08-04-c4-route-4b-2-review-findings.md) →
[round 2 (delta)](2026-08-04-c4-route-4b-2-delta-review-findings.md) →
[round 3](2026-08-04-c4-route-4b-2-round3-correction.md) →
[round 4](2026-08-04-c4-route-4b-2-round4-correction.md).
**4b-3 flips the LAST remote classification on: `SetPosition` — teleport
(TELEPORT_TS advanced) and cell-less (the body has no committed cell) — through
4b-1's `RuntimeRemotePlacementDriveController`, runs retail's `teleport_hook`
before the placement, and deletes the legacy remote-teleport machinery:
`RemoteTeleportController` (605 lines), `RemoteTeleportPlacement` (85),
`RemoteShadowPlacementSynchronizer` (49, two classes), the
`remotePlacementRequired` predicate, the `TeleportHookRequired` timestamp
plumbing, the legacy pre-operation `ConstrainTo` fallback, and the player arm's
legacy `!update.IsGrounded` fallback — plus 1,709 lines of their tests.**
This retires AP-137's cell-less enqueue-vs-place delta. AP-135 does NOT retire
(its two writes sit physically inside the method this slice rewrites — see
"Must remain true" item 8). AP-131 does not retire. #276 does not close.
## Retail ground truth — verified in `acclient_2013_pseudo_c.txt`, verify again yourself
`CPhysicsObj::MoveOrTeleport` @0x00516330, teleport/cell-less branch:
```
00516375 eax_8 = CPhysicsObj::newer_event(this_1, TELEPORT_TS, arg3);
00516386 if ((eax_8 != 0 || this_1->cell == 0)) {
005163ef CPhysicsObj::teleport_hook(this_1, edx_2);
005163f8 SetPositionStruct::SetPositionStruct(&var_64);
00516406 SetPositionStruct::SetPosition(&var_64, arg2);
00516414 SetPositionStruct::SetFlags(&var_64, 0x1012);
00516420 CPhysicsObj::SetPosition(this_1, &var_64);
00516438 return 1;
00516386 }
0051638e if (arg4 != 0) { ... near @0x005163AF / far @0x005163C1-E8 ... }
0051636d return 0;
```
Five facts in that listing decide this slice:
1. **The teleport branch is decided BEFORE the contact test** (`arg4` is only
read @0x0051638E, after the branch). A teleport/cell-less packet places
unconditionally — airborne wire bit, airborne body, any distance. acdream's
routing must therefore decide the teleport arm AHEAD of every
airborne/landing carve-out (see design decision D5).
2. **`this_1->cell == 0` is the BODY's current cell** — "this object has no
resolved cell right now" — not the wire destination's cell. See D1: the
classifier's current input implements a different (and dead) predicate.
3. **`teleport_hook` @0x00514ED0 runs BEFORE the placement** and is, complete
(each guarded on the manager existing):
`MovementManager::CancelMoveTo(0x3C)` @0x00514EDF,
`PositionManager::UnStick` @0x00514EEE,
`PositionManager::StopInterpolating` @0x00514EFD,
`PositionManager::UnConstrain` @0x00514F0C,
`TargetManager::ClearTarget` @0x00514F1B +
`TargetManager::NotifyVoyeurOfEvent(Teleported_TargetStatus)` @0x00514F28,
`CPhysicsObj::report_collision_end(this, 1)` @0x00514F31.
4. **`SetFlags(0x1012)`** (`Teleport|Slide|SendPositionEvent`) @0x00516414 —
acdream's analog is the classifier's `AuthoritativeTeleportFlags`, carried on
`route.SetPositionFlags` into
`RuntimeSetPositionState.TryPrepareAndSubmitAuthoredPlacement`'s `flags`
parameter. (There is NO separate "teleport hook phase support" inside
`RuntimeSetPositionState` — the phase lives on the route,
`RuntimeTeleportHookPhase.BeforePositionOperation`, emitted at the
classifier's remote teleport/cell-less branch. The handoff's phrasing
implied otherwise; this is the correction.)
5. **The branch returns 1 and discards `SetPosition`'s error**, so
`SmartBox::HandleReceivedPosition` @0x00453FD0 arms `ConstrainTo`
@0x00454272 — the remote arm's ONLY arming site, shared by all three
nonzero-returning branches, anchored `&arg2->m_position` read live
(post-move). Retail arms even when the placement failed. So: hook
`UnConstrain` first, place, then ONE post-operation re-arm — never a second
arming site, and arm on non-commit outcomes too (the 4b-2 B8/R5 lesson).
## What must REMAIN true (process rule 1 — the contract causes the defect)
For every path this slice adds or rewrites, including every refusal,
contention, rejection, and short-circuit:
1. **The pose still advances.** A teleport-classified packet whose canonical
placement never reached the engine (`Refused` / `Contention` /
`RejectedPreparation` / `NotApplicable`) still commits the accepted
destination pose to the canonical body — the same
`StoresAcceptedDestination` partition and the same
`StoreAcceptedDestinationPose` the far arm uses, which is retail's
`store_position` @0x00515CE2 on the no-transition branch. `Deferred` and
`RejectedByPlacement` do NOT store, for the reasons already pinned in the
enum's own doc (`ParkDeferred` already snapped; the engine ran and
refused / a settled pose must survive). Do not re-litigate the partition —
extend it to the teleport arm unchanged.
2. **The render entity still advances.** The teleport arm's tail syncs
`WorldEntity` (position, `ParentCellId`, rotation) from the RESOLVED body
and publishes the collision shadow, exactly as both existing grounded arm
tails do (`LiveEntityShadowPublisher.TryPublishRemote`). A teleport must
never leave the rendered pose a packet behind the body. #312's lesson:
presentation state is where this family breaks; tests must assert it
(see Test plan).
3. **The object clock keeps running and the entity stays in the world** on
every non-commit outcome: `body.InWorld`, `TransientStateFlags.Active`,
`record.ObjectClock` active, `FullCellId != 0`, spatial projection intact,
`IsSpatiallyVisible` unchanged. No park survives the controller
(`CancelToken` with `restoreCancelledPark: true`); the pre-flight
(`CanAttemptDestination`) stays an OPTIMISATION, never the reason a remote
stops tracking or becomes invisible.
4. **The interpolation queue is empty after the teleport arm runs** —
cleared by the hook's `StopInterpolating` (@0x00514EFD), NOT by the route
flag: the classifier's teleport branch deliberately carries
`StopInterpolating: false` because retail's clear lives inside the hook.
The far arm's route-flag-driven clear is untouched.
5. **The leash is armed exactly once per accepted packet**, post-operation,
anchored post-move, by the classification partition — never zero times
(the current shipped hole: `remotePlacementRequired` returns ahead of every
arming site while `RemoteTeleportHook` has already `UnConstrain`ed, so a
remote that teleports and stands still is leash-less until the next
packet), and never twice. See D4 for the complete partition.
6. **The per-packet prologue keeps running for every classification**:
`TryApplyGenericRemoteRenderPose` (see D6 for the gate), the
`RebucketLiveEntity(update.Guid, p.LandblockId)` spatial-bucket
transaction, the velocity install
(`TryCommitAuthoritativeVelocity` — retail's PositionPack `set_velocity`,
upstream of `MoveOrTeleport`), and the incarnation re-validation
chain. Retail's remote teleport never writes velocity itself
(`ZeroVelocity` is the LOCAL player's `SmartBox::TeleportPlayer`
@0x004541B4 only); the teleport arm must not add a velocity write.
7. **`AcceptedPositionSource`/authority validation is unchanged.** The
teleport arm executes only an already-classified route from
`ClassifyRemoteAcceptedPosition` — the shared builder
(`RuntimeAcceptedPositionRouteRequests`), never a re-derivation.
8. **AP-135's two writes stay**, on both arms' airborne no-op
neighbourhoods: the server-cell adopt (`rmState.CellId = p.LandblockId`)
and the `LastServerPos`/`LastServerPosTime` sample. They are 4a-owned
acdream-only bookkeeping for the free-fall sweep gate
(`RuntimeRemotePhysicsUpdater`'s `rm.CellId != 0` gate) and the
first-grounded-packet velocity synthesis. This slice rewrites the method
they sit in; they do not go. The register row does not retire.
9. **`ParkCollisionResidents`'s overlap throw stays unreachable** — see
"Proof obligations".
10. **`ArmLostFamilyDeadlines`' reaper gains no production caller.**
`TickLostCellDeadlines` and `TryDequeueExpiredLostCell`
(`RuntimeSetPositionState`) have zero production callers today; this
slice must not add one. `ParkDeferred` arming the family (its
`ArmLostFamilyDeadlines` call) is pre-existing and stays inert.
11. **Route 1's classification inputs are unchanged.**
`RuntimeInitialCreateContinuationExecutor`'s calls into the shared
builder keep their current semantics (D1 adds an overload for the remote
PositionEvent path; the builder's own doc mandates "an overload here,
never a third copy").
12. **The 4a dispositions are untouched**: `NoPositionOperation` writes
nothing (plus item 8's bookkeeping), `Interpolate` enqueues, the landing
block still hard-snaps a wire-grounded packet for a not-in-contact body
on 4a-owned classifications, AP-87's snap conditions and AP-139's landing
clear are unchanged.
13. **The far arm (4b-2) is untouched** except where a shared gate widens to
include the teleport arm (D6, D7) — widening must not change the far
arm's own behaviour. Everything round 2's/round 4's "do not churn" lists
verified stays: the `StoresAcceptedDestination` partition, the
`IsRestorableQuiescencePark` relocation, the pose-parity composition, the
two-arm guard/arm ordering.
## Design decisions — pinned, not open for redesign
### D1 — the classifier's cell-less input becomes the PRE-merge committed cell (resolves trap T1 and undetermined item 3)
**Finding (undetermined item 3, now determined by reading the code):** the
merge cannot zero a previously-nonzero `FullCellId` — it does the opposite.
`RuntimeEntityObjectLifetime.TryApplyPosition` calls
`Entities.RefreshSnapshot(canonical, snapshot, refreshPosition: acceptedPosition)`
for every accepted Position, and `RuntimeEntityRecord.RefreshDerivedState`
then executes `SetFullCell(position.LandblockId, …)` from the just-merged
snapshot. So at classification time (OnPosition classifies after the
authority gate's merge, before the prologue rebucket), `canonical.FullCellId`
IS the accepted wire cell. A wire cell of 0 fails
`LandDefs.InboundValidCellId` inside `PositionFrameValidation.IsValid`, which
the classifier's `ValidPosition` check turns into `RejectedData` BEFORE the
cell-less test. **Consequence: the classifier's remote `cellless` predicate,
as fed today (`CommittedCellId: canonical.FullCellId` in
`RuntimeAcceptedPositionRouteRequests.Build`), is unreachable for a remote
PositionEvent — the remote `SetPosition` classification currently fires on
`TeleportAdvanced` alone.** It is not a subset relationship with
`remotePlacementRequired`; it is a dead predicate. (The classifier's own
comment above the test — "only a later Runtime SetPosition or simulation
commit may change FullCellId" — is falsified by `RefreshDerivedState` and
must be corrected in this slice; process rule 6.)
Retail's predicate is the BODY's current cell (`this_1->cell == 0` read at
`MoveOrTeleport` entry, before any placement) — "this object is not resident
anywhere right now", e.g. the first Position after an unwield-to-3D
(`SetFullCell(0,0)` at the pickup/parent leave-world sites) or after any
canonical withdrawal. The acdream value with exactly that meaning is the
committed cell BEFORE this packet's merge — the `wasCellless`/`beforeCell`
pair `TryApplyPosition` already measures.
**Pinned:** thread the pre-merge committed cell out of the merge and into the
remote classification's `CommittedCellId` input, via an overload of the shared
builder (never a third copy; never a change to the existing overloads' route-1
semantics). Plumbing shape is the implementer's choice — the natural carrier
is `AcceptedPhysicsTimestamps` (whose `TeleportHookRequired` field this slice
deletes; replacing a policy tri-OR with a data field is strictly better) or
`AcceptedPositionNetworkUpdate`. Constraints: the value is the pre-merge
`FullCellId` measured by the SAME `TryApplyPosition` call that merged the
packet (never re-read after the merge); no fabrication (a known record always
has an honest value — 0 means "was celless", not "unknown").
**What is deliberately NOT adopted from `remotePlacementRequired`:** the
graphical `projectionRequiresTeleportHook` arm
(`LiveEntityRuntime.TryApplyPosition`: pre-merge `FullCellId == 0` OR
`!IsSpatiallyProjected` OR `!IsSpatiallyVisible`). Its visibility half is a
presentation predicate with NO retail analogue — it made "not currently
rendered" fire the whole teleport machinery on a routine hot path (trap T1's
warning). After this slice the teleport classification is retail's exact pair
(`TeleportAdvanced || wasCellless`); a not-visible remote's Position
classifies by distance like any other, the canonical placement decides
placeability, and visibility remains presentation-only. This behavioural
change is recorded in the AP-137 rewrite (D8). The whole
`projectionRequiresTeleportHook` computation, the lifetime parameter, the
headless `false` at `RuntimeLiveEntitySessionController`, and the
`TeleportHookRequired` field + its `timestamps with {…}` write are deleted.
### D2 — classifier-null and `Rejected*` keep 4b-2's stated policy (resolves trap T2); the legacy airborne fallback is replaced by the retail return-0 shape
The scoping doc's T2 described the pre-4b-2 world; 4b-2 already deleted the
legacy near/far blocks and routed `null` / `RejectedAuthority` /
`RejectedData` to `UnroutedCatchUp`
(`RuntimeRemoteFarSnapPosition.ResolveArm`, AP-137). **4b-3 keeps that policy
unchanged** — during the login window (null `_playerController` →
classification refuses; every remote packet) remotes keep tracking through
AP-87's catch-up, exactly as today. One consequence to state in the register
rewrite: a TELEPORT_TS-advancing packet that arrives while classification is
null consumes its teleport sequence in the timestamp gate but runs no hook —
benign in the only producing window (fresh session: no moveto, stick, leash,
or target exists yet to tear down), stated rather than discovered later.
What T2 actually leaves 4b-3 is the player arm's legacy
`!update.IsGrounded` fallback (the block whose comment says "4b deletes this
fallback" — a comment this slice at last makes true), which handled
wire-airborne packets for classifications 4a does not own. After D1 the
unowned set shrinks to null and `Rejected*`. **Pinned replacement:** a
wire-airborne packet with a null/`Rejected*` classification takes the retail
return-0 shape applied to the acdream-only states — AP-135's two bookkeeping
writes, no body write, no queue write, no render write, no leash arm, return.
This deletes the legacy block's entity-revert quirk
(`entity.SetPosition(rmState.Body.Position)`) and unifies the player and NPC
arms on one leftover-airborne behaviour. Recorded in the AP-137 rewrite.
### D3 — one teleport-hook implementation, triggered by the route, run inside the teleport arm before the placement
The hook trigger moves from `timestamps.TeleportHookRequired` (deleted) to
the classification: `route.TeleportHookPhase ==
RuntimeTeleportHookPhase.BeforePositionOperation`, which the classifier
already emits on exactly the remote teleport/cell-less branch. The hook runs
inside the new teleport arm, immediately before
`TryExecuteAcceptedRemotePosition` — retail's order (hook @0x005163EF before
`SetPosition` @0x00516420), and it runs REGARDLESS of what the placement then
yields (retail runs it before knowing the outcome).
There must be exactly ONE hook implementation. The existing
`RemoteTeleportHook.Execute` sequence — the six actions in retail order with
a currency re-check between every step — is the port and must be preserved
verbatim; whether the file moves into `AcDream.Runtime` (every action is
expressible there: `remote.Movement.CancelMoveTo`,
`rmState.Host.PositionManager.UnStick/UnConstrain`, `remote.Interp.Clear()`,
`host.NotifyTeleported()`, `Physics.Engine.ShadowObjects.Suspend(localId)`)
or stays an App bundle invoked from the arm is the implementer's choice.
Runtime residence is preferred (it is where the sibling arm logic lives and
what a future headless remote-motion consumer needs), but not at the cost of
inventing a second hook path. `RemoteTeleportHookTests` (38 lines) moves or
adapts with it — not silently dropped.
**Undetermined item 2 is RESOLVED — yes, `EntityPhysicsHost.NotifyTeleported()`
covers retail's TargetManager pair.** Verified on both sides:
`NotifyTeleported` executes `_targetManager.ClearTarget()` then
`_targetManager.NotifyVoyeurOfEvent(TargetStatus.Teleported)`
(`src/AcDream.Runtime/Physics/EntityPhysicsHost.cs`), and retail's
`teleport_hook` executes `TargetManager::ClearTarget` @0x00514F1B then
`NotifyVoyeurOfEvent(Teleported_TargetStatus)` @0x00514F28 under one
`target_manager != 0` guard. One-to-one; no open question remains here.
### D4 — the single `ConstrainTo` arm, and its complete partition
The legacy pre-operation arming call (the `OwnsAfterOperationConstraint`-gated
fallback in the player/NPC shared section, whose comment already says "4b-3
deletes it") is DELETED. The post-operation site —
`RuntimeRemoteSteadyStatePosition.TryArmConstraintAfterOperation`, called once
per arm after routing — becomes the only arm, matching retail's single site
@0x00454272. Its predicate widens from
`OwnsSteadyState || OwnsFarSnap` to the full retail partition:
| classification (routed arm) | wire contact | retail return | arm? |
|---|---|---|---|
| teleport / cell-less (`SetPosition`, new arm) | any | 1 | **yes** — after the operation, on every placement outcome (retail discards the error), post-move anchor |
| `Interpolate` (near) | grounded | 1 | yes (unchanged, 4a) |
| `SetPositionSimple` (far) | grounded | 1 | yes (unchanged, 4b-2) |
| `NoPositionOperation` (airborne no-op) | airborne | 0 | **no** (unchanged, 4a) |
| null / `Rejected*` → `UnroutedCatchUp` | grounded | (no retail state; analog: nonzero) | yes — via the same post-operation site, which must therefore accept a null route for this case |
| null / `Rejected*`, wire-airborne (D2 shape) | airborne | (analog: 0) | **no** — the current legacy pre-op arm DOES arm these; that was a divergence and it retires with the site |
The one-packet unarmed residual on a superseded incarnation (the currency
guard returning before arming) is AP-138(3) and extends to the teleport arm
unchanged. This partition closes the shipped hole named in scoping
correction 2: a remote hard teleport currently arms the leash NOWHERE
(`remotePlacementRequired` returns ahead of every arming site after the
hook's `UnConstrain`); after this slice the hook `UnConstrain`s and the
single post-operation site re-arms — retail's exact sequence.
### D5 — routing order: the teleport arm precedes every contact carve-out; sticky does not suppress it
Retail decides the teleport branch before reading `arg4`. Therefore:
- `RuntimeRemoteFarSnapPosition.ResolveArm` (or its successor) returns the
new teleport arm ahead of everything else, and
`ApplyRemoteContactRouting` dispatches it BEFORE the `!remote.Body.InContact`
free-flight carve-out — an airborne-body teleport packet places, it does
not `AirborneSnap`.
- The player arm's landing block (`!rmState.Body.InContact` hard-snap +
return) must not claim a teleport-classified packet: the teleport
classification is routed before it (or the block is gated to classifications
it owns — implementer's choice, pinned outcome: a teleport-classified
packet always reaches the teleport arm regardless of wire or body contact).
- The 4a `IsAirborneNoOperation` early returns are classification-gated
already and cannot claim a `SetPosition` route — unchanged.
- **The NPC arm's TS-44 sticky suppression (`snapSuppressedByStick`) does not
suppress the teleport arm.** Retail's sticky cannot survive a teleport —
`UnStick` is the hook's second action. The suppression remains exactly as
it is for the near/far/leftover arms (its register row describes an
NPC-only steady-state gate, which stays true).
### D6 — the two per-packet gates widen to the teleport arm, same rule as the far arm
- `TryApplyGenericRemoteRenderPose`: the gate stays `OwnsSteadyState` — the
teleport arm (like the far arm) takes the early wire-pose write, and its
tail re-syncs the render entity from the resolved body (invariant 2). This
resolves the standing "route 4b-3 revisits the gate" comment: the answer is
"unchanged, now stated"; delete the forward reference.
- `TryAdoptWireCellAfterRouting`: the suppression (currently
`arm is FarSnapPlacement`) widens to the teleport arm, for the same reason —
after a canonical placement the placement is the cell authority; retail
resolves the destination cell through `AdjustPosition`/`set_cell` and
nothing writes the wire cell over it.
### D7 — the post-placement currency guard covers the teleport arm; consolidation is sanctioned
Both arms' re-validation ("the far arm is re-entrant — re-validate position
ownership on EVERY placement status before writing anything else, arming
included") widens to `Arm is FarSnapPlacement or <TeleportArm>`. Round 2's
"do not churn" explicitly named for 4b-3 that this guard "belongs behind a
testable Runtime seam rather than duplicated at two App call sites" — moving
the duplicated guard into the Runtime seam is sanctioned in this slice, but
only if the two arms' observable ordering (guard before arm, both arms
identical — the R5 invariant) is preserved and pinned by test.
### D8 — register bookkeeping, in the implementation commit
- **AP-137 is REWRITTEN, not deleted.** Its cell-less enqueue-vs-place delta
(part R2) retires — that is this slice's headline. But the row also records
the two surviving acdream-only states (null classification during the login
window; `RejectedData`/`RejectedAuthority` applied through
`UnroutedCatchUp`), which have no retail mechanism and therefore keep a
row. Rewrite the row to exactly the survivors plus D1's visibility-arm
deletion and D2's wire-airborne leftover shape. (The handoff says "row
deletion"; a deletion that silently dropped the surviving divergences would
violate register rule 1 — this contract overrides that wording, and the
summary reports the contradiction.)
- **AD-42 must be updated**: it cites
`src/AcDream.App/Physics/RemoteTeleportController.cs (ResolvePlacement)` as
a surviving two-call enter-world split path. That citation dies with the
class; the headless portal-arrival resync and
`PhysicsEngine.ResolvePlacement` citations remain.
- **AP-136 / AP-138 (D5 scoping text)**: both name "`RemoteTeleportController`'s
rollback" as the shipped writer that can rebucket `record.FullCellId` to a
third landblock under a retained retry. That writer is deleted; the
surviving non-Position rebucket writers are the projection materializer
(`DatLiveEntityProjectionMaterializer`) and the equipped-child renderer
(`EquippedChildRenderController.TickChild`). Update both rows and the same
claim inside `RuntimeSetPositionState`'s `CurrentCellId` doc and
`RuntimeRemotePlacementDriveController.CanAttemptDestination`'s doc
(process rule 6).
- **AP-138's Risk column gains the teleport arm as a second producer** of the
visible-without-collision residual: a remote that teleports into a
non-published landblock and stands still is exactly the AP-136/AP-138
shape, now reachable through this arm. No new machinery — the row's
retirement path is already #309.
- **AP-135 is untouched.**
## Deletion inventory — every file, wiring site, and test
Files deleted (739 production lines):
| file | lines |
|---|---|
| `src/AcDream.App/Physics/RemoteTeleportController.cs` | 605 |
| `src/AcDream.App/Physics/RemoteTeleportPlacement.cs` | 85 |
| `src/AcDream.App/Physics/RemoteShadowPlacementSynchronizer.cs` (contains BOTH `RemoteShadowPlacementSynchronizer` and `RemoteTeleportPlacementPresentation`) | 49 |
`src/AcDream.App/Physics/RemoteTeleportHook.cs` (57) is NOT deleted — it is
the retail `teleport_hook` port and moves/re-wires per D3.
Tests deleted (1,709 lines):
| file | lines |
|---|---|
| `tests/AcDream.App.Tests/Physics/RemoteTeleportControllerTests.cs` | 1,515 |
| `tests/AcDream.App.Tests/Physics/RemoteTeleportPlacementTests.cs` | 194 |
`tests/AcDream.App.Tests/Physics/RemoteTeleportHookTests.cs` (38) moves with
the hook.
Wiring sites (the handoff's list of eight was a raw grep; the true set is
below — two sites the handoff missed, and three of its entries are
comment-only):
| site | what happens |
|---|---|
| `src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs` | The heart of the slice. Delete: ctor param + `_remoteTeleportController` field; `remoteHardTeleport` / `remotePlacementRequired`; `RunRemoteTeleportHook` (moves per D3); the `BeginPlacement` call; the whole `remotePlacementRequired` placement block (the `TryApply` call and its `Applied`/`Superseded` tails); the classification gate's `&& !remotePlacementRequired`; the legacy `!update.IsGrounded` fallback (D2); the legacy pre-operation `ConstrainTo` call (D4). Add: the teleport arm dispatch (D5), widened guards (D6/D7). |
| `src/AcDream.App/Composition/LivePresentationComposition.cs` | Delete the `RemoteShadowPlacementSynchronizer` + `RemoteTeleportPlacementPresentation` constructions, the `remoteTeleportLease` acquisition, the `RemoteTeleport` record member, and the lease parameter threading. |
| `src/AcDream.App/Composition/SessionPlayerComposition.cs` | Three `live.RemoteTeleport` pass-throughs (network-update controller ctor, teardown controller ctor, and the third composition site) — replaced by nothing; the drive controller is already threaded. **Missing from the handoff's list.** |
| `src/AcDream.App/Net/LiveSessionRuntimeFactory.cs` | Delete the `RemoteTeleport` record member and the reset-plan binding `RemoteTeleport = _world.RemoteTeleport.Clear`. |
| `src/AcDream.App/Net/LiveSessionResetManifest.cs` | Delete the `required Action RemoteTeleport` member and its `new("remote teleport", …)` stage. **Missing from the handoff's list.** Reset coverage is not lost: the drive controller's teardown is `DetachRoute` (cancels every live operation) and its ledger convergence is already asserted. |
| `src/AcDream.App/Rendering/GameWindow.cs` | Delete the `_remoteTeleportController` field and its assignment from the composition result. |
| `src/AcDream.App/Rendering/GameWindowLifetime.cs` | Delete the `RemoteTeleportController? RemoteTeleport` shutdown-root member and the `Hard("remote teleport", …)` stage. |
| `src/AcDream.App/World/LiveEntityRuntimeTeardownController.cs` | Delete the ctor param, field, and the `_remoteTeleport.Forget(record)` cleanup entry. Per-entity teardown coverage is not lost: the drive controller self-heals on `IsPlacementCurrent` and `Forget`-on-accepted-Position retires operations. |
| `src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs` | **Comment-only** (two doc comments citing `RemoteTeleportPlacement.Apply` — the isCurrent-delegate note and the five-writer `Airborne` list, which becomes a four-writer list; the teleport path's `Airborne` derivation is now the canonical placement commit's, already on the list). Correct both (process rule 6). |
| `src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs` | **Comment-only** (the `CurrentCellId` retained-retry doc naming `RemoteTeleportController`'s rollback — D8). No code change; there is no teleport-hook machinery in this file to touch. |
| `src/AcDream.Core/Physics/EntityCollisionFlags.cs` | **Comment-only** (TS-23 history note naming the pre-P3 inlined call sites). Historical statement — rewrite to past tense or leave verifiably historical; do not let it read as a live citation. |
| `src/AcDream.Runtime/Session/RuntimeRemotePlacementDriveController.cs` | Widen `OwnsPlacement`'s scope comment (it ALREADY matches `SetPosition` + `Teleport` flag — no predicate change needed for the teleport disposition); add the teleport-arm entry point (D3); correct the `Advance()`/`CanAttemptDestination` docs' writer list (D8). |
| `src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs` + `src/AcDream.App/World/LiveEntityRuntime.cs` + `src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs` + `src/AcDream.Runtime/Entities/InboundPhysicsStateController.cs` | Delete the `TeleportHookRequired` field, its `timestamps with {…}` computation, the `projectionRequiresTeleportHook` parameter/computation/`false` argument, and thread the pre-merge cell instead (D1). One doc comment in `InboundPhysicsStateController` cites `TeleportHookRequired`-adjacent bookkeeping — reword. |
| `src/AcDream.App/World/LiveEntityPresentationController.cs` | `BeginAuthoritativePlacement` and `CompleteAuthoritativePlacement(deferShadowRestore: false)` lose their only production callers (`DeferShadowRestore`, `HasActivePlacement`, `HasDeferredShadowRestore` already have none), leaving `_activePlacementOwners` write-never while `IsPlacementActive` still reads it in the visibility suspend/restore gates. Delete the dead half in this commit — do not leave a zombie set that silently gates nothing — but trace the `IsPlacementActive` consumers first and state in the commit what each gate degenerates to. If tracing shows a live non-teleport dependency, STOP and report rather than deleting blind. |
Test files updated (not deleted) — each references the deleted machinery
incidentally: `UpdateFrameOrchestratorTests` (a `typeof(RemoteTeleportPlacementPresentation)`
row), `RuntimeEntityOwnershipTests` (two `typeof(RemoteTeleportController)`
exact-key assertions), `GameWindowLiveEntityCompositionTests`
(`[InlineData("RunRemoteTeleportHook")]`), `LiveEntityLifecycleStressTests`
(constructs the controller and calls `TryApply` — its scenario must be
re-expressed against the canonical teleport arm, not dropped),
`LiveSessionResetPlanTests` (the "remote teleport" stage),
`CurrentGameRuntimeAdapterTests` (a noop binding),
`RuntimeInitialCreateResidenceStateTests`
(`RemoteTeleportSuffixIsQueuedBehindInitialAdmission` — verify what it pins;
it is about suffix ordering under initial admission and likely survives with
a rename), plus the eight files matching `TeleportHookRequired`.
Stale comments this slice must make true (process rule 6 — verify each
against the code beside it, and prefer symbol references): the
`// 4b deletes this fallback` line (D2 deletes the fallback); the
`// 4b-3 deletes it` on the legacy arm site (D4); the `remotePlacementRequired
guarantees the classifier's teleport disposition never reaches here` route
comment; `ClassifyRemoteAcceptedPosition`'s caller-doc sentence "whose
remotePlacementRequired gate is already false"; `SeedRemoteSpawnPlacement`'s
"Mirrors `RemoteTeleportPlacement`'s commit"; the AP-140 comment's citation of
`RemoteTeleportPlacementTests.Apply_PendingGroundToSteepContact_…` as an
`Airborne`-definition dependent (the test dies; the dependency list shrinks);
the classifier's "only a later Runtime SetPosition or simulation commit may
change FullCellId" (D1 shows it false); `TryApplyGenericRemoteRenderPose`'s
"Route 4b-3 revisits the gate" (D6 resolves it).
## Proof obligations (must prove, not assume)
1. **`ParkCollisionResidents`'s overlap throw stays unreachable.** The
argument is 4b-1/4b-2's, extended: every operation this route begins goes
through `TryBeginExclusiveAuthoredPlacement` (one live operation per key —
the Begin refuses a second), `DeferredCell` outcomes are cancelled
synchronously (no park survives the controller), and retained entries are
preparation retries bounded by the pre-flight re-check and the `Advance()`
window-drop. The teleport arm adds packets to the same machinery, not a
new operation shape. State this in the contract-conformance section of the
implementation commit and keep the honest caveat 4b-1's B2 established:
the guarded property is `TryAcquireCollisionPrefixMutationPermission`'s
`HasOldPrefixPlacementDebt` refusal (a stall, not a throw), and the
ledger convergence tests are the floor under it.
2. **Ledger convergence with teleports in flight**: teardown, session reset,
and generation change converge `RemotePlacementDrivePendingCount` (both
registrations) to zero with retained teleport retries present — the same
suite shape 4b-1 built, driven through the new arm.
3. **The one-arm partition (D4)**: a test that counts arming calls per packet
across the partition table's rows — exactly one for every "yes" row,
exactly zero for every "no" row. The 4b-2 lesson (R7): assert the
observable (the arm count / the anchor), not the code shape.
## What this slice does NOT do
- **AP-131** (shared merge call) — C5.
- **AP-135** — stays, writes preserved (invariant 8).
- **#276** — `SeedRemoteSpawnPlacement` is still not classification-gated;
the AD-61 settle is untouched.
- **#309 / AP-136 / AP-138 residuals** — the lost-cell/hidden-until-cell-load
behaviour is not built here; the teleport arm inherits the far arm's
store-and-stay-visible residual and the register rows say so (D8).
- **Route 5 (projectile)** — `OwnsPlacement` keeps excluding
`ProjectileAuthoritative`; no widening here.
- **No headless remote consumer** — `RuntimeLiveEntitySessionController`
still returns early for non-local GUIDs; the vacuous-satisfaction statement
stays in the interface doc and the AP-137 rewrite.
- **No changes to route 2's local-player ForcePosition/teleport paths**, the
local-player teleport transit (`LocalPlayerTeleportController` keeps its own
`NotifyTeleported` call), or route 1's executor.
- **The recorded-not-consumed route facts stay recorded-not-consumed**:
`UnparentBeforeRouting` / `ApplyPlacementFrameBeforeRouting` have no reader
today (the unparent edge is owned by the merge's `EndChildProjection` and
the hydration recovery); this slice adds no reader and does not delete the
facts.
## Test plan
Tests must assert the layer that broke historically (process rule 4 —
presentation/visibility, not only `InWorld`/clock/residency), and every new
test must fail against a broken implementation (no source-text pins, no
tautologies).
Focused Runtime tests (`tests/AcDream.Runtime.Tests`):
1. Teleport-classified commit: body at resolved destination, `FullCellId` =
resolved cell, hook ran (moveto cancelled, stick released, interp queue
empty, leash re-armed post-operation at the post-move anchor), clock
active, `InWorld`.
2. Teleport refused (destination outside the service window): pose STILL
advances to the accepted destination (invariant 1), no park opened, no
operation retained, entity `InWorld` + clock active + spatial root intact,
AND the hook still ran (retail runs it before the placement decision).
3. Teleport `RejectedByPlacement` (engine refused): pose does NOT move;
`Cancelled`-after-commit: settled pose survives. (The far arm's tests
exist; these drive the teleport arm through the same partition.)
4. Cell-less classification now fires: a record whose PRE-merge committed
cell is 0 (unwield-to-3D shape) classifies `SetPosition` and places
unconditionally — the AP-137-retiring behaviour. Companion: the same
packet with a nonzero pre-merge cell and no TELEPORT_TS advance does NOT
classify `SetPosition` (proves D1's input is the pre-merge value, not the
post-merge wire cell — this is the test that discriminates the fix from
the shipped dead predicate).
5. D4 partition: arm-count table test (proof obligation 3), including the
wire-airborne null/`Rejected*` no-arm rows and the
teleport-arm-on-every-outcome row.
6. D5 ordering: an airborne-body teleport packet places (does not
`AirborneSnap`, does not take the landing block); a stuck NPC's teleport
packet runs the hook (`UnStick`) and places despite TS-44's suppression.
7. Currency: teleport arm's synchronous receipt deletes/replaces the
incarnation → nothing further written for the packet (guard before arm,
both arms — the R5 shape, now for the teleport arm).
8. Ledger/teardown: proof obligation 2.
App-layer tests (`tests/AcDream.App.Tests`):
9. **The presentation assertion (#312's layer):** after a remote teleport
commit, the render `WorldEntity` pose equals the resolved body pose,
`ParentCellId` equals the resolved cell, the entity is spatially visible,
and the collision shadow was published. After a refused teleport, the
render pose tracks the stored destination and the entity REMAINS visible.
10. D2's leftover-airborne shape: wire-airborne null-classified packet writes
exactly AP-135's two fields and nothing else (body, entity, queue, leash
all untouched).
11. AP-135 preservation on the rewritten arms (both airborne no-op paths).
12. The generic render-pose + wire-cell-adoption gates: teleport arm takes
the early write and suppresses the post-routing wire-cell adopt (D6) —
asserted through the existing extracted entry points, not restated
logic.
Live-execution proof (process rule 5): a `[remote-teleport]` probe line —
`PhysicsDiagnostics`-owned, `ACDREAM_PROBE_REMOTE_TELEPORT=1`, one line per
routed teleport arm with guid, cause (`teleport-ts` vs `cellless`), hook-ran,
and placement status, marked TEMPORARY with the existing probe family. The
connected gate below is recorded as a pass ONLY if the probe line shows the
new arm executed (a clean-looking session with zero probe lines is a
not-run, exactly like #309's park probe).
## Gates
- Focused Runtime + App tests above.
- Complete Release suite: `$env:ACDREAM_PAK_PATH` set,
`dotnet test AcDream.slnx -c Release -m:1`. **Baseline 11,027 passed / 4
skipped / 0 failed** at `2eb39a02`. The net count will move (1,709 test
lines deleted, new tests added) — measure and record the new figure; do not
inherit 11,027 as the expectation. Two known flakes, do not chase and do
NOT conflate: **#302** (`PortalProjectionTests.ClipToRegion_FrameOwnedStore_…`,
GC-allocation assertion, App.Tests) and **#308**
(`NakEmissionTests.LossSoak_…`, wall-clock deadline, Core.Net.Tests,
full-suite load only). If either appears, re-run and say which.
- **Two-client connected teleport gate (user-run).** **CORRECTION (2026-08-04
fix round, both independent Opus reviews):** the recipe below originally
specified "a second character" as the teleport target. That is WRONG — a
player-character target cannot exercise this arm's NPC/creature code path
at all, and two of the three MAJOR defects the fix round found (A1's
zero-arm leash regression, A2/R3's synthesized-velocity/run-cycle defect)
are BOTH on the NPC-guid branch only; `RemoteServerControlledVelocityCycle.Apply`
itself early-returns for any `0x50xxxxxx` guid, so a player-target run
would report a clean pass while both defects shipped underneath it. **The
target MUST be an NPC/creature** (an ACE admin teleport — `@teleto` /
`@teleloc` — applied to a drudge/mosswart/etc., NOT a second player
character) for the gate to see what it is supposed to see. Recipe: acdream
stands as observer; the creature target is teleported with an ACE admin
teleport (`@teleto` / `@teleloc`) — those route through `Teleport()` /
`SendUpdatePosition(true)` and advance **ObjectTeleport** (TELEPORT_TS),
which is exactly this arm's trigger on the observer (`@pklite` is route
2's ForcePosition lever, NOT this gate — see
[`2026-08-03-c4-route-2-visual-gate.md`](2026-08-03-c4-route-2-visual-gate.md)).
Run with `ACDREAM_PROBE_REMOTE_TELEPORT=1`. Correct: the observed creature
vanishes from the old spot and appears at the destination in one step (no
glide, no interpolated streak), **stands STILL with correct idle
animation — NOT sprinting/running in place** (A2/R3's specific symptom:
a synthesized teleport-distance velocity planning a RunForward cycle at
the destination), and moves normally afterward (leash re-armed: no
rubber-band, no tether — A1's specific symptom is the ABSENCE of a
rubber-band/re-anchor where one should exist, since a creature that was
knocked airborne on its first accepted Position after this teleport would
otherwise never arm at all). Also teleport the remote OUT of view and
back: it must re-appear correctly. Regressions to watch: a remote gliding
across the map at teleport (queue not cleared), freezing at the old spot
(the 4b-2 round-1 freeze class), invisible-but-audible at the destination
(#312 class), invisible-but-solid (#184 class), a rubber-band after
arrival, or the creature sprinting/running in place at the destination
(A2/R3). Confirm at least one `[remote-teleport]` line per teleport, with
the expected cause. Graceful close (ACE session-clear) per the standing
rule.
## Budget
**~400-700 non-comment production lines** (net LOC strongly negative — the
deletions are 739 production + 1,709 test lines). For calibration: 4a was
364, 4b-1 was 230+57, 4b-2 landed within its 350-500. Exceed 700 new lines
and STOP and report rather than pushing through.
## Open questions routed to the retail-conformance reviewer
1. **D1's evidence chain** (the merge stamps `FullCellId` with the wire cell
before classification, making the shipped cell-less predicate dead) rests
on the reading `TryApplyPosition` → `RefreshSnapshot(refreshPosition:
acceptedPosition)` → `RefreshDerivedState` → `SetFullCell(wire cell)`,
with classification after the gate and before the prologue rebucket.
Verify independently — it is the load-bearing claim of this contract, and
if it is wrong the D1 plumbing is unnecessary churn.
2. **`report_collision_end(this, 1)` ↔ `ShadowObjects.Suspend(localEntityId)`**:
the existing hook maps retail's collision-end report to a broadphase
shadow suspension. This mapping predates 4b-3 and is carried, not
re-derived. Confirm against @0x00514F31's callee that the "1" argument
(report-to-partners) has no unported half, or file the delta on the
AP-137 successor row.
3. **The `LiveEntityPresentationController._activePlacementOwners` deletion**
(deletion-inventory last row): confirm by reading `IsPlacementActive`'s
two consumers that removing the write-never set cannot change a
Hidden/UnHide or visibility-edge restore for non-teleport entities.
4. **`RemoteTeleportSuffixIsQueuedBehindInitialAdmission`**
(`RuntimeInitialCreateResidenceStateTests`): confirm what it pins and
that it survives (renamed) rather than being deleted as collateral.
## Contradictions with the handoff/scoping docs — reported, not smoothed over
- **The handoff's "AP-137 row deletion belongs in the implementation
commit"** conflicts with the row's own content: null and `Rejected*` are
acdream-only divergences that survive this slice and must keep a row.
Pinned as rewrite-in-place (D8). If the reviewer prefers
delete-and-refile-narrow, either satisfies register rule 1; silent whole-row
deletion does not.
- **The handoff's wiring-site list (8 sites) is a raw grep, not a wiring
list**: `SessionPlayerComposition.cs` and `LiveSessionResetManifest.cs` are
real wiring sites it missed; `RuntimeRemotePhysicsUpdater.cs`,
`RuntimeSetPositionState.cs`, and `EntityCollisionFlags.cs` are
comment-only.
- **The handoff's "`RuntimeSetPositionState.cs` — the teleport-hook phase
support and SetFlags analog"**: there is no teleport-hook phase support in
that file. The phase lives on the classifier route
(`RuntimeTeleportHookPhase`), and the SetFlags analog is the `flags`
parameter of `TryPrepareAndSubmitAuthoredPlacement` fed from
`route.SetPositionFlags`.
- **The scoping doc's T1 ("they disagree in both directions")** understates
the finding: as fed today the classifier's cell-less predicate is not
merely different — it is unreachable for remote PositionEvents (D1). The
design decision is therefore not "reconcile two predicates" but "feed the
classifier retail's predicate at all".
- **The scoping doc's T2** describes the pre-4b-2 tree; 4b-2 already
established the null/`Rejected*` policy. What 4b-3 actually decides is the
wire-airborne leftover shape (D2), which neither doc names.

352
docs/research/2026-08-04-c4-route-4b-3-retail-review-round2.md Normal file
View file

@ -0,0 +1,352 @@
# C4 route 4b-3 — retail-conformance review, round 2 (delta) — 2026-08-04
Delta review of the round-1 fix pass, same working tree, still uncommitted.
Round 1: [`2026-08-04-c4-route-4b-3-retail-review.md`](2026-08-04-c4-route-4b-3-retail-review.md).
Adversarial review: [`2026-08-04-c4-route-4b-3-architecture-review.md`](2026-08-04-c4-route-4b-3-architecture-review.md).
Scope: the deltas only. Everything round 1 verified sound and did not flag
(the retail address set, D1's evidence chain, D3's hook order, the
`SetFlags(0x1012)` mapping, D5's routing order, D6/D7, invariants 1/2/4/10/11,
the AD-42/AP-136/AP-138 updates) was re-checked for *disturbance*, not
re-derived.
---
## VERDICT: **PASS**
All three MAJOR findings are fixed, and each fix is retail-correct on the
merits rather than merely symptom-suppressing:
- **R1/A7** — the D2 return-0 shape is now shared by both branches, so
retail's "no `this == player` distinction" holds on this shape for the first
time. AP-137's claim is now true.
- **R2** — the hook's sixth action now routes to the *actual* port of
@0x00514620, and `LeaveWorld`'s retained-peer / retained-environment
semantics match retail's arg2=1 behaviour exactly (verified below, since
that was the remaining open question).
- **R3/A2** — the synthesized velocity and the cycle plan are both excluded
for a teleport-classified route, and the exclusion is *positive* rather than
relying on the incidental moveto guard the hook itself destroys.
- **A1** — `ToConstraintArm`'s `AirborneSnap` mapping is now an arming value.
**I scrutinized this hardest per the coordinator's request and it is
correct** — see §1.
Two of my round-1 MINORs (R7, R9) were fixed by rewriting the comments rather
than the code, which is the right call in both cases. R4, R5, R6, R8 are
genuinely fixed. No new retail divergence introduced by any fix. No fix
disturbed anything round 1 had verified.
Gates observed: `dotnet build AcDream.slnx -c Release` **0 warnings / 0 errors**;
`AcDream.Runtime.Tests` **1,125 / 0 skipped / 0 failed**; `AcDream.App.Tests`
**4,081 / 3 skipped / 0 failed**. Per process rule 3 none of this is treated as
conformance evidence; the discriminating power of the three new tests is
assessed separately in §5.
---
## 1. A1's fix — is `NearInterpolate` the retail-correct arm for `AirborneSnap`?
**Yes, and for the right reason.** The coordinator asked me to check the
mapping's *whole partition* against "retail returned nonzero", not the arm
label. That is exactly the right framing, because retail's arming is not
per-arm at all — `ConstrainTo` @0x00454272 sits inside
`if (MoveOrTeleport(...) != 0)` @0x00454254, one site, and every branch of
`MoveOrTeleport` that does anything returns 1.
`TryArmConstraintAfterOperation` consumes the arm value **only** as an
arms/doesn't-arm discriminator (it is a `switch` yielding `bool`), so the label
is cosmetic and the only question that matters is which side of the partition
each `RemoteContactArm` lands on. Re-derived:
| `RemoteContactArm` | reachable when | retail's `MoveOrTeleport` | must arm? | maps to | arms? |
|---|---|---|---|---|---|
| `TeleportPlacement` | teleport/cell-less classification | branch @0x00516386, `return 1` @0x00516438 | yes | `TeleportPlacement` | ✓ |
| `FarSnapPlacement` | `SetPositionSimple`, ≥96 m | far branch, `return 1` @0x005163E8 | yes | `FarSnapPlacement` | ✓ |
| `SteadyStateInterpolate` | `Interpolate`, <96 m | near branch, `return 1` @0x005163BE | yes | `NearInterpolate` | ✓ |
| `AirborneSnap` | **wire-grounded** packet whose *body* lacks a contact plane | near or far branch (the wire bit is set, so `arg4 != 0`), `return 1` | **yes** | `NearInterpolate` | ✓ **(the fix)** |
| `UnroutedCatchUp` | null/`Rejected*`, wire-grounded | no retail state; analogue is nonzero | yes | `UnroutedCatchUp` | ✓ |
| retail's `arg4 == 0` | — | `return 0` @0x0051636D | no | *not expressible* → `throw` | n/a |
The load-bearing claim is the fourth row's "wire-grounded". I verified it holds
on **both** branches after the fix, by enumerating every classification that
can reach `ApplyRemoteContactRouting`:
- A wire-airborne packet can only classify `NoPositionOperation` (classifier's
`!effectiveContact` branch, `RuntimeAuthoritativePositionRouteClassifier.cs:430-448`),
`null`, `RejectedAuthority`/`RejectedData`, or `SetPosition` (the teleport
branch, which precedes the contact test). `Interpolate` and
`SetPositionSimple` are unreachable with a clear wire bit. The
`SameIncarnationCreate` `effectiveContact` override cannot apply here — this
path always passes `RuntimeAcceptedPositionSource.PositionEvent`
(`RuntimeEntityObjectLifetime.cs:643`).
- `NoPositionOperation` returns at each branch's `IsAirborneNoOperation` gate
(player `:2313`, NPC `:2692`).
- `SetPosition` dispatches ahead of the carve-out (player `:2330`, and inside
`ApplyRemoteContactRouting` itself for the NPC path).
- `null`/`Rejected*` now return at each branch's D2 call (player `:2370`,
**NPC `:2717`, the R1 fix** — and this is precisely why the fix had to land
for the mapping to be sound).
So after the fix, **nothing wire-airborne reaches the carve-out**, and
`AirborneSnap` implies `arg4 != 0` implies retail returned nonzero implies
retail armed. The mapping's partition is exactly "retail returned nonzero", and
the one case it cannot express (retail's true `arg4 == 0` no-op) is the one the
throwing default now enforces as unreachable rather than silently absorbing.
Two supporting observations:
- The `NearInterpolate` *label* is semantically loose for a hard snap, but the
justification given ("the same arming value the player arm's own LANDING
TRANSITION block already uses explicitly for the identical scenario —
grounded wire, body not in contact") is accurate: `:2469-2471` hard-codes
`NearInterpolate` for exactly that packet shape. Consistency between the two
sites is worth more here than a new enum member, and the doc block at
`:1454-1480` states the reasoning rather than asserting the conclusion.
- The A1 fix is *dependent on* the R1 fix. Had R1 been declined ("scope the
AP-137 row to the player arm" — the option the architecture review offered
at A7), `AirborneSnap` would still be reachable wire-airborne on the NPC arm
and mapping it to an arming value would have produced the **opposite**
divergence (arming where retail returns 0). The two fixes are only jointly
correct. Worth noting in the commit message so a future revert of one does
not silently invert the other.
---
## 2. R2's fix — `LeaveWorld` vs retail @0x00514620 with `arg2 = 1`
Per instruction I did not re-litigate the `LeaveWorld`-vs-`ForceEnd` choice
(the coordinator's reasoning about `_admissionBlocked` is right, and `ForceEnd`
is private). The remaining open question was whether `LeaveWorld`'s stated
retained semantics — *"Incoming peer records and the environment latch are
intentionally retained"* — match retail. **They do, on both halves.**
Re-read of `report_collision_end` @0x00514620 (line 282526ff) and
`report_object_collision_end` @0x00510A90 (line 278590ff):
| retail behaviour | acdream `LeaveWorld` → `ForceEnd` → `EndExpiredObjectCollisions(force: true)` | match |
|---|---|---|
| Iterates **only** `this->collision_table` (the owner's own table) | `_owners[ownerKey].Order`/`.Records` only (`:1115-1141`) | ✓ |
| `arg2 != 0` bypasses both the >1 s staleness test @0x005146D2 and the still-touching test @0x005146DA (`goto label_514706` in each) | `if (!force && !(age > 1d) && !(collision.Ethereal && age > 0d)) continue;` — `force` short-circuits both (`:1132-1137`) | ✓ |
| Deletes the complete selected set **before** the first callback (`DeleteCurrent` in the walk, callbacks in the tail loop @0x0051474D) | "Retail deletes the complete expired set before issuing any end callback" — precommit loop at `:1149-1155` ahead of the callback loop at `:1164` | ✓ |
| Fires `DoCollisionEnd` on **both** weenies, each gated on its own `state & 8` (REPORT_COLLISIONS) @0x00510AC8 / @0x00510AE0 | `PublishResolvedObjectEnd` publishes owner→target and target→owner, each gated on that side's `PhysicsStateFlags.ReportCollisions` (`:1234-1268`) | ✓ |
| Does **not** touch the partner's own `collision_table` — the partner's record of `this` survives to its own expiry/force pass | only `RemoveReverseOwner(collision.Key, ownerKey)` (`:1154`), which prunes the Runtime-side `_ownersByPeer` **index**, not the peer's `Records` | ✓ ("incoming peer records … retained") |
| Does **not** read or write `colliding_with_environment` — that is `handle_all_collisions`' business @0x00514800 | `CollidingWithEnvironment` is untouched by `ForceEnd`/`EndExpiredObjectCollisions`; only `HandleReports` (`:847-859`) writes it | ✓ ("the environment latch … retained") |
The doc sentence is therefore not a caveat to be excused — it is a precise
statement of retail's own scoping. **No residual delta; nothing owed to the
register.** Correspondingly, AP-137 carrying no `report_collision_end` row is
now correct (round 1's requirement was "confirm *or* file"; the fix confirms).
**Dropping the `ShadowObjects.Suspend` side is also right, and safe.** Retail's
`teleport_hook` does not remove shadows; `SetPositionInternal` does that itself
as part of the commit. The strongest argument for safety is one the doc does
not make explicitly and should: **the far arm has never suspended either**, and
it reaches the engine through the identical `TryExecuteAcceptedRemotePosition`
→ `SubmitAndResolve` path. So after this fix the teleport arm's shadow handling
is byte-identical to the far arm's, which has been through two live gates. The
frames-scale intangibility window round 1 flagged is closed by construction
rather than by compensation.
---
## 3. R1's and R3's fixes
**R1 — `ApplyWireAirborneLeftoverBookkeeping`, both branches.** Retail-correct:
`arg4 == 0` → `return 0` @0x0051636D writes nothing, and the two writes that
remain are AP-135's acdream-only free-fall-sweep bookkeeping, whose row is
untouched. The NPC call site (`:2717-2722`) is correctly placed *after* the
`IsAirborneNoOperation` return and *before* the synth-velocity block and the
sticky gate, so a wire-airborne leftover NPC packet no longer reaches the
free-flight carve-out, the arming site, or the render/shadow publish. **AP-137's
"unifies player and NPC remotes on one behaviour" sentence is now TRUE** — I
re-read the row and traced both call sites.
One side effect worth stating and not a defect: the NPC arm's synth-velocity
install no longer runs for wire-airborne leftover packets (they return above
it). That moves *toward* retail, which writes no velocity on the `return 0`
branch.
**R3/A2 — the `!isTeleportRoute` gates.** Both halves are gated (install
`:2741`, cycle apply `:2886`), and the gate is positive rather than relying on
`rm.MoveTo`, which the hook's own `CancelMoveTo` invalidates — the reasoning in
the comment at `:2724-2740` is correct and is the non-obvious part. Retail's
teleport branch writes no velocity (verified round 1: `set_velocity` @0x004541B4
is inside the *local player's* branch only), so this is the faithful shape.
`rmState.LastServerPos` is advanced to the destination at `:2884`, after
routing, so the *next* packet's synthesis no longer spans the teleport jump —
the comment claims this and it holds.
Non-finding, checked because it looked like one: a stale `HasServerVelocity`
from a pre-teleport packet survives the teleport packet, but nothing consumes
it into a cycle in that window (`RuntimeRemotePhysicsUpdater.cs:198-208` only
*zeroes* it, and `RemoteServerControlledVelocityCycle.Apply`'s only other
caller is that zeroing path). And a creature that was mid-run when teleported
keeps its current cycle — which is also what retail does, since `teleport_hook`
cancels the moveto but never calls `SetCycle`.
---
## 4. The two new shared helpers — per-call-site behaviour delta
Checked each site against the code it replaced.
**`ApplyWireAirborneLeftoverBookkeeping` (2 sites).**
- Player `:2390` — previously wrote `LastServerPos`/`LastServerPosTime` only;
now also `remote.CellId = wireCellId`. Verified genuinely redundant: this arm
already wrote `rmState.CellId = p.LandblockId` unconditionally at `:2251`,
and the argument passed is the same `p.LandblockId`. **No behaviour change.**
The comment at `:2386-2389` states the redundancy, which is the honest
framing.
- NPC `:2719` — new, and the intended R1 fix.
**`RunRemoteArmTail` (3 sites).**
- Player teleport `:2333` — the old inline guard was *unconditional*; the
helper's is gated on `FarSnapPlacement or TeleportPlacement`. Equivalent
here, because the caller has already tested `OwnsTeleportPlacement(route)`
and `ApplyRemoteContactRouting` tests the same predicate first, so the arm is
always `TeleportPlacement`. **No behaviour change.**
- Player grounded `:2566` — old guard was `FarSnapPlacement` only; the helper
adds `TeleportPlacement`, which is unreachable at this site (dispatched and
returned above). **No behaviour change.**
- NPC `:2818` — old guard was already `FarSnapPlacement or TeleportPlacement`.
**Character-identical.**
All three previously computed `willBeDrTicked` as
`WillAdvanceRemoteMotion(update.Guid, rmState)`, which the helper reproduces,
and all three retain guard-before-arm (the R5 invariant). The only intended
delta is the hook closure now capturing `canonical`/`remote` instead of
`update.Guid`/`entity.Id` — which is R6's fix, and is a strict improvement:
`RunRemoteTeleportHook` no longer re-resolves the `RemoteMotion` by GUID, so
the six actions operate on the same `rmState` the placement does, and
`hookRan=True` can no longer be printed for a hook that silently no-opped.
**One incidental simplification I checked separately because it was not flagged
by either review:** the NPC velocity block's `!IsPlayerGuid(update.Guid)`
guards (both the synth condition and the `else if` zeroing branch) were dropped
when the block was wrapped in `if (!isTeleportRoute)`. This is safe — the block
sits below `if (IsPlayerGuid(update.Guid)) { … return; }`, and I confirmed
every path inside that block returns (`:2316`, `:2367`, `:2396`, `:2541`,
`:2668`), so `IsPlayerGuid` is unconditionally false there. **No behaviour
change**, but it is an unremarked edit inside a fix hunk; worth a line in the
commit message.
---
## 5. Test discrimination
The three new App tests target the three MAJORs and each one fails against the
pre-fix code by construction, not by coincidence:
- `NpcAirborneSnap_LandingPacket_StillArmsTheLeash` — asserts
`host.PositionManager.Constraint` is null before and non-null after. The only
path that creates it for this packet is
`TryArmConstraintAfterOperation(ToConstraintArm(AirborneSnap))`; the pre-fix
mapping returns `AirborneNoOperation`, which never arms, so the assertion
fails. The packet shape genuinely produces `AirborneSnap` (creature guid,
`teleportSequence` matching the spawn so no TELEPORT_TS advance, wire
grounded, `TransientState = Active` so no contact bit). This is the
end-to-end arm-count coverage proof obligation 3 asked for and round 1 found
missing.
- `NpcTeleport_DoesNotInstallASynthesizedVelocity` — deliberately seeds
`LastServerPos`/`LastServerPosTime` so a broken implementation has a real
distance and interval to synthesize from. Without that seeding it would have
been a degenerate first-packet no-op, i.e. a tautology; the doc says so.
- `NullClassifiedNpc_WireAirbornePacket_WritesOnlyBookkeepingNoBodyOrShadow` —
asserts no body write and exactly one spawn-pose shadow entry. Pre-fix, the
free-flight carve-out hard-snaps the body to `wirePos`, so both fail.
**Minor gap (not blocking):** that last test's doc claims "body, entity, queue,
leash all untouched" but only asserts body and shadow. Adding
`Assert.Null(host.PositionManager.Constraint)` and an interp-queue-depth
assertion would make the test say what its name and doc say. Cheap, and the
leash half is the one D4's partition table calls out as a "no arm" row.
---
## 6. Spot-check of round-1 MINORs
| # | status |
|---|---|
| R4 (rebucket comment claimed it feeds `CommittedCellId`) | **fixed** — `:2036-2038` now reads "commits its canonical FullCellId (the ConstraintDistance cell key)"; the false feedback clause is gone and the surviving clause is true. |
| R5 (architecture docs describe deleted classes as live) | **fixed** — `acdream-architecture.md:467-468` and `code-structure.md:265, 418-419` now record the deletion and name the canonical placement owner. |
| R6 (hook re-resolved by GUID; probe could lie) | **fixed** — `RunRemoteTeleportHook(canonical, remote, isCurrent)` takes the caller's own `rmState`; `host` comes from `remote.Host`, the same bound reference. A null host is now genuinely "the manager doesn't exist yet", matching retail's per-manager guards. |
| R7 (`hookRan` consumed only by the probe) | **fixed as documentation** — `:1103-1108` states the retail justification (@0x005163EF runs regardless of what @0x00516420 yields). Correct call: the behaviour was already right. |
| R8 (undocumented second null-producer) | **fixed** — AP-137 now names the dormant initial-residence enqueue path explicitly, *and* carries my "unverified from static reading" flag forward rather than upgrading it to a claim. That is the right handling of a flagged unknown. |
| R9 (duplicate AP-135 writes / misleading comment) | **fixed as documentation** — the shared helper's doc and the player call site both state the write is redundant there and load-bearing on the NPC arm. |
| A5 (landing-block comment said "always classifies Interpolate") | **fixed** — `:2455-2467` now enumerates the four classifications that reach the block and states why `NearInterpolate` is correct for all of them. Consistent with §1's partition. |
| A8 (dead `wasCellless` local) | **fixed** — the local is gone; the surviving comment at `:1757-1766` refers to `beforeCell`, which still exists. |
---
## 7. The corrected gate recipe
**The correction text is right, and its "why" is right.** I verified both
claims independently:
- `RemoteServerControlledVelocityCycle.Apply` early-returns for
`0x50xxxxxx` guids (`:27-49`), so A2/R3 is structurally invisible to a
player target — and the player arm is doubly immune because its teleport
block returns at `:2367`, above the player synth-velocity at `:2608`.
- A1 is invisible to a player target because `AirborneSnap` is unreachable on
the player arm: the LANDING TRANSITION block returns whenever
`!Body.InContact`, so routing is only ever entered with a body in contact.
The comment at `:2548-2550` states exactly this and is accurate.
The wording is precisely scoped ("cannot exercise this arm's **NPC/creature
code path**"), which matters — a player-target teleport is still a useful run:
it exercises the hook, the placement, invariant 2's presentation sync, the
queue clear, the leash re-arm, and the probe. It is just not sufficient. The
recipe now names the two specific symptoms (sprinting in place; absence of a
re-anchor) rather than only the generic ones, which is what makes it a gate
rather than a look-around.
---
## 8. Judgment on the disclosed-not-fixed items
**Acceptable, all three**, with one filing recommendation.
1. **No dedicated bidirectional collision-partner-notification test for the R2
path — ACCEPTABLE.** The bidirectional publication and the
delete-before-callback ordering are already covered by
`RuntimeCollisionReportingStateTests` against `LeaveWorld` itself
(`:1304`, `:1431`, `:1524`), and §2 verifies the semantics against the
decomp directly. What is genuinely untested is the *wiring* — that the
teleport hook reaches `LeaveWorld` at all. That is a one-line assertion on
the existing teleport fixture (observe the report stream, or assert the
owner's table is empty after the packet) and is worth adding, but its
absence does not put a retail divergence in the tree.
2. **A3, the stress test's hand-written teleport step — ACCEPTABLE as
disclosed.** It is a coverage loss, not a false pass: the Hidden/
DeferredShadowRestore half of that scenario still discriminates, and the
teleport arm itself now has three dedicated App tests plus seven Runtime
ones. Re-expressing it against the canonical arm is a follow-up, not a
blocker.
3. **The per-packet `runTeleportHook` closure — ACCEPTABLE, but file it.** It
is a real regression against `3e002993`, where the inner lambda was
allocated only when `remoteHardTeleport` was true; now a display class plus
delegate is allocated for every remote accepted Position. It is on the
packet path (5-10 Hz per remote), not the physics-resolve path Slice I's
"0 B/resolve" discipline governs, so it does not violate a standing gate.
But it is a one-line fix (cache a `Func<bool>` per controller, or pass the
hook as a method group with the state already in scope) and the next slice
adds a fourth call site. The `teleportStatus.ToString()` half is already
fixed by hoisting the probe guard to the call site (`:1123`).
---
## 9. Still owed (unchanged from round 1, not blocking this verdict)
- The complete Release suite figure the Gates section requires ("measure and
record the new figure; do not inherit 11,027") — the work is still
uncommitted, so there is no commit message carrying it.
- Proof obligation 1's `ParkCollisionResidents` statement (architecture review
A9) — owed to the implementation commit.
- The two-client connected gate, now correctly specified as a **creature**
target.
- The joint dependency between the R1 and A1 fixes (§1) belongs in the commit
message: reverting R1 alone would invert A1's mapping from correct to
wrong-in-the-other-direction.

449
docs/research/2026-08-04-c4-route-4b-3-retail-review.md Normal file
View file

@ -0,0 +1,449 @@
# C4 route 4b-3 — retail-conformance review (2026-08-04)
Reviewer lens: **does this diff do what the retail client does?** Architecture,
style, and layering are a separate reviewer's lane.
Subject: the uncommitted working tree on `claude/acdream-physics-divergence-5aa784`
at HEAD `3e002993` (`git diff HEAD` + four untracked files; the contract
`docs/research/2026-08-04-c4-route-4b-3-contract.md` is itself untracked and is
not part of the change under review).
Build: `dotnet build AcDream.slnx -c Release` — **green, 0 warnings**.
Focused check: `AcDream.Runtime.Tests --filter Teleport` — 29/29 pass.
Per process rule 3, neither is treated as evidence of conformance.
---
## VERDICT: **FAIL**
Three MAJOR findings. R1 and R2 are contract requirements that were pinned and
not met, and R1 additionally ships a **register row asserting behaviour the code
does not have** — the exact defect class process rule 6 exists to stop. R3 is a
newly-reachable retail divergence whose symptom is precisely what the connected
gate recipe lists as its acceptance criterion ("stands with correct animation").
None of the three is hard to fix. The core of the slice — the D1 pre-merge cell,
the arm ordering, the single `ConstrainTo` site, the hook order, the flags — is
**correct and verified against the decomp**. The failures are at the edges the
findings chain keeps warning about: the NPC copy, and an unported half nobody
re-derived.
---
## Part 1 — independent verification of the contract's retail claims
Every address in the contract's "Retail ground truth" section was re-read in
`docs/research/named-retail/acclient_2013_pseudo_c.txt`. **All five load-bearing
facts confirmed.**
| claim | verified |
|---|---|
| `MoveOrTeleport` @0x00516330; branch @0x00516386 `if (eax_8 != 0 \|\| this_1->cell == 0)` | ✓ line 284304ff. `this_1 = this` is assigned @0x00516334 from the incoming `CPhysicsObj*`, so `this_1->cell` **is the body's own current cell**, read at entry, before any placement. The whole D1 design rests on this and it is right. |
| `arg4` is read only @0x0051638E, *after* the branch | ✓ — the teleport branch's `return 1` @0x00516438 executes without `arg4` ever being touched. A teleport/cell-less packet places unconditionally: airborne wire bit, airborne body, any distance. |
| `teleport_hook` @0x005163EF runs BEFORE `SetPosition` @0x00516420 | ✓ |
| `teleport_hook` @0x00514ED0 action list and order | ✓ line 283115ff, exactly: `CancelMoveTo(0x3C)` @0x00514EDF → `UnStick` @0x00514EEE → `StopInterpolating` @0x00514EFD → `UnConstrain` @0x00514F0C → `ClearTarget` @0x00514F1B + `NotifyVoyeurOfEvent(Teleported_TargetStatus)` @0x00514F28 (one `target_manager != 0` guard over the pair) → `report_collision_end(this, 1)` @0x00514F31. Each manager guarded on non-null. |
| `SetFlags(0x1012)` @0x00516414 | ✓ = `Teleport(0x002) \| Slide(0x010) \| SendPositionEvent(0x1000)`. acdream's `AuthoritativeTeleportFlags` (`RuntimeAuthoritativePositionRouteClassifier.cs:197-200`) is bit-for-bit the same against `PhysicsSetPosition.cs:64-76`. |
| ONE `ConstrainTo` @0x00454272 via `HandleReceivedPosition` @0x00453FD0 | ✓ line 92896ff. The remote branch (`arg2 != this->player` @0x0045414D) is `if (MoveOrTeleport(...) != 0)` @0x00454254 → `ConstrainTo(arg2, &arg2->m_position, …)` @0x00454272 → `return`. Single site, shared by all three nonzero-returning branches, anchor read live off `arg2->m_position` (post-move). `MoveOrTeleport` discards `SetPosition`'s error and returns 1 regardless, so **retail arms even when the placement failed** — confirmed. |
| `ZeroVelocity` is local-player-only | ✓ `set_velocity(player_2, {0,0,0}, 1)` @0x004541B4 sits inside the `arg2 == this->player` + `newer_event(TELEPORT_TS)` branch. The remote branch writes no velocity at all. |
### Contract errata (does not change any decision)
**C1 (MINOR).** The contract's pseudo-C excerpt presents `return 0` @0x0051636D
as the fallthrough of the `arg4` test. It is actually the else-label of an outer
sequence gate at @0x00516364 (`if (-((eax_4 - eax_4)) == 0)` — a Binary-Ninja-
mangled `POSITION_TS`/`update_times[4]` comparison that wraps the entire body).
The `arg4 == 0` path does fall into the same label, so the behavioural reading
("writes nothing, returns 0") is correct; the listing just implies a flatter
control flow than the binary has. Worth correcting if the contract is reused.
**C2 (MINOR, out of scope but noted).** `HandleReceivedPosition` runs
`unset_parent(arg2)` @0x00454129 and `SetPlacementFrame` @0x00454142 (when
`!HasAnims`) *before* `MoveOrTeleport`. The contract's "recorded-not-consumed"
list (`UnparentBeforeRouting` / `ApplyPlacementFrameBeforeRouting`) is therefore
accurate — retail really does perform both ahead of the teleport branch, and
acdream still records-without-reading them. Correctly deferred, correctly stated.
---
## Part 2 — answers to the four open questions routed to this review
### Q1 — D1's evidence chain: **CONFIRMED. The plumbing is necessary, not churn.**
Verified end to end in source, not inferred:
- `RuntimeEntityObjectLifetime.TryApplyPosition` measures the pre-merge value at
`src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs:1723-1727`
(`hadCanonical` / `beforeCell` / `wasCellless`) **before** calling
`Entities.TryApplyPosition` at `:1728`.
- The merge onto the canonical record happens later, at `:1786-1789`:
`Entities.RefreshSnapshot(canonical, snapshot, refreshPosition: acceptedPosition)`.
- `RuntimeEntityDirectory.RefreshSnapshot:238` → `record.RefreshDerivedState(refreshPosition)`.
- `RuntimeEntityRecord.RefreshDerivedState:230-237` → `SetFullCell(position.LandblockId, …)`,
and `SetFullCell:244-251` writes `FullCellId` outright.
So after the merge `canonical.FullCellId` **is** the accepted wire cell, and the
classifier — which the App calls after the merge and before the prologue
rebucket (`LiveEntityNetworkUpdateController.cs:1832-1840`, ahead of
`RebucketLiveEntity` at `:1865`) — was reading the wire cell through the
route-1 `Build` overload. A wire cell of 0 is refused as `RejectedData` at
`RuntimeAuthoritativePositionRouteClassifier.cs:321-322`, *before* the cell-less
test at `:402-404`. **The shipped remote cell-less predicate was unreachable.**
D1's characterisation is exactly right and the fix is correct: the pre-merge
value is threaded on `AcceptedPhysicsTimestamps.PreMergeCommittedCellId` and
consumed through a genuine overload (never a third copy), with `null` meaning
"no opinion" rather than a fabricated 0.
One consequence D1 did not name — see **R8** below.
### Q2 — `report_collision_end(this, 1)` ↔ `ShadowObjects.Suspend`: **there IS an unported half.** See **R2**.
### Q3 — the `_activePlacementOwners` deletion: **safe. Confirmed behaviour-preserving.**
`IsPlacementActive` had four consumers (Hidden suspend, UnHide restore,
`RestoreOrdinaryShadowInsideProjection`, `SuspendOrdinaryShadowOutsideProjection`).
The set's only writers were `BeginAuthoritativePlacement` /
`CompleteAuthoritativePlacement`, whose only production caller was the deleted
`RemoteTeleportPlacementPresentation`. With the writers gone the set is
permanently empty, so every consumer degenerates to the `false` branch — which
is precisely what the diff hard-codes. No Hidden/UnHide or visibility-edge
restore changes for non-teleport entities. `HasDeferredShadowRestore` and
`_suspendedShadowOwners` are untouched.
### Q4 — `RemoteTeleportSuffixIsQueuedBehindInitialAdmission`: **survives, correctly re-pointed.**
It pins that a dormant initial-residence FIFO preserves teleport-sequence
ordering (0,1,2) across continuations. The two `TeleportHookRequired` assertions
were replaced with `Assert.Null(...PreMergeCommittedCellId)` on the same two
continuations — a real assertion about the new field, not a tautology. But see
**R8**: what that assertion *proves* is a behaviour change nobody recorded.
---
## Part 3 — findings
### R1 (MAJOR) — the D2 wire-airborne return-0 shape exists only on the player arm; the NPC arm still writes. AP-137 asserts otherwise.
**Where:** `src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs:2188-2211`
(the player-remote D2 block) has no counterpart on the NPC arm. The NPC arm's
only wire-airborne early return is `:2516-2523`, gated on
`IsAirborneNoOperation(earlyRemoteRoute)`, which
(`RuntimeRemoteSteadyStatePosition.cs:81-86`) matches **only** the
`NoPositionOperation` disposition — never `null` and never `Rejected*`.
**What happens instead.** An NPC remote with a `null` or `Rejected*`
classification and a clear wire contact bit falls through `:2525-2543`, `:2560`,
and reaches `ApplyRemoteContactRouting` at `:2600`, where:
- body not in contact → `:1145-1148` writes `remote.Body.Position = worldPos;
remote.Body.Orientation = rotation;` and returns `AirborneSnap`;
- body in contact → `:1196-1203` `ApplyInterpolate` enqueues a waypoint, then
`:2642` arms the leash.
**Retail:** `MoveOrTeleport` @0x0051638E reads `arg4`, finds it 0, and falls to
`return 0` — no body write, no queue write, and (because `ConstrainTo` sits
inside `if (MoveOrTeleport(...) != 0)` @0x00454254) no leash arm. Retail makes
**no player/NPC distinction anywhere in `MoveOrTeleport`**. The `null`
classification is reachable for every remote through the whole login window
(`ClassifyRemoteAcceptedPosition` returns null until `_playerController`
exists), so this is not a corner.
**Why MAJOR rather than "pre-existing".** The behaviour predates the slice, but
two things make it a failure of *this* slice:
1. D2 pinned the outcome — "unifies the player and NPC arms on one leftover-
airborne behaviour" — and only half of it was implemented.
2. The rewritten **AP-137 row now states as fact**: *"This deletes the legacy
player-arm fallback's entity-revert quirk … and unifies player and NPC
remotes on one behaviour."* That sentence is false against
`docs/architecture/retail-divergence-register.md` line 287's own code. A
register row that describes behaviour the code does not have is worse than no
row: it is the thing that stops the next reader from finding the divergence.
**Correct behaviour:** give the NPC arm the same early return the player arm
now has — AP-135's two bookkeeping writes, then `return` — gated on the same
predicate (classification is `null`/`Rejected*` **and** `!update.IsGrounded`).
Or, if the intent is to keep the NPC snap deliberately, rewrite the AP-137
sentence to say so and give the asymmetry its own row.
**Test gap that let it through:** contract test-plan item 10 ("D2's leftover-
airborne shape: wire-airborne null-classified packet writes exactly AP-135's two
fields and nothing else") was not written for either arm, and item 11 (AP-135
preservation on **both** airborne no-op paths) was not written either. The D4
partition test that *was* written
(`RuntimeRemoteSteadyStatePositionTests.TryArmConstraintAfterOperation_MatchesTheCompletePartition`)
asserts the Runtime helper's arm→bool mapping, which is correct, but the
partition table's two "no arm" rows for the *wire-airborne leftover* case are a
property of the **caller returning early** — and that property is only true on
the player arm. The test cannot see the gap.
---
### R2 (MAJOR) — `report_collision_end(this, 1)` is mismapped; the faithful port exists in-tree and is not called.
**Where:** `src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs:254` —
`ReportCollisionEnd: () => _physicsEngine.ShadowObjects.Suspend(localEntityId)`.
**Retail @0x00514F31 → @0x00514620.** `report_collision_end(this, arg2)` walks
`this->collision_table`. With `arg2 != 0` the staleness/still-touching tests are
bypassed (`@0x005146F3`, `@0x005146E7` both `goto label_514706`), so **every**
record is deleted and each is passed to `report_object_collision_end`
@0x00510A90, which fires `weenie_obj->DoCollisionEnd(partnerId)` on this object
(gated on `this->state & 8` = REPORT_COLLISIONS) **and**
`DoCollisionEnd(this->id)` on the partner (gated on the partner's own 0x8, and
skipped wholesale when the partner carries `state & 0x200000`). It is a
**force-end-all with bidirectional notification**.
**What acdream substitutes.** `ShadowObjectRegistry.Suspend`
(`src/AcDream.Core/Physics/ShadowObjectRegistry.cs:1480-1499`) removes the
entity from every cell collision list while retaining its registration. That
method's *own doc comment* says what it is: *"the registry counterpart of retail
`CPhysicsObj::remove_shadows_from_cells`"* — a **different retail function**,
which `teleport_hook` does not call. (Retail's shadow removal for a teleport
happens later and internally, inside `SetPositionInternal`.)
**The faithful port already exists.** `RuntimeCollisionReportingState`
(`src/AcDream.Runtime/Physics/RuntimeCollisionReportingState.cs`) is an explicit
port of `CPhysicsObj::collision_table` and publishes bidirectional
`ObjectCollisionEnd` with the same `ReportCollisions` gates on both sides
(`PublishResolvedObjectEnd:1225-1270`). It has a private `ForceEnd(record, key)`
(`:1435-1450`) that is exactly `report_collision_end(this, 1)` — reached today
only from the destruction / leave-world / batch-retirement edges (`:547`,
`:881`, `:913`, `:943`). **The teleport hook does not reach it.**
The canonical placement does *not* cover the gap: the SetPosition batch dispatch
runs `EndExpiredObjectCollisions(..., force: false, ...)` (`:493-500`), which is
retail's *other* call — `handle_all_collisions` → `report_collision_end(this, 0)`
@0x005147F0. Retail runs **both**: force-end-all in the hook, then the ordinary
non-forcing pass inside the placement.
**Observable delta.** After a remote teleports away, any object still holding a
`CollisionRecord` against it keeps it until the ordinary ~1 s staleness pass, and
neither side receives the immediate `ObjectCollisionEnd` retail fires. Retail
ends it on the spot, on both sides.
**Added-divergence half.** Conversely, suspending the shadow at hook time is
something retail does *not* do there: between the hook and the placement — and,
on any non-commit outcome, until the next `SyncRemoteShadowToBody` — the remote
is non-collidable. The window is bounded (the placement's
`Register`/`RefreshPositionRows` both clear `_suspendedEntities`, and the DR
tick's `ShouldSynchronizeShadow` re-syncs on the next pose delta), so this is a
frames-scale residual rather than the #184 permanent class — but it is real, and
it is not what retail does at this call site.
**Contract compliance:** the contract's open question 2 required the reviewer to
"confirm … **or file the delta on the AP-137 successor row**." The delta is real
and the row does not mention `report_collision_end`, `ShadowObjects.Suspend`, or
the notification half. No row anywhere in the register covers it.
**Correct behaviour:** route the hook's sixth action at a
`RuntimeCollisionReportingState` force-end entry point (the analogue of
`ForceEnd`), and drop or separately justify the shadow suspension. If the
force-end is deliberately deferred, it needs its own register row naming
@0x00514F31 and @0x00514620.
---
### R3 (MAJOR) — a teleported NPC now plans a `RunForward` cycle from a teleport-distance-derived velocity.
**Where:** `src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs:2525-2543`
then `:2684-2709`.
**Newly reachable.** In the pre-slice tree the shared `if (remotePlacementRequired)`
block (`HEAD:1958`) sat **above** the NPC path and returned on every branch, so a
teleport packet never reached the NPC synth-velocity code. This slice deletes
that block, and a teleport-classified NPC packet now flows straight through it.
**What it does.** `serverVelocity` is `update.Velocity`; when ACE omits velocity
(the ordinary `UpdatePosition` case) `:2530-2532` synthesises
`(worldPos − rmState.LastServerPos) / elapsed` — across the **whole teleport
distance**, over one packet interval. `:2536-2537` installs it. At `:2704`
`RemoteServerControlledVelocityCycle.Apply` then calls
`ServerControlledLocomotion.PlanFromVelocity`
(`src/AcDream.Core/Physics/ServerControlledLocomotion.cs:40-62`), which returns
`MotionCommand.RunForward` for anything above `RunThreshold`, and
`ae.Sequencer.SetCycle(style, RunForward, …)` fires. None of the guards block it:
`rm.Airborne` is false at a walkable destination; `rm.MoveTo` was just
invalidated by the hook's own `CancelMoveTo`; a standing NPC's `Ready` passes
`CanApplyVelocityCycle`. `SpeedMod` is clamped, the motion is not.
The cycle persists until the next packet or the 0.6 s
`ServerControlledVelocityStaleSeconds` pass in `RuntimeRemotePhysicsUpdater.cs:198-208`.
**Retail.** The teleport branch writes no velocity at all (contract invariant 6,
verified above), and `teleport_hook`'s first action cancels the moveto. A retail
observer sees the creature appear at the destination and stand. acdream will
show it sprinting in place.
**Impact is animation-only** — `ServerVelocity` drives cycle selection, not
translation (`RuntimeRemotePhysicsUpdater.cs:188-193`) — so this is not a
body-motion defect. But "stands with correct animation" is a literal line in the
contract's own two-client gate recipe, so the gate is expected to catch it if it
is run against an NPC. It will **not** be caught against a player remote:
`RemoteServerControlledVelocityCycle.Apply:27-49` returns early for player GUIDs.
**Correct behaviour:** the teleport arm must not leave a teleport-derived
`ServerVelocity` installed. The narrow fix is to skip the synth (and the cycle
apply) for a teleport-classified packet, which is also what invariant 6's "the
teleport arm must not add a velocity write" was reaching for; the register row
for the AP-80 velocity-cycle adaptation should name the exclusion.
---
### R4 (MINOR) — stale comment: the prologue rebucket no longer feeds the classifier's `CommittedCellId`.
`LiveEntityNetworkUpdateController.cs:1848-1853` still reads: *"it is the only
site that moves an ordinary moving remote's draw bucket, **commits its canonical
FullCellId (which feeds back as the classifier's own CommittedCellId** and as the
ConstraintDistance cell key)…"*. After D1 the remote classifier's
`CommittedCellId` is the **pre-merge** value threaded on
`AcceptedPhysicsTimestamps`, never the rebucket's commit. The comment sits four
lines below the classification call the slice rewrote and directly contradicts
the fix; a reader who believes it would "simplify" D1 away. Correct it in the
same commit (process rule 6). The ConstraintDistance half is still true.
### R5 (MINOR) — the architecture docs still describe the deleted classes as live.
`docs/architecture/acdream-architecture.md:314, 316, 469-471` and
`docs/architecture/code-structure.md:265, 267, 420-423` document
`RemoteTeleportController` / `RemoteTeleportPlacement` as the live remote
placement owners. CLAUDE.md: *"When the architecture doc and reality diverge,
update one or the other — never leave them out of sync."* The deletion inventory
covered every `.cs` wiring site but not these two docs.
### R6 (MINOR) — the `[remote-teleport]` probe can report `hookRan=True` for a hook that did nothing.
`RunRemoteTeleportHook` (`LiveEntityNetworkUpdateController.cs:234-256`)
re-resolves the `RemoteMotion` and `EntityPhysicsHost` **by GUID**, rather than
using the `rmState` the arm already holds and hands to
`ApplyAcceptedRemoteTeleport`. Every action is `remote?.` / `host?.`, so on a
null lookup all six silently no-op and `RemoteTeleportHook.Execute` still
returns `true` — and `PhysicsDiagnostics.LogRemoteTeleport` prints
`hookRan=True`. Since the probe exists specifically to satisfy process rule 5
("a clean-looking live session is not a passed gate"), a probe that cannot
distinguish "ran" from "no-opped" undercuts the gate it was added for. Passing
`rmState` through (it is in scope at all three call sites) removes both the
mismatch risk and the false-positive.
### R7 (MINOR) — `hookRan` is consumed only by the probe. Correct, but say so.
`ApplyRemoteContactRouting:183` captures the hook's currency result and the
routing proceeds regardless. That **is** retail-faithful — retail has no
currency concept and runs the hook before knowing the placement outcome — but
the code reads as if a `false` were being dropped on the floor. One line of
comment naming @0x005163EF's unconditional ordering would close it.
### R8 (MINOR) — D1's null-widening has a second, undocumented producer.
The AP-137 rewrite names exactly one new null-producing reason: *"the merge
observed no PRIOR canonical record for this entity."* There is a second: the
**dormant initial-residence enqueue path**. `TryApplyPosition` returns at
`RuntimeEntityObjectLifetime.cs:1705-1721` (`EnqueueDormant`) **before** the
`:1767-1770` `PreMergeCommittedCellId` write, so those timestamps carry `null` —
which the diff's own changed assertion in
`RuntimeInitialCreateResidenceStateTests` (`Assert.Null(...PreMergeCommittedCellId)`)
now pins. A `TELEPORT_TS`-advancing packet on that path previously classified
`SetPosition` (via `TeleportAdvanced`, which is unaffected by the cell input) and
now classifies **null** → `UnroutedCatchUp`. I could not determine from static
reading whether the App's `OnPosition` reaches `ClassifyRemoteAcceptedPosition`
for an enqueued packet — **flagging as unverified rather than guessing**. Either
way the AP-137 row should name the producer the test now pins.
### R9 (MINOR) — the D2 block's "AP-135's two writes" are the packet's second copy.
`LiveEntityNetworkUpdateController.cs:2206-2209` writes `LastServerPos` /
`LastServerPosTime`, but `:2106-2107` already wrote both unconditionally for this
packet (and `rmState.CellId` at `:2076`), with a second `DateTime.UtcNow` read
producing a slightly later timestamp. Harmless, but the block's comment reads as
though these are the AP-135 writes being preserved, when the preservation
actually happened 100 lines earlier. Contract invariant 8 is satisfied on both
arms (player: `:2076` + `:2106-2107`; NPC: `:2519-2521`) — this is a clarity
issue, not a correctness one.
---
## Part 4 — what the diff gets right (verified, not assumed)
Recorded so a fix round does not disturb it:
- **D1 plumbing.** Pre-merge cell measured by the same `TryApplyPosition` call
that merged the packet, never re-read; `null` propagated honestly through both
`TryBuild` and `Build`; route-1 semantics untouched (the old overload still
passes `canonical.FullCellId`, `RuntimeAcceptedPositionRouteRequests.cs:38-54`).
The discriminating test pair (`CellLessRecord_…` /
`CompanionTest_NonzeroPreMergeCellWithNoTeleportAdvance_…`) genuinely
distinguishes the fix from the shipped dead predicate.
- **D3 hook.** `RemoteTeleportHook.Execute` preserves retail's six actions in
retail's order with a currency re-check between each; `WeenieError.ITeleported`
is 0x3C; `EntityPhysicsHost.NotifyTeleported` is the correct one-to-one for the
`ClearTarget` + `NotifyVoyeurOfEvent` pair under retail's single guard.
- **D4 single arming site.** The legacy pre-operation call is gone; the
post-operation site is the only one, matching @0x00454272. Dropping the
`ConstrainAfterRouting` route-flag check is behaviour-preserving —
`Interpolate` and `SetPositionSimple` both carry
`ConstrainPhase.AfterPositionOperation`
(`RuntimeAuthoritativePositionRouteClassifier.cs:473`), and
`NoPositionOperation`'s `None` is already excluded by the arm mapping. Keying
on the **routing outcome** rather than the raw classification is the right
call: it is the only input that separates a grounded `UnroutedCatchUp` (arms,
retail's nonzero analogue) from `AirborneSnap` (does not).
- **D5 ordering.** The teleport dispatch is the first thing
`ApplyRemoteContactRouting` does, ahead of the `!remote.Body.InContact`
carve-out; on the player arm it precedes both the wire-airborne block and the
landing block; on the NPC arm the sticky gate is widened
(`!snapSuppressedByStick || isTeleportRoute`) rather than duplicated. All three
match "decided before `arg4`". `UnStick` being the hook's second action is the
right justification for the sticky widening.
- **D6/D7.** `TryApplyGenericRemoteRenderPose` gate unchanged and now stated;
`TryAdoptWireCellAfterRouting` suppression widened to the teleport arm; the
re-entrancy guard covers both placement arms and sits **before** the arming on
both, preserving the R5 invariant.
- **Invariant 1.** `ApplyAcceptedRemoteTeleport` reuses the far arm's
`StoresAcceptedDestination` partition and `StoreAcceptedDestinationPose`
unchanged — the partition was not re-litigated.
- **Invariant 2.** Both teleport tails sync the render entity from the resolved
body and publish the shadow (`:2171-2184` player, `:2719-2732` NPC). The
placement writes the resolved cell back through
`RuntimeSetPositionState.cs:5013` (`remote.CellId = result.CellId`), so
`entity.ParentCellId` really is the resolved cell.
- **Invariant 4.** The classifier's teleport branch carries
`StopInterpolating: false` deliberately; the queue clear comes from the hook's
`Interp.Clear()`, matching @0x00514EFD.
- **Invariant 10.** No new production caller for `TickLostCellDeadlines` /
`TryDequeueExpiredLostCell`; `ArmLostFamilyDeadlines` keeps its single
pre-existing `ParkDeferred` call.
- **Register bookkeeping.** AP-137 rewritten in place rather than deleted
(correct — the contract overrides the handoff here); AD-42's dead citation
removed; AP-136 and AP-138 updated to the two surviving non-Position rebucket
writers; AP-138's Risk column gains the teleport arm; AP-135 untouched. All
correct **except** the false sentence named in R1.
---
## Part 5 — gate evidence not present
Stated, not held against the verdict:
- The complete Release suite figure the contract's Gates section requires
("measure and record the new figure; do not inherit 11,027") is not recorded —
the change is uncommitted and there is no commit message. I ran the build and a
focused filter only.
- The two-client connected teleport gate is user-run and cannot be evidenced by a
reviewer. Note that per R3 an **NPC** target is the discriminating case: a
player-remote teleport will not exercise the velocity-cycle path at all.
- Contract test-plan items 10 (D2 shape) and 11 (AP-135 preservation on both
airborne no-op paths) have no corresponding test in the diff. Item 10's absence
is what left R1 invisible.
---
## Recommended fix order
1. **R1** — give the NPC arm the same D2 return-0 shape, and make the AP-137
sentence true (or split the asymmetry into its own row). Add contract test 10
for **both** arms.
2. **R2** — route the hook's sixth action at the collision-reporting force-end,
or file the delta on AP-137 with the @0x00514F31/@0x00514620 citations.
3. **R3** — exclude a teleport-classified packet from the NPC synth-velocity
install and the cycle apply; note the exclusion on the AP-80 row.
4. **R4/R5** — the two stale-documentation fixes, same commit.
5. **R6-R9** — clarity and probe-honesty, at the implementer's discretion.