Displaying Return To Start Path on Route Map

[KesarSidhu]

Summary

• Closes each route loop on the results map so the last stop connects back to the starting point, showing the drive home even though optimizer output omits the return leg.
• Applies to road-following polylines via Google DirectionsService (return segment follows roads, not a straight line).
• With a depot/start location, the route is depot → stops → depot; without a depot, the first stop acts as home.

Motivation

• Optimized routes end at the last delivery stop, but drivers need to return to the start/depot.
• The results map previously drew only the outbound path, which made routes look incomplete and understated total distance.
• Users expect the map to reflect the full round trip for planning and review.

Changes

Frontend (app/ui)

• Map.tsx — buildRoutePath
  • Build route points (depot + stops, or stops only when no depot).
  • Append the first point to the end of the path to close the loop.
  • All existing polyline consumers (directions fetch, cache key, straight-line fallback, pin-drag draft/committed paths) use this shared path builder, so the return leg is consistent everywhere.

Backend / mobile app / infra

• No backend, mobile app, or infrastructure changes in this PR.

Validation

Frontend

• npm --prefix app/ui run lint
• npm --prefix app/ui run format:check
• npm --prefix app/ui run typecheck
• npm --prefix app/ui run test
• npm --prefix app/ui run build
• npm --prefix app/ui run test:e2e
• npm --prefix app/mobile run lint
• npm --prefix app/mobile run typecheck

Backend

• cmake --preset dev
• .github/scripts/check-backend-static.sh build/dev
• cmake --build --preset dev --parallel
• ctest --preset dev --output-on-failure --no-tests=error -LE 'e2e|docker'
• docker compose -f deploy/compose/docker-compose.arm64.yml --env-file deploy/env/http-server.arm64.env config

Manual checks

1. Optimize → open /results: each route polyline returns to the depot/start along roads.
2. Routes without a depot: polyline returns to the first stop.
3. Multiple vehicles: each route closes its own loop independently.
4. Edit mode pin drag: draft and committed paths still include the return leg.
5. Route distance updates reflect the added return segment.

Risk

• Low–medium (map rendering): Single change in shared path builder; stop coordinates and solver output unchanged.
• Directions API: Return leg is included in the same directions request; may slightly increase API usage per route (one additional leg in the round trip).
• Waypoint limit: Routes with many stops may still hit the existing >25-waypoint straight-line fallback (unchanged behavior).
• Distance display: Reported route distance will increase to include the return leg (intentional).

Rollout and Recovery

• Frontend-only deploy of app/ui; no migrations or feature flags.
• Suggested base branch: saving-marker-location (this branch adds one commit on top).
• Roll back by reverting this PR; polylines will end at the last stop again.

High-Signal PR Checklist

• The summary states the user-visible or operational outcome, not just file names.
• The motivation explains why this change is needed now.
• The change list separates frontend, backend, infrastructure, and documentation work where applicable.
• Validation includes exact commands run, relevant output, and any checks intentionally skipped.
• Risks, migrations, feature flags, and rollback steps are called out when relevant.
• Screenshots or request/response examples are included for UI and API behavior changes.

[markboenigk]

Review

Nice fix for the underlying problem (routes visually ending mid-trip instead of showing the drive home) — the buildRoutePath loop-closing logic itself is correct and handles the depot/no-depot/single-stop cases well. Found one bug I'd consider blocking, plus a few smaller things.

🔴 Blocking: duplicate/orphaned polylines on load

Map.tsx — the two useEffects in RoutePolylinesOverlay now race on the same polylinesByVehicleRef.current object:

1. Effect 1 (~line 202) synchronously clears the ref, then kicks off an unawaited async loop that fetches Directions results sequentially per route.
2. Effect 2 (~line 328) runs synchronously right after, in the same commit. This branch is new in this PR: previously it did if (!poly) continue;; now if a route is missing from byVehicle it creates a new placeholder Polyline and stores it ("Effect 1 may have cleared refs before this runs; show a path immediately").
3. When effect 1's async loop later resolves that route's Directions call, it does polylinesByVehicleRef.current[vehicleId] = roadPoly, overwriting the ref entry without calling .setMap(null) on the placeholder effect 2 created.

The placeholder is still attached to the map and is now unreachable from any tracked state, so it's never cleaned up — not even by the next effect-1 cleanup pass, which only iterates whatever's currently in the ref. This isn't a rare interleaving; it reproduces on essentially every initial load of the results page with routes on it (mount → effect 1 clears+starts → effect 2 creates placeholders → each resolves and overwrites without disposing). Expect visible overlapping straight-line + road-line duplicates per route, plus a Polyline leak.

It's also more likely to trigger now that handleEditModeChange calls savePendingPinMove() on exit, since that changes stop geometry and re-triggers both effects together.

Suggested fix: before overwriting polylinesByVehicleRef.current[vehicleId] (in effect 1's cache/success/fallback branches, and in effect 2's "create new" branch), call .setMap(null) on any existing entry for that vehicle first.

🟡 Smaller things

• extractRoadPath: within a leg, non-first steps correctly drop their leading point (step.path.slice(1)), but the first step of each subsequent leg is pushed in full, duplicating the point at leg boundaries. Low impact since this only runs when overview_path is missing, but cheap to fix (track across legs, not just within one).
• Sequential Directions requests: reasonable rate-limit mitigation, but worth confirming render time was checked with realistic fleet sizes (10–20+ vehicles) — going from parallel to fully serial awaits multiplies load time by route count.
• clientValidationPending (page.tsx): module-level mutable flag gating hydration, shared across all ResultsPage instances. Follows the file's existing module-level caching pattern, but it's a timing gate rather than a memoized value — assumes exactly one instance is ever mounted. Consider scoping it to a ref/state inside the component instead.
• Scope: this PR also rewrites page.tsx's error-state derivation (useSyncExternalStore + RAF-gated hydration) and changes edit-mode-exit behavior (pending pin moves are now saved instead of discarded, which looks like a real, welcome fix but isn't mentioned in the description or covered by the validation checklist). Might be worth splitting out, or at minimum calling out explicitly and adding a manual check for it.
• No tests were added/updated for buildRoutePath's new loop-closing behavior or extractRoadPath's leg-stitching — both are pure functions and good unit-test candidates given this PR is specifically about geometry correctness.
• gh pr view currently shows this as CONFLICTING against main, and the description names saving-marker-location as the suggested base rather than main — worth confirming the intended base before merge.

Nit: page.tsx — savePendingPinMove() already calls setPendingPinMove(null) internally, so the explicit setPendingPinMove(null) right after it in handleEditModeChange is redundant (harmless, just noise).