feat(physics): C4 route 4b-1 — remote placement infrastructure (dormant)
Builds the machinery route 4b-2 and 4b-3 will flip on, and changes no remote behaviour: it has no production caller, so RemotePlacementDrivePendingCount is provably 0 and IsConverged is unchanged. Five pieces: a per-entity remote placement owner (RuntimeRemotePlacementDriveController), a Position-time service-window guard with a Runtime interface plus BOTH host implementations, N3's headless RetryPending pump, parked-count observability in the ownership ledger, and the service-window optimisation that avoids parks we can cheaply predict. Landed alone because it is where the park-withdraws-the-entity failure was decided; that decision is fixed at the source in the preceding commit and must not share a review signal with a behaviour flip. Two parts of route 2's controller are deliberately NOT ported, both verified against retail rather than assumed. There is no ack: SendPositionEvent is called only inside HandleReceivedPosition's local-player FORCE_POSITION gate @0x0045400C-@0x00454091, and the remote arm @0x0045414D has no equivalent. There is no re-issue funnel: retail never re-attempts a position it could not apply — stale timestamps merely bump error_count @0x004542AC — and re-issuing packet N after N+1 has merged would apply a pose the newer packet already superseded, which is correct for a one-shot ForcePosition and wrong for a 5-10 Hz stream. The service-window guard is an OPTIMISATION, not the correctness mechanism. The original contract had it the other way round, justified by a claim that retail cannot represent "arrived but not placeable" — false, and corrected in the review findings: retail's GotoLostCell/reenter_visibility path represents it exactly. A pre-flight guard also cannot be complete, because Core defers on the entity's CURRENT cell, on the swept QueriedCellIds footprint spanning neighbouring landblocks, and on residency evaluated after AdjustToOutside — conditions only Core can see. Review found and this commit fixes: DetachRoute cleared two maps of LIVE Core operations without cancelling them (route 2's AbandonPending is the correct mirror, not the first-entry controller) and its test asserted that blindness as convergence; the headless predicate answered "can ever publish" rather than "is published", and after the first fix still matched only 1 of the 9 landblocks this host publishes; OwnsPlacement admitted remote top-level Creates until gated on the Teleport flag as well as the disposition; Advance re-submitted without re-checking the window; and four comments cited a report that did not exist. Contract item 6 is met by the structural proof, not the earlier test: HasOldPrefixPlacementDebt refuses collision-prefix mutation permission before ParkCollisionResidents is ever entered, so its overlap throw is unreachable. That same mechanism is the unbounded stall filed as #310, which 4b-1 does not bound — it only avoids widening it. #311 files the remaining per-tick allocation in RetryPendingProjections; the early-out for the empty-FIFO case landed via a new HasPendingReceipts accessor so hosts still never touch .Placements. directly. Gates: complete Release solution 10,973 passed / 4 skipped / 0 failed (baseline 10,938). Four review rounds; every fix discrimination-verified by revert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
634bc5513a
commit
2e8e09acd0
14 changed files with 3151 additions and 6 deletions
175
docs/research/2026-08-04-c4-route-4b-1-review-findings.md
Normal file
175
docs/research/2026-08-04-c4-route-4b-1-review-findings.md
Normal file
|
|
@ -0,0 +1,175 @@
|
|||
# C4 route 4b-1 — dual review FAIL, and a design correction (2026-08-04)
|
||||
|
||||
Both mandated reviews returned **FAIL**. Nothing is committed. This supersedes
|
||||
the 4b-1 contract, **whose central premise was factually wrong.**
|
||||
|
||||
## The premise error — mine
|
||||
|
||||
The contract justified refuse-rather-than-park with: *"retail's world is fully
|
||||
resident, so 'arrived but not placeable' is unrepresentable there."*
|
||||
|
||||
**False.** Retail represents it explicitly, with a working park:
|
||||
|
||||
- `CPhysicsObj::SetPositionInternal` @0x00515BD0 — when `AdjustPosition` yields
|
||||
no cell (@0x00515C1D): `prepare_to_leave_visibility` @0x00515CDA,
|
||||
`store_position` @0x00515CE2 (**the destination pose IS committed**),
|
||||
`CObjectMaint::GotoLostCell` @0x00515CF2, clear transient 0x80 @0x00515CF7,
|
||||
return `OK_SPE` @0x00515D07.
|
||||
- `CObjectMaint::GotoLostCell` @0x00508210 appends the object to that cell's
|
||||
lost list.
|
||||
- `CObjectMaint::InitObjCell` @0x00508260 drains the list on cell load and calls
|
||||
`reenter_visibility` per object @0x00508296.
|
||||
- `MoveOrTeleport` discards the `SetPositionError` from both
|
||||
`SetPosition` @0x00516420 and `SetPositionSimple` @0x005163D9 and returns 1
|
||||
regardless, so `ConstrainTo` @0x00454272 is armed **even when the placement
|
||||
failed**.
|
||||
|
||||
Retail's reaction to an unplaceable remote: advance the pose, hide the object,
|
||||
register it lost, wake it on cell arrival, arm the leash anyway. Our refusal
|
||||
leaves the remote **visible at a stale pose**. For a remote that teleports into
|
||||
a non-resident landblock and then stops moving — ACE stops broadcasting for a
|
||||
stationary entity — "the next packet is the retry" never arrives and the
|
||||
divergence is permanent.
|
||||
|
||||
## Refuse cannot be made complete — the second reason to abandon it
|
||||
|
||||
The guard checks ONE landblock (`accepted.LandblockId`). Core defers on at least
|
||||
four independent conditions:
|
||||
|
||||
1. **`PlacementTouchesPrefix` matches the entity's CURRENT cell**, not only the
|
||||
destination (`RuntimeSetPositionState.cs:3668-3673`, consumed `:2916`). A
|
||||
remote standing in a quiescing landblock, moving to a perfectly published
|
||||
destination, parks. That is the contract's own producing sequence with
|
||||
ordinary streaming churn behind it.
|
||||
2. **`ResultTouchesPrefix` matches every cell in `QueriedCellIds`**
|
||||
(`:3675-3690`, consumed `:2981`) — the sweep footprint spans neighbouring
|
||||
landblocks near a boundary, so a quiescing NEIGHBOUR parks a fine placement.
|
||||
3. **Engine-level non-residency after `AdjustToOutside`**
|
||||
(`PhysicsEngine.cs:1309-1318`, `:1464-1475`) — the adjusted cell can land in
|
||||
an adjacent landblock, evaluated against THAT landblock. Not visible to any
|
||||
pre-flight caller.
|
||||
4. Headless, where the predicate is weaker still (see below).
|
||||
|
||||
**A pre-flight guard cannot close conditions that only Core can see.** Refuse is
|
||||
structurally incapable of being complete.
|
||||
|
||||
## The actual root cause — and it is shipped, not new
|
||||
|
||||
`ParkDeferred` (`RuntimeSetPositionState.cs:4088-4120`) withdraws the entity:
|
||||
`body.InWorld = false`, `Active` cleared, `WithdrawCanonical` (→
|
||||
`RemoveSpatialProjection` + `SetFullCell(record, 0u, 0u)`), `SuspendObjectClock`.
|
||||
|
||||
`CancelCoreDeferred` (`:5131-5198`) removes the operation, rewrites
|
||||
`Withdraw`→`Discard`, and **restores none of it**. The only `InWorld = true` in
|
||||
the file is the local-player dormant-activation commit (`:2591`).
|
||||
|
||||
So cancelling a wakeable park is strictly worse than retaining one: the park is
|
||||
at least wakeable; the cancel destroys the only object that could wake it.
|
||||
|
||||
**This affects route 2's DeferredCell path too.** Route 2 compensates with its
|
||||
re-issue funnel — which is correct for a one-shot ForcePosition and wrong for a
|
||||
5-10 Hz remote stream. So the underlying defect has been masked, not fixed.
|
||||
|
||||
## Corrected direction for the next round
|
||||
|
||||
**Make the park work, at the source, modelled on retail's lost-cell.** A
|
||||
cancellation of a wakeable park must restore what `ParkDeferred` withdrew —
|
||||
`InWorld`, the object clock, and canonical residency — or the park must survive
|
||||
the merge-time `Forget` so its collision-generation wake can still fire.
|
||||
|
||||
The 4b-1 contract said a withdrawal-restore inside `RuntimeSetPositionState`
|
||||
required STOP-and-report. **That stop has now happened and this is the answer**:
|
||||
refuse is structurally incomplete, retail has a working park, and the restore
|
||||
fixes route 2's latent path as well. Proceed with it deliberately.
|
||||
|
||||
The service-window guard still has value as an OPTIMISATION — avoiding parks we
|
||||
can cheaply predict — but it is no longer the correctness mechanism and must not
|
||||
be presented as one.
|
||||
|
||||
Whatever residual divergence remains after this needs a
|
||||
`docs/architecture/retail-divergence-register.md` row measured against retail's
|
||||
`GotoLostCell`/`reenter_visibility` behaviour, not against a "retail-shaped"
|
||||
label.
|
||||
|
||||
## The other blocking findings
|
||||
|
||||
**B1 — the headless predicate is not a service window.**
|
||||
`HeadlessSessionWorldProjection.cs:254-266` is a pure 3x3 Chebyshev GEOMETRY
|
||||
test against `_requestedCenterLandblock`, and returns `true` outright when no
|
||||
centre has been requested. Its own pre-existing doc says "can EVER
|
||||
collision-publish". The new interface promises "currently published". Signature
|
||||
match, predicate mismatch — the same over-permissiveness the graphical adapter
|
||||
explicitly rejected `IsNearTierOrPending` for. `IsReady` (`:268-285`) is the
|
||||
correct shape and sits fourteen lines below. Remove the "same question" claim.
|
||||
|
||||
**B2 — the `ParkCollisionResidents` evidence is void, and the real hazard is a
|
||||
different one.** The delivered test demonstrates the two states that were
|
||||
already safe and calls `ParkCollisionResidents` DIRECTLY, bypassing
|
||||
`TryAcquireCollisionPrefixMutationPermission`'s `HasOldPrefixPlacementDebt`
|
||||
check (`:3641-3666`, consumed `:887`) — which is the thing that actually makes
|
||||
the throw unreachable. The genuine hazard is not a throw but an **indefinite
|
||||
streaming stall**: that predicate refuses permission on every poll while a
|
||||
retained retry is held, so the landblock never retires. `RetrySetupUnavailable`
|
||||
on an asset that never loads makes it permanent, and `DetachRoute` clears
|
||||
`_pending` WITHOUT cancelling the operation, orphaning it until session reset
|
||||
while it continues to pin the prefix.
|
||||
|
||||
**B3 — `Advance()` re-submits with no service-window re-check** (`:298-332`,
|
||||
`SubmitAndResolve` `:334-394`). A retained entry can sit across many frames
|
||||
while its destination retires.
|
||||
|
||||
**B4 — `Committed` leaves an untracked live operation.** It returns and retains
|
||||
nothing while the operation sits at `AwaitingCommitAcknowledgement`, retired
|
||||
only by `AcknowledgeProjection`. `RemotePlacementDrivePendingCount` cannot see
|
||||
it, so the ledger is blind to exactly the class that produces B2's stall.
|
||||
|
||||
**B5 — N3's actual fix is untested.** `HeadlessSessionHost.cs:328` has zero
|
||||
coverage; the new test hand-builds the route and never touches `Tick`. Also:
|
||||
per-tick `RetryPendingProjections()` does `_pendingProjection.Values.ToArray()`,
|
||||
a new per-tick allocation K4's 30-session envelope was measured without; and
|
||||
headless dereferences the route directly with no generation latch, where
|
||||
graphical goes through `RuntimePlacementProjectionRetrySlot` which refuses a
|
||||
stale-generation callback.
|
||||
|
||||
**B6 — `OwnsPlacement` keys on Disposition alone** (`:189-191`). The classifier
|
||||
also emits `SetPositionSimple` for the local player's FORCE_POSITION and
|
||||
teleport branches, and `SetPosition` for every initial Create. `record` and
|
||||
`route` are separate parameters, so a mismatched pair is expressible. One
|
||||
`route.OperationKind is RemoteAuthoritative` guard makes ownership exact.
|
||||
|
||||
**B7 — three comments cite a "route 4b-1 report" that does not exist**
|
||||
(`GraphicalRemotePlacementServiceWindow.cs:57`,
|
||||
`RuntimeRemotePlacementDriveController.cs:122`, and the test file `:28`). Two of
|
||||
them point at precisely the evidence the contract demanded.
|
||||
|
||||
**B8 — advisory for 4b-2/4b-3**: retail arms `ConstrainTo` even when the
|
||||
placement failed, so the successors must arm on refusal/rejection too, not only
|
||||
on commit. "Arm on Committed" is the natural misreading and is the same shape as
|
||||
the already-recorded unarmed-leash bug.
|
||||
|
||||
## Verified correct — do not churn
|
||||
|
||||
- Both omissions are right: `SendPositionEvent` is local-player-FORCE-only
|
||||
(@0x00454091 inside the @0x0045400C gate); the remote arm @0x0045414D has no
|
||||
equivalent, and retail never re-attempts — stale timestamps just bump
|
||||
`error_count` @0x004542AC.
|
||||
- The disposition mapping is exact for Remote-kind routes: `SetPosition` ≡
|
||||
teleport-or-cell-less (@0x00516386, flags 0x1012), `SetPositionSimple` ≡ far
|
||||
snap (@0x005163C1-E8).
|
||||
- The graphical co-extensivity argument holds in both directions, independently
|
||||
verified by both reviewers. `IsNearTier` over `IsNearTierOrPending` is right.
|
||||
- Contract item 2 holds: no production caller, no behaviour change, ledger
|
||||
member always 0 in production.
|
||||
- The "Do NOT touch" list was respected — AP-135's writes, the teleport classes,
|
||||
the legacy far halves, the single `ConstrainTo` site, route 1's executor.
|
||||
- Per-entity mechanics are otherwise sound: incarnation-keyed identity,
|
||||
self-heal, `_driveScratch` snapshotting, `_driving` re-entry guard.
|
||||
- N3's ordering matches the graphical route.
|
||||
|
||||
## Gate
|
||||
|
||||
Complete Release suite, not a subset. Baseline **10,938 / 4 / 0**; the 4b-1
|
||||
state measured 10,955 / 4 / 0 while being defective. Two known flakes, do not
|
||||
chase and do not conflate: **#302** (`PortalProjectionTests…`, GC-allocation
|
||||
assertion, App.Tests) and **#308** (`NakEmissionTests.LossSoak_…`, wall-clock
|
||||
deadline, Core.Net.Tests, full-suite load only).
|
||||
Loading…
Add table
Add a link
Reference in a new issue