Fix predecessor DAG construction in k-shortest paths algorithm

[networmix]

Summary

This PR fixes bugs in the predecessor DAG (Directed Acyclic Graph) construction within the k-shortest paths algorithm. The changes correct how parent nodes and edges are indexed when building the DAG structure.

Key Changes

• Fixed DAG parent/edge indexing: Replaced sequential indexing with proper offset-based indexing using dag.parent_offsets. The previous implementation used a simple counter (idx) that didn't account for the correct position of each node in the parent arrays, causing incorrect DAG construction.

• Updated path comparison logic: Changed the path prefix comparison to use j + 1 instead of j when comparing node sequences. This ensures the correct number of nodes are compared when identifying paths that share a common prefix.

Implementation Details

The main issue was in how parents and edges were being stored in the DAG arrays. The old code:

std::size_t idx = 0;
for (std::size_t i = 1; i < P.nodes.size(); ++i) {
  dag.parents[idx] = P.nodes[i-1]; 
  dag.via_edges[idx] = P.edges[i-1]; 
  ++idx;
}

Has been corrected to:

for (std::size_t i = 1; i < P.nodes.size(); ++i) {
  auto v = P.nodes[i];
  auto base = static_cast<std::size_t>(dag.parent_offsets[static_cast<std::size_t>(v)]);
  dag.parents[base] = P.nodes[i-1]; 
  dag.via_edges[base] = P.edges[i-1];
}

This ensures each parent is stored at the correct offset corresponding to its destination node in the DAG structure. These fixes appear in two locations within the algorithm (lines 203-210 and 338-345).

https://claude.ai/code/session_011Wz1nQQWEnVTaLwfgz9LDA

[Copilot]

Pull request overview

Fixes incorrect predecessor DAG (PredDAG) construction in k_shortest_paths by aligning parent/edge storage with the project’s CSR-style parent_offsets layout, and corrects spur-prefix matching when excluding edges during candidate generation.

Changes:

• Store dag.parents / dag.via_edges using parent_offsets[v] (CSR base) instead of a sequential index.
• Fix prefix comparison during edge masking to compare j + 1 nodes (not j) when detecting shared prefixes.

Comments suppressed due to low confidence (1)

src/k_shortest_paths.cpp:258

• Add a focused test for the spur-prefix comparison/masking behavior around j: verify that only paths sharing the prefix of length (j+1) are considered for excluding edge P.edges[j]. This helps prevent regressions where comparing only j nodes causes overly aggressive edge masking and can drop valid candidates.

      for (auto const& P : paths) {
        if (P.nodes.size() > j && std::equal(P.nodes.begin(), P.nodes.begin() + static_cast<std::ptrdiff_t>(j + 1), last.nodes.begin())) {
          // Exclude edge at position j for this path
          if (P.edges.size() > j) {
            edge_mask_local[static_cast<std::size_t>(P.edges[j])] = 0;
          }
        }

---

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

[Copilot]

Consider adding a regression test that validates the returned PredDAG encodes the concrete path in true CSR form (parents/via_edges stored in the range parent_offsets[v]:parent_offsets[v+1] for each node v). The previous sequential fill could pass basic size/monotonicity checks but still produce incorrect parent lookups; a test that reconstructs the path from the DAG (e.g., via resolve_to_paths or by directly checking parents[parent_offsets[v]]) on a path whose node IDs are not in increasing order would catch this.