From 51bad71be1bf6d4af915c53ce1316d438a591b43 Mon Sep 17 00:00:00 2001 From: malice Date: Tue, 26 May 2026 23:49:55 -0600 Subject: [PATCH] navi-offroute: Auto classify-once / route-once (kill 4-mode contest) (#36) * navi-offroute: Auto classify-once / route-once (kill 4-mode contest) PR 1 of the Auto rewrite. _route_auto no longer routes all four modes and keeps a min-time winner; it classifies each endpoint (category map, spatial probe only as untagged-click fallback), picks the first AUTO_MODE_PRIORITY mode in the eligible intersection, and routes ONCE. This removes the measured ~3.03s in-town 4-mode probe (single-mode probing line in journald) -- in-town Auto drops from ~6s toward ~1s. Trade-off (intentional, PR1): no routing-failure fall-through and no min-time refinement -- if the capability-picked mode cannot route, the error is returned. The hybrid path (unchanged here, still gated on the 24km MIN_HYBRID_DISTANCE_KM) recovers road->offroad plans. Semantic hybrid gate is PR 2. Scope: router.py (_route_auto contest loop only) + test_offroute.py (contest tests -> capability-pick tests + new tagged-no-spatial and untagged-spatial-once tests). _try_hybrid_auto body, the hybrid gate, AUTO_MODE_PRIORITY/MODE_PROFILES, and all other modules untouched. Full offroute suite: 84 passed. Design: recon_refactor/auto-rewrite-plan.md (artifacts dir). Co-Authored-By: Claude Opus 4.7 (1M context) * Auto: foot-as-last-resort fallback when picked mode fails to route If the capability-picked mode cannot route, retry foot ONCE (foot always routes modulo bbox limits) instead of surfacing a wall to the user. On success, ship the foot route tagged with auto_fallback_from= for the UI; on foot failure, return the original error. No fallback when the picked mode is already foot. selected_mode_set still reflects the original capability intersection. Tests: no_fallthrough -> falls_back_to_foot_on_error; + both-fail returns error; + no-fallback-when-picked-is-foot. Full offroute suite: 86 passed. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Matt Co-authored-by: Claude Opus 4.7 (1M context) --- backend/services/navi_offroute/router.py | 56 ++++++--- .../navi_offroute/tests/test_offroute.py | 118 +++++++++++------- 2 files changed, 113 insertions(+), 61 deletions(-) diff --git a/backend/services/navi_offroute/router.py b/backend/services/navi_offroute/router.py index b04030a..68543e8 100755 --- a/backend/services/navi_offroute/router.py +++ b/backend/services/navi_offroute/router.py @@ -857,31 +857,53 @@ class OffrouteRouter: priority = [m for m in AUTO_MODE_PRIORITY if m in intersection] - # Probe every eligible candidate and pick the one that minimises total trip - # time, NOT the first that succeeds. When the start is in wilderness and the - # end is on-road, falling through to foot would compute the whole network leg - # at foot pace. The wilderness leg always uses foot regardless; the candidate - # only changes the network leg. + # Classify-once / route-once: the eligible-mode sets above already identify the + # fastest mode both endpoints can traverse -- the first in AUTO_MODE_PRIORITY + # that survives the intersection. Pick it and route a SINGLE time, instead of + # routing all four candidates and keeping a min-time winner (the old 4-mode + # contest cost ~3s on in-town trips). No routing-failure fall-through: if the + # picked mode cannot route, the error is returned. See auto-rewrite-plan.md. + mode = priority[0] if priority else "foot" best_result = None best_minutes = None last_error = None _probe_t0 = time.perf_counter() - for candidate in priority: - result = self.route( + result = self.route( + start_lat, start_lon, end_lat, end_lon, + mode=mode, boundary_mode=boundary_mode, annotate_mvum=False + ) + if result.get("status") == "ok": + best_result = result + best_result["selected_mode"] = mode + best_minutes = (result.get("summary") or {}).get( + "total_effort_minutes", float("inf")) + else: + last_error = result + logger.info("auto: classified mode=%s, routed once in %.2fs", + mode, time.perf_counter() - _probe_t0) + + # Foot-as-last-resort: foot always routes (modulo bbox limits), so if the + # capability-picked mode failed, fall back to foot ONCE rather than surface a + # wall to the user. selected_mode_set still reflects the original eligibility. + if result.get("status") != "ok" and mode != "foot": + _foot_t0 = time.perf_counter() + foot_result = self.route( start_lat, start_lon, end_lat, end_lon, - mode=candidate, boundary_mode=boundary_mode, annotate_mvum=False + mode="foot", boundary_mode=boundary_mode, annotate_mvum=False ) - if result.get("status") == "ok": - minutes = (result.get("summary") or {}).get( + if foot_result.get("status") == "ok": + best_result = foot_result + best_result["selected_mode"] = "foot" + best_result["auto_fallback_from"] = mode # surface to client/UI + best_minutes = (foot_result.get("summary") or {}).get( "total_effort_minutes", float("inf")) - if best_minutes is None or minutes < best_minutes: - best_minutes = minutes - best_result = result - best_result["selected_mode"] = candidate + last_error = None + logger.info("auto: %s failed, foot fallback succeeded in %.2fs", + mode, time.perf_counter() - _foot_t0) else: - last_error = result - logger.info("single-mode probing took %.2fs (%d candidate modes)", - time.perf_counter() - _probe_t0, len(priority)) + # foot also failed -- keep the original last_error (return original error) + logger.info("auto: %s failed, foot fallback also failed in %.2fs", + mode, time.perf_counter() - _foot_t0) if best_result is not None: # MVUM Layer 3a: a "drive to a trailhead, switch, continue offroad" plan may diff --git a/backend/services/navi_offroute/tests/test_offroute.py b/backend/services/navi_offroute/tests/test_offroute.py index 03089c8..a37d106 100644 --- a/backend/services/navi_offroute/tests/test_offroute.py +++ b/backend/services/navi_offroute/tests/test_offroute.py @@ -312,9 +312,9 @@ def _typed_all(monkeypatch): lambda self, cat: ALL_MODES) -def test_route_auto_equal_times_keep_priority(monkeypatch): - # All candidates succeed with no recorded time -> tie -> highest priority wins, - # and every candidate is probed (no early return under min-time selection). +def test_route_auto_picks_capability_mode(monkeypatch): + # Classify-once: typed road endpoints -> intersection = all modes -> the first + # AUTO_MODE_PRIORITY mode (vehicle) is picked and routed ONCE (no 4-mode contest). _typed_all(monkeypatch) calls = [] monkeypatch.setattr(OffrouteRouter, "route", _stub_route(_all_ok(), calls)) @@ -323,27 +323,29 @@ def test_route_auto_equal_times_keep_priority(monkeypatch): assert out["status"] == "ok" assert out["selected_mode"] == "vehicle" assert out["selected_mode_set"] == sorted(ALL_MODES) - assert calls == ["vehicle", "4w", "2w", "foot"] + assert calls == ["vehicle"] # ONE route call, not four -def test_route_auto_falls_through_to_foot(monkeypatch): +def test_route_auto_falls_back_to_foot_on_error(monkeypatch): + # Foot-as-last-resort: the capability-picked mode (vehicle) fails, so Auto retries + # foot ONCE and ships it, tagging auto_fallback_from for the UI. _typed_all(monkeypatch) calls = [] - per_mode = { - "vehicle": {"status": "error", "message": "No roads found"}, - "4w": {"status": "error", "message": "No tracks found"}, - "2w": {"status": "error", "message": "No tracks found"}, - "foot": {"status": "ok"}, - } + per_mode = {m: {"status": "error", "message": f"{m} failed"} for m in AUTO_MODE_PRIORITY} + per_mode["foot"] = {"status": "ok"} monkeypatch.setattr(OffrouteRouter, "route", _stub_route(per_mode, calls)) r = object.__new__(OffrouteRouter) out = r._route_auto(42.0, -114.0, 42.5, -114.5, "pragmatic") assert out["status"] == "ok" assert out["selected_mode"] == "foot" - assert calls == ["vehicle", "4w", "2w", "foot"] + assert out["auto_fallback_from"] == "vehicle" + assert out["selected_mode_set"] == sorted(ALL_MODES) # original eligibility, not foot-only + assert calls == ["vehicle", "foot"] -def test_route_auto_all_error_returns_error(monkeypatch): +def test_route_auto_returns_error_when_picked_and_foot_both_fail(monkeypatch): + # Picked mode AND the foot fallback both fail -> original error surfaces, exactly + # two attempts (picked, then foot). _typed_all(monkeypatch) calls = [] per_mode = {m: {"status": "error", "message": f"{m} failed"} for m in AUTO_MODE_PRIORITY} @@ -353,23 +355,52 @@ def test_route_auto_all_error_returns_error(monkeypatch): assert out["status"] == "error" assert "selected_mode" not in out assert out["selected_mode_set"] == sorted(ALL_MODES) - assert calls == ["vehicle", "4w", "2w", "foot"] + assert calls == ["vehicle", "foot"] -def test_route_auto_selected_mode_present_in_ok_response(monkeypatch): - _typed_all(monkeypatch) +def test_route_auto_no_fallback_when_picked_is_foot(monkeypatch): + # When the picked mode is already foot (foot-only intersection), there is no second + # attempt -- foot cannot fall back to itself. calls = [] - per_mode = { - "vehicle": {"status": "error", "message": "No roads found"}, - "4w": {"status": "ok", "summary": {"total_effort_minutes": 90.0}}, - "2w": {"status": "ok", "summary": {"total_effort_minutes": 40.0}}, - "foot": {"status": "ok", "summary": {"total_effort_minutes": 600.0}}, - } + per_mode = {"foot": {"status": "error", "message": "foot failed"}} monkeypatch.setattr(OffrouteRouter, "route", _stub_route(per_mode, calls)) r = object.__new__(OffrouteRouter) - out = r._route_auto(42.0, -114.0, 42.5, -114.5, "pragmatic") - assert out["status"] == "ok" - assert out["selected_mode"] == "2w" # fastest success wins + out = r._route_auto(42.0, -114.0, 42.5, -114.5, "pragmatic", + start_category="building:house", end_category="natural:peak") # -> {foot} + assert out["status"] == "error" + assert out["selected_mode_set"] == ["foot"] + assert calls == ["foot"] # no fallback attempt + + +def test_route_auto_tagged_road_to_road_no_spatial_probe(monkeypatch): + # Tagged road endpoints -> pure category classification, the spatial probe must + # NOT fire, and exactly one route call (mode=vehicle) is made. + calls = [] + spatial_calls = [] + monkeypatch.setattr(OffrouteRouter, "route", _stub_route(_all_ok(), calls)) + monkeypatch.setattr(OffrouteRouter, "_spatial_eligible_modes", + lambda self, lat, lon, sc: spatial_calls.append((lat, lon)) or ALL_MODES) + r = object.__new__(OffrouteRouter) + out = r._route_auto(42.0, -114.0, 42.5, -114.5, "pragmatic", + start_category="building:house", end_category="highway:residential") + assert out["selected_mode"] == "vehicle" + assert calls == ["vehicle"] + assert spatial_calls == [] # no spatial probe for tagged endpoints + + +def test_route_auto_untagged_spatial_called_once_per_endpoint(monkeypatch): + # One untagged endpoint -> _spatial_eligible_modes fires exactly once (for that + # endpoint only); the tagged endpoint stays a dict lookup. + calls = [] + spatial_calls = [] + monkeypatch.setattr(OffrouteRouter, "route", _stub_route(_all_ok(), calls)) + monkeypatch.setattr(OffrouteRouter, "_spatial_eligible_modes", + lambda self, lat, lon, sc: spatial_calls.append((lat, lon)) or ALL_MODES) + r = object.__new__(OffrouteRouter) + out = r._route_auto(42.0, -114.0, 42.5, -114.5, "pragmatic", + start_category="building:house", end_category=None) # end untagged + assert spatial_calls == [(42.5, -114.5)] # exactly once, for the untagged end + assert calls == ["vehicle"] # ── _route_auto with category type hints (real _eligible_modes_from_category) ── @@ -381,7 +412,7 @@ def test_route_auto_address_to_address_picks_vehicle(monkeypatch): out = r._route_auto(42.0, -114.0, 42.5, -114.5, "pragmatic", start_category="building:house", end_category="highway:residential") assert out["selected_mode"] == "vehicle" - assert calls == ["vehicle", "4w", "2w", "foot"] + assert calls == ["vehicle"] def test_route_auto_address_to_trailhead_picks_atv(monkeypatch): @@ -391,7 +422,7 @@ def test_route_auto_address_to_trailhead_picks_atv(monkeypatch): out = r._route_auto(42.0, -114.0, 42.5, -114.5, "pragmatic", start_category="building:house", end_category="highway:trailhead") assert out["selected_mode"] == "4w" - assert "vehicle" not in calls + assert calls == ["4w"] # capability pick, single call assert out["selected_mode_set"] == sorted({"4w", "2w", "foot"}) @@ -420,6 +451,7 @@ def test_route_auto_both_unknown_uses_spatial_fallback(monkeypatch): out = r._route_auto(42.0, -114.0, 42.5, -114.5, "pragmatic") # no categories assert len(spatial_calls) == 2 # both endpoints resolved spatially assert out["selected_mode"] == "vehicle" + assert calls == ["vehicle"] # one route after classification # ── _spatial_eligible_modes — Valhalla classification.classification + .use ── @@ -827,10 +859,13 @@ def test_pathfind_wilderness_bbox_pad_is_1_5km(monkeypatch): assert abs((bounds["east"] - (-115.0)) - 0.015) < 1e-9 -# ── Auto min-time selection + per-leg breakdown (feat/auto-picks-fastest) ── +# ── Auto classify-once priority pick (replaces the old min-time contest) ── -def test_route_auto_picks_min_time(monkeypatch): - # vehicle is first in AUTO_MODE_PRIORITY but slow; 4w is fastest -> 4w wins. +def test_route_auto_picks_priority_not_min_time(monkeypatch): + # Classify-once picks the first AUTO_MODE_PRIORITY mode in the intersection + # (vehicle) and routes ONCE -- it no longer probes all modes to find a faster one. + # (The old min-time contest, where a slower-priority mode could win, is gone -- a + # documented PR1 trade-off in auto-rewrite-plan.md.) _typed_all(monkeypatch) calls = [] per_mode = { @@ -843,8 +878,8 @@ def test_route_auto_picks_min_time(monkeypatch): r = object.__new__(OffrouteRouter) out = r._route_auto(42.0, -114.0, 42.5, -114.5, "pragmatic") assert out["status"] == "ok" - assert out["selected_mode"] == "4w" # fastest, not first-priority - assert calls == ["vehicle", "4w", "2w", "foot"] # probed every candidate + assert out["selected_mode"] == "vehicle" # first priority, picked by capability + assert calls == ["vehicle"] # routed once, no contest def test_route_auto_per_leg_breakdown(): @@ -865,25 +900,20 @@ def test_route_auto_per_leg_breakdown(): assert abs((summ["wilderness_minutes"] + summ["network_minutes"]) - summ["total_effort_minutes"]) < 1e-6 -def test_route_auto_annotates_only_winner(monkeypatch): - # 4 candidates probed (annotate_mvum=False each); _annotate_network_segments must be - # called exactly once, on the min-time winner. +def test_route_auto_annotates_picked_mode_once(monkeypatch): + # Classify-once routes the picked mode with annotate_mvum=False, then + # _annotate_network_segments runs exactly once, on that picked mode (vehicle). _typed_all(monkeypatch) calls = [] - per_mode = { - "vehicle": {"status": "ok", "summary": {"total_effort_minutes": 100.0}}, - "4w": {"status": "ok", "summary": {"total_effort_minutes": 40.0}}, # fastest - "2w": {"status": "ok", "summary": {"total_effort_minutes": 80.0}}, - "foot": {"status": "ok", "summary": {"total_effort_minutes": 500.0}}, - } - monkeypatch.setattr(OffrouteRouter, "route", _stub_route(per_mode, calls)) + monkeypatch.setattr(OffrouteRouter, "route", _stub_route(_all_ok(), calls)) annotated = [] monkeypatch.setattr(OffrouteRouter, "_annotate_network_segments", lambda self, result, mode: annotated.append(mode)) r = object.__new__(OffrouteRouter) out = r._route_auto(42.0, -114.0, 42.5, -114.5, "pragmatic") - assert out["selected_mode"] == "4w" - assert annotated == ["4w"] # annotated once, on the winner only + assert out["selected_mode"] == "vehicle" + assert calls == ["vehicle"] + assert annotated == ["vehicle"] # annotated once, on the picked mode # ── Layer 3b: parking as a hybrid transition candidate source ──