routing: fix one-way and partial-edge correctness in edge-snapped routes#16
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5a964318a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let start_exit = EdgeTraversal { | ||
| node: from.to_node, | ||
| time_s: from_edge.travel_time_s * (1.0 - from.position), | ||
| distance_m: from_edge.distance_m * (1.0 - from.position), |
There was a problem hiding this comment.
If snap_to_edge returns position == 0.0 (or symmetrically 1.0), the traveler is already at from.from_node (to.to_node). This code still forces the search to depart via from.to_node, which can turn valid routes into NoPath or charge an entire extra edge. On the reviewed commit, a network with one-way edges 0->1 and 0->2 reproduces this: snap_to_edge(Coord::new(0.0, 0.0)) snaps to edge 0->1 at position: 0.0, and routing to a point halfway along 0->2 now fails even though the trip should just traverse half of 0->2.
Useful? React with 👍 / 👎.
| if to.position < from.position { | ||
| return Err(RoutingError::NoPath { | ||
| from: from.original, | ||
| to: to.original, | ||
| }); |
There was a problem hiding this comment.
This early NoPath check breaks common two-way roads modeled as overlapping 0->1 and 1->0 edges. snap_to_edge is geometry-only, and SegmentIndex::nearest_segment keeps the first equally-close segment, so both points on that road can snap to the same 0->1 edge. A trip from 80% back to 20% along the road then fails here, even though the reverse-direction sibling edge provides a valid path; I reproduced that locally with edges (0,1) and (1,0), where both snaps landed on EdgeIdx(0) and route_edge_snapped returned NoPath.
Useful? React with 👍 / 👎.
1 participant
Summary
NoPathValidation
cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningscargo test edge_snapped --test integrationcargo testFixes #3