Fix/audit findings

CITATION.cff

@@ -12,5 +12,5 @@ authors:
 repository-code: 'https://github.com/Grufoony/DynamicalSystemFramework'
 url: 'https://grufoony.github.io/DynamicalSystemFramework/'
 license: AGPL-3.0-only
-version: 7.0.4
-date-released: '2026-09-15'
+version: 7.1.0
+date-released: '2026-09-16'

src/dsf/base/Node.hpp

@@ -10,6 +10,7 @@
 #include "../utility/queue.hpp"
 #include "../utility/Typedef.hpp"
 
+#include <algorithm>
 #include <functional>
 #include <utility>
 #include <stdexcept>

src/dsf/base/PathCollection.cpp

@@ -1,42 +1,73 @@
 #include "PathCollection.hpp"
 
 #include <stdexcept>
+#include <unordered_set>
+
+namespace {
+  std::list<std::vector<dsf::Id>> explodeImpl(dsf::PathCollection const& collection,
+                                              dsf::Id const sourceId,
+                                              dsf::Id const targetId,
+                                              std::unordered_set<dsf::Id>& onStack);
+}  // namespace
 
 std::list<std::vector<dsf::Id>> dsf::PathCollection::explode(Id const sourceId,
                                                              Id const targetId) const {
-  std::list<std::vector<Id>> paths;
+  std::unordered_set<Id> onStack;
+  return explodeImpl(*this, sourceId, targetId, onStack);
+}
 
