Architecture Review Report — 2026-08-28

Mode: /architecture-review (full) · Verdict: CONCERNS
Engine: Unity 6.3 LTS (6000.3) · URP 2D (Render Graph) · NGO 2.x · Input System 1.20 · Addressables 2.x
Loaded: 5 full GDDs + 8 quick-specs + art bible §4.3/§7/§8 (13 design docs) · 8 ADRs (all Proposed) · architecture.md v1.0 · engine reference (VERSION, breaking-changes, deprecated-apis, current-best-practices, modules ×4, plugins/addressables) · tr-registry.yaml (was empty) · docs/consistency-failures.md absent.
Method: three Sonnet verifiers checked every baseline row against the source doc text and confirmed the ADR-driven GDD amendments; the Unity specialist (Opus) audited every engine claim against the live 6000.3 / NGO 2.x / Addressables 2.x / Input System 1.20 docs.

Traceability Summary

architecture.md claims "102 requirements"; its table holds 94 rows. All 94 were confirmed against their source docs (3 misstated — corrected in the registry) and 17 additional architectural requirements were surfaced → 111 total, now registered as stable TR-IDs in tr-registry.yaml (version 2).

Foundation layer: 0 gaps. Every gap sits in Core/Feature/Presentation and is assigned to a not-yet-written ADR (0008–0014) or deliberately deferred to implementation. The full matrix is in traceability-index.md.

Baseline corrections (registry text updated, IDs unchanged)

• TR-save-003 — the quick-spec already restated Rule 3 to per-drain durability within flushDeadlineMs (ADR-0004); the baseline described the superseded per-action model.
• TR-ui-001 — "UI Toolkit screens" is an architecture.md decision; the ui-hud quick-spec explicitly defers the tech choice ("not a design fact").
• TR-net-006/007/008 — originate from the TD reviews / ADR-0002, not the netcode quick-spec (Rule 7 reserves the wire).
• TR-tide-008 — the GDD types tint intensities as double, not float.

Coverage gaps (no ADR)

Cross-ADR Conflicts

C1 — Two callers of tide Advance · Pattern / Timing · CONCERN (blocking once code exists)

• ADR-0001 §4 claims: Advance is scheduled from an Awaitable.WaitForSecondsAsync timer at NextBoundary() (and − PreloadLeadTime), with "ADR-0008 may override with an NGO-tick-aligned mechanism".
• ADR-0015 §1 claims: HostTickOrchestrator is the sole NetworkTickSystem.Tick subscriber and calls tide Advance as step 1 of every host tick. ADR-0002 step 1 assumes Advance "has already run this frame"; ADR-0005 hangs LandNet spawn/despawn and Append off Advance's events (tide-edge carve-out).
• Impact: a timer continuation has no documented ordering vs NetworkTickSystem.Tick, so SoftClose-first is not guaranteed under the timer path; if both callers exist, a boundary tick calls Advance twice — idempotent for tint floats, not for NetworkObject.Spawn(). The specialist independently rated this the sharpest finding.
• Resolution options: (1) recommended — ADR-0015 is recorded as amending ADR-0001 §4: Advance is called only by the orchestrator (per-tick, ≤ 50 µs); the Awaitable timer survives solely as the Presentation-role / IPreloadSchedule pull wake-up, where clients genuinely need it. State the consequence for ADR-0004 §3 ("Persist after every Advance that changes t" → one dirty tide table per second while idle; either accept or rate-limit to boundaries). (2) keep the timer as the sole caller and have the orchestrator only read — loses same-tick guarantees; not recommended.

C2 — clientSeq dedup keyed by clientId vs PlayerKey · State · CONCERN

• ADR-0002 §8/§9 claims: the host keeps the last clientSeq per client and drops seq ≤ last; "rejoin replays are deduplicated".
• ADR-0015 §1 claims: clientId is documented unstable across reconnects and may be reassigned; every session row is keyed by PlayerKey.
• Impact: a rejoined client arrives with a fresh clientId and no seq history — the rejoin-replay claim does not hold as written; worse, a reassigned clientId could inherit another player's seq window.
• Resolution options: (1) key the seq window by PlayerKey and have the owner persist its counter across rejoin; (2) scope dedup to one connection and drop the "rejoin replays" claim from §9.

C3 — Declared dependency cycles · Dependency · 🔴 flagged per protocol (substantively benign)

