From 594b9074e330e9f97ba65c1e9e5c9c0f3529f074 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 8 Sep 2026 14:47:11 -0700 Subject: [PATCH 1/3] Forget a selected airport the new dataset no longer carries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A leg's airport is remembered in the app group as a record ID, and the FAA retires airports between cycles and occasionally corrects the site number identifying one. Installing a dataset that lacks the record left the selection in place, where the widget and Siri resolved it, failed, and suggested reloading the data that had just removed it — advice that could not work, since no later cycle restores the record either. Installing now forgets a selection the incoming generation cannot resolve, along with the runway chosen on it. The check reads the generation just installed rather than the container the views still hold, which points at the dataset being replaced. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QZ3UWydnavm3g9Gs62Xirb --- SF50 Shared/Defaults.swift | 12 ++++++ .../NavDataLoaderViewModel.swift | 39 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/SF50 Shared/Defaults.swift b/SF50 Shared/Defaults.swift index a0510d3..b5f4d83 100644 --- a/SF50 Shared/Defaults.swift +++ b/SF50 Shared/Defaults.swift @@ -230,6 +230,18 @@ extension Operation { case .landing: Defaults[.landingRunway] } } + + /// Forgets the airport and the runway selected for this leg. + public func clearSelection() { + switch self { + case .takeoff: + Defaults[.takeoffAirport] = nil + Defaults[.takeoffRunway] = nil + case .landing: + Defaults[.landingAirport] = nil + Defaults[.landingRunway] = nil + } + } } // MARK: - Measurement diff --git a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift index 4d7b8c9..04490bd 100644 --- a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift +++ b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift @@ -350,10 +350,49 @@ final class NavDataLoaderViewModel: WithIdentifiableError { private func install(generation: Int) throws { try installer.install(generation: generation) clearNOTAMs() + clearSelectionsMissing(fromGeneration: generation) // The app watches the active generation and reopens its own store; doing it here as well would // race that, and leave the container the views hold pointing at the older file. } + /// Forgets a selected airport the incoming dataset no longer carries, and the runway chosen on + /// it. + /// + /// The FAA retires airports between cycles, and occasionally corrects the site number that + /// identifies one, so a selection made under an earlier dataset can name a record the new one + /// does not hold. Left in place it resolves to nothing in the widget and in Siri, which report a + /// missing airport and suggest reloading the very data that removed it. + private func clearSelectionsMissing(fromGeneration generation: Int) { + guard let context = try? navDataContext(forGeneration: generation) else { return } + for operation in Operation.allCases { + guard let recordID = operation.selectedAirportRecordID, + airportIsMissing(recordID, from: context) + else { continue } + operation.clearSelection() + logger.notice( + "Cleared the \(operation.rawValue, privacy: .public) airport, absent from the new dataset" + ) + } + } + + /// Whether the dataset lacks the airport a selection names. A failed fetch reads as present, so + /// nothing is cleared on the strength of an error. + private func airportIsMissing(_ recordID: String, from context: ModelContext) -> Bool { + do { return try findAirport(for: recordID, in: context) == nil } catch { return false } + } + + /// A context on the generation just installed. + /// + /// The container the views hold still reads the previous generation here, so this check opens the + /// new one itself. In-memory stores cannot be shared between containers, so tests and previews + /// read the container they were given. + private func navDataContext(forGeneration generation: Int) throws -> ModelContext { + guard !container.configurations.contains(where: \.isStoredInMemoryOnly) else { + return ModelContext(container) + } + return ModelContext(try AppStore.makeContainer(layout: .appGroup, generation: generation)) + } + /// Discards the NOTAMs the pilot entered against the dataset just replaced. /// /// A NOTAM carries no effective time, so one written against a previous cycle would otherwise From cdc3b05d63a3a01fd71f19c0ed595a7d258ba119 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Wed, 9 Sep 2026 08:33:53 -0700 Subject: [PATCH 2/3] Import SwiftData where RunwayPicker reads the model context The view takes `\.modelContext` from the environment without naming the module that defines it, which MemberImportVisibility reports as a warning today and will reject outright. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01471376Uhug2TYZN8RDFGtq --- SF50 TOLD/Views/Pickers/RunwayPicker.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/SF50 TOLD/Views/Pickers/RunwayPicker.swift b/SF50 TOLD/Views/Pickers/RunwayPicker.swift index ceca41b..66f7f13 100644 --- a/SF50 TOLD/Views/Pickers/RunwayPicker.swift +++ b/SF50 TOLD/Views/Pickers/RunwayPicker.swift @@ -1,4 +1,5 @@ import SF50_Shared +import SwiftData import SwiftUI struct RunwayPicker: View { From 1e9d5e9537ab6a467a965efa6592f7cf416321e6 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Wed, 9 Sep 2026 12:44:35 -0700 Subject: [PATCH 3/3] Cover the selection a new dataset drops, and the one it keeps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Installing a generation forgets a selected airport the incoming dataset cannot resolve, and leaves alone one it still carries. Only the first half is visible in a failure a pilot would report, so a prune that fired on every install would look like a fix while quietly costing the airport chosen for the flight. The check opens the generation being installed rather than the container the views hold, and that path is only taken for a store on disk — a view model given an in-memory container reads back the container it was handed. So the tests hand it a file-backed store and write the incoming dataset into the app group, where an install looks for it, carrying one airport and not the other. Each leg is asserted in both directions, so neither a prune that never fires nor one that always fires passes. `clearSelectionsMissing(fromGeneration:)` becomes internal, which is what the tests drive; the entry point above it is a download. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Vch7d69o8AYxDmprJ7syCR --- .../NavDataLoaderViewModel.swift | 4 +- .../NavDataSelectionPruningTests.swift | 176 ++++++++++++++++++ 2 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 SF50 TOLDTests/NavDataSelectionPruningTests.swift diff --git a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift index 04490bd..c3ee05d 100644 --- a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift +++ b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift @@ -362,7 +362,9 @@ final class NavDataLoaderViewModel: WithIdentifiableError { /// identifies one, so a selection made under an earlier dataset can name a record the new one /// does not hold. Left in place it resolves to nothing in the widget and in Siri, which report a /// missing airport and suggest reloading the very data that removed it. - private func clearSelectionsMissing(fromGeneration generation: Int) { + /// + /// Internal so a test can run the check against a generation on disk without downloading one. + func clearSelectionsMissing(fromGeneration generation: Int) { guard let context = try? navDataContext(forGeneration: generation) else { return } for operation in Operation.allCases { guard let recordID = operation.selectedAirportRecordID, diff --git a/SF50 TOLDTests/NavDataSelectionPruningTests.swift b/SF50 TOLDTests/NavDataSelectionPruningTests.swift new file mode 100644 index 0000000..3fe594b --- /dev/null +++ b/SF50 TOLDTests/NavDataSelectionPruningTests.swift @@ -0,0 +1,176 @@ +import Defaults +import Foundation +import SF50_Shared +import SwiftData +import Testing + +@testable import SF50_TOLD + +/// A leg's airport is remembered in the app group as a record ID, and the FAA retires airports +/// between cycles. Installing a dataset that no longer carries one has to forget the selection, +/// while a dataset that still carries it must leave the selection alone — a prune that fired on +/// every install would cost the pilot the airport chosen for the flight. +/// +/// The check opens the generation being installed rather than the container the views hold, so +/// these tests hand the view model a store on disk and write the incoming dataset where production +/// writes it. A view model given an in-memory store reads back the container it was handed, which +/// is not the path this behavior takes on a device. +@Suite(.serialized) +@MainActor +struct `Selection Pruning on Install` { + private static let carriedAirport = "CARRIED", retiredAirport = "RETIRED", + selectedRunway = "18" + + /// The generation the view model's own container reads: the dataset being replaced. + private static let supersededGeneration = 1 + + private static func airport(recordID: String) -> Airport { + .init( + recordID: recordID, + locationID: "TEST", + ICAO_ID: nil, + name: "Test Airport", + city: nil, + dataSource: .NASR, + latitude: .init(value: 0, unit: .degrees), + longitude: .init(value: 0, unit: .degrees), + elevation: .init(value: 0, unit: .feet), + variation: .init(value: 0, unit: .degrees), + timeZone: nil + ) + } + + /// Selects an airport for each leg, writes a generation carrying only ``carriedAirport``, and + /// runs `body` against a view model whose own container reads the dataset being replaced. + /// + /// The generation is written into the app group, since that is where an install looks for it. A + /// process killed before the store is removed leaves one behind, which the next launch reclaims. + /// + /// `body` must not suspend. These tests are hosted by the app, whose performance view models + /// observe these very defaults and forget an airport ID they cannot resolve in the store they + /// hold — which neither of these IDs resolves in. Nothing of theirs can interleave while the + /// body stays synchronous on the main actor. + /// + /// - Parameters: + /// - takeoff: The record ID selected for the takeoff leg. + /// - landing: The record ID selected for the landing leg. + /// - body: Runs with the view model and the generation being installed. + private static func withInstalledGeneration( + takeoff: String, + landing: String, + _ body: (NavDataLoaderViewModel, Int) throws -> Void + ) throws { + let selections = LegSelections.current(), + generation = NavDataStoreInstaller(layout: .appGroup).reserveGeneration() + defer { + selections.restore() + StoreLayout.removeStore(at: StoreLayout.appGroup.navStoreURL(generation: generation)) + } + + try write(carriedAirport, toGeneration: generation) + Defaults[.takeoffAirport] = takeoff + Defaults[.landingAirport] = landing + Defaults[.takeoffRunway] = selectedRunway + Defaults[.landingRunway] = selectedRunway + + let viewModel = NavDataLoaderViewModel(container: try supersededContainer()) + try body(viewModel, generation) + } + + /// A temporary store standing in for the dataset the install is replacing. + /// + /// It holds a dataset that reads as current, so the view model's launch-state poll settles on + /// its first pass rather than running for the rest of the test process. The store is left on + /// disk for the same reason: that poll outlives the test, and one reading a store deleted + /// underneath it turns into an error reported every half second. + private static func supersededContainer() throws -> ModelContainer { + let layout = StoreLayout( + baseDirectory: FileManager.default.temporaryDirectory + .appending(path: "SelectionPruningTests-\(UUID().uuidString)") + ) + let context = ModelContext( + try AppStore.makeWritableContainer(layout: layout, generation: supersededGeneration) + ) + context.insert(airport(recordID: carriedAirport)) + context.insert( + Cycle(dataSource: .nasr, name: "TEST", effective: .distantPast, expires: .distantFuture) + ) + try context.save() + return context.container + } + + private static func write(_ recordID: String, toGeneration generation: Int) throws { + let context = ModelContext( + try AppStore.makeWritableContainer(layout: .appGroup, generation: generation) + ) + context.insert(airport(recordID: recordID)) + try context.save() + } + + @Test + func `forgets a takeoff airport the incoming dataset dropped`() throws { + try Self.withInstalledGeneration( + takeoff: Self.retiredAirport, + landing: Self.carriedAirport + ) { viewModel, generation in + viewModel.clearSelectionsMissing(fromGeneration: generation) + + #expect(Defaults[.takeoffAirport] == nil) + #expect(Defaults[.takeoffRunway] == nil) + #expect(Defaults[.landingAirport] == Self.carriedAirport) + #expect(Defaults[.landingRunway] == Self.selectedRunway) + } + } + + @Test + func `forgets a landing airport the incoming dataset dropped`() throws { + try Self.withInstalledGeneration( + takeoff: Self.carriedAirport, + landing: Self.retiredAirport + ) { viewModel, generation in + viewModel.clearSelectionsMissing(fromGeneration: generation) + + #expect(Defaults[.landingAirport] == nil) + #expect(Defaults[.landingRunway] == nil) + #expect(Defaults[.takeoffAirport] == Self.carriedAirport) + #expect(Defaults[.takeoffRunway] == Self.selectedRunway) + } + } + + @Test + func `keeps a selection the incoming dataset still carries`() throws { + try Self.withInstalledGeneration( + takeoff: Self.carriedAirport, + landing: Self.carriedAirport + ) { viewModel, generation in + viewModel.clearSelectionsMissing(fromGeneration: generation) + + #expect(Defaults[.takeoffAirport] == Self.carriedAirport) + #expect(Defaults[.takeoffRunway] == Self.selectedRunway) + #expect(Defaults[.landingAirport] == Self.carriedAirport) + #expect(Defaults[.landingRunway] == Self.selectedRunway) + } + } + + /// The defaults a leg selection lives in, captured so one test's selections cannot reach + /// another's. + private struct LegSelections { + let takeoffAirport, takeoffRunway, landingAirport, landingRunway: String? + + static func current() -> Self { + .init( + takeoffAirport: Defaults[.takeoffAirport], + takeoffRunway: Defaults[.takeoffRunway], + landingAirport: Defaults[.landingAirport], + landingRunway: Defaults[.landingRunway] + ) + } + + func restore() { + Defaults[.takeoffAirport] = takeoffAirport + Defaults[.takeoffRunway] = takeoffRunway + Defaults[.landingAirport] = landingAirport + Defaults[.landingRunway] = landingRunway + } + } +}