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

Issue #319 — architecture / adversarial review, ROUND 2 (delta) — 2026-08-05

Verdict: FAIL — one blocking finding, a one-line test change.

Round 1: 2026-08-05-issue-319-architecture-review.md.

Every production concern from round 1 is closed, and I verified each by reading the code rather than accepting the summary. The A1 ordering fix is correct on both hosts, the A6 deletion argument is provable and all three of its links check out, and A2/A3/A5 are gone by deletion rather than relocated. The single blocker is evidentiary: the new test that exists specifically to guard the A1 ordering property has three headline assertions that are vacuous in its own fixture, so the guard against the exact regression it was written for is one incidental assertion. That is fixable by changing one constant.

Verification performed for this round:

• dotnet build -c Release — succeeded, 0 warnings, 0 errors.
• AcDream.Runtime.Tests (ParentAttachmentStateTests, RuntimeEntityChildCellPropagationTests) — 38/38 (was 32).
• AcDream.App.Tests (EquippedChildProjectionWithdrawalTests, LiveEntityHydrationControllerTests, LiveEntityPresentationControllerTests, LiveEntityRuntimeTests) — 208/208 (was 206).
• Production delta measured independently: 76 non-comment lines added, 13 removed — matches the stated 76/13, down from 91/24.

---

B1 — MAJOR, BLOCKING — PrepareAndTryRealize_MismatchedIncarnation_RefusesBeforeCanonicalCommit's three canonical assertions cannot fail, in either ordering

tests/AcDream.App.Tests/Rendering/EquippedChildProjectionWithdrawalTests.cs, the new A1 test.

