fix(simulation): address PR #39 review — 3 warnings + 6 suggestions

Warnings fixed:
- contraband.rs: scan event now always emits even when NPC already
  knows (was skipped by early `continue`). Contract matches doc.
- test_world/mod.rs: ScanEventBuffer added to player spawn bundle
  so check_contraband_scan doesn't silently no-op in gauntlet mode.
- npc/mod.rs → simulation/mod.rs: moved check_contraband_scan
  registration to SimulationPlugin (operates on player inventory and
  snapshot pipeline, consistent with process_talk_interaction).

Suggestions addressed:
- cross_room_transitions.rs T1: clarified standalone position vs
  constants.rs observer position in comment.
- dialogue.rs: Vec<&str> dedup replaced with BTreeSet<&str> for
  deterministic iteration (project convention).
- contraband.rs: added test for multiple simultaneous ScanAuthority
  NPCs in range (564 tests total).
- dialogue.rs: doc-comment on relationship_to_trust explaining
  KnowledgeConfidence ordering and Suspects default.
- cross_room_transitions.rs T5: noted direct KG API usage vs full
  perception system.
- sprint_gauntlet.rs: documented intentional Contentment { level: 0 }.
- content_scaling.rs: noted GAUNTLET_NPC_COUNT is manually maintained.
- contraband.rs: doc-comment on cross-plugin registration rationale.

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

server/src/npc/mod.rs

