Architecture Review Report — 2026-09-07

Mode: /architecture-review (full; incremental over architecture-review-2026-09-01.md; RTM built — stories and tests now exist) · Verdict: CONCERNS
Engine: Unity 6.3 LTS (6000.3.18f1) · URP 17.3.0 2D (Render Graph) · Shader Graph 17.3.0 · Input System 1.20.0 · Addressables 2.9.1 · Newtonsoft 3.2.2 · C# 9 / .NET Standard 2.1 · NGO not installed (Gate 0)
Loaded: 5 GDDs (+ systems-index.md, game-concept.md) · 10 ADRs (all Accepted) · tr-registry.yaml (111 IDs) · traceability-index.md · control-manifest.md · docs/registry/architecture.yaml (49 stances) · engine reference (VERSION, breaking-changes, deprecated-apis, current-best-practices, modules) · 87 story files · 118 test files
Method: the prior review's resolutions re-verified against source, not the changelog; ADR diffs since dc7c1b0 read in full; an independent cross-ADR + implementation-deviation pass (19 rulings, 8 conflicts, 22 cleared items) and an engine-specialist audit (16 findings), both cross-checked by the reviewer against the files they cite. Every "unverified" item from both passes is carried in Appendix D rather than silently dropped.

Prior review's items — verified against source

Pre-gate checklist (Glob-checked): tests/unit/ ✅ · tests/integration/ ✅ · .github/workflows/tests.yml ✅ · design/accessibility-requirements.md ✅ · design/ux/interaction-patterns.md ✅.

Traceability Summary

No GDD technical requirement changed since 2026-09-01. game-concept.md's edits are the Gate-0 re-affirmation prose; systems-index.md was renumbered. No ADR was added. Coverage is therefore unchanged: 73 ✅ / 26 ⚠️ / 12 ❌ of 111 (66 %). Foundation and Presentation are gap-free; all 12 gaps route to ADR-0008/0009/0010/0011/0012 or the TR-cam-002 implementation deferral. tr-registry.yaml is unmodified — no new IDs, no rewording, no deprecations.

Requirements Traceability Matrix — new this review

Full chain GDD → ADR → Story → Test, every stated evidence path checked on disk (docs/architecture/requirements-traceability.md, 111 rows).

Per layer: Foundation 31/39 (79 %) · Presentation 6/8 (75 %) · Core 21/38 (55 %) · Feature 3/26 (12 %).

Findings from building it:

• Every Done story has a real test file. The first pass reported 27 "missing"; all were parser noise except three stated-name drifts in story text: foundation-input/story-005 names InputAdapterPlayModeTests (actual Tests/PlayMode/InputAdapterTests.cs); foundation-input/story-007 names RebindRoundTripTests (actual RebindServiceTests.cs); presentation-tide-tint/story-006 names TintFenceTests (actual TideTintFenceTests.cs). Documentation drift, not missing tests.
• Eight bootstrap/game-data stories carry no TR-ID — they trace to ADR sections ("ADR-0003 §6", "Migration step 2"). The RTM attributes six by section (TR-net-006/007/009, TR-bb-003/008, TR-combat-012), each confirmed by the test file's own header citing the story. Two (bootstrap-001 asmdef graph, bootstrap-008 CI wiring) and four game-data infrastructure stories are genuinely cross-cutting and stay un-attributed.
• TR-inv-002 and TR-inv-004 are COVERED / NO ADR: their single-machine halves are implemented and tested (core-inventory-carry 001/003) while the authority-rail halves remain ADR-0010's. The index's ❌ is correct for the ADR column and now misleading about implementation state — annotated in the RTM.

Cross-ADR Conflicts and Deviation Rulings

Severity-ordered. In every case below the shipped code is correct; the defect is in ADR text — except C7/#3, where the code should change. Full rulings (19) are in Appendix A; full conflict blocks (8) in Appendix B.

