fix(simulation): Clippy cleanup and CI enforcement (#635)

Fix all Clippy warnings across the server codebase (2411 insertions, 1341 deletions). Raise type-complexity-threshold to 750 and too-many-arguments to 12 in .clippy.toml for idiomatic Bevy ECS system signatures. The server now passes `cargo clippy -- --deny warnings` cleanly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-17 10:33:15 +01:00
parent 7f7706442a
commit aa79dd97e7
84 changed files with 2409 additions and 1339 deletions
@@ -4,8 +4,8 @@
 // Scope tag system: NPCs with active scope tags stay pinned to ActiveSim (#98).
 // Timestamp-based eviction: LRU eviction when ActiveSim exceeds capacity (#97).

-use std::collections::{BTreeSet, BinaryHeap};
 use std::cmp::Reverse;
+use std::collections::{BTreeSet, BinaryHeap};

 use bevy_app::prelude::*;
 use bevy_ecs::prelude::*;
@@ -14,8 +14,8 @@ use serde::{Deserialize, Serialize};
 use crate::knowledge::graph::KnowledgeGraph;
 use crate::knowledge::registry::StableEntityId;
 use crate::knowledge::types::KnowledgeConfidence;
-use crate::npc::{Npc, RelationshipKind};
 use crate::npc::relationships::RelationshipGraph;
+use crate::npc::{Npc, RelationshipKind};
 use crate::simulation::movement::{PlayerCharacter, TilePosition};

 // --- Tier radius constants (D-026) ---
@@ -238,7 +238,10 @@ pub fn assign_scope_tags(
        .relationships_of(&player_id)
        .into_iter()
        .filter(|(_, edge)| {
-            matches!(edge.kind, RelationshipKind::Friend | RelationshipKind::Colleague)
+            matches!(
+                edge.kind,
+                RelationshipKind::Friend | RelationshipKind::Colleague
+            )
        })
        .map(|(target_id, _)| *target_id)
        .collect();
@@ -324,19 +327,17 @@ pub fn update_last_interaction_tick(

    // Update existing LastInteractionTick for visible NPCs.
    for (pos, mut last_tick) in &mut npcs_with_tick {
-        if pos.z == vis_geo.observer_z
-            && vis_geo.visible_positions.contains(&(pos.x, pos.y))
-        {
+        if pos.z == vis_geo.observer_z && vis_geo.visible_positions.contains(&(pos.x, pos.y)) {
            last_tick.0 = current_tick;
        }
    }

    // Insert LastInteractionTick for NPCs that don't have it yet but are visible.
    for (entity, pos) in &npcs_without_tick {
-        if pos.z == vis_geo.observer_z
-            && vis_geo.visible_positions.contains(&(pos.x, pos.y))
-        {
-            commands.entity(entity).insert(LastInteractionTick(current_tick));
+        if pos.z == vis_geo.observer_z && vis_geo.visible_positions.contains(&(pos.x, pos.y)) {
+            commands
+                .entity(entity)
+                .insert(LastInteractionTick(current_tick));
        }
    }
 }
@@ -392,9 +393,15 @@ pub fn evict_excess_active(

        let dist = tile_distance(player_pos, &pos);
        if dist > BACKGROUND_RADIUS {
-            commands.entity(entity).remove::<ActiveSim>().insert(StateSaved);
+            commands
+                .entity(entity)
+                .remove::<ActiveSim>()
+                .insert(StateSaved);
        } else {
-            commands.entity(entity).remove::<ActiveSim>().insert(BackgroundSim);
+            commands
+                .entity(entity)
+                .remove::<ActiveSim>()
+                .insert(BackgroundSim);
        }
        evicted += 1;
    }
@@ -517,8 +524,7 @@ mod tests {
        let background = world.spawn(BackgroundSim).id();
        let _state_saved = world.spawn(StateSaved).id();

-        let mut query =
-            world.query_filtered::<bevy_ecs::entity::Entity, With<BackgroundSim>>();
+        let mut query = world.query_filtered::<bevy_ecs::entity::Entity, With<BackgroundSim>>();
        let results: Vec<bevy_ecs::entity::Entity> = query.iter(&world).collect();

        assert_eq!(results.len(), 1, "only one BackgroundSim entity expected");
@@ -532,8 +538,7 @@ mod tests {
        let _background = world.spawn(BackgroundSim).id();
        let state_saved = world.spawn(StateSaved).id();

-        let mut query =
-            world.query_filtered::<bevy_ecs::entity::Entity, With<StateSaved>>();
+        let mut query = world.query_filtered::<bevy_ecs::entity::Entity, With<StateSaved>>();
        let results: Vec<bevy_ecs::entity::Entity> = query.iter(&world).collect();

        assert_eq!(results.len(), 1, "only one StateSaved entity expected");
@@ -610,11 +615,13 @@ mod tests {
            world.get::<BackgroundSim>(entity).is_some(),
            "BackgroundSim added"
        );
-        assert!(world.get::<ActiveSim>(entity).is_none(), "ActiveSim removed");
+        assert!(
+            world.get::<ActiveSim>(entity).is_none(),
+            "ActiveSim removed"
+        );

        // Must NOT appear in ActiveSim query after demotion
-        let mut active_query =
-            world.query_filtered::<bevy_ecs::entity::Entity, With<ActiveSim>>();
+        let mut active_query = world.query_filtered::<bevy_ecs::entity::Entity, With<ActiveSim>>();
        assert_eq!(
            active_query.iter(&world).count(),
            0,
@@ -686,7 +693,10 @@ mod tests {
        let npc = world.spawn((ActiveSim, make_pos(60, 0))).id();
        run_tier_update(&mut world);
        assert!(world.get::<ActiveSim>(npc).is_none(), "ActiveSim removed");
-        assert!(world.get::<BackgroundSim>(npc).is_some(), "BackgroundSim added");
+        assert!(
+            world.get::<BackgroundSim>(npc).is_some(),
+            "BackgroundSim added"
+        );
    }

    #[test]
@@ -707,7 +717,10 @@ mod tests {
        world.spawn((PlayerCharacter, make_pos(0, 0)));
        let npc = world.spawn((BackgroundSim, make_pos(20, 0))).id();
        run_tier_update(&mut world);
-        assert!(world.get::<BackgroundSim>(npc).is_none(), "BackgroundSim removed");
+        assert!(
+            world.get::<BackgroundSim>(npc).is_none(),
+            "BackgroundSim removed"
+        );
        assert!(world.get::<ActiveSim>(npc).is_some(), "ActiveSim added");
    }

@@ -718,7 +731,10 @@ mod tests {
        world.spawn((PlayerCharacter, make_pos(0, 0)));
        let npc = world.spawn((BackgroundSim, make_pos(200, 0))).id();
        run_tier_update(&mut world);
-        assert!(world.get::<BackgroundSim>(npc).is_none(), "BackgroundSim removed");
+        assert!(
+            world.get::<BackgroundSim>(npc).is_none(),
+            "BackgroundSim removed"
+        );
        assert!(world.get::<StateSaved>(npc).is_some(), "StateSaved added");
    }

@@ -741,7 +757,10 @@ mod tests {
        let npc = world.spawn((StateSaved, make_pos(80, 0))).id();
        run_tier_update(&mut world);
        assert!(world.get::<StateSaved>(npc).is_none(), "StateSaved removed");
-        assert!(world.get::<BackgroundSim>(npc).is_some(), "BackgroundSim added");
+        assert!(
+            world.get::<BackgroundSim>(npc).is_some(),
+            "BackgroundSim added"
+        );
    }

    #[test]
@@ -761,13 +780,14 @@ mod tests {
        let mut world = World::new();
        world.spawn((PlayerCharacter, TilePosition::new(0, 0, 0)));
        // Spawn as ActiveSim at same x/y but different floor
-        let npc = world
-            .spawn((ActiveSim, TilePosition::new(0, 0, 1)))
-            .id();
+        let npc = world.spawn((ActiveSim, TilePosition::new(0, 0, 1))).id();
        run_tier_update(&mut world);
        // Should demote: u32::MAX > BACKGROUND_RADIUS → StateSaved
        assert!(world.get::<ActiveSim>(npc).is_none(), "ActiveSim removed");
-        assert!(world.get::<StateSaved>(npc).is_some(), "StateSaved due to z-distance");
+        assert!(
+            world.get::<StateSaved>(npc).is_some(),
+            "StateSaved due to z-distance"
+        );
    }

    #[test]
@@ -775,18 +795,28 @@ mod tests {
        // Distance = ACTIVE_RADIUS exactly → should stay Active (threshold is >)
        let mut world = World::new();
        world.spawn((PlayerCharacter, make_pos(0, 0)));
-        let npc = world.spawn((ActiveSim, make_pos(ACTIVE_RADIUS as i32, 0))).id();
+        let npc = world
+            .spawn((ActiveSim, make_pos(ACTIVE_RADIUS as i32, 0)))
+            .id();
        run_tier_update(&mut world);
-        assert!(world.get::<ActiveSim>(npc).is_some(), "stays Active at exact boundary");
+        assert!(
+            world.get::<ActiveSim>(npc).is_some(),
+            "stays Active at exact boundary"
+        );
    }

    #[test]
    fn npc_one_tile_beyond_active_radius_demotes() {
        let mut world = World::new();
        world.spawn((PlayerCharacter, make_pos(0, 0)));
-        let npc = world.spawn((ActiveSim, make_pos(ACTIVE_RADIUS as i32 + 1, 0))).id();
+        let npc = world
+            .spawn((ActiveSim, make_pos(ACTIVE_RADIUS as i32 + 1, 0)))
+            .id();
        run_tier_update(&mut world);
-        assert!(world.get::<ActiveSim>(npc).is_none(), "demoted to Background");
+        assert!(
+            world.get::<ActiveSim>(npc).is_none(),
+            "demoted to Background"
+        );
        assert!(world.get::<BackgroundSim>(npc).is_some());
    }

@@ -934,7 +964,9 @@ mod tests {
        world.init_resource::<RelationshipGraph>();

        // NPC exists but no PlayerCharacter
-        let npc = world.spawn((Npc, StableEntityId(crate::knowledge::types::StableId(1)))).id();
+        let npc = world
+            .spawn((Npc, StableEntityId(crate::knowledge::types::StableId(1))))
+            .id();

        run_assign_scope_tags(&mut world);

@@ -970,7 +1002,9 @@ mod tests {

        run_assign_scope_tags(&mut world);

-        let scope_tag = world.get::<ScopeTag>(npc).expect("ScopeTag should be assigned");
+        let scope_tag = world
+            .get::<ScopeTag>(npc)
+            .expect("ScopeTag should be assigned");
        assert!(
            scope_tag.contains(ScopeTagKind::KnownContact),
            "NPC known at KnowsOf level should get KnownContact tag"
@@ -1017,7 +1051,9 @@ mod tests {

        run_assign_scope_tags(&mut world);

-        let scope_tag = world.get::<ScopeTag>(npc).expect("ScopeTag assigned for colleague");
+        let scope_tag = world
+            .get::<ScopeTag>(npc)
+            .expect("ScopeTag assigned for colleague");
        assert!(
            scope_tag.contains(ScopeTagKind::Colleague),
            "Friend relationship should grant Colleague scope tag"
@@ -1067,7 +1103,10 @@ mod tests {

        assert_eq!(unpinned_results.len(), 1, "only one unpinned NPC");
        assert_eq!(unpinned_results[0], unpinned);
-        assert!(!unpinned_results.contains(&pinned), "pinned NPC excluded from eviction query");
+        assert!(
+            !unpinned_results.contains(&pinned),
+            "pinned NPC excluded from eviction query"
+        );
    }

    // -----------------------------------------------------------------------
@@ -1090,9 +1129,15 @@ mod tests {
        world.spawn((PlayerCharacter, make_pos(0, 0)));

        // Spawn 3 active NPCs (under cap of 5)
-        let npc1 = world.spawn((Npc, ActiveSim, make_pos(5, 0), LastInteractionTick(10))).id();
-        let npc2 = world.spawn((Npc, ActiveSim, make_pos(6, 0), LastInteractionTick(20))).id();
-        let npc3 = world.spawn((Npc, ActiveSim, make_pos(7, 0), LastInteractionTick(30))).id();
+        let npc1 = world
+            .spawn((Npc, ActiveSim, make_pos(5, 0), LastInteractionTick(10)))
+            .id();
+        let npc2 = world
+            .spawn((Npc, ActiveSim, make_pos(6, 0), LastInteractionTick(20)))
+            .id();
+        let npc3 = world
+            .spawn((Npc, ActiveSim, make_pos(7, 0), LastInteractionTick(30)))
+            .id();

        run_evict_excess_active(&mut world);

@@ -1112,16 +1157,28 @@ mod tests {
        world.spawn((PlayerCharacter, make_pos(0, 0)));

        // 3 NPCs, cap=2 → must evict 1 (the oldest: tick 10)
-        let oldest = world.spawn((Npc, ActiveSim, make_pos(5, 0), LastInteractionTick(10))).id();
-        let mid = world.spawn((Npc, ActiveSim, make_pos(6, 0), LastInteractionTick(20))).id();
-        let newest = world.spawn((Npc, ActiveSim, make_pos(7, 0), LastInteractionTick(30))).id();
+        let oldest = world
+            .spawn((Npc, ActiveSim, make_pos(5, 0), LastInteractionTick(10)))
+            .id();
+        let mid = world
+            .spawn((Npc, ActiveSim, make_pos(6, 0), LastInteractionTick(20)))
+            .id();
+        let newest = world
+            .spawn((Npc, ActiveSim, make_pos(7, 0), LastInteractionTick(30)))
+            .id();

        run_evict_excess_active(&mut world);

        assert!(world.get::<ActiveSim>(oldest).is_none(), "oldest evicted");
-        assert!(world.get::<BackgroundSim>(oldest).is_some(), "oldest → Background");
+        assert!(
+            world.get::<BackgroundSim>(oldest).is_some(),
+            "oldest → Background"
+        );
        assert!(world.get::<ActiveSim>(mid).is_some(), "mid stays Active");
-        assert!(world.get::<ActiveSim>(newest).is_some(), "newest stays Active");
+        assert!(
+            world.get::<ActiveSim>(newest).is_some(),
+            "newest stays Active"
+        );
    }

    #[test]
@@ -1134,20 +1191,30 @@ mod tests {
        world.spawn((PlayerCharacter, make_pos(0, 0)));

        // 2 NPCs, cap=1. The oldest is ScopePinned → skip it, evict the other.
-        let pinned = world.spawn((
-            Npc, ActiveSim, ScopePinned,
-            ScopeTag::with(ScopeTagKind::KnownContact),
-            make_pos(5, 0), LastInteractionTick(5),
-        )).id();
-        let unpinned = world.spawn((
-            Npc, ActiveSim,
-            make_pos(6, 0), LastInteractionTick(20),
-        )).id();
+        let pinned = world
+            .spawn((
+                Npc,
+                ActiveSim,
+                ScopePinned,
+                ScopeTag::with(ScopeTagKind::KnownContact),
+                make_pos(5, 0),
+                LastInteractionTick(5),
+            ))
+            .id();
+        let unpinned = world
+            .spawn((Npc, ActiveSim, make_pos(6, 0), LastInteractionTick(20)))
+            .id();

        run_evict_excess_active(&mut world);

-        assert!(world.get::<ActiveSim>(pinned).is_some(), "pinned NPC stays Active");
-        assert!(world.get::<ActiveSim>(unpinned).is_none(), "unpinned NPC evicted");
+        assert!(
+            world.get::<ActiveSim>(pinned).is_some(),
+            "pinned NPC stays Active"
+        );
+        assert!(
+            world.get::<ActiveSim>(unpinned).is_none(),
+            "unpinned NPC evicted"
+        );
        assert!(world.get::<BackgroundSim>(unpinned).is_some());
    }

@@ -1161,21 +1228,25 @@ mod tests {
        world.spawn((PlayerCharacter, make_pos(0, 0)));

        // NPC at distance 200 (beyond BACKGROUND_RADIUS=120) → StateSaved
-        let far = world.spawn((
-            Npc, ActiveSim,
-            make_pos(200, 0), LastInteractionTick(5),
-        )).id();
+        let far = world
+            .spawn((Npc, ActiveSim, make_pos(200, 0), LastInteractionTick(5)))
+            .id();
        // NPC at distance 5 (within ACTIVE_RADIUS) → stays
-        let near = world.spawn((
-            Npc, ActiveSim,
-            make_pos(5, 0), LastInteractionTick(50),
-        )).id();
+        let near = world
+            .spawn((Npc, ActiveSim, make_pos(5, 0), LastInteractionTick(50)))
+            .id();

        run_evict_excess_active(&mut world);

        assert!(world.get::<ActiveSim>(far).is_none(), "far NPC evicted");
-        assert!(world.get::<StateSaved>(far).is_some(), "far NPC → StateSaved");
-        assert!(world.get::<ActiveSim>(near).is_some(), "near NPC stays Active");
+        assert!(
+            world.get::<StateSaved>(far).is_some(),
+            "far NPC → StateSaved"
+        );
+        assert!(
+            world.get::<ActiveSim>(near).is_some(),
+            "near NPC stays Active"
+        );
    }

    #[test]
@@ -1189,16 +1260,21 @@ mod tests {

        // NPC without LastInteractionTick defaults to tick 0 (most stale)
        let no_tick = world.spawn((Npc, ActiveSim, make_pos(5, 0))).id();
-        let with_tick = world.spawn((
-            Npc, ActiveSim,
-            make_pos(6, 0), LastInteractionTick(100),
-        )).id();
+        let with_tick = world
+            .spawn((Npc, ActiveSim, make_pos(6, 0), LastInteractionTick(100)))
+            .id();

        run_evict_excess_active(&mut world);

-        assert!(world.get::<ActiveSim>(no_tick).is_none(), "no-tick NPC evicted first");
+        assert!(
+            world.get::<ActiveSim>(no_tick).is_none(),
+            "no-tick NPC evicted first"
+        );
        assert!(world.get::<BackgroundSim>(no_tick).is_some());
-        assert!(world.get::<ActiveSim>(with_tick).is_some(), "with-tick NPC stays");
+        assert!(
+            world.get::<ActiveSim>(with_tick).is_some(),
+            "with-tick NPC stays"
+        );
    }

    #[test]
@@ -1219,7 +1295,10 @@ mod tests {
        let pressure = world.resource::<SimSpacePressure>();
        // active_count is set BEFORE eviction runs (it reads the pre-eviction count).
        // The actual count changes via deferred commands, which apply after the system.
-        assert_eq!(pressure.active_count, 3, "pressure tracks pre-eviction count");
+        assert_eq!(
+            pressure.active_count, 3,
+            "pressure tracks pre-eviction count"
+        );
    }

    #[test]