From 9aa8a2ede1f565e832a26da63058cf129dc989b3 Mon Sep 17 00:00:00 2001 From: FuelBoard Contributor Date: Tue, 13 Oct 2026 10:30:41 +0100 Subject: [PATCH] Clarify Best Value route confidence --- FuelBoard/ContentView.swift | 115 ++++++++++++++---- FuelBoard/FavouritesView.swift | 4 + FuelBoard/FuelSavingsViews.swift | 19 ++- FuelBoard/StationDetailView.swift | 19 +++ FuelBoard/StationsView.swift | 2 + .../FuelBoardSharedTests/FuelBoardTests.swift | 35 ++++++ .../FuelSavingsCalculationAuditTests.swift | 4 +- .../FuelSavingsEstimateTests.swift | 62 +++++++++- Shared/FuelSavings.swift | 89 ++++++++++++-- Shared/FuelStore.swift | 35 ++++++ 10 files changed, 338 insertions(+), 46 deletions(-) diff --git a/FuelBoard/ContentView.swift b/FuelBoard/ContentView.swift index 04c3ef1..2f50179 100644 --- a/FuelBoard/ContentView.swift +++ b/FuelBoard/ContentView.swift @@ -244,38 +244,43 @@ struct ContentView: View { 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 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, - fuel: selectedFuel, + fuel: fuel, baseline: baseline, - settings: fuelSavingsSettings, - amount: sessionFuelSavingsAmount, - journey: savingsJourneyMiles(for: station, location: location), - baselineJourney: baselineJourney + settings: settings, + amount: amount, + journey: journeys[station.id], + baselineJourney: baselineID.flatMap { journeys[$0] } ?? baselineJourney ) } } - /// The nearest station that sells the selected fuel — the Worth-the-Drive - /// comparison baseline. - /// - /// Single O(n) pass on purpose: `min(by:)` re-evaluates its closure twice - /// per comparison, and each evaluation reads the road-distance cache. With - /// the full UK list that is thousands of redundant lookups per render, all - /// on the main thread during scene creation (see `loadRoadDistances`). + /// 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 + /// 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? { guard let location else { return nil } - var bestStation: FuelStation? - var bestMiles = Double.infinity - for station in stations where station.prices[selectedFuel] != nil { - let miles = savingsFallbackMiles(for: station, location: location) - if miles < bestMiles { - bestMiles = miles - bestStation = station - } - } - return bestStation + return FuelSavingsEstimator.baselineStation( + stations, + fuel: selectedFuel, + journeyMiles: { [self] station in savingsJourneyMiles(for: station, location: location) } + ) } /// The sort the UI and the list actually use. @@ -296,11 +301,19 @@ struct ContentView: View { /// that every station the app lists can carry a saving. private func savingsJourneyMiles(for station: FuelStation, location: Coordinate) -> FuelSavingsJourney? { if let roadMeters = FuelStore.roadJourneyMeters(for: station, userLat: location.lat, userLng: location.lng) { - return FuelSavingsJourney(miles: roadMeters / FuelSavingsCalculator.metresPerMile, basis: .roadRoute) + let confidence: JourneyConfidence = FuelStore.hasMeasuredReturnRoadDistance( + for: station, userLat: location.lat, userLng: location.lng + ) ? .confirmed : .estimated + return FuelSavingsJourney( + miles: roadMeters / FuelSavingsCalculator.metresPerMile, + basis: .roadRoute, + confidence: confidence + ) } return FuelSavingsJourney( miles: savingsFallbackMiles(for: station, location: location), - basis: .straightLine + basis: .straightLine, + confidence: .estimated ) } @@ -335,6 +348,57 @@ struct ContentView: View { } #if DEBUG + private func journeyDiagnostics(for station: FuelStation, baseline: FuelStation?) -> FuelSavingsJourneyDiagnostics? { + guard let location else { return nil } + let stationDistance = FuelStore.roadDistanceDiagnostics(for: station, userLat: location.lat, userLng: location.lng) + let baselineDiagnostics = baseline.map { FuelStore.roadDistanceDiagnostics(for: $0, userLat: location.lat, userLng: location.lng) } + let journey = savingsJourneyMiles(for: station, location: location) + let baselineJourney = baseline.flatMap { savingsJourneyMiles(for: $0, location: location) } + let availability = FuelSavingsEstimator.availability( + station: station, + fuel: selectedFuel, + baseline: baseline, + settings: fuelSavingsSettings, + amount: sessionFuelSavingsAmount, + journey: journey, + baselineJourney: baselineJourney + ) + let result = availability?.result + + func miles(_ value: Double?) -> String { + value.map { String(format: "%.3f mi", $0) } ?? "—" + } + func meters(_ value: Double?) -> String { + value.map { String(format: "%.0f m", $0) } ?? "—" + } + func source(_ basis: FuelSavingsDistanceBasis) -> String { + basis == .roadRoute ? "road route" : "straight-line estimate" + } + let price = station.prices[selectedFuel].map { String(format: "%.1fp/L", $0) } ?? "—" + let baselinePrice = baseline?.prices[selectedFuel].map { String(format: "%.1fp/L", $0) } ?? "—" + let rows: [(String, String)] = [ + ("Station", "\(station.name) (\(station.id))"), + ("Displayed distance", "\(distanceUnit.format(stationDistance.displayDistanceKM)) · \(source(stationDistance.displaySource))"), + ("Outward road", meters(stationDistance.outwardMeters)), + ("Return road", stationDistance.hasMeasuredReturn ? meters(stationDistance.returnMeters) : "inferred or unavailable"), + ("Total journey", meters(stationDistance.totalJourneyMeters)), + ("Cache", stationDistance.hasValidCacheEntry ? "valid" : "missing/invalid"), + ("Baseline", baseline.map { "\($0.name) (\($0.id))" } ?? "—"), + ("Baseline distance", baselineDiagnostics.map { "\(distanceUnit.format($0.displayDistanceKM)) · \(source($0.displaySource))" } ?? "—"), + ("Baseline total", meters(baselineDiagnostics?.totalJourneyMeters)), + ("Station journey", miles(journey?.miles)), + ("Baseline journey", miles(baselineJourney?.miles)), + ("Additional journey", miles(result?.extraDistanceMiles)), + ("Confidence", result?.journeyConfidence.rawValue ?? "estimated"), + ("Fuel price", price), + ("Baseline price", baselinePrice), + ("Gross saving", result.map { FuelSavingsFormat.money($0.grossSavingGBP) } ?? "—"), + ("Travel cost", result.map { FuelSavingsFormat.money($0.extraDrivingCostGBP) } ?? "—"), + ("Net saving", result.map { FuelSavingsFormat.money($0.netSavingGBP) } ?? "—") + ] + return FuelSavingsJourneyDiagnostics(rows: rows) + } + /// Last FB-DRIVE line printed, so the ranking (which runs on every render) /// only reports when the result actually changes. private static var lastDriveLogLine: String? @@ -443,6 +507,7 @@ struct ContentView: View { fuelSavings: savingsLookup(baseline: nearestSavingsBaseline)(station), fuelSavingsAmount: sessionFuelSavingsAmount, savingsBaselineStationName: nearestSavingsBaseline?.name, + journeyDiagnostics: journeyDiagnostics(for: station, baseline: nearestSavingsBaseline), isFavourite: favouriteIDs.contains(station.id), onToggleFavourite: { toggleFavourite(station, fuel: selectedFuel) } ) @@ -1033,6 +1098,7 @@ struct ContentView: View { fuelSavingsAmount: $sessionFuelSavingsAmount, savingsForStation: savingsLookup(baseline: baselineStation), savingsBaselineStationName: baselineStation?.name, + journeyDiagnosticsForStation: { station in journeyDiagnostics(for: station, baseline: baselineStation) }, onToggleFavourite: toggleFavourite, onRefresh: { await refresh(force: true) } ) @@ -1053,6 +1119,7 @@ struct ContentView: View { fuelSavingsAmount: $sessionFuelSavingsAmount, savingsForStation: savingsLookup(baseline: baselineStation), savingsBaselineStationName: baselineStation?.name, + journeyDiagnosticsForStation: { station in journeyDiagnostics(for: station, baseline: baselineStation) }, onToggleFavourite: toggleFavourite, onReorder: reorderFavourites, onHistoryUnavailable: { if dataStatus == .live { dataStatus = .connectionProblem } }, diff --git a/FuelBoard/FavouritesView.swift b/FuelBoard/FavouritesView.swift index 252d3a1..ba5f1ba 100644 --- a/FuelBoard/FavouritesView.swift +++ b/FuelBoard/FavouritesView.swift @@ -25,6 +25,7 @@ struct FavouritesView: View { /// the same baseline and definition the Stations list uses. var savingsForStation: (FuelStation) -> FuelSavingsAvailability? = { _ in nil } var savingsBaselineStationName: String? + var journeyDiagnosticsForStation: (FuelStation) -> FuelSavingsJourneyDiagnostics? = { _ in nil } var onToggleFavourite: (FuelStation, FuelType) -> Void = { _, _ in } /// Persists a reordered favourites array (after drag-and-drop). var onReorder: ([FavouriteEntry]) -> Void = { _ in } @@ -84,6 +85,7 @@ struct FavouritesView: View { fuelSavingsAmount: Binding, savingsForStation: @escaping (FuelStation) -> FuelSavingsAvailability? = { _ in nil }, savingsBaselineStationName: String? = nil, + journeyDiagnosticsForStation: @escaping (FuelStation) -> FuelSavingsJourneyDiagnostics? = { _ in nil }, onToggleFavourite: @escaping (FuelStation, FuelType) -> Void = { _, _ in }, onReorder: @escaping ([FavouriteEntry]) -> Void = { _ in }, onHistoryUnavailable: (() -> Void)? = nil, @@ -98,6 +100,7 @@ struct FavouritesView: View { _fuelSavingsAmount = fuelSavingsAmount self.savingsForStation = savingsForStation self.savingsBaselineStationName = savingsBaselineStationName + self.journeyDiagnosticsForStation = journeyDiagnosticsForStation self.onToggleFavourite = onToggleFavourite self.onReorder = onReorder self.onHistoryUnavailable = onHistoryUnavailable @@ -219,6 +222,7 @@ struct FavouritesView: View { fuelSavings: savingsForStation(station), fuelSavingsAmount: fuelSavingsAmount, savingsBaselineStationName: savingsBaselineStationName, + journeyDiagnostics: journeyDiagnosticsForStation(station), // Every station listed here IS a favourite for the tab's // fuel, so the star starts filled; tapping it unstars. isFavourite: true, diff --git a/FuelBoard/FuelSavingsViews.swift b/FuelBoard/FuelSavingsViews.swift index f5bc283..76b12e4 100644 --- a/FuelBoard/FuelSavingsViews.swift +++ b/FuelBoard/FuelSavingsViews.swift @@ -92,9 +92,11 @@ struct FuelSavingsSummaryView: View { // it to one line, so rows stay even. VStack(alignment: .leading, spacing: 2) { capsule(for: result, compact: true) + .accessibilityLabel(accessibilityHeadline(for: result)) if let extraTravel = FuelSavingsFormat.extraTravelLabel( extraDistanceMiles: result.extraDistanceMiles, - unit: distanceUnit + unit: distanceUnit, + confidence: result.journeyConfidence ) { Text(extraTravel) .font(.caption2) @@ -114,9 +116,11 @@ struct FuelSavingsSummaryView: View { // matched neither the listing nor each other. VStack(alignment: .leading, spacing: 10) { capsule(for: result, compact: false) + .accessibilityLabel(accessibilityHeadline(for: result)) if let extraTravel = FuelSavingsFormat.extraTravelLabel( extraDistanceMiles: result.extraDistanceMiles, - unit: distanceUnit + unit: distanceUnit, + confidence: result.journeyConfidence ) { Text(extraTravel) .font(.subheadline) @@ -130,8 +134,8 @@ struct FuelSavingsSummaryView: View { Text("Compared with \(baselineStationName ?? "your nearest station") for \(amountText).").font(.caption).foregroundStyle(.secondary) LabeledContent("Fuel-price saving", value: FuelSavingsFormat.money(result.grossSavingGBP)) LabeledContent("Extra driving cost", value: FuelSavingsFormat.money(result.extraDrivingCostGBP)) - if result.distanceBasis.isEstimate { - Text("Estimated from straight-line distance — refined once the route is measured.") + if result.journeyConfidence.isEstimated { + Text("Estimated from straight-line or incomplete route data — refined once both routes are measured.") .font(.caption2) .foregroundStyle(.secondary) } @@ -157,6 +161,13 @@ struct FuelSavingsSummaryView: View { } } + private func accessibilityHeadline(for result: FuelSavingsResult) -> String { + let headline = result.headline() + return result.journeyConfidence.isEstimated && result.status != .baseline + ? "Approximate: \(headline.replacingOccurrences(of: "~", with: ""))" + : headline + } + /// The colour-coded verdict capsule — ONE implementation for the listing rows /// and the detail sheet, so the verdict reads identically in both; the sheet /// just gets the larger font and padding. diff --git a/FuelBoard/StationDetailView.swift b/FuelBoard/StationDetailView.swift index 768f231..2e0c251 100644 --- a/FuelBoard/StationDetailView.swift +++ b/FuelBoard/StationDetailView.swift @@ -2,6 +2,12 @@ import SwiftUI import MapKit import UIKit +/// DEBUG-only explanation of the Best Value numbers for one station. It carries +/// no precise user coordinates and is never persisted. +struct FuelSavingsJourneyDiagnostics { + let rows: [(label: String, value: String)] +} + /// Detail sheet for a tapped station (when Settings → Tap station = More info). /// Shows a small map preview, the address, distance from the current location, /// and a Directions button that opens Apple Maps — the same destination a direct @@ -14,6 +20,7 @@ struct StationDetailView: View { var fuelSavings: FuelSavingsAvailability? = nil var fuelSavingsAmount: FuelSavingsFillAmount = FuelSavingsSettings.defaultPreferredAmount var savingsBaselineStationName: String? = nil + var journeyDiagnostics: FuelSavingsJourneyDiagnostics? = nil /// Whether this station is starred for the fuel currently being viewed, and /// the toggle that flips it. Passed from the list that opened the sheet, so /// the star here and the star on the row are the same fuel-scoped @@ -86,6 +93,18 @@ struct StationDetailView: View { } } +#if DEBUG + if let journeyDiagnostics { + Section("Best Value Diagnostics") { + ForEach(journeyDiagnostics.rows, id: \.label) { row in + LabeledContent(row.label, value: row.value) + .font(.caption) + .monospacedDigit() + } + } + } +#endif + Section { Button { if let url = station.mapsDirectionsURL { diff --git a/FuelBoard/StationsView.swift b/FuelBoard/StationsView.swift index f96bf6c..807f63a 100644 --- a/FuelBoard/StationsView.swift +++ b/FuelBoard/StationsView.swift @@ -20,6 +20,7 @@ struct StationsView: View { @Binding var fuelSavingsAmount: FuelSavingsFillAmount var savingsForStation: (FuelStation) -> FuelSavingsAvailability? = { _ in nil } var savingsBaselineStationName: String? + var journeyDiagnosticsForStation: (FuelStation) -> FuelSavingsJourneyDiagnostics? = { _ in nil } var onToggleFavourite: (FuelStation, FuelType) -> Void = { _, _ in } var onRefresh: () async -> Void = {} @@ -317,6 +318,7 @@ struct StationsView: View { fuelSavings: savingsForStation(station), fuelSavingsAmount: fuelSavingsAmount, savingsBaselineStationName: savingsBaselineStationName, + journeyDiagnostics: journeyDiagnosticsForStation(station), isFavourite: favouriteIDs.contains(station.id), onToggleFavourite: { onToggleFavourite(station, selectedFuel) } ) diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift index 774e445..ea50973 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift @@ -688,6 +688,41 @@ final class RoadDistanceCacheTests: XCTestCase { XCTAssertEqual(km, moved.distanceKM(to: 51.6, lng2: -0.1), accuracy: 0.0001, "road value routed to the old pin leaked onto a different coordinate") } + + func testPartialRoadCacheCanMixRoadAndStraightLineDistances() { + let routed = station("routed", 51.5074, -0.1278) + let estimated = station("estimated", 51.5100, -0.1300) + FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, + entries: ["routed": .init(meters: 3200, lat: 51.5074, lng: -0.1278, returnMeters: 3600)]) + + XCTAssertEqual(FuelStore.roadDistanceMeters(for: routed, userLat: 51.6, userLng: -0.1), 3200) + XCTAssertNil(FuelStore.roadDistanceMeters(for: estimated, userLat: 51.6, userLng: -0.1)) + XCTAssertEqual(FuelStore.displayDistanceKM(station: routed, userLat: 51.6, userLng: -0.1), 3.2, accuracy: 0.0001) + XCTAssertEqual(FuelStore.displayDistanceKM(station: estimated, userLat: 51.6, userLng: -0.1), + estimated.distanceKM(to: 51.6, lng2: -0.1), + accuracy: 0.0001) + XCTAssertEqual(FuelStore.roadJourneyMeters(for: routed, userLat: 51.6, userLng: -0.1), 6800, + "outward and return road routes are combined once, not doubled") + XCTAssertTrue(FuelStore.hasMeasuredReturnRoadDistance(for: routed, userLat: 51.6, userLng: -0.1)) + XCTAssertFalse(FuelStore.hasMeasuredReturnRoadDistance(for: estimated, userLat: 51.6, userLng: -0.1)) + } + + func testLegacyOneWayRoadCacheIsUsableButNotConfirmed() { + let legacy = station("legacy", 51.5074, -0.1278) + FuelStore.saveRoadDistances(sourceLat: 51.6, sourceLng: -0.1, + entries: ["legacy": .init(meters: 3200, lat: 51.5074, lng: -0.1278)]) + + XCTAssertEqual(FuelStore.roadJourneyMeters(for: legacy, userLat: 51.6, userLng: -0.1), 6400) + XCTAssertFalse(FuelStore.hasMeasuredReturnRoadDistance(for: legacy, userLat: 51.6, userLng: -0.1), + "legacy one-way cache entries infer the return leg and must remain estimated") + let diagnostics = FuelStore.roadDistanceDiagnostics(for: legacy, userLat: 51.6, userLng: -0.1) + XCTAssertEqual(diagnostics.displaySource, .roadRoute) + XCTAssertEqual(diagnostics.outwardMeters, 3200) + XCTAssertNil(diagnostics.returnMeters) + XCTAssertEqual(diagnostics.totalJourneyMeters, 6400) + XCTAssertFalse(diagnostics.hasMeasuredReturn) + XCTAssertTrue(diagnostics.hasValidCacheEntry) + } } // MARK: - Install identity diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift index 62ac058..53c93d2 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift @@ -392,8 +392,8 @@ final class FuelSavingsCalculationAuditTests: XCTestCase { ) XCTAssertEqual(mixed?.result?.distanceBasis, .straightLine, "one unmeasured leg makes the whole estimate an estimate") - let headline = mixed?.result?.headline(compact: true) ?? "" - XCTAssertTrue(headline.hasSuffix(" (est.)"), "an estimate must be marked as one, got \(headline)") + XCTAssertEqual(mixed?.result?.journeyConfidence, .estimated) + XCTAssertEqual(mixed?.result?.headline(compact: true), "~£0.17 worse off after travel") } func testBaselineStationIsLabelledClosestStationNotBreakEven() { diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift index 07512f1..8c4195d 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift @@ -159,6 +159,60 @@ final class FuelSavingsEstimateTests: XCTestCase { XCTAssertEqual(result?.distanceBasis, .straightLine) } + func testBaselineStationUsesTheSameJourneyValuesAsSavings() { + let geographicallyNearest = station("geographically-nearest", price: 145.9) + let roadNearest = station("road-nearest", price: 146.9) + let noPrice = station("no-price", price: nil) + + let baseline = FuelSavingsEstimator.baselineStation( + [geographicallyNearest, roadNearest, noPrice], + fuel: .e10, + journeyMiles: { station in + switch station.id { + case "geographically-nearest": return FuelSavingsJourney(miles: 8.0, basis: .roadRoute) + case "road-nearest": return FuelSavingsJourney(miles: 4.0, basis: .roadRoute) + case "no-price": return FuelSavingsJourney(miles: 1.0, basis: .roadRoute) + default: return nil + } + } + ) + + XCTAssertEqual(baseline?.id, "road-nearest", + "the Best Value baseline must be the shortest usable journey, not a separate straight-line/display-distance choice") + } + + func testBaselineStationFallsBackThenChangesWhenRoadRoutesArrive() { + let stationA = station("a", price: 145.9) + let stationB = station("b", price: 146.9) + + let noRoadRoutes = FuelSavingsEstimator.baselineStation( + [stationA, stationB], + fuel: .e10, + journeyMiles: { station in + switch station.id { + case "a": return FuelSavingsJourney(miles: 2.0, basis: .straightLine) + case "b": return FuelSavingsJourney(miles: 3.0, basis: .straightLine) + default: return nil + } + } + ) + XCTAssertEqual(noRoadRoutes?.id, "a") + + let partialRoadRoutes = FuelSavingsEstimator.baselineStation( + [stationA, stationB], + fuel: .e10, + journeyMiles: { station in + switch station.id { + case "a": return FuelSavingsJourney(miles: 5.0, basis: .roadRoute) + case "b": return FuelSavingsJourney(miles: 3.0, basis: .straightLine) + default: return nil + } + } + ) + XCTAssertEqual(partialRoadRoutes?.id, "b", + "baseline must be recalculated from the latest journey values when routes arrive") + } + func testStationWithoutAPriceIsUnavailable() { let baseline = station("baseline", price: 145.9) let noPrice = station("no-price", price: nil) @@ -237,7 +291,8 @@ final class FuelSavingsEstimateTests: XCTestCase { XCTAssertEqual(result.extraDrivingCostGBP, 1.730, accuracy: 0.005) XCTAssertEqual(result.netSavingGBP, -0.334, accuracy: 0.005) XCTAssertEqual(result.status, .notWorthTheDrive) - XCTAssertEqual(result.headline(compact: true), "£0.33 worse off after travel (est.)") + XCTAssertEqual(result.journeyConfidence, .estimated) + XCTAssertEqual(result.headline(compact: true), "~£0.33 worse off after travel") } func testMeasuredRoadRoutesDropTheEstimateMarker() { @@ -255,6 +310,7 @@ final class FuelSavingsEstimateTests: XCTestCase { ) XCTAssertEqual(availability?.result?.distanceBasis, .roadRoute) + XCTAssertEqual(availability?.result?.journeyConfidence, .confirmed) XCTAssertEqual(availability?.result?.headline(compact: true), "£0.33 worse off after travel") XCTAssertEqual(availability?.result?.headline(), "£0.33 worse off after travel") } @@ -301,7 +357,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 (est.)") + XCTAssertEqual(result.headline(compact: true), "~Save £3.66 after travel") } func testNeutralResultHeadlineIsBreakEven() { @@ -420,7 +476,7 @@ final class FuelSavingsEstimateTests: XCTestCase { status: .worthTheDrive, distanceBasis: .straightLine ) - XCTAssertEqual(saving.capsuleHeadline(), "Save £1.00 after travel (est.)") + XCTAssertEqual(saving.capsuleHeadline(), "~Save £1.00 after travel") let worse = FuelSavingsResult( fillLitres: 30, diff --git a/Shared/FuelSavings.swift b/Shared/FuelSavings.swift index 9e904d6..490a8f9 100644 --- a/Shared/FuelSavings.swift +++ b/Shared/FuelSavings.swift @@ -68,6 +68,17 @@ enum FuelSavingsDistanceBasis: String, Codable, Equatable { var isEstimate: Bool { self == .straightLine } } +/// Whether a Best Value comparison is backed by complete road-route data. +enum JourneyConfidence: String, Codable, Equatable { + /// Both baseline and candidate have measured outward and return road routes. + case confirmed + /// At least one journey is straight-line, missing, failed, or using a legacy + /// one-way road cache whose return leg is inferred rather than measured. + case estimated + + var isEstimated: Bool { self == .estimated } +} + struct FuelSavingsResult: Codable, Equatable { let fillLitres: Double let grossSavingGBP: Double @@ -76,6 +87,7 @@ struct FuelSavingsResult: Codable, Equatable { let netSavingGBP: Double let status: FuelSavingsStatus let distanceBasis: FuelSavingsDistanceBasis + let journeyConfidence: JourneyConfidence init(fillLitres: Double, grossSavingGBP: Double, @@ -83,7 +95,8 @@ struct FuelSavingsResult: Codable, Equatable { extraDistanceMiles: Double, netSavingGBP: Double, status: FuelSavingsStatus, - distanceBasis: FuelSavingsDistanceBasis = .roadRoute) { + distanceBasis: FuelSavingsDistanceBasis = .roadRoute, + journeyConfidence: JourneyConfidence? = nil) { self.fillLitres = fillLitres self.grossSavingGBP = grossSavingGBP self.extraDrivingCostGBP = extraDrivingCostGBP @@ -91,6 +104,7 @@ struct FuelSavingsResult: Codable, Equatable { self.netSavingGBP = netSavingGBP self.status = status self.distanceBasis = distanceBasis + self.journeyConfidence = journeyConfidence ?? (distanceBasis.isEstimate ? .estimated : .confirmed) } } @@ -115,9 +129,17 @@ enum FuelSavingsFormat { /// always MILES while the row's distance line is one-way, so the wording says /// "more driving": the extra is an out-and-back figure and can legitimately /// exceed the one-way distance, which read as a contradiction before. - static func extraTravelLabel(extraDistanceMiles: Double, unit: DistanceUnit) -> String? { + static func extraTravelLabel(extraDistanceMiles: Double, + unit: DistanceUnit, + confidence: JourneyConfidence = .confirmed) -> String? { guard extraDistanceMiles > 0.05 else { return nil } - return "\(distance(extraDistanceMiles, unit: unit)) more driving than closest" + let distanceText = distance(extraDistanceMiles, unit: unit) + switch confidence { + case .confirmed: + return "\(distanceText) more driving than closest" + case .estimated: + return "Approx. \(distanceText) additional travel" + } } } @@ -137,26 +159,28 @@ extension FuelSavingsResult { extraDistanceMiles: 0, netSavingGBP: 0, status: .baseline, - distanceBasis: distanceBasis + distanceBasis: distanceBasis, + journeyConfidence: journeyConfidence ) } /// Row headline, e.g. "Save £0.52 after travel", "£0.31 worse off after /// travel", "Break even", "Closest station". /// - /// `compact` (list rows) also marks a straight-line estimate with "(est.)"; - /// the detail view spells that out in full instead. + /// 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. func headline(compact: Bool = false) -> String { - let estimate = (compact && distanceBasis.isEstimate) ? " (est.)" : "" + let prefix = journeyConfidence.isEstimated ? "~" : "" switch status { case .baseline: return Self.baselineHeadline case .breakEven: return Self.breakEvenHeadline case .worthTheDrive: - return "Save \(FuelSavingsFormat.money(netSavingGBP)) after travel\(estimate)" + return "\(prefix)Save \(FuelSavingsFormat.money(netSavingGBP)) after travel" case .notWorthTheDrive: - return "\(FuelSavingsFormat.money(abs(netSavingGBP))) worse off after travel\(estimate)" + return "\(prefix)\(FuelSavingsFormat.money(abs(netSavingGBP))) worse off after travel" } } @@ -174,6 +198,15 @@ extension FuelSavingsResult { struct FuelSavingsJourney: Equatable { let miles: Double let basis: FuelSavingsDistanceBasis + let confidence: JourneyConfidence + + init(miles: Double, + basis: FuelSavingsDistanceBasis, + confidence: JourneyConfidence? = nil) { + self.miles = miles + self.basis = basis + self.confidence = confidence ?? (basis.isEstimate ? .estimated : .confirmed) + } } enum FuelSavingsAvailability: Equatable { @@ -300,6 +333,7 @@ enum FuelSavingsCalculator { baselineJourneyMiles: Double, candidateJourneyMiles: Double, distanceBasis: FuelSavingsDistanceBasis = .roadRoute, + journeyConfidence: JourneyConfidence? = nil, breakEvenToleranceGBP: Double = Self.breakEvenToleranceGBP) throws -> FuelSavingsResult { guard isValidMPG(mpg) else { throw FuelSavingsError.invalidMPG } guard baselinePricePencePerLitre.isFinite, baselinePricePencePerLitre > 0, @@ -333,7 +367,8 @@ enum FuelSavingsCalculator { extraDistanceMiles: extraDistance, netSavingGBP: netSaving, status: status, - distanceBasis: distanceBasis + distanceBasis: distanceBasis, + journeyConfidence: journeyConfidence ) } @@ -362,6 +397,29 @@ enum FuelSavingsCalculator { /// Pure and injectable (no MapKit, CoreLocation or SwiftUI) so the ranking /// rules are unit tested directly. 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 + /// reference, instead of mixing a separate straight-line baseline with + /// road-route savings. + static func baselineStation(_ stations: [FuelStation], + fuel: FuelType, + journeyMiles: (FuelStation) -> FuelSavingsJourney?) -> FuelStation? { + var bestStation: FuelStation? + var bestMiles = Double.infinity + for station in stations where station.prices[fuel] != nil { + guard let journey = journeyMiles(station) else { continue } + let miles = journey.miles + guard miles.isFinite, miles >= 0 else { continue } + if miles < bestMiles { + bestMiles = miles + bestStation = station + } + } + return bestStation + } + /// Savings for a single station against `baseline`, or nil when there is /// nothing to show at all (feature off). static func availability(station: FuelStation, @@ -376,10 +434,14 @@ enum FuelSavingsEstimator { guard let baselinePrice = baseline.prices[fuel] else { return .unavailable("No comparison station") } guard let price = station.prices[fuel] else { return .unavailable("No price") } guard let journey else { return .unavailable("Distance unavailable") } - // Both legs must be measured for the estimate to count as a road-route - // one; otherwise it is reported as a straight-line estimate. + // Both journeys must be measured, complete road routes for the comparison + // to be confirmed. Legacy one-way caches keep the road-route basis for + // distance arithmetic but are still provisional because the return leg is + // inferred rather than measured. let basis: FuelSavingsDistanceBasis = (journey.basis == .roadRoute && baselineJourney.basis == .roadRoute) ? .roadRoute : .straightLine + let confidence: JourneyConfidence = + (journey.confidence == .confirmed && baselineJourney.confidence == .confirmed) ? .confirmed : .estimated do { let result = try FuelSavingsCalculator.calculate( mpg: settings.mpg, @@ -388,7 +450,8 @@ enum FuelSavingsEstimator { candidatePricePencePerLitre: price, baselineJourneyMiles: baselineJourney.miles, candidateJourneyMiles: journey.miles, - distanceBasis: basis + distanceBasis: basis, + journeyConfidence: confidence ) // The baseline compares against itself: label it as the closest // station rather than a meaningless "Break even". diff --git a/Shared/FuelStore.swift b/Shared/FuelStore.swift index aa47b74..b668775 100644 --- a/Shared/FuelStore.swift +++ b/Shared/FuelStore.swift @@ -1343,6 +1343,29 @@ struct FuelStore { roadDistanceEntry(for: station, userLat: userLat, userLng: userLng) != nil } + /// True only for current-format route cache entries with a separately + /// measured return route. Legacy one-way entries are still usable for journey + /// arithmetic (`outward × 2`) but Best Value must describe them as estimated. + static func hasMeasuredReturnRoadDistance(for station: FuelStation, userLat: Double, userLng: Double) -> Bool { + roadDistanceEntry(for: station, userLat: userLat, userLng: userLng)?.returnMeters != nil + } + + static func roadDistanceDiagnostics(for station: FuelStation, userLat: Double, userLng: Double) -> RoadDistanceDiagnostics { + let displayKM = displayDistanceKM(station: station, userLat: userLat, userLng: userLng) + let entry = roadDistanceEntry(for: station, userLat: userLat, userLng: userLng) + let outward = entry?.meters + let returnMeters = entry?.returnMeters + return RoadDistanceDiagnostics( + displayDistanceKM: displayKM, + displaySource: outward == nil ? .straightLine : .roadRoute, + outwardMeters: outward, + returnMeters: returnMeters, + totalJourneyMeters: outward.map { $0 + (returnMeters ?? $0) }, + hasMeasuredReturn: returnMeters != nil, + hasValidCacheEntry: entry != nil + ) + } + private static func roadDistanceEntry(for station: FuelStation, userLat: Double, userLng: Double) -> CachedRoadDistance? { guard let cache = loadRoadDistances(), let entry = cache.entries[station.id] else { return nil } @@ -1432,6 +1455,18 @@ struct FuelStore { } } +/// Lightweight description of the distance value shown for a station and any +/// usable road-route cache behind it. No user coordinates are included. +struct RoadDistanceDiagnostics: Equatable { + let displayDistanceKM: Double + let displaySource: FuelSavingsDistanceBasis + let outwardMeters: Double? + let returnMeters: Double? + let totalJourneyMeters: Double? + let hasMeasuredReturn: Bool + let hasValidCacheEntry: Bool +} + /// 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