Skip to content

Fix MLD search functions to return unpacked edges#7408

Draft
Claude wants to merge 3 commits intomasterfrom
claude/fix-search-functions-return-structure
Draft

Fix MLD search functions to return unpacked edges#7408
Claude wants to merge 3 commits intomasterfrom
claude/fix-search-functions-return-structure

Conversation

@Claude
Copy link
Contributor

@Claude Claude AI commented Mar 6, 2026

The MLD search wrapper was discarding pre-computed unpacked edges, forcing callers to recreate them via redundant facade.FindEdge() lookups. This eliminated work already done during the search phase.

Changes

  • Updated MLD search wrapper (routing_base_mld.hpp): Added unpacked_edges parameter to return both nodes and edges from search
  • Added unpackPath overload: Accepts pre-computed edges to bypass FindEdge() calls
  • Modified shortest path implementation (shortest_path_impl.hpp):
    • Updated searchWithUTurn and multi-candidate search to handle edges
    • Modified route_state structure to track edges alongside nodes
    • Updated unpackLegs to use pre-computed edges via new unpackPath overload
    • Adapted shortestPathWithWaypointUTurns and shortestPathWithWaypointContinuation for edge storage
  • Updated unit test wrapper (offline_facade.cpp): Matched new signature

Example

Before:

// Search computes edges but discards them
auto unpacked_path = search(...);
weight = unpacked_path.weight;
unpacked_nodes = std::move(unpacked_path.nodes);
// unpacked_path.edges discarded!

// Later: recreate edges via FindEdge
for (auto [from, to] : node_pairs) {
    edges.push_back(facade.FindEdge(from, to));
}

After:

// Search returns both nodes and edges
auto unpacked_path = search(...);
weight = unpacked_path.weight;
unpacked_nodes = std::move(unpacked_path.nodes);
unpacked_edges = std::move(unpacked_path.edges);

// Use pre-computed edges directly
unpackPath(facade, unpacked_nodes, unpacked_edges, ...);
Original prompt

This section details on the original issue you should resolve

<issue_title>search functions should return unpacked nodes and edges</issue_title>
<issue_description>As a left-over from the MLD shortest path plugin implementation in two stub functions unpacked_edges array is ignored and later created from unpacked_nodes via facade.FindEdge.

To simplify interfaces and avoid unnecessary work the search functions should return unpacked nodes and edges.

EDIT: removed 5.8 milestone after inspecting the shortest path plugin code: "easy" but not desirable solution requires duplication of paths concatenation logic.</issue_description>

<agent_instructions>Check if this still holds true and devise a fix for the issue described</agent_instructions>

Comments on the Issue (you are @claude[agent] in this section)

- Updated MLD search wrapper to accept unpacked_edges parameter
- Added new unpackPath overload that accepts pre-computed edges
- Modified shortestPathWithWaypointUTurns to store and use edges
- Modified shortestPathWithWaypointContinuation to store and use edges
- Updated route_state structure to track edges alongside nodes
- Updated unit test wrapper to match new signature
- Eliminates unnecessary facade.FindEdge calls by reusing edges from search

Co-authored-by: DennisOSRM <1067895+DennisOSRM@users.noreply.github.com>
@Claude Claude AI changed the title [WIP] Fix search functions to return unpacked nodes and edges Return unpacked edges from MLD search functions to avoid redundant FindEdge calls Mar 6, 2026
Co-authored-by: DennisOSRM <1067895+DennisOSRM@users.noreply.github.com>
@Claude Claude AI changed the title Return unpacked edges from MLD search functions to avoid redundant FindEdge calls Fix MLD search functions to return unpacked edges Mar 7, 2026
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.

search functions should return unpacked nodes and edges

2 participants