-  // Base case: if source equals target, return a path with just the source
-  if (sourceId == targetId) {
-    paths.push_back({sourceId});
-    return paths;
-  }
+namespace {
+  using dsf::Id;
 
-  // Check if sourceId exists in the map
-  auto it = this->find(sourceId);
-  if (it == this->end()) {
-    throw std::runtime_error(
-        std::format("Source {} not found in PathCollection", sourceId));
-  }
+  std::list<std::vector<Id>> explodeImpl(dsf::PathCollection const& collection,
+                                         Id const sourceId,
+                                         Id const targetId,
+                                         std::unordered_set<Id>& onStack) {
+    std::list<std::vector<Id>> paths;
+
+    // Base case: if source equals target, return a path with just the source
+    if (sourceId == targetId) {
+      paths.push_back({sourceId});
+      return paths;
+    }
+
+    // Check if sourceId exists in the map
+    auto it = collection.find(sourceId);
+    if (it == collection.end()) {
+      throw std::runtime_error(
+          std::format("Source {} not found in PathCollection", sourceId));
+    }
 
-  auto const& nextHops = it->second;
-
-  // For each possible next hop from sourceId
-  for (auto const& hop : nextHops) {
-    if (hop == targetId) {
-      // Direct path found
-      paths.push_back({sourceId, targetId});
-    } else {
-      // Recursively find paths from hop to target
-      auto subPaths = explode(hop, targetId);
-
-      // Prepend sourceId to each sub-path
-      for (auto& subPath : subPaths) {
-        subPath.insert(subPath.begin(), sourceId);
-        paths.push_back(std::move(subPath));
+    // The hop graph is built to be acyclic, but a hand-assembled PathCollection need
+    // not be; without this guard a cycle recurses until the stack runs out.
+    if (!onStack.insert(sourceId).second) {
+      return paths;
+    }
+    struct StackGuard {
+      std::unordered_set<Id>& set;
+      Id id;
+      ~StackGuard() { set.erase(id); }
+    } guard{onStack, sourceId};
+
+    auto const& nextHops = it->second;
+
+    // For each possible next hop from sourceId
+    for (auto const& hop : nextHops) {
+      if (hop == targetId) {
+        // Direct path found
+        paths.push_back({sourceId, targetId});
+      } else {
+        // Recursively find paths from hop to target
+        auto subPaths = explodeImpl(collection, hop, targetId, onStack);
+
+        // Prepend sourceId to each sub-path
+        for (auto& subPath : subPaths) {
+          subPath.insert(subPath.begin(), sourceId);
+          paths.push_back(std::move(subPath));
+        }
       }
     }
-  }
 
-  return paths;
-}
+    return paths;
+  }
+}  // namespace

src/dsf/dsf.hpp

@@ -8,8 +8,8 @@
 #include <spdlog/sinks/basic_file_sink.h>
 
 static constexpr uint8_t DSF_VERSION_MAJOR = 7;
-static constexpr uint8_t DSF_VERSION_MINOR = 0;
-static constexpr uint8_t DSF_VERSION_PATCH = 4;
+static constexpr uint8_t DSF_VERSION_MINOR = 1;
+static constexpr uint8_t DSF_VERSION_PATCH = 0;
 
 static auto const DSF_VERSION =
     std::format("{}.{}.{}", DSF_VERSION_MAJOR, DSF_VERSION_MINOR, DSF_VERSION_PATCH);

src/dsf/mdt/PointsCluster.cpp

@@ -46,16 +46,15 @@ namespace dsf::mdt {
     m_centroid = dsf::geometry::Point(compute_median(xs), compute_median(ys));
   }
 
-  void PointsCluster::addActivityPoint(ActivityPoint const& activityPoint) noexcept {
+  void PointsCluster::addActivityPoint(ActivityPoint const& activityPoint) {
     m_points.emplace_back(activityPoint);
     m_bSorted = false;
     m_centroid.reset();
   }
-  void PointsCluster::addPoint(std::time_t timestamp,
-                               dsf::geometry::Point const& point) noexcept {
+  void PointsCluster::addPoint(std::time_t timestamp, dsf::geometry::Point const& point) {
     this->addActivityPoint(ActivityPoint{timestamp, point});
   }
-  void PointsCluster::sort() const noexcept {
+  void PointsCluster::sort() const {
     if (m_bSorted) {
       return;
     }

src/dsf/mdt/PointsCluster.hpp

@@ -16,7 +16,7 @@ namespace dsf::mdt {
   private:
     mutable std::vector<ActivityPoint> m_points;
     mutable std::optional<dsf::geometry::Point> m_centroid;
-    mutable bool m_bSorted;
+    mutable bool m_bSorted{true};
     /// @brief Update the centroid of the cluster based on current activity points.
     /// The centroid is computed as the median of the x and y coordinates of the points.
     /// @throws std::runtime_error if the cluster is empty.
@@ -34,13 +34,13 @@ namespace dsf::mdt {
     PointsCluster& operator=(PointsCluster const& other) = default;
     /// @brief Add an activity point to the cluster.
     /// @param activityPoint The activity point to add.
-    void addActivityPoint(ActivityPoint const& activityPoint) noexcept;
+    void addActivityPoint(ActivityPoint const& activityPoint);
     /// @brief Add a point with timestamp to the cluster.
     /// @param timestamp The timestamp of the activity point.
     /// @param point The geometric point of the activity point.
-    void addPoint(std::time_t timestamp, dsf::geometry::Point const& point) noexcept;
+    void addPoint(std::time_t timestamp, dsf::geometry::Point const& point);
     /// @brief Sort the activity points in the cluster by timestamp.
-    void sort() const noexcept;
+    void sort() const;
     /// @brief Compute and return the centroid of the cluster.
     /// @return The centroid point of the cluster.
     dsf::geometry::Point centroid() const;

src/dsf/mdt/Trajectory.cpp

@@ -97,7 +97,7 @@ namespace dsf::mdt {
     }
   }
 
-  void Trajectory::sort() noexcept {
+  void Trajectory::sort() {
     if (m_bSorted) {
       return;
     }

src/dsf/mdt/Trajectory.hpp

@@ -14,7 +14,7 @@ namespace dsf::mdt {
   class Trajectory {
   private:
     std::vector<dsf::mdt::PointsCluster> m_points;
-    bool m_bSorted;
+    bool m_bSorted{true};
 
   public:
     Trajectory() = default;
@@ -34,7 +34,7 @@ namespace dsf::mdt {
     /// @param max_speed_kph The max allowed speed (in km/h) to consider a cluster as a stop point.
     void filter(double const cluster_radius_km, double const max_speed_kph);
     /// @brief Sort the trajectory points by timestamp.
-    void sort() noexcept;
+    void sort();
     /// @brief Get the number of points in the trajectory.
     /// @return The size of the trajectory.
     inline std::size_t size() const noexcept { return m_points.size(); }

src/dsf/mdt/TrajectoryCollection.cpp

@@ -112,14 +112,10 @@ namespace dsf::mdt {
           (currentCluster.lastTimestamp() + currentCluster.firstTimestamp()) * 0.5;
       auto const previous_time =
           (previousCluster.lastTimestamp() + previousCluster.firstTimestamp()) * 0.5;
-      if (current_time < previous_time) {
-        // Should never happen if data is clean
-        throw std::runtime_error(
-            "Timestamps are not in increasing order within the trajectory.");
-      }
-      if (current_time == previous_time) {
+      if (current_time <= previous_time) {
         spdlog::debug(
-            "Non-increasing timestamps detected. Skipping speed check for these points.");
+            "Non-increasing cluster midpoints detected. Skipping the speed check for "
+            "these clusters.");
         return true;
       }
       auto const speed_kph =
@@ -217,13 +213,14 @@ namespace dsf::mdt {
         if (!bShouldSplit) {
           bShouldSplit = !check_min_duration(currentCluster);
         }
-        // If constraint violated (max speed or min duration) - finalize current trajectory and start a new one
+        // If a constraint is violated, finalise the current trajectory and start a
+        // fresh one. The next iteration adds points[i]; re-adding currentCluster here
+        // would duplicate it across the two segments.
         if (bShouldSplit && !newTrajectory.empty()) {
           if (newTrajectory.size() >= min_points_per_trajectory) {
             trajectories.emplace_back(std::move(newTrajectory));
           }
           newTrajectory = Trajectory();
-          newTrajectory.addCluster(currentCluster);
         }
       }
       if (newTrajectory.size() >= min_points_per_trajectory) {

src/dsf/mobility/Agent.cpp

@@ -67,14 +67,17 @@ namespace dsf::mobility {
     m_distance += distance;
   }
   void Agent::updateItinerary() {
-    if (m_itineraryIdx < m_trip.size() - 1) {
+    if (!m_trip.empty() && m_itineraryIdx < m_trip.size() - 1) {
       ++m_itineraryIdx;
     }
   }
   void Agent::reset(std::time_t const& spawnTime) {
     m_spawnTime = spawnTime;
     m_freeTime = 0;
     m_streetId = std::nullopt;
+    // NOTE: m_nextStreetId is deliberately preserved. FirstOrderDynamics'
+    // reinsertion path (m_reinsertAgents) relies on it to know where to put the
+    // agent back; clearing it here makes m_evolveAgents kill the agent instead.
     m_speed = 0.;
     m_distance = 0.;
     m_itineraryIdx = 0;