acdream/docs/research/2026-08-06-280-review-architecture.md

#280 architecture review — ownership, layering, blast radius, test quality

• Commit under review: 3aab05b0 — fix(streaming): derive the portal reveal window from the live streaming radii (#280)
• Branch / HEAD: claude/acdream-physics-divergence-5aa784 @ fafc0b65
• Worktree: .claude/worktrees/peaceful-visvesvaraya-e0a196
• Scope: ownership, layering, blast radius, test quality. Retail fidelity is a separate reviewer's.
• Mode: read-only. Production edits were made only as sabotage probes and reverted; the tree is clean at fafc0b65 with no working-tree changes.

Verdict: FAIL

One confirmed defect, reproduced end-to-end through production code paths. The predicate D2 widened from a 3×3 always-Near neighbourhood to the full Far window now demands IsRenderReady from landblocks that the streaming system permanently takes out of render-readiness while leaving them loaded. There is no code path that restores them. Under two ordinary player flows — one of which is the exact mid-hold radius change this commit added ReconcileDestinationReservationRadius to support — the reveal gate becomes unsatisfiable and the client stays in "In Portal Space - Please Wait…" forever.

Everything else checked out. Layering held, the no-window host is genuinely untouched, the allocation fix is real, and the tests are unusually well disciplined about not re-encoding the constant under test. The failure is a single missed state transition, not sloppy work — but it is the failure mode the change's own proof obligation P2 was written to exclude, discharged against the wrong predicate.

---

Defects

D-1 — A demoted-but-loaded landblock is permanently not IsRenderReady, so the widened gate can never open (CONFIRMED, reproduced)

Severity: defect. Blocks the fix.

src/AcDream.App/Streaming/StreamingController.cs:270-278 — the new outer arm requires _state.IsRenderReady(canonical) of every in-bounds member out to farRadius.

IsRenderReady is not a property of "loaded":

• src/AcDream.App/Streaming/GpuWorldState.cs:179-181 IsRenderReady = _loaded.ContainsKey(id) && _wbSpawnAdapter.IsLandblockRenderReady(id)
• src/AcDream.App/Rendering/Wb/LandblockSpawnAdapter.cs:135-139 returns false when the landblock has no registration, or a registration with WantsLoaded == false.

A Near→Far demote destroys exactly that registration while keeping the landblock loaded:

• src/AcDream.App/Streaming/StreamingController.cs:555-559 DemoteLandblock → LandblockPresentationPipeline.EnqueueNearLayerRetirement
• src/AcDream.App/Streaming/LandblockRetirementCoordinator.cs:309-312 a NearLayer ticket still runs CoreStages, which includes LandblockRetirementStage.MeshReferences
• src/AcDream.App/Streaming/LandblockRetirementCoordinator.cs:738-740 → GpuWorldState.ReleaseLandblockMeshReferences
• src/AcDream.App/Streaming/GpuWorldState.cs:1559-1560 → LandblockSpawnAdapter.OnLandblockUnloaded → WantsLoaded = false, registration dropped
• src/AcDream.App/Streaming/GpuWorldState.cs:1800-1835 meanwhile DetachNearLayer keeps _loaded[canonical] and flips _tierByLandblock[canonical] = Far

Nothing re-publishes an already-loaded landblock. RecenterTo (StreamingRegion.cs:232-244) emits ToLoadFar only for ids absent from _tierResidence, and a demoted block is still present as Far. Only a promote back into the Near ring (StreamingRegion.cs:239-244 → PublicationKind.PromoteExisting → ActivateLandblockPresentation → OnLandblockLoaded) restores the registration. Members of the band NearRadius+1 … FarRadius are never promoted, so they stay dead for the rest of the window's life.

Evidence (temporary probes, run then removed):

Probe	Result
GpuWorldState.IsRenderReady after LandblockPresentationPipeline.BeginNearLayerRetirement (the production demote entry point)	false
GpuWorldState.IsRenderReady after GpuWorldState.RemoveEntitiesFromLandblock (same DetachNearLayer + ReleaseLandblockMeshReferences pair)	false
Full Near 5×5 window; controller.IsRenderNeighborhoodResident(dest, 1, 2) before one outer-ring demote	true
…the same call after that single demote	false, and never recovers

All three probes used a real GpuWorldState + real LandblockSpawnAdapter + real LandblockPresentationPipeline.

Reachable failure scenarios

1. Mid-hold quality change (the one this commit explicitly added code for). StreamingController.ReconfigureRadii at :484-488 emits DemoteLandblock for every currently-Near landblock that the new preset puts in the Far band. Presets are High (4,12) → Low (2,5) (src/AcDream.UI.Abstractions/Settings/QualityPreset.cs:31-34), so a High→Low switch demotes the Chebyshev 3–5 band, which is inside the new Far window. The gate then requires IsRenderReady from all of them. Result: the hold never ends, and WorldRevealCoordinator.ReconcileDestinationReservationRadius (:521-543) cheerfully re-opens a reservation on a square that can no longer converge. Note also that NearRadius/FarRadius only publish at transaction convergence (StreamingController.cs:886-888) while the demote mutations run earlier (:860-876), so for several frames the barrier measures the OLD wide window against blocks already demoted for the NEW narrow one.

2. A reveal that does not recenter the origin. LocalPlayerTeleportController.cs:934 only calls _streaming.BeginRecenter when transition.ChangesStreamingCenter, i.e. when the destination landblock differs from the world-origin landblock. Origin recenter is the only thing that clears the window (GpuWorldState.cs:1481 _loaded.Clear()). Walking ≳ NearRadius + 2 landblocks from the last origin (StreamingRegion.cs:263-270) demotes blocks into the Far band; a subsequent teleport back to the origin landblock (lifestone recall → travel → lifestone recall) takes the no-recenter path and inherits them. Blocks that land back inside the Near ring are promoted and recover; blocks in near+1 … far do not.

Why the old gate was safe: at OutdoorNeighborhoodRadius = 1 the required set was the destination's own 3×3, which during a hold is pinned to the streaming centre (StreamingFrameController.SelectObserver:194-200 freezes the observer at _origin while PlayerState.PortalSpace) and therefore always Near tier. Demotion inside the Near ring cannot happen. The defect is introduced by this commit, not exposed by it.

Why P2 missed it. docs/research/2026-08-05-280-contract.md:876-884 states P2 as "an outer-ring member cannot be evicted while it is inside FarRadius" and cites the FarRadius + 2 unload threshold. That is true and irrelevant: the gate's atom is IsRenderReady, not residency. P2 was discharged against the wrong predicate. Every remaining sentence of the static proof (hysteresis, recenter, dungeon collapse) is correct.

Also note: the P1 test's own comment (tests/AcDream.App.Tests/Streaming/StreamingControllerReadinessTests.cs:567-570) describes its subject as "A Near-shaped completion that the streaming window has since demoted to Far". It is not — it is a fresh PublishAsFar of a landblock that was never loaded, which is the case that does hold. The comment names the one case that is false. A future reader will take it as coverage of the demote path.

---

Latent risks

R-1 — ACDREAM_PROBE_REVEAL_RADIUS=0 is accepted by the parser and rejected by Runtime

src/AcDream.App/Streaming/StreamingDiagnostics.cs:54-55 accepts any value >= 0. ApplyRevealRadiusOverride then yields StreamingRevealWindow(0, 0), so RequiredWindow returns far = 0 for an outdoor destination, and src/AcDream.Runtime/World/RuntimeWorldTransitState.cs:585-597 fails invalid-readiness-shape on every acknowledgement (outdoor ⇒ >= 1). The A/B probe would hang the route it is meant to measure. The documented value is 1; the parser should refuse 0 rather than let the two halves of the same commit disagree. One line.

R-2 — the composite-warmup trigger moved onto the far window's critical path, undocumented

WorldRevealReadinessBarrier.Prepare:120-133 starts composite preparation only once _isRenderNeighborhoodReady(dest, near, far) is true. Before this commit that predicate was the destination's 3×3; it is now the entire 25×25. The commit message and D3 justify keeping the composite radius at NearRadius (correct — Far builds carry no entities), but say nothing about the trigger. Net effect: composite upload can no longer overlap far-ring streaming; the hold is longer than the streaming work alone requires by the whole warmup duration. This is a real serialisation the "expect longer holds" paragraph does not account for, and it is separable — warming composites at Near-ready would restore the overlap without weakening the gate.

R-3 — two different windows describe "destination publication incomplete"

StreamingController.cs:719-724 computes the destination-lane preference with (Math.Min(NearRadius, DestinationRadius), DestinationRadius) — live NearRadius against the reservation's radius — while the gate uses RequiredWindow's (clamp(NearRadius, 0, FarRadius), FarRadius). Between a preset change and reservation reconciliation these name different squares, so the work-lane prioritisation can consider the destination complete on a square the gate still rejects. Cosmetic today; it becomes a real starvation question once R-1/D-1 are fixed and holds get long.

R-4 — Runtime lost its only cross-host consistency check

The indoor ? 0 : 1 equality became indoor ⇒ 0, outdoor ⇒ ≥ 1 (RuntimeWorldTransitState.cs:585-597). The layering argument is right — Runtime cannot learn the graphical host's streaming configuration, and plumbing App radii in would be exactly the C5b failure. The cost is that Runtime can no longer detect an App regression to a hardcoded radius: a future revert of D1 would emit 1 and pass. Accepted, but worth recording that the invariant is now shape-only and the only remaining guard on the value is the App-side tests.

---

What was checked and is sound

Layering / ownership (attack 6) — held. WorldRevealCoordinator and WorldRevealReadinessBarrier are internal to AcDream.App. src/AcDream.Headless/AcDream.Headless.csproj references only AcDream.Runtime. Both non-graphical producers (HeadlessSessionWorldProjection.cs:979, RuntimeLiveEntitySessionController.cs:782) keep emitting their own centre-ring token and are legal under the loosened shape by construction, and both were annotated in place explaining why they must not track the graphical radius. No App type crossed into Runtime. AcDream.Headless.Tests passes 89/89 including its dependency/loaded-assembly guards. C5b's lesson was applied, not repeated.

P1 (attack 2) — independently verified TRUE, by reading rather than by reading the test. PublishAsFar (LandblockPresentationPipeline.cs:432-522) constructs a LoadedLandblock with Array.Empty<WorldEntity>() and PhysicsDatBundle.Empty; the spatial commit (LandblockPresentationPipeline.cs:900-916) routes PublicationKind.Far through CommitLandblockSpatial, which returns a publication with RequiresActivation defaulted true (GpuWorldState.cs:24), so ActivateLandblockPresentation (:1009-1035) calls OnLandblockLoaded with an empty entity list and empty AdditionalRenderIds. The registration is created with WantsLoaded = true and both reference dictionaries empty, so IsLandblockRenderReady returns true with no IWbMeshAdapter upload. The one early-out (GpuWorldState.cs:913-923, a stale Far completion over a live Near tier) returns RequiresActivation: false, but that case is already registered. P1 holds. The fix shape does not collapse. Its failure is D-1, a different transition.

Same check for the collision arm, which is equally load-bearing and was not called out as a proof obligation: LandblockPhysicsPublisher.AdvanceBeginOne (:390-435) builds the terrain surface from the heightmap with no dat bundle and stages it, so a Far publication does register terrain collision; AdvanceDemotion (:675-683) calls DemoteCollisionToTerrain, which retains terrain. IsNeighborhoodTerrainResident therefore stays satisfiable across a demote. The render arm is the only one that breaks.

Reveal-hang sweep (attack 1) — all other configurations converge.

Configuration	Result
Map edge (coords clamped 0/254)	Safe. Gate skips > 254 (StreamingController.cs:265-266); StreamingRegion loads up to 0xFF (:80). The loaded set is a strict superset of the required set, and PhysicsEngine.cs:157 uses the identical bound.
Dungeon → outdoor	Safe. TryCommitOriginRecenterCore:1235-1251 clears _collapsed for a non-dungeon destination and nulls _region so the next tick bootstraps the full window.
Indoor destination	Safe. RequiredWindow returns (0,0) before touching the live window (WorldRevealReadinessBarrier.cs:206-215), so no streaming state can gate it.
Recenter in flight	Safe. Tick is blocked while _originRecenterRetirement is open, radii requests defer, and _loaded.Clear() (GpuWorldState.cs:1481) guarantees every member of the rebuilt window gets a fresh publication and registration.
Destination/observer landblock offset	Safe during a hold: StreamingFrameController.SelectObserver:194-200 pins the observer to _origin while PlayerState.PortalSpace, so the gate's inner ring and the streaming Near ring are the same square. Worth noting the new inner arm has zero margin here where the old one had NearRadius − 1; the pin is now load-bearing.
FarRadius == 0 on an outdoor destination	Not reachable from presets (min far = 5); reachable only via R-1.

D6 allocation fix — real, and the sabotage number is honest. PhysicsEngine.cs:49-52,146-149. Single production call site (SessionPlayerComposition.cs:384), main thread, leaf method — the instance-owned scratch is safe, and it is cleared at entry so no stale state can leak across sessions.

Test discipline — good, with two gaps. No test re-encodes the constant under test: every radius assertion references the fake window's own input (WorldRevealReadinessBarrierTests, WorldRevealDerivedWindowIntegrationTests). No new Skip=. No test deleted — one was renamed (OutdoorReveal_JoinsNearRenderTexturesAndTerrain → RequiredWindow_IsRereadOnEveryEvaluationWithoutReconstruction) with its original assertions carried into a new OutdoorReveal_JoinsRenderTexturesAndTerrainOverTheDerivedWindow; coverage is preserved and the +36 arithmetic holds.

Gaps:

• No test exercises a demote. That is D-1.
• WorldRevealDerivedWindowIntegrationTests advertises itself as end-to-end against "the REAL StreamingController, GpuWorldState, and PhysicsEngine", but constructs new GpuWorldState() with no spawn adapter, so IsRenderReady degenerates to IsLoaded via the ?? true at GpuWorldState.cs:181. The single most load-bearing predicate in the whole change is stubbed out by a null in the test named after it. It also builds its own revealWindow lambda rather than the production one in SessionPlayerComposition.cs:373-379, so the ApplyRevealRadiusOverride wrapper is never exercised in composition.

Sabotage spot-checks — 4 of 9 families reproduced (asked for ≥ 3).

Sabotage	Named test(s)	Result
PhysicsEngine: scratch → new HashSet<uint>()	WarmedNeighborhoodQuery_AtTheFarRadius_AllocatesNothing	FAIL, "allocated 27,712,000 bytes" — S5's claimed figure reproduces to the byte
StreamingController: revert tier split to !IsNearTier || !IsRenderReady	TieredWindow_OuterRingFarTierMemberIsResident, TieredWindow_AbsentOuterRingMemberIsNotResident, TieredWindow_MapCornerDestinationConvergesAtAWideRadius, OutdoorReveal_HoldsUntilTheWholeDerivedWindowIsPublished ×2, LoginReveal_UsesTheSameWidenedGateAsPortalArrival	6 FAIL
RuntimeWorldTransitState: shape → == (isIndoor ? 0 : 1)	OutdoorReadinessShape_AcceptsAnyDerivedStreamingRadius	6 FAIL (all inline radii)
WorldRevealCoordinator: neuter ReconcileDestinationReservationRadius	MidHoldRadiusChange_ReopensTheReservationOnTheSameGeneration	1 FAIL

All four reverted; git status clean afterwards. No sign of the C5b-D3 class (a test that passes with its own change reverted) in the families checked.

Suite at HEAD fafc0b65, Release, verified by running it: 11,179 passed / 4 skipped / 0 failed — exactly the expected 11,178 + 1 Core settler test. Per project: UI 546, Content 124, Runtime 1217, Cli 4, Bake 15, App 4157/3 skips, Headless 89, Core.Net 764, Core 4263/1 skip. None of #302/#308/#321 surfaced in this run and none are conflated with the finding above.

Process rules — clean. No suppression flag, grace period, retry loop, or symptom guard was introduced. ACDREAM_PROBE_REVEAL_RADIUS lives in a diagnostic owner per Code Structure Rule 5 and is correctly argued as a measurement probe rather than a shipped knob. AD-2 was amended and AP-149 filed in the same commit, satisfying the divergence-register rule. The ACDREAM_STREAM_RADIUS CLAUDE.md correction is accurate against SessionPlayerComposition.ComposeCore:249-257.

---

Recommended disposition

Do not ship the gate widening until D-1 is closed. The two shapes worth considering, in preference order:

1. Make a demote re-register the Far tier. The demote already leaves a fully valid Far-tier landblock behind; the registration it drops is the Near mesh set. LandblockRetirementStage.MeshReferences on a NearLayer ticket should release the Near references and then re-assert an empty Far registration, exactly as PublishAsFar does — i.e. the same OnLandblockLoaded(landblock, empty) call the Far publication makes. This makes IsRenderReady mean "drawable at its current tier", which is what both the render path and the gate already assume it means.
2. Re-publish demoted members inside a destination reservation. Weaker: it fixes the gate without fixing the predicate, and leaves the next caller of IsRenderReady holding the same trap.

Whichever is chosen, it needs a test at the demote transition — Near publish → demote → IsRenderNeighborhoodResident(dest, near, far) — and the WorldRevealDerivedWindowIntegrationTests fixture should be given a real LandblockSpawnAdapter so its IsRenderReady stops being a tautology. R-1 and R-2 are one-line and one-decision respectively and can ride along.