refactor: improve Swift 6 concurrency safety, API parsing o11y (#78)

sozercan · web-flow · commit 543edd59bbf4 · 2026-01-17T16:41:54.000-08:00
diff --git a/Core/Services/AI/AIErrorHandler.swift b/Core/Services/AI/AIErrorHandler.swift
@@ -123,16 +123,44 @@ enum AIErrorHandler {
     /// Handles specific GenerationError cases.
     private static func handleGenerationError(_ error: LanguageModelSession.GenerationError) -> AIError {
         switch error {
-        case let .exceededContextWindowSize(info):
-            self.logger.warning("Context window exceeded: \\(String(describing: info))")
+        case .exceededContextWindowSize:
+            self.logger.warning("Context window exceeded")
             return .contextWindowExceeded
 
         case .guardrailViolation:
             self.logger.warning("Content blocked by guardrails")
             return .contentBlocked
 
+        case .assetsUnavailable:
+            self.logger.warning("Model assets unavailable")
+            return .notAvailable(reason: "Model assets are not available")
+
+        case .unsupportedGuide:
+            self.logger.warning("Unsupported generation guide")
+            return .unknown(underlying: error)
+
+        case .unsupportedLanguageOrLocale:
+            self.logger.warning("Unsupported language or locale")
+            return .notAvailable(reason: "Language not supported")
+
+        case .decodingFailure:
+            self.logger.warning("Failed to decode model response")
+            return .unknown(underlying: error)
+
+        case .rateLimited:
+            self.logger.warning("Rate limited by model")
+            return .sessionBusy
+
+        case .concurrentRequests:
+            self.logger.warning("Concurrent request limit exceeded")
+            return .sessionBusy
+
+        case .refusal:
+            self.logger.warning("Model refused to respond")
+            return .contentBlocked
+
         @unknown default:
-            self.logger.error("Unknown generation error: \\(error.localizedDescription)")
+            self.logger.error("Unknown generation error: \(error.localizedDescription)")
             return .unknown(underlying: error)
         }
     }
@@ -145,9 +173,9 @@ enum AIErrorHandler {
     /// - Returns: A formatted string suitable for UI display.
     static func userMessage(for error: AIError) -> String {
         if let suggestion = error.recoverySuggestion {
-            return "\(error.localizedDescription ?? "Error"). \(suggestion)"
+            return "\(error.errorDescription ?? "Error"). \(suggestion)"
         }
-        return error.localizedDescription ?? "An error occurred"
+        return error.errorDescription ?? "An error occurred"
     }
 
     /// Logs an error with appropriate severity and returns a display message.
diff --git a/Core/Services/AI/FoundationModelsService.swift b/Core/Services/AI/FoundationModelsService.swift
@@ -193,12 +193,8 @@ final class FoundationModelsService {
 
         let session = LanguageModelSession()
 
-        do {
-            // Use the official prewarm API instead of sending a dummy prompt
-            try await session.prewarm()
-            self.logger.debug("Foundation Models prewarm completed successfully")
-        } catch {
-            self.logger.error("Failed to prewarm session: \(error.localizedDescription)")
-        }
+        // Use the official prewarm API to load model resources
+        await session.prewarm()
+        self.logger.debug("Foundation Models prewarm completed successfully")
     }
 }
diff --git a/Core/Services/API/Parsers/LyricsParser.swift b/Core/Services/API/Parsers/LyricsParser.swift
@@ -2,6 +2,8 @@ import Foundation
 
 /// Parses lyrics responses from YouTube Music API.
 enum LyricsParser {
+    private static let logger = DiagnosticsLogger.api
+
     /// Extracts the lyrics browse ID from the "next" endpoint response.
     /// - Parameter data: The response from the "next" endpoint
     /// - Returns: The browse ID for fetching lyrics, or nil if unavailable
@@ -12,6 +14,7 @@ enum LyricsParser {
               let watchNextTabbedResults = tabbedRenderer["watchNextTabbedResultsRenderer"] as? [String: Any],
               let tabs = watchNextTabbedResults["tabs"] as? [[String: Any]]
         else {
+            self.logger.debug("LyricsParser: Failed to extract lyrics browse ID structure")
             return nil
         }
 
diff --git a/Core/Services/API/Parsers/RadioQueueParser.swift b/Core/Services/API/Parsers/RadioQueueParser.swift
@@ -13,6 +13,8 @@ struct RadioQueueResult {
 
 /// Parses radio queue responses from YouTube Music API.
 enum RadioQueueParser {
+    private static let logger = DiagnosticsLogger.api
+
     /// Parses the radio queue from the "next" endpoint response.
     /// - Parameter data: The response from the "next" endpoint with a radio playlist ID
     /// - Returns: RadioQueueResult containing songs and optional continuation token
@@ -30,6 +32,7 @@ enum RadioQueueParser {
               let playlistPanelRenderer = queueContent["playlistPanelRenderer"] as? [String: Any],
               let playlistContents = playlistPanelRenderer["contents"] as? [[String: Any]]
         else {
+            self.logger.debug("RadioQueueParser: Failed to parse radio queue structure. Top keys: \(data.keys.sorted())")
             return RadioQueueResult(songs: [], continuationToken: nil)
         }
 
diff --git a/Core/Services/API/Parsers/SearchResponseParser.swift b/Core/Services/API/Parsers/SearchResponseParser.swift
@@ -2,6 +2,8 @@ import Foundation
 
 /// Parser for search responses from YouTube Music API.
 enum SearchResponseParser {
+    private static let logger = DiagnosticsLogger.api
+
     /// Parses a search response.
     static func parse(_ data: [String: Any]) -> SearchResponse {
         var songs: [Song] = []
@@ -19,6 +21,7 @@ enum SearchResponseParser {
               let sectionListRenderer = tabContent["sectionListRenderer"] as? [String: Any],
               let sectionContents = sectionListRenderer["contents"] as? [[String: Any]]
         else {
+            Self.logger.debug("SearchResponseParser: Failed to parse response structure. Top keys: \(data.keys.sorted())")
             return SearchResponse.empty
         }
 
diff --git a/Core/Services/API/Parsers/SearchSuggestionsParser.swift b/Core/Services/API/Parsers/SearchSuggestionsParser.swift
@@ -2,6 +2,8 @@ import Foundation
 
 /// Parser for search suggestions responses from YouTube Music API.
 enum SearchSuggestionsParser {
+    private static let logger = DiagnosticsLogger.api
+
     /// Parses a search suggestions response.
     /// - Parameter data: The raw JSON response from the API.
     /// - Returns: An array of search suggestions.
@@ -10,6 +12,7 @@ enum SearchSuggestionsParser {
 
         // Navigate to contents array
         guard let contents = data["contents"] as? [[String: Any]] else {
+            Self.logger.debug("SearchSuggestionsParser: No contents array found. Top keys: \(data.keys.sorted())")
             return suggestions
         }
 
diff --git a/Core/Services/API/Parsers/SongMetadataParser.swift b/Core/Services/API/Parsers/SongMetadataParser.swift
@@ -2,6 +2,8 @@ import Foundation
 
 /// Parses song metadata from YouTube Music API responses.
 enum SongMetadataParser {
+    private static let logger = DiagnosticsLogger.api
+
     /// Contains parsed menu data including feedback tokens, library status, and like status.
     struct MenuParseResult {
         var feedbackTokens: FeedbackTokens?
diff --git a/Core/Services/NetworkMonitor.swift b/Core/Services/NetworkMonitor.swift
@@ -61,11 +61,8 @@ final class NetworkMonitor {
         }
     }
 
-    // swiftformat:disable modifierOrder
-    /// NWPathMonitor accessed from deinit, which is non-isolated in Swift 6.
-    /// Using nonisolated(unsafe) since monitor.cancel() is thread-safe.
-    nonisolated(unsafe) private let monitor: NWPathMonitor
-    // swiftformat:enable modifierOrder
+    /// NWPathMonitor is Sendable and immutable, so no isolation annotation needed.
+    private let monitor: NWPathMonitor
     private let queue: DispatchQueue
     private let logger = DiagnosticsLogger.network
 
diff --git a/Core/Services/Notification/NotificationService.swift b/Core/Services/Notification/NotificationService.swift
@@ -8,8 +8,8 @@ final class NotificationService {
     private let settingsManager: SettingsManager
     private let logger = DiagnosticsLogger.notification
     // swiftformat:disable modifierOrder
-    /// Task accessed from deinit, which is non-isolated in Swift 6.
-    /// Using nonisolated(unsafe) since Task.cancel() is thread-safe.
+    /// Task for observing player changes, cancelled in deinit.
+    /// nonisolated(unsafe) required for deinit access; Swift 6.2 warning is expected.
     nonisolated(unsafe) private var observationTask: Task<Void, Never>?
     // swiftformat:enable modifierOrder
     private var lastNotifiedTrackId: String?
diff --git a/Core/Services/WebKit/WebKitManager.swift b/Core/Services/WebKit/WebKitManager.swift
@@ -46,6 +46,10 @@ enum KeychainCookieStorage {
             for (key, value) in properties {
                 stringProperties[key.rawValue] = value
             }
+            // Note: Cookie properties dictionary contains types like String, Date, Number, Bool
+            // which all support NSSecureCoding. However, using requiringSecureCoding: false here
+            // because [String: Any] doesn't directly conform to NSSecureCoding.
+            // The unarchive side uses explicit class allowlists for security.
             return try? NSKeyedArchiver.archivedData(
                 withRootObject: stringProperties,
                 requiringSecureCoding: false
@@ -54,8 +58,8 @@ enum KeychainCookieStorage {
 
         guard !cookieData.isEmpty,
               let data = try? NSKeyedArchiver.archivedData(
-                  withRootObject: cookieData,
-                  requiringSecureCoding: false
+                  withRootObject: cookieData as NSArray,
+                  requiringSecureCoding: true
               )
         else {
             Self.logger.error("Failed to serialize cookies for Keychain")
diff --git a/Core/Utilities/ImageCache.swift b/Core/Utilities/ImageCache.swift
@@ -244,7 +244,8 @@ actor ImageCache {
 
     /// Evicts oldest files until disk cache is under the size limit.
     /// Uses LRU (Least Recently Used) eviction based on file modification dates.
-    private func evictDiskCacheIfNeeded() {
+    /// Marked async to document the I/O-bound nature and satisfy actor isolation.
+    private func evictDiskCacheIfNeeded() async {
         let currentSize = self.diskCacheSize()
         guard currentSize > Self.maxDiskCacheSize else { return }
 
diff --git a/Core/ViewModels/ChartsViewModel.swift b/Core/ViewModels/ChartsViewModel.swift
@@ -18,9 +18,11 @@ final class ChartsViewModel {
     /// The API client (exposed for navigation to detail views).
     let client: any YTMusicClientProtocol
     private let logger = DiagnosticsLogger.api
-
-    /// Task for background loading of additional sections.
-    private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:disable modifierOrder
+    /// Task for background loading, cancelled in deinit.
+    /// nonisolated(unsafe) required for deinit access; Swift 6.2 warning is expected.
+    nonisolated(unsafe) private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:enable modifierOrder
 
     /// Number of background continuations loaded.
     private var continuationsLoaded = 0
@@ -32,6 +34,10 @@ final class ChartsViewModel {
         self.client = client
     }
 
+    deinit {
+        self.backgroundLoadTask?.cancel()
+    }
+
     /// Loads charts content with fast initial load.
     func load() async {
         guard self.loadingState != .loading else { return }
diff --git a/Core/ViewModels/ExploreViewModel.swift b/Core/ViewModels/ExploreViewModel.swift
@@ -18,9 +18,11 @@ final class ExploreViewModel {
     /// The API client (exposed for navigation to detail views).
     let client: any YTMusicClientProtocol
     private let logger = DiagnosticsLogger.api
-
-    /// Task for background loading of additional sections.
-    private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:disable modifierOrder
+    /// Task for background loading, cancelled in deinit.
+    /// nonisolated(unsafe) required for deinit access; Swift 6.2 warning is expected.
+    nonisolated(unsafe) private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:enable modifierOrder
 
     /// Number of background continuations loaded.
     private var continuationsLoaded = 0
@@ -32,6 +34,10 @@ final class ExploreViewModel {
         self.client = client
     }
 
+    deinit {
+        self.backgroundLoadTask?.cancel()
+    }
+
     /// Loads explore content with fast initial load.
     func load() async {
         guard self.loadingState != .loading else { return }
diff --git a/Core/ViewModels/HomeViewModel.swift b/Core/ViewModels/HomeViewModel.swift
@@ -18,9 +18,11 @@ final class HomeViewModel {
     /// The API client (exposed for navigation to detail views).
     let client: any YTMusicClientProtocol
     private let logger = DiagnosticsLogger.api
-
-    /// Task for background loading of additional sections.
-    private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:disable modifierOrder
+    /// Task for background loading, cancelled in deinit.
+    /// nonisolated(unsafe) required for deinit access; Swift 6.2 warning is expected.
+    nonisolated(unsafe) private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:enable modifierOrder
 
     /// Number of background continuations loaded.
     private var continuationsLoaded = 0
@@ -32,6 +34,10 @@ final class HomeViewModel {
         self.client = client
     }
 
+    deinit {
+        self.backgroundLoadTask?.cancel()
+    }
+
     /// Loads home content with fast initial load.
     func load() async {
         guard self.loadingState != .loading else { return }
diff --git a/Core/ViewModels/MoodsAndGenresViewModel.swift b/Core/ViewModels/MoodsAndGenresViewModel.swift
@@ -18,9 +18,11 @@ final class MoodsAndGenresViewModel {
     /// The API client (exposed for navigation to detail views).
     let client: any YTMusicClientProtocol
     private let logger = DiagnosticsLogger.api
-
-    /// Task for background loading of additional sections.
-    private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:disable modifierOrder
+    /// Task for background loading, cancelled in deinit.
+    /// nonisolated(unsafe) required for deinit access; Swift 6.2 warning is expected.
+    nonisolated(unsafe) private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:enable modifierOrder
 
     /// Number of background continuations loaded.
     private var continuationsLoaded = 0
@@ -32,6 +34,10 @@ final class MoodsAndGenresViewModel {
         self.client = client
     }
 
+    deinit {
+        self.backgroundLoadTask?.cancel()
+    }
+
     /// Loads moods and genres content with fast initial load.
     func load() async {
         guard self.loadingState != .loading else { return }
diff --git a/Core/ViewModels/NewReleasesViewModel.swift b/Core/ViewModels/NewReleasesViewModel.swift
@@ -18,9 +18,11 @@ final class NewReleasesViewModel {
     /// The API client (exposed for navigation to detail views).
     let client: any YTMusicClientProtocol
     private let logger = DiagnosticsLogger.api
-
-    /// Task for background loading of additional sections.
-    private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:disable modifierOrder
+    /// Task for background loading, cancelled in deinit.
+    /// nonisolated(unsafe) required for deinit access; Swift 6.2 warning is expected.
+    nonisolated(unsafe) private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:enable modifierOrder
 
     /// Number of background continuations loaded.
     private var continuationsLoaded = 0
@@ -32,6 +34,10 @@ final class NewReleasesViewModel {
         self.client = client
     }
 
+    deinit {
+        self.backgroundLoadTask?.cancel()
+    }
+
     /// Loads new releases content with fast initial load.
     func load() async {
         guard self.loadingState != .loading else { return }
diff --git a/Core/ViewModels/PodcastsViewModel.swift b/Core/ViewModels/PodcastsViewModel.swift
@@ -17,9 +17,11 @@ final class PodcastsViewModel {
     /// The API client (exposed for navigation to detail views).
     let client: any YTMusicClientProtocol
     private let logger = DiagnosticsLogger.api
-
-    /// Task for background loading of additional sections.
-    private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:disable modifierOrder
+    /// Task for background loading, cancelled in deinit.
+    /// nonisolated(unsafe) required for deinit access; Swift 6.2 warning is expected.
+    nonisolated(unsafe) private var backgroundLoadTask: Task<Void, Never>?
+    // swiftformat:enable modifierOrder
 
     /// Number of background continuations loaded.
     private var continuationsLoaded = 0
@@ -31,6 +33,10 @@ final class PodcastsViewModel {
         self.client = client
     }
 
+    deinit {
+        self.backgroundLoadTask?.cancel()
+    }
+
     /// Loads podcasts content with fast initial load.
     func load() async {
         guard self.loadingState != .loading else { return }
diff --git a/Core/ViewModels/SearchViewModel.swift b/Core/ViewModels/SearchViewModel.swift
@@ -96,13 +96,22 @@ final class SearchViewModel {
 
     let client: any YTMusicClientProtocol
     private let logger = DiagnosticsLogger.api
-    private var searchTask: Task<Void, Never>?
-    private var suggestionsTask: Task<Void, Never>?
+    // swiftformat:disable modifierOrder
+    /// Tasks for search operations, cancelled in deinit.
+    /// nonisolated(unsafe) required for deinit access; Swift 6.2 warning is expected.
+    nonisolated(unsafe) private var searchTask: Task<Void, Never>?
+    nonisolated(unsafe) private var suggestionsTask: Task<Void, Never>?
+    // swiftformat:enable modifierOrder
 
     init(client: any YTMusicClientProtocol) {
         self.client = client
     }
 
+    deinit {
+        searchTask?.cancel()
+        suggestionsTask?.cancel()
+    }
+
     /// Fetches search suggestions with debounce.
     func fetchSuggestions() {
         self.suggestionsTask?.cancel()
diff --git a/Tests/KasetTests/NetworkMonitorTests.swift b/Tests/KasetTests/NetworkMonitorTests.swift
diff --git a/Tools/api-explorer.swift b/Tools/api-explorer.swift
diff --git a/Views/macOS/PlayerBar.swift b/Views/macOS/PlayerBar.swift
diff --git a/docs/api-discovery.md b/docs/api-discovery.md

Original file line number	Diff line number	Diff line change
`@@ -193,12 +193,8 @@ final class FoundationModelsService {`
`193`	`193`
`194`	`194`	`let session = LanguageModelSession()`
`195`	`195`
`196`		`- do {`
`197`		`- // Use the official prewarm API instead of sending a dummy prompt`
`198`		`- try await session.prewarm()`
`199`		`- self.logger.debug("Foundation Models prewarm completed successfully")`
`200`		`- } catch {`
`201`		`- self.logger.error("Failed to prewarm session: \(error.localizedDescription)")`
`202`		`- }`
	`196`	`+ // Use the official prewarm API to load model resources`
	`197`	`+ await session.prewarm()`
	`198`	`+ self.logger.debug("Foundation Models prewarm completed successfully")`
`203`	`199`	`}`
`204`	`200`	`}`
Original file line number	Diff line number	Diff line change
`@@ -61,11 +61,8 @@ final class NetworkMonitor {`
`61`	`61`	`}`
`62`	`62`	`}`
`63`	`63`
`64`		`- // swiftformat:disable modifierOrder`
`65`		`- /// NWPathMonitor accessed from deinit, which is non-isolated in Swift 6.`
`66`		`- /// Using nonisolated(unsafe) since monitor.cancel() is thread-safe.`
`67`		`- nonisolated(unsafe) private let monitor: NWPathMonitor`
`68`		`- // swiftformat:enable modifierOrder`
	`64`	`+ /// NWPathMonitor is Sendable and immutable, so no isolation annotation needed.`
	`65`	`+ private let monitor: NWPathMonitor`
`69`	`66`	`private let queue: DispatchQueue`
`70`	`67`	`private let logger = DiagnosticsLogger.network`
`71`	`68`