fix(map): recover missing route shapes - #608
Conversation
Retry a bounded set of boardable trip shapes and fall back to route-level geometry so one bad trip cannot hide an active route. Cache results to avoid request amplification during arrival polling.\n\nCloses OneBusAway#607
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change filters departed arrivals, exposes ordered trip candidates, and improves route shape rendering. Shape requests now use caching, alternate trip attempts, and route-level fallback segments. Rendering and teardown handle multiple polylines per route. ChangesRoute shape resilience
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to The change improves route recovery and is mergeable, but a route whose trip and fallback geometry all fail may remain missing for the current selection rather than retrying automatically after the failure-cache window. Sequence Diagram(s)sequenceDiagram
participant StopRoutesLayer
participant TransitAPI
participant ShapeCache
participant MapProvider
StopRoutesLayer->>ShapeCache: request cached trip shape
ShapeCache->>TransitAPI: fetch trip details and shape points
TransitAPI-->>ShapeCache: return shape or failure
StopRoutesLayer->>TransitAPI: fetch route fallback after trip failures
TransitAPI-->>StopRoutesLayer: return fallback segments
StopRoutesLayer->>MapProvider: create and reveal route polylines
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
aaronbrethorst
left a comment
There was a problem hiding this comment.
This is a careful piece of work on a genuinely awkward problem. The escalation — try up to three boardable trips in arrival order, then fall back to route-level geometry, rather than dropping the route off the map — is the right shape, and I like that the fallback is honest about being degraded rather than pretending it's the real trip shape.
The parts I checked hardest and found sound:
- The
polylinesByRouteIdsingle-to-array conversion is complete.teardown(),reassertLinePaintOrder(), the promotion effect, anddrawRoutesall handle the array form, and both providers already acceptrevealPolylines({only})with multiple polylines and per-polylineremovePolyline/setPolylineLayer, so nothing diverges between OSM and Google. polledRouteIdsis the right fix for the actual leak: vehicle polling starts for every active route including ones whose shape never resolves, so teardown has to snapshot the union rather than just the drawn keys.- The stale-token path inside the multi-segment loop correctly removes every segment the superseded route already attached, not just the one in flight.
filterDeparted(arrivals, now)matches its real signature, and gating onNumber.isFinite(now)keeps the no-options callers on the old unfiltered behavior.
One thing worth a follow-up, not a blocker. routeColors now derives from the departed-filtered boardableRoutes, but StopPane seeds its rows from the unfiltered entry on first render and only applies filterDeparted on subsequent polls. So a route whose only cold-load arrival has already departed renders a row with no assigned color and falls back to route?.color until the first poll corrects it. Cosmetic and self-healing, and the comment above routeColors about two consumers with different staleness semantics is now carrying a third axis it doesn't mention — worth a line when you're next in there.
Merging.
Summary
Testing
Closes #607
Summary by CodeRabbit