Skip to content

Conversation

@Grufoony
Copy link
Collaborator

No description provided.

@codecov
Copy link

codecov bot commented Nov 12, 2025

Codecov Report

❌ Patch coverage is 97.80702% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.47%. Comparing base (30a595d) to head (aba9af1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
test/mobility/Test_graph.cpp 96.82% 0 Missing and 4 partials ⚠️
src/dsf/mobility/RoadNetwork.hpp 98.73% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #365      +/-   ##
==========================================
+ Coverage   82.91%   83.47%   +0.56%     
==========================================
  Files          50       51       +1     
  Lines        4892     5090     +198     
  Branches      559      587      +28     
==========================================
+ Hits         4056     4249     +193     
- Misses        828      829       +1     
- Partials        8       12       +4     
Flag Coverage Δ
unittests 83.47% <97.80%> (+0.56%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new RoadNetwork::shortestPath function that computes the shortest path between two specific nodes, along with a new PathCollection class to encapsulate path data structures. The PR also refactors the existing allPathsTo function to use the new PathCollection type.

  • Added PathCollection class with an explode method to enumerate all possible paths
  • Implemented shortestPath function using Dijkstra's algorithm with support for equivalent paths
  • Refactored allPathsTo to return PathCollection instead of raw std::unordered_map

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/dsf/mobility/PathCollection.hpp Defines the new PathCollection class extending std::unordered_map<Id, std::vector<Id>> with path enumeration functionality
src/dsf/mobility/PathCollection.cpp Implements the explode method for recursively enumerating all paths from source to target
src/dsf/mobility/RoadNetwork.hpp Adds shortestPath function and updates allPathsTo to use PathCollection; contains outdated comments and documentation issues
src/dsf/mobility/Itinerary.hpp Updates setPath and path methods to use PathCollection type; has parameter naming inconsistency in documentation
src/dsf/mobility/Itinerary.cpp Updates implementation to use PathCollection; has parameter naming inconsistency with header
src/dsf/bindings.cpp Adds Python bindings for PathCollection class and shortestPath method with dictionary-like interface
test/mobility/Test_graph.cpp Adds comprehensive test coverage for shortestPath and PathCollection::explode; contains misleading references to A* algorithm
test/mobility/Test_itinerary.cpp Updates tests to use PathCollection type and simplified namespace usage
benchmark/Bench_Network.cpp Renames existing benchmark to BM_RoadNetwork_AllPathsTo and adds new BM_RoadNetwork_ShortestPath benchmark

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

/// @brief Set the itinerary's path
/// @param path An adjacency matrix made by a SparseMatrix representing the itinerary's path
void setPath(std::unordered_map<Id, std::vector<Id>> path);
/// @param pathCollection A dsf::mobility::PathCollection representing all equivalent paths to the destination
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameter name mismatch in documentation. The documentation refers to @param pathCollection but the actual parameter name is path. The documentation should be updated to @param path to match the function signature.

Suggested change
/// @param pathCollection A dsf::mobility::PathCollection representing all equivalent paths to the destination
/// @param path A dsf::mobility::PathCollection representing all equivalent paths to the destination

Copilot uses AI. Check for mistakes.
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cppcheck (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@Grufoony Grufoony merged commit de34709 into main Nov 12, 2025
43 checks passed
@Grufoony Grufoony deleted the shortestPath branch November 12, 2025 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants