Skip to content

feat: Traffic-aware route forecast#181

Open
hir-al-14 wants to merge 26 commits into
benevolentbandwidth:mainfrom
hir-al-14:traffic_forecast
Open

feat: Traffic-aware route forecast#181
hir-al-14 wants to merge 26 commits into
benevolentbandwidth:mainfrom
hir-al-14:traffic_forecast

Conversation

@hir-al-14

@hir-al-14 hir-al-14 commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds traffic-aware forecasting after the weather-aware route solve, so the backend can estimate traffic delay before returning the final optimized route.
  • Adds forecast metadata for traffic impact, delay, provider, thresholds, and whether traffic caused a second route solve.

Motivation

  • Weather alone does not explain route timing well enough for real driver planning.
  • This adds the next basic piece of the forecast pipeline: use the optimized route legs, check expected traffic delay, and only rerun the route when the delay is large enough to matter.

Changes

  • Backend:

    • Added traffic forecast options from env, including enable flag, Google Maps base URL/API key, and reoptimization thresholds.
    • Added Google Maps traffic request helpers for route legs from the VROOM result.
    • Added traffic delay calculation and threshold logic.
    • Wired traffic after the weather-aware VROOM solve.
    • Reruns VROOM only when traffic delay crosses the configured threshold.
    • Adds traffic forecast metadata to the optimize response.
    • Fixed traffic leg departure timing so Google traffic requests use the correct route clock.
  • Infrastructure:

    • Passed traffic forecast env settings through Docker compose config.

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/mobile run lint
  • npm --prefix app/mobile run typecheck

Backend

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

Risk

  • Google Maps traffic calls depend on a valid API key and network access.
  • The current reroute model adds traffic delay into service time as a simple version. It does not build a traffic-aware matrix.

Rollout and Recovery

  • Roll out by enabling DELIVERYOPTIMIZER_TRAFFIC_FORECAST_ENABLED=1 and setting GOOGLE_MAPS_API_KEY.
  • The route solve falls back to the existing weather/VROOM behavior. (DELIVERYOPTIMIZER_TRAFFIC_FORECAST_ENABLED to 0)

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

Copy link
Copy Markdown
Collaborator

Hey Hiral — good progression from #180. The traffic layer is well-structured: the feature-flag defaults are safe, the env variable naming is consistent, the threshold logic mirrors the weather path cleanly, and the new unit tests cover the traffic path well (ReadsTrafficLegsFromVroomSteps, ReadsGoogleTrafficDelay, BuildsGoogleTrafficPath are all good additions). The OSRM entrypoint CRLF fix and Windows portability guards are also nice housekeeping.

A few things to work through before merge:

Must-fix

  1. ReadRouteTraffic serially blocks the event loop — new, critical. FetchTrafficDelay is called per leg from the event loop completion lambda, and each call blocks via future.wait_for(5s). A 10-stop route can stall the server for up to 50 seconds. This is the same class of bug as the weather future.wait_for() — it just compounds by stop count here. All three blocking callers (OpenWeather fetch, weather reoptimize runner->Run(), and now the traffic leg loop) need to move off the event loop thread — worker thread pool, drogon::async_run, or a chained async callback are all valid paths.

  2. ResolveTrafficForecastOptionsFromEnv() called per job in WorkerLoop. Same issue as ResolveWeatherForecastOptionsFromEnv() from feat: use route duration accurate for weather-aware optimization #180 — both are now resolved inside the per-job loop in optimization_job_runtime.cpp. Both should be resolved once at construction and stored on the runtime.

  3. Thunder double-count in DelayFromHourlyForecast — carryover from feat: use route duration accurate for weather-aware optimization #180, not fixed here. Thunderstorm hours (weather id 200–299) also include a rain object in the OpenWeather payload, so the function adds 240 + 90 = 330s instead of 240s.

Worth fixing

  1. ReadLegDeparture heuristic needs a comment. The check offset >= route_start_seconds - 24h is how the code distinguishes "VROOM produced absolute Unix timestamps" from "VROOM produced relative seconds." That's clever, but nothing tells the next reader it's intentional. A single comment line would prevent someone from "simplifying" it away. A test for the relative-offset path would also help.

  2. #undef GetJob needs an explanation. The undef in two files is clearly a Windows SDK macro conflict, but without a comment it reads as magic. One line (// Windows SDK defines GetJob as a macro; undef before including jsoncpp) is enough.

  3. Traffic delay spread uniformly across stops. Dividing total delay equally per job is a known simplification, but for routes with high variance in leg length it gives VROOM a misleading service-time vector. Even a rough proportional split by VROOM leg duration would be more accurate. Worth flagging as a known limitation in a comment if not changing it now.

  4. Redundant response fields — carryover from feat: use route duration accurate for weather-aware optimization #180: baseline_duration_seconds == baseline_route_duration_seconds always, and weather_adjusted_duration_seconds == predicted_duration_seconds always. These will confuse API consumers.

Frontend carryovers from #180

Location Issue
StopCard.tsx "Call" button uses onClick={onNavigate} — should open tel:${stop.phoneNumber}
StopCard.tsx Hardcoded "Deliver between 4:00pm - 5:00pm" — should use the stop's actual time window
upload-route/page.tsx JSX still uses className="ur-root" / "ur-content" but CSS was renamed to .upload-root / .upload-content — element is unstyled
upload-route/page.tsx const [isProcessing] = useState(false) — setter removed, state is dead; handleContinue has no error handling for a rejected file.text()
summary/page.tsx Warning button has no onClick
summary/page.tsx setTimeout(0) workaround doesn't reliably fix the read-after-navigate race
manifest.ts purpose: "maskable" on a full-bleed SVG — content reaches the canvas edges and will be clipped on Android

The event-loop threading fix is the one I'd want to see before this merges. Everything else can be sequenced, but the leg-loop blocking is a regression in server availability under real traffic conditions. Happy to talk through the async approach if that's helpful.

@markboenigk

Copy link
Copy Markdown
Collaborator

Good iteration — all three must-fixes from the last round are in and clean. The thunder double-count fix is especially nice, the else if reads clearly and the test locks it down. The threading refactor in FinishWithTraffic and the constructor-time env resolution are both right.

A few new things surfaced:

Worth fixing before merge

  1. Job ID / array-position mismatch in BuildTrafficAdjustedVroomInput (forecast_optimizer.cpp:955–958)
    ReadJobTravelSeconds fills travel_seconds[raw_job_id - 1], so delay allocation is indexed by VROOM job ID. BuildTrafficAdjustedVroomInput then writes traffic_delays[index] into payload["jobs"][index] by array position. These align only if input job IDs are 1-based and sequential. If they're not (non-contiguous IDs, or a partial job set), stop A gets stop B's traffic penalty — VROOM sees a wrong service-time vector and the reopt solve is silently wrong. Worth adding an ID→position map or documenting the assumption with an assertion.

  2. useSyncExternalStore no-op subscribe on summary/page.tsx (page.tsx:1110–1114)
    Replacing setTimeout(0) with useSyncExternalStore is the right idea, but subscribeToRouteStore = () => () => undefined means the component can't react if localStorage is written after mount. If the active-route page hasn't flushed before Summary mounts, readSavedRoute() returns null, the redirect fires, and the driver loses the summary screen with no recovery path. A beforeunload flush on the active-route side — or a storage event subscription here — would close this properly.

Lower priority

  1. FetchTrafficDelay timeout contract (forecast_optimizer.cpp:757–764)
    The promise wait_for(5s) path when not_ready is safe. The question is whether drogon guarantees the response callback is always called on timeout (so the promise is always resolved). If drogon can silently drop the callback, the future resolves only after the destructor — which is fine here since wait_for guards it, but worth verifying against drogon's docs before a production key is set.

Frontend carryovers (same as last time, sequencing is fine):
StopCard.tsx "Call" still routes via onNavigate instead of tel:, upload-route/page.tsx dead isProcessing setter + no error handling on file.text().

The job ID indexing one is the new must-look-at — everything else can be sequenced or documented. Happy to talk through the ID→position fix if useful.

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.

2 participants