From 2a2823c180f2b1e03ec188646b6b85f35ec1720d Mon Sep 17 00:00:00 2001 From: FuelBoard Contributor Date: Tue, 13 Oct 2026 18:01:45 +0100 Subject: [PATCH] Accurate distances toggle, Approx. distance markers, and Closest-mode speedup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Distance confidence - Every distance surface takes the figure AND its "Approx." marker from one FuelStore.stationDistanceDisplay call (station rows, detail sheet, widget, Live Activity, watch, Siri), so a surface can never mark one figure approximate while showing a routed one. Radius filters and Closest ordering test that same displayed distance, so no row can read a distance outside the radius that filtered it (the reported "14.6 mi inside a 10-mile list"). Accurate distances (Settings -> Distances, default off) - Off: the browse layer serves the straight line, marked Approx., for every row, so two installs agree to the metre and the app makes no route requests at all. - On: routed distances for the whole search area, unmarked, served only from a COMPLETE set anchored within 600 m — a partial fill reverts every row at once rather than mixing road and straight-line bases. - Best Value maths sits deliberately outside the toggle: it keeps using measured road routes and its own confidence marking, so money figures never move with the switch. - Fills run only while the toggle is on and the app is frontmost; capacity raised to 600 stations per pass; completeness tolerates pins Apple Maps has no route for, which otherwise left an area permanently incomplete and the whole list stuck on estimates. Performance (the reported slowdown switching to Closest / launching with it) - FuelStore.accurateDistancesEnabled was a UserDefaults(suiteName:) read per call: a synchronous cfprefsd round-trip measured at ~1.9 us, ~200k calls per Closest sort. Memoised, with the setter keeping it in step. - Both keyed sorts decorate their keys — one distance per station, carried as an index instead of copying whole stations into the decorated array — and the comparison baseline plus per-station journeys now come from ONE pass instead of two full scans per tab. The Stations tab also reuses its single sorted list for the count and the TOP badge instead of sorting the country three times. - Measured on iPhone SE (7,914-station Closest pool): sort 36.6 ms -> 17.1 ms per list, Cheapest 6 ms, Best Value 11 ms. - DEBUG probes: FB_PERF list timing, FB_PERF_BENCH toggle-read benchmark, and FB_DIST_FOCUS now reports the served value, marker and mode (and re-prints when a fill lands). FB_ACCURATE_DISTANCES / FB_SORT_MODE env forms for devices, where devicectl eats a leading-dash argument. Tests: 198, 0 failures. Device and simulator Debug builds green. --- FuelBoard/ContentView.swift | 344 +++++++++++++++--- FuelBoard/FuelBoardApp.swift | 4 + FuelBoard/FuelSavingsViews.swift | 26 +- FuelBoard/LiveActivityManager.swift | 18 +- FuelBoard/ProximityMonitor.swift | 14 +- FuelBoard/RoadDistanceService.swift | 78 +++- FuelBoard/SettingsView.swift | 17 + FuelBoard/StationDetailView.swift | 4 +- FuelBoard/WatchSyncManager.swift | 7 + FuelBoard/WidgetMockScreen.swift | 21 +- .../FuelBoardSharedTests/FuelBoardTests.swift | 183 ++++++++++ .../FuelSavingsCalculationAuditTests.swift | 8 +- .../FuelSavingsEstimateTests.swift | 124 ++++++- FuelBoardWatch Watch App/ContentView.swift | 7 +- .../FuelBoardLiveActivityView.swift | 43 ++- FuelBoardWidgets/FuelPriceWidget.swift | 11 +- Shared/FuelBoardLiveActivity.swift | 6 + Shared/FuelPriceWidgetViews.swift | 10 +- Shared/FuelSavings.swift | 59 ++- Shared/FuelStore.swift | 148 +++++++- Shared/SiriCheapestLookup.swift | 8 +- 21 files changed, 1009 insertions(+), 131 deletions(-) diff --git a/FuelBoard/ContentView.swift b/FuelBoard/ContentView.swift index 709f6aa..e35f1ba 100644 --- a/FuelBoard/ContentView.swift +++ b/FuelBoard/ContentView.swift @@ -15,10 +15,18 @@ struct ContentView: View { @Environment(\.scenePhase) private var scenePhase @State private var stations: [FuelStation] = FuelStore.loadStations() + /// Bumped when a road-distance fill finishes (or the accurate-distances toggle + /// changes). Row distances are read from the shared store INSIDE the row + /// views, so nothing else would invalidate the list and a freshly completed + /// fill would not appear until some other state changed. + @State private var roadDistanceTick = 0 + /// Set from `FB_DIST_FOCUS`/`-distFocus` so a road-distance fill can re-print + /// the affected station's display value once the new distances land. + @State private var distFocusQuery: String? @State private var selectedFuel: FuelType = FuelStore.loadSelectedFuel() @State private var dataRefreshMode: DataRefreshMode = FuelStore.loadDataRefreshMode() - @State private var sortMode: SortMode = FuelStore.loadSortMode() - @State private var stationLimit: Int = FuelStore.loadStationLimit() + @State private var sortMode: SortMode = ContentView.qaSortModeOverride(default: FuelStore.loadSortMode()) + @State private var stationLimit: Int = ContentView.qaStationRadiusOverride(default: FuelStore.loadStationLimit()) @State private var distanceUnit: DistanceUnit = FuelStore.loadDistanceUnit() @State private var priceDisplayStyle: PriceDisplayStyle = FuelStore.loadPriceDisplayStyle() @State private var stationTapAction: StationTapAction = FuelStore.loadStationTapAction() @@ -40,6 +48,40 @@ struct ContentView: View { return nil }() + // MARK: - QA overrides (DEBUG) + + /// QA hooks: `-stationRadius 10` / `-sortMode cheapest` override the stored + /// results filter and sort for ONE launch, so filtering can be verified at + /// each radius without tapping the picker (simctl has no tap command). Never + /// persisted — the stored setting survives untouched. + private static func qaStationRadiusOverride(default fallback: Int) -> Int { +#if DEBUG + let args = ProcessInfo.processInfo.arguments + if let i = args.firstIndex(of: "-stationRadius"), i + 1 < args.count, + let miles = Int(args[i + 1]), FuelStore.stationRadiusOptions.contains(miles) { + return miles + } +#endif + return fallback + } + + private static func qaSortModeOverride(default fallback: SortMode) -> SortMode { +#if DEBUG + let args = ProcessInfo.processInfo.arguments + if let i = args.firstIndex(of: "-sortMode"), i + 1 < args.count, + let mode = SortMode(rawValue: args[i + 1]) { + return mode + } + // devicectl parses a leading-dash app argument as one of its OWN options, + // so on a real device the env form is the one that arrives. + if let env = ProcessInfo.processInfo.environment["FB_SORT_MODE"], + let mode = SortMode(rawValue: env) { + return mode + } +#endif + return fallback + } + /// The radius the geofence actually uses: the manual alert radius, or — /// when "Follow search" is on — the Stations-tab distance capped at 8 mi. private var effectiveAlertsRadiusKM: Double { @@ -152,8 +194,12 @@ struct ContentView: View { guard let location else { return selling } if sortMode == .closest { return selling } // radius disabled in Closest let radiusKM = distanceUnit.toKM(Double(stationLimit)) // chosen units → km + // Test the distance the ROW shows (cached road route where there is one), + // not the straight line: a station 9.8 mi straight-line but 14.6 mi by road + // used to pass a 10-mile filter and then display 14.6 mi inside it. return selling.filter { - $0.distanceKM(to: location.lat, lng2: location.lng) <= radiusKM + FuelStore.isWithinDisplayRadius(station: $0, radiusKM: radiusKM, + userLat: location.lat, userLng: location.lng) } } @@ -169,7 +215,8 @@ struct ContentView: View { if sortMode == .closest, let location { let radiusKM = distanceUnit.toKM(Double(stationLimit)) // chosen units → km let within = pool.filter { - $0.distanceKM(to: location.lat, lng2: location.lng) <= radiusKM + FuelStore.isWithinDisplayRadius(station: $0, radiusKM: radiusKM, + userLat: location.lat, userLng: location.lng) } if let localMin = within.compactMap({ $0.prices[selectedFuel] }).min() { return localMin @@ -182,7 +229,14 @@ struct ContentView: View { /// First occurrence in display order wins on price ties. private var topStationID: String? { guard let baselinePrice else { return nil } - return displayedStations.first { $0.prices[selectedFuel] == baselinePrice }?.id + return topStationID(in: displayedStations, baselinePrice: baselinePrice) + } + + /// The TOP-badge station within an ALREADY-SORTED list. Split out so a caller + /// that has the sorted list (the Stations tab) does not sort the whole + /// country a second time just to find one id. + private func topStationID(in list: [FuelStation], baselinePrice: Double) -> String? { + list.first { $0.prices[selectedFuel] == baselinePrice }?.id } /// The app's own "cheapest in range" — the TOP-badge station — mirrored @@ -199,7 +253,7 @@ struct ContentView: View { station: station, fuel: selectedFuel, radiusKM: distanceUnit.toKM(Double(stationLimit)), - distanceKM: station.distanceKM(to: location.lat, lng2: location.lng) + distanceKM: FuelStore.displayDistanceKM(station: station, userLat: location.lat, userLng: location.lng) ) } @@ -207,54 +261,116 @@ struct ContentView: View { sortedStations } + /// Timing wrapper. The work lives in `computeSortedStations()` so the DEBUG + /// probe can wrap it while Release/Archive calls the plain function. private var sortedStations: [FuelStation] { +#if DEBUG + if ProcessInfo.processInfo.environment["FB_PERF"] != nil { + let started = Date() + let result = computeSortedStations() + print("FB-PERF sortedStations mode=\(effectiveSortMode.rawValue) stations=\(result.count) " + + "ms=\(String(format: "%.1f", Date().timeIntervalSince(started) * 1_000))") + return result + } +#endif + return computeSortedStations() + } + + /// The display list, sorted. + /// + /// Both keyed paths DECORATE first: each station's display distance is + /// computed once per station rather than twice per comparison, so ranking the + /// whole country in Closest mode costs ~8k distance reads instead of ~200k. + /// That is the difference between a list that appears instantly and a picker + /// that hangs for seconds — the display distance is not free (it consults the + /// accurate-distances gate and the route cache). + private func computeSortedStations() -> [FuelStation] { let pool = poolStations switch effectiveSortMode { case .closest: guard let location else { return pool.sorted { $0.prices[selectedFuel]! < $1.prices[selectedFuel]! } } - return pool.sorted { lhs, rhs in - // Closest first; price only breaks ties. - let lDist = lhs.distanceKM(to: location.lat, lng2: location.lng) - let rDist = rhs.distanceKM(to: location.lat, lng2: location.lng) - if lDist != rDist { return lDist < rDist } - return lhs.prices[selectedFuel]! < rhs.prices[selectedFuel]! + let loc = location + var decorated = pool.indices.map { index in + (index: index, + distance: FuelStore.displayDistanceKM(station: pool[index], userLat: loc.lat, userLng: loc.lng)) } + decorated.sort { lhs, rhs in + // Closest first; price only breaks ties. Ordered on the SAME + // distance the row shows, or a row reading "9.9 mi" can sit above + // one reading "8.0 mi". + if lhs.distance != rhs.distance { return lhs.distance < rhs.distance } + return pool[lhs.index].prices[selectedFuel]! < pool[rhs.index].prices[selectedFuel]! + } + return decorated.map { pool[$0.index] } case .cheapest: - return pool.sorted { lhs, rhs in - // Cheapest first; distance only breaks ties. - let lPrice = lhs.prices[selectedFuel]! - let rPrice = rhs.prices[selectedFuel]! - if lPrice != rPrice { return lPrice < rPrice } - guard let location else { return false } - return lhs.distanceKM(to: location.lat, lng2: location.lng) < - rhs.distanceKM(to: location.lat, lng2: location.lng) + // Cheapest first; distance only breaks ties. Decorated so a tie never + // re-reads a distance inside the comparison (equal prices are common, + // and ties are where the old comparator did all its cache work). + var decorated = pool.indices.map { index in + (index: index, + price: pool[index].prices[selectedFuel]!, + distance: location.map { + FuelStore.displayDistanceKM(station: pool[index], userLat: $0.lat, userLng: $0.lng) + } ?? .greatestFiniteMagnitude) } + decorated.sort { lhs, rhs in + if lhs.price != rhs.price { return lhs.price < rhs.price } + return lhs.distance < rhs.distance + } + return decorated.map { pool[$0.index] } case .worthTheDrive: return sortedForWorthTheDrive(pool) } } - /// Row/detail savings lookup with the comparison baseline resolved ONCE. + /// The comparison baseline AND each station's journey, from ONE pass. /// - /// `nearestSavingsBaseline` scans the whole station list, and StationsView - /// asks for savings once per visible row (plus again for the detail sheet), - /// so handing the view a per-station method would rescan ~8k stations - /// dozens of times per render. - private func savingsLookup(baseline: FuelStation?) -> (FuelStation) -> FuelSavingsAvailability? { + /// Every savings label on a row needs this pair, and it used to be built + /// twice per tab: `nearestSavingsBaseline` scanned all ~8k stations + /// (computing a journey for each) and `savingsLookup` immediately scanned them + /// all AGAIN to fill the per-station map. Both now share one result. + /// + /// The baseline is chosen with EXACTLY the loop and guards of + /// `FuelSavingsEstimator.baselineStation`, so the row labelled "Closest + /// station" cannot move — and it is still selected when the feature is + /// unconfigured (it names that row), while the maps are only built when the + /// labels will actually be shown. + private func savingsScaffold() -> (baseline: FuelStation?, journeys: [String: FuelSavingsJourney]) { + guard let location else { return (nil, [:]) } + let configured = fuelSavingsSettings.isConfigured + var journeys: [String: FuelSavingsJourney] = [:] + var bestStation: FuelStation? + var bestMiles = Double.infinity + for station in stations where station.prices[selectedFuel] != nil { + guard let journey = savingsJourneyMiles(for: station, location: location) else { continue } + if configured { journeys[station.id] = journey } + let miles = journey.miles + guard miles.isFinite, miles >= 0 else { continue } + if miles < bestMiles { + bestMiles = miles + bestStation = station + } + } + return (bestStation, configured ? journeys : [:]) + } + + /// Row/detail savings lookup with the comparison baseline and the per-station + /// journeys already resolved by `savingsScaffold()`. + /// + /// StationsView asks for savings once per visible row, so handing the view a + /// per-station method would rescan ~8k stations dozens of times per render. + private func savingsLookup(baseline: FuelStation?, + journeys: [String: FuelSavingsJourney]) + -> (FuelStation) -> FuelSavingsAvailability? { guard fuelSavingsSettings.isConfigured else { return { _ in nil } } guard let location else { return { _ in .unavailable("Location unavailable") } } - let baselineJourney = baseline.flatMap { savingsJourneyMiles(for: $0, location: location) } + let baselineJourney: FuelSavingsJourney? = baseline.flatMap { + journeys[$0.id] ?? savingsJourneyMiles(for: $0, location: location) + } let baselineID = baseline?.id let settings = fuelSavingsSettings let amount = sessionFuelSavingsAmount let fuel = selectedFuel - var journeys: [String: FuelSavingsJourney] = [:] - for station in stations { - if journeys[station.id] == nil, - let journey = savingsJourneyMiles(for: station, location: location) { - journeys[station.id] = journey - } - } return { station in FuelSavingsEstimator.availability( station: station, @@ -271,7 +387,7 @@ struct ContentView: View { /// The nearest station that sells the selected fuel — the Best Value /// comparison baseline. It is chosen with the SAME journey values used by /// the savings calculation, so a cached road route can change the baseline - /// and keep the "more driving than closest" labels synchronized with the + /// and keep the "extra return travel" labels synchronized with the /// cost calculation. When no road route is cached, the journey falls back to /// straight-line out-and-back, and the result is marked as an estimate. private var nearestSavingsBaseline: FuelStation? { @@ -470,8 +586,62 @@ struct ContentView: View { FuelStore.refreshedFavourites(favourites, from: stations) } +#if DEBUG + /// Prints the DISPLAYED distance and its source for stations matching `query` + /// (see `FB_DIST_FOCUS`). Called once a fix is available and again whenever a + /// road-distance fill lands: that before/after is how the accurate-distances + /// toggle is verified on a real device, where no screenshot tool exists. + static func logDistanceFocus(query: String, stations: [FuelStation], location: Coordinate?) { + guard let loc = location else { + print("FB-DIST no location fix (stations=\(stations.count))") + return + } + let matches = stations.filter { $0.name.localizedCaseInsensitiveContains(query) } + print("FB-DIST query=\"\(query)\" matches=\(matches.count) pool=\(stations.count) " + + "mode=\(FuelStore.accurateDistancesEnabled) " + + "ready=\(FuelStore.accurateDistancesReady(userLat: loc.lat, userLng: loc.lng))") + for s in matches { + let d = FuelStore.roadDistanceDiagnostics(for: s, userLat: loc.lat, userLng: loc.lng) + let straight = s.distanceKM(to: loc.lat, lng2: loc.lng) + let shown = FuelStore.stationDistanceDisplay(station: s, userLat: loc.lat, userLng: loc.lng) + print("FB-DIST \"\(s.name)\" display=\(String(format: "%.3f", d.displayDistanceKM))km " + + "straight=\(String(format: "%.3f", straight))km source=\(d.displaySource.rawValue) " + + "outward=\(d.outwardMeters.map { String(format: "%.0f", $0) } ?? "nil")m " + + "return=\(d.returnMeters.map { String(format: "%.0f", $0) } ?? "nil")m " + + "shown=\(String(format: "%.3f", shown.km))km approx=\(shown.isApproximate)") + } + } +#endif + +#if DEBUG + /// Times the two forms of the accurate-distances read: the raw suite read + /// (one `UserDefaults(suiteName:)` construction plus a cfprefsd round-trip, + /// which is what `stationDistanceDisplay` did on EVERY call) against the + /// memoised property. Prints microseconds per read so the cost of the old hot + /// path can be multiplied out from measurement instead of guessed at. + static func runToggleReadBenchmark() { + let reads = 20_000 + var sink = 0 + var started = Date() + for _ in 0..` prints the DISPLAYED distance and its + // source for every matching station, read from THIS device's own + // route cache (FB-DIST). Two devices can legitimately disagree + // (one has a routed distance, the other falls back to the straight + // line), and this is the only way to compare them side by side + // without a device screenshot tool. Waits (bounded) for a fix. + var distFocusQuery: String? = nil + if let i = args.firstIndex(of: "-distFocus"), i + 1 < args.count { distFocusQuery = args[i + 1] } + // devicectl swallows a leading-dash app argument (it parses `-distFocus` + // as its own `-t`), so the env form — set as DEVICECTL_CHILD_FB_DIST_FOCUS + // — is the one that works on a real device. + if let env = ProcessInfo.processInfo.environment["FB_DIST_FOCUS"], !env.isEmpty { + distFocusQuery = env + } + if let query = distFocusQuery { + self.distFocusQuery = query + Task { @MainActor in + for _ in 0..<12 { + if location != nil, !stations.isEmpty { break } + try? await Task.sleep(nanoseconds: 1_500_000_000) + } + ContentView.logDistanceFocus(query: query, stations: stations, location: location) + } + } // `-seedFavourite ` pins the first matching station // for Unleaded via the normal save path (keychain + app group) // so screenshot captures can show a populated Favourites tab. @@ -1087,10 +1322,19 @@ struct ContentView: View { /// The Stations tab, extracted from `body` so the TabView expression stays /// within the compiler's type-check budget. private var stationsTab: some View { - let baselineStation = nearestSavingsBaseline + // ONE scaffold (baseline + journeys) and ONE sort serve the list, its + // count, the TOP badge and every savings label. Asking for + // `nearestSavingsBaseline`, `displayedStations`, `sortedStations.count`, + // `topStationID` and `savingsLookup` separately walked the full ~8k list + // six times per render — seconds of main-thread work in Closest mode, + // where the pool is every station in the UK. + let scaffold = savingsScaffold() + let baselineStation = scaffold.baseline + let displayed = displayedStations + let topID = baselinePrice.flatMap { topStationID(in: displayed, baselinePrice: $0) } return StationsView( - stations: displayedStations, - totalCount: sortedStations.count, + stations: displayed, + totalCount: displayed.count, isLoading: isLoading, selectedFuel: $selectedFuel, sortMode: Binding(get: { effectiveSortMode }, set: { sortMode = $0 }), @@ -1098,13 +1342,13 @@ struct ContentView: View { distanceUnit: distanceUnit, priceDisplayStyle: priceDisplayStyle, baselinePrice: baselinePrice, - topStationID: topStationID, + topStationID: topID, location: location, favouriteIDs: favouriteIDs, stationTapAction: stationTapAction, fuelSavingsSettings: fuelSavingsSettings, fuelSavingsAmount: $sessionFuelSavingsAmount, - savingsForStation: savingsLookup(baseline: baselineStation), + savingsForStation: savingsLookup(baseline: baselineStation, journeys: scaffold.journeys), savingsBaselineStationName: baselineStation?.name, journeyDiagnosticsForStation: { station in journeyDiagnostics(for: station, baseline: baselineStation) }, onToggleFavourite: toggleFavourite, @@ -1115,7 +1359,8 @@ struct ContentView: View { /// The Favourites tab, extracted from `body` for the same type-check /// budget reason. private var favouritesTab: some View { - let baselineStation = nearestSavingsBaseline + let scaffold = savingsScaffold() + let baselineStation = scaffold.baseline return FavouritesView( favourites: refreshedFavourites, selectedFuel: selectedFuel, @@ -1125,7 +1370,7 @@ struct ContentView: View { stationTapAction: stationTapAction, fuelSavingsSettings: fuelSavingsSettings, fuelSavingsAmount: $sessionFuelSavingsAmount, - savingsForStation: savingsLookup(baseline: baselineStation), + savingsForStation: savingsLookup(baseline: baselineStation, journeys: scaffold.journeys), savingsBaselineStationName: baselineStation?.name, journeyDiagnosticsForStation: { station in journeyDiagnostics(for: station, baseline: baselineStation) }, onToggleFavourite: toggleFavourite, @@ -1451,10 +1696,14 @@ struct StationRow: View { .lineLimit(1) .truncationMode(.tail) if let location { - Text(distanceUnit.format(FuelStore.displayDistanceKM(station: station, userLat: location.lat, userLng: location.lng))) + let distance = FuelStore.stationDistanceDisplay( + station: station, userLat: location.lat, userLng: location.lng + ) + Text(distance.label(unit: distanceUnit)) .font(.caption2) .foregroundStyle(.secondary) .monospacedDigit() + .accessibilityLabel(distance.spokenLabel(unit: distanceUnit)) } } .frame(maxWidth: .infinity, alignment: .leading) @@ -1513,7 +1762,8 @@ struct StationRow: View { } if fuelSavings != nil { // Full-width verdict banner under the row. Inline in the middle - // column it truncated ("Save £2.75 after tra…") because the + // column it truncated ("Approx. £2.75 better off after tra…") + // because the // price block squeezes that column — and the verdict is the // point of this sort mode, so it gets its own line. FuelSavingsSummaryView( diff --git a/FuelBoard/FuelBoardApp.swift b/FuelBoard/FuelBoardApp.swift index edb1d02..b71d702 100644 --- a/FuelBoard/FuelBoardApp.swift +++ b/FuelBoard/FuelBoardApp.swift @@ -42,6 +42,10 @@ struct FuelBoardApp: App { pricePence: 1499, priceDisplayStyle: FuelStore.loadPriceDisplayStyle(), distanceKM: 8.0, + // QA mock: marked approximate so the hook renders the LONGEST + // distance form ("Approx. 8.0 mi") and the island / Lock Screen + // layout can be checked against it. + distanceIsApproximate: true, lat: 51.5, lng: -0.12, updatedAt: Date() diff --git a/FuelBoard/FuelSavingsViews.swift b/FuelBoard/FuelSavingsViews.swift index 76b12e4..2449dd2 100644 --- a/FuelBoard/FuelSavingsViews.swift +++ b/FuelBoard/FuelSavingsViews.swift @@ -75,7 +75,7 @@ struct FuelSavingsSummaryView: View { let amount: FuelSavingsFillAmount let baselineStationName: String? var compact: Bool = false - /// Sizes the "more driving than closest" caption in the user's own unit (the + /// Sizes the "extra return travel" caption in the user's own unit (the /// stored figure is miles) — used by both the row banner and the detail sheet. var distanceUnit: DistanceUnit = .miles @@ -84,7 +84,8 @@ struct FuelSavingsSummaryView: View { case .available(let result): if compact { // FULL-WIDTH banner under the row. Inline inside the middle - // column the capsule truncated ("Save £2.75 after tra…") because + // column the capsule truncated ("Approx. £2.75 better off after + // tra…") because // the column is squeezed by the price block — the verdict is the // point of this sort mode, so it gets its own line with room. // Colour carries the verdict: green save, red worse off, grey @@ -103,6 +104,7 @@ struct FuelSavingsSummaryView: View { .foregroundStyle(.secondary) .lineLimit(1) .minimumScaleFactor(0.85) + .accessibilityLabel(spokenExtraTravel(for: result, fallback: extraTravel)) } } // Aligns the banner with the row's text column (46 pt logo + 12 @@ -125,6 +127,7 @@ struct FuelSavingsSummaryView: View { Text(extraTravel) .font(.subheadline) .foregroundStyle(.secondary) + .accessibilityLabel(spokenExtraTravel(for: result, fallback: extraTravel)) } if result.status == .baseline { Text("Your closest station with a price for this fuel — every other saving is measured against it.") @@ -161,11 +164,22 @@ struct FuelSavingsSummaryView: View { } } + /// VoiceOver form of the extra-travel caption ("Approximately 6.4 miles extra + /// return travel"): the abbreviation is spelled out and the unit is a word. + /// Falls back to the visible string if no spoken form exists. + private func spokenExtraTravel(for result: FuelSavingsResult, fallback: String) -> String { + FuelSavingsFormat.extraTravelSpokenLabel( + extraDistanceMiles: result.extraDistanceMiles, + unit: distanceUnit, + confidence: result.journeyConfidence + ) ?? fallback + } + + /// VoiceOver form of the verdict. `spokenHeadline()` spells "Approx." out for + /// estimated comparisons and leaves the plain words ("Break even", "Closest + /// station") alone — one source, so visible and spoken copy cannot drift. private func accessibilityHeadline(for result: FuelSavingsResult) -> String { - let headline = result.headline() - return result.journeyConfidence.isEstimated && result.status != .baseline - ? "Approximate: \(headline.replacingOccurrences(of: "~", with: ""))" - : headline + result.spokenHeadline() } /// The colour-coded verdict capsule — ONE implementation for the listing rows diff --git a/FuelBoard/LiveActivityManager.swift b/FuelBoard/LiveActivityManager.swift index cd0297a..2a4abd7 100644 --- a/FuelBoard/LiveActivityManager.swift +++ b/FuelBoard/LiveActivityManager.swift @@ -50,6 +50,11 @@ enum LiveActivityManager { return } + // ONE cache read for the figure AND its confidence: the activity shows + // "Approx." exactly when the app's own rows would. + let distance = FuelStore.stationDistanceDisplay( + station: best, userLat: location.lat, userLng: location.lng + ) let state = FuelBoardLiveActivityAttributes.ContentState( fuel: fuel, stationID: best.id, @@ -57,7 +62,8 @@ enum LiveActivityManager { brand: best.brand, pricePence: price, priceDisplayStyle: priceDisplayStyle ?? FuelStore.loadPriceDisplayStyle(), - distanceKM: FuelStore.displayDistanceKM(station: best, userLat: location.lat, userLng: location.lng), + distanceKM: distance.km, + distanceIsApproximate: distance.isApproximate, lat: best.lat, lng: best.lng, updatedAt: Date() @@ -119,14 +125,20 @@ enum LiveActivityManager { lat: Double, lng: Double ) -> FuelStation? { + // Radius and tie-break both use the distance the pill SHOWS, so the + // activity can never name a station the app's own list has filtered out. stations .filter { $0.prices[fuel] != nil } - .filter { $0.distanceKM(to: lat, lng2: lng) <= radiusKM } + .filter { + FuelStore.isWithinDisplayRadius(station: $0, radiusKM: radiusKM, + userLat: lat, userLng: lng) + } .min { lhs, rhs in let lPrice = lhs.prices[fuel]! let rPrice = rhs.prices[fuel]! if lPrice != rPrice { return lPrice < rPrice } - return lhs.distanceKM(to: lat, lng2: lng) < rhs.distanceKM(to: lat, lng2: lng) + return FuelStore.displayDistanceKM(station: lhs, userLat: lat, userLng: lng) + < FuelStore.displayDistanceKM(station: rhs, userLat: lat, userLng: lng) } } } diff --git a/FuelBoard/ProximityMonitor.swift b/FuelBoard/ProximityMonitor.swift index 529dd95..8dfb3ea 100644 --- a/FuelBoard/ProximityMonitor.swift +++ b/FuelBoard/ProximityMonitor.swift @@ -178,8 +178,8 @@ final class ProximityMonitor: NSObject, ObservableObject, @preconcurrency CLLoca .filter { !favIDs.contains($0.id) } .sorted { lhs, rhs in guard let location else { return false } - return lhs.distanceKM(to: location.lat, lng2: location.lng) < - rhs.distanceKM(to: location.lat, lng2: location.lng) + return FuelStore.displayDistanceKM(station: lhs, userLat: location.lat, userLng: location.lng) < + FuelStore.displayDistanceKM(station: rhs, userLat: location.lat, userLng: location.lng) } candidates.append(contentsOf: others) @@ -316,12 +316,15 @@ final class ProximityMonitor: NSObject, ObservableObject, @preconcurrency CLLoca return } let sellers = stations.filter { $0.prices[fuel] != nil } + // Display distance, so the "N in range" count here matches the list the + // user is actually looking at. let inRange = sellers.filter { - $0.distanceKM(to: fix.coordinate.lat, lng2: fix.coordinate.lng) <= radiusKM + FuelStore.isWithinDisplayRadius(station: $0, radiusKM: radiusKM, + userLat: fix.coordinate.lat, userLng: fix.coordinate.lng) } let cheapest = inRange.min { ($0.prices[fuel] ?? .infinity) < ($1.prices[fuel] ?? .infinity) } let cheapestDistance = cheapest.map { - $0.distanceKM(to: fix.coordinate.lat, lng2: fix.coordinate.lng) + FuelStore.displayDistanceKM(station: $0, userLat: fix.coordinate.lat, userLng: fix.coordinate.lng) } debugStatus = DebugLocationStatus( coordinate: fix.coordinate, @@ -674,7 +677,8 @@ final class ProximityMonitor: NSObject, ObservableObject, @preconcurrency CLLoca // identical to handleEntry's withinRadius logic. let inRadius = sellers.filter { station in guard let location else { return true } - return station.distanceKM(to: location.lat, lng2: location.lng) <= radiusKM + return FuelStore.isWithinDisplayRadius(station: station, radiusKM: radiusKM, + userLat: location.lat, userLng: location.lng) } let candidate = inRadius.min { ($0.prices[fuel] ?? .infinity) < ($1.prices[fuel] ?? .infinity) } diff --git a/FuelBoard/RoadDistanceService.swift b/FuelBoard/RoadDistanceService.swift index 246eb6f..84ec587 100644 --- a/FuelBoard/RoadDistanceService.swift +++ b/FuelBoard/RoadDistanceService.swift @@ -7,19 +7,26 @@ // here, not in the widget. The widget/Live Activity just read the cache. // // Throttling: recompute at most every `throttleMinutes`, or when the user has -// moved `moveThresholdMeters` from where the cache was built. Bounded to the -// `candidatesPerPass` nearest stations so a pass stays a handful of route calls. +// moved `moveThresholdMeters` from where the cache was built. In +// "Accurate distances" mode a pass routes the WHOLE search area (up to +// `fillCapacity` stations) so the browse layer can serve routed distances for +// every row at once; the mode must also be ON for any pass to run at all. import Foundation import MapKit +import UIKit import WidgetKit enum RoadDistanceService { - /// Upper bound on stations routed per pass, so a pass stays a bounded set of - /// route calls. Worth-the-Drive now asks for outward AND return routes, so - /// this must stay conservative; a larger value can make first launch feel - /// unresponsive while MapKit drains dozens of route requests. - static let candidatesPerPass = 12 + /// Upper bound on stations routed per pass. Accurate-distances mode routes the + /// whole search area in ONE pass, because a 12-per-pass budget could never + /// make a list fully routed — that is exactly why two devices (and two + /// radii) disagreed about the same station. Capped so a pathological dataset + /// can't turn one pass into thousands of route requests; a capped pass stays + /// incomplete, and the browse layer keeps showing straight-line + "Approx." + /// until a later pass finishes the job. Measured on device: ~3.2 route calls + /// per second, so 600 stations (1,200 calls) is ~6 min at the very worst. + static let fillCapacity = 600 /// How many Worth-the-Drive candidates may claim slots in a pass. Those /// rows are ranked by NET SAVING and are typically FURTHER out than the /// nearest stations — routing only the nearest stations (the old behaviour) @@ -46,6 +53,14 @@ enum RoadDistanceService { lat: Double, lng: Double, prioritising priorityStations: [FuelStation] = []) async { + // "Accurate distances" OFF (the default) means the browse layer never + // serves a routed value, so any routing would be pure cost with nothing + // visible to show for it: no pass, no requests, no widget reloads. + guard FuelStore.accurateDistancesEnabled else { return } + // Only fill while the app is on screen. The browse layer is the only + // consumer, so a background fill would spend radio and battery routing a + // position nothing is displaying — worst case, a long drive. + guard await appIsActive() else { return } guard await refreshGate.begin() else { return } defer { Task { await refreshGate.end() } } @@ -79,7 +94,7 @@ enum RoadDistanceService { var candidates: [FuelStation] = [] var seen = Set() func consider(_ station: FuelStation) { - guard candidates.count < candidatesPerPass else { return } + guard candidates.count < fillCapacity else { return } guard station.distanceKM(to: lat, lng2: lng) <= maxRadiusKM else { return } guard seen.insert(station.id).inserted else { return } guard !FuelStore.hasRoadDistance(for: station, userLat: lat, userLng: lng) else { return } @@ -88,19 +103,23 @@ enum RoadDistanceService { for station in priorityStations.prefix(priorityCandidates) { consider(station) } let nearest = stations.sorted { $0.distanceKM(to: lat, lng2: lng) < $1.distanceKM(to: lat, lng2: lng) } for station in nearest { - if candidates.count >= candidatesPerPass { break } + if candidates.count >= fillCapacity { break } consider(station) } guard !candidates.isEmpty else { return } + let passStart = Date() + let origin = CLLocationCoordinate2D(latitude: lat, longitude: lng) var entries: [String: CachedRoadDistance] = originCompatible ? (existing?.entries ?? [:]) : [:] + var routedCount = 0 for station in candidates { let dest = CLLocationCoordinate2D(latitude: station.lat, longitude: station.lng) async let outward = roadMeters(from: origin, to: dest) async let back = roadMeters(from: dest, to: origin) if let meters = await outward { + routedCount += 1 // Store the exact pin that was routed so the display layer can // refuse to serve this value if the station later appears with // a different coordinate (corrected pin / other data source). @@ -115,13 +134,50 @@ enum RoadDistanceService { return } - FuelStore.saveRoadDistances(sourceLat: lat, sourceLng: lng, entries: entries) - // Wake the widgets so the new road distances surface immediately. + // A pass is complete when nothing in the search area is left UNRESOLVED: + // every station either has a route from this anchor, or was attempted in + // THIS pass and Apple Maps returned none. A few pins genuinely have no + // route (offshore, deleted forecourt, bad snapshot) and one of those must + // never leave the area permanently incomplete — that would pin the whole + // list on straight-line estimates forever. Those stations show their + // straight line, marked Approx., and the radius filter uses that same + // distance, so nothing is ever out of step with what a row displays. + let attempted = Set(candidates.map(\.id)) + let capped = candidates.count >= fillCapacity + let unresolved = capped || stations.contains { station in + station.distanceKM(to: lat, lng2: lng) <= maxRadiusKM + && !FuelStore.hasRoadDistance(for: station, userLat: lat, userLng: lng) + && !attempted.contains(station.id) + } + FuelStore.saveRoadDistances(sourceLat: lat, sourceLng: lng, entries: entries, + complete: !unresolved) +#if DEBUG + // `FB-ROUTE` prints what a pass actually cost (stations, route calls, + // wall-clock, unroutable pins, whether the set is now complete) — the + // evidence for any "how taxing is routing?" answer, and how a bigger pass + // gets scaled from measurement instead of guesswork. + print("FB-ROUTE pass stations=\(candidates.count) calls=\(candidates.count * 2) " + + "elapsed=\(String(format: "%.2f", Date().timeIntervalSince(passStart)))s " + + "cached=\(entries.count) failed=\(candidates.count - routedCount) " + + "complete=\(!unresolved)") +#endif + // Wake the widgets so the new road distances surface immediately, and + // tell the app so its rows re-read the distance they are showing. WidgetCenter.shared.reloadAllTimelines() + await MainActor.run { + NotificationCenter.default.post(name: .fuelBoardRoadDistancesUpdated, object: nil) + } } private static let refreshGate = RoadDistanceRefreshGate() + /// Accurate mode fills only while the app is on screen: the browse layer is + /// the only consumer of routed distances, so a background fill would spend + /// radio and battery routing a position nothing is looking at. + private static func appIsActive() async -> Bool { + await MainActor.run { UIApplication.shared.applicationState == .active } + } + /// Driving distance (metres) between two coordinates via Apple Maps routing. private static func roadMeters(from: CLLocationCoordinate2D, to: CLLocationCoordinate2D) async -> Double? { let request = MKDirections.Request() diff --git a/FuelBoard/SettingsView.swift b/FuelBoard/SettingsView.swift index f1b04a5..cd002c8 100644 --- a/FuelBoard/SettingsView.swift +++ b/FuelBoard/SettingsView.swift @@ -63,6 +63,9 @@ struct SettingsView: View { /// Hidden developer flag — the Debug section only appears when on. Toggled /// by tapping the About → Version row five times (NOT a user-facing switch). @State private var debugMode: Bool = FuelStore.loadDebugMode() + /// "Accurate distances" — read once on appear and written straight through to + /// the shared store, which is what every display surface actually consults. + @State private var accurateDistances: Bool = FuelStore.accurateDistancesEnabled @State private var versionTapCount = 0 @State private var lastVersionTap = Date.distantPast /// Widget diagnostics: the extension's last makeEntry beacon PER intent @@ -186,6 +189,20 @@ struct SettingsView: View { Text("Distances and search radii across the app, widget and alerts are shown in this unit. Prices can be shown as on a station sign (129.9) or in pounds and pence (£1.29⁹/L).") } + Section { + Toggle("Accurate distances", isOn: $accurateDistances) + .onChange(of: accurateDistances) { _, newValue in + FuelStore.setAccurateDistancesEnabled(newValue) + WidgetCenter.shared.reloadAllTimelines() + NotificationCenter.default.post(name: .fuelBoardAccurateDistancesChanged, + object: nil) + } + } header: { + Text("Distances") + } footer: { + Text("Off (default): distances are straight-line estimates marked Approx. and the app makes no route requests. On: Apple Maps driving distances for every station in range, filled only while this app is open. Search radii then mean miles of driving.") + } + Section { Picker("Tap station", selection: $stationTapAction) { ForEach(StationTapAction.allCases) { action in diff --git a/FuelBoard/StationDetailView.swift b/FuelBoard/StationDetailView.swift index 2e0c251..55b0fd6 100644 --- a/FuelBoard/StationDetailView.swift +++ b/FuelBoard/StationDetailView.swift @@ -61,10 +61,10 @@ struct StationDetailView: View { postcode: station.postcode, name: station.name)) if let location { - let km = FuelStore.displayDistanceKM( + let distance = FuelStore.stationDistanceDisplay( station: station, userLat: location.lat, userLng: location.lng ) - LabeledContent("Distance", value: distanceUnit.format(km)) + LabeledContent("Distance", value: distance.label(unit: distanceUnit)) .monospacedDigit() } ForEach(StationNotice.notices(for: station)) { notice in diff --git a/FuelBoard/WatchSyncManager.swift b/FuelBoard/WatchSyncManager.swift index 9731133..23eceb1 100644 --- a/FuelBoard/WatchSyncManager.swift +++ b/FuelBoard/WatchSyncManager.swift @@ -4,6 +4,13 @@ import WatchConnectivity extension Notification.Name { static let fuelBoardWatchRefreshRequested = Notification.Name("FuelBoardWatchRefreshRequested") static let fuelBoardShowStationDetail = Notification.Name("FuelBoardShowStationDetail") + /// Posted when a road-distance fill finishes, so the app's rows re-read the + /// distance they are showing — those reads happen inside the row views, so + /// nothing else would invalidate them. + static let fuelBoardRoadDistancesUpdated = Notification.Name("FuelBoardRoadDistancesUpdated") + /// Posted when "Accurate distances" is switched, so the browse layer can + /// re-render and start a fill from where the user actually is. + static let fuelBoardAccurateDistancesChanged = Notification.Name("FuelBoardAccurateDistancesChanged") } @MainActor diff --git a/FuelBoard/WidgetMockScreen.swift b/FuelBoard/WidgetMockScreen.swift index 19a815b..af218ee 100644 --- a/FuelBoard/WidgetMockScreen.swift +++ b/FuelBoard/WidgetMockScreen.swift @@ -66,14 +66,25 @@ struct WidgetMockScreen: View { } } - /// Real stations (loaded dump), cheapest-first for the chosen fuel — the - /// same selection the widget's own provider makes. + /// Real stations (loaded dump) for the widget's DEFAULT configuration — + /// Unleaded / Cheapest / 5 miles — with the SAME radius test the widget's + /// provider applies. Without it this preview happily listed the UK-wide + /// cheapest (Banbridge, 184 mi away) under "Cheapest fuel near you", which + /// is not what the widget ever shows. private func mockEntry(family: WidgetFamily) -> FuelPriceWidgetEntryData { let fuel = FuelType.e10 - let all = FuelStore.loadStations() - let selling = all.filter { $0.prices[fuel] != nil } - .sorted { ($0.prices[fuel] ?? .infinity) < ($1.prices[fuel] ?? .infinity) } let location = FuelStore.loadLocation().map { Coordinate(lat: $0.lat, lng: $0.lng) } + // 5 miles — the widget config's default Distance. WidgetDistance itself + // lives in the widget target, so the preview mirrors its default here. + let radiusKM = 5 * 1.60934 + let all = FuelStore.loadStations() + let selling = all.filter { station in + guard station.prices[fuel] != nil else { return false } + guard let location else { return true } + return FuelStore.isWithinDisplayRadius(station: station, radiusKM: radiusKM, + userLat: location.lat, userLng: location.lng) + } + .sorted { ($0.prices[fuel] ?? .infinity) < ($1.prices[fuel] ?? .infinity) } let count = family == .systemSmall ? 1 : 3 return FuelPriceWidgetEntryData( date: Date(), diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift index ea50973..4b71781 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift @@ -632,6 +632,9 @@ final class RoadDistanceCacheTests: XCTestCase { // test or run would pollute these. Overwrite with an empty, far-away // cache (source at (0,0)) so every test starts from a clean slate. FuelStore.saveRoadDistances(sourceLat: 0, sourceLng: 0, entries: [:]) + // "Accurate distances" is a persisted setting, so a previous run (or a + // test that turned it on) would leak into these. Off is the default. + FuelStore.setAccurateDistancesEnabled(false) } private func station(_ id: String, _ lat: Double, _ lng: Double) -> FuelStation { @@ -647,6 +650,7 @@ final class RoadDistanceCacheTests: XCTestCase { } func testRoadDistanceUsedWhenCachedNear() { + FuelStore.setAccurateDistancesEnabled(true) let s = station("a", 51.5074, -0.1278) // Cache a road distance of 3.2 km for this station from the user's fix. FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, @@ -656,6 +660,7 @@ final class RoadDistanceCacheTests: XCTestCase { } func testRoadDistanceNilWhenOriginFar() { + FuelStore.setAccurateDistancesEnabled(true) let s = station("a", 51.5074, -0.1278) // Cache built in London, but the user is now ~200 km away -> stale. FuelStore.saveRoadDistances(sourceLat: 51.5074, sourceLng: -0.1278, @@ -668,6 +673,7 @@ final class RoadDistanceCacheTests: XCTestCase { } func testRoadDistanceUsedForOtherStationNotFound() { + FuelStore.setAccurateDistancesEnabled(true) FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, entries: ["a": .init(meters: 3200, lat: 51.5074, lng: -0.1278)]) // A station that isn't in the cache falls back to straight-line. @@ -677,6 +683,7 @@ final class RoadDistanceCacheTests: XCTestCase { } func testRoadDistanceNotServedWhenStationPinDiffers() { + FuelStore.setAccurateDistancesEnabled(true) // Route a road distance to station "a" at pin P1. FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, entries: ["a": .init(meters: 3200, lat: 51.5074, lng: -0.1278)]) @@ -689,7 +696,12 @@ final class RoadDistanceCacheTests: XCTestCase { "road value routed to the old pin leaked onto a different coordinate") } + /// A partial cache legitimately mixes road and straight-line distances for + /// the SAVINGS maths (Best Value marks the estimated ones itself), but the + /// browse layer only ever serves a COMPLETE set — see the accurate-distances + /// tests below. func testPartialRoadCacheCanMixRoadAndStraightLineDistances() { + FuelStore.setAccurateDistancesEnabled(true) let routed = station("routed", 51.5074, -0.1278) let estimated = station("estimated", 51.5100, -0.1300) FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, @@ -707,6 +719,39 @@ final class RoadDistanceCacheTests: XCTestCase { XCTAssertFalse(FuelStore.hasMeasuredReturnRoadDistance(for: estimated, userLat: 51.6, userLng: -0.1)) } + /// The reported bug: "10 miles cheapest" listed Jet Chidswell at 14.6 mi. + /// Its straight line was inside 10 miles, its routed distance was not — the + /// filter tested the former while the row displayed the latter. + func testRadiusFilterUsesTheDisplayedDistanceNotTheStraightLine() { + FuelStore.setAccurateDistancesEnabled(true) + let sourceLat = 51.6, sourceLng = -0.1 + let far = station("far", 51.5074, -0.1278) // in by straight line… + let near = station("near", 51.5400, -0.1000) + FuelStore.saveRoadDistances( + sourceLat: sourceLat, sourceLng: sourceLng, + entries: ["far": .init(meters: 23_500, lat: 51.5074, lng: -0.1278, returnMeters: 23_500)]) + + let tenMilesKM = 10 * 1.60934 + XCTAssertLessThan(far.distanceKM(to: sourceLat, lng2: sourceLng), tenMilesKM, + "fixture: the straight line must sit INSIDE the 10-mile radius") + XCTAssertGreaterThan(FuelStore.displayDistanceKM(station: far, userLat: sourceLat, userLng: sourceLng), + tenMilesKM, "fixture: the routed distance must sit OUTSIDE it") + + XCTAssertTrue(FuelStore.isWithinDisplayRadius(station: near, radiusKM: tenMilesKM, + userLat: sourceLat, userLng: sourceLng)) + XCTAssertFalse(FuelStore.isWithinDisplayRadius(station: far, radiusKM: tenMilesKM, + userLat: sourceLat, userLng: sourceLng), + "a station whose row reads 14.6 mi must not appear in a 10-mile list") + // The same station is back once the radius actually covers its distance. + XCTAssertTrue(FuelStore.isWithinDisplayRadius(station: far, radiusKM: 15 * 1.60934, + userLat: sourceLat, userLng: sourceLng)) + // …and an unknown station (no cached route) still tests its displayed + // straight line, so nothing changes for the bulk of the list. + let unrouted = station("unrouted", 51.5400, -0.1000) + XCTAssertTrue(FuelStore.isWithinDisplayRadius(station: unrouted, radiusKM: tenMilesKM, + userLat: sourceLat, userLng: sourceLng)) + } + func testLegacyOneWayRoadCacheIsUsableButNotConfirmed() { let legacy = station("legacy", 51.5074, -0.1278) FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, @@ -723,6 +768,144 @@ final class RoadDistanceCacheTests: XCTestCase { XCTAssertFalse(diagnostics.hasMeasuredReturn) XCTAssertTrue(diagnostics.hasValidCacheEntry) } + + /// A station distance the user can see is marked "Approx." until a routed road + /// distance exists for it. Figure and marker come out of ONE cache read, so a + /// surface can never show a routed value labelled approximate (or the reverse). + func testStationDistanceIsMarkedApproximateUntilRoutingLands() { + FuelStore.setAccurateDistancesEnabled(true) + let routed = station("routed", 51.5074, -0.1278) + let unrouted = station("unrouted", 51.5200, -0.1400) + FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, + entries: ["routed": .init(meters: 3200, lat: 51.5074, lng: -0.1278, returnMeters: 3600)]) + + let confirmed = FuelStore.stationDistanceDisplay(station: routed, userLat: 51.6, userLng: -0.1) + XCTAssertFalse(confirmed.isApproximate, "a cached road distance is not an approximation") + XCTAssertEqual(confirmed.km, 3.2, accuracy: 0.0001) + XCTAssertEqual(confirmed.label(unit: .miles), "2.0 mi") + XCTAssertEqual(confirmed.spokenLabel(unit: .miles), "2.0 miles") + + let approximate = FuelStore.stationDistanceDisplay(station: unrouted, userLat: 51.6, userLng: -0.1) + XCTAssertTrue(approximate.isApproximate, "the straight-line fallback must be identified as one") + XCTAssertEqual(approximate.km, unrouted.distanceKM(to: 51.6, lng2: -0.1), accuracy: 0.0001) + XCTAssertEqual(approximate.label(unit: .miles), + "Approx. " + DistanceUnit.miles.format(approximate.km)) + XCTAssertEqual(approximate.spokenLabel(unit: .kilometers), + "Approximately " + DistanceUnit.kilometers.spoken(approximate.km)) + } + + /// The marker tracks the cache on its own: same station, bare figure once its + /// route lands, "Approx." again once the origin moves out of tolerance. + func testStationDistanceLabelFollowsTheRoadCache() { + FuelStore.setAccurateDistancesEnabled(true) + let moved = station("moved", 51.5074, -0.1278) + XCTAssertTrue(FuelStore.stationDistanceDisplay(station: moved, userLat: 51.6, userLng: -0.1).isApproximate, + "nothing has been routed yet") + FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, + entries: ["moved": .init(meters: 3200, lat: 51.5074, lng: -0.1278, returnMeters: 3600)]) + XCTAssertFalse(FuelStore.stationDistanceDisplay(station: moved, userLat: 51.6, userLng: -0.1).isApproximate, + "route data landed, so the marker must drop without any other change") + XCTAssertTrue(FuelStore.stationDistanceDisplay(station: moved, userLat: 59.0, userLng: -0.1).isApproximate, + "a cache built from another origin must not confirm this one") + } + + // MARK: - Accurate distances toggle (browse layer only) + + /// The toggle must be opt-in: unset means off, so a fresh install makes no + /// route requests and every device shows the same straight-line figures. + func testAccurateDistancesDefaultsToOffWhenUnset() { + UserDefaults(suiteName: FuelStore.appGroupSuite)?.removeObject(forKey: FuelStore.accurateDistancesKey) + XCTAssertFalse(FuelStore.accurateDistancesEnabled) + } + + /// Mode OFF: a COMPLETE routed cache exists and the browse layer must still + /// show the straight line, marked Approx., for every row — while the savings + /// maths keeps using the measured routes. That boundary is the feature. + func testAccurateDistancesOffIgnoresCachedRoutes() { + let routed = station("routed", 51.5074, -0.1278) + FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, + entries: ["routed": .init(meters: 23_500, lat: 51.5074, + lng: -0.1278, returnMeters: 23_500)], + complete: true) + + let display = FuelStore.stationDistanceDisplay(station: routed, userLat: 51.6, userLng: -0.1) + XCTAssertTrue(display.isApproximate, "the browse layer must never serve routes with the mode off") + XCTAssertEqual(display.km, routed.distanceKM(to: 51.6, lng2: -0.1), accuracy: 0.0001) + XCTAssertTrue(FuelStore.isWithinDisplayRadius(station: routed, radiusKM: 10 * 1.60934, + userLat: 51.6, userLng: -0.1), + "with the mode off the radius is a straight-line radius, agreeing with the rows") + + XCTAssertEqual(FuelStore.roadJourneyMeters(for: routed, userLat: 51.6, userLng: -0.1), 47_000, + "Best Value keeps its measured routes whichever way the toggle is set") + XCTAssertTrue(FuelStore.hasMeasuredReturnRoadDistance(for: routed, userLat: 51.6, userLng: -0.1)) + } + + /// An incomplete fill must never be half-served: the row that HAS a route + /// falls back too, so a list can't read "14.6 mi" beside rows that its radius + /// filter measured as straight lines. + func testAccurateDistancesOnWithIncompleteSetServesStraightLineForEveryRow() { + FuelStore.setAccurateDistancesEnabled(true) + let routed = station("routed", 51.5074, -0.1278) + FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, + entries: ["routed": .init(meters: 23_500, lat: 51.5074, + lng: -0.1278, returnMeters: 23_500)], + complete: false) + + let display = FuelStore.stationDistanceDisplay(station: routed, userLat: 51.6, userLng: -0.1) + XCTAssertTrue(display.isApproximate) + XCTAssertEqual(display.km, routed.distanceKM(to: 51.6, lng2: -0.1), accuracy: 0.0001, + "a partial set must not be served, even for a station that has a route") + } + + /// Mode ON with a complete set from a location we are standing at: routed + /// values, unmarked, and the radius means miles of driving. + func testAccurateDistancesOnServesRoutedValuesWhenCompleteAndNear() { + FuelStore.setAccurateDistancesEnabled(true) + let routed = station("routed", 51.5074, -0.1278) + FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, + entries: ["routed": .init(meters: 23_500, lat: 51.5074, + lng: -0.1278, returnMeters: 23_500)], + complete: true) + + let display = FuelStore.stationDistanceDisplay(station: routed, userLat: 51.6, userLng: -0.1) + XCTAssertFalse(display.isApproximate) + XCTAssertEqual(display.km, 23.5, accuracy: 0.0001) + XCTAssertFalse(FuelStore.isWithinDisplayRadius(station: routed, radiusKM: 10 * 1.60934, + userLat: 51.6, userLng: -0.1), + "in this mode a 10-mile radius is 10 miles of driving, matching the routed rows") + } + + /// Same complete cache, but the user has since driven away: nothing is served + /// (every row falls back together) rather than showing routes measured from a + /// place the user has left. Walking back inside tolerance serves it again + /// with no refill. + func testAccurateDistancesOnFallsBackTogetherWhenTheAnchorMoves() { + FuelStore.setAccurateDistancesEnabled(true) + let routed = station("routed", 51.5074, -0.1278) + FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, + entries: ["routed": .init(meters: 23_500, lat: 51.5074, + lng: -0.1278, returnMeters: 23_500)], + complete: true) + let movedLat = 51.6 + 0.018 // ~2 km north, outside the 600 m tolerance + XCTAssertTrue(FuelStore.stationDistanceDisplay(station: routed, userLat: movedLat, userLng: -0.1).isApproximate) + XCTAssertFalse(FuelStore.stationDistanceDisplay(station: routed, userLat: 51.6, userLng: -0.1).isApproximate) + } + + /// The toggle is a browse-layer switch, not a data switch: Best Value's + /// journey metres and its measured-return confidence must be identical in + /// both positions. + func testAccurateDistancesSavingsMathsIsUnaffectedByTheToggle() { + let routed = station("routed", 51.5074, -0.1278) + FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, + entries: ["routed": .init(meters: 23_500, lat: 51.5074, + lng: -0.1278, returnMeters: 23_600)], + complete: true) + for mode in [false, true] { + FuelStore.setAccurateDistancesEnabled(mode) + XCTAssertEqual(FuelStore.roadJourneyMeters(for: routed, userLat: 51.6, userLng: -0.1), 47_100) + XCTAssertTrue(FuelStore.hasMeasuredReturnRoadDistance(for: routed, userLat: 51.6, userLng: -0.1)) + } + } } // MARK: - Install identity diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift index 53c93d2..8f06460 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift @@ -110,9 +110,9 @@ final class FuelSavingsCalculationAuditTests: XCTestCase { // 2.05 mi shows as "2.0 mi" at one decimal place — the same caption. XCTAssertEqual(FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 2.05, unit: .miles), - "2.0 mi more driving than closest") + "2.0 mi extra return travel") XCTAssertEqual(FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 2.00, unit: .miles), - "2.0 mi more driving than closest") + "2.0 mi extra return travel") } func testBriefCaseMFGHareAndHoundsIsWorseOff() throws { @@ -242,7 +242,7 @@ final class FuelSavingsCalculationAuditTests: XCTestCase { XCTAssertEqual(result.extraDrivingCostGBP, 0.701613, accuracy: 1e-6) XCTAssertEqual(result.netSavingGBP, 1.217509, accuracy: 1e-6) XCTAssertEqual(result.status, .worthTheDrive) - XCTAssertEqual(result.headline(), "Save £1.22 after travel") + XCTAssertEqual(result.headline(), "£1.22 better off after travel") } func testNearerCandidateIsNeverChargedExtraTravel() throws { @@ -393,7 +393,7 @@ final class FuelSavingsCalculationAuditTests: XCTestCase { XCTAssertEqual(mixed?.result?.distanceBasis, .straightLine, "one unmeasured leg makes the whole estimate an estimate") XCTAssertEqual(mixed?.result?.journeyConfidence, .estimated) - XCTAssertEqual(mixed?.result?.headline(compact: true), "~£0.17 worse off after travel") + XCTAssertEqual(mixed?.result?.headline(compact: true), "Approx. £0.17 worse off after travel") } func testBaselineStationIsLabelledClosestStationNotBreakEven() { diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift index 10b73ca..091ea4f 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift @@ -313,7 +313,7 @@ final class FuelSavingsEstimateTests: XCTestCase { XCTAssertEqual(result.netSavingGBP, -0.334, accuracy: 0.005) XCTAssertEqual(result.status, .notWorthTheDrive) XCTAssertEqual(result.journeyConfidence, .estimated) - XCTAssertEqual(result.headline(compact: true), "~£0.33 worse off after travel") + XCTAssertEqual(result.headline(compact: true), "Approx. £0.33 worse off after travel") } func testMeasuredRoadRoutesDropTheEstimateMarker() { @@ -378,7 +378,7 @@ final class FuelSavingsEstimateTests: XCTestCase { // £60 buys 37.5 L at 160p; 10p/litre cheaper saves £3.75; 0.4 extra // return miles cost ~£0.09. Net ≈ £3.66. XCTAssertEqual(result.netSavingGBP, 3.659, accuracy: 0.005) - XCTAssertEqual(result.headline(compact: true), "~Save £3.66 after travel") + XCTAssertEqual(result.headline(compact: true), "Approx. £3.66 better off after travel") } func testNeutralResultHeadlineIsBreakEven() { @@ -413,7 +413,7 @@ final class FuelSavingsEstimateTests: XCTestCase { netSavingGBP: 0.52, status: .worthTheDrive, distanceBasis: .roadRoute) - XCTAssertEqual(saving.headline(), "Save £0.52 after travel") + XCTAssertEqual(saving.headline(), "£0.52 better off after travel") let loss = FuelSavingsResult(fillLitres: 34.9, grossSavingGBP: 1.40, @@ -460,11 +460,11 @@ final class FuelSavingsEstimateTests: XCTestCase { func testExtraTravelLabelFollowsTheDisplayUnit() { XCTAssertEqual( FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 19.9, unit: .miles), - "19.9 mi more driving than closest" + "19.9 mi extra return travel" ) XCTAssertEqual( FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 19.9, unit: .kilometers), - "32.0 km more driving than closest" + "32.0 km extra return travel" ) XCTAssertNil(FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 0, unit: .miles), "the baseline row has no extra travel to report") @@ -497,7 +497,7 @@ final class FuelSavingsEstimateTests: XCTestCase { status: .worthTheDrive, distanceBasis: .straightLine ) - XCTAssertEqual(saving.capsuleHeadline(), "~Save £1.00 after travel") + XCTAssertEqual(saving.capsuleHeadline(), "Approx. £1.00 better off after travel") let worse = FuelSavingsResult( fillLitres: 30, @@ -510,6 +510,118 @@ final class FuelSavingsEstimateTests: XCTestCase { XCTAssertEqual(worse.capsuleHeadline(), "£0.31 worse off after travel") } + // MARK: - Estimate wording (Approx. labels) + + /// The estimate marker is the WORD "Approx.", never a "~", and it is applied to + /// both directions of the money result. + func testEstimatedHeadlinesUseTheWordApproxForBothDirections() { + let saving = FuelSavingsResult( + fillLitres: 30, + grossSavingGBP: 4.00, + extraDrivingCostGBP: 2.75, + extraDistanceMiles: 6.4, + netSavingGBP: 1.25, + status: .worthTheDrive, + distanceBasis: .straightLine + ) + XCTAssertEqual(saving.headline(), "Approx. £1.25 better off after travel") + + let loss = FuelSavingsResult( + fillLitres: 30, + grossSavingGBP: 1.40, + extraDrivingCostGBP: 2.17, + extraDistanceMiles: 6.4, + netSavingGBP: -0.77, + status: .notWorthTheDrive, + distanceBasis: .straightLine + ) + XCTAssertEqual(loss.headline(), "Approx. £0.77 worse off after travel") + } + + /// A confirmed road-route comparison must never carry the marker — in either + /// direction, and not as a stray tilde either. + func testConfirmedHeadlinesCarryNoApproxMarker() { + let saving = FuelSavingsResult( + fillLitres: 30, + grossSavingGBP: 4.00, + extraDrivingCostGBP: 2.75, + extraDistanceMiles: 6.4, + netSavingGBP: 1.25, + status: .worthTheDrive, + distanceBasis: .roadRoute, + journeyConfidence: .confirmed + ) + XCTAssertEqual(saving.headline(), "£1.25 better off after travel") + + let loss = FuelSavingsResult( + fillLitres: 30, + grossSavingGBP: 1.40, + extraDrivingCostGBP: 2.17, + extraDistanceMiles: 6.4, + netSavingGBP: -0.77, + status: .notWorthTheDrive, + distanceBasis: .roadRoute, + journeyConfidence: .confirmed + ) + XCTAssertEqual(loss.headline(), "£0.77 worse off after travel") + + for headline in [saving.headline(), loss.headline(), saving.capsuleHeadline()] { + XCTAssertFalse(headline.contains("Approx"), "\"\(headline)\" must not claim to be an estimate") + XCTAssertFalse(headline.contains("~"), "\"\(headline)\" must use a word, not a symbol") + } + } + + /// The extra-travel caption names the RETURN journey in both states. The + /// estimated caption repeats "Approx." deliberately: it describes a different + /// value from the verdict above it. + func testExtraTravelNamesTheReturnJourney() { + XCTAssertEqual( + FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 6.4, unit: .miles), + "6.4 mi extra return travel" + ) + XCTAssertEqual( + FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 6.4, unit: .miles, confidence: .estimated), + "Approx. 6.4 mi extra return travel" + ) + } + + /// VoiceOver gets words, not abbreviations: "Approx." → "Approximately" and + /// "mi"/"km" → "miles"/"kilometres". + func testSpokenLabelsSpellOutTheAbbreviations() { + let estimated = FuelSavingsResult( + fillLitres: 30, + grossSavingGBP: 1.40, + extraDrivingCostGBP: 2.17, + extraDistanceMiles: 6.4, + netSavingGBP: -0.77, + status: .notWorthTheDrive, + distanceBasis: .straightLine + ) + XCTAssertEqual(estimated.spokenHeadline(), "Approximately £0.77 worse off after travel") + + XCTAssertEqual( + FuelSavingsFormat.extraTravelSpokenLabel(extraDistanceMiles: 6.4, unit: .miles, confidence: .estimated), + "Approximately 6.4 miles extra return travel" + ) + XCTAssertEqual( + FuelSavingsFormat.extraTravelSpokenLabel(extraDistanceMiles: 19.9, unit: .kilometers), + "32.0 kilometres extra return travel" + ) + + let confirmed = FuelSavingsResult( + fillLitres: 30, + grossSavingGBP: 1.40, + extraDrivingCostGBP: 2.17, + extraDistanceMiles: 6.4, + netSavingGBP: -0.77, + status: .notWorthTheDrive, + distanceBasis: .roadRoute, + journeyConfidence: .confirmed + ) + XCTAssertEqual(confirmed.spokenHeadline(), confirmed.headline(), + "a confirmed comparison has no abbreviation to spell out") + } + // MARK: - Slider bounds (Settings and the in-list control share these) /// Both fill-up controls are slider-only now, so every position the slider diff --git a/FuelBoardWatch Watch App/ContentView.swift b/FuelBoardWatch Watch App/ContentView.swift index 941bece..07a96e0 100644 --- a/FuelBoardWatch Watch App/ContentView.swift +++ b/FuelBoardWatch Watch App/ContentView.swift @@ -392,7 +392,10 @@ struct ContentView: View { let cheapest = WatchFuelCache.cheapestFavourite(in: fuelFavourites, fuel: fuelCase) return fuelFavourites.map { favourite in - let distanceText = lastLocation.map { distanceUnit.format(km: favourite.station.distanceKM(to: $0)) } + // watchOS has no routed road distances (no MKDirections here and no + // shared route cache), so EVERY watch figure is the straight-line + // fallback and must say so — same rule as the app. + let distanceText = lastLocation.map { "Approx. " + distanceUnit.format(km: favourite.station.distanceKM(to: $0)) } let priceText = favourite.station.prices[fuelCase].map { WatchFuelCache.priceText($0, style: priceStyle) } ?? "—" return WatchFavouriteRow( id: favourite.id, @@ -480,7 +483,7 @@ struct ContentView: View { .fontWeight(.bold) Spacer() if let location = lastLocation { - Text(distanceUnit.format(km: favourite.station.distanceKM(to: location))) + Text("Approx. " + distanceUnit.format(km: favourite.station.distanceKM(to: location))) .font(.caption) .foregroundStyle(.secondary) } diff --git a/FuelBoardWidgets/FuelBoardLiveActivityView.swift b/FuelBoardWidgets/FuelBoardLiveActivityView.swift index 6474755..63cdeec 100644 --- a/FuelBoardWidgets/FuelBoardLiveActivityView.swift +++ b/FuelBoardWidgets/FuelBoardLiveActivityView.swift @@ -135,12 +135,21 @@ private struct FuelBoardLiveActivityView: View { VStack(alignment: .leading, spacing: 2) { Text("Cheapest \(context.state.fuel.displayName)") .font(.headline) - Text("\(context.state.stationName) · \(context.state.distanceText)") - .font(.subheadline) - .foregroundStyle(.secondary) - .lineLimit(1) - .minimumScaleFactor(0.75) - .truncationMode(.tail) + // Name and distance are SEPARATE texts: the "Approx." marker + // lengthened this line, and as one tail-truncated string the + // FIGURE was what got cut ("… · Approx…"). The distance keeps its + // intrinsic width and the NAME truncates instead — the number is + // the informative part. + HStack(spacing: 4) { + Text(context.state.stationName) + .lineLimit(1) + .truncationMode(.tail) + Text("· \(context.state.distanceText)") + .lineLimit(1) + .layoutPriority(1) + } + .font(.subheadline) + .foregroundStyle(.secondary) } .frame(maxWidth: .infinity, alignment: .leading) @@ -223,9 +232,17 @@ private struct FuelBoardLiveActivityStationView: View { var body: some View { Link(destination: FuelBoardLiveActivityTap.tapDestination(for: context.state)) { - Text("\(context.state.stationName) · \(context.state.distanceText)") - .font(.caption) - .lineLimit(1) + // Same split as the Lock Screen card: keep the distance and its + // "Approx." marker whole, truncate the name. + HStack(spacing: 4) { + Text(context.state.stationName) + .lineLimit(1) + .truncationMode(.tail) + Text("· \(context.state.distanceText)") + .lineLimit(1) + .layoutPriority(1) + } + .font(.caption) } } } @@ -248,9 +265,13 @@ private extension FuelBoardLiveActivityView { // MARK: - ContentState display helpers extension FuelBoardLiveActivityAttributes.ContentState { - /// Display distance using the user's chosen unit (miles default). + /// Display distance using the user's chosen unit (miles default), marked + /// "Approx." when the figure is the straight-line fallback rather than a + /// routed road distance — the same rule and formatter as the app's station + /// rows, so no surface can claim more confidence than it has. var distanceText: String { - FuelStore.loadDistanceUnit().format(distanceKM) + StationDistanceDisplay(km: distanceKM, isApproximate: distanceIsApproximate ?? false) + .label(unit: FuelStore.loadDistanceUnit()) } /// Price string from pence, in the user's chosen display style diff --git a/FuelBoardWidgets/FuelPriceWidget.swift b/FuelBoardWidgets/FuelPriceWidget.swift index 8f12de6..0743fdc 100644 --- a/FuelBoardWidgets/FuelPriceWidget.swift +++ b/FuelBoardWidgets/FuelPriceWidget.swift @@ -227,7 +227,8 @@ struct FuelPriceTimelineProvider String? { guard extraDistanceMiles > 0.05 else { return nil } let distanceText = distance(extraDistanceMiles, unit: unit) - switch confidence { - case .confirmed: - return "\(distanceText) more driving than closest" - case .estimated: - return "Approx. \(distanceText) additional travel" - } + return confidence == .estimated + ? "Approx. \(distanceText) extra return travel" + : "\(distanceText) extra return travel" + } + + /// VoiceOver form of `extraTravelLabel`. The abbreviation and the unit suffix + /// are spoken as words ("Approximately 6.4 miles extra return travel") because + /// synthesizers render "Approx." and "mi" inconsistently. + static func extraTravelSpokenLabel(extraDistanceMiles: Double, + unit: DistanceUnit, + confidence: JourneyConfidence = .confirmed) -> String? { + guard extraDistanceMiles > 0.05 else { return nil } + let distanceText = spokenDistance(extraDistanceMiles, unit: unit) + return confidence == .estimated + ? "Approximately \(distanceText) extra return travel" + : "\(distanceText) extra return travel" + } + + /// A distance stored in MILES, spoken: "6.4 miles" / "10.3 kilometres". The + /// miles→km hop is deliberate: the stored figure is miles while + /// `DistanceUnit.spoken` speaks a KM value, the same boundary `distance(_:unit:)` + /// crosses. + static func spokenDistance(_ miles: Double, unit: DistanceUnit) -> String { + unit.spoken(miles * FuelSavingsCalculator.kilometresPerMile) } } @@ -164,26 +183,34 @@ extension FuelSavingsResult { ) } - /// Row headline, e.g. "Save £0.52 after travel", "£0.31 worse off after + /// Row headline, e.g. "£1.22 better off after travel", "£0.31 worse off after /// travel", "Break even", "Closest station". /// - /// Estimated comparisons are prefixed with `~` for positive/negative money - /// results. Break-even/baseline remain words, with estimate context carried - /// by the extra-distance caption or detail explanation. + /// Estimated comparisons are prefixed "Approx. " for positive/negative money + /// results — the WORD, never a "~": a tilde reads as decoration (and assistive + /// tech may skip it) instead of saying "this is an estimate". Break-even and + /// baseline stay plain words, with the estimate context carried by the + /// extra-distance caption or the detail explanation. func headline(compact: Bool = false) -> String { - let prefix = journeyConfidence.isEstimated ? "~" : "" + let prefix = journeyConfidence.isEstimated ? "Approx. " : "" switch status { case .baseline: return Self.baselineHeadline case .breakEven: return Self.breakEvenHeadline case .worthTheDrive: - return "\(prefix)Save \(FuelSavingsFormat.money(netSavingGBP)) after travel" + return "\(prefix)\(FuelSavingsFormat.money(netSavingGBP)) better off after travel" case .notWorthTheDrive: return "\(prefix)\(FuelSavingsFormat.money(abs(netSavingGBP))) worse off after travel" } } + /// VoiceOver form of `headline()`: "Approx." is spelled out so a synthesizer + /// cannot read the abbreviation as "approx" or drop it entirely. + func spokenHeadline() -> String { + headline().replacingOccurrences(of: "Approx. ", with: "Approximately ") + } + /// Row capsule copy: the headline, plus what the baseline row IS. Without the /// suffix the baseline capsule just repeated the status word above it /// ("Closest station" twice) and said nothing about the saving being measured @@ -400,7 +427,7 @@ enum FuelSavingsEstimator { /// Baseline station for Best Value: the station with the shortest journey /// among stations that sell the selected fuel, using the SAME journey source /// the savings calculation will use. This keeps the row labelled "Closest - /// station" and every "more driving than closest" value tied to one + /// station" and every "extra return travel" value tied to one /// reference, instead of mixing a separate straight-line baseline with /// road-route savings. static func baselineStation(_ stations: [FuelStation], diff --git a/Shared/FuelStore.swift b/Shared/FuelStore.swift index b668775..1901b5d 100644 --- a/Shared/FuelStore.swift +++ b/Shared/FuelStore.swift @@ -192,6 +192,20 @@ enum DistanceUnit: String, Codable, CaseIterable, Identifiable { func format(_ km: Double) -> String { String(format: "%.1f %@", fromKM(km), shortName) } + + /// Spoken form of a KM distance ("6.4 miles", "10.3 kilometres"), for + /// VoiceOver. `format(_:)` returns the compact "6.4 mi"/"1.2 km" suffix that a + /// synthesizer may read as letters or skip, and "Approx." needs a word too, so + /// every spoken distance goes through here. + func spoken(_ km: Double) -> String { + switch self { + case .miles: + let miles = fromKM(km) + return String(format: "%.1f %@", miles, label(for: miles)) + case .kilometers: + return String(format: "%.1f kilometres", km) + } + } } /// Display style for fuel prices. Internally prices are always stored and @@ -1282,6 +1296,63 @@ struct FuelStore { /// catches any real pin change. static let roadDistancePinToleranceDegrees: Double = 1e-4 + // MARK: Accurate distances (browse-layer toggle) + + /// "Accurate distances". OFF (default) = the browse layer never uses a + /// routed road distance: every surface shows the straight-line value marked + /// "Approx.", so two devices agree exactly and the app makes no route + /// requests at all. ON = routed distances are shown, but only once the whole + /// search area has been routed from an anchor we are still standing near, so + /// a list is never a mix of road and straight-line values. + static let accurateDistancesKey = "fuelboard.accurateDistances" + + /// Memoised, because this is read from the DISPLAY hot path. + /// + /// `UserDefaults(suiteName:)` builds a fresh suite object and forces a + /// synchronous cfprefsd read, and `stationDistanceDisplay` consults this + /// property on every call — which the Closest sort used to make twice per + /// comparison (~200k reads for the whole-country pool), so switching to + /// Closest or launching with it selected stalled the main thread for seconds + /// on XPC alone. Same failure class as the keychain read that once tripped + /// the launch watchdog. Only this app writes the key, and it writes it + /// through `setAccurateDistancesEnabled`, which keeps the memo in step; + /// each PROCESS reads the stored value once (a widget extension has its own). + private static var accurateDistancesMemo: Bool? + + static var accurateDistancesEnabled: Bool { + if let memo = accurateDistancesMemo { return memo } + let stored = UserDefaults(suiteName: appGroupSuite)?.bool(forKey: accurateDistancesKey) ?? false + accurateDistancesMemo = stored + return stored + } + + static func setAccurateDistancesEnabled(_ enabled: Bool) { + accurateDistancesMemo = enabled + UserDefaults(suiteName: appGroupSuite)?.set(enabled, forKey: accurateDistancesKey) + } + + /// Whether routed distances may be shown right now: the toggle is on AND the + /// cached routes cover the whole search area from a location we are still + /// near. Anything else serves straight-line + "Approx." for EVERY row, which + /// is what stops a list from mixing bases (a row reading "14.6 mi" inside a + /// "10 miles" list) and stops membership changing row by row as a fill lands. + static func accurateDistancesReady(userLat: Double, userLng: Double) -> Bool { + guard accurateDistancesEnabled else { return false } + guard let cache = loadRoadDistances(), cache.complete == true else { return false } + return haversineMetres(cache.sourceLat, cache.sourceLng, userLat, userLng) + <= roadDistanceOriginToleranceMeters + } + + private static func haversineMetres(_ lat1: Double, _ lng1: Double, _ lat2: Double, _ lng2: Double) -> Double { + let r = 6371000.0 + let dLat = (lat2 - lat1) * .pi / 180 + let dLng = (lng2 - lng1) * .pi / 180 + let a = sin(dLat / 2) * sin(dLat / 2) + + cos(lat1 * .pi / 180) * cos(lat2 * .pi / 180) * + sin(dLng / 2) * sin(dLng / 2) + return r * 2 * atan2(sqrt(a), sqrt(1 - a)) + } + /// In-memory copy of the road-distance cache. /// /// `loadRoadDistances()` reads the KEYCHAIN — a synchronous `securityd` XPC @@ -1294,9 +1365,12 @@ struct FuelStore { private static var roadDistanceMemo: RoadDistanceCache? /// Distinguishes "not loaded yet" from "loaded, and there is no cache". private static var roadDistanceMemoLoaded = false - static func saveRoadDistances(sourceLat: Double, sourceLng: Double, entries: [String: CachedRoadDistance]) { + static func saveRoadDistances(sourceLat: Double, sourceLng: Double, + entries: [String: CachedRoadDistance], + complete: Bool = true) { let cache = RoadDistanceCache(sourceLat: sourceLat, sourceLng: sourceLng, - updatedAt: Date().timeIntervalSince1970, entries: entries) + updatedAt: Date().timeIntervalSince1970, entries: entries, + complete: complete) if let data = try? JSONEncoder().encode(cache) { saveString(data.base64EncodedString(), service: roadDistancesKey) } @@ -1389,10 +1463,36 @@ struct FuelStore { /// Distance (km) to display for a station: cached ROAD distance when /// available (matches Apple Maps), else straight-line haversine. static func displayDistanceKM(station: FuelStation, userLat: Double, userLng: Double) -> Double { - if let meters = roadDistanceMeters(for: station, userLat: userLat, userLng: userLng) { - return meters / 1000.0 + stationDistanceDisplay(station: station, userLat: userLat, userLng: userLng).km + } + + /// Distance to show for a station AND whether it came from a routed road + /// distance or the straight-line fallback. Both come out of ONE cache read so + /// a surface can never mark a figure "Approx." while showing a routed value + /// (or the reverse) — the failure mode when the number and its label are + /// computed separately. + static func stationDistanceDisplay(station: FuelStation, userLat: Double, userLng: Double) -> StationDistanceDisplay { + // Accurate-distances mode: a routed value is served ONLY when the whole + // search area has been routed from a nearby anchor. Otherwise every row + // falls back to straight-line together, so a list never mixes bases and + // the radius filter (which reads this same function) always agrees with + // what the rows show. + if accurateDistancesEnabled, + accurateDistancesReady(userLat: userLat, userLng: userLng), + let meters = roadDistanceMeters(for: station, userLat: userLat, userLng: userLng) { + return StationDistanceDisplay(km: meters / 1000.0, isApproximate: false) } - return station.distanceKM(to: userLat, lng2: userLng) + return StationDistanceDisplay(km: station.distanceKM(to: userLat, lng2: userLng), isApproximate: true) + } + + /// Radius test on the SAME distance the UI shows. Filtering straight-line + /// while rows display a cached ROAD distance let a station read "14.6 mi" + /// inside a list set to "within 10 miles" — both numbers were individually + /// right (9.8 mi straight-line, 14.6 mi by road) but the list looked broken. + /// Uncached stations still test straight-line, i.e. their displayed value, so + /// nothing changes for them. + static func isWithinDisplayRadius(station: FuelStation, radiusKM: Double, userLat: Double, userLng: Double) -> Bool { + stationDistanceDisplay(station: station, userLat: userLat, userLng: userLng).km <= radiusKM } // MARK: Low-level keychain helpers @@ -1467,6 +1567,29 @@ struct RoadDistanceDiagnostics: Equatable { let hasValidCacheEntry: Bool } +/// The station distance a surface should show, plus whether it came from a routed +/// road distance or the straight-line fallback. The confidence travels WITH the +/// value, so a label can never claim a confidence the figure doesn't have. +struct StationDistanceDisplay: Equatable { + let km: Double + let isApproximate: Bool + + /// Row/sheet label: a straight-line fallback is marked "Approx." so a figure + /// that is NOT a routed road distance can never be read as one, and a cached + /// road distance stays a bare figure. It updates by itself once routing lands, + /// because `FuelStore.stationDistanceDisplay` re-reads the cache every call. + func label(unit: DistanceUnit) -> String { + let text = unit.format(km) + return isApproximate ? "Approx. \(text)" : text + } + + /// VoiceOver form of `label(unit:)`. + func spokenLabel(unit: DistanceUnit) -> String { + let text = unit.spoken(km) + return isApproximate ? "Approximately \(text)" : text + } +} + /// Cached Apple-Maps road distances for nearby stations (see /// A single cached road distance plus the station pin it was routed to. Keeping /// the pin lets `roadDistanceMeters` refuse to serve a route computed for a @@ -1493,4 +1616,19 @@ struct RoadDistanceCache: Codable { let sourceLng: Double let updatedAt: TimeInterval let entries: [String: CachedRoadDistance] + /// True when `entries` covers EVERY station within + /// `RoadDistanceService.maxRadiusKM` of the source — only then may the browse + /// layer serve routed distances (see `FuelStore.accurateDistancesReady`). + /// Absent in caches written before the accurate-distances toggle existed, + /// which were 12-per-pass sets and never complete: absent = not complete. + let complete: Bool? + + init(sourceLat: Double, sourceLng: Double, updatedAt: TimeInterval, + entries: [String: CachedRoadDistance], complete: Bool? = nil) { + self.sourceLat = sourceLat + self.sourceLng = sourceLng + self.updatedAt = updatedAt + self.entries = entries + self.complete = complete + } } diff --git a/Shared/SiriCheapestLookup.swift b/Shared/SiriCheapestLookup.swift index 229a3a2..884eb6a 100644 --- a/Shared/SiriCheapestLookup.swift +++ b/Shared/SiriCheapestLookup.swift @@ -22,8 +22,14 @@ enum SiriCheapestLookup { withinMiles: Double ) -> FuelStation? { let radiusKM = withinMiles * 1.60934 + // Same radius rule as the app list: the distance the answer would SHOW + // (cached road route where there is one). return stations - .filter { $0.prices[fuel] != nil && $0.distanceKM(to: lat, lng2: lng) <= radiusKM } + .filter { + $0.prices[fuel] != nil + && FuelStore.isWithinDisplayRadius(station: $0, radiusKM: radiusKM, + userLat: lat, userLng: lng) + } .min { lhs, rhs in guard let lp = lhs.prices[fuel], let rp = rhs.prices[fuel] else { return false