settled-reach/docs/audits/architecture-review-2026-02-11.md

---
title: "Architecture Review — Sprint 1"
description: "Full adversarial two-round architecture audit by Tyre and Troblum covering all 40 confirmed decisions, implementation, and risk profile. Architecture rated GREEN; knowledge graph and observer pipeline identified as underspecified critical systems."
type: audit
status: archived
created: 2026-02-11
updated: 2026-03-05
---

# Architecture Review Audit — The Settled Reach

**Date:** 2026-02-11
**Reviewers:** Tyre (Technical Architect, internal) + Troblum (External Technical Consultant)
**Scope:** Full architecture review covering all 40 confirmed decisions, existing implementation, current code, and technical risk profile.
**Format:** Independent reviews (Round 1) followed by cross-review debate (Round 2), synthesized into consensus recommendations.

---

## Executive Summary

Both reviewers independently conclude the architecture is **sound and well-chosen**. The Godot 4 client + Rust/bevy_ecs server via subprocess/IPC design (D-020) correctly enforces information boundaries at the protocol level, supports deterministic simulation, and future-proofs multiplayer without incurring multiplayer tax in v0.1.

**Architecture health: GREEN.**

The primary concerns are not about the architecture itself but about **underspecified systems** (knowledge graph, observer snapshot pipeline) and **operational gaps** (blocking I/O, error recovery, profiling infrastructure). The reviewers diverge on urgency of some items but converge on the final priority list.

