feat(route): walk-to-destination mode with corridor + detour scoring#121
Merged
Conversation
Move metresBetween, midpoint, filterToEllipse, beamSearch (→ beamSearchOrienteering), and finalise (→ finaliseRoute) from scripts/plan_route.ts into the new pure helper module _routePlanner.ts. Add filterToCorridor (corridor / cross-track filter needed by the upcoming routePostboxes Cloud Function). Smoke test output unchanged (36 points, 5 stops). Six new mocha tests for filterToCorridor all passing (274 total). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove `visitedKey` from the exported `SearchState` type — it was an internal beam-search implementation detail that leaked into the public API. The dedup key is now computed inline inside the expansion loop. Also add a clarifying comment on the degenerate-segment fallback and tighten the half-width-0 test to `strictEqual` now that `pb_on`'s exact-zero perpendicular distance is confirmed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
New HTTPS callable that accepts a start/dest pair and corridor or detour
mode, queries nearby postboxes via a geohash prefix scan, strips already-
claimed-today boxes, applies filterToCorridor or filterToEllipse+
beamSearchOrienteering, and returns { count, points, directDistanceM,
budgetDistanceM, warnings } — no per-postbox IDs or coordinates leaked.
Input validated (auth, lat/lng ranges, mode-specific params, 30 km cap);
21 new tests added covering auth, all validation paths, corridor/detour
pure-logic happy paths, claimed-today filter, and response shape.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pulls the searching→results→empty/quiz/quizFailed/claimed state machine out of Claim into a new reusable ClaimQuizSheet widget (lib/widgets/claim_quiz_sheet.dart). Claim now owns only the initial state (scan button + streak badge) and delegates everything from the nearbyPostboxes call onward to the sheet via onCompleted/onCancel callbacks. ClaimQuizResult carries claimedCount/pointsEarned/quizFailed/ empty back to the parent. The compact flag scales up touch targets for the upcoming LiveRouteScreen bottom-sheet variant (Task 8). Behaviour parity preserved: same Cloud Function calls, quiz option generation, James messages, analytics events, confetti, and animations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Pass the parent _ClaimState's streakStream into ClaimQuizSheet via a new optional parameter; remove ClaimQuizSheet's own StreakService instance and subscription so only one listener is active at a time. LiveRouteScreen (T8) can pass its own stream or leave it null to omit the streak chip. - Unify bottom-padding across all five _build* stages behind three state getters (_bottomPad / _buttonHeight / _optionHeight); remove the six inline local variable declarations and the inconsistent 100.0 literals (non-compact path now uniformly uses kJamesStripClearance = 80). - Invert the _runSearch guard to an early-return pattern; drop the empty-branch narration comment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a public positionStream() factory to lib/location_service.dart that wraps Geolocator.getPositionStream with the same Wear OS / AndroidSettings branching as the existing getPosition(). Permissions are the caller's responsibility; the stream surfaces platform errors directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Introduces lib/route/ with three new files: - route_session.dart: mutable session holder (RoutePace, RouteMode enums, corridorMetres/detourMinutes clamped at construction, speedKmh getter) - destination_picker_screen.dart: map-tap-to-pin picker with one-shot GPS via getPosition(), location-error handling with retry, disabled button until pin placed, initialPosition injection hook for testability - route_preview_screen.dart: stub scaffold ready for T7 Adds test/route_destination_picker_test.dart (10 tests: 6 RouteSession unit tests + 4 widget tests). All 115 Flutter tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds NominatimService (OSM Nominatim, 1-rps throttle, GB-biased, User-Agent compliant) and wires a debounced search bar into DestinationPickerScreen; result taps drop the pin, centre the map, and populate destinationLabel on RouteSession. Adds http ^1.2.0 dependency. 24 new tests (10 unit + 4 widget). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the stub with a full implementation: pace SegmentedButton, corridor/detour sliders, 400 ms debounced routePostboxes callable, postbox count/points headline (loading/result/error states), and a Start Route button gated on a first successful response. Adds the LiveRouteScreen stub and 10 widget tests (all green, analyze clean). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements T8 of the route-mode plan: - LiveRouteScreen: full StatefulWidget with injected position/compass/nearby streams, destination card (distance/bearing/ETA), RouteCompassView, hint button wired to JamesController, arrival detection at <25 m, abandon dialog, and per-route points/claimed status row. - RouteCompassView: composite widget layering FuzzyCompass with a precise destination arrow (Transform.rotate on Icons.navigation). - RouteCompletionScreen: stub scaffold (T9 will add the full completion UI). - 16 new tests in route_live_screen_test.dart; all 155 Dart tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Full RouteCompletionScreen layout (points, claimed count, walking time, pace chip, confetti burst, Postman James greeting) replacing the T8 stub. Adds RouteNotifications service (flutter_local_notifications) that fires a local "You're here!" notification on arrival; wired into LiveRouteScreen (_navigateToCompletion + initState permission ask) and initialised in main. Android AndroidManifest.xml already had POST_NOTIFICATIONS; no new manifest changes required. iOS runtime permission handled by the plugin. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add OutlinedButton.icon "Walk to a destination" in Nearby initial state, navigating via Navigator.pushNamed(context, '/route') - Register /route -> DestinationPickerScreen in MaterialApp.routes with the same _guardRoute auth-guard wrapper used by all other named routes - Add routeStart / routePostboxNearby / routeArrival JamesMessage pools (8 lines each) and routeHint(direction) function (5 lines per direction) in JamesMessages; no em-dashes; falls back to "ahead" pool for unknown direction with a debugPrint warning - Wire James messages in LiveRouteScreen (routeStart on initState, one-shot routePostboxNearby on first claim-sheet open, routeHint in hint button) and RouteCompletionScreen (routeArrival replaces hardcoded placeholder) - Add 15 tests: routeStart/Nearby/Arrival pool coverage, routeHint for all four directions + unknown fallback + em-dash guard, /route named-route smoke test, and NavigatorObserver spy for the Nearby entry button Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Retrofit ClaimQuizSheet with injectable NearbyPostboxesCallableFn and StartScoringCallableFn so it can be rendered headless in tests. Thread nearbyCallableForSheet / startScoringCallableForSheet through LiveRouteScreen into the modal sheet. Add two deferred T11 tests: proximity trigger (sheet opens when a postbox is within 30 m) and dedupe (same postbox within 60 s cooldown does not reopen the sheet). Also fix the pre-existing hint-message assertion that tested for literal direction words instead of just a non-empty James message. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Transitive androidx deps (browser 1.9.0, activity 1.12.4, core 1.18.0, navigationevent 1.0.2) pulled in by flutter_local_notifications and related plugins require compileSdk 36 and AGP 8.9.1. Bump Gradle wrapper to 8.11.1 to match. Local builds now require JDK 17. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The master-side _startScan block referenced fields removed by the ClaimQuizSheet refactor (_count, _claimedToday, _startQuiz, currentStage); error/empty/already-claimed handling now lives inside ClaimQuizSheet. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The ClaimQuizSheet refactor reduced the quiz to a single picked cipher, but the UI still prompts "What's the cipher on one of the nearby postboxes?" when multiple unclaimed boxes are in range. A correct identification of a different nearby cipher was wrongly failed. Restore the multi-cipher accept-list behaviour from master: collect every valid nearby cipher into a Set, include them all in the option pool (up to 4) padded with distractors, and accept any of them as correct. The wear OS variant already had this logic.
…ger fires LiveRouteScreen.checkForNearbyClaimable reads box['distance'] from the nearbyPostboxes response to decide when a postbox sits inside the 30 m claim radius and the ClaimQuizSheet should be auto-opened. The slim postbox shape only carried monarch + claimedToday, so distance was always undefined and the trigger never fired in production. (The test stub injected distance directly, masking the gap.) Carry distance through applyUserClaims when present on the underlying LookupResult. Privacy footprint is small: the user already has their own GPS fix, so an unclaimed box's distance is bounded by the scan radius they specified — at most the 2 km server cap.
The post-import update used the count of docs written in this run, which
excludes:
- docs skipped because they carry correctedBy (an admin's reviewReport
fix); they still exist in the postbox collection
- manual_<reportId> docs created when an admin accepts a
missing_postbox report
Either case caused the lifetime leaderboard's "X of Y postboxes (Z%)"
denominator to silently undercount the true population. Use a Firestore
count() aggregation to set the stored figure to the actual collection
size after the import commits.
…claims today recomputeUserAggregates unconditionally wrote dailyDate/weekStart/ monthStart to the current period when re-summing claims for a user affected by a cypher correction. If that user had no claims in the current period, periodSums.dailyPoints was 0 but dailyDate landed at today — making shouldNotifyFirstClaim / shouldNotifyOvertake treat the user as already-having-claimed-today and silently suppress legitimate friend notifications. Only write each marker when the corresponding period sum is positive, so the markers track actual claim activity. (startScoring is unaffected — it only runs the lifetime tx after a successful claim, so the daily sum there is always > 0.)
backfill_lifetime_scores.js unconditionally stamped dailyDate/weekStart/ monthStart even when the corresponding period sum was 0. Same trap as the recompute path fixed in 94ef721: shouldNotifyFirstClaim and shouldNotifyOvertake treat dailyDate === today as evidence the user claimed today, so the backfill silently suppressed legitimate friend notifications for the rest of the London day on every user with no claims in the current period. Only write each marker when the corresponding sum is positive, matching the runtime path.
…'t scan Between LiveRouteScreen._arrived = true and the pushReplacement landing on RouteCompletionScreen, the position stream keeps firing. The old guard only short-circuited the navigate call; setState + _maybeScan still ran on every late frame, so a slow nav transition could trigger a stray nearbyPostboxes call (and pointless bearing-chip rebuilds) for a screen the user is already leaving. Skip the whole handler when _arrived is true.
When a partial-success run leaves some claims already rewritten to the target monarch, a follow-up retry would re-stamp those claims with correctedFromMonarch set to the *new* monarch (read from d.monarch on the second pass), erasing the only on-doc record of the real pre-correction value. Skip claims whose monarch and points already match the target. This also avoids a no-op write on every admin retry. claimCount now reflects actual rewrites rather than the read total, so the admin sees an accurate "rescoredClaims" figure on retries. Added a test covering the partial-rewrite retry case.
Both completion-screen buttons popped to Home, so 'Plan another route' silently did nothing distinguishable from 'Done'. Pop the route-attempt stack first and then push a fresh destination picker so the user lands on the picker with a clean back-stack back to Home.
ReportCypherScreen seeded its dropdown with widget.currentMonarch when that value matched a known cypher, so a user reporting "this is wrong" could submit a suggestion that matched the existing record — a no-op correction the admin still has to triage. Start at notSureCypher and require the user to actively pick a value (or leave the dropdown alone and add a note/photo).
Gradle drops android/build/ (and android/app/build/) on assemble; the Flutter-tools-generated .gitignore only had **/android/.gradle. Added both build dirs so 'git status' is clean after a release build.
…eports The accept dialog already had a copy-to-clipboard button next to the storage path so the admin can paste it into the Firebase console to download the .osc file. The same path on reviewed reports below the list was plain text only — adminstrators looking at it later had to scrape it from the rendered text. Adds the same copy button for consistency.
When a user dragged the corridor or detour slider, _scheduleFetch queued a 400 ms debounced fetch but left _headlineState on the prior result. The Start-route button stayed enabled with the stale headline showing, and a fast tap could start a route whose stats banner showed the previous params' count/points. Set state to loading the moment a param change comes in so the headline reflects "we're recomputing" and the Start-route button is disabled until the new request lands. Existing test for the completion-screen "Plan another route" tweaked to register the /route named route so the post-pop pushNamed has a destination.
_PhotoThumb was a StatelessWidget that called FirebaseStorage.instance.ref(path).getDownloadURL() inside a FutureBuilder built in its build() method, so every rebuild of the parent _ReportCard (busy toggles, Accept dialog opening, sibling state changes) fired a fresh Storage round-trip per thumb. On a pending-tab full of multi-photo reports that was easily 10+ wasted calls per interaction. Convert to StatefulWidget with a per-path cached Future. Same image shows the moment a previous fetch resolved; new path (rare) builds a fresh Future. Photo-detail dialog also reuses the cached URL instead of firing its own getDownloadURL.
Adds a regression test for 7c7940a: changing a slider during a route preview must disable Start-route immediately rather than waiting for the 400 ms debounce, so the user can't start a route whose headline banner shows the old params' postbox count/points.
AdminReportsScreen was a StatelessWidget that called AdminAccess.isAdmin(forceRefresh: true) inside FutureBuilder on every build. Each rebuild fired a fresh getIdTokenResult(true) network round trip to refresh the auth token — the admin claim is granted out of band and shouldn't change during a session, so once is enough. Convert to StatefulWidget and hold the Future in a late final so an orientation flip or Theme update doesn't re-roundtrip Firebase Auth.
LiveRouteScreen wraps its FlutterCompass.events subscription in try/catch because the events stream throws a MissingPluginException in test environments without the platform channel. The Nearby screen and the Wear OS compass page subscribed bare — a Dart test that pumped either widget without mocking the channel would crash on initState. Apply the same try/catch + null-out pattern, matching the existing guard in LiveRouteScreen.
LoginButton (email) was correctly disabled when state.isSubmitting was true, but GoogleLoginButton stayed enabled. A second tap during the Google sign-in flow would dispatch another LoginWithGooglePressed, the bloc would re-emit loading and re-call signInWithGoogle, and the plugin could either throw or stack a second authentication on top of the first. Wrap in BlocBuilder filtered on isSubmitting and null the onPressed during loading, matching the email button's guard.
If the user starts typing an address then back-navigates within the 500 ms debounce window, the Timer would fire after dispose, calling _runSearch -> _nominatimService.search() to issue a Nominatim HTTP request whose result _runSearch's `if (!mounted) return;` then throws away. Burns a slot in the 1-rps OSM throttle and races against the next picker mount. Cancel _debounceTimer in dispose, same as the route preview screen.
NominatimService creates an http.Client in its default constructor and held it for the service's lifetime — but the destination picker created a fresh NominatimService on each open without ever calling close(). Repeated opens (route the user abandons + reopens) accumulated client connection pools. Add a close() method that closes the client only when we created it (tests inject their own), and have the destination picker call it in dispose. Tracks _ownsClient / _ownsNominatimService so injected instances aren't closed under the caller.
Analytics.observer was a getter that returned a new FirebaseAnalyticsObserver on every call. main.dart hands it to MaterialApp.navigatorObservers — which is read on every PostboxGame rebuild — so each rebuild created a fresh observer, replaced the Navigator's subscription, and orphaned the previous one (which still holds an internal RouteObserver subscriber list). Change to a single static final instance so the same observer hangs off the Navigator across rebuilds.
Both the lifetime and per-county rescore transactions read the current total and add countyDelta — which can be negative when a cypher correction reduces the points value (e.g. EVIIR 9 → GR 4). In healthy state the user has earned at least |countyDelta| from those claims so the sum stays positive, but a missing per-county stats doc (legacy claim made before per-county tracking, never backfilled) or an under-reported lifetimePoints (earlier bug) could push the sum negative, which would then leave the user sorted below users with 0 points on the leaderboard. Clamp both newTotal and newLifetimePoints at 0 as defensive insurance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds Route Mode: pick a destination, see how many unclaimed postboxes are en-route and how many points they're worth, then walk it with a precise destination indicator + the existing fuzzy compass for postboxes + the existing quiz-driven claim flow as a bottom sheet. Preserves the app's "fuzzy compass, never pinpoint" contract — the server never returns postbox locations.
Built from
docs/superpowers/specs/…plan; 15 commits sequenced as algorithm extraction → backend callable → claim-quiz refactor → location streaming → screens (picker, preview, live, completion) → wiring → tests → docs → Android SDK bump.Screens added
DestinationPickerScreen(lib/route/destination_picker_screen.dart) — tap-on-map viaPostboxMap.onTap+ Nominatim address search bar (1 req/sec throttle, UK-biased).RoutePreviewScreen— pace toggle (Walk 4.5 km/h / Jog 8.5 km/h), corridor slider 50–500 m, extra-time slider 0–60 min (switches mode to detour when > 0), debouncedroutePostboxescall, "X postboxes worth Y points" headline.LiveRouteScreen—positionStreamGPS, precise distance + bearing arrow + ETA at pace,FuzzyCompassrotated to the destination bearing, "Where now, postie?" hint button (picks the highest-count fuzzy sector, translates to ahead/left/right/behind, fires a James line from the new pool). PeriodicnearbyPostboxesscan; when a returned box hasdistance ≤ 30 && !claimedToday, opens the extractedClaimQuizSheetas acompact: truemodal. 60s dedupe so the same box doesn't re-prompt. On arrival (< 25 m) fires a local notification thenpushReplacements toRouteCompletionScreen.RouteCompletionScreen— points earned (gold headline), claims, walk time, pace chip, James congratulation line, Done + "Plan another route" CTAs.Backend changes
functions/src/_routePlanner.ts— extracted the orienteering beam search + ellipse filter from the existingscripts/plan_route.tsCLI, added a newfilterToCorridor. Single source of truth for both the CLI and the new callable.routePostboxes(functions/src/routePostboxes.ts) — auth-gated, validates inputs (30 km hard cap), fetches via the 9-cell geohash prefix pattern, applies the user's claimed-today set, returns ONLY{ count, points, directDistanceM, budgetDistanceM, warnings }. Never leaks postbox IDs or locations.pointsForMonarch,getTodayLondon,setPrecision, the claims query pattern fromnearbyPostboxes/startScoring.Reuse highlights
lib/widgets/claim_quiz_sheet.dartextracted from the 1109-linelib/claim.dart(now 282 lines). Both Route Mode and the existing Claim screen use the same widget — anti-cheat parity, no score-table drift, same James messages.lib/location_service.dartgot a newpositionStream()alongside the existing one-shotgetPosition().FuzzyCompasswidget composes with a newRouteCompassView(destination arrow overlay) — no changes to fuzzy compass internals.Build / dep changes
http: ^1.2.0(for Nominatim).flutter_local_notificationswas already in pubspec; T9 added a thinRouteNotificationswrapper rather than touching the existingNotificationService.compileSdk35 → 36, AGP 8.7.0 → 8.9.1, Gradle wrapper 8.9 → 8.11.1, prompted by transitive androidx requirements from the plugin set. Local builds now require JDK 17.Test plan
flutter analyze— cleanflutter test— 181 passed (was 105 pre-feature; +76 new acrossroute_session,route_destination_picker,nominatim_service,route_preview_screen,route_live_screen,route_completion_screen, James pools)functions/ npm test— 295 passed (was 274; +21 forroutePostboxesandfilterToCorridor)node functions/lib/scripts/plan_route.js …still producesTotal: 36 pointsfor the canonical 6-postbox fixtureroutePostboxesagainst staging, hit it from the Functions shell with a known London start/destination, comparecountagainst a manual Firestore query.Notes for review
sentinel in geohash prefix queries renders invisibly in terminal diffs. Bytes verified via xxd; the prefix scan is correct.🤖 Generated with Claude Code