Checked — no conflict (22 items with evidence, Appendix B): the amended Data → Tide edge is consistent with every other ADR and both purity fences (which carry a vacuity guard); Presentation → Runtime is the legal direction; asmdef GUID references are refused so the fence cannot pass vacuously; the toolchain corrections (readonly struct + IEquatable, IReadOnlyCollection<SocketId>) are implemented as specified; drain step 2's ≥ threshold and step 3's post-loop, snapshot-walked release match ADR-0002 §5 exactly; the seq-less tiebreak and the interim OwnerClientId dedup key are declared and dated; harvestDiffs is the one Droppable and its keys are never cached; TD-ADR C3 is enforced by a call-order assertion; IInstanceMetadata : IGearHooks is the precedent story-004 followed; ADR-0013/0014 declare mutual independence from both sides; ADR-0002↔0006 and 0004↔0006 cycles remain broken; all 49 control-manifest stances exist in docs/registry/architecture.yaml and vice versa (49 = 49).

ADR Dependency Order

Foundation (Accepted):
  1. ADR-0001 Tide Scheduler pure assembly
  2. ADR-0002 Host commit model               (← 0001; 0006 is Related, not a dependency)
  3. ADR-0003 Assembly + purity boundaries    (← 0001, 0002)
  4. ADR-0004 Save format + reconcile         (← 0001, 0002, 0003; 0006 Amends, not a dependency)
  5. ADR-0006 Game data loading + validation  (← 0001, 0003)
  6. ADR-0005 Land lifecycle on Addressables  (← 0001, 0002, 0003, 0004, 0006)
  7. ADR-0007 Input action map                (← 0003, 0004, 0006)
Presentation / Core (Accepted):
  8. ADR-0013 Tide-tint render pipeline       (← 0001, 0003, 0005)
  9. ADR-0015 Combat model                    (← 0002, 0003, 0006, 0007)
 10. ADR-0014 UI stack                        (← 0003, 0004, 0006, 0007)
Not yet written (Enables rows only — correct usage):
  ADR-0008 Netcode wire (playtest-gated) → 0009, 0010, 0011, 0012

**No cycles. No Depends On names a non-existent or non-Accepted ADR.** Numbering gap 0008–0012 is intentional (ADR numbers are Required-ADR row numbers).

GDD Revision Flags (Architecture → Design Feedback)

None in design/gdd/ — no GDD assumption is contradicted by verified engine behaviour. No systems-index row changes.

One art-bible clause (not a GDD, recorded for the art director): design/art/art-bible.md:261 — "2–5 concurrent lands stays well under budget with URP 2D SRP batching" — did not receive the N > 1 caveat §4.3 got on 2026-09-01 (ADR-0013 §4: no two sockets are ever in phase, so one global pair cannot represent two lands), and "SRP batching" reads as a draw-call merge, which it is not (it removes per-draw CBUFFER setup). The hard batching rule that follows it (globals, never MPBs) is verified correct.

Engine Compatibility