The test injects wrongRelation with ChildPositionSequence: 1 against a child created by fixture.RegisterOnly(childGuid, generation: 1, hasPosition: true). That helper builds the spawn from ControllerFixture.SpawnData, which sets Timestamps.Position: 0 and never sets the top-level PositionSequence (WorldSession.EntitySpawn's ushort PositionSequence = 0 default, WorldSession.cs:163). Unlike its sibling NoPositionCreateParent_CommitsAfterParentPartArrayValidation, this test does not call TryApplyCreateParent/TryApplyParent first, so nothing ever advances the child's POSITION_TS.

The canonical half is CommitStagedParent → RuntimeEntityObjectLifetime.TryCommitParent:1406-1412 → InboundPhysicsStateController.TryCommitParent:299-312, whose gate is:

if (!TryGet(childGuid, out gate, out child)
    || gate.PositionTimestamp != positionSequence   // 0 != 1
    || child.PositionSequence != positionSequence)  // 0 != 1
{ accepted = default; return false; }

positionSequence is relation.ChildPositionSequence = 1; both gate values are 0. The canonical commit refuses on POSITION_TS before the incarnation is ever consulted — in either ordering.

Consequence. Revert the fix (put CanCommitIncarnation back after CommitStagedParent, or delete it entirely) and trace the test:

assertion	reverted-order outcome
Assert.Null(snapshot.ParentGuid)	still passes — TryCommitParent refused, snapshot untouched
Assert.NotNull(snapshot.Position)	still passes — same reason
Assert.False(TryGetCommittedParent(...))	still passes — CommitProjection never reached
Assert.False(TryGetStagedProjection(...))	fails — return default leaves the relation staged

So the test does fail under sabotage — the letter of "every new test must fail against broken behaviour" is met — but it fails on the stranding assertion, not the tearing one. The claim relayed to me ("asserts the child's snapshot stays un-parented; sabotage-verified by reverting the order") is literally true and materially misleading: the snapshot assertions never execute against a live canonical commit, so they assert nothing.

Why this blocks rather than being cosmetic. The regression this guard exists to catch is "someone lets the canonical half run before the incarnation check." A plausible future variant — moving the check back inside CommitProjection while also calling RejectProjection on refusal — would re-tear the transaction and pass this test on all four assertions. The guard does not cover its own finding.

Fix (one line). Change wrongRelation's ChildPositionSequence: 1 to 0 so it matches the fixture's gate. Then under a reverted ordering TryCommitParent succeeds, nulls snapshot.Position, writes ParentGuid, and all three canonical assertions bite — while the fixed code still refuses at CanCommitIncarnation (which reads only the parent incarnation and is independent of POSITION_TS) and ValidateParentProjection still returns Ready. Re-run the sabotage and confirm the failure message names one of the three snapshot assertions, not the staged-projection one.

---

What I verified as CLOSED

A1 — ordering fix: torn-transaction window genuinely closed on both hosts ✓

• ParentAttachmentState.CanCommitIncarnation:589-605 is genuinely pure: it reads resolveParentInstance and writes only stderr. No table is touched.
• Graphical (EquippedChildRenderController.cs:974-989): the pre-check runs before CommitStagedParent, refuses via RejectProjection, and returns CanAdvanceWireQueue: true. Correct on three counts I checked separately: (i) no canonical mutation precedes it; (ii) RejectProjection clears _stagedByChild, so Resolve's early return (:457-458) is unblocked — the round-1 "stranded forever" outcome is gone; (iii) CanAdvanceWireQueue: true lets ResolveAndTryRealize's while (true) loop continue, and it still terminates, because each iteration either breaks or consumes one relation from the finite _unresolvedByChild queue, and Resolve can only re-stage on exact incarnation equality — which is precisely the condition CanCommitIncarnation accepts.
• Headless (RuntimeLiveEntitySessionController.cs:408-417): same pre-check, before TryCommitParent, with RejectProjection + return false.
• CommitProjection now returns false instead of throwing, and its internal check (:628-629) sits after the _stagedByChild lookup but before RemoveCommittedChild/_lastAcceptedByChild, so it mutates nothing before refusing.
• The residual window I looked for does not exist: between the pre-check and CommitProjection, the only mutation is TryCommitParent writing the child's snapshot; the parent's _snapshots entry — the tripwire's input — is untouched, so the two reads cannot disagree.

Route 3's N3 principle is now honoured: the condition is refused loudly and recoverably, never fatally.

A6 — the deletion argument: all three links independently verified ✓

This was the justification for deleting rather than fixing, so I checked it rather than accepting it.

1. RegisterEntityCore defers the whole CreateObject. RuntimeEntityObjectLifetime.cs:795-812: uint parentGuid = incoming.ParentGuid ?? incoming.Physics?.Parent?.Guid ?? 0u; then if (beginInitialResidence && parentGuid != 0u && !Entities.TryGetActive(parentGuid, out _)) → EnqueueDeferredCreate + DeferredForParent: true. Confirmed this is the earliest branch in the method that can admit a create — it precedes PreviewCreateDisposition (:818), the pending-residence early return, and AcceptCreate (:866-868).
2. beginInitialResidence is true for every graphical-host CreateObject. LiveEntityRuntime.cs:583 is the sole App registration entry and uses RegisterEntityWithInitialResidence. The three internal replay/drain sites (RuntimeEntityObjectLifetime.cs:234, :317, :400) also use it, so the gate re-applies on deferred-create replay rather than being bypassed by it.
3. CreateParentUpdate cannot precede the gate. It exists only inside InboundCreateResult.SameGenerationEvents, produced by BuildSameGenerationEvents in InboundPhysicsStateController.AcceptCreate (:87) / AcceptCreateDeferredSameGeneration, both called at RuntimeEntityObjectLifetime.cs:866-868 — after the gate. I traced every consumer and all three originate there: LiveEntityHydrationController.cs:340 (result.SameGenerationEvents), LiveEntityNetworkUpdateController.cs:400-401 (via ApplySameGeneration/LiveEntitySameGenerationUpdateRouter), and RuntimeEntityObjectLifetime.cs:2544 (AdmitSameGenerationCreate, also post-gate).

The chain holds. AcceptLateBoundCreateObjectRelation's else branch is structurally unreachable for both producers, and the deletion removes no real path.

A2 / A3 / A5 — gone by deletion, not relocated ✓

Repo-wide grep for DeferCreateObjectRelation, LateBindParentInstance, and lateBind returns zero hits in src/ and tests/ (only unrelated InteractionUiLateBindings matches). ParentAttachmentState.Resolve is byte-identical to HEAD — the diff touches only CanCommitIncarnation and CommitProjection, and ParentAttachmentRelation gains no field. So:

• A2 (missing child POSITION_TS gate on a relation that skipped accept) — no relation skips accept any more; Resolve's single path is the original ParentEvent one.
• A3 (placeholder ParentInstanceSequence = 0 misread by FilterParentCandidates) — no relation is enqueued with a placeholder; the only unresolved-queue writer is the pre-existing Enqueue.
• A5 (unbounded accumulation of deferred CreateObject relations) — that population no longer exists. The surviving _unresolvedByChild accumulation question is pre-existing ParentEvent behaviour, untouched by this fix.

The replacement — a loud stderr line with no state mutation — is the right trade: if the upstream invariant ever breaks, the failure mode is a non-attached (cell-less, position-less) child plus a log line, not a resurrected cross-generation relation.

Test 9 — covers the shapes I named ✓

Three dual-class tests in ParentAttachmentStateTests. Checked against the A2/A5 shapes specifically:

• LedgerConvergence_ChildRemoval_ZeroesEveryTable asserts all four counters (CommittedRelationCount, RecoveryRelationCount, StagedRelationCount, UnresolvedRelationCount) plus the _committedChildrenByParent reverse index via ChildrenAttachedToParent — that reverse index is the table round 1's stranding scenarios would have leaked into, and asserting it separately from HasCommittedParent is the right call.
• LedgerConvergence_ParentRemoval_ZeroesEveryTable covers RemoveObject's parent-reference sweep — the A2 "relation outlives its parent" half.
• LedgerConvergence_Teardown_ZeroesEveryTableWithMixedPendingState holds a committed child and an unresolved ParentEvent simultaneously before Clear() — the A5 mixed-state shape.

Each fails against broken behaviour non-vacuously (they establish nonzero counts before the removal and assert zero after, so a no-op removal fails).

A4 — sufficient ✓

The RestoreShadow comment now states the correction plainly (route 7's D1 already gives creature-parented children a nonzero cell, so the clause is a live behaviour change for that class), and contract §7 Half B now carries the explicit watch instruction (:549, :554). Given the population is bounded by ShadowObjectRegistry.UpdatePosition's unregistered-entity no-op (ShadowObjectRegistry.cs:696-697), naming it in the connected gate is the right resolution — a synthetic test cannot settle it, and the gate is where it gets observed.

A8 — confirmed non-blocking ✓

AcceptCreateObjectRelation remains public with an optional resolver. With A1's pre-check now at both production sites, a future producer regression is caught before any mutation, so the cost of tightening the API is not worth the ~12 test call sites.

---

Non-blocking observations

B2 — LOW — the gate and the producer test different oracles. The upstream gate (RuntimeEntityObjectLifetime.cs:797) uses Entities.TryGetActive (active-record table); AcceptLateBoundCreateObjectRelation uses _liveEntities.TryGetSnapshot (the InboundPhysicsStateController._snapshots map). Active ⇒ snapshot holds because AddActive is fed from the snapshot AcceptCreate wrote, but there is a narrow inversion inside TryDeleteEntity: TryDelete removes _snapshots[guid] (InboundPhysicsStateController.cs:101-102) several statements before RemoveActive (RuntimeEntityObjectLifetime.cs:~2094). Reaching the else branch through it would need a re-entrant child CreateObject inside that window — not reachable from a single-threaded pump. Worth one sentence in the remark, since the remark's argument is phrased in the gate's predicate but enforced with a different one.

B3 — LOW — the structural invariant is host-scoped and the remark does not say so. RuntimeLiveEntitySessionController.cs:144 calls Entities.RegisterEntity (beginInitialResidence: false) when _worldProjection is null, which bypasses the parent-deferral gate entirely. That host has no EquippedChildRenderController, so the producer is unreachable there — but the remark reads as an unconditional claim. One clause ("in the graphical host; the content-less direct host has no CreateObject-carried relation producer at all") would make it audit-proof.

B4 — INFO — the refusal is unconditionally destructive. RejectProjection discards the relation, where Resolve's equivalent staleness rule (ParentAttachmentState.cs:484-498) is conditional: a relation naming a newer-than-live parent generation is retained until that generation arrives. Unreachable today, because a staged relation can only have been staged on exact equality — but if the staging rules ever loosen, discard is the wrong arm for the packet-ahead case.

B5 — INFO — CommitProjection's XML doc slightly overclaims. "non-tearing on its own terms for ANY caller" is true of this method's tables, not of a caller's canonical state: a caller that commits canonically first and then calls CommitProjection still tears when the internal check refuses. That is exactly the shape A1 removed, and both production sites now pre-check, so the guard is genuine belt-and-braces — the sentence just shouldn't imply it protects callers who get the ordering wrong.

B6 — LOW — unbounded stderr on a per-packet path. Both new Console.Error.WriteLine sites (AcceptLateBoundCreateObjectRelation's else, CanCommitIncarnation's refusal) log unconditionally. If the invariant ever breaks for a repeating producer, this spams once per packet. A log-once-per-guid latch would suit the "should be structurally unreachable — investigate if seen" framing better.

B7 — INFO — test 9's scope. The three tests exercise RemoveChild, RemoveObject, and Clear — not DeleteGeneration/EndGeneration, the two paths carrying the WaitOwner is Parent retention rule (ParentAttachmentState.cs:819-821, :844-846). That rule is pre-existing and unchanged by this fix, so it is legitimately out of scope; the ledger claim simply should not be read as covering generation boundaries.

---

What a PASS needs

One change: ChildPositionSequence: 1 → 0 in PrepareAndTryRealize_MismatchedIncarnation_RefusesBeforeCanonicalCommit's wrongRelation, then re-run the ordering sabotage and confirm the failure names a snapshot assertion rather than the staged-projection one.

Everything else in this round is PASS-quality. B2/B3 are one-clause comment improvements; B4–B7 are informational.