fix(simulation): address PR #55 review — stale comment, range, duplication, docs

- Fix protocol version comment (12 → 13) in ObserverSnapshot doc
- Widen Want intensity range from 3..=9 to 1..=10 to match spec and test
- Replace duplicated pool selection in trigger_recognition_monologue with
  call to select_pool_line helper (~50 lines removed)
- Document NaiveSpatialIndex migration cost for grid/quadtree swap
- Remove misleading Default derive from TellCategory (Nervous is not a
  sensible default for neutral NPCs)
- Add caller invariant doc on generate_npc (no TilePosition spawned)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

server/src/bridge/types.rs

@@ -37,7 +37,7 @@ pub const PROTOCOL_VERSION: u8 = 13;
 /// Future fields: ambient sound events, HUD state (D-020 expansion).
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ObserverSnapshot {
-    /// Protocol version for forward compatibility. Current: 12.
+    /// Protocol version for forward compatibility. Current: 13.
     pub version: u8,
     /// Simulation tick when this snapshot was produced
     pub tick: u64,

server/src/npc/generate.rs

@@ -149,7 +149,7 @@ fn pick_combat_style(idx: usize) -> CombatStyle {
 
 fn gen_want(rng: &mut SimRng, role: &RoleDefinition) -> Want {
     let kind_idx = rng.rng.random_range(0..9_usize);
-    let intensity = rng.rng.random_range(3_u8..=9);
+    let intensity = rng.rng.random_range(1_u8..=10);
     Want {
         primary: pick_want_kind(kind_idx),
         intensity,
@@ -421,6 +421,11 @@ fn gen_skills(rng: &mut SimRng, role: &RoleDefinition) -> (SkillSet, Option<Comb
 /// Spawns the entity into `world` with all 10 D-024 axis components.
 /// All randomness flows through `rng` — deterministic for a fixed seed (D-010).
 ///
+/// **Caller invariant:** The spawned entity does NOT include `TilePosition`
+/// or `ActiveSim`. The caller must place the NPC in the world (add
+/// `TilePosition`, register in `EntityRegistry`, and assign a simulation
+/// tier) after generation.
+///
 /// Returns the newly spawned `Entity` ID.
 pub fn generate_npc(role: &RoleDefinition, world: &mut World, rng: &mut SimRng) -> Entity {
     // Generate all axes before spawning to keep the borrow checker happy.

server/src/npc/tell_state.rs

@@ -35,10 +35,9 @@ use crate::simulation::tier::ActiveSim;
 ///
 /// Derived each tick from NPC simulation state — not authored per NPC.
 /// Five categories correspond to the D-024 tell taxonomy.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default, Serialize, Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub enum TellCategory {
     /// NPC exhibits nervous behaviour: Major secret + stress exceeds half of threshold.
-    #[default]
     Nervous,
     /// NPC exhibits angry behaviour: low contentment and Hostile mood.
     Angry,

server/src/simulation/monologue.rs

@@ -352,53 +352,9 @@ pub fn trigger_recognition_monologue(
     };
 
     // Try content pools for observe_anomaly trigger lines
-    let line = if let Some(ref content) = content {
-        let character = state.character.as_str();
-        let mut candidates: Vec<(&str, &str)> = Vec::new();
-
-        for district in content.0.districts.values() {
-            for pool in &district.monologue_pools {
-                if pool.character != character {
-                    continue;
-                }
-                for line in &pool.lines {
-                    if line.trigger != "observe_anomaly" {
-                        continue;
-                    }
-                    if state.shown_ids.contains(&line.id) {
-                        continue;
-                    }
-                    candidates.push((&line.id, &line.text));
-                }
-            }
-        }
-
-        if candidates.is_empty() {
-            // Fallback: allow repeats from content pools
-            for district in content.0.districts.values() {
-                for pool in &district.monologue_pools {
-                    if pool.character != character {
-                        continue;
-                    }
-                    for line in &pool.lines {
-                        if line.trigger != "observe_anomaly" {
-                            continue;
-                        }
-                        candidates.push((&line.id, &line.text));
-                    }
-                }
-            }
-        }
-
-        if !candidates.is_empty() {
-            let i = rng.rng.random_range(0..candidates.len());
-            Some((candidates[i].0.to_string(), candidates[i].1.to_string()))
-        } else {
-            None
-        }
-    } else {
-        None
-    };
+    let line = content
+        .as_deref()
+        .and_then(|c| select_pool_line("observe_anomaly", &state, c, &mut rng.rng));
 
     // Use content pool line or hardcoded fallback
     let (id, text) = if let Some((id, text)) = line {

server/src/simulation/spatial.rs

@@ -33,6 +33,16 @@ pub trait SpatialIndex: Send + Sync {
 /// Replace with grid or quadtree when profiling shows this is a bottleneck.
 /// Deterministic iteration: entries stored in insertion order, but callers
 /// should not depend on ordering (sort by Entity::to_bits() if needed).
+///
+/// ## Migration cost for grid/quadtree swap
+///
+/// `sync_spatial_index` takes `ResMut<NaiveSpatialIndex>` directly because
+/// bevy_ecs cannot store `dyn SpatialIndex` as a Resource. A swap to Grid or
+/// BVH requires changing the concrete type in: (1) `sync_spatial_index` system
+/// parameter, (2) `SimulationPlugin` resource registration, (3) any system
+/// that queries `Res<NaiveSpatialIndex>` (currently: `update_follow_state`).
+/// The `SpatialIndex` trait ensures the API surface stays identical — only the
+/// type name changes at call sites. Estimated: ~5 lines per caller.
 #[derive(Resource, Debug, Default)]
 pub struct NaiveSpatialIndex {
     entries: Vec<(Entity, TilePosition)>,