@@ -24,15 +24,8 @@ impl Plugin for NpcPlugin {
             .init_resource::<routine::PreviousDayPhase>()
             .add_systems(
                 Update,
-                (
+                routine::check_phase_transition
-                    routine::check_phase_transition
+                    .before(crate::simulation::pathfinding::compute_paths),
-                        .before(crate::simulation::pathfinding::compute_paths),
-                    crate::simulation::contraband::check_contraband_scan
-                        .after(crate::simulation::movement::validate_movement)
-                        .before(
-                            crate::perception::observer::compute_observer_snapshot,
-                        ),
-                ),
             );
 
         tracing::debug!("NpcPlugin initialized");

server/src/simulation/contraband.rs

@@ -68,7 +68,12 @@ impl ScanEventBuffer {
 /// 1. Query player's carried items for Contraband marker
 /// 2. If found and NPC doesn't already know: update NPC's KnowledgeGraph
 ///    with HasContraband fact (KnowsDetails, DirectObservation source)
-/// 3. Emit ScanEvent to the player's ScanEventBuffer
+/// 3. Emit ScanEvent to the player's ScanEventBuffer (always, regardless of
+///    detection result or prior knowledge — client renders the scan animation)
+///
+/// Registered in SimulationPlugin (not NpcPlugin) because it operates on
+/// player inventory and writes to the snapshot pipeline. Consistent with
+/// process_talk_interaction and other cross-entity systems.
 ///
 /// System ordering: after validate_movement, before compute_observer_snapshot.
 #[allow(clippy::type_complexity)]
@@ -110,34 +115,32 @@ pub fn check_contraband_scan(
         let fact_id = FactId(format!("contraband.detected_{}", player_sid.0));
 
         if has_contraband {
-            // Skip if NPC already knows about this player's contraband
+            // Only update KG on first detection (idempotent — don't overwrite existing fact)
-            if npc_kg.fact_at_least(&fact_id, KnowledgeConfidence::KnowsDetails) {
+            if !npc_kg.fact_at_least(&fact_id, KnowledgeConfidence::KnowsDetails) {
-                continue;
+                npc_kg.facts.insert(
+                    fact_id,
+                    FactKnowledge {
+                        confidence: KnowledgeConfidence::KnowsDetails,
+                        source: KnowledgeSource::DirectObservation { tick: time.tick },
+                        state: KnowledgeState::Active,
+                        acquired_tick: time.tick,
+                    },
+                );
+
+                // Also ensure the NPC has entity knowledge of the player
+                npc_kg.observe_entity(player_sid, player_pos, time.tick);
+
+                tracing::info!(
+                    npc_id = npc_sid.0,
+                    player_id = player_sid.0,
+                    tick = time.tick,
+                    "Contraband detected: NPC scanned player and found contraband"
+                );
             }
-
-            // Update NPC's KG: record HasContraband fact
-            npc_kg.facts.insert(
-                fact_id,
-                FactKnowledge {
-                    confidence: KnowledgeConfidence::KnowsDetails,
-                    source: KnowledgeSource::DirectObservation { tick: time.tick },
-                    state: KnowledgeState::Active,
-                    acquired_tick: time.tick,
-                },
-            );
-
-            // Also ensure the NPC has entity knowledge of the player
-            npc_kg.observe_entity(player_sid, player_pos, time.tick);
-
-            tracing::info!(
-                npc_id = npc_sid.0,
-                player_id = player_sid.0,
-                tick = time.tick,
-                "Contraband detected: NPC scanned player and found contraband"
-            );
         }
 
-        // Emit scan event regardless (client renders the scan itself)
+        // Emit scan event regardless of detection or prior knowledge
+        // (client renders the scan animation itself)
         scan_buffer.push(ScanEvent {
             scanner_entity_id: npc_sid.0,
             detected_contraband: has_contraband,
@@ -475,6 +478,72 @@ mod tests {
         let npc_kg = world.get::<KnowledgeGraph>(npc).unwrap();
         let fact = npc_kg.facts.get(&fact_id).unwrap();
         assert_eq!(fact.acquired_tick, 0, "should not overwrite existing knowledge");
+
+        // Scan event should still fire even though NPC already knew
+        let mut buffer = world.get_mut::<ScanEventBuffer>(player).unwrap();
+        let events = buffer.take();
+        assert_eq!(events.len(), 1, "scan event should emit even for already-known contraband");
+        assert!(events[0].detected_contraband);
+    }
+
+    #[test]
+    fn multiple_scan_authority_npcs_each_emit_event() {
+        let mut world = setup_world();
+
+        let player = world
+            .spawn((
+                PlayerCharacter,
+                TilePosition::new(5, 5, 0),
+                KnowledgeGraph::new(),
+                ScanEventBuffer::default(),
+            ))
+            .id();
+        let player_sid = world.resource_mut::<EntityRegistry>().register(player);
+
+        world.spawn((
+            CarriedBy(player_sid),
+            ItemName("Lattice Component".into()),
+            InventorySlot(0),
+            Contraband,
+        ));
+
+        // Two NPCs with ScanAuthority, both in range
+        let npc1 = world
+            .spawn((
+                Npc,
+                TilePosition::new(5, 6, 0),
+                KnowledgeGraph::new(),
+                ScanAuthority,
+            ))
+            .id();
+        world.resource_mut::<EntityRegistry>().register(npc1);
+
+        let npc2 = world
+            .spawn((
+                Npc,
+                TilePosition::new(6, 5, 0),
+                KnowledgeGraph::new(),
+                ScanAuthority,
+            ))
+            .id();
+        world.resource_mut::<EntityRegistry>().register(npc2);
+
+        let mut schedule = bevy_ecs::schedule::Schedule::default();
+        schedule.add_systems(check_contraband_scan);
+        schedule.run(&mut world);
+
+        // Both NPCs should have KG entries
+        let fact_id = FactId(format!("contraband.detected_{}", player_sid.0));
+        let npc1_kg = world.get::<KnowledgeGraph>(npc1).unwrap();
+        assert!(npc1_kg.fact_at_least(&fact_id, KnowledgeConfidence::KnowsDetails));
+        let npc2_kg = world.get::<KnowledgeGraph>(npc2).unwrap();
+        assert!(npc2_kg.fact_at_least(&fact_id, KnowledgeConfidence::KnowsDetails));
+
+        // Both should emit separate scan events
+        let mut buffer = world.get_mut::<ScanEventBuffer>(player).unwrap();
+        let events = buffer.take();
+        assert_eq!(events.len(), 2, "each ScanAuthority NPC should emit a scan event");
+        assert!(events.iter().all(|e| e.detected_contraband));
     }
 
     #[test]

server/src/simulation/dialogue.rs

@@ -15,6 +15,8 @@
 //! - Writes DialogueResponseBuffer for snapshot inclusion
 //! - Uses SimRng for deterministic weighted random selection
 
+use std::collections::BTreeSet;
+
 use bevy_ecs::prelude::*;
 use rand::Rng;
 
@@ -177,6 +179,19 @@ pub fn available_access_tiers(relationship: RelationshipState) -> Vec<AccessTier
 ///   - Secret: Friendly + KnowsDetails+ (deep rapport + actionable knowledge)
 ///   - Real: (Friendly or Known) + KnowsOf+ (rapport + substantive knowledge)
 ///   - Surface: everything else (baseline, always available)
+/// Map relationship + knowledge confidence to trust tier (D-075).
+///
+/// Trust tier gates which dialogue lines are available. The layered gate
+/// requires BOTH sufficient relationship AND sufficient KG confidence:
+///   Surface: any relationship, any confidence (baseline)
+///   Real:    (Friendly|Known) + KnowsOf+  (rapport + substantive knowledge)
+///   Secret:  Friendly + KnowsDetails+     (deep rapport + actionable knowledge)
+///
+/// KnowledgeConfidence ordering is load-bearing here — the >= comparison
+/// relies on the derive(PartialOrd) order: Suspects < KnowsOf < KnowsDetails < Direct.
+///
+/// Unknown NPCs (no KG entry) default to Suspects, yielding Surface tier.
+/// This is correct: you can't have deep dialogue with someone you know nothing about.
 pub fn relationship_to_trust(
     relationship: RelationshipState,
     confidence: crate::knowledge::types::KnowledgeConfidence,
@@ -383,6 +398,8 @@ pub fn process_talk_interaction(
     let situations = derive_situations(time.day_phase(), relationship);
 
     // Layer 3: Trust tier from relationship + confidence (D-075)
+    // Default to Suspects for unknown NPCs — no KG entry means no basis for
+    // deeper dialogue, which correctly yields Surface trust tier.
     let confidence = target_stable
         .and_then(|sid| observer_kg.confidence_of(&sid))
         .unwrap_or(crate::knowledge::types::KnowledgeConfidence::Suspects);
@@ -390,7 +407,7 @@ pub fn process_talk_interaction(
 
     // Query Layers 1-3: collect candidates across all available access tiers
     let mut candidates: Vec<&IndexedDialogueLine> = Vec::new();
-    let mut seen_ids: Vec<&str> = Vec::new();
+    let mut seen_ids: BTreeSet<&str> = BTreeSet::new();
 
     for access in &access_tiers {
         let results = line_pool.0.query_dialogue(
@@ -401,9 +418,8 @@ pub fn process_talk_interaction(
             trust,
         );
         for line in results {
-            // Deduplicate across access tiers
+            // Deduplicate across access tiers (BTreeSet for deterministic iteration)
-            if !seen_ids.contains(&line.id.as_str()) {
+            if seen_ids.insert(&line.id) {
-                seen_ids.push(&line.id);
                 candidates.push(line);
             }
         }

server/src/simulation/mod.rs

@@ -39,6 +39,9 @@ impl Plugin for SimulationPlugin {
                     movement::validate_movement.after(path_follow::follow_paths),
                     path_follow::cleanup_path_blocked.after(movement::validate_movement),
                     listening::update_listening_focus.after(movement::validate_movement),
+                    contraband::check_contraband_scan
+                        .after(movement::validate_movement)
+                        .before(crate::perception::observer::compute_observer_snapshot),
                     time::advance_tick.after(path_follow::cleanup_path_blocked),
                 ),
             );

server/src/test_world/mod.rs

@@ -58,6 +58,7 @@ use crate::simulation::inventory::ItemName;
 #[cfg(feature = "gauntlet")]
 use crate::simulation::listening::ListeningFocus;
 #[cfg(feature = "gauntlet")]
+use crate::simulation::contraband::ScanEventBuffer;
 use crate::simulation::monologue::{MonologueBuffer, MonologueState, SprintAnomalyQueue};
 #[cfg(feature = "gauntlet")]
 use crate::simulation::movement::{PlayerCharacter, TilePosition, WalkabilityMap};
@@ -148,6 +149,7 @@ pub fn setup_gauntlet(app: &mut App) {
             MonologueState::default(),
             MonologueBuffer::default(),
             SprintAnomalyQueue::default(),
+            ScanEventBuffer::default(),
             CognitiveDelay::default(),
             ListeningFocus::new(player_pos),
             profile,

server/src/test_world/rooms/sprint_gauntlet.rs

@@ -58,7 +58,7 @@ pub fn spawn_entities(app: &mut App, registry: &mut EntityRegistry) {
                 intensity: 4,
                 description: "Sprint Gauntlet: visible NPC during sprint".to_string(),
             },
-            Contentment { level: 0 },
+            Contentment { level: 0 }, // Neutral — test NPC, contentment not load-bearing here
             ToleranceThreshold {
                 current_stress: 10,
                 threshold: 50,

server/tests/content_scaling.rs

@@ -47,6 +47,9 @@ const STRESS_TICKS: usize = 100;
 /// Fog Theater: 4, Occlusion Corridor: 4, Inventory Warehouse: 1, Pause Chamber: 1,
 /// Dialogue Room: 4, Crowd Plaza: 15, Sprint Gauntlet: 1, Eavesdrop Alcove: 2,
 /// Confrontation Stage: 2 = 34 total.
+///
+/// Manually maintained — update when rooms are added/changed. Future: derive
+/// from StableId ranges in constants.rs to avoid manual sync.
 const GAUNTLET_NPC_COUNT: usize = 34;
 
 /// Extra NPCs to spawn on top of the Gauntlet baseline to reach Active tier ceiling.

server/tests/cross_room_transitions.rs

@@ -63,8 +63,9 @@ fn run_listening_system(world: &mut World) {
 
 /// T1 — Sprint Exit.
 ///
-/// Player at Sprint Gauntlet observer position (4, 12 absolute) with Walk
+/// Player at (4, 12 absolute) — standalone scenario position south of the
-/// stance. A Readable sign at (6, 12) is within CLOSE_RANGE (distance 2).
+/// Sprint Gauntlet observer (4, 10 per constants.rs). A Readable sign at
+/// (6, 12) is within CLOSE_RANGE (distance 2).
 ///
 /// During Walk: sign appears in interaction buffer.
 /// During Sprint: buffer is empty (D-055 suppression).
@@ -365,13 +366,17 @@ fn t4_knowledge_graph_survives_room_transition() {
 /// T5 — Fog Carry-Over.
 ///
 /// Player observes NPC in Fog Theater at Direct confidence.
-/// Player moves to Hub (NPC now out of LOS). Observation system simulates
+/// Player moves to Hub (NPC now out of LOS). Confidence downgrades
-/// the confidence downgrade: Direct → KnowsDetails.
+/// from Direct to KnowsDetails.
 ///
 /// Asserts: KG entry persists (entity is remembered, not erased).
 /// Server-side "fog carry-over" means previously-seen entities remain in
 /// the KG at reduced confidence so the client can render a "last seen"
 /// fog state rather than a clean erasure.
+///
+/// NOTE: Uses direct KG API calls (observe_entity, observe_entity_leaving_los)
+/// rather than running the full perception system. This isolates the KG
+/// persistence contract from perception scheduling.
 #[test]
 fn t5_entity_knowledge_downgrades_on_los_exit_not_erased() {
     use settled_reach_server::knowledge::types::KnowledgeConfidence;