acdream/docs/research/2026-08-05-issue-319-architecture-review.md

# Issue #319 — independent architecture / adversarial review (2026-08-05)

**Verdict: FAIL.**

Reviewed: the uncommitted working tree at branch
`claude/acdream-physics-divergence-5aa784`, HEAD `af828a8a` (`git diff HEAD`
plus the untracked contract doc). Production delta measured independently:
**91 non-comment lines added, 24 removed** across five files — the
implementer's claim 3 is exact.

Verification performed for this review (not inherited):

- `dotnet build -c Release` — green (exit 0).
- `AcDream.Runtime.Tests` filtered to `ParentAttachmentStateTests` +
  `RuntimeEntityChildCellPropagationTests` — 32/32 pass.
- `AcDream.App.Tests` filtered to `EquippedChildProjectionWithdrawalTests` +
  `LiveEntityHydrationControllerTests` + `LiveEntityPresentationControllerTests`
  + `LiveEntityRuntimeTests` — 206/206 pass.

The key fix (F1's staged half) is correct, well-anchored, and its tests are
genuinely sabotage-sensitive. The bookkeeping (AP-142 clause (f), AP-132
clarification, new AP-146, issue #320, route-7 §7 supersession note) is the
most thorough in the campaign so far. **The FAIL rests on two things: the
commit-time tripwire is wired at the wrong point in the transaction so that
when it fires it tears the commit it was built to protect (A1), and the
deferred late-bind branch — the larger and more novel half of F1 — is
production-unreachable, carries a freshness hole its ParentEvent sibling does
not, and its one test reaches it only by bypassing the production seam under
an incorrect stated rationale (A2, A3, A6).**

---

## A1 — MAJOR — the tripwire throws *after* the canonical parent commit has already landed; when it fires it produces a torn transaction, which is the one outcome F1 pinned against

**`src/AcDream.App/Rendering/EquippedChildRenderController.cs:955-963`**

```csharp
if (candidateKind is ParentProjectionCandidateKind.Staged)
{
    if (!_liveEntities.CommitStagedParent(relation, out _)      // canonical commit
        || !Relations.CommitProjection(relation, ResolveLiveParentInstance))  // throws here
    {
        return default;
    }
```

**`src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs:408-412`** —
identical ordering:

```csharp
if (!Entities.TryCommitParent(staged, acknowledgeProjection: null, out _)
    || !relations.CommitProjection(staged, _resolveParentInstance))
```

`CommitStagedParent`/`TryCommitParent` is the *canonical* half. It runs
`InboundPhysicsStateController.TryCommitParent` → `ApplyParent`
(`InboundPhysicsStateController.cs:1347-1373`), which nulls the child's
snapshot `Position`, writes `ParentGuid`/`ParentLocation`/`PlacementId`, and
stamps POSITION_TS. Only *then* does `CommitProjection`
(`ParentAttachmentState.cs:651-661`) evaluate the tripwire and throw.

**Failure scenario.** Any input that reaches the tripwire with a mismatch:
the child's canonical snapshot has already been rewritten as parented (its
world Position destroyed, POSITION_TS advanced), `_lastAcceptedByChild` has
*not* been written, and the relation is still sitting in `_stagedByChild`.
The exception then unwinds through `PrepareAndTryRealize` → `Tick()` /
the inbound sink. Net state: a child that the canonical layer believes is
parented, with no committed relation for D1/D2 to find, and a staged relation
that `Resolve`'s early return (`ParentAttachmentState.cs:457-458`) will
now block forever for that child.

That is strictly worse than the silent mis-keyed commit #319 shipped. The
contract's F1 pinned outcome was "**never a silent success under a mismatched
key**"; the XML doc on `CommitProjection` promises it "refuses loudly rather
than silently filing the relation under a key D1/D2 can never find again."
Neither is what the code does — it half-files, then throws.

**On reachability, stated honestly.** I traced every path that can advance a
parent's live `InstanceSequence` while a staged relation naming that parent
survives, and found none reachable today:

- Both producers set the sequence from the live snapshot at stage time
  (`EquippedChildRenderController.cs:845-857`; `ParentAttachmentState.Resolve`
  `:473-498`).
- The only incarnation bump is a `NewGeneration` CreateObject, and
  `RuntimeEntityObjectLifetime.cs:1004-1010` calls
  `ParentAttachments.EndGeneration(...)` → `RemoveParentReferences(_stagedByChild, guid)`
  (`ParentAttachmentState.cs:825`), purging every staged relation naming that
  parent.
- Delete removes both the gate and the snapshot
  (`InboundPhysicsStateController.cs:101-102`) and is immediately followed by
  `DeleteGeneration` (`RuntimeEntityObjectLifetime.cs:2088-2090`), which purges
  the same way — so after a delete the producers *defer* rather than stage a
  stale key, and the next create is `InitialGeneration` (no `EndGeneration`
  needed).

So the mismatch is unreachable — **but only as an emergent property of a
three-file invariant chain that nothing records**, and one link is
order-fragile: `AcceptCreate` publishes the new incarnation into `_snapshots`
*before* `RegisterEntityCore` reaches `EndGeneration`, with `RemoveActive` and
`WithdrawCommittedChildrenToCellless` (which publishes deltas to synchronous
App observers) executing inside that window.

This is exactly route 3's N3 shape
(`docs/research/2026-08-04-c4-route-3-retail-review-round2.md:346-362`):
"Refusing to fake success is right; converting a possibly-transient condition
into a process-killing throw on the host that must survive K4's 30-session /
two-hour endurance profile is the wrong end of that trade." The tripwire's own
comment asserts a diagnosis it cannot establish — "a mismatch here can only be
a producer regression" — when a parent replacement whose purge ordering ever
changed would produce the same mismatch. On the headless site the throw lands
in `OnParentUpdated`, an inbound sink; `HeadlessSessionHost.cs:52/64` will
quarantine it, i.e. one bot session of 30 dies rather than the process — still
a lost endurance row, and the torn canonical state above is what it dies
holding.

**Fix direction (cheap, no redesign).** Move the incarnation check *above*
`CommitStagedParent`/`TryCommitParent` at both sites and make it a refusal,
not a throw: log loudly (the codebase's existing refusal idiom) and return
`false`, so the staged relation is rejected (`RejectProjection`) rather than
left to block `Resolve`. That satisfies F1's pinned outcome exactly — no
silent success under a mismatched key — without a fatal, and without tearing
the transaction. If the project wants a hard assertion, it belongs in a
`Debug.Assert`/test-only seam, not on the live packet path.

---

## A2 — MAJOR — a deferred late-bind relation skips `accept`, so it carries no child-freshness gate at all, and is retained across the child's own generation boundary

**`src/AcDream.Runtime/Entities/ParentAttachmentState.cs:506-524`** (the
`if (!lateBind)` wrapper around the `accept(update)` call) and **`:412-426`**
(`DeferCreateObjectRelation` enqueues with `WaitOwner = Parent`).

For every ParentEvent-sourced relation, `accept` is
`TryApplyParent`/`TryAcceptParentForProjection` → the child's
`PhysicsTimestampGate.TryAcceptPositionChannelEvent`
(`InboundPhysicsStateController.cs:258-273`). That call does two things: it
*gates* the relation on the child's POSITION_TS, and it *advances* the child's
gate to `update.ChildPositionSequence` — which is precisely what makes the
subsequent `TryCommitParent` gate
(`InboundPhysicsStateController.cs:299-312`: `gate.PositionTimestamp !=
positionSequence || child.PositionSequence != positionSequence`) satisfiable.

A late-bind relation skips both. Its only remaining gate is that same
`TryCommitParent` equality — against a `ChildPositionSequence` captured when
the relation was *enqueued*, possibly many packets earlier.

Compounding it: `DeleteGeneration` and `EndGeneration` both run
`FilterChildCandidates(guid, relation => relation.WaitOwner is
ParentAttachmentWaitOwner.Parent)` (`:819-821`, `:844-846`) — i.e. they
**retain** relations whose `WaitOwner` is `Parent`. `DeferCreateObjectRelation`
sets `WaitOwner = Parent` at enqueue, and `Resolve`'s re-queue would set it
anyway, so a deferred late-bind relation survives its own child's delete or
replacement.

**Failure scenario.** Child C's CreateObject/`CreateParentUpdate` defers a
late-bind relation naming parent P (P unaddressable). C is then replaced by a
new generation (`EndGeneration(C, n+1)` — an ordinary re-observe shape) or
deleted and its GUID recycled. The relation survives. P later becomes
addressable → `RetryWaitingDescendants(P)` → `Resolve(C, …)` → the late-bind
branch adopts P's *current* incarnation and stages, with no `accept` gate to
reject the stale `ChildPositionSequence`. Two outcomes, both bad:

- `TryCommitParent`'s POSITION_TS equality fails (the new child generation has
  a different stamp) → `CommitStagedParent` returns false →
  `PrepareAndTryRealize` returns `default` → **the relation is stranded in
  `_stagedByChild` permanently.** `Resolve`'s early return (`:457-458`) then
  blocks every subsequent legitimate parent relation for that child for the
  rest of the session, and `CopyPendingProjectionChildrenTo` retries it every
  frame forever.
- Or the stamps happen to coincide and the attach commits — binding a dead
  wire event's relation onto a new child incarnation and whatever object now
  holds P's GUID. This is AP-132's documented GUID-reuse hazard, applied on the
  one producer that now has no gate at all.

Note the pre-fix behaviour self-healed: `AcceptCreateObjectRelation`
(`:391-394`) is an unconditional `_stagedByChild[child] = relation`
assignment, so a newer relation always superseded a stuck one. Routing through
the queue removes that self-healing property.

**Reachability caveat, stated honestly:** see A6 — the deferred branch appears
to be production-unreachable today, so this is latent, not live. It is still a
MAJOR defect in newly added code, because the code's stated purpose is to be
the fail-safe.

**Fix direction.** Either (a) subject late-bind relations to the same child
POSITION_TS gate (call `accept` and let it advance the stamp — the retail
argument for skipping it is about the *parent* incarnation, not the *child*
timestamp), or (b) if the branch stays as a pure fail-safe, do not enqueue it
with `WaitOwner = Parent` and add an explicit child-incarnation field so the
generation filters can drop it with its child.

---

## A3 — MEDIUM — the deferred relation carries a placeholder `ParentInstanceSequence = 0` that the generation filters read as a wire-named incarnation

**`src/AcDream.App/Rendering/EquippedChildRenderController.cs:838-844`** builds
the relation with the literal `ParentInstanceSequence: 0` and hands that exact
value to `DeferCreateObjectRelation` when the parent is unknown. The
`LateBindParentInstance` flag records "this 0 is meaningless" — but only
`Resolve` reads the flag.

`EndGeneration` (`ParentAttachmentState.cs:828-833`) and `DeleteGeneration`
(`:853-857`) both filter the unresolved queue with predicates that compare
`relation.ParentInstanceSequence` against a real generation via
`PhysicsTimestampGate.IsNewer`. For a late-bind relation with the placeholder
0 and a *player* parent (`TotalLogins` ≥ 1, never 0):

- `EndGeneration(P, 8)`: `0 == 8` false; `IsNewer(8, 0)` false → **dropped.**
- `DeleteGeneration(P, 7)`: `IsNewer(7, 0)` false → **dropped.**

**Failure scenario.** A late-bind relation whose semantic is "attach to
whatever currently holds this GUID" is judged as "names generation 0, which is
older than the replacement" and silently discarded at exactly the moment the
replacement it should bind to arrives. Player-class only — #319's own failure
signature, reintroduced in a narrower window.

Not reachable today only because a `NewGeneration` disposition requires a
pre-existing snapshot, and a pre-existing snapshot means the producer would
have taken the *staged* branch rather than deferring. That coupling is
undocumented and is not a property either file states.

**Fix direction.** Make `FilterParentCandidates`'s callers retain
`LateBindParentInstance` relations unconditionally (they name no generation to
compare), or carry a sentinel the filters can recognise.

---

## A4 — MEDIUM — the `RestoreShadow` gate is *not* behaviour-preserving at HEAD; contract §3.2's premise is wrong for creature-parented children

**`src/AcDream.App/World/LiveEntityPresentationController.cs:229`**

The contract argues (§3.2) that `RestoreShadow` "no-ops today on
`record.FullCellId == 0`", so the new `HasCommittedParent` clause is inert at
HEAD and only matters post-fix. **That premise holds only for
player-parented children.** Route 7's D1 already re-cells *creature*-parented
children to a nonzero cell — that is exactly the class route 7 shipped and
gated. So today an NPC's wielded weapon that has a shadow registration and
crosses a Hidden→Visible edge reaches `ShadowPositionSynchronizer.Sync` and
gets its broadphase row refreshed; with this gate it no longer does.

The change is in the *right* direction — route 7's P4 record
(`RuntimeEntityDirectory.cs:451-465`) says a committed child never owns a
broadphase row, and `ShadowObjectRegistry.UpdatePosition`
(`ShadowObjectRegistry.cs:696-697`) no-ops for an unregistered entity, so the
live population is bounded by "children that already carry a registration."
But this is a live behaviour change on a shipped path, justified in the
contract by a premise that does not hold, and route 7's connected gate never
exercised it.

**Failure scenario if the premise is wrong in the other direction:** a
formerly-world object that is picked up and equipped keeps a suspended
registration in `_shadows` that `RestoreShadow` will now never restore, while
`_suspendedShadowOwners` keeps its key (`:203` is only reached when `restored`
is true). `Forget`/`Clear` (`:111-127`) converge it at teardown, so this is
bounded — but it is a state the contract did not analyse because it assumed
the population was empty.

**Fix direction.** Not a code change necessarily — but the creature-parented
half must be named in the connected gate (§7 Half B) as a thing to watch, and
the contract's §3.2 "correct today" claim corrected. The test
`CommittedChild_HiddenThenVisible_NeverInstallsShadowRow` does assert against
the real `ShadowObjectRegistry` (the fixture pre-registers the entity at
`LiveEntityPresentationControllerTests.cs:700-710`), so the AP-145/#318 rule
is honoured — good.

---

## A5 — MEDIUM — unbounded accumulation of deferred late-bind relations (the un-written §6 test 9 would have caught this)

`_unresolvedByChild` is removed for a child only by `RemoveChild`/`RemoveObject`
(`ParentAttachmentState.cs:782`, `:878`) or by `Clear()` (`:890`), and per
parent guid by `RemoveObject(parentGuid)` (`:788-798`) and the two generation
filters. A late-bind relation naming a parent GUID that **never becomes
addressable and is never itself deleted** (an equip whose holder stays outside
visibility) is removed by none of them and is retained across the child's own
delete by the `WaitOwner is Parent` rule (A2).

Each such relation is a permanent `+1` on `PendingRelationCount` and a
permanent per-parent-spawn scan cost in `ChildrenWaitingForParent`
(`:715-719`, a `queue.Any(lambda)` per child). Over a two-hour, 30-session
endurance run this accumulates monotonically. `Clear()` converges it at reset,
so a teardown-only ledger assertion would pass — which is exactly why the
missing §6 test 9 is not the whole answer here.

**Fix direction.** Bound the unresolved queue per child (the existing
`Enqueue` has the same shape, so this is a pre-existing class the new producer
widens), or age deferred late-bind relations out.

---

## A6 — MEDIUM — the deferred branch is production-unreachable for *both* producers, and its only test reaches it by bypassing the production seam under an incorrect stated rationale

Implementer claim 1 is **verified** for the raw CreateObject path:
`RuntimeEntityObjectLifetime.cs:797-812` defers the whole CreateObject when
`beginInitialResidence && parentGuid != 0u && !Entities.TryGetActive(parentGuid, out _)`,
and `TryGetActive` ⇒ `TryGetSnapshot` (the snapshot is written by
`AcceptCreate` before `AddActive`). So `OnSpawn` never sees an unaddressable
parent.

**But the same gate covers `CreateParentUpdate`.** The test's own justification
(`EquippedChildProjectionWithdrawalTests.cs`, the doc comment on
`OnCreateParentAccepted_ParentNotYetKnown_DefersThenLateBindsOnArrival`) says
`CreateParentUpdate`'s producer "has no equivalent parent-addressability
precondition." That is wrong: `CreateParentUpdate` is manufactured by
`BuildSameGenerationEvents` inside the very `RegisterEntityCore` call whose
line-797 gate reads `incoming.ParentGuid ?? incoming.Physics?.Parent?.Guid` —
a same-generation CreateObject carrying a Parent hits the identical deferral.
The test reaches the deferred branch only because it calls
`fixture.Live.TryApplyCreateParent(...)` and `fixture.Controller.OnCreateParentAccepted(...)`
directly, below the routing layer that would have prevented it.

So `DeferCreateObjectRelation`, the `LateBindParentInstance` field, and
`Resolve`'s late-bind branch — roughly half of F1's added lines and all of its
novel state — are **dead in production**, and they carry A2's freshness hole
and A3's placeholder-sequence hazard. A fail-safe that resurrects relations
across generation boundaries with no timestamp gate is not a safer state than
the assertion it replaces.

**Fix direction.** Either establish a real production path (and then test it
through the production seam), or reduce the branch to something with no
independent failure modes — e.g. refuse the relation outright with a loud log
when the parent is unaddressable, which is what the layer above already
guarantees cannot happen.

---

## A7 — LOW — the hydration gate is correctly scoped

**`src/AcDream.App/World/LiveEntityHydrationController.cs:561-562`** — verified
behaviour-preserving at HEAD and necessary post-fix, on stronger grounds than
the contract gave:

- `LiveEntityRecord.ProjectionCellId` (`LiveEntityRuntime.cs:387-389`) is
  `WorldEntity is not null ? FullCellId : Snapshot.Position?.LandblockId ??
  FullCellId`. A committed child always has a null snapshot `Position`
  (`ApplyParent` nulls it, `InboundPhysicsStateController.cs:1358/1366`), so
  all three sources of `projectionCellId` collapse to `FullCellId` — 0 today,
  the parent's cell post-fix. The gate is therefore exactly a no-op at HEAD
  for both parent classes and exactly required after.
- The `continue` skips all three downstream branches (`CreateSupersessionRecovery`,
  `RebucketLiveEntity`, `SpatialRecovery`), which is correct: none of them was
  reachable for this population before, so nothing legitimate is newly dropped.
- The predicate matches the precedent it cites
  (`RuntimeSetPositionState.IsAffectedCollisionResident`), and stale committed
  entries for a replaced child are cleared by `RemoveCommittedChild` inside
  both generation paths.

No finding beyond noting the gate is GUID-keyed while the loop iterates
canonical records; that is safe today only because `_lastAcceptedByChild` is
purged on every child generation change.

---

## A8 — LOW — `AcceptCreateObjectRelation` remains a public, unguarded producer

`ParentAttachmentState.cs:391-394` still accepts any `ParentInstanceSequence`
and is called raw from twelve test sites. The tripwire is the only thing
standing between a future caller and #319 verbatim — and per A1 that tripwire
is optional (`resolveParentInstance` defaults to `null`) and fires after the
canonical commit. Consider making the parameter required, or making the
late-bound wrapper the only public entry.

---

## Implementer claims — adjudicated

| claim | verdict |
|---|---|
| 1. `OnSpawn`'s deferred branch is structurally unreachable in production | **VERIFIED** (`RuntimeEntityObjectLifetime.cs:797-812`). But it is unreachable for the `CreateParentUpdate` producer too, which the diff and its test both deny — see A6. Not acceptable as an untested fail-safe in its current shape. |
| 2. Sabotage: restoring the literal `0` fails the player row via the tripwire before the value assertion | **CONCERN REFUTED.** With the tripwire removed and the literal `0` restored, `CommitProjection` returns true and files `(parentGuid, 0)`, so `Assert.Equal(parentIncarnation, committedInstance)` fails on its own; the D1 assertion (`childCanonical.FullCellId == parent.Canonical.FullCellId`) and the D2 assertion also fail independently, because `CommitAcceptedParentCellless`'s `parent.Incarnation == parentInstanceSequence` gate and `ChildrenAttachedToParent(guid, Incarnation)` both miss. The matrix proves what it claims. |
| 3. 91 added / 24 removed, net +67 vs a 35–80 estimate | **VERIFIED exactly.** The overshoot is entirely the tripwire plumbing plus the second commit site — i.e. the two things A1 says should be restructured. |

## §6 gaps — do they block?

- **Test 5 (residence non-event)** — **does not block.** Contract §3.3's premise
  is verified at `RuntimeEntityObjectLifetime.cs:2751-2752`
  (`if (canonical.FullCellId != 0u) Entities.SetFullCell(canonical, 0u, 0u)`
  immediately before `InitialCreateResidences.Begin`), so `Begin`'s
  `FullCellId != 0` refusal is genuinely unreachable and no production code on
  that path changed.
- **Test 6 (unwield classification population)** — **does not block.** No
  production code on the classifier path changed, and the contract already
  recorded (`af828a8a`) that the short-circuit OR at
  `RuntimeAuthoritativePositionRouteClassifier.cs:391` labels the drop
  `teleport-ts` either way, so the observable outcome is unchanged.
- **Test 9 (ledger convergence)** — **BLOCKS.** It is the one missing test whose
  subject is exactly where the new state lives, and A2/A5 are precisely
  ledger-convergence defects: a late-bind relation that outlives its child's
  generation, blocks `Resolve` for that child forever, and accumulates in
  `_unresolvedByChild` with no bound. A convergence test written against the
  new deferred population (child deleted with a deferred relation pending;
  parent guid never arriving; teardown *and mid-session* counts) would have
  surfaced both.

## Route 7 invariants

Re-checked against the diff and found intact: removal propagates ZERO
(untouched; a new dual-parent-class withdrawal test was added);
`RebucketLiveEntityPresentationOnly` is not made a canonical writer (the D4
demotion stands, `LiveEntityRuntime.cs:1033-1060` untouched); no per-tick
cross-cell rebuild is introduced; no new `SetFullCell` call site exists; the
ParentEvent path's AP-132 incarnation gating is preserved verbatim in
`Resolve`'s `else if` arm.

## What a PASS would need

1. **A1** — move the incarnation check above the canonical commit at both sites
   and make it a logged refusal returning `false` (rejecting the staged
   relation), not a throw.
2. **A2/A3/A6** — either give the deferred branch a real production path and a
   test that reaches it through production routing, or reduce it to a loud
   refusal with no independent state. If it stays, close the missing child
   freshness gate and the placeholder-sequence filter hazard.
3. **Test 9** — a ledger-convergence test over the new deferred population,
   asserting both mid-session boundedness and teardown convergence.
4. **A4** — correct the contract's §3.2 premise and name the creature-parented
   shadow behaviour change in the connected gate's Half B watch list.

Nothing here requires redesign; items 1 and 3 are small, item 2 is a scoping
decision.