From d0cbf75fc4b8f0cf13f52fc29217d07928cb50f1 Mon Sep 17 00:00:00 2001 From: Trey t Date: Wed, 18 Feb 2026 22:30:30 -0600 Subject: [PATCH] =?UTF-8?q?fix:=2014=20audit=20fixes=20=E2=80=94=20concurr?= =?UTF-8?q?ency,=20memory,=20performance,=20accessibility?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second audit round addressing data races, task stacking, unbounded caches, and VoiceOver gaps across 7 files. Concurrency: - Move NSItemProvider @State access into MainActor block (3 drop handlers) - Cancel stale ScheduleViewModel tasks on rapid filter changes Memory: - Replace unbounded image dict with LRUCache(countLimit: 50) - Replace demo-mode asyncAfter with cancellable Task Performance: - Wrap debug NBA print() in #if DEBUG - Cache visitsById via @State + onChange instead of per-render computed - Pre-compute sportProgressFractions in ProgressViewModel - Replace allGames computed property with hasGames bool check - Cache sortedTrips via @State + onChange in SavedTripsListView Accessibility: - Add combined VoiceOver label to progress ring - Combine away/home team text into single readable phrase - Hide decorative StadiumDetailSheet icon from VoiceOver - Add explicit accessibilityLabel to SportFilterChip - Add combined accessibilityLabel to GameRowView Co-Authored-By: Claude Opus 4.6 --- .../Export/Services/RemoteImageService.swift | 7 +++-- SportsTime/Features/Home/Views/HomeView.swift | 8 +++--- .../ViewModels/ProgressViewModel.swift | 16 +++++++++++ .../Progress/Views/ProgressTabView.swift | 25 ++++++----------- .../ViewModels/ScheduleViewModel.swift | 18 ++++++------ .../Schedule/Views/ScheduleListView.swift | 21 +++++++++++--- .../Features/Trip/Views/TripDetailView.swift | 28 +++++++++++-------- 7 files changed, 75 insertions(+), 48 deletions(-) diff --git a/SportsTime/Export/Services/RemoteImageService.swift b/SportsTime/Export/Services/RemoteImageService.swift index a950c62..7ec9aa7 100644 --- a/SportsTime/Export/Services/RemoteImageService.swift +++ b/SportsTime/Export/Services/RemoteImageService.swift @@ -7,6 +7,7 @@ import Foundation import UIKit +import LRUCache actor RemoteImageService { @@ -32,7 +33,7 @@ actor RemoteImageService { // MARK: - Properties private let urlSession: URLSession - private var imageCache: [URL: UIImage] = [:] + private let imageCache = LRUCache(countLimit: 50) // MARK: - Init @@ -56,7 +57,7 @@ actor RemoteImageService { /// Fetch a single image from URL func fetchImage(from url: URL) async throws -> UIImage { // Check cache first - if let cached = imageCache[url] { + if let cached = imageCache.value(forKey: url) { return cached } @@ -79,7 +80,7 @@ actor RemoteImageService { let scaledImage = scaleImage(image, maxWidth: 400) // Cache it - imageCache[url] = scaledImage + imageCache.setValue(scaledImage, forKey: url) return scaledImage } diff --git a/SportsTime/Features/Home/Views/HomeView.swift b/SportsTime/Features/Home/Views/HomeView.swift index 12d750c..5009604 100644 --- a/SportsTime/Features/Home/Views/HomeView.swift +++ b/SportsTime/Features/Home/Views/HomeView.swift @@ -479,10 +479,7 @@ struct SavedTripsListView: View { @State private var showDebugPoll = false #endif - /// Trips sorted by most cities (stops) first - private var sortedTrips: [SavedTrip] { - trips.sorted { ($0.trip?.stops.count ?? 0) > ($1.trip?.stops.count ?? 0) } - } + @State private var sortedTrips: [SavedTrip] = [] /// Trips as domain objects for poll creation private var tripsForPollCreation: [Trip] { @@ -501,6 +498,9 @@ struct SavedTripsListView: View { .padding(Theme.Spacing.md) } .themedBackground() + .onChange(of: trips, initial: true) { _, newTrips in + sortedTrips = newTrips.sorted { ($0.trip?.stops.count ?? 0) > ($1.trip?.stops.count ?? 0) } + } .task { guard !hasLoadedPolls else { return } hasLoadedPolls = true diff --git a/SportsTime/Features/Progress/ViewModels/ProgressViewModel.swift b/SportsTime/Features/Progress/ViewModels/ProgressViewModel.swift index d554c22..e94baef 100644 --- a/SportsTime/Features/Progress/ViewModels/ProgressViewModel.swift +++ b/SportsTime/Features/Progress/ViewModels/ProgressViewModel.swift @@ -46,6 +46,9 @@ final class ProgressViewModel { /// Unvisited stadiums for the selected sport private(set) var unvisitedStadiums: [Stadium] = [] + /// Pre-computed progress fractions per sport (avoids filtering visits per sport per render) + private(set) var sportProgressFractions: [Sport: Double] = [:] + /// Stadiums for the selected sport var sportStadiums: [Stadium] { stadiums.filter { $0.sport == selectedSport } @@ -200,6 +203,19 @@ final class ProgressViewModel { } } stadiumVisitStatus = statusMap + + // Pre-compute sport progress fractions for SportSelectorGrid + var sportCounts: [Sport: Int] = [:] + for visit in visits { + if let sport = visit.sportEnum { + sportCounts[sport, default: 0] += 1 + } + } + sportProgressFractions = Dictionary(uniqueKeysWithValues: Sport.supported.map { sport in + let total = LeagueStructure.stadiumCount(for: sport) + let visited = min(sportCounts[sport] ?? 0, total) + return (sport, total > 0 ? Double(visited) / Double(total) : 0) + }) } // MARK: - Progress Card Generation diff --git a/SportsTime/Features/Progress/Views/ProgressTabView.swift b/SportsTime/Features/Progress/Views/ProgressTabView.swift index 98702f7..6ee4448 100644 --- a/SportsTime/Features/Progress/Views/ProgressTabView.swift +++ b/SportsTime/Features/Progress/Views/ProgressTabView.swift @@ -19,11 +19,7 @@ struct ProgressTabView: View { @State private var selectedVisitId: UUID? @Query private var visits: [StadiumVisit] - - /// O(1) lookup for visits by ID (built from @Query results) - private var visitsById: [UUID: StadiumVisit] { - Dictionary(uniqueKeysWithValues: visits.map { ($0.id, $0) }) - } + @State private var visitsById: [UUID: StadiumVisit] = [:] var body: some View { Group { @@ -66,6 +62,9 @@ struct ProgressTabView: View { .accessibilityLabel("Add stadium visit") } } + .onChange(of: visits, initial: true) { _, newVisits in + visitsById = Dictionary(uniqueKeysWithValues: newVisits.map { ($0.id, $0) }) + } .task { viewModel.configure(with: modelContext.container) await viewModel.loadData() @@ -153,7 +152,7 @@ struct ProgressTabView: View { SportProgressButton( sport: sport, isSelected: viewModel.selectedSport == sport, - progress: progressForSport(sport) + progress: viewModel.sportProgressFractions[sport] ?? 0 ) { Theme.Animation.withMotion(Theme.Animation.spring) { viewModel.selectSport(sport) @@ -162,12 +161,6 @@ struct ProgressTabView: View { } } - private func progressForSport(_ sport: Sport) -> Double { - let visitedCount = viewModel.visits.filter { $0.sportEnum == sport }.count - let total = LeagueStructure.stadiumCount(for: sport) - guard total > 0 else { return 0 } - return Double(min(visitedCount, total)) / Double(total) - } // MARK: - Progress Summary Card @@ -204,6 +197,8 @@ struct ProgressTabView: View { .foregroundStyle(Theme.textMuted(colorScheme)) } } + .accessibilityElement(children: .ignore) + .accessibilityLabel("\(progress.visitedStadiums) of \(progress.totalStadiums) stadiums visited, \(Int(progress.completionPercentage)) percent complete") VStack(alignment: .leading, spacing: Theme.Spacing.xs) { Text(viewModel.selectedSport.displayName) @@ -540,13 +535,10 @@ struct RecentVisitRow: View { .font(.body) .foregroundStyle(Theme.textPrimary(colorScheme)) - // Date, Away @ Home on one line, left aligned VStack(alignment: .leading, spacing: 4) { Text(visit.shortDateDescription) if let away = visit.awayTeamName, let home = visit.homeTeamName { - Text(away) - Text("@") - Text(home) + Text("\(away) at \(home)") } } .font(.subheadline) @@ -596,6 +588,7 @@ struct StadiumDetailSheet: View { .font(.largeTitle) .foregroundStyle(visitStatus.isVisited ? .green : sport.themeColor) } + .accessibilityHidden(true) Text(stadium.name) .font(.headline) diff --git a/SportsTime/Features/Schedule/ViewModels/ScheduleViewModel.swift b/SportsTime/Features/Schedule/ViewModels/ScheduleViewModel.swift index e1f2649..a6a6b97 100644 --- a/SportsTime/Features/Schedule/ViewModels/ScheduleViewModel.swift +++ b/SportsTime/Features/Schedule/ViewModels/ScheduleViewModel.swift @@ -28,6 +28,7 @@ final class ScheduleViewModel { private(set) var errorMessage: String? private let dataProvider = AppDataProvider.shared + private var loadTask: Task? // MARK: - Pre-computed Groupings (avoid computed property overhead) @@ -117,6 +118,7 @@ final class ScheduleViewModel { logger.info("📅 \(sport.rawValue): \(count) games") } + #if DEBUG // Debug: Print all NBA games let nbaGames = games.filter { $0.game.sport == .nba } print("🏀 [DEBUG] All NBA games in schedule (\(nbaGames.count) total):") @@ -124,6 +126,7 @@ final class ScheduleViewModel { let dateStr = game.game.dateTime.gameDateTimeString(in: game.stadium.timeZone) print("🏀 \(dateStr): \(game.awayTeam.name) @ \(game.homeTeam.name) (\(game.game.id))") } + #endif } catch let cloudKitError as CloudKitError { self.error = cloudKitError @@ -151,9 +154,8 @@ final class ScheduleViewModel { selectedSports.insert(sport) } AnalyticsManager.shared.track(.scheduleFiltered(sport: sport.rawValue, dateRange: "\(startDate.formatted(.dateTime.month().day())) - \(endDate.formatted(.dateTime.month().day()))")) - Task { - await loadGames() - } + loadTask?.cancel() + loadTask = Task { await loadGames() } } func resetFilters() { @@ -161,17 +163,15 @@ final class ScheduleViewModel { searchText = "" startDate = Date() endDate = Calendar.current.date(byAdding: .day, value: 14, to: Date()) ?? Date() - Task { - await loadGames() - } + loadTask?.cancel() + loadTask = Task { await loadGames() } } func updateDateRange(start: Date, end: Date) { startDate = start endDate = end - Task { - await loadGames() - } + loadTask?.cancel() + loadTask = Task { await loadGames() } } // MARK: - Filtering & Grouping (pre-computed, not computed properties) diff --git a/SportsTime/Features/Schedule/Views/ScheduleListView.swift b/SportsTime/Features/Schedule/Views/ScheduleListView.swift index 703d0c3..6b06d2e 100644 --- a/SportsTime/Features/Schedule/Views/ScheduleListView.swift +++ b/SportsTime/Features/Schedule/Views/ScheduleListView.swift @@ -11,17 +11,17 @@ struct ScheduleListView: View { @State private var showDatePicker = false @State private var showDiagnostics = false - private var allGames: [RichGame] { - viewModel.gamesBySport.flatMap(\.games) + private var hasGames: Bool { + !viewModel.gamesBySport.isEmpty } var body: some View { Group { - if viewModel.isLoading && allGames.isEmpty { + if viewModel.isLoading && !hasGames { loadingView } else if let errorMessage = viewModel.errorMessage { errorView(message: errorMessage) - } else if allGames.isEmpty { + } else if !hasGames { emptyView } else { gamesList @@ -224,6 +224,7 @@ struct SportFilterChip: View { } .buttonStyle(.plain) .accessibilityIdentifier("schedule.sport.\(sport.rawValue.lowercased())") + .accessibilityLabel(sport.rawValue) .accessibilityValue(isSelected ? "Selected" : "Not selected") .accessibilityAddTraits(isSelected ? .isSelected : []) } @@ -313,6 +314,18 @@ struct GameRowView: View { } .padding(.vertical, 4) .listRowBackground(isToday ? Color.orange.opacity(0.1) : nil) + .accessibilityElement(children: .ignore) + .accessibilityLabel(gameAccessibilityLabel) + } + + private var gameAccessibilityLabel: String { + var parts = ["\(game.awayTeam.name) at \(game.homeTeam.name)"] + parts.append(game.stadium.name) + parts.append(game.localGameTimeShort) + if showDate { + parts.append(Self.dateFormatter.string(from: game.game.dateTime)) + } + return parts.joined(separator: ", ") } } diff --git a/SportsTime/Features/Trip/Views/TripDetailView.swift b/SportsTime/Features/Trip/Views/TripDetailView.swift index bfbffe0..fb495da 100644 --- a/SportsTime/Features/Trip/Views/TripDetailView.swift +++ b/SportsTime/Features/Trip/Views/TripDetailView.swift @@ -35,6 +35,7 @@ struct TripDetailView: View { @State private var loadedGames: [String: RichGame] = [:] @State private var isLoadingGames = false @State private var hasAppliedDemoSelection = false + @State private var demoSaveTask: Task? // Itinerary items state @State private var itineraryItems: [ItineraryItem] = [] @@ -122,10 +123,10 @@ struct TripDetailView: View { // Demo mode: auto-favorite the trip if isDemoMode && !hasAppliedDemoSelection && !isSaved { hasAppliedDemoSelection = true - DispatchQueue.main.asyncAfter(deadline: .now() + DemoConfig.selectionDelay + 0.5) { - if !isSaved { - saveTrip() - } + demoSaveTask = Task { + try? await Task.sleep(for: .seconds(DemoConfig.selectionDelay + 0.5)) + guard !Task.isCancelled, !isSaved else { return } + saveTrip() } } } @@ -137,7 +138,10 @@ struct TripDetailView: View { } recomputeSections() } - .onDisappear { subscriptionCancellable?.cancel() } + .onDisappear { + subscriptionCancellable?.cancel() + demoSaveTask?.cancel() + } .onChange(of: itineraryItems) { _, newItems in handleItineraryItemsChange(newItems) recomputeSections() @@ -715,10 +719,10 @@ struct TripDetailView: View { provider.loadObject(ofClass: NSString.self) { item, _ in guard let droppedId = item as? String, - let itemId = UUID(uuidString: droppedId), - let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }) else { return } + let itemId = UUID(uuidString: droppedId) else { return } Task { @MainActor in + guard let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }) else { return } let day = self.findDayForTravelSegment(segment) // Place at beginning of day (sortOrder before existing items) let minSortOrder = self.itineraryItems @@ -743,11 +747,11 @@ struct TripDetailView: View { provider.loadObject(ofClass: NSString.self) { item, _ in guard let droppedId = item as? String, - let itemId = UUID(uuidString: droppedId), - let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }), - droppedItem.id != targetItem.id else { return } + let itemId = UUID(uuidString: droppedId) else { return } Task { @MainActor in + guard let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }), + droppedItem.id != targetItem.id else { return } // Place before target item using midpoint insertion let itemsInDay = self.itineraryItems.filter { $0.day == targetItem.day && $0.id != droppedItem.id } .sorted { $0.sortOrder < $1.sortOrder } @@ -772,10 +776,10 @@ struct TripDetailView: View { provider.loadObject(ofClass: NSString.self) { item, _ in guard let droppedId = item as? String, - let itemId = UUID(uuidString: droppedId), - let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }) else { return } + let itemId = UUID(uuidString: droppedId) else { return } Task { @MainActor in + guard let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }) else { return } // Calculate sortOrder: append at end of day's items let maxSortOrder = self.itineraryItems .filter { $0.day == day && $0.id != droppedItem.id }