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

[bmander]

Problem

Fixes the "plain bug" in #1947: the trip planner shows a "Try walking instead" advisory (and no route) even for trips far too long to actually walk.

Root cause — a client ordering bug, not an OTP bug

When the app sends a transit request, OTP2 also computes a direct WALK itinerary — the planConnection.modes default is documented as "all transit modes are usable and WALK is used for direct street suggestions." When that walk's generalized cost beats every transit option, OTP's filter chain (RemoveTransitIfWalkingIsBetter → RoutingErrorsAttacher) deletes the transit itineraries, attaches WALKING_BETTER_THAN_TRANSIT, and keeps the walk-only itinerary in planConnection.edges.

Two notes on the "you clearly can't walk" reports:

• Generalized cost folds in wait/transfer/boarding penalties, not just distance, and OTP's direct-walk limit defaults to hours — so an hour-plus walk can "beat" a bus that requires a wait.
• The only case with genuinely nothing to show is the degenerate SameEdgeAdjuster path (origin == destination), which returns no itineraries.

Otp2Planner.plan checked routingErrors before the itineraries, so it threw the advisory and discarded the valid walk route in the same payload.

Fix

Extract resolveOtp2Plan(), which makes itineraries win over routing errors: return any itineraries present, and only classify a routing error when edges is empty. This shows OTP's walk route as a normal result. It's safe for every fatal code (LOCATION_NOT_FOUND, OUTSIDE_BOUNDS, NO_TRANSIT_CONNECTION, the same-location advisory) because those always arrive with empty edges, so they still classify exactly as before.

Tests

New Otp2PlanResolveTest (4 cases): walk-itinerary-wins (the regression), empty-edges advisory, fatal error still surfaces, and no-route. Compiles clean under -PwarningsAsErrors=true; new + existing decode/error-mapping tests pass.

Scope

This is the "defend against the bug" half of #1947. The issue also asks to eventually drop the advisory string entirely and proactively request walk-only itineraries for the genuinely-empty case — left as follow-on. The advisory now only appears in the degenerate origin == destination case.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Trip planning now returns available itineraries even when advisory routing warnings are also reported.
  • Improved handling of routing failures, including clearer classification of location-related errors and no-route results.

• Tests

  • Added coverage for successful itineraries with warnings, routing errors without itineraries, and no-route scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1bbc717d-828f-4b6d-8803-37d15073b1d9

📥 Commits

Reviewing files that changed from the base of the PR and between d1926dd and cca5ff3.

📒 Files selected for processing (2)

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

---

📝 Walkthrough

Walkthrough

OTP2 planning now prioritizes returned itineraries over routing errors. Response resolution is extracted into a helper, with tests covering walk itineraries, advisory and fatal errors, and no-route responses.

Changes

OTP2 plan resolution

Layer / File(s)	Summary
Resolve itineraries before routing errors onebusaway-android/src/main/java/org/onebusaway/android/ui/tripplan/Otp2Planner.kt	plan() delegates to resolveOtp2Plan, which returns non-empty itineraries before classifying routing errors and otherwise throws the appropriate planning error.
Validate OTP2 resolution outcomes onebusaway-android/src/test/java/org/onebusaway/android/ui/tripplan/Otp2PlanResolveTest.kt	Tests cover surviving walk itineraries, advisory errors, classified location errors, no-route behavior, and Apollo-style response fixtures.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• Trip planner should never advise "try walking instead" — return a walk route instead #1947 — Addresses returning an available walk itinerary instead of surfacing WALKING_BETTER_THAN_TRANSIT as an advisory error.

Possibly related PRs

• OneBusAway/onebusaway-android#1794 — Introduced the OTP2 GraphQL trip-planning support that this response handling builds on.
• OneBusAway/onebusaway-android#1942 — Added the planning error classification used by the new resolver.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: prioritizing a valid OTP2 walking itinerary over the walking advisory.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/1947-walk-better-than-transit

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}