Engine: Unity 6.3 LTS (6000.3.18f1) · ADRs with Engine Compatibility section: 10 / 10 · Version agreement: 10 / 10 Deprecated API references: 0 binding usages in shipped code — every hit (SetupRenderPasses, MaterialPropertyBlock, TextMeshProUGUI, EventSystem, PanelEventHandler, [ServerRpc], legacy UnityEngine.Input) is a negative-test fence literal or a ban comment. Stale version references: 0 in ADRs; 6 engine-reference module docs (rendering, ui, animation, audio, navigation, physics) cite 6000.0 URLs / 2022-LTS framing without a banner — E10, carried. Post-Cutoff API conflicts: 0 between ADRs; 1 ADR-vs-reference-doc conflict (ADR-0013 §6 vs modules/rendering.md's pre-Render-Graph signature — the ADR is right). Package pins vs ADR assumptions: Input System 1.20.0 ✅ (ADR-0007 exact) · Addressables 2.9.1 ✅ · URP / Shader Graph 17.3.0 ✅ · Newtonsoft ✅ (ADR-0004) · activeInputHandler: 1 ✅ · NGO absent — no com.unity.netcode.gameobjects, no transport, no Relay; zero using Unity.Netcode. Consistent with Gate 0; consequence: every NGO claim in ADR-0001/0002/0005/0015 is unverifiable against a real assembly and modules/networking.md is itself banner-marked 1.x-era. ADR-0008 must verify live.

Engine Specialist Findings

Ruling on the ADR-0013 batching measurements (E20/E21). Both halves are true and independent: the ADR is wrong about observable draw behaviour on 6.3 URP 2D, and UnityStats.batches is the wrong instrument. The spike controlled the confound (all 32 sprites share one material and one texture); the stock URP sprite shader scaled 1:1 exactly like DriftLayerSprite. The shipped substitute gate — ShaderUtil.GetSRPBatcherCompatibilityCode reading 0 for DriftLayerSprite and 10 for a deliberately incompatible control — is the correct instrument and strictly stronger than V2: it is the meter shown reading non-zero before a zero is trusted, and code 0 is the statement that the tint properties sit outside UnityPerMaterial, which the V1 spike left open. Correct instruments on 6.3: compatibility code (automatable) · Frame Debugger on a built player for grouping (manual; spritePackerMode = 3 makes editor grouping meaningless) · UnityStats.drawCalls as a marginal delta · UnityStats.setPassCalls for CPU cost. UnityStats is UnityEditor-internal and does not exist in a player build. Proposed V2/V4 wording is in Appendix C.

Architecture Document Coverage

Required-ADR rows 1–7, 13–15 map to the ten Accepted ADRs with no scope drift; rows 8–12 remain ⏳ Gate 0; the header's "5 remaining: rows 8–12" is correct. No orphaned architecture; every systems-index.md system appears in the layer tables. Carried housekeeping: docs/consistency-failures.md does not exist (not created — the skill appends only).

Verdict: CONCERNS

Not FAIL: no Foundation gap; no cross-ADR contradiction survives that changes a decision; in every conflict above the shipped code is on the right side and, in the three cases that matter most (C4, C8, U1), self-enforcing — TideClock throws on the wrong epoch order, the assembly graph refuses the wrong reference, the drift tripwire fires on a formula change. The project's testing discipline (mutation-verified fences, meters validated with controls, evidence paths on disk) is what made this review's findings documentation findings rather than shipped defects.

Off PASS for: one HIGH design defect shared by ADR-0003 and ADR-0004 (C4 — the cost of which rises with every table); two ADR sections that cannot compile as written (#10 ADR-0004 §2, U1 ADR-0005 IPreloadSchedule); one ADR the next epic will fail to build against (C3 ADR-0015); two verification criteria in an Accepted ADR that cannot be measured as worded (ADR-0013 V2/V4); no numeric-width rule for configs (C7); and E10 carried a third time.

Blocking issues — none (advisory verdict)

Required before the next epic is storied (recommended order)

• C4 — typed reconcile findings (ADR-0003 §3 + ADR-0004 §5; one story).
• #10 — ADR-0004 §2 to the shipped IReconcilable split.
• C3 / #5 — ADR-0015 ICombatState : ISwingerStateQuery (before the combat epic).
• #14 / #15 — ADR-0013 V2/V4 + spritePackerMode; control-manifest rows 312/313/317 name their meters.
• C7 / #3 — ADR-0006 §2a numeric-width rule + HarvestConfig retype story.
• U1 / #13, #9 — ADR-0005 (int socket, NextEvictionInstant, Debug Build Layout).
• Documentation-only: C1, C8, C6, C5, #2, U2, E10 banners, ADR-0015 ms budget.

Required ADRs (unchanged priority)

• ADR-0008 Netcode wire — playtest-gated; resolves 9 ⚠️ + 2 ❌ + TR-tint-004. NGO is not yet in the manifest.
• ADR-0010 Inventory replication — 5 items; TR-inv-002/004 single-machine halves are already implemented and tested.
• ADR-0009 Player transform authority; 4. ADR-0011 Structure registry; 5. ADR-0012 Enemy simulation.

✅ Resolution — HIGH + MEDIUM amendment sets applied 2026-09-07 (same session, user-approved per ADR)

Not applied here (stories, not doc edits): ReconcileFinding type + 3 call-site migration (C4, HIGH); HarvestConfig → double (C7); Driftlands.Editor fence + negative (C5); NextEvictionInstant onto IPreloadSchedule (#13); spritePackerMode EditMode assertion (V8); HostCommitQueue header (#19); E10 banners; ADR-0015 ms budget (figure is the user's call); fold/delete the stray ADR-0006-game-data-loading.md (U2); regenerate control-manifest.md (E22). The control manifest's Manifest Version is now stale against seven amended ADRs — /story-done will flag it until regenerated.

Appendix A — Deviation rulings (19)

Appendix B — Cross-ADR conflict blocks

C1 — ADR-0003 §6 vs §1 (self-conflict). Type: stale rule text. §6 row 203 "Data references nothing but BCL"; §1 correction (2026-09-03) "Data → Tide; Asm_NoUpwardReferences allows BCL + Driftlands.Tide". Impact: §6 is the test's specification; a test written from it fails the correct graph. Code right on both counts (Data.asmdef → ["Driftlands.Tide"]; AssemblyRules.cs:134-143). Resolution: (1, recommended) fix row 203; (2) footnote. MEDIUM.

C2 — ADR-0002 §5 drain order vs HostCommitQueue.Drain. Type: partial implementation. Steps 2 (IsDroppedBySoftClose, :117-122) and 3 (ReleasePendingTeardowns, :104) shipped; 1, 1b, 4, 5 absent. Step 1 is inverted — the set arrives on HostContext.SoftClosedSockets, so the "Advance already ran" guarantee is a convention until HostTickOrchestrator (ADR-0015) lands. Latent: PlaceStructureCommand will need step 4. Resolution: (1, recommended) migration-status line in ADR-0002 §5 + header fix; (2) empty flush points — rejected (vacuous scaffold). LOW as conflict; HIGH-attention note for ADR-0015's orchestrator ordering test.

C3 — ADR-0015 ICombatState list vs ISwingerStateQuery. Type: interface surface divergence. Not a breach (add, not remove/retype; HostContext.Combat still ICombatState; follows IInstanceMetadata : IGearHooks). Resolution: (1, recommended) declare inheritance + members in ADR-0015; (2) move the seam into ADR-0003 §3's shape list; (3) collapse — rejected. MEDIUM.

C4 — ADR-0004 §5 / ADR-0003 §3 LogDropped. Type: under-specified channel. Both ADRs and code agree — a shared design defect. Three call sites (HarvestDiffTable.cs:109 drop; InventorySaveTables.cs:78,117 keeps). Resolution: (1, recommended, now) Action<ReconcileFinding> Report; (2) parallel channel — rejected; (3) defer to save-epic completion — rejected (eight stages will have written strings). HIGH.

C5 — Driftlands.Editor vs ADR-0003 fences. Type: fence gap. Rule holds in fact (only Editor itself and Tests.EditMode reference it; PlayMode reaches it by reflection); no AssemblyRules case enforces it. Resolution: (1, recommended) add the case + negative; (2) tidy "seven-assembly" arithmetic and state the EditMode-may/PlayMode-may-not asymmetry deliberately. MEDIUM.

C6 — ADR-0001 "only writer" vs IEpochAdvancer, Restore, ApplyPoolChange. Type: invariant wording vs surface. (i) seam consistent, note premise stale; (ii) Restore assigns (TideClock.cs:79, code comment at :222 says "only INCREMENT"); (iii) ApplyPoolChange (:168) undeclared. Resolution: (1, recommended) one amendment pass — wording in Appendix A #C6; (2) also retype TideConfig claims (readonly struct). MEDIUM.

C7 — no float/double pin. Type: missing decision. ADR-0001 pins tide math only; ADR-0006 silent; ADR-0003 §2's "gameplay geometry is float" is spatial, not durations. Resolution: (1, recommended) ADR-0006 §2a double + retype HarvestConfig; (2) float everywhere — rejected (CombatConfig accumulators); (3) rely on the snap — rejected. MEDIUM.

C8 — ADR-0006 §5 diagram vs prose. Type: internal contradiction. Line 135 Restore → AdvanceEpoch → Advance; lines 115/164 and SaveLoader.cs:77 after; TideClock.cs:219 throws. Resolution: (1, recommended) correct the line to Restore → Advance(loadUtc) → if N ≠ socketCount → AdvanceEpoch(config, clock.T) → reconcile 4..9; (2) drop ordering from the diagram. MEDIUM.

Checked — no conflict: N1 Data→Tide edge consistent (ADR-0006 GameConfigs.Tide, ADR-0005 TemplateId in Tide, Tide root) · N2 both purity fences + vacuity guard (AssemblyEnforcementTests.cs:111,119) · N3 Presentation→Runtime legal · N4 test implements amended rule · N5 absent Assembly-CSharp is PASS · N6 GUID refs refused · N7 toolchain corrections implemented · N8 ICommand/HostContext order · N9 ReasonCode append-only · N10 int exception documented both sides · N11 drain step 2 ≥ · N12 step 3 post-loop snapshot · N13 seq-less tiebreak, mixed batch refused · N14 OwnerClientId dedup interim declared · N15 IInstanceMetadata : IGearHooks · N16 ReconcileContext ten members verbatim · N17 TD-ADR C3 call-order assertion · N18 one Droppable · N19 diff keys never cached · N20 instance caches (bodies of Purity_NoStaticMutableFields not read) · N21 0013/0014 independence · N22 cycles broken · N23 (reviewer) manifest ↔ registry 49 = 49.

Appendix C — Proposed ADR-0013 V2/V4 wording (engine specialist; offered, not applied)

V2 / EDITOR + PLAYMODE (revised 2026-09-07, review E21). SRP-batcher compatibility is asserted structurally, not by counting: ShaderUtil.GetSRPBatcherCompatibilityCode(DriftLayerSprite) returns 0 after one rendered frame, measured alongside a deliberately incompatible control shader that must return non-zero (observed 10) — the meter must read non-zero before a zero is evidence. This subsumes the UnityPerMaterial CBuffer claim V1 left open. ⚠️ UnityStats.batches cannot verify this and must not be used: on 6000.3.18f1 / URP 17.3.0 2D it scales 1:1 with sprite count in all four batcher settings — the SRP Batcher does not merge draws, it removes per-draw CBUFFER setup. "One batch per sprite atlas" survives only as a manual Frame Debugger check on a built player (spritePackerMode = 3 packs at build time only). CPU cost, if wanted, is UnityStats.setPassCalls.

V4 / PLAYMODE (revised 2026-09-07, review E20). Entering build mode at maximum reach with the ghost dashed adds ≤ 2 draw calls in the editor — grid ≤ 1, ghost ≤ 1 — as a marginal delta, never "total < 100". §8's "ghost 0" is a built-player claim: with spritePackerMode = 3 no atlas exists in Play Mode, so the editor cost is necessarily +1. The atlas-batched 0 is DEFERRED to a Frame Debugger check on a built player and is not a gate until then. Structural guarantees (one material, pre-authored dashed frames, no MPB) are asserted by BuildOverlaySetupTests and TideTintNegativesTests.

§8 budget line: "ghost 0 + grid 1 = 1" → "grid 1 + ghost 1 = 2 in the editor; 1 in a built player if the home atlas batches the ghost as designed (unverified — deferred)."

Plus: a new Engine Compatibility → Verification Required row naming EditorSettings.spritePackerMode (E23); control-manifest.md rows 312/313/317 to name their meters (E22).

Appendix D — Not verified (carried, never guessed)

• Ghost batching with the home atlas in a built player — needs a player build + manual Frame Debugger; nobody has run it.
• Every NGO 2.x claim (NetworkVariable<long> permissions, NetworkList spawn atomicity, [Rpc(SendTo.…)], NetworkTickSystem.Tick, NetworkTransform.OnUpdate) — package not installed; reference doc 1.x-era. ADR-0008 verifies live.
• m_EnterPlayModeOptions bitmask semantics — conclusion (domain reload ON) robust; bit values from engine knowledge.
• GetSRPBatcherCompatibilityCode code table — 0/10 taken from measured output with a working control.
• IL2CPP parity — all measurements are Editor/Mono/Metal; ADR-0001 AC-2 fixture and ADR-0004 round-trip on IL2CPP still open.
• Whether spritePackerMode = 3 is intentional (value and consequence confirmed; intent not).
• Bodies of animation/audio/navigation/physics.md beyond headers.
• Deck-chaining cold-cache cost at far-future t within AC-44 — no benchmark exists.
• Purity_NoStaticMutableFields, Commands_ExecuteIsSynchronous, Net_OnlyChannelDeclaresServerRpc, Interop_SingleConversionSite, Schema_* bodies and the 14 + 14 count.
• HostContext member count (17 vs 18) — settle by compiler.
• ApplyPoolChange's round-boundary semantics vs tide EC13 (surface undeclared is the finding, not incorrectness).
• Rulings #9, #14, #15, #16 were made from story quotations of ADR text plus grep context; re-read the source section before applying each amendment (done in the amendment-drafting step).

History