From e36cc204c5dd54fc823eeebe5c4a3b43039b8975 Mon Sep 17 00:00:00 2001 From: malice Date: Wed, 27 May 2026 23:13:51 -0600 Subject: [PATCH] =?UTF-8?q?navi-offroute:=20Auto=20Valhalla=20bypass=20?= =?UTF-8?q?=E2=80=94=20fire=20on=20untagged=20road=20clicks=20too=20(PR=20?= =?UTF-8?q?47)=20(#47)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: mj Co-authored-by: Claude Opus 4.7 (1M context) --- backend/services/navi_offroute/router.py | 39 +++++++++---- .../navi_offroute/tests/test_offroute.py | 56 +++++++++++++++++++ 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/backend/services/navi_offroute/router.py b/backend/services/navi_offroute/router.py index ba89b15..5c916ca 100755 --- a/backend/services/navi_offroute/router.py +++ b/backend/services/navi_offroute/router.py @@ -754,6 +754,26 @@ class OffrouteRouter: return CATEGORY_ELIGIBLE_MODES.get(f"{key}:*") return None + def _bypass_eligible(self, lat: float, lon: float, + category: Optional[str], snap_cache: dict) -> bool: + """True when this endpoint is clearly ON a paved road. Gates the Valhalla bypass + in _route_auto. Two safe paths: + 1. positive OSM category hint (CATEGORY_ELIGIBLE_MODES grants "vehicle"), OR + 2. cached auto-snap is tight (<= AUTO_SNAP_TIGHT_M) AND the snapped road is a + paved highway class. + Deliberately EXCLUDES the relaxed-snap-with-flat case (d_auto <= 100m + flat + terrain), which can grant 'vehicle' eligibility up to 100m off a road — bypassing + there would silently re-snap a wilderness/park-lot click to the road and lose any + walk-then-drive leg the unified flow would have produced.""" + typed = self._eligible_modes_from_category(category) + if typed is not None: + return "vehicle" in typed + snap = snap_cache.get((lat, lon, "auto")) + if snap is None: + return False # tagged-endpoint short-circuit didn't pre-populate; safe default + return (snap["snap_distance_m"] <= AUTO_SNAP_TIGHT_M + and snap.get("road_class") in PAVED_HIGHWAY_CLASSES) + def _is_terrain_flat(self, lat: float, lon: float) -> bool: """True if the DEM is flat (max-min < FLAT_TERRAIN_DELTA_M) across the center and four cardinal points FLAT_SAMPLE_RADIUS_M away. Conservative: any DEM read @@ -839,16 +859,15 @@ class OffrouteRouter: seed_set = sorted(start_eligible | end_eligible) # === Valhalla bypass: pure road↔road skips the raster pipeline entirely === - # When BOTH endpoints are tagged with a vehicle-eligible category (only _MODES_ALL - # contains "vehicle"), the trip is on-road end-to-end -> hand it straight to the same - # inline-Valhalla path explicit vehicle requests use, saving the ~7.5s raster build + - # unified A*. Untagged endpoints (category -> None) and any non-paved category fall - # through to the unified flow. Boundary-mode MVUM exclusions are not applied here (urban - # roads aren't MVUM-gated); this matches a pragmatic in-town vehicle route. - bp_start = self._eligible_modes_from_category(start_category) - bp_end = self._eligible_modes_from_category(end_category) - if (bp_start is not None and bp_end is not None - and "vehicle" in bp_start and "vehicle" in bp_end): + # When BOTH endpoints are clearly ON a paved road -- by vehicle-eligible category hint + # OR a tight (<=5m) auto-snap to a paved highway (_bypass_eligible) -- the trip is + # on-road end-to-end, so hand it to the same inline-Valhalla path explicit vehicle + # requests use, saving the ~7.5s raster build + unified A*. Untagged raw-coordinate + # clicks now qualify via the tight-snap path; the relaxed (<=100m) snap is excluded so + # an off-road click isn't silently re-snapped to the road. Boundary-mode MVUM + # exclusions aren't applied here (urban roads aren't MVUM-gated). + if (self._bypass_eligible(start_lat, start_lon, start_category, snap_cache) + and self._bypass_eligible(end_lat, end_lon, end_category, snap_cache)): _bt0 = time.perf_counter() bypass = self._route_D_network_only(start_lat, start_lon, end_lat, end_lon, "vehicle") if bypass.get("status") == "ok": diff --git a/backend/services/navi_offroute/tests/test_offroute.py b/backend/services/navi_offroute/tests/test_offroute.py index 0939f4b..3881161 100644 --- a/backend/services/navi_offroute/tests/test_offroute.py +++ b/backend/services/navi_offroute/tests/test_offroute.py @@ -1372,3 +1372,59 @@ def test_route_auto_bypass_falls_through_on_valhalla_error(monkeypatch, caplog): assert out["status"] == "error" assert "Failed to load terrain" in out["message"] # fell through to the unified flow assert "auto_bypass" not in (out.get("summary") or {}) + + +# ── PR 47: bypass fires on untagged road clicks (tight snap), not relaxed/off-road ── + +def _spatial_tight_paved(self, lat, lon, snap_cache): + """Stub _spatial_eligible_modes: tight (3m) snap to a paved residential road, like a + raw in-town map click ON the road. Populates snap_cache the way the real probe does.""" + snap_cache[(lat, lon, "auto")] = {"snap_distance_m": 3.0, "road_class": "residential", + "use": "road", "on_network": True, + "snapped_lat": lat, "snapped_lon": lon} + return frozenset({"vehicle", "4w", "2w", "foot"}) + + +def _spatial_relaxed_paved(self, lat, lon, snap_cache): + """Stub: relaxed (50m) snap to a paved road + 'vehicle' eligibility (the flat-terrain + grace). 50m > AUTO_SNAP_TIGHT_M, so _bypass_eligible must reject this.""" + snap_cache[(lat, lon, "auto")] = {"snap_distance_m": 50.0, "road_class": "residential", + "use": "road", "on_network": True, + "snapped_lat": lat, "snapped_lon": lon} + return frozenset({"vehicle", "4w", "2w", "foot"}) + + +def test_route_auto_untagged_tight_snap_fires_bypass(monkeypatch): + r = object.__new__(OffrouteRouter) + monkeypatch.setattr(OffrouteRouter, "_spatial_eligible_modes", _spatial_tight_paved) + monkeypatch.setattr(OffrouteRouter, "_fetch_auto_rasters", _bp_fetch_should_not_run) + monkeypatch.setattr(_p4router.requests, "post", lambda *a, **k: _BypassOKResp()) + out = r._route_auto(42.5558, -114.4701, 42.5644, -114.4631, "pragmatic") # no categories + assert out["status"] == "ok" + assert out["selected_mode"] == "vehicle" + assert out["summary"]["auto_bypass"] is True + + +def test_route_auto_untagged_relaxed_snap_skips_bypass(monkeypatch): + r = object.__new__(OffrouteRouter) + monkeypatch.setattr(OffrouteRouter, "_spatial_eligible_modes", _spatial_relaxed_paved) + monkeypatch.setattr(OffrouteRouter, "_route_D_network_only", + lambda *a, **k: (_ for _ in ()).throw(AssertionError("D must not run"))) + monkeypatch.setattr(OffrouteRouter, "_fetch_auto_rasters", _bp_fetch_sentinel) + out = r._route_auto(42.5558, -114.4701, 42.5644, -114.4631, "pragmatic") # no categories + assert out["status"] == "error" + assert "Failed to load terrain" in out["message"] # relaxed snap rejected -> unified flow + + +def test_route_auto_e2e_http_in_town_fires_bypass(client, monkeypatch): + # Full HTTP path through /api/offroute with the PRODUCTION request shape: NO categories. + # This is the test that would have caught PR #46's wiring gap. + monkeypatch.setattr(OffrouteRouter, "_spatial_eligible_modes", _spatial_tight_paved) + monkeypatch.setattr(OffrouteRouter, "_fetch_auto_rasters", _bp_fetch_should_not_run) + monkeypatch.setattr(_p4router.requests, "post", lambda *a, **k: _BypassOKResp()) + resp = _post(client, {"start": [42.5558, -114.4701], "end": [42.5644, -114.4631], + "mode": "auto", "boundary_mode": "pragmatic"}) + assert resp.status_code == 200 + data = resp.get_json() + assert data["selected_mode"] == "vehicle" + assert data["summary"]["auto_bypass"] is True