• ADR-0002 → ADR-0006 → ADR-0003 → ADR-0002 (ADR-0002 "Depends On" 0006 as "weak: one load-time value"; 0006 depends on 0003; 0003 depends on 0002).
• ADR-0006 ⇄ ADR-0004 (mutual "Depends On" — 0006 amends 0004's tide block; 0004's gate reads 0006's configVersion).
• Resolution: reclassify ADR-0002→0006 as Related and ADR-0006→0004 as Amends. No decision changes.

C4 — ADR-0006 stage 4 "telegraph ≥ 400 ms is a NEW load rule with no combat AC" · Integration · NOTE

Combat AC-20 and the telegraphFloor knob ("hard floor 400 = the Rule 11 contract") already specify the load rejection. Correct the wording; Rule = combat.AC-20.

C5 — Unarmed SwingLanded · Integration · NOTE

Combat GDD (Crafting dependency row) says durability events are "never emitted for unarmed swings"; ADR-0003 emits SwingLanded with WeaponInstance = null for unarmed so presenters get DamageDealt. Keep the ADR shape; amend the GDD row to "Crafting ignores null instances".

C6 — Knob file homes · Data · NOTE

ADR-0005 places landTemplateBudgetMB in land-dock.json; ADR-0006 §1's land-dock.json row does not list it. PreloadLeadTime is owned by tide.json (ADR-0001) and only "cited" by land-dock.json — say "read-only reference".

C7 — TR-harv-001 assignment · Ownership · NOTE

architecture.md Required-ADR table assigns harvest-node replication to row 8; ADR-0005 and §2 assign it to ADR-0010 (QQ-13). Pick one (ADR-0010 recommended — it already owns the NodesReady gate).

Known conflict-prone areas: docs/consistency-failures.md does not exist; nothing to carry forward.

ADR Dependency Order (topologically sorted, after C3 relabel)

Foundation:
  1. ADR-0001 Tide Scheduler pure assembly            (no deps)
  2. ADR-0002 Host commit model                       (0001; 0006 → Related)
  3. ADR-0003 Assembly + purity boundaries            (0001, 0002)
  4. ADR-0006 Game data loading + validation          (0001, 0003; 0004 → Amends)
  5. ADR-0004 Save format + reconcile pipeline        (0001, 0002, 0003, [0006 weak])
  6. ADR-0005 Land lifecycle on Addressables          (0001, 0002, 0003, 0004, 0006)
  7. ADR-0007 Input action map                        (0003, 0004, 0006)
Core:
  8. ADR-0015 Combat model                            (0002, 0003, 0006, 0007)
Not yet written:
  ADR-0008 Netcode wire (friend-group playtest) → ADR-0009, 0010, 0011, 0012 → ADR-0013, 0014

⚠️ Every "Depends On" currently points at a Proposed ADR (all eight are Proposed). Per the TD sign-off condition, ADRs 1–7 must be Accepted before the first implementation story; this review recommends the amendments below land first (user ruling 2026-08-28: apply amendments 1–5, then flip).

GDD Revision Flags (Architecture → Design Feedback)

No GDD revision flags — no GDD assumption conflicts with verified engine behaviour. The specialist confirmed the tide purity model, the two rails, per-drain durability, SoftClose-first and the no-offline-simulation pillar are all deliverable on 6000.3 as designed. Systems index left unchanged.

Three doc-sync items (not design revisions):

Confirmed already landed: tide EC18 0.40; save Rule 3 + saveDebounceMs restatement; combat maxTargets / reviveReach rows + OQ10; movement Rule 5 downedCrawlSpeed carve-out; BB OQ6 resolved (gridCellSize = 1 u); art bible §8.A.5 / §8.B.1 atlas wording.

Engine Compatibility Issues

Engine Audit Results

Engine: Unity 6.3 LTS (6000.3) · ADRs with Engine Compatibility section: 8 / 8 · Version agreement: 8 / 8 Deprecated API references: none (every ServerRpc/ClientRpc/SetupRenderPasses/UnityEngine.Input/NetworkTransform.Update/Compatibility-Mode/Multiplay hit is in "don't use" context). Stale version references: none. Post-cutoff API conflicts between ADRs: none beyond C1.

Engine Specialist Findings (Opus, live-doc verified)

Confirmed correct by live docs: NetworkTickSystem.Tick fires "before NetworkVariable changes are collected" (NGO 2.5 — the stage is still undocumented, so ADR-0002 Verification 1 stays open); reliable RPC order guaranteed per NetworkObject (NGO 2.6); NetworkVariable values delivered before OnNetworkSpawn; SubsystemRegistration precedes scene Awake; project-wide actions auto-instantiate and auto-enable; NetworkPrefabHandler asset-readiness warning is verbatim; all four Awaitable members incl. AwaitableCompletionSource<T>; all four Addressables 2.x members; Application.wantsToQuit semantics; File.Replace atomicity.

Architecture Document Coverage

• All 13 MVP systems + Tide-Tint appear in the layer map and module tables; data-flow §3.2 covers every cross-system event the GDDs define; no orphaned modules (FeedbackPresenters is presentation-only and needs no GDD).
• Stale: header "ADRs Referenced: ADR-0001 … ADR-0004, ADR-0006 — 10 remaining" (8 exist; 7 remain); "## ADR Audit … contains no ADRs" and "Traceability Coverage: 0 of 102" are pre-ADR text; the requirement count is 94 (now 111 registered), not 102; TR-craft-006 was assigned to no Required-ADR row.
• .claude/docs/technical-preferences.md "Architecture Decisions Log: [No ADRs yet]" is stale.
• design/gdd/systems-index.md has two rows numbered 15 (Tide-Tint Polish and Base-Defense Events).
• tr-registry.yaml was empty while ADR-0001/0002/0003/0004/0005/0006 Validation Criteria each say "/architecture-review shows TR-… covered" — registered by this review.

Verdict: CONCERNS

Foundation is fully covered and internally consistent in substance; no engine finding invalidates a decision. Off PASS because of: two real cross-ADR inconsistencies (C1, C2), a declared dependency cycle to relabel (C3), three engine CONCERNs (E3/E4 implementation-shaped, E10 process), and 19 gaps — all Core/Feature/Presentation, all assigned to ADRs 0008–0014 whose wire halves are gated on the friend-group playtest by CD ruling. Not FAIL: no Foundation requirement is uncovered and no conflict requires re-deciding anything.

Blocking issues

None at FAIL level. Required before flipping ADR-0001–0007 + 0015 to Accepted (user ruling 2026-08-28):

• ADR-0001 §4 ← ADR-0015 amendment: single Advance caller; timer = presentation/preload wake-up only (C1 / E1 / E3); state the ADR-0004 §3 tide-dirty consequence.
• ADR-0002 §8/§9: dedup keyed by PlayerKey (C2).
• ADR-0002 / ADR-0006 dependency fields relabelled (C3).
• ADR-0007: fix the PanelEventHandler sentence (E2). ADR-0004 §4: wantsToQuit latch + bounded blocking wait + Editor parity (E4).
• ADR-0006 stage 4 wording (C4); ADR-0005 NetworkList / "~1 s" hedges (E5); ADR-0004 library wording (E7).

Required ADRs (most foundational first)

• ADR-0008 Netcode wire — playtest-gated; resolves 9 ⚠️ + 2 ❌ + TR-tide-013 + TR-tint-004.
• ADR-0010 Inventory replication — 5 inventory items incl. the newly-surfaced CARRY representation (TR-inv-007), transferBatchWindow, harvest-node NetworkList. Recommend extending its charter — or adding Required-ADR row 16 "Crafting commit, durability drain and InstanceMetadata" — because TR-craft-003/004/006/008 currently have no owning row.
• ADR-0009 Player transform authority — TR-move-002/004/005, TR-combat-008, TR-bb-007 write mode; pre-playtest snap check.
• ADR-0011 Structure registry + wall colliders (TR-bb-002/005/007, TR-move-003).
• ADR-0012 Enemy simulation (TR-ai-001/002/004, TR-ldu-005).
• ADR-0013 Tide-tint render pipeline — pulled to MVP by the index; TR-tint-001..003, TR-bb-009, TR-cam-001.
• ADR-0014 UI stack (TR-ui-001..004, TR-combat-013).

Pre-gate checklist (checked 2026-08-28)

• ❌ tests/unit/, tests/integration/ — run /test-setup
• ❌ .github/workflows/tests.yml — run /test-setup
• ❌ design/accessibility-requirements.md — run /ux-design
• ❌ design/ux/interaction-patterns.md — run /ux-design

Re-run /architecture-review after each new ADR to verify coverage improves.

History