**Key consensus findings:**
- The information boundary system (pillar 1) is the most important and least specified critical system
- Blocking I/O in LocalBridge must be resolved before end-to-end integration works
- Deterministic replay (#201) is non-negotiable by Sprint 2
- HashMap iteration order is a determinism time bomb — enforce BTreeMap/IndexMap in simulation code
- IPC round-trip latency must be measured as a Sprint 1 exit criterion
- Sprint 1 is on track; the building blocks for "character moves on screen through IPC" are in place

---

## Table of Contents

1. [Round 1: Tyre's Architecture Review](#round-1-tyres-architecture-review)
2. [Round 1: Troblum's Architecture Evaluation](#round-1-troblums-architecture-evaluation)
3. [Round 2: Tyre's Debate Response](#round-2-tyres-debate-response)
4. [Round 2: Troblum's Debate Response](#round-2-troblums-debate-response)
5. [Consensus Recommendations](#consensus-recommendations)

---

# Round 1: Tyre's Architecture Review

**Author:** Tyre (Technical Architect)

---

## 1. Architecture Overview Assessment

### 1.1 Core Architecture: Godot 4 Client + Rust/bevy_ecs Server via Subprocess/IPC

The fundamental architecture is **sound and well-chosen**. D-020 (subprocess/IPC over GDExtension) was the right call, and Troblum's risk assessment that pushed us away from GDExtension was spot-on. The current architecture eliminates entire categories of risk that would have consumed sprints by now.

**What is working well:**

- **True client-server separation.** The `server/` crate has ZERO Godot dependencies. `Cargo.toml` confirms: `bevy_ecs`, `bevy_app`, `serde`, `rmp-serde`, standard Rust crates. No `godot` crate. This is exactly right.
- **SimBridge trait abstraction** (`server/src/bridge/mod.rs` lines 28-34) cleanly separates transport from protocol. `LocalBridge` (Unix sockets) and future `NetworkBridge` implement the same interface. Multiplayer really would be a transport swap, not a rewrite.
- **MessagePack serialization with length-prefixed framing** (`server/src/bridge/framing.rs`) — simple, correct, tested. 4-byte big-endian length prefix, 16MB max message, EOF-safe reads.
- **Deterministic simulation foundation.** Injectable `SimulationTime`, `SimRng` (ChaCha20 with stored seed), and `InputQueue` resources (`server/src/simulation/`). D-030 requirement 7 (deterministic replay) has its plumbing in place.
- **Protocol codec parity.** The GDScript `Protocol` class (`client/scripts/protocol/protocol.gd`) correctly handles rmp_serde's named-field encoding (unit variants as bare strings, data variants as single-element maps). Cross-language fixture files exist in `client/tests/fixtures/msgpack/`.
- **ObserverSnapshot as sole boundary type.** D-010 principle 2 enforced at the API level — client never sees raw world state.

**Architecture Rating: GREEN.** The core split is the right one. The implementation so far is clean, well-documented, and testable independently on both sides.

### 1.2 Implementation Progress

Current implementation status as of Sprint 1:

| Component | Status | Files |
|-----------|--------|-------|
| Server binary + bevy_app bootstrap | Working (single-tick) | `server/src/main.rs` |
| SimulationPlugin (time, rng, input, movement) | Working, tested | `server/src/simulation/` |
| BridgePlugin (SimBridge trait, LocalBridge) | Working, tested | `server/src/bridge/` |
| NPC component model (10 axes) | Stubbed with types | `server/src/npc/mod.rs` |
| PerceptionPlugin | Stub only | `server/src/perception/mod.rs` |
| StorytellerPlugin | Stub only | `server/src/storyteller/mod.rs` |
| CauseChain (provenance tracking) | Working, tested | `server/src/cause_chain.rs` |
| WalkabilityMap + tile collision | Working, tested | `server/src/simulation/movement.rs` |
| Godot client main loop | Working (test mode) | `client/scripts/main.gd` |
| SimBridge autoload (GDScript) | Working (test mode + wire protocol) | `client/scripts/autoloads/sim_bridge.gd` |
| Protocol codec (MessagePack) | Working, tested via fixtures | `client/scripts/protocol/protocol.gd` |
| EntityRenderer | Placeholder | `client/scripts/rendering/entity_renderer.gd` |
| FogRenderer | Stub | `client/scripts/rendering/fog_renderer.gd` |

Sprint 1 goal: "get a player-controlled character moving on screen through working IPC bridge" — tickets #78/#79/#81/#82/#83. The building blocks are in place. The gap is the actual subprocess spawn and real-time loop.

### 1.3 Test Infrastructure

**Rating: GREEN with minor gaps.**

D-030 three-layer IPC testing architecture partially implemented:

- **Layer 1 (fixture-based serialization):** Done. `server/tests/serialization.rs` does MessagePack roundtrip on all types. `server/tests/gen_fixtures.rs` generates cross-language fixtures. Client has `client/tests/fixtures/msgpack/` with 5 fixture files.
- **Layer 2 (protocol state machine):** Partially done. `server/tests/bridge_ipc.rs` tests snapshot and input roundtrip over real Unix sockets. Missing: mock protocol state machine for client-side testing.
- **Layer 3 (real subprocess integration):** Not yet implemented. Sprint 1 ticket #81 addresses this.

Build pipeline via Makefile is solid: `make ci` chains `lint -> build -> test` for both sides. `cargo nextest` for Rust, gdUnit4 for Godot.

---

## 2. Decision Coherence Analysis

### 2.1 Internal Consistency

Reviewed all 40 confirmed decisions across 5 domain files. The decisions are **remarkably consistent** — zero hard contradictions found.

**Strong coherence chains:**

- D-010 (multiplayer-ready baseline) -> D-020 (subprocess/IPC) -> D-030 (testability) form a clean dependency chain.
- D-011 (fog of perception) -> D-015 (locked camera) -> D-017 (perception modes) -> D-018 (three-range sound) form a coherent perception stack.
- D-005 (single character) -> D-027 (two-character vertical slice) -> D-034 (THE FRIEND pattern) demonstrate scope escalation that is architecturally safe.
- D-023 (three-tier content) -> D-024 (10-axis NPC model) -> D-025 (social site templates) -> D-028 (tagged line pools) -> D-035 (tag taxonomy) form a complete content pipeline specification.

**Mild tension points (not contradictions):**

1. **D-003 vs current scope.** D-003 says "build a framework/engine, the Settled Reach is the first campaign." But the NPC axes (D-024), contraband spec (D-037), and setting details (D-036) are deeply Settled Reach-specific. Fine for v0.1 — the "framework" claim should be understood as aspirational, not architectural.
2. **D-012 (chunk-based maps, future borderless) vs D-014 (bounded 150x150).** No contradiction, but chunk-based architecture is over-engineered for v0.1 scope (150x150 = ~25 chunks at 32x32). Investment justified by design principle.
3. **D-009 (multiplayer-ready) cost estimate ("15-20% slower").** Unverifiable at this stage. With subprocess/IPC, multiplayer readiness is essentially free because the architecture IS client-server.

### 2.2 Decision Gaps

| Gap | Severity | Notes |
|-----|----------|-------|
| **Save/Load system** | HIGH | bevy_ecs World serialization is non-trivial. No decision or design exists. |
| **Pathfinding algorithm** | MEDIUM | MoveIntent handles tile-by-tile validation but no A* or similar for multi-tile paths. |
| **Shadowcasting algorithm** | MEDIUM | D-011 mandates LOS shadowcasting. Implementation not specified. |
| **NPC AI state machine** | HIGH | D-026 mentions "4 state machines" for Background tier. No specification of states or transitions. |
| **Knowledge graph data structure** | HIGH | D-010 principle 2 requires state tagged with "who knows it." Actual data structure for entity knowledge is unspecified. |
| **Map data format** | MEDIUM | D-014 specifies 5-8 hand-crafted buildings. Authoring pipeline undefined. |
| **Sound propagation algorithm** | LOW | D-018 specifies three ranges. Algorithm unspecified. |
| **Entity ID stability** | MEDIUM | `VisibleEntity.entity_id` is a `u64`. How stable IDs are generated and mapped client-side needs specification. |

---

## 3. Technical Risk Assessment

### RISK 1: Information Boundary Implementation (CRITICAL)

The core mechanic depends on getting this right, and it is the least-specified critical system.

D-010 principle 2 says "every piece of game state is tagged with who knows it." D-024 defines `InformationInventory` as `Vec<String>` — a placeholder. For v0.1 with 15 NPCs, this might work as a prototype. For 500+ active NPCs with knowledge graphs about other NPCs, this is O(N^2) in information space.

What is needed and not yet designed:
- A `KnowledgeState` component tracking "what does entity A know about entity B's state at time T"
- Efficient observer queries computing `ObserverSnapshot` without iterating the entire world
- Spatial indexing for perception queries
- Information decay (D-011: "fog returns when you leave")
- Q-016 (knowledge hierarchy) is open and load-bearing

**Mitigation:** Dedicated architecture spike before Sprint 2. Define knowledge data structure, observer query algorithm, and profile at 80 NPCs.

### RISK 2: Observer Snapshot Bandwidth (MEDIUM)

`ObserverSnapshot` currently contains only `tick` and `Vec<VisibleEntity>`. Per D-020, it needs fog/visibility grid, sound events, internal monologue triggers, HUD widget data, entity relationship colors, ambient state changes.

At 10 tps with 30-80 visible entities plus fog grid plus sound events — snapshot size matters. MessagePack is compact but frame budget is ~100ms per tick.

**Current risk:** LOW at v0.1 scale (15 NPCs). MEDIUM at full Active tier (80 NPCs).

**Mitigation:** Delta compression (only send changes since last acknowledged snapshot). bevy_ecs `Changed<T>` provides infrastructure for dirty-flagging.

### RISK 3: Synchronous IPC Blocking (HIGH)

Current `LocalBridge` uses blocking I/O on Unix sockets. Both `send_snapshot` and `receive_inputs` are blocking calls behind a Mutex. In a real game loop:

1. If client is slow to read, `send_snapshot` blocks the server tick.
2. If client is slow to send input, `receive_inputs` blocks until data arrives or EOF.

**Mitigation:** Sprint 1 must address this. Recommended: separate IO thread with crossbeam channels feeding into the bevy_ecs game loop. The `SimBridge` trait already abstracts this — the fix is internal to `LocalBridge`.

### RISK 4: Client-Side Entity Lifecycle Management (MEDIUM)

Current issues in `entity_renderer.gd`:
- Checks for `entity_data.has("id")` (line 19) but Protocol returns `entity_id`, not `id`. Will silently skip all entities.
- Entity creation uses `ColorRect` — fine for prototyping.
- No interpolation between ticks. At 10 tps, entities teleport between tiles.
- No entity pooling.

### RISK 5: Bevy Version Churn (LOW-MEDIUM)

`Cargo.toml` shows `bevy_ecs = "0.18"`, `bevy_app = "0.18"`. Version pinning is correct. Risk is over project lifetime.

### RISK 6: GDScript MessagePack Library Dependency (LOW-MEDIUM)

Fixture-based cross-language tests (D-030 Layer 1) are the correct mitigation.

### RISK 7: Kill Switch Timeline (META)

D-020 specifies: "If no working prototype by week 8, pivot to pure Godot." Current trajectory is healthy — Sprint 1 is on track. Week 8 deadline should be tracked explicitly.

---

## 4. Scalability and Performance Assessment

### 4.1 Simulation Tier Budget Feasibility

D-026 defines four tiers:

| Tier | Count | Update Rate | Per-Entity Cost |
|------|-------|-------------|-----------------|
| Active | 30-80 | 10-20 ticks/sec, full sim | HIGH |
| Background | 500-2,000 | 1/game-minute (10 ticks) | LOW |
| State-saved | 10,000+ | Frozen, ~1-2KB serialized | ZERO |
| Ungenerated | unlimited | N/A | ZERO |

At 80 Active NPCs, 10 tps: 80 perception queries per tick at ~1ms each = ~80ms. Exceeds 100ms tick budget with naive implementation. But: not all 80 NPCs need full LOS every tick. Spatial hashing and staggered updates bring this within budget.

**For v0.1 with 15 NPCs, performance is a non-issue. Scaling concern is real for the full game but achievable with spatial indexing and staggered perception.**

### 4.2 Asymmetric Information at Scale

The `InformationInventory { known_facts: Vec<String> }` placeholder needs to become a structured, queryable data store. Recommended: `HashMap<Entity, KnowledgeEntry>` per entity, with spatial queries using grid-based visibility cache.

---

## 5. Implementation Readiness Assessment

### 5.1 Well-Specified (Ready to Build)

| System | Quality | Key Decisions |
|--------|---------|---------------|
| IPC transport (bridge) | Excellent | D-020, D-030 |
| Time system | Excellent | D-031 |
| NPC component model | Good | D-024 |
| Content tag taxonomy | Excellent | D-035 |
| Tile collision | Good | D-012 |
| Test architecture | Good | D-030 |
| Build pipeline | Good | DEVOPS.md |

### 5.2 Underspecified (Needs Architecture Spike)

| System | Priority |
|--------|----------|
| Knowledge graph / information boundaries | CRITICAL — Sprint 2 blocker |
| Observer snapshot generation | CRITICAL — Sprint 2 blocker |
| LOS shadowcasting | HIGH — Sprint 2 |
| NPC AI / state machines | HIGH — Sprint 3 |
| Pathfinding | HIGH — Sprint 3 |
| Save/Load | MEDIUM — before vertical slice |
| Map data authoring | MEDIUM — Sprint 2-3 |
| Entity ID stability | MEDIUM — Sprint 1 blocker |
| Sound propagation | LOW — v0.1 visual only |
| Storyteller design | LOW for v0.1, HIGH for vertical slice |

---

## 6. Tyre's Prioritized Recommendations

### Tier 1: CRITICAL (Before Sprint 2)

- **R-01:** Design Knowledge Graph data structure (replace `Vec<String>` with structured `KnowledgeGraph` component)
- **R-02:** Design Observer Snapshot generation pipeline (spatial query -> LOS check -> sound propagation -> knowledge filtering -> assembly)
- **R-03:** Fix blocking I/O in LocalBridge (separate IO thread with crossbeam channels)

### Tier 2: HIGH (Sprint 2-3)

- **R-04:** Choose and implement shadowcasting algorithm (symmetric or recursive)
- **R-05:** Define NPC pathfinding strategy (A* on WalkabilityMap)
- **R-06:** Specify NPC AI state machines
- **R-07:** Fix entity_renderer.gd field name mismatch (`id` vs `entity_id`)

### Tier 3: MEDIUM (Before Vertical Slice)

- **R-08:** Design Save/Load architecture (custom SaveState struct)
- **R-09:** Define map authoring pipeline (Tiled recommended)
- **R-10:** Add delta compression interface to ObserverSnapshot

### Tier 4: LOW (Track)

- **R-11:** Resolve Q-007 (target platforms — Unix sockets vs Windows)
- **R-12:** Document entity ID strategy
- **R-13:** Add performance benchmarks

---

## 7. Strengths Worth Preserving

1. **Subprocess/IPC over GDExtension** (D-020). Every day without fighting gdext FFI validates this choice.
2. **Pure Rust server with zero Godot dependencies.** Testable without Godot. Integration tests run in CI without display server.
3. **bevy_ecs for the simulation.** Component model maps naturally to NPC axes. Change detection essential for efficient observer queries.
4. **The test architecture** (D-030). Three-layer IPC testing, cross-language fixtures, CauseChain as production component.
5. **Kill switch criteria** (D-020). Honest, measurable, time-boxed.
6. **Single-ownership NPC model with reference links** (D-025). Avoids lifecycle management nightmare.

---

## 8. Tyre's Summary

**Overall Architecture Health: GOOD.** Core decisions sound, implementation clean, risk profile well-understood.

**Primary concern:** Information boundary system is the most important and least specified part.

**Secondary concern:** Sprint 1 IPC loop needs blocking I/O resolved.

**Trajectory:** Week 8 kill switch comfortably far away. Current pace healthy. Biggest risk is not technical failure but scope creep in content design.

---

# Round 1: Troblum's Architecture Evaluation

**Author:** Troblum (External Technical Consultant)

---

## 1. Technology Stack Assessment

### 1.1 The Subprocess/IPC Choice

For the stated requirements (asymmetric information, occlusion-based detective game, Rimworld-style simulation, multiplayer-ready), the Godot 4 + Rust/bevy_ecs subprocess/IPC architecture is **well-suited**.

The shift from GDExtension to subprocess/IPC eliminated the highest-severity risks. Good decision.

**Comparison to alternatives:**

| Stack | Pros | Cons | Verdict |
|-------|------|------|---------|
| **Current (Godot + Rust/IPC)** | Info boundaries enforced, deterministic sim, multiplayer-ready | IPC latency, two-language debugging | Best fit |
| Pure Godot (GDScript) | Faster prototyping, single language | Performance ceiling ~100 NPCs, harder to retrofit multiplayer | Viable for v0.1 only |
| Pure Bevy | Everything in Rust, no IPC, excellent ECS | No visual editor, immature UI, steeper learning curve | Wrong for developer profile |
| GDExtension (R-006) | No IPC latency, tighter Godot integration | FFI complexity, thread safety, gdext pre-1.0 instability | Correctly rejected |

---

## 2. IPC & Integration Risk

### 2.1 Unix Socket Architecture

`SimBridge` trait -> `LocalBridge` (Unix domain sockets) -> `NetworkBridge` (future TCP). Architecture is clean.

**Framing protocol:** 4-byte big-endian length prefix + MessagePack payload. Maximum 16MB. EOF detection. This is solid.

**What can go wrong:**
1. **Blocking I/O** — current implementation blocks on read/write
2. **Version drift** — no protocol handshake
3. **Flow control** — no backpressure mechanism
4. **Error recovery** — undefined

### 2.2 Serialization Assessment

MessagePack is a good choice: compact binary, schema-optional, cross-language. The fixture-based testing (D-030 Layer 1) catches encoding divergence early.

**Performance concern:** GDScript MessagePack library maturity and performance at scale needs monitoring.

---

## 3. bevy_ecs Suitability

### 3.1 Entity-Component Fit

The 10-axis NPC model (D-024) maps naturally to bevy_ecs components. Each axis = one component. Systems operate on component queries. Standard ECS pattern.

**Critical gap: Spatial partitioning.** Perception queries without spatial indexing are O(observers x entities). At 80 Active NPCs, this exceeds tick budget.

**Recommendation:** Implement grid-based spatial partitioning (~200 lines of Rust) before perception systems exist.

### 3.2 Determinism Support

Systems using `.after()` for explicit ordering. `movement::validate_movement` runs after `time::advance_tick`. Deterministic.

**Gap:** Only two systems registered so far. When 10 systems are added with cross-dependencies, will ordering be maintained?

**Recommendation:** Create a system dependency graph before Sprint 2. Document which systems read/write which components.

### 3.3 Storyteller System

Architecturally straightforward in bevy_ecs — just another system. But it needs access to almost everything and is likely the most complex single system. Needs early prototyping.

### 3.4 Chunk-Based Maps

At 150x150 map with 15x15 tile chunks: 100 chunks, 9 loaded at once, ~14,000 tiles in memory. Negligible. Future borderless supported by same architecture.

---

## 4. Godot 4 Client Concerns

### 4.1 GDScript Performance

GDScript is adequate for the client's role (deserializing snapshots, updating sprites, UI, audio). All heavy computation (LOS, pathfinding, AI) is on the Rust side. Correct separation.

**Concern:** D-017 dynamic HUD composition requires runtime UI creation. Profile HUD rebuild; if >16ms, pre-instantiate widgets and show/hide instead of create/destroy.

### 4.2 Fog-of-War Rendering

Options: TileMapLayer (simplest), shader-based (most performant), LightOccluder2D (visually impressive but complex).

**Recommendation:** Start with TileMapLayer for v0.1. Upgrade to shader if profiling shows tile updates are slow.

### 4.3 Entity Color (D-033)

`CanvasItem.modulate` multiplies existing pixel colors. A red sprite modulated green looks brown, not green.

**Recommendation:** Base sprites in white/grayscale, apply relationship color via `modulate`.

### 4.4 Audio

Godot's audio system is excellent for 8 audio files with event-driven + ambient loops. No concerns.

---

## 5. Architectural Patterns & Anti-patterns

### 5.1 Patterns Working Well

1. Client-server separation (D-010, D-020)
2. Observer-query model (D-017)
3. Resource-based injection (SimRng, SimulationTime, InputQueue)
4. Chunk-based maps (D-012)
5. Three-tier content model (D-023)

### 5.2 Patterns That Are Concerning

1. **No spatial partitioning** — critical gap for scaling
2. **Error handling incomplete** — no recovery strategy defined
3. **Flow control undefined** — pipe buffer overflow possible at scale
4. **No protocol versioning** — version drift will cause subtle bugs
5. **Determinism designed but untested** — #201 not implemented

### 5.3 Missing Patterns

1. **Save/load architecture** — not addressed
2. **Mod support** (Q-004) — not addressed
3. **Crash reporting / telemetry** — gap for external playtesting
4. **Performance monitoring** — no cross-boundary profiling

### 5.4 Anti-patterns Correctly Avoided

1. NOT using GDExtension for game logic
2. NOT storing game state in Godot scene tree
3. NOT baking player identity into simulation
4. NOT using wall-clock time in simulation

---

## 6. Scope vs Architecture Fit

### 6.1 v0.1 Scope Assessment

Architecture is adequate for v0.1 scope. Possibly over-engineered (15 NPCs don't need ECS or client-server), but over-engineering pays off in future-proofing.

### 6.2 Scaling Limits

The 500-2,000 NPC target is achievable IF:
- Spatial partitioning is implemented
- Tier transitions work as designed
- Perception queries are optimized

### 6.3 Scope Cut Priority (If Needed)

1. **First cut:** Background tier size (500-2,000 -> 100-500)
2. **Second cut:** Perception mode count (6 -> 3)
3. **Third cut:** Multi-character vertical slice (drop detective)
4. **Fourth cut:** Chunk-based procedural maps (hand-craft everything)
5. **Nuclear option:** Pure Godot, abandon Rust/ECS

### 6.4 Additional Kill Switch Criteria (Proposed)

- Perception queries exceed 50ms per tick at 30 NPCs
- IPC latency exceeds 10ms per tick

---

## 7. Troblum's Recommendations

### CRITICAL (Do Now)

1. Implement spatial partitioning this sprint
2. Add protocol version handshake (~50 lines)
3. Define error recovery strategy
4. Profile the full IPC loop now

### HIGH (Sprint 2-3)

5. Implement deterministic replay (#201) — non-negotiable by Sprint 2
6. Build minimal storyteller prototype
7. Stress-test tier transitions (500 dummy NPCs)
8. Document system dependency graph

### MEDIUM (Sprint 4-5)

9. Add performance telemetry
10. Plan save/load architecture
11. Prototype one non-natural perception mode

### LOW (Post-v0.1)

12. Investigate GDScript MessagePack performance
13. Add crash reporting

---

## 8. Time Bombs

**The determinism assumption.** Determinism is fragile. A single non-deterministic operation breaks it:
- `HashMap` iteration order (use `BTreeMap` or `IndexMap`)
- Parallel system execution without explicit ordering
- Floating-point non-associativity
- External state reads

**Ticket #201 (deterministic replay) is non-negotiable. Implement by end of Sprint 2.**

---

## 9. Troblum's Verdict

**Yes, this is the right stack.** For the stated requirements, the architecture is well-suited. The shift from GDExtension to subprocess/IPC eliminated the highest-severity risks.

**The biggest risk is not the architecture — it's execution.** The risk is spending 8 weeks building infrastructure with zero visible gameplay. D-020's kill switch is the right safety valve.

**Path to success:** Each sprint delivers visible gameplay. Infrastructure in service of gameplay, not the other way around.

---

# Round 2: Tyre's Debate Response

**In response to:** Troblum's Round 1 findings

---

## 1. Spatial Partitioning

**AGREE on the need. PUSH BACK on timing.**

At v0.1 scale (1 player observer, ~15-25 NPCs), perception is 25 checks per tick — microseconds on modern hardware. The O(N^2) alarm is accurate for scaling but irrelevant at current scope.

Where Troblum IS right: writing `for entity in all_entities` in perception is an anti-pattern that becomes load-bearing technical debt.

Where I push back: Sprint 1 has two server tickets (#78, #236). Adding spatial partitioning scope-creeps the sprint.

**Synthesis: Define the `SpatialIndex` trait now, use naive `Vec<Entity>` behind it, swap in grid-based lookup in Sprint 2.**

```rust
pub trait SpatialIndex {
    fn entities_in_range(&self, center: IVec2, range: i32) -> Vec<Entity>;
    fn entities_at(&self, position: IVec2) -> Vec<Entity>;
    fn update(&mut self, entity: Entity, old_pos: IVec2, new_pos: IVec2);
}
```

**Priority: Define trait Sprint 1. Implement grid Sprint 2.**

---

## 2. Protocol Version Handshake

**AGREE. Fully.**

15 minutes of work, eliminates an entire class of debugging pain. I did not call this out. Troblum caught it. He is right.

**Priority: Sprint 1, alongside #78.**

---

## 3. Error Recovery Strategy

**AGREE on the gap. PUSH BACK on urgency.**

For v0.1 single-player subprocess, the failure modes are:
- **Deserialization failure:** Development-time bug. Log, dump bytes, show "Internal error."
- **Server crash:** Godot gets SIGCHLD. Show "Simulation crashed," offer restart.
- **Client disconnect:** In subprocess mode, the pipe broke = child died.

Recovery (reconnection, state reload) is multiplayer infrastructure. Defer to post-v0.1.

**Synthesis: Define minimal error handling in Sprint 1. Not recovery — handling.**

---

## 4. Full IPC Loop Profiling

**AGREE. Strongly.**

D-020 says "1-5ms serialization latency per tick, acceptable." That was my estimate. Estimates are not measurements.

If the loop takes 1ms, golden. If 20ms, problem at 10 tps. If GDScript MessagePack is the bottleneck, we need to know NOW.

**Synthesis: Sprint 1 exit criterion. IPC round-trip measured.** Measure:
1. `rmp_serde::to_vec()` on representative `ObserverSnapshot` (15 entities)
2. `pipe.write()` latency
3. GDScript `StreamPeer.get_data()` + MessagePack decode
4. Scene tree update (create/move 15 sprites)
5. Total tick-to-render latency

If total exceeds 5ms, flag as risk before Sprint 2.

---

## 5. Deterministic Replay by Sprint 2

**AGREE. Reinforces D-030 Decision 7.**

Troblum's framing is sharper: "determinism is a claim you are making, and claims require proof." A deterministic replay system IS the proof.

**Synthesis: Sprint 2 hard requirement.** Record `(tick_number, input)` pairs. Replay with same seed + inputs. Compare entity positions. ~100 lines of Rust.

---

## 6. HashMap Iteration Order

**AGREE. Genuine time bomb.**

Rust's `HashMap` uses `RandomState` by default — randomized hasher, different iteration orders across runs. If ANY system iterates a `HashMap` and order affects outcomes, the simulation is non-deterministic even with the same seed.

I did not flag this. Troblum caught it. Exactly the kind of detail that matters for D-010 principle 4.

**Synthesis: Coding standard from Sprint 1.** No `HashMap` in gameplay-affecting code. Use `BTreeMap` (deterministic, sorted) or `IndexMap` (deterministic, insertion order). Add `#[deny(clippy::disallowed_types)]` for `std::collections::HashMap` in the simulation crate.

---

## 7. Flow Control / Backpressure

**PUSH BACK. Not a real risk at v0.1 scale.**

An `ObserverSnapshot` with 15 entities: ~3.5KB. At 10 tps = 35KB/s. Pipe buffer is 64KB. Client needs to stall for ~2 seconds before pipe blocks.

At D-026 scale (80 NPCs, ~20KB snapshots, 20 tps = 400KB/s) — pipe fills in 160ms. THAT is a backpressure scenario, but not v0.1.

**Synthesis: Monitor pipe write latency in Sprint 1 profiling. Implement only if measured as problem.**

---

## 8. System Dependency Graph

**AGREE. But let the code generate it.**

Hand-maintained dependency graph will be stale by Sprint 2. bevy_ecs already knows the dependency graph.

**Synthesis: Write a `print_system_graph` debug command that dumps bevy_ecs schedule ordering. Run in CI. Diff across commits.**

---

## 9-10. Tier Transition Stress Test + Archetype Fragmentation

**PUSH BACK. Premature.**

D-026 tier transitions are not implemented yet. The simulation does not have NPCs yet, let alone 500 of them. Stress-testing a system that does not exist is speculative benchmarking.

Troblum flagged archetype fragmentation as LOW risk in his own original evaluation. He is now contradicting his earlier assessment. At v0.1 scale (25 NPCs, 3 tiers), fragmentation produces at most 3 archetypes with 5-25 entities each.

**Synthesis: Define performance budget now. Benchmark when code exists.**

---

## Troblum's Unique Additions Accepted

| Finding | Verdict |
|---------|---------|
| TileMapLayer for fog rendering | Accept — debuggable, simplest for v0.1 |
| Sprite modulate for entity color | Accept — grayscale base sprites convention |
| Web export (Q-007) | Already flagged. No new action. |
| Crash reporting | Accept as post-v0.1 concern |
| Performance monitoring (`tracing`) | Accept as Sprint 1 infrastructure |

---

## Where Troblum Under-Indexes

**The content architecture.** His review focuses on infrastructure robustness but does not engage with the content systems (monologue pipeline, CauseChain, observation events). These are where architecture meets gameplay — and where the project succeeds or fails.

---

# Round 2: Troblum's Debate Response

**In response to:** Tyre's Round 1 findings

---

## 1. Knowledge Graph — CRITICAL Agreement + Push Forward

**AGREE with Tyre's assessment. PUSH BACK that he didn't specify the structure.**

Tyre correctly identifies `InformationInventory { known_facts: Vec<String> }` as a placeholder and rates it CRITICAL. But he doesn't propose a replacement.

**What's needed:**

```rust
#[derive(Component)]
pub struct KnowledgeGraph {
    pub entities: HashMap<Entity, EntityKnowledge>,
}

pub struct EntityKnowledge {
    pub last_known_position: Option<IVec2>,
    pub last_known_tick: u64,
    pub confidence: KnowledgeConfidence,
    pub relationship_state: Option<RelationshipSnapshot>,
    pub known_facts: HashSet<FactId>,
    pub source: KnowledgeSource,
}

pub enum KnowledgeConfidence {
    Certain,   // Directly observed
    Probable,  // Inferred
    Rumor,     // Told by someone
    Outdated,  // Haven't seen recently
}
```

This is pillar-1 foundation. Everything depends on it — observation, monologue, dialogue, success criteria.

---

## 2. Observer Snapshot Pipeline — CRITICAL Agreement + Algorithm Proposal

**AGREE. But Tyre flags CRITICAL without proposing the algorithm.**

```rust
fn generate_observer_snapshot(
    world: &World,
    observer: Entity,
    knowledge_graph: &KnowledgeGraph,
) -> ObserverSnapshot {
    // 1. Spatial query — what's nearby?
    let nearby = spatial_query(observer.position, MAX_RENDER_DISTANCE);

    // 2. Line-of-sight — what can observer SEE?
    let visible = nearby.filter(|e| line_of_sight(observer, e));

    // 3. Sound propagation — what can observer HEAR?
    let audible = sound_events.filter(|e| sound_reaches(observer, e));

    // 4. Knowledge filtering — what is observer ALLOWED to know?
    let filtered = visible.map(|e| filter_by_knowledge(observer, e, knowledge_graph));

    // 5. Snapshot assembly
    ObserverSnapshot { ... }
}
```

Each step has edge cases: spatial query data structure, shadowcasting integration, three-range model, knowledge filtering depends on KnowledgeGraph.

---

## 3. Blocking I/O — Upgrade to CRITICAL

**Tyre rates HIGH. I rate CRITICAL.**

Tyre says "will deadlock." My diagnosis: won't deadlock, will STUTTER. Stutter is worse — the game "works" but feels terrible.

If simulation tick takes 50-100ms and client calls `bridge.tick()` synchronously, the entire Godot render thread blocks. At 60 FPS (16ms budget), a 50ms block = 3-4 dropped frames every tick.

**Recommended fix:** Threaded bridge with crossbeam channels.

```rust
impl SimBridge for LocalBridge {
    fn tick(&mut self, input: PlayerInput) -> Option<ObserverSnapshot> {
        self.input_tx.send(input);
        self.snapshot_rx.try_recv().ok()  // Non-blocking, 1-frame delay acceptable
    }
}
```

---

## 4. Save/Load — Versioning Is Non-Negotiable

**AGREE with Tyre's custom SaveState approach over bevy_reflect.**

**Push back:** SaveState is a third data model alongside ECS + ObserverSnapshot. Every NPC component change requires updating 3 places. Real maintenance burden.

**Addition:** Save versioning with migration strategy. Players will complain if saves break.

---

## 5. Missing Operational Concerns

**Tyre's review finds missing game systems. He didn't look for missing operational systems.**

### 5.1 Error Handling Strategy

What happens when things go wrong?
- Simulation crashes -> ?
- MessagePack deserialization fails -> ?
- Pathfinding exceeds budget -> ?
- NPC enters invalid state -> ?

**Recommendation:**
1. Unrecoverable (crash): show error, emergency save, exit
2. Recoverable (bad data): log warning, skip frame, continue
3. Logic errors (invalid state): log error, reset to safe state

### 5.2 Profiling Infrastructure

Tyre checks budgets but doesn't specify HOW we measure.

```rust
pub struct TickProfiler {
    pub perception_time: Duration,
    pub pathfinding_time: Duration,
    pub ai_time: Duration,
    pub snapshot_generation_time: Duration,
    pub total_time: Duration,
}
```

Without this, we can't validate D-026 in practice.

### 5.3 Debug Visualization

When LOS is wrong or information leaks, how do we debug? Debug overlay in Godot showing pathfinding waypoints, LOS rays, vision cones, information boundary tags, tick timing. Development tool, not player-facing.

### 5.4 Timing Assertions in Integration Tests

```rust
#[test]
fn tick_completes_within_budget() {
    let mut sim = SimulationBuilder::new()
        .with_test_map(150, 150)
        .with_npcs(80)
        .build();
    let start = Instant::now();
    sim.tick();
    assert!(start.elapsed() < Duration::from_millis(100));
}
```

---

## 6. Risk Re-Assessment Comparison

| Risk | Tyre | Troblum | Rationale |
|------|------|---------|-----------|
| Information Boundaries | CRITICAL | CRITICAL | Full agreement |
| Observer Snapshot Bandwidth | MEDIUM | MEDIUM | Full agreement |
| Synchronous IPC Blocking | HIGH | **CRITICAL** | Blocking I/O makes game unplayable |
| Error Handling Gaps | Not rated | **HIGH** | Missing from Tyre's analysis |
| Profiling Infrastructure | Not rated | **MEDIUM-HIGH** | Can't validate budgets without measurement |

**Tyre under-weighted operational risks.** Focused on "what's missing" (correct) but not "how do we measure/debug/recover" (also needed).

---

## 7. Troblum's Verdict on Tyre's Review

**GOOD.** Core decisions sound. Gap analysis thorough. Testability defensible. Risk profile well-understood.

**But:** Operational infrastructure under-specified. Blocking I/O flagged but not elevated enough. KnowledgeGraph and observer pipeline called out but not designed.

---

# Consensus Recommendations

**Merged priority list both reviewers would sign off on.**

## Sprint 1 (Current)

| # | Recommendation | Source | Effort |
|---|---------------|--------|--------|
| 1 | Protocol version handshake in IPC connection | Troblum | XS (15 min) |
| 2 | Define `SpatialIndex` trait (naive impl behind it) | Troblum + Tyre synthesis | XS (30 min) |
| 3 | Basic error handling: log + dump + user message | Troblum, scoped by Tyre | S (1-2 hrs) |
| 4 | IPC round-trip timing as Sprint 1 exit criterion | Troblum, endorsed by Tyre | S (2-3 hrs) |
| 5 | `HashMap` ban in simulation crate (clippy rule) | Troblum | XS (15 min) |
| 6 | `tracing` crate setup for server instrumentation | Troblum | S (1 hr) |
| 7 | White/grayscale base sprites + modulate convention | Troblum | Convention only |
| 8 | TileMapLayer for fog rendering in v0.1 | Troblum | Convention only |
| 9 | Fix entity_renderer.gd `id` vs `entity_id` bug | Tyre | XS (5 min) |

## Sprint 2

| # | Recommendation | Source | Effort |
|---|---------------|--------|--------|
| 10 | Grid-based `SpatialIndex` implementation | Troblum + Tyre | M (3-5 days) |
| 11 | Deterministic replay: record/replay test (#201) | Both — non-negotiable | M (3-5 days) |
| 12 | KnowledgeGraph data model design spike | Both — CRITICAL | M (3-5 days) |
| 13 | Observer snapshot generation pipeline design | Both — CRITICAL | M (3-5 days) |
| 14 | Async/threaded LocalBridge implementation | Both — HIGH/CRITICAL | M (3-5 days) |
| 15 | System dependency graph debug command | Troblum, synthesized by Tyre | S (1-2 days) |
| 16 | Tier transition performance budgets defined | Troblum, scoped by Tyre | XS (document) |
| 17 | Tick budget overflow policy: slow real-time, don't skip ticks | Troblum | XS (document) |

## Sprint 3+

| # | Recommendation | Source | Effort |
|---|---------------|--------|--------|
| 18 | Backpressure monitoring (only if measured as issue) | Troblum | S if needed |
| 19 | Tier transition benchmarks (when code exists) | Troblum | M |
| 20 | Crash reporting infrastructure (before external playtest) | Troblum | M |
| 21 | Debug visualization overlay | Troblum | M |
| 22 | Timing assertions in integration tests | Troblum | S |
| 23 | Save/Load architecture design (with versioning) | Both | L |
| 24 | Map authoring pipeline (Tiled recommended) | Tyre | M |

## Not Accepted

| Recommendation | Reason |
|---------------|--------|
| Spatial partitioning implementation THIS sprint | Scope creep. Trait now, impl Sprint 2. |
| Full error recovery (reconnect, state reload) | Multiplayer concern. v0.1 is subprocess. |
| 500-NPC stress test now | System does not exist yet. |
| Backpressure implementation now | Math does not support urgency at v0.1 scale. |

---

## Final Assessment

**Architecture: GREEN.** The core decisions are sound. Subprocess/IPC was the right call. The information boundary model is architecturally correct. Deterministic simulation infrastructure is in place.

**Implementation plan: AMBER.** Critical systems (knowledge graph, observer pipeline) are underspecified. Operational infrastructure (profiling, error handling, debug tooling) has gaps. Blocking I/O needs resolution before end-to-end integration.

**Trajectory: ON TRACK.** Sprint 1 building blocks are in place. Week 8 kill switch is comfortably distant. The strongest signal from both reviews: **measure the IPC loop in Sprint 1, enforce determinism from Sprint 2, and never let infrastructure work orphan from a visible gameplay outcome.**

---

*Troblum catches the bugs. Tyre designs the systems. The synthesis is stronger than either review alone.*

---

## Files Reviewed

### Server (Rust)
- `server/Cargo.toml`
- `server/src/main.rs`
- `server/src/lib.rs`
- `server/src/bridge/mod.rs`
- `server/src/bridge/framing.rs`
- `server/src/bridge/local.rs`
- `server/src/bridge/types.rs`
- `server/src/simulation/mod.rs`
- `server/src/simulation/input.rs`
- `server/src/simulation/rng.rs`
- `server/src/simulation/tier.rs`
- `server/src/simulation/time.rs`
- `server/src/simulation/movement.rs`
- `server/src/npc/mod.rs`
- `server/src/perception/mod.rs`
- `server/src/storyteller/mod.rs`
- `server/src/cause_chain.rs`
- `server/tests/smoke.rs`
- `server/tests/bridge_ipc.rs`
- `server/tests/movement.rs`
- `server/tests/serialization.rs`
- `server/tests/gen_fixtures.rs`

### Client (GDScript)
- `client/scripts/main.gd`
- `client/scripts/autoloads/sim_bridge.gd`
- `client/scripts/autoloads/game_state.gd`
- `client/scripts/autoloads/input_mapper.gd`
- `client/scripts/protocol/protocol.gd`
- `client/scripts/rendering/entity_renderer.gd`
- `client/scripts/rendering/fog_renderer.gd`
- `client/scripts/rendering/world_renderer.gd`
- `client/scenes/main.tscn`

### Architecture & Decisions
- `decisions/architecture.md`
- `decisions/perception.md`
- `decisions/scope.md`
- `decisions/content.md`
- `decisions/process.md`
- `decisions/questions.md`
- `decisions/rejected.md`
- `docs/architecture/eval-godot-rust-bridge.md`
- `docs/architecture/risk-godot-rust-bridge.md`
- `docs/sprints/sprint-1/server.md`
- `docs/sprints/sprint-1/client.md`
- `Makefile`