diff --git a/Sources/AdaptiveSound/PlaylistsModel+DeadFiles.swift b/Sources/AdaptiveSound/PlaylistsModel+DeadFiles.swift new file mode 100644 index 0000000..f8d4a87 --- /dev/null +++ b/Sources/AdaptiveSound/PlaylistsModel+DeadFiles.swift @@ -0,0 +1,37 @@ +import Foundation +import LibraryStore + +// MARK: - PlaylistsModel + dead/missing-file resolution (F) + +/// Missing-file resolution helpers, split from `PlaylistsModel` for type-body length. The mutating +/// verbs (`removeMissingEntries` / `relocateEntry`) stay in the main file (they set `actionError`, +/// whose setter is file-private); these are stateless / read-only. +extension PlaylistsModel { + /// Off-main file-existence resolution (F): entry ids whose track resolved AND whose file exists. + /// `PlaylistEntry` / `LibraryTrackDisplay` are `Sendable`, so the `fileExists` stat loop runs + /// detached at `.utility` off the main actor (a large playlist mustn't jank the open). + static func availableEntryIDs(entries: [PlaylistEntry], + displays: [Int64: LibraryTrackDisplay]) async -> Set { + await Task.detached(priority: .utility) { + var available = Set() + for entry in entries { + if let url = displays[entry.trackID]?.url, FileManager.default.fileExists(atPath: url.path) { + available.insert(entry.id) + } + } + return available + }.value + } + + /// Count of unavailable (missing-file) entries in the open detail — drives the "Remove missing" + /// affordance + the count-line suffix. + var missingEntryCount: Int { + detail.count { !$0.isAvailable } + } + + /// Whether a scan / metadata pass / reconcile is populating the library — availability is a live + /// `fileExists` snapshot that flickers "missing" mid-scan, so bulk "Remove missing" is gated off it. + var isLibraryPopulating: Bool { + library.scanProgress != nil || library.metadataProgress != nil || library.isReconciling + } +} diff --git a/Sources/AdaptiveSound/PlaylistsModel.swift b/Sources/AdaptiveSound/PlaylistsModel.swift index 4febc6c..040e469 100644 --- a/Sources/AdaptiveSound/PlaylistsModel.swift +++ b/Sources/AdaptiveSound/PlaylistsModel.swift @@ -52,6 +52,10 @@ final class PlaylistsModel { private(set) var detailState: LoadState = .idle private var detailEpoch = 0 + /// A transient per-ACTION error (Locate / Remove-missing) shown as an alert — never routed through + /// `detailState` (a failed row-action mustn't blow the whole pane into load-error; F review). + private(set) var actionError: String? + /// The `library.libraryRevision` last folded in, so `reloadOnLibraryChange` refreshes exactly /// once per library-content change (a track deletion CASCADE-drops entries → counts move). private var lastLoadedRevision = 0 @@ -213,8 +217,14 @@ final class PlaylistsModel { do { let entries = try await store.entries(inPlaylist: id) let byID = try await store.tracksDisplay(ids: entries.map(\.trackID)) + // File-existence is a disk stat per row — resolve it OFF the main actor so a large + // playlist doesn't jank (F). A resolved track whose file is gone = "unavailable". + let availableIDs = await Self.availableEntryIDs(entries: entries, displays: byID) guard epoch == detailEpoch else { return } // a newer open/reload superseded this one - detail = entries.map { PlaylistDetailEntry(entry: $0, display: byID[$0.trackID]) } + detail = entries.map { + PlaylistDetailEntry(entry: $0, display: byID[$0.trackID], + isAvailable: availableIDs.contains($0.id)) + } detailState = detail.isEmpty ? .empty : .loaded } catch { guard epoch == detailEpoch else { return } @@ -240,10 +250,11 @@ final class PlaylistsModel { // MARK: - Play verbs (delegate to AudioViewModel; C) - /// Resolved (playable) entries of the open playlist, in order. Unresolved (missing) entries are - /// dropped — that IS skip-on-play for C; Chunk F adds the unavailable badge + Locate. + /// Playable entries of the open playlist, in order — the AVAILABLE ones (track resolved AND file + /// on disk). Unavailable (missing-file) entries are SKIPPED on play, never halting (F); the UI + /// still shows them, badged, with Locate / Remove. private var resolvedEntries: [PlaylistDetailEntry] { - detail.filter { $0.display != nil } + detail.filter(\.isAvailable) } private func playableFiles() -> [AudioFile] { @@ -322,6 +333,49 @@ final class PlaylistsModel { detailState = .failed(error.localizedDescription) } } + + // MARK: - Dead/missing-file handling (F) + + /// Remove ALL unavailable (missing-file) entries from the open playlist (bulk "Remove missing"). + /// Returns the count removed; reloads detail + tree. + @discardableResult + func removeMissingEntries() async -> Int { + guard let store, let playlistID = openPlaylistID else { return 0 } + let missing = detail.filter { !$0.isAvailable }.map(\.entry.id) + guard !missing.isEmpty else { return 0 } + do { + try await store.removeEntries(ids: missing, playlistID: playlistID) + await loadDetail(id: playlistID) + await loadTree() + return missing.count + } catch { + actionError = "Couldn’t remove the missing tracks: \(error.localizedDescription)" + return 0 + } + } + + func clearActionError() { + actionError = nil + } + + /// Locate: re-point a missing entry's TRACK to a user-chosen file, preserving the track id (so + /// every playlist referencing it is fixed at once), then reload so it resolves. Reuses the + /// id-preserving `moveTrack` seam; the file becomes loose (`folder_id = nil`) until a rescan + /// re-associates it — a full metadata re-scan is out of F scope. A URL already owned by another + /// track throws `URLConflict` (caught → surfaced via `detailState`). + func relocateEntry(_ entryID: Int64, to url: URL) async { + guard let store, let playlistID = openPlaylistID, + let trackID = detail.first(where: { $0.id == entryID })?.entry.trackID else { return } + do { + try await store.moveTrack(id: trackID, newURL: url, newFolderID: nil, newRelativePath: "") + await loadDetail(id: playlistID) + } catch is URLConflict { + // The picked file is already another track's — a per-row failure, NOT a pane-wide error. + actionError = "That file is already in your library under a different track." + } catch { + actionError = "Couldn’t relocate the file: \(error.localizedDescription)" + } + } } // MARK: - Detail row model @@ -333,6 +387,10 @@ final class PlaylistsModel { struct PlaylistDetailEntry: Identifiable { let entry: PlaylistEntry let display: LibraryTrackDisplay? + /// True when the track resolved AND its file exists on disk (S10.3 F). False = "unavailable": + /// the file moved/was deleted (a loose file gone, or a move not yet reconciled) or the track row + /// is absent — the row is badged, SKIPPED on play (never halts), and offered Locate / Remove. + let isAvailable: Bool var id: Int64 { entry.id diff --git a/Sources/AdaptiveSound/UI/Library/SongsTable.swift b/Sources/AdaptiveSound/UI/Library/SongsTable.swift index 9036cd1..94afc91 100644 --- a/Sources/AdaptiveSound/UI/Library/SongsTable.swift +++ b/Sources/AdaptiveSound/UI/Library/SongsTable.swift @@ -13,7 +13,6 @@ import SwiftUI /// (show/hide/reorder/resize + persistence, §11) via `columnCustomization`. struct SongsTable: View { @Environment(LibraryBrowseModel.self) private var model - @Environment(PlaylistsModel.self) private var playlists @State private var selection = Set() /// The track whose Info popover is open (mirrors the album-detail affordance). @State private var infoTarget: LibraryTrackDisplay? diff --git a/Sources/AdaptiveSound/UI/Playlist/PlaylistDetailView+Actions.swift b/Sources/AdaptiveSound/UI/Playlist/PlaylistDetailView+Actions.swift new file mode 100644 index 0000000..6d3d2df --- /dev/null +++ b/Sources/AdaptiveSound/UI/Playlist/PlaylistDetailView+Actions.swift @@ -0,0 +1,94 @@ +import SwiftUI + +// MARK: - PlaylistDetailView + actions (split out for type-body length) + +/// Play / reorder / keyboard-nav / restore-toast actions for the playlist detail, plus the +/// missing-file (F) helpers. A same-type extension, split from `PlaylistDetailView` for type-body +/// length; reaches its `internal` state. +extension PlaylistDetailView { + /// Locate / Remove for a missing entry — shared by the badge menu, the row context menu, and the + /// VoiceOver rotor so all three stay in sync. + @ViewBuilder + func missingRowActions(_ row: PlaylistDetailEntry) -> some View { + Button("Locate…") { locatingEntryID = row.id } + Button("Remove from Playlist", role: .destructive) { + Task { await model.removeEntry(row.id) } + } + } + + /// The "N file(s) missing" help string (real pluralization) for the header affordance. + var missingHelp: String { + let count = model.missingEntryCount + return "\(count) \(count == 1 ? "file" : "files") missing" + } + + /// Play the playlist (replace queue, undoable), optionally from a tapped row, and raise the + /// transient restore-queue affordance — ONLY if a replace actually happened (an all-unavailable + /// playlist no-ops, and must not resurface a stale toast from an earlier real Play). + func playNow(startingAt entryID: Int64? = nil) { + if model.playPlaylist(startingAt: entryID) { raiseRestoreToast() } + } + + func raiseRestoreToast() { + let token = (restoreToastToken ?? 0) &+ 1 + restoreToastToken = token + restoreDismissTask?.cancel() + restoreDismissTask = Task { [token] in + try? await Task.sleep(for: .seconds(6)) + guard !Task.isCancelled, restoreToastToken == token else { return } + restoreToastToken = nil + } + } + + func dismissRestoreToast() { + restoreDismissTask?.cancel() + restoreToastToken = nil + } + + /// Move `fromID` to land AT `toEntryID`'s row and persist the new order — matching the queue's + /// convention (`AudioViewModel.moveByDrop`): dragging DOWN inserts AFTER the target, UP before + /// it, so the last slot is reachable and the two lists behave identically. + func moveEntry(fromID: Int64, toEntryID: Int64) -> Bool { + var ids = model.detail.map(\.id) + guard let from = ids.firstIndex(of: fromID), let to = ids.firstIndex(of: toEntryID), + from != to else { return false } + ids.move(fromOffsets: IndexSet(integer: from), toOffset: from < to ? to + 1 : to) + Task { await model.reorderEntries(ids) } + return true + } + + /// ↑/↓ traverse only the AVAILABLE (playable) rows — the "unavailable" (missing-file) rows are + /// non-interactive except via their context menu (Locate / Remove), so selection skips them (F). + func moveSelection(by delta: Int) -> KeyPress.Result { + let ids = model.detail.filter(\.isAvailable).map(\.id) + guard !ids.isEmpty else { return .ignored } + guard let current = selectedEntryID, let index = ids.firstIndex(of: current) else { + selectedEntryID = ids.first + return .handled + } + let next = index + delta + guard next >= 0, next < ids.count else { return .ignored } + selectedEntryID = ids[next] + return .handled + } + + func playSelected() -> KeyPress.Result { + guard let id = selectedEntryID else { return .ignored } + playNow(startingAt: id) + return .handled + } + + func removeSelected() -> KeyPress.Result { + guard let id = selectedEntryID else { return .ignored } + // Pre-select the neighbor (next, else previous) so selection lands there — not back at the + // top — once the async remove + reload lands. Use the SAME available-row set `moveSelection` + // traverses, so the neighbor is a selectable row (not an unavailable placeholder). + let ids = model.detail.filter(\.isAvailable).map(\.id) + if let index = ids.firstIndex(of: id) { + selectedEntryID = index + 1 < ids.count ? ids[index + 1] + : (index - 1 >= 0 ? ids[index - 1] : nil) + } + Task { await model.removeEntry(id) } + return .handled + } +} diff --git a/Sources/AdaptiveSound/UI/Playlist/PlaylistDetailView.swift b/Sources/AdaptiveSound/UI/Playlist/PlaylistDetailView.swift index 3d7e611..5f2fb93 100644 --- a/Sources/AdaptiveSound/UI/Playlist/PlaylistDetailView.swift +++ b/Sources/AdaptiveSound/UI/Playlist/PlaylistDetailView.swift @@ -1,5 +1,6 @@ import LibraryStore import SwiftUI +import UniformTypeIdentifiers // MARK: - Playlist detail (S10.3 — the open-playlist content pane) @@ -8,20 +9,27 @@ import SwiftUI /// (with a `PlaylistEntryDragItem` grip). Chunk C: the three play verbs (Play replaces the queue /// with a one-level restore-queue undo; Play Next / Add to Queue), tap-to-play-from-row, grip-drag /// reorder, remove, and ↑/↓/Return/⌫ keys — the same scaffold the queue's `PlaylistItemList` uses. -/// Chunk F renders the unavailable state for an entry whose file moved/was deleted. +/// Chunk F renders the unavailable state for an entry whose file moved/was deleted. Several members +/// are `internal` (not `private`) so the same-type `PlaylistDetailView+Actions` extension (split out +/// for type-body length) can reach them. struct PlaylistDetailView: View { let playlistID: Int64 - @Environment(PlaylistsModel.self) private var model + @Environment(PlaylistsModel.self) var model /// Keyboard-selected row (a ScrollView/LazyVStack doesn't own key focus like a `List`). - @State private var selectedEntryID: Int64? + @State var selectedEntryID: Int64? /// The entry a reorder drag is hovering over (drop-target border). Nil when no drag is active. @State private var dropTargetEntryID: Int64? @FocusState private var listFocused: Bool /// Transient "Restore previous queue" affordance after a Play-replace; the token re-triggers the /// auto-dismiss even on a repeated Play. - @State private var restoreToastToken: Int? - @State private var restoreDismissTask: Task? + @State var restoreToastToken: Int? + @State var restoreDismissTask: Task? + /// The entry being re-pointed via Locate… (drives the file importer); nil when closed (F). + /// `internal` so the `+Actions` extension's `missingRowActions` can set it. + @State var locatingEntryID: Int64? + /// Confirm before the irreversible bulk "Remove missing" (no undo — F review). + @State private var confirmingRemoveMissing = false /// Track-number column sized to the widest index, so a 3-digit position never wraps (queue idiom). private var numberColumnWidth: CGFloat { @@ -38,6 +46,37 @@ struct PlaylistDetailView: View { .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top) .background(DesignSystem.Color.window) .overlay(alignment: .bottom) { restoreToast } + // Locate… (F): pick the moved file → re-point the track (id preserved) → it resolves. + .fileImporter( + isPresented: Binding(get: { locatingEntryID != nil }, + set: { if !$0 { locatingEntryID = nil } }), + allowedContentTypes: [.audio] + ) { result in + if case let .success(url) = result, let id = locatingEntryID { + Task { await model.relocateEntry(id, to: url) } + } + locatingEntryID = nil + } + // A per-action failure (Locate URL-conflict, remove-missing) — a transient alert, NOT the + // pane-wide load-error state (F review). + .alert("Couldn’t Complete That", isPresented: Binding( + get: { model.actionError != nil }, + set: { if !$0 { model.clearActionError() } } + )) { + Button("OK", role: .cancel) {} + } message: { + Text(model.actionError ?? "") + } + // Bulk remove is irreversible → confirm (F review). + .confirmationDialog( + "Remove \(model.missingEntryCount) missing \(model.missingEntryCount == 1 ? "track" : "tracks")?", + isPresented: $confirmingRemoveMissing, titleVisibility: .visible + ) { + Button("Remove Missing", role: .destructive) { Task { await model.removeMissingEntries() } } + Button("Cancel", role: .cancel) {} + } message: { + Text("The playlist entries for files that are missing from disk will be removed. This can’t be undone.") + } // Reload whenever the selected playlist changes (a new sidebar selection reuses this view). .task(id: playlistID) { await model.loadDetail(id: playlistID) } } @@ -82,13 +121,29 @@ struct PlaylistDetailView: View { } .labelStyle(.iconOnly) .help("Add to Queue") + // Bulk "Remove missing" (F) — only when some entries' files are gone. Disabled during a + // scan/reconcile (availability is unreliable then — files flicker to "missing"). + if model.missingEntryCount > 0 { + Menu { + Button("Remove \(model.missingEntryCount) Missing", systemImage: "trash", role: .destructive) { + confirmingRemoveMissing = true + } + .disabled(model.isLibraryPopulating) + } label: { + Label("More", systemImage: "ellipsis.circle") + } + .labelStyle(.iconOnly) + .help(missingHelp) + } } .disabled(model.detailState != .loaded) } private var countLine: String { - let count = model.detail.count - return "\(count.formatted(.number)) \(count == 1 ? "track" : "tracks")" + let total = model.detail.count + let base = "\(total.formatted(.number)) \(total == 1 ? "track" : "tracks")" + let missing = model.missingEntryCount + return missing > 0 ? "\(base) · \(missing) unavailable" : base } // MARK: Content @@ -137,7 +192,7 @@ struct PlaylistDetailView: View { @ViewBuilder private func detailRow(index: Int, row: PlaylistDetailEntry) -> some View { - if let display = row.display { + if row.isAvailable, let display = row.display { PlaylistItemRow( file: AudioFile(display), index: index, @@ -170,24 +225,49 @@ struct PlaylistDetailView: View { } } } else { - unavailableRow(index: index) + unavailableRow(index: index, row: row) } } - private func unavailableRow(index: Int) -> some View { + /// A missing-file entry (F): its metadata dimmed, with a trailing warning-badge MENU (Locate / + /// Remove) — a visible, click-and-keyboard-reachable affordance, not just right-click (review). + /// Not tappable-to-play (skipped on play). VoiceOver reads it as one element + the same actions. + private func unavailableRow(index: Int, row: PlaylistDetailEntry) -> some View { HStack(spacing: 12) { Text(index + 1, format: .number.grouping(.never)) .font(DesignSystem.Font.monoSmall) .foregroundStyle(DesignSystem.Color.labelTertiary) .frame(width: numberColumnWidth, alignment: .trailing) - Text("Track unavailable") - .font(DesignSystem.Font.body) - .foregroundStyle(DesignSystem.Color.labelTertiary) + VStack(alignment: .leading, spacing: 1) { + Text(row.display?.title ?? "Unknown Track") + .font(DesignSystem.Font.body) + .foregroundStyle(DesignSystem.Color.labelSecondary) + .lineLimit(1) + if let artist = row.display?.artistName, !artist.isEmpty { + Text(artist) + .font(DesignSystem.Font.caption) + .foregroundStyle(DesignSystem.Color.labelTertiary) + .lineLimit(1) + } + } Spacer() + Menu { + missingRowActions(row) + } label: { + Image(systemName: "exclamationmark.triangle.fill").foregroundStyle(.orange) + } + .menuStyle(.borderlessButton) + .fixedSize() + .help("File missing — moved or deleted. Locate… to re-point it, or remove it.") } .padding(.vertical, DesignSystem.Spacing.xSmall) .padding(.horizontal, DesignSystem.Spacing.small) .frame(maxWidth: .infinity, alignment: .leading) + .contentShape(Rectangle()) + .contextMenu { missingRowActions(row) } + .accessibilityElement(children: .combine) + .accessibilityLabel("\(row.display?.title ?? "Unknown Track"), unavailable — file missing") + .accessibilityActions { missingRowActions(row) } } // MARK: Restore-queue undo toast @@ -213,76 +293,4 @@ struct PlaylistDetailView: View { .transition(.opacity) } } - - // MARK: Actions - - /// Play the playlist (replace queue, undoable), optionally from a tapped row, and raise the - /// transient restore-queue affordance — ONLY if a replace actually happened (an all-unavailable - /// playlist no-ops, and must not resurface a stale toast from an earlier real Play). - private func playNow(startingAt entryID: Int64? = nil) { - if model.playPlaylist(startingAt: entryID) { raiseRestoreToast() } - } - - private func raiseRestoreToast() { - let token = (restoreToastToken ?? 0) &+ 1 - restoreToastToken = token - restoreDismissTask?.cancel() - restoreDismissTask = Task { [token] in - try? await Task.sleep(for: .seconds(6)) - guard !Task.isCancelled, restoreToastToken == token else { return } - restoreToastToken = nil - } - } - - private func dismissRestoreToast() { - restoreDismissTask?.cancel() - restoreToastToken = nil - } - - /// Move `fromID` to land AT `toEntryID`'s row and persist the new order — matching the queue's - /// convention (`AudioViewModel.moveByDrop`): dragging DOWN inserts AFTER the target, UP before - /// it, so the last slot is reachable and the two lists behave identically. - private func moveEntry(fromID: Int64, toEntryID: Int64) -> Bool { - var ids = model.detail.map(\.id) - guard let from = ids.firstIndex(of: fromID), let to = ids.firstIndex(of: toEntryID), - from != to else { return false } - ids.move(fromOffsets: IndexSet(integer: from), toOffset: from < to ? to + 1 : to) - Task { await model.reorderEntries(ids) } - return true - } - - /// ↑/↓ traverse only the RESOLVED (playable) rows — the "unavailable" placeholders are - /// non-interactive until Chunk F, so selection never lands on one. - private func moveSelection(by delta: Int) -> KeyPress.Result { - let ids = model.detail.filter { $0.display != nil }.map(\.id) - guard !ids.isEmpty else { return .ignored } - guard let current = selectedEntryID, let index = ids.firstIndex(of: current) else { - selectedEntryID = ids.first - return .handled - } - let next = index + delta - guard next >= 0, next < ids.count else { return .ignored } - selectedEntryID = ids[next] - return .handled - } - - private func playSelected() -> KeyPress.Result { - guard let id = selectedEntryID else { return .ignored } - playNow(startingAt: id) - return .handled - } - - private func removeSelected() -> KeyPress.Result { - guard let id = selectedEntryID else { return .ignored } - // Pre-select the neighbor (next, else previous) so selection lands there — not back at the - // top — once the async remove + reload lands. Use the SAME resolved-row set `moveSelection` - // traverses, so the neighbor is a selectable row (not an unavailable placeholder). - let ids = model.detail.filter { $0.display != nil }.map(\.id) - if let index = ids.firstIndex(of: id) { - selectedEntryID = index + 1 < ids.count ? ids[index + 1] - : (index - 1 >= 0 ? ids[index - 1] : nil) - } - Task { await model.removeEntry(id) } - return .handled - } } diff --git a/Sources/LibraryStore/LibraryStore+Playlists.swift b/Sources/LibraryStore/LibraryStore+Playlists.swift index 0eb3c2f..cf999d5 100644 --- a/Sources/LibraryStore/LibraryStore+Playlists.swift +++ b/Sources/LibraryStore/LibraryStore+Playlists.swift @@ -242,6 +242,16 @@ public extension LibraryStore { try await dbWriter.read { db in try Int(Int64.fetchOne(db, sql: Self.selectPlaylistCountSQL) ?? 0) } } + /// EXPLAIN QUERY PLAN rows for the two playlist reads (list-with-count + entries) — the scale + /// tripwire (G / design §5): `playlist_entries` must be reached via `idx_playlist_entries_playlist`, + /// never a full table scan (a long playlist / hundreds of playlists would otherwise degrade). + func explainPlaylistReadsPlan() async throws -> [String] { + try await dbWriter.read { db in + try Self.collectQueryPlan(db, Self.selectPlaylistsSQL) + + Self.collectQueryPlan(db, Self.selectEntriesSQL) + } + } + func entries(inPlaylist id: Int64) async throws -> [PlaylistEntry] { try await dbWriter.read { db in try PlaylistEntry.fetchAll(db, sql: Self.selectEntriesSQL, arguments: [id]) } } diff --git a/Sources/VerifyLibraryStore/ChecksMigrationConvergence.swift b/Sources/VerifyLibraryStore/ChecksMigrationConvergence.swift index 57bf712..f893283 100644 --- a/Sources/VerifyLibraryStore/ChecksMigrationConvergence.swift +++ b/Sources/VerifyLibraryStore/ChecksMigrationConvergence.swift @@ -11,7 +11,6 @@ import Foundation import GRDB -import LibraryStore // MARK: - Registration diff --git a/Sources/VerifyLibraryStore/ChecksPlaylistScan.swift b/Sources/VerifyLibraryStore/ChecksPlaylistScan.swift new file mode 100644 index 0000000..f6a28e3 --- /dev/null +++ b/Sources/VerifyLibraryStore/ChecksPlaylistScan.swift @@ -0,0 +1,147 @@ +// ChecksPlaylistScan — S10.3 Chunk G: playlist ⇄ scanner/scale invariants. US-PLIST-08 (membership +// survives a real-scanner move), reorder isolation, the EXPLAIN scale tripwire, and a playlist write +// concurrent with a real scan. Registered via playlistScanCheckCases() in main.swift. + +import Foundation +import LibraryScan +import LibraryStore + +// MARK: - Registration + +func playlistScanCheckCases() -> [CheckCase] { + [ + CheckCase(label: "pl-move-membership-survives", run: checkPlaylistMoveMembershipSurvives), + CheckCase(label: "pl-reorder-isolation", run: checkPlaylistReorderIsolation), + CheckCase(label: "pl-explain-plan", run: checkPlaylistExplainPlan), + CheckCase(label: "pl-write-during-scan", run: checkPlaylistWriteDuringScan), + ] +} + +// MARK: - US-PLIST-08 + +/// pl-move-membership-survives: a track's file moves on disk; the REAL scanner move-matches it +/// (preserving `tracks.id`), so a playlist entry referencing it by id is still intact after reconcile. +func checkPlaylistMoveMembershipSurvives(number: Int, url: URL) async -> Bool { + do { + let store = try await LibraryStore(url: url, appBuild: "verify") + let root = try ScanFixtureBuilder.makeCaseRoot("pl-move-survive") + let oldURL = try ScanFixtureBuilder.writeFile(at: root, subdirs: ["A"], fileName: "keeper.flac", byteCount: 32) + let folderID = try await store.addRoot(root) + _ = try await LibraryScanner().scan(root: root, folderID: folderID, into: store) + guard let id = try await store.track(url: oldURL)?.id else { + printFail(number, "pl-move-survive: row missing after first scan"); return false + } + let playlistID = try await store.createPlaylist(name: "Keepers") + _ = try await store.appendEntry(playlistID: playlistID, trackID: id) + + // Move the file, then reconcile through the real scanner (move-match keeps the id). + let destDir = try ScanFixtureBuilder.makeDirectory(at: root, ["B", "C"]) + let newURL = destDir.appendingPathComponent("keeper.flac", isDirectory: false) + try FileManager.default.moveItem(at: oldURL, to: newURL) + _ = try await LibraryScanner().scan(root: root, folderID: folderID, into: store) + + guard let moved = try await store.track(url: newURL), moved.id == id else { + printFail(number, "pl-move-survive: track id NOT preserved across the move"); return false + } + let entries = try await store.entries(inPlaylist: playlistID) + guard entries.count == 1, entries[0].trackID == id else { + printFail(number, "pl-move-survive: membership LOST across the move (entries=\(entries.map(\.trackID)))") + return false + } + printPass(number, "US-PLIST-08: a playlist entry SURVIVES a Finder move of its file — move-match " + + "keeps tracks.id, so membership (by id) is intact after reconcile") + return true + } catch { printFail(number, "pl-move-membership-survives threw: \(error)"); return false } +} + +// MARK: - Reorder isolation + +/// pl-reorder-isolation: reordering one playlist leaves every OTHER playlist's order untouched +/// (positions are per-playlist; the renumber is scoped by `playlist_id`). +func checkPlaylistReorderIsolation(number: Int, url: URL) async -> Bool { + do { + let seeded = try await seedTracks(url, root: "/M/RI", + paths: ["/M/RI/a.flac", "/M/RI/b.flac", "/M/RI/c.flac"]) + let store = seeded.store + let t = seeded.trackIDs + let p1 = try await store.createPlaylist(name: "One") + let p2 = try await store.createPlaylist(name: "Two") + _ = try await store.appendEntries(playlistID: p1, trackIDs: t) + _ = try await store.appendEntries(playlistID: p2, trackIDs: t) + let p2Before = try await store.entries(inPlaylist: p2).map(\.trackID) + + // Reverse P1's order — must not touch P2. + let p1Entries = try await store.entries(inPlaylist: p1) + try await store.reorderPlaylist(id: p1, entryIDsInOrder: p1Entries.map(\.id).reversed()) + let p1After = try await store.entries(inPlaylist: p1).map(\.trackID) + let p2After = try await store.entries(inPlaylist: p2).map(\.trackID) + guard p1After == Array(t.reversed()), p2After == p2Before else { + printFail(number, "pl-reorder-isolation: reordering P1 leaked into P2 (p1=\(p1After) p2=\(p2After))") + return false + } + printPass(number, "pl-reorder-isolation: reordering one playlist leaves every other playlist's " + + "order untouched (per-playlist renumber)") + return true + } catch { printFail(number, "pl-reorder-isolation threw: \(error)"); return false } +} + +// MARK: - EXPLAIN scale tripwire + +/// pl-explain-plan: the playlist list + entries reads must be INDEX-driven — `playlist_entries` is +/// reached via `idx_playlist_entries_playlist`, never a full table scan (scale: long playlists). +func checkPlaylistExplainPlan(number: Int, url: URL) async -> Bool { + do { + let seeded = try await seedTracks(url, root: "/M/EP", paths: ["/M/EP/a.flac", "/M/EP/b.flac"]) + let store = seeded.store + let playlistID = try await store.createPlaylist(name: "P") + _ = try await store.appendEntries(playlistID: playlistID, trackIDs: seeded.trackIDs) + let plan = try await store.explainPlaylistReadsPlan() + + let fullScan = plan.contains { $0.contains("SCAN playlist_entries") && !$0.contains("USING") } + guard !fullScan else { + printFail(number, "pl-explain-plan: playlist_entries is FULL-SCANNED: \(plan)"); return false + } + guard plan.contains(where: { $0.contains("idx_playlist_entries_playlist") }) else { + printFail(number, "pl-explain-plan: idx_playlist_entries_playlist not used: \(plan)"); return false + } + printPass(number, "pl-explain-plan: the playlist list + entries reads use " + + "idx_playlist_entries_playlist — never a full SCAN of playlist_entries") + return true + } catch { printFail(number, "pl-explain-plan threw: \(error)"); return false } +} + +// MARK: - Write during scan + +/// pl-write-during-scan: a playlist write concurrent with a REAL scan completes without deadlock or +/// corruption — the single serialized `DatabaseWriter` serializes them, and the entry lands. +func checkPlaylistWriteDuringScan(number: Int, url: URL) async -> Bool { + do { + let store = try await LibraryStore(url: url, appBuild: "verify") + let root = try ScanFixtureBuilder.makeCaseRoot("pl-write-scan") + for index in 0 ..< 40 { + _ = try ScanFixtureBuilder.writeFile(at: root, fileName: "s\(index).flac", byteCount: 16) + } + let folderID = try await store.addRoot(root) + _ = try await LibraryScanner().scan(root: root, folderID: folderID, into: store) + guard let someID = try await store.allTracksDisplay(sortedBy: .artistAlbumTrack, limit: 1).first?.id else { + printFail(number, "pl-write-scan: no track after the seed scan"); return false + } + let playlistID = try await store.createPlaylist(name: "Concurrent") + + // Add more files so the second scan has work, then run it CONCURRENTLY with a playlist write. + for index in 40 ..< 110 { + _ = try ScanFixtureBuilder.writeFile(at: root, fileName: "s\(index).flac", byteCount: 16) + } + async let scan = LibraryScanner().scan(root: root, folderID: folderID, into: store) + async let write = store.appendEntry(playlistID: playlistID, trackID: someID) + _ = try await (scan, write) + + let entries = try await store.entries(inPlaylist: playlistID) + guard entries.contains(where: { $0.trackID == someID }) else { + printFail(number, "pl-write-scan: the concurrent playlist write was lost"); return false + } + printPass(number, "pl-write-during-scan: a playlist write concurrent with a real scan completes " + + "without deadlock/corruption — the serialized writer keeps both safe") + return true + } catch { printFail(number, "pl-write-during-scan threw: \(error)"); return false } +} diff --git a/Sources/VerifyLibraryStore/main.swift b/Sources/VerifyLibraryStore/main.swift index 260ad06..6ddeb3d 100644 --- a/Sources/VerifyLibraryStore/main.swift +++ b/Sources/VerifyLibraryStore/main.swift @@ -180,6 +180,7 @@ func allCheckCases() -> [CheckCase] { + reachabilityCheckCases() + browseReadsCheckCases() + searchCheckCases() + songsSortCheckCases() + decodeCoverageCheckCases() + hardeningCheckCases() + playlistSpineCheckCases() + playlistFolderCheckCases() + + playlistScanCheckCases() + migrationConvergenceCheckCases() } diff --git a/docs/sprints/s10-3-playlists-ux-design.md b/docs/sprints/s10-3-playlists-ux-design.md index d32ea9c..da63d5e 100644 --- a/docs/sprints/s10-3-playlists-ux-design.md +++ b/docs/sprints/s10-3-playlists-ux-design.md @@ -59,11 +59,11 @@ Adjacency-list `playlist_folders(id, parent_id NULL self-ref, name, position, cr - **A0** — ✅ DONE: `library.sqlite3` → `eraseDatabaseOnSchemaChange = false` (additive-only, protects playlists + track user-state) + the `additive-preserve-schema-bump` green-gate. *(The separate-store detour was built then reverted per the QA break-it — §0.)* - **A** — ✅ DONE (7392b1e): `playlist_folders` additive v5 migration + folder DAO (CRUD/reparent/cycle-guard/cascade-delete-with-subtree-snapshot for undo). (Deleted-track cleanup is the same-file FK cascade.) `PlaylistsModel` moved to B (kept with its UI consumer so it isn't dead code under Periphery). - **B** — ✅ DONE (8277ab8): `PlaylistsModel` peer + composition wiring + sidebar Playlists section (one ScrollView/LazyVStack, unified selection enum, built-in filtered, scroll, select→detail, create/inline-rename/delete) + read-only `PlaylistDetailView`. Plus the A+B review-fix batch (§8). -- **C** — `PlaylistDetailList` (queue-row reuse): play (verbs + undo), remove, reorder; `ForEach` on entry id. -- **D** — folders UI: flattened disclosure tree, create/rename/delete(+undo), drag-reparent, cycle-guard, depth cap. -- **E** — add-to-playlist: context menu + searchable sheet (US-PLIST-02 incl. loose file) + `LibraryTrackDragItem` UTType + drops (US-PLIST-03/04); `PlaylistDropRouter` + FileMover-spy + strict-gate grep **here**. -- **F** — dead/loose: skip-on-play + unavailable indicator + Locate + remove-missing + orphan-sweep UI. -- **G** — US-PLIST-08 real-scanner seam + `pl-reorder-isolation`/`pl-explain-plan`/`pl-write-during-scan` + strict-gate. +- **C** — ✅ DONE (e79dd44, in #59): `PlaylistDetailView` (queue-row reuse) — play verbs + restore-queue undo, remove, reorder; `ForEach` on entry id. +- **D** — ❌ CUT 2026-07-16 (founder): "don't need folder structures for playlists." The folder DATA LAYER (v5 migration + DAO + checks) already shipped in #59 and is left DORMANT (harmless, additive, tested; re-enabling is free). No folders UI is built. The review-deferred sidebar items (single `SidebarEdit` owner, source-of-truth collapse, `keyboardListNavigation` scaffold) are moot without a second inline editor. +- **E** — ✅ DONE (E1 d6bcc34 + E2 cd4179c + add-everywhere b540423, in #59): add-to-playlist on every surface (Songs/detail/tiles context menu + searchable picker + drag-to-sidebar) + `LibraryTrackDragItem` + pure add-only `PlaylistDropRouter` + the strict-gate no-file-move grep. +- **F** — ⏳ IN PROGRESS (`sprint/s10-3-f-dead-files`): entry `isAvailable` resolution (track resolved + file on disk, checked off-main) → skip-missing-on-play, unavailable badge, Locate (id-preserving `moveTrack`), bulk remove-missing. (No orphan-sweep — deleted *library* tracks cascade; the real dead case is a loose file gone.) +- **G** — US-PLIST-08 real-scanner seam + `pl-reorder-isolation`/`pl-explain-plan`/`pl-write-during-scan` + strict-gate. (Follow-up PR with F.) ## 7. Open sub-decisions (recommend + proceed unless vetoed) - **Play a folder** — *recommended:* plays all descendant playlists' tracks in tree order (bounded by a queue-size safety cap). Same for a multi-playlist selection. diff --git a/docs/sprints/sprint-plan.md b/docs/sprints/sprint-plan.md index 23bd87b..13e31d6 100644 --- a/docs/sprints/sprint-plan.md +++ b/docs/sprints/sprint-plan.md @@ -69,7 +69,7 @@ The rationale is sound: **an audiophile player lives or dies on library + playba ### Status -**S6–S9 ✅ + S10.1 ✅ + S10.2 ✅ + S10.4 ✅ (#58) + S10.6 ✅ shipped · S10.3 in progress (last R1 gate).** S9 browse complete; S10.1 (playlist/queue spine), S10.2 (queue UX), S10.4 (macOS system control — media keys + Now Playing/Control Center), and S10.6 (Recently Played — frecency) shipped & merged. Current work: **S10.3** (Playlists UX — [design](s10-3-playlists-ux-design.md)), scoped to include playlist FOLDERS + case-insensitive names; smart playlists decoupled to a post-R1 fast-follow. Progress: chunk A (folder DAO + additive v5/v6 migrations) + chunk B (PlaylistsModel + sidebar Playlists section + read-only detail) landed on the branch, hardened by a qa-expert + architect/Fool break-it (migrator-posture strict-gate guard + schema-convergence check; corruption-signal; folder data-safety checks). Remaining: play verbs + undo, folders UI, add-to-playlist + drag/drop, dead-file handling, then R1. **R1 gates on S10.1–S10.4 + S10.3** (S10.5/S10.6 are enhancements/polish, not gates). Deprioritized 2026-07-14 (founder): drag-from-Library-into-queue + M3U/M3U8 import-export (see [roadmap Deferred](../product/roadmap.md)). **This line is the single prose status surface for the project** — README/roadmap defer here; everything finer-grained lives in the source + git log (which are authoritative if they disagree with this). +**S6–S9 ✅ + S10.1 ✅ + S10.2 ✅ + S10.4 ✅ (#58) + S10.6 ✅ + S10.3 core ✅ (#59) shipped · S10.3 tail (dead-files) on a branch → then R1.** S9 browse complete; S10.1 (playlist/queue spine), S10.2 (queue UX), S10.4 (macOS system control), S10.6 (Recently Played — frecency), and the **S10.3 CORE** (#59) shipped & merged. S10.3 core = one-store posture (never-erase user data + migrator-posture/schema-convergence guards), playlist CRUD, the sidebar Playlists section + detail (play/next/queue + restore-queue undo, reorder, remove), and **Add to Playlist everywhere** (Songs/album/artist/genre detail + grid tiles + searchable picker + drag-to-sidebar; reference-add that provably never moves a file). **Playlist FOLDERS CUT from scope 2026-07-16 (founder)** — the folder *data layer* (v5 migration + DAO + checks) shipped in #59 and is left DORMANT (re-enabling is free); no folders UI. **Smart playlists** stay a post-R1 fast-follow. Remaining S10.3 (in a follow-up PR): **dead/missing-file handling** (skip-on-play + unavailable badge + Locate + remove-missing — built on `sprint/s10-3-f-dead-files`) + the US-PLIST-08 real-scanner seam & scan-isolation checks (G). **R1 gates on S10.1–S10.4 + S10.3** (S10.5/S10.6 are enhancements, not gates). Deprioritized 2026-07-14 (founder): drag-from-Library-into-queue + M3U/M3U8 import-export (see [roadmap Deferred](../product/roadmap.md)). **This line is the single prose status surface for the project** — README/roadmap defer here; everything finer-grained lives in the source + git log (which are authoritative if they disagree with this). ---