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

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:

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.