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