fix(simulation): address PR #12 review — z-level filter, visible tile dedup, version bump

- Filter remembered entities by z-level (Hoshe + Tyre warning)
- Skip remembered ghosts on currently visible tiles (Hoshe warning)
- Bump ObserverSnapshot version to 3 (Tyre suggestion)
- Add edge case tests: visible tile collision, different z-level,
  knowledge without position (Hoshe suggestion)
- Regenerate msgpack fixtures for v3

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

server/src/bridge/mod.rs

@@ -107,7 +107,7 @@ pub fn generate_snapshot(
         visible.len()
     );
     buffer.snapshot = Some(ObserverSnapshot {
-        version: 2,
+        version: 3,
         tick: time.tick,
         game_time,
         player_facing: FacingDirection::default(),

server/src/bridge/types.rs

@@ -12,11 +12,12 @@ pub use crate::simulation::time::DayPhase;
 /// Contains all information visible to the observer at a given tick.
 ///
 /// v2 adds: game_time, player_facing, visible_tiles, visibility sectors.
+/// v3 adds: relationship (D-033 entity color), observation (Visible/Remembered).
 /// Future fields: ambient sound events, internal monologue triggers,
 /// HUD state (D-020 expansion).
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ObserverSnapshot {
-    /// Protocol version for forward compatibility. Current: 2.
+    /// Protocol version for forward compatibility. Current: 3.
     pub version: u8,
     /// Simulation tick when this snapshot was produced
     pub tick: u64,

server/src/perception/observer.rs

@@ -141,6 +141,17 @@ pub fn compute_observer_snapshot(
             continue;
         };
 
+        // Skip if remembered position is on a different z-level than the observer
+        if position.z != z {
+            continue;
+        }
+
+        // Skip if the remembered tile is currently visible — if the player
+        // can see the tile and the entity isn't there, don't show a ghost.
+        if visible_positions.contains(&(position.x, position.y)) {
+            continue;
+        }
+
         // Direct confidence means the entity should be in LOS — if it isn't,
         // that's a transient data inconsistency. Skip rather than show a ghost.
         if knowledge.confidence == KnowledgeConfidence::Direct {
@@ -185,9 +196,9 @@ pub fn compute_observer_snapshot(
         visible_tiles.len(),
     );
 
-    // Step 8: Assemble snapshot
+    // Step 8: Assemble snapshot (v3: added relationship + observation fields)
     buffer.snapshot = Some(ObserverSnapshot {
-        version: 2,
+        version: 3,
         tick: time.tick,
         game_time,
         player_facing: facing,
@@ -229,7 +240,7 @@ mod tests {
 
         let buffer = world.resource::<SnapshotBuffer>();
         let snapshot = buffer.snapshot.as_ref().expect("snapshot should exist");
-        assert_eq!(snapshot.version, 2);
+        assert_eq!(snapshot.version, 3);
         assert_eq!(snapshot.entities.len(), 1);
         assert!(matches!(snapshot.entities[0].kind, EntityKind::Player));
         assert_eq!(snapshot.entities[0].observation, EntityVisibility::Visible);
@@ -445,15 +456,16 @@ mod tests {
         let mut world = setup_world(32, 32);
         let mut registry = EntityRegistry::new(0);
 
-        // NPC exists but is behind a wall (not visible)
+        // NPC exists far behind the player (not visible)
         let npc = world
-            .spawn((crate::npc::Npc, TilePosition::new(16, 10, 0)))
+            .spawn((crate::npc::Npc, TilePosition::new(16, 30, 0)))
             .id();
         let npc_sid = registry.register(npc);
 
-        // Player previously saw NPC, now it left LOS (KnowsDetails)
+        // Player previously saw NPC at (16, 28) — behind the player (south),
+        // well beyond peripheral range. The tile is NOT in the player's FOV.
         let mut kg = KnowledgeGraph::new();
-        kg.observe_entity(npc_sid, TilePosition::new(16, 14, 0), 50);
+        kg.observe_entity(npc_sid, TilePosition::new(16, 28, 0), 50);
         kg.observe_entity_leaving_los(&npc_sid, 60);
         kg.set_relationship(&npc_sid, RelationshipState::PersonOfInterest);
 
@@ -466,11 +478,6 @@ mod tests {
             ))
             .id();
         registry.register(player);
-
-        // Wall blocks LOS to NPC's actual position
-        let mut walkability = world.resource_mut::<WalkabilityMap>();
-        walkability.set_walkable(&TilePosition::new(16, 12, 0), false);
-
         world.insert_resource(registry);
         world.insert_resource(SimulationTime { tick: 100, paused: false });
 
@@ -490,9 +497,9 @@ mod tests {
         assert_eq!(remembered.len(), 1, "should have one remembered entity");
         assert_eq!(remembered[0].relationship, RelationshipState::PersonOfInterest);
 
-        // Remembered entity should be at last_known_position (16, 14) not actual (16, 10)
+        // Remembered entity at last_known_position (16, 28), not actual (16, 30)
         assert_eq!(remembered[0].x, 16.5);
-        assert_eq!(remembered[0].y, 14.5);
+        assert_eq!(remembered[0].y, 28.5);
 
         if let EntityVisibility::Remembered { confidence, age_ticks } = &remembered[0].observation {
             assert_eq!(*confidence, KnowledgeConfidence::KnowsDetails);
@@ -549,4 +556,149 @@ mod tests {
             "Direct-confidence entities should not appear as remembered ghosts"
         );
     }
+
+    #[test]
+    fn remembered_entity_on_visible_tile_not_shown() {
+        // If the player can see a tile and the entity isn't there,
+        // don't show a ghost — the player knows it moved.
+        let mut world = setup_world(32, 32);
+        let mut registry = EntityRegistry::new(0);
+
+        let npc = world
+            .spawn((crate::npc::Npc, TilePosition::new(30, 30, 0)))
+            .id();
+        let npc_sid = registry.register(npc);
+
+        // Player remembers NPC at (16, 15) — a tile the player can currently see
+        let mut kg = KnowledgeGraph::new();
+        kg.observe_entity(npc_sid, TilePosition::new(16, 15, 0), 50);
+        kg.observe_entity_leaving_los(&npc_sid, 60);
+
+        let player = world
+            .spawn((
+                PlayerCharacter,
+                TilePosition::new(16, 16, 0),
+                Facing(FacingDirection::North),
+                kg,
+            ))
+            .id();
+        registry.register(player);
+        world.insert_resource(registry);
+
+        let mut schedule = bevy_ecs::schedule::Schedule::default();
+        schedule.add_systems(compute_observer_snapshot);
+        schedule.run(&mut world);
+
+        let buffer = world.resource::<SnapshotBuffer>();
+        let snapshot = buffer.snapshot.as_ref().unwrap();
+
+        let remembered: Vec<_> = snapshot
+            .entities
+            .iter()
+            .filter(|e| matches!(e.observation, EntityVisibility::Remembered { .. }))
+            .collect();
+        assert!(
+            remembered.is_empty(),
+            "ghost should not appear on a tile the player can currently see"
+        );
+    }
+
+    #[test]
+    fn remembered_entity_different_z_not_shown() {
+        // Remembered entity on a different z-level should not appear
+        let mut world = setup_world(32, 32);
+        let mut registry = EntityRegistry::new(0);
+
+        let npc = world
+            .spawn((crate::npc::Npc, TilePosition::new(5, 5, 1)))
+            .id();
+        let npc_sid = registry.register(npc);
+
+        // Player remembers NPC at z=1, but player is at z=0
+        let mut kg = KnowledgeGraph::new();
+        kg.observe_entity(npc_sid, TilePosition::new(5, 5, 1), 50);
+        kg.observe_entity_leaving_los(&npc_sid, 60);
+
+        let player = world
+            .spawn((
+                PlayerCharacter,
+                TilePosition::new(16, 16, 0),
+                Facing(FacingDirection::North),
+                kg,
+            ))
+            .id();
+        registry.register(player);
+        world.insert_resource(registry);
+
+        let mut schedule = bevy_ecs::schedule::Schedule::default();
+        schedule.add_systems(compute_observer_snapshot);
+        schedule.run(&mut world);
+
+        let buffer = world.resource::<SnapshotBuffer>();
+        let snapshot = buffer.snapshot.as_ref().unwrap();
+
+        let remembered: Vec<_> = snapshot
+            .entities
+            .iter()
+            .filter(|e| matches!(e.observation, EntityVisibility::Remembered { .. }))
+            .collect();
+        assert!(
+            remembered.is_empty(),
+            "remembered entity on different z-level should not appear in snapshot"
+        );
+    }
+
+    #[test]
+    fn knowledge_without_position_not_shown() {
+        // Entity known via gossip (no last_known_position) should not appear
+        let mut world = setup_world(32, 32);
+        let mut registry = EntityRegistry::new(0);
+
+        let npc = world
+            .spawn((crate::npc::Npc, TilePosition::new(5, 5, 0)))
+            .id();
+        let npc_sid = registry.register(npc);
+
+        // Player knows about NPC but has never seen it (no position)
+        let mut kg = KnowledgeGraph::new();
+        // Insert knowledge manually without a position
+        kg.entities.insert(npc_sid, crate::knowledge::EntityKnowledge {
+            last_known_position: None,
+            last_observed_tick: 0,
+            last_updated_tick: 50,
+            confidence: KnowledgeConfidence::KnowsOf,
+            source: crate::knowledge::KnowledgeSource::Background,
+            state: crate::knowledge::KnowledgeState::Active,
+            relationship: RelationshipState::PersonOfInterest,
+            known_attributes: std::collections::BTreeMap::new(),
+        });
+
+        let player = world
+            .spawn((
+                PlayerCharacter,
+                TilePosition::new(16, 16, 0),
+                Facing(FacingDirection::North),
+                kg,
+            ))
+            .id();
+        registry.register(player);
+        world.insert_resource(registry);
+
+        let mut schedule = bevy_ecs::schedule::Schedule::default();
+        schedule.add_systems(compute_observer_snapshot);
+        schedule.run(&mut world);
+
+        let buffer = world.resource::<SnapshotBuffer>();
+        let snapshot = buffer.snapshot.as_ref().unwrap();
+
+        let remembered: Vec<_> = snapshot
+            .entities
+            .iter()
+            .filter(|e| matches!(e.observation, EntityVisibility::Remembered { .. }))
+            .collect();
+        assert!(
+            remembered.is_empty(),
+            "entity without last_known_position should not appear as ghost"
+        );
+    }
 }

server/tests/bridge_ipc.rs

@@ -33,7 +33,7 @@ fn snapshot_roundtrip_over_unix_socket() {
         let bridge = LocalBridge::accept(&server_path).expect("failed to accept");
 
         let snapshot = ObserverSnapshot {
-            version: 2,
+            version: 3,
             tick: 42,
             game_time: GameTime {
                 day: 0,

server/tests/bridge_tcp.rs

@@ -19,7 +19,7 @@ fn snapshot_roundtrip_over_tcp() {
         let bridge = TcpBridge::accept_on(listener).expect("failed to accept");
 
         let snapshot = ObserverSnapshot {
-            version: 2,
+            version: 3,
             tick: 42,
             game_time: GameTime {
                 day: 0,

server/tests/game_loop.rs

@@ -61,7 +61,7 @@ fn player_moves_north_through_full_pipeline() {
         rmp_serde::from_slice(&response).expect("deserialize snapshot");
 
     // Snapshot captures state at end of tick 0 (before advance_tick increments to 1)
-    assert_eq!(snapshot.version, 2);
+    assert_eq!(snapshot.version, 3);
     assert_eq!(snapshot.tick, 0);
     assert_eq!(snapshot.entities.len(), 1);
 

server/tests/gen_fixtures.rs

@@ -18,7 +18,7 @@ fn write_fixture(name: &str, bytes: &[u8]) {
 /// Helper to create a minimal v2 snapshot for fixtures
 fn fixture_snapshot(tick: u64, entities: Vec<VisibleEntity>) -> ObserverSnapshot {
     ObserverSnapshot {
-        version: 2,
+        version: 3,
         tick,
         game_time: GameTime {
             day: 0,
@@ -150,7 +150,7 @@ fn generate_msgpack_fixtures() {
 
     // v2 snapshot with visible_tiles and game_time populated
     let snapshot_v2_full = ObserverSnapshot {
-        version: 2,
+        version: 3,
         tick: 500,
         game_time: GameTime {
             day: 1,

server/tests/serialization.rs

@@ -7,7 +7,7 @@ use std::fs;
 /// Helper to create a minimal v2 snapshot for tests
 fn test_snapshot(tick: u64, entities: Vec<VisibleEntity>) -> ObserverSnapshot {
     ObserverSnapshot {
-        version: 2,
+        version: 3,
         tick,
         game_time: GameTime {
             day: 0,
@@ -40,7 +40,7 @@ fn observer_snapshot_roundtrip() {
     let bytes = rmp_serde::to_vec_named(&snapshot).expect("serialize");
     let decoded: ObserverSnapshot = rmp_serde::from_slice(&bytes).expect("deserialize");
 
-    assert_eq!(decoded.version, 2);
+    assert_eq!(decoded.version, 3);
     assert_eq!(decoded.tick, 42);
     assert_eq!(decoded.entities.len(), 1);
     assert_eq!(decoded.entities[0].entity_id, 1);
@@ -178,7 +178,7 @@ fn all_entity_kind_variants_roundtrip() {
 #[test]
 fn snapshot_v2_fields_roundtrip() {
     let snapshot = ObserverSnapshot {
-        version: 2,
+        version: 3,
         tick: 100,
         game_time: GameTime {
             day: 3,
@@ -216,7 +216,7 @@ fn snapshot_v2_fields_roundtrip() {
     let bytes = rmp_serde::to_vec_named(&snapshot).expect("serialize");
     let decoded: ObserverSnapshot = rmp_serde::from_slice(&bytes).expect("deserialize");
 
-    assert_eq!(decoded.version, 2);
+    assert_eq!(decoded.version, 3);
     assert_eq!(decoded.game_time.day, 3);
     assert_eq!(decoded.game_time.time_of_day, 720);
     assert_eq!(decoded.game_time.day_phase, DayPhase::Evening);
@@ -244,7 +244,7 @@ fn all_facing_direction_variants_roundtrip() {
 
     for dir in directions {
         let snapshot = ObserverSnapshot {
-            version: 2,
+            version: 3,
             tick: 0,
             game_time: GameTime {
                 day: 0,