Show OTP2 walk route instead of a "try walking instead" advisory (#1947)

onebusaway-android/src/main/java/org/onebusaway/android/ui/tripplan/Otp2Planner.kt

@@ -95,16 +95,42 @@ class Otp2Planner @Inject constructor(
             throw TripPlanException(TripPlanError.Unknown, e)
         }
 
-        data.planConnection?.routingErrors?.firstOrNull()?.let {
-            throw TripPlanException(otp2ErrorFor(it.code, it.inputField))
-        }
+        return resolveOtp2Plan(data)
+    }
+}
 
-        val itineraries = data.toTripItineraries()
-        if (itineraries.isEmpty()) {
-            throw TripPlanException(TripPlanError.NoRoute)
-        }
+/**
+ * Resolves an OTP2 `planConnection` [PlanQuery.Data] into itineraries, or throws a classified
+ * [TripPlanException] when there is genuinely nothing to show.
+ *
+ * **Itineraries win over routing errors.** A `routingErrors` entry can accompany a perfectly good
+ * result, so we must return the result rather than surface the error. The motivating case (#1947) is
+ * [RoutingErrorCode.WALKING_BETTER_THAN_TRANSIT]: OTP2 always computes a direct WALK itinerary
+ * alongside transit — the `planConnection.modes` default is documented as "all transit modes are
+ * usable and WALK is used for direct street suggestions" — and when that walk's *generalized* cost
+ * (which folds in wait/transfer/boarding penalties, not just distance) beats every transit option,
+ * the filter chain deletes the *transit* itineraries and attaches this error while **keeping the
+ * walk-only itinerary in `edges`**. Surfacing the error there would throw away a valid walk route and
+ * show a "Try walking instead" advisory with no result — even for trips too long to actually walk.
+ * Returning whatever itineraries came back shows that walk route as a normal option instead.
+ *
+ * This is safe for every *fatal* code — `LOCATION_NOT_FOUND`, `OUTSIDE_BOUNDS`,
+ * `NO_TRANSIT_CONNECTION`, the same-location `WALKING_BETTER_THAN_TRANSIT` raised by OTP's
+ * `SameEdgeAdjuster`, … — because those always come back with empty `edges`, so consulting
+ * `routingErrors` only when there are no itineraries still classifies them exactly as before.
+ *
+ * Top-level and `internal` (no [Context] dependency) so it's JVM-unit-testable from a
+ * [PlanQuery.Data] fixture without Apollo, like [otp2ErrorFor].
+ */
+internal fun resolveOtp2Plan(data: PlanQuery.Data): List<TripItinerary> {
+    val itineraries = data.toTripItineraries()
+    if (itineraries.isNotEmpty()) {
         return itineraries
     }
+    data.planConnection?.routingErrors?.firstOrNull()?.let {
+        throw TripPlanException(otp2ErrorFor(it.code, it.inputField))
+    }
+    throw TripPlanException(TripPlanError.NoRoute)
 }
 
 /**

onebusaway-android/src/test/java/org/onebusaway/android/ui/tripplan/Otp2PlanResolveTest.kt

@@ -0,0 +1,144 @@
+/*
+ * Copyright (C) 2026 Open Transit Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onebusaway.android.ui.tripplan
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertThrows
+import org.junit.Test
+import org.onebusaway.android.R
+import org.onebusaway.android.api.graphql.PlanQuery
+import org.onebusaway.android.api.graphql.fragment.PlaceFields
+import org.onebusaway.android.api.graphql.type.InputField
+import org.onebusaway.android.api.graphql.type.Mode
+import org.onebusaway.android.api.graphql.type.RoutingErrorCode
+import org.onebusaway.android.directions.model.TripMode
+
+/**
+ * Covers [resolveOtp2Plan]: the rule that OTP2 itineraries win over a coexisting `routingErrors`
+ * entry, and that fatal errors (which always arrive with empty `edges`) still classify as before.
+ *
+ * The regression this guards is #1947: OTP2 emits [RoutingErrorCode.WALKING_BETTER_THAN_TRANSIT]
+ * *while keeping* the walk-only itinerary in the response, and the old code threw that advisory
+ * before ever reading the itinerary — hiding a valid walk route behind a "Try walking instead"
+ * message. Pure JVM: builds Apollo-generated [PlanQuery.Data] fixtures directly, no Apollo/HTTP.
+ */
+class Otp2PlanResolveTest {
+
+    /**
+     * The core fix: a `WALKING_BETTER_THAN_TRANSIT` error alongside a surviving walk itinerary must
+     * yield the walk itinerary, not throw the advisory.
+     */
+    @Test
+    fun walkingBetterThanTransit_withWalkItinerary_returnsTheWalk() {
+        val data = planData(
+            routingErrors = listOf(routingError(RoutingErrorCode.WALKING_BETTER_THAN_TRANSIT)),
+            edges = listOf(walkEdge()),
+        )
+
+        val itineraries = resolveOtp2Plan(data)
+
+        assertEquals(1, itineraries.size)
+        assertEquals(TripMode.WALK, itineraries[0].legs[0].mode)
+    }
+
+    /**
+     * The same-location degenerate case (OTP's `SameEdgeAdjuster`) emits the same code but with no
+     * itineraries — with nothing to show, the advisory still surfaces.
+     */
+    @Test
+    fun walkingBetterThanTransit_withoutItineraries_throwsAdvisory() {
+        val data = planData(
+            routingErrors = listOf(routingError(RoutingErrorCode.WALKING_BETTER_THAN_TRANSIT)),
+            edges = emptyList(),
+        )
+
+        val error = assertThrows(TripPlanException::class.java) { resolveOtp2Plan(data) }.error
+        assertEquals(TripPlanError.Category.ADVISORY, error.category)
+        assertEquals(R.string.tripplanner_error_walking_better_than_transit, error.detailRes)
+    }
+
+    /** A fatal error (always empty `edges`) still classifies exactly as before the fix. */
+    @Test
+    fun fatalError_withoutItineraries_throwsClassifiedError() {
+        val data = planData(
+            routingErrors = listOf(routingError(RoutingErrorCode.LOCATION_NOT_FOUND, InputField.FROM)),
+            edges = emptyList(),
+        )
+
+        val error = assertThrows(TripPlanException::class.java) { resolveOtp2Plan(data) }.error
+        assertEquals(TripPlanError.Category.LOCATION, error.category)
+        assertEquals(R.string.tripplanner_error_geocode_from_not_found, error.detailRes)
+    }
+
+    /** No itineraries and no error is a plain no-route result. */
+    @Test
+    fun noItinerariesNoError_throwsNoRoute() {
+        val data = planData(routingErrors = emptyList(), edges = emptyList())
+
+        val error = assertThrows(TripPlanException::class.java) { resolveOtp2Plan(data) }.error
+        assertEquals(TripPlanError.NoRoute, error)
+    }
+
+    // ---- fixtures ----
+
+    private fun planData(
+        routingErrors: List<PlanQuery.RoutingError>,
+        edges: List<PlanQuery.Edge>,
+    ) = PlanQuery.Data(
+        planConnection = PlanQuery.PlanConnection(
+            searchDateTime = "2026-07-11T10:00:00-07:00",
+            routingErrors = routingErrors,
+            edges = edges,
+        ),
+    )
+
+    private fun routingError(code: RoutingErrorCode, inputField: InputField? = null) =
+        PlanQuery.RoutingError(code = code, description = code.name, inputField = inputField)
+
+    private fun walkEdge(): PlanQuery.Edge {
+        val leg = PlanQuery.Leg(
+            mode = Mode.WALK,
+            duration = 2400.0,
+            distance = 3200.0,
+            realTime = null,
+            start = PlanQuery.Start(scheduledTime = "2026-07-11T10:00:00-07:00", estimated = null),
+            end = PlanQuery.End(scheduledTime = "2026-07-11T10:40:00-07:00", estimated = null),
+            from = from("Origin", 47.60, -122.30),
+            to = to("Destination", 47.62, -122.34),
+            route = null,
+            trip = null,
+            legGeometry = null,
+            steps = null,
+        )
+        val node = PlanQuery.Node(
+            start = "2026-07-11T10:00:00-07:00",
+            end = "2026-07-11T10:40:00-07:00",
+            duration = 2400L,
+            numberOfTransfers = 0,
+            legs = listOf(leg),
+        )
+        return PlanQuery.Edge(node = node)
+    }
+
+    private fun placeFields(name: String, lat: Double, lon: Double) =
+        PlaceFields(name, lat, lon, null, null, null, null)
+
+    private fun from(name: String, lat: Double, lon: Double) =
+        PlanQuery.From(__typename = "Place", placeFields = placeFields(name, lat, lon))
+
+    private fun to(name: String, lat: Double, lon: Double) =
+        PlanQuery.To(__typename = "Place", placeFields = placeFields(name, lat, lon))
+}