parent
0e018a79dd
commit
5bcf55f808
@ -0,0 +1,9 @@ |
||||
import Foundation |
||||
|
||||
// A single slot in the manual "Up Next" queue. Carries its own stable identity so |
||||
// the same track can be queued more than once without SwiftUI confusing the rows — |
||||
// Track.id alone is not unique across duplicate queue entries. |
||||
nonisolated struct QueueEntry: Identifiable, Sendable { |
||||
let id = UUID() |
||||
let track: Track |
||||
} |
||||
@ -0,0 +1,67 @@ |
||||
import SwiftUI |
||||
|
||||
// The right-docked "Up Next" panel. The manual "Queue" section is reorderable and |
||||
// removable; the "Next from" section is the read-only upcoming context (double-click |
||||
// a row to jump to it). |
||||
struct QueueView: View { |
||||
var player: PlayerViewModel |
||||
|
||||
var body: some View { |
||||
List { |
||||
if player.manualQueue.isEmpty && player.upcomingContext.isEmpty { |
||||
Text("Queue is empty.\nRight-click a track → Add to Queue.") |
||||
.font(.system(size: 12)) |
||||
.foregroundStyle(.secondary) |
||||
.multilineTextAlignment(.center) |
||||
.frame(maxWidth: .infinity, alignment: .center) |
||||
.padding(.vertical, 24) |
||||
.listRowSeparator(.hidden) |
||||
} |
||||
|
||||
if !player.manualQueue.isEmpty { |
||||
Section("Queue") { |
||||
ForEach(player.manualQueue) { entry in |
||||
HStack(spacing: 8) { |
||||
trackRow(entry.track) |
||||
Spacer() |
||||
Button { |
||||
if let idx = player.manualQueue.firstIndex(where: { $0.id == entry.id }) { |
||||
player.removeFromQueue(at: IndexSet(integer: idx)) |
||||
} |
||||
} label: { |
||||
Image(systemName: "xmark.circle.fill") |
||||
.foregroundStyle(.tertiary) |
||||
} |
||||
.buttonStyle(.plain) |
||||
} |
||||
} |
||||
.onMove(perform: player.moveInQueue) |
||||
} |
||||
} |
||||
|
||||
if !player.upcomingContext.isEmpty { |
||||
Section("Next from: \(player.contextName ?? "Library")") { |
||||
ForEach(Array(player.upcomingContext.enumerated()), id: \.offset) { _, track in |
||||
trackRow(track) |
||||
.contentShape(Rectangle()) |
||||
.onTapGesture(count: 2) { player.play(track) } |
||||
} |
||||
} |
||||
} |
||||
} |
||||
.listStyle(.inset) |
||||
.frame(width: 280) |
||||
} |
||||
|
||||
private func trackRow(_ track: Track) -> some View { |
||||
VStack(alignment: .leading, spacing: 2) { |
||||
Text(track.title) |
||||
.font(.system(size: 12, weight: .medium)) |
||||
.lineLimit(1) |
||||
Text(track.artist) |
||||
.font(.system(size: 10)) |
||||
.foregroundStyle(.secondary) |
||||
.lineLimit(1) |
||||
} |
||||
} |
||||
} |
||||
@ -0,0 +1,42 @@ |
||||
import Testing |
||||
import Foundation |
||||
import GRDB |
||||
@testable import Music |
||||
|
||||
@MainActor |
||||
struct DBBackupFTS5Tests { |
||||
|
||||
// Pins down the root cause: does DatabaseService.backup() produce a copy whose |
||||
// FTS5 `tracks_ft` table is functional? GRDB's ValueObservation introspects the |
||||
// whole schema on start, and that introspection throws "no such table: tracks_ft" |
||||
// if the FTS5 shadow tables didn't survive the copy. |
||||
@Test |
||||
func backupCopyHasFunctionalFTS5Table() throws { |
||||
// 1. Build a source DB (DatabasePool/WAL, like the running app) with 3 tracks. |
||||
let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) |
||||
try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) |
||||
defer { try? FileManager.default.removeItem(at: tempDir) } |
||||
let srcPath = tempDir.appendingPathComponent("src.sqlite").path |
||||
let src = try DatabaseService(path: srcPath) |
||||
for i in 1...3 { |
||||
var t = Track.fixture(fileURL: "/s\(i).mp3", title: "Song \(i)") |
||||
try src.insert(&t) |
||||
} |
||||
|
||||
// 2. Copy the DB exactly as the host serves it today. |
||||
let copyPath = tempDir.appendingPathComponent("copy.sqlite").path |
||||
try src.backup(to: copyPath) |
||||
|
||||
// 3. Open the copy and run the same schema-introspection query GRDB runs when |
||||
// a ValueObservation starts. If FTS5 didn't transfer, this throws. |
||||
let copy = try DatabaseService(path: copyPath) |
||||
try copy.dbPool.read { db in |
||||
_ = try Row.fetchAll( |
||||
db, |
||||
sql: "SELECT rootpage, sql FROM sqlite_master WHERE (type = 'table' OR type = 'view')" |
||||
) |
||||
// And an actual FTS5 query must work. |
||||
_ = try Row.fetchAll(db, sql: "SELECT * FROM tracks_ft LIMIT 1") |
||||
} |
||||
} |
||||
} |
||||
@ -0,0 +1,54 @@ |
||||
import Foundation |
||||
import Testing |
||||
@testable import Music |
||||
|
||||
// Reproduces the playlist-bar duplication bug. |
||||
// |
||||
// Regular playlists and smart playlists live in two separate SQLite tables, each |
||||
// with its own `autoIncrementedPrimaryKey("id")`. The two id sequences are |
||||
// independent, so a regular playlist and a smart playlist routinely share the |
||||
// same `id` value (both start at 1, 2, 3, ...). PlaylistViewModel.allPlaylists |
||||
// merges the two kinds into one [any PlaylistRepresentable] collection, and |
||||
// PlaylistBarView's `ForEach(playlists, id: \.id)` keyed off the bare `id`. |
||||
// |
||||
// When two items share an id, SwiftUI collapses them into a single identity: |
||||
// it renders one row twice and ties both buttons to the same view, so selecting |
||||
// or updating one leaks to the other (the reported "shown twice / name changes |
||||
// on both buttons" symptom). The fix is a type-disambiguated `listIdentity` that |
||||
// stays unique across the merged collection. |
||||
struct PlaylistBarIdentityTests { |
||||
|
||||
// Step 1: build a regular playlist and a smart playlist that share id == 1, |
||||
// exactly as the two independent autoincrement tables would produce. |
||||
// Step 2: collect the identities the playlist bar uses to key its ForEach. |
||||
// Step 3: assert the two identities are distinct, so SwiftUI keeps two rows. |
||||
@Test func regularAndSmartPlaylistWithSameIdHaveDistinctListIdentity() { |
||||
let regular = Playlist.fixture(id: 1, name: "Rock") |
||||
let smart = SmartPlaylist.fixture(id: 1, name: "Recently Added") |
||||
|
||||
let items: [any PlaylistRepresentable] = [regular, smart] |
||||
let identities = items.map(\.listIdentity) |
||||
|
||||
#expect(Set(identities).count == items.count) |
||||
} |
||||
|
||||
// Step 1: build the merged collection the way allPlaylists does, with regular |
||||
// and smart playlists whose ids overlap across the two tables. |
||||
// Step 2: map every item to its list identity. |
||||
// Step 3: assert all identities are unique across the whole collection — the |
||||
// invariant SwiftUI ForEach needs to avoid duplicate/leaking rows. |
||||
@Test func mergedPlaylistsHaveUniqueListIdentities() { |
||||
let regulars: [any PlaylistRepresentable] = [ |
||||
Playlist.fixture(id: 1, name: "Rock"), |
||||
Playlist.fixture(id: 2, name: "Jazz"), |
||||
] |
||||
let smarts: [any PlaylistRepresentable] = [ |
||||
SmartPlaylist.fixture(id: 1, name: "Recently Added"), |
||||
SmartPlaylist.fixture(id: 2, name: "Top Rated"), |
||||
] |
||||
let all = regulars + smarts |
||||
let identities = all.map(\.listIdentity) |
||||
|
||||
#expect(Set(identities).count == all.count) |
||||
} |
||||
} |
||||
@ -0,0 +1,36 @@ |
||||
import Testing |
||||
import Foundation |
||||
@testable import Music |
||||
|
||||
@MainActor |
||||
struct PlaylistViewModelTests { |
||||
|
||||
// Verifies createPlaylistAndAddTrack does the full job in one call: |
||||
// 1. Seed a track into an in-memory DB and build a PlaylistViewModel over it. |
||||
// 2. Call createPlaylistAndAddTrack with a name and the seeded track. |
||||
// 3. The returned playlist has the given name and a real (non-nil) id. |
||||
// 4. The DB shows that playlist now contains exactly the seeded track. |
||||
// 5. The new playlist is recorded as the last-used playlist. |
||||
@Test func createPlaylistAndAddTrackCreatesPlaylistAndAddsTrack() throws { |
||||
// 1. Seed a track and build the view model. |
||||
let db = try DatabaseService(inMemory: true) |
||||
var track = Track.fixture(fileURL: "/song.mp3", title: "Song A") |
||||
try db.insert(&track) |
||||
let vm = PlaylistViewModel(db: db) |
||||
|
||||
// 2. Create a new playlist and add the track in one step. |
||||
let created = try vm.createPlaylistAndAddTrack(name: "Road Trip", track: track) |
||||
|
||||
// 3. The returned playlist is well-formed (a real id was assigned, name matches). |
||||
let createdId = try #require(created.id) |
||||
#expect(created.name == "Road Trip") |
||||
|
||||
// 4. The playlist contains exactly the seeded track. |
||||
let tracks = try db.fetchPlaylistTracks(playlistId: createdId) |
||||
#expect(tracks.count == 1) |
||||
#expect(tracks[0].id == track.id) |
||||
|
||||
// 5. The new playlist became the last-used playlist. |
||||
#expect(vm.lastUsedPlaylistId == createdId) |
||||
} |
||||
} |
||||
@ -0,0 +1,123 @@ |
||||
import Testing |
||||
import Foundation |
||||
import GRDB |
||||
@testable import Music |
||||
|
||||
/// Tests for the remote-DB transfer integrity guard. The bug under test: connecting to |
||||
/// a remote host downloaded a database that opened with "database disk image is malformed" |
||||
/// (SQLITE_CORRUPT) on `SELECT * FROM sqlite_master`. The fix validates the database with |
||||
/// `PRAGMA quick_check` on both ends (host before serving, client after writing) so a |
||||
/// malformed image is rejected with a clear error instead of crashing the UI. |
||||
@MainActor |
||||
struct RemoteDBIntegrityTests { |
||||
|
||||
/// Build a real DatabaseService (DatabasePool/WAL, like the running app) at a fresh |
||||
/// temp path, populated with `count` tracks, and return (tempDir, dbPath). |
||||
private func makePopulatedDB(count: Int) throws -> (dir: URL, path: String) { |
||||
let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) |
||||
try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) |
||||
let path = tempDir.appendingPathComponent("src.sqlite").path |
||||
let db = try DatabaseService(path: path) |
||||
for i in 1...count { |
||||
var t = Track.fixture(fileURL: "/track\(i).mp3", title: "Song number \(i) with several words") |
||||
try db.insert(&t) |
||||
} |
||||
return (tempDir, path) |
||||
} |
||||
|
||||
@Test |
||||
func backupCopyIsWellFormedAndSurvivesHTTPFraming() throws { |
||||
// 1. Build a source DB large enough to span many pages, like the real ~5.8MB library. |
||||
let (tempDir, srcPath) = try makePopulatedDB(count: 2000) |
||||
defer { try? FileManager.default.removeItem(at: tempDir) } |
||||
|
||||
// 2. Produce the served copy exactly as the host does (VACUUM INTO via backup()). |
||||
let copyPath = tempDir.appendingPathComponent("copy.sqlite").path |
||||
try DatabaseService(path: srcPath).backup(to: copyPath) |
||||
|
||||
// 3. The copy must pass the integrity gate the host now applies before serving. |
||||
#expect(DatabaseService.isWellFormedDatabase(atPath: copyPath)) |
||||
|
||||
// 4. Frame the copy into an HTTP response byte-for-byte like HostServer.sendHTTP, |
||||
// then parse it back like RemoteClient.handleDBData (split on the first \r\n\r\n). |
||||
let bodyBytes = try Data(contentsOf: URL(fileURLWithPath: copyPath)) |
||||
var response = Data("HTTP/1.1 200 OK\r\nContent-Type: application/octet-stream\r\nContent-Length: \(bodyBytes.count)\r\nConnection: close\r\n\r\n".utf8) |
||||
response.append(bodyBytes) |
||||
let separator: [UInt8] = [0x0D, 0x0A, 0x0D, 0x0A] |
||||
let sepRange = try #require(response.range(of: Data(separator))) |
||||
let parsedBody = Data(response[sepRange.upperBound...]) |
||||
|
||||
// 5. The framed-then-parsed body must be byte-identical to the original copy. |
||||
#expect(parsedBody == bodyBytes) |
||||
|
||||
// 6. Writing it out yields a DB that passes the gate, opens cleanly, and has all rows. |
||||
let recvPath = tempDir.appendingPathComponent("received.sqlite").path |
||||
try parsedBody.write(to: URL(fileURLWithPath: recvPath)) |
||||
#expect(DatabaseService.isWellFormedDatabase(atPath: recvPath)) |
||||
let reopened = try DatabaseService(path: recvPath) |
||||
let n = try reopened.dbPool.read { db in try Int.fetchOne(db, sql: "SELECT count(*) FROM tracks") } |
||||
#expect(n == 2000) |
||||
} |
||||
|
||||
@Test |
||||
func truncatedDownloadIsRejected() throws { |
||||
// Reproduces the user's exact symptom: a page-aligned but incomplete SQLite image. |
||||
// 1. Build a valid served copy. |
||||
let (tempDir, srcPath) = try makePopulatedDB(count: 2000) |
||||
defer { try? FileManager.default.removeItem(at: tempDir) } |
||||
let copyPath = tempDir.appendingPathComponent("copy.sqlite").path |
||||
try DatabaseService(path: srcPath).backup(to: copyPath) |
||||
let full = try Data(contentsOf: URL(fileURLWithPath: copyPath)) |
||||
|
||||
// 2. Keep the first half — still a whole number of 4096-byte pages with a valid |
||||
// SQLite header, exactly the shape of the user's "5828608 bytes" malformed file. |
||||
let truncatedLen = (full.count / 2 / 4096) * 4096 |
||||
let truncated = Data(full.prefix(truncatedLen)) |
||||
let truncPath = tempDir.appendingPathComponent("truncated.sqlite").path |
||||
try truncated.write(to: URL(fileURLWithPath: truncPath)) |
||||
|
||||
// 3. The gate must reject it. Before the fix the client handed this straight to |
||||
// GRDB, which crashed with "database disk image is malformed". |
||||
#expect(DatabaseService.isWellFormedDatabase(atPath: truncPath) == false) |
||||
} |
||||
|
||||
@Test |
||||
func corruptInteriorBytesAreRejected() throws { |
||||
// 1. Build a valid served copy. |
||||
let (tempDir, srcPath) = try makePopulatedDB(count: 2000) |
||||
defer { try? FileManager.default.removeItem(at: tempDir) } |
||||
let copyPath = tempDir.appendingPathComponent("copy.sqlite").path |
||||
try DatabaseService(path: srcPath).backup(to: copyPath) |
||||
var bytes = try Data(contentsOf: URL(fileURLWithPath: copyPath)) |
||||
|
||||
// 2. Leave the SQLite header intact (so it's still recognized as a database — this |
||||
// yields SQLITE_CORRUPT/error 11, like the user saw, not "not a database"/error 26) |
||||
// but smash a stretch of an interior b-tree page. |
||||
for i in 4096..<4600 { bytes[i] = 0xFF } |
||||
let corruptPath = tempDir.appendingPathComponent("corrupt.sqlite").path |
||||
try bytes.write(to: URL(fileURLWithPath: corruptPath)) |
||||
|
||||
// 3. The gate must reject the malformed image. |
||||
#expect(DatabaseService.isWellFormedDatabase(atPath: corruptPath) == false) |
||||
} |
||||
|
||||
@Test |
||||
func emptyMissingAndGarbageFilesAreRejected() throws { |
||||
let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) |
||||
try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) |
||||
defer { try? FileManager.default.removeItem(at: tempDir) } |
||||
|
||||
// 1. A missing file is not well-formed. |
||||
#expect(DatabaseService.isWellFormedDatabase(atPath: tempDir.appendingPathComponent("nope.sqlite").path) == false) |
||||
|
||||
// 2. An empty file (0 bytes) is not well-formed. |
||||
let emptyPath = tempDir.appendingPathComponent("empty.sqlite").path |
||||
try Data().write(to: URL(fileURLWithPath: emptyPath)) |
||||
#expect(DatabaseService.isWellFormedDatabase(atPath: emptyPath) == false) |
||||
|
||||
// 3. A short text body (e.g. an HTTP error message written as if it were a DB) is rejected. |
||||
let garbagePath = tempDir.appendingPathComponent("garbage.sqlite").path |
||||
try Data("Failed to read database".utf8).write(to: URL(fileURLWithPath: garbagePath)) |
||||
#expect(DatabaseService.isWellFormedDatabase(atPath: garbagePath) == false) |
||||
} |
||||
} |
||||
@ -0,0 +1,98 @@ |
||||
import Testing |
||||
import Foundation |
||||
import MusicShared |
||||
import Network |
||||
@testable import Music |
||||
|
||||
@MainActor |
||||
struct RemoteLibraryDisplayTests { |
||||
|
||||
// Reproduces the reported remote-mode bug: "connect + DB download seem fine, |
||||
// but no tracks show up". This exercises the exact step enterRemoteMode performs |
||||
// after the download — pointing a LibraryViewModel at the downloaded DB — and |
||||
// asserts the view model's `tracks` array actually populates. |
||||
@Test(.timeLimit(.minutes(1))) |
||||
func remoteLibraryViewModelPopulatesTracksAfterDownload() async throws { |
||||
// 1. Create a host database and insert three tracks (the "host library"). |
||||
let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) |
||||
try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) |
||||
defer { try? FileManager.default.removeItem(at: tempDir) } |
||||
let hostDBPath = tempDir.appendingPathComponent("host.sqlite").path |
||||
let hostDB = try DatabaseService(path: hostDBPath) |
||||
for i in 1...3 { |
||||
var t = Track.fixture(fileURL: "/song\(i).mp3", title: "Song \(i)") |
||||
try hostDB.insert(&t) |
||||
} |
||||
|
||||
// 2. Start the HostServer configured with the DB so GET /db serves a backup copy. |
||||
let server = HostServer(dbPath: hostDBPath) |
||||
server.configure(player: nil, db: hostDB) |
||||
try server.start() |
||||
try await Task.sleep(for: .milliseconds(200)) |
||||
let port = server.actualPort! |
||||
defer { server.stop() } |
||||
|
||||
// 3. Download the DB over HTTP and write the body to disk, exactly as |
||||
// RemoteClient.handleDBData does when a remote connects to a host. |
||||
let body = try await httpGet(host: "127.0.0.1", port: port, path: "/db") |
||||
let downloadedPath = tempDir.appendingPathComponent("remote_db.sqlite").path |
||||
try body.write(to: URL(fileURLWithPath: downloadedPath)) |
||||
|
||||
// 4. Point a LibraryViewModel at the downloaded DB — this is precisely what |
||||
// MusicApp.enterRemoteMode() does after a successful connect. |
||||
let remoteDB = try DatabaseService(path: downloadedPath) |
||||
let library = LibraryViewModel(db: remoteDB) |
||||
|
||||
// 5. The LibraryViewModel loads via an async GRDB ValueObservation, so poll |
||||
// briefly for the observation to deliver its initial value. |
||||
try await waitUntil { library.tracks.count == 3 } |
||||
|
||||
// 6. The remote library MUST display all three downloaded tracks. If this |
||||
// fails with 0 tracks, it reproduces the "no tracks after connect" bug. |
||||
#expect(library.tracks.count == 3) |
||||
} |
||||
|
||||
// MARK: - Helpers |
||||
|
||||
/// Polls `condition` on the main actor until it becomes true or the timeout elapses. |
||||
private func waitUntil( |
||||
timeout: Duration = .seconds(3), |
||||
_ condition: @MainActor () -> Bool |
||||
) async throws { |
||||
let deadline = ContinuousClock.now + timeout |
||||
while ContinuousClock.now < deadline { |
||||
if condition() { return } |
||||
try await Task.sleep(for: .milliseconds(50)) |
||||
} |
||||
} |
||||
|
||||
/// Performs a simple HTTP GET using NWConnection and returns the response body. |
||||
private func httpGet(host: String, port: UInt16, path: String) async throws -> Data { |
||||
try await withCheckedThrowingContinuation { continuation in |
||||
let connection = NWConnection( |
||||
host: NWEndpoint.Host(host), |
||||
port: NWEndpoint.Port(rawValue: port)!, |
||||
using: .tcp |
||||
) |
||||
connection.stateUpdateHandler = { state in |
||||
if case .ready = state { |
||||
let request = "GET \(path) HTTP/1.1\r\nHost: \(host)\r\nConnection: close\r\n\r\n" |
||||
connection.send(content: Data(request.utf8), completion: .contentProcessed { _ in }) |
||||
connection.receiveMessage { data, _, _, error in |
||||
if let error { |
||||
continuation.resume(throwing: error) |
||||
} else if let data, let range = data.range(of: Data("\r\n\r\n".utf8)) { |
||||
continuation.resume(returning: Data(data[range.upperBound...])) |
||||
} else { |
||||
continuation.resume(returning: data ?? Data()) |
||||
} |
||||
connection.cancel() |
||||
} |
||||
} else if case .failed(let error) = state { |
||||
continuation.resume(throwing: error) |
||||
} |
||||
} |
||||
connection.start(queue: .main) |
||||
} |
||||
} |
||||
} |
||||
@ -0,0 +1,327 @@ |
||||
# Add "New Playlist…" to the Add-to-Playlist menu — Implementation Plan |
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
||||
|
||||
**Goal:** Let a user create a new regular playlist from a track's "Add to Playlist" context submenu, name it via a prompt, and have the track added to it on save. |
||||
|
||||
**Architecture:** A new "New Playlist…" item in the existing `Menu("Add to Playlist")` (in `TrackContextMenuModifier`) calls a new optional closure on `TrackContextMenuConfig`. `ContentView` owns that closure: it stashes the pending track and presents an `.alert` + `TextField` (mirroring the app's existing New Playlist alert). On save it calls a new `PlaylistViewModel.createPlaylistAndAddTrack(name:track:)`, which creates the regular playlist and adds the track in one step. |
||||
|
||||
**Tech Stack:** Swift, SwiftUI, GRDB, Swift Testing (`@Test`), Xcode (`Music` scheme). |
||||
|
||||
**Git note:** This project's owner never auto-commits — commits are made by the user via the `/commit` skill. The "Commit" steps below describe the *suggested grouping* of changes for when the user chooses to commit; do **not** run `git commit` yourself. |
||||
|
||||
**Spec:** `docs/superpowers/specs/2026-05-30-add-to-new-playlist-design.md` |
||||
|
||||
--- |
||||
|
||||
## File Structure |
||||
|
||||
- `Music/ViewModels/PlaylistViewModel.swift` — **Modify.** Add `createPlaylistAndAddTrack(name:track:)` orchestration method. |
||||
- `MusicTests/PlaylistViewModelTests.swift` — **Create.** Unit test for the new method. |
||||
- `Music/Models/TrackContextMenuConfig.swift` — **Modify.** Add `onAddToNewPlaylist` optional closure (+ init param, default `nil`). |
||||
- `Music/Views/TrackContextMenuModifier.swift` — **Modify.** Add "New Playlist…" button + relax the submenu visibility guard. |
||||
- `Music/ContentView.swift` — **Modify.** New `@State` for the pending track, wire `onAddToNewPlaylist`, present the name alert. |
||||
|
||||
--- |
||||
|
||||
## Task 1: `PlaylistViewModel.createPlaylistAndAddTrack` |
||||
|
||||
**Files:** |
||||
- Test: `MusicTests/PlaylistViewModelTests.swift` (create) |
||||
- Modify: `Music/ViewModels/PlaylistViewModel.swift` (add method after `addTrack`, ~line 77) |
||||
|
||||
- [ ] **Step 1: Write the failing test** |
||||
|
||||
Create `MusicTests/PlaylistViewModelTests.swift`: |
||||
|
||||
```swift |
||||
import Testing |
||||
import Foundation |
||||
@testable import Music |
||||
|
||||
@MainActor |
||||
struct PlaylistViewModelTests { |
||||
|
||||
// Verifies createPlaylistAndAddTrack does the full job in one call: |
||||
// 1. Seed a track into an in-memory DB and build a PlaylistViewModel over it. |
||||
// 2. Call createPlaylistAndAddTrack with a name and the seeded track. |
||||
// 3. The returned playlist has the given name and a real (non-nil) id. |
||||
// 4. The DB shows that playlist now contains exactly the seeded track. |
||||
// 5. The new playlist is recorded as the last-used playlist. |
||||
@Test func createPlaylistAndAddTrackCreatesPlaylistAndAddsTrack() throws { |
||||
// 1. Seed a track and build the view model. |
||||
let db = try DatabaseService(inMemory: true) |
||||
var track = Track.fixture(fileURL: "/song.mp3", title: "Song A") |
||||
try db.insert(&track) |
||||
let vm = PlaylistViewModel(db: db) |
||||
|
||||
// 2. Create a new playlist and add the track in one step. |
||||
let created = try vm.createPlaylistAndAddTrack(name: "Road Trip", track: track) |
||||
|
||||
// 3. The returned playlist is well-formed. |
||||
#expect(created.id != nil) |
||||
#expect(created.name == "Road Trip") |
||||
|
||||
// 4. The playlist contains exactly the seeded track. |
||||
let tracks = try db.fetchPlaylistTracks(playlistId: created.id!) |
||||
#expect(tracks.count == 1) |
||||
#expect(tracks[0].id == track.id) |
||||
|
||||
// 5. The new playlist became the last-used playlist. |
||||
#expect(vm.lastUsedPlaylistId == created.id) |
||||
} |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 2: Run the test to verify it fails** |
||||
|
||||
Run: |
||||
```bash |
||||
xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:MusicTests/PlaylistViewModelTests |
||||
``` |
||||
Expected: **build/compile failure** — `value of type 'PlaylistViewModel' has no member 'createPlaylistAndAddTrack'`. |
||||
|
||||
- [ ] **Step 3: Write the minimal implementation** |
||||
|
||||
In `Music/ViewModels/PlaylistViewModel.swift`, add this method directly after `addTrack(_:to:)` (after line 77): |
||||
|
||||
```swift |
||||
@discardableResult |
||||
func createPlaylistAndAddTrack(name: String, track: Track) throws -> Playlist { |
||||
let playlist = try db.createPlaylist(name: name) |
||||
try addTrack(track, to: playlist) |
||||
return playlist |
||||
} |
||||
``` |
||||
|
||||
`db.createPlaylist` returns a `Playlist` with its assigned `id`; the existing `addTrack` inserts the join row and sets `lastUsedPlaylistId`. |
||||
|
||||
- [ ] **Step 4: Run the test to verify it passes** |
||||
|
||||
Run: |
||||
```bash |
||||
xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:MusicTests/PlaylistViewModelTests |
||||
``` |
||||
Expected: **PASS** (`createPlaylistAndAddTrackCreatesPlaylistAndAddsTrack`). |
||||
|
||||
- [ ] **Step 5: Commit (suggested grouping — leave to the user / `/commit`)** |
||||
|
||||
Changed files: `MusicTests/PlaylistViewModelTests.swift`, `Music/ViewModels/PlaylistViewModel.swift`. |
||||
Suggested message: `feat: add PlaylistViewModel.createPlaylistAndAddTrack` |
||||
|
||||
--- |
||||
|
||||
## Task 2: Add `onAddToNewPlaylist` to `TrackContextMenuConfig` |
||||
|
||||
This is a pure data struct; the new field is optional with a `nil` default, so existing call sites and `TrackContextMenuConfigTests` keep compiling. No new unit test (the struct just stores a closure). |
||||
|
||||
**Files:** |
||||
- Modify: `Music/Models/TrackContextMenuConfig.swift` |
||||
|
||||
- [ ] **Step 1: Add the stored property** |
||||
|
||||
In `TrackContextMenuConfig`, add the property after `onAddToQueue` (after line 15): |
||||
|
||||
```swift |
||||
// nil hides the "New Playlist…" item (e.g. tests that don't supply it). |
||||
let onAddToNewPlaylist: ((Track) -> Void)? |
||||
``` |
||||
|
||||
- [ ] **Step 2: Add the init parameter (with default `nil`)** |
||||
|
||||
In the explicit `init`, add the parameter after `onAddToQueue` (line 30): |
||||
|
||||
```swift |
||||
onAddToQueue: ((Track) -> Void)? = nil, |
||||
onAddToNewPlaylist: ((Track) -> Void)? = nil, |
||||
onGetInfo: (([Track]) -> Void)? = nil |
||||
``` |
||||
|
||||
And add the assignment in the body, after `self.onAddToQueue = onAddToQueue` (line 40): |
||||
|
||||
```swift |
||||
self.onAddToNewPlaylist = onAddToNewPlaylist |
||||
``` |
||||
|
||||
- [ ] **Step 3: Build to verify it compiles** |
||||
|
||||
Run: |
||||
```bash |
||||
xcodebuild build -scheme Music -destination 'platform=macOS' |
||||
``` |
||||
Expected: **BUILD SUCCEEDED** (existing call sites still compile because the new param defaults to `nil`). |
||||
|
||||
- [ ] **Step 4: Run the existing config tests to confirm no regression** |
||||
|
||||
Run: |
||||
```bash |
||||
xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:MusicTests/TrackContextMenuConfigTests |
||||
``` |
||||
Expected: **PASS** (all existing tests). |
||||
|
||||
- [ ] **Step 5: Commit (suggested grouping — leave to the user / `/commit`)** |
||||
|
||||
Changed file: `Music/Models/TrackContextMenuConfig.swift`. |
||||
Suggested message: `feat: add onAddToNewPlaylist to TrackContextMenuConfig` |
||||
|
||||
--- |
||||
|
||||
## Task 3: Add "New Playlist…" to the submenu |
||||
|
||||
SwiftUI view code; no unit test (consistent with the codebase). Verify by build. |
||||
|
||||
**Files:** |
||||
- Modify: `Music/Views/TrackContextMenuModifier.swift:30-38` |
||||
|
||||
- [ ] **Step 1: Replace the "Add to Playlist" submenu block** |
||||
|
||||
Replace the current block (lines 30–38): |
||||
|
||||
```swift |
||||
if !config.playlists.isEmpty { |
||||
Menu("Add to Playlist") { |
||||
ForEach(config.playlists) { playlist in |
||||
Button(playlist.name) { |
||||
config.onAddToPlaylist(track, playlist) |
||||
} |
||||
} |
||||
} |
||||
} |
||||
``` |
||||
|
||||
with: |
||||
|
||||
```swift |
||||
if !config.playlists.isEmpty || config.onAddToNewPlaylist != nil { |
||||
Menu("Add to Playlist") { |
||||
if let onAddToNewPlaylist = config.onAddToNewPlaylist { |
||||
Button("New Playlist…") { |
||||
onAddToNewPlaylist(track) |
||||
} |
||||
if !config.playlists.isEmpty { |
||||
Divider() |
||||
} |
||||
} |
||||
ForEach(config.playlists) { playlist in |
||||
Button(playlist.name) { |
||||
config.onAddToPlaylist(track, playlist) |
||||
} |
||||
} |
||||
} |
||||
} |
||||
``` |
||||
|
||||
Notes: the button label uses a real ellipsis character `…` (macOS convention for "opens a prompt"). The submenu now appears even when the user has zero playlists, showing just "New Playlist…". The `Divider` only appears when there are existing playlists to separate from. |
||||
|
||||
- [ ] **Step 2: Build to verify it compiles** |
||||
|
||||
Run: |
||||
```bash |
||||
xcodebuild build -scheme Music -destination 'platform=macOS' |
||||
``` |
||||
Expected: **BUILD SUCCEEDED**. |
||||
|
||||
- [ ] **Step 3: Commit (suggested grouping — leave to the user / `/commit`)** |
||||
|
||||
Changed file: `Music/Views/TrackContextMenuModifier.swift`. |
||||
Suggested message: `feat: add "New Playlist…" item to Add to Playlist submenu` |
||||
|
||||
--- |
||||
|
||||
## Task 4: Wire the prompt in `ContentView` |
||||
|
||||
SwiftUI view code; no unit test. Verify by build + manual check. |
||||
|
||||
**Files:** |
||||
- Modify: `Music/ContentView.swift` — add `@State` (near the other playlist alert state, e.g. by `playlistNameInput`/`showNewPlaylistAlert`), wire the closure in `trackContextMenuConfig` (line 433–434 area), add the alert (by the existing New Playlist alert at line 273). |
||||
|
||||
- [ ] **Step 1: Add state for the pending track** |
||||
|
||||
Find the existing `@State` declarations for the playlist alerts (the same scope that declares `showNewPlaylistAlert` and `playlistNameInput`). Add nearby: |
||||
|
||||
```swift |
||||
@State private var newPlaylistTrack: Track? |
||||
@State private var newPlaylistNameInput = "" |
||||
``` |
||||
|
||||
(`newPlaylistNameInput` is kept separate from the sidebar's `playlistNameInput` so the two flows can't clobber each other's text.) |
||||
|
||||
- [ ] **Step 2: Wire the closure in `trackContextMenuConfig`** |
||||
|
||||
In `trackContextMenuConfig` (line 412), add `onAddToNewPlaylist` to the `TrackContextMenuConfig(...)` call. Insert it after the `onAddToQueue:` argument (line 433), before `onGetInfo:`: |
||||
|
||||
```swift |
||||
onAddToQueue: queueEnabled ? { track in player.addToQueue(track) } : nil, |
||||
onAddToNewPlaylist: { track in newPlaylistTrack = track }, |
||||
onGetInfo: { tracks in |
||||
if !tracks.isEmpty { infoRequest = TrackInfoRequest(tracks: tracks) } |
||||
} |
||||
``` |
||||
|
||||
(Wired unconditionally — matches `onAddToPlaylist`, which is also not gated on `queueEnabled`.) |
||||
|
||||
- [ ] **Step 3: Add the name-prompt alert** |
||||
|
||||
Immediately after the existing New Playlist alert block (after line 283, the closing `}` of `.alert("New Playlist", isPresented: $showNewPlaylistAlert)`), add: |
||||
|
||||
```swift |
||||
.alert("New Playlist", isPresented: Binding( |
||||
get: { newPlaylistTrack != nil }, |
||||
set: { if !$0 { newPlaylistTrack = nil; newPlaylistNameInput = "" } } |
||||
)) { |
||||
TextField("Playlist name", text: $newPlaylistNameInput) |
||||
Button("Cancel", role: .cancel) { |
||||
newPlaylistNameInput = "" |
||||
newPlaylistTrack = nil |
||||
} |
||||
Button("Create") { |
||||
let name = newPlaylistNameInput.trimmingCharacters(in: .whitespaces) |
||||
if !name.isEmpty, let track = newPlaylistTrack { |
||||
try? playlist.createPlaylistAndAddTrack(name: name, track: track) |
||||
} |
||||
newPlaylistNameInput = "" |
||||
newPlaylistTrack = nil |
||||
} |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 4: Build to verify it compiles** |
||||
|
||||
Run: |
||||
```bash |
||||
xcodebuild build -scheme Music -destination 'platform=macOS' |
||||
``` |
||||
Expected: **BUILD SUCCEEDED**. |
||||
|
||||
- [ ] **Step 5: Manual verification** |
||||
|
||||
Launch the app. Right-click a track → **Add to Playlist** → **New Playlist…**. Enter a name, click **Create**. Confirm: |
||||
- A new playlist with that name appears in the sidebar. |
||||
- Selecting it shows the track you added. |
||||
- The sidebar selection did **not** change automatically (stayed on the current view). |
||||
- Re-open the context menu: "Add to <new name>" now appears as the last-used playlist shortcut. |
||||
- Empty name → clicking Create does nothing (no empty playlist created). |
||||
|
||||
- [ ] **Step 6: Commit (suggested grouping — leave to the user / `/commit`)** |
||||
|
||||
Changed file: `Music/ContentView.swift`. |
||||
Suggested message: `feat: prompt for name and add track when creating playlist from menu` |
||||
|
||||
--- |
||||
|
||||
## Final verification |
||||
|
||||
- [ ] Run the full test target once: |
||||
|
||||
```bash |
||||
xcodebuild test -scheme Music -destination 'platform=macOS' |
||||
``` |
||||
Expected: **all tests PASS**, including the new `PlaylistViewModelTests`. |
||||
|
||||
--- |
||||
|
||||
## Self-review (done while writing this plan) |
||||
|
||||
- **Spec coverage:** view-model create+add (Task 1), config closure (Task 2), submenu item + empty-list handling (Task 3), prompt + wiring + behavior decisions: single clicked track, no navigation, empty-name no-op (Task 4). All spec sections map to a task. |
||||
- **Placeholder scan:** none — every code step shows the exact code. |
||||
- **Type/name consistency:** `createPlaylistAndAddTrack(name:track:)` and `onAddToNewPlaylist` are used identically across Tasks 1–4; `newPlaylistTrack` / `newPlaylistNameInput` are consistent within Task 4. |
||||
@ -0,0 +1,607 @@ |
||||
# Fix `bitrate = 0` Tracks Implementation Plan |
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
||||
|
||||
**Goal:** Stop the importer from ever storing a bitrate of `0`, and backfill the real bitrate onto existing tracks where it is `0` or `NULL`. |
||||
|
||||
**Architecture:** Two independent pieces. (1) A pure, unit-tested Swift function `ScannerService.resolveBitrate` that derives kbps from AVFoundation's estimate with a file-size/duration fallback, returning `nil` (never `0`) when nothing is derivable; wired into `extractMetadata`. (2) A stdlib-only Python backfill script `scripts/backfill_bitrate.py` (modeled on the existing `backfill_itunes_dates.py`) that recomputes bitrate via `ffprobe`, falling back to the same formula, with dry-run default and `--apply` + timestamped backup. |
||||
|
||||
**Tech Stack:** Swift (AVFoundation, swift-testing), Python 3 stdlib (`sqlite3`, `subprocess`), `ffprobe` (optional external tool). |
||||
|
||||
**Project conventions to respect:** |
||||
- User's global rule: **never auto-commit.** Each "Commit checkpoint" step means *stop and run the `/commit` skill* — do not run `git commit` directly. |
||||
- Tests use swift-testing (`import Testing`, `@Test`, `#expect`), one struct per file, every test carries a step-by-step comment. |
||||
- Swift static utilities in `ScannerService` are `nonisolated static`. |
||||
|
||||
**Spec:** `docs/superpowers/specs/2026-05-30-fix-zero-bitrate-design.md` |
||||
|
||||
--- |
||||
|
||||
### Task 1: `ScannerService.resolveBitrate` pure function (Swift, TDD) |
||||
|
||||
**Files:** |
||||
- Test: `MusicTests/ScannerServiceTests.swift` (add tests to the existing `ScannerServiceTests` struct) |
||||
- Modify: `Music/Services/ScannerService.swift` (add the static function) |
||||
|
||||
- [ ] **Step 1: Write the failing tests** |
||||
|
||||
Add these five tests inside the existing `struct ScannerServiceTests { ... }` in `MusicTests/ScannerServiceTests.swift`, just before the closing brace: |
||||
|
||||
```swift |
||||
// Verifies resolveBitrate uses the OS estimate when it is positive. |
||||
// 1. Passes a positive estimatedDataRate in bits/sec (320450). |
||||
// 2. Expects it rounded to kbps (320450/1000 = 320.45 -> 320), ignoring size/duration. |
||||
@Test func resolveBitrateUsesEstimateWhenPositive() { |
||||
let kbps = ScannerService.resolveBitrate(estimatedDataRate: 320_450, |
||||
fileSizeBytes: 5_000_000, |
||||
durationSeconds: 200) |
||||
#expect(kbps == 320) |
||||
} |
||||
|
||||
// Verifies the size/duration fallback when the OS estimate is 0 (the AVFoundation bug). |
||||
// 1. Passes estimatedDataRate 0 with a real file size and duration. |
||||
// 2. Expects 230_358_479 * 8 / 7198.54 / 1000 -> ~256.0 -> 256 kbps (matches ffprobe). |
||||
@Test func resolveBitrateFallsBackToSizeAndDuration() { |
||||
let kbps = ScannerService.resolveBitrate(estimatedDataRate: 0, |
||||
fileSizeBytes: 230_358_479, |
||||
durationSeconds: 7198.5371428571425) |
||||
#expect(kbps == 256) |
||||
} |
||||
|
||||
// Verifies nil (never 0) when the estimate is 0 and duration is unusable. |
||||
// 1. Zero duration cannot yield a value -> nil. |
||||
// 2. A NaN duration (CMTimeGetSeconds can return NaN) is also nil, not 0. |
||||
@Test func resolveBitrateReturnsNilWhenNoDuration() { |
||||
#expect(ScannerService.resolveBitrate(estimatedDataRate: 0, |
||||
fileSizeBytes: 230_358_479, |
||||
durationSeconds: 0) == nil) |
||||
#expect(ScannerService.resolveBitrate(estimatedDataRate: 0, |
||||
fileSizeBytes: 230_358_479, |
||||
durationSeconds: .nan) == nil) |
||||
} |
||||
|
||||
// Verifies nil when the estimate is 0 and there is no file size. |
||||
// 1. Missing fileSizeBytes with estimate 0 -> nil (never 0). |
||||
@Test func resolveBitrateReturnsNilWhenNoFileSize() { |
||||
#expect(ScannerService.resolveBitrate(estimatedDataRate: 0, |
||||
fileSizeBytes: nil, |
||||
durationSeconds: 200) == nil) |
||||
} |
||||
|
||||
// Verifies the core invariant: no input combination ever yields 0. |
||||
// 1. All-zero inputs return nil so the UI renders "—" instead of "0 kbps". |
||||
@Test func resolveBitrateNeverReturnsZero() { |
||||
#expect(ScannerService.resolveBitrate(estimatedDataRate: 0, |
||||
fileSizeBytes: 0, |
||||
durationSeconds: 0) == nil) |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 2: Run the tests to verify they fail** |
||||
|
||||
Run: |
||||
```bash |
||||
xcodebuild test -project /Users/laurentmorvillier/code/Music/Music.xcodeproj \ |
||||
-scheme Music -destination 'platform=macOS' \ |
||||
-only-testing:MusicTests/ScannerServiceTests 2>&1 | tail -30 |
||||
``` |
||||
Expected: **compile failure** — `type 'ScannerService' has no member 'resolveBitrate'`. |
||||
(If the destination errors, list options with `xcodebuild -showdestinations -project Music.xcodeproj -scheme Music` and use the macOS one.) |
||||
|
||||
- [ ] **Step 3: Write the minimal implementation** |
||||
|
||||
In `Music/Services/ScannerService.swift`, add this method right after the `discoverAudioFiles` static function (after line 35, inside the class): |
||||
|
||||
```swift |
||||
/// Resolve a track's bitrate in kbps from the OS estimate, falling back to a |
||||
/// size/duration average. Returns nil when nothing can be derived — never 0, |
||||
/// so the UI shows "—" instead of a meaningless "0 kbps". |
||||
/// |
||||
/// AVFoundation's `estimatedDataRate` returns 0 for some files (observed on |
||||
/// long/VBR MP3s); for those we compute the true average bitrate from the |
||||
/// file size and duration, which matches ffprobe to the kbps. |
||||
nonisolated static func resolveBitrate(estimatedDataRate: Double, |
||||
fileSizeBytes: Int64?, |
||||
durationSeconds: Double?) -> Int? { |
||||
if estimatedDataRate > 0 { |
||||
return Int((estimatedDataRate / 1000).rounded()) |
||||
} |
||||
// NaN-safe: `dur > 0` is false for .nan, so we return nil rather than 0. |
||||
if let size = fileSizeBytes, size > 0, |
||||
let dur = durationSeconds, dur > 0 { |
||||
return Int((Double(size) * 8 / dur / 1000).rounded()) |
||||
} |
||||
return nil |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 4: Run the tests to verify they pass** |
||||
|
||||
Run: |
||||
```bash |
||||
xcodebuild test -project /Users/laurentmorvillier/code/Music/Music.xcodeproj \ |
||||
-scheme Music -destination 'platform=macOS' \ |
||||
-only-testing:MusicTests/ScannerServiceTests 2>&1 | tail -30 |
||||
``` |
||||
Expected: **TEST SUCCEEDED**, all `ScannerServiceTests` pass (the new five plus the existing `discoverAudioFiles`). |
||||
|
||||
- [ ] **Step 5: Commit checkpoint** |
||||
|
||||
Stop and run the `/commit` skill (do not `git commit` directly). Suggested message: `feat: add ScannerService.resolveBitrate with size/duration fallback`. |
||||
|
||||
--- |
||||
|
||||
### Task 2: Wire `resolveBitrate` into `extractMetadata` |
||||
|
||||
**Files:** |
||||
- Modify: `Music/Services/ScannerService.swift:146-162` (the bitrate block inside `extractMetadata`) |
||||
|
||||
- [ ] **Step 1: Move the file-stats computation above the bitrate block** |
||||
|
||||
In `extractMetadata`, `TrackFileStats.compute` currently runs at line 162, *after* the bitrate block. Move it up so its `fileSize` is available to `resolveBitrate`. Replace the current block that spans from the duration load through the bitrate computation (lines 146-160): |
||||
|
||||
```swift |
||||
let duration = try await asset.load(.duration) |
||||
let durationSeconds = CMTimeGetSeconds(duration) |
||||
|
||||
var bitrate: Int? |
||||
var sampleRate: Int? |
||||
if let audioTrack = try await asset.loadTracks(withMediaType: .audio).first { |
||||
let estimatedRate = try await audioTrack.load(.estimatedDataRate) |
||||
bitrate = Int(estimatedRate / 1000) |
||||
let descriptions = try await audioTrack.load(.formatDescriptions) |
||||
if let desc = descriptions.first { |
||||
if let asbd = CMAudioFormatDescriptionGetStreamBasicDescription(desc) { |
||||
sampleRate = Int(asbd.pointee.mSampleRate) |
||||
} |
||||
} |
||||
} |
||||
``` |
||||
|
||||
with this: |
||||
|
||||
```swift |
||||
let duration = try await asset.load(.duration) |
||||
let durationSeconds = CMTimeGetSeconds(duration) |
||||
|
||||
let stats = try TrackFileStats.compute(for: url) |
||||
|
||||
var bitrate: Int? |
||||
var sampleRate: Int? |
||||
if let audioTrack = try await asset.loadTracks(withMediaType: .audio).first { |
||||
let estimatedRate = try await audioTrack.load(.estimatedDataRate) |
||||
bitrate = Self.resolveBitrate(estimatedDataRate: estimatedRate, |
||||
fileSizeBytes: stats.fileSize, |
||||
durationSeconds: durationSeconds) |
||||
let descriptions = try await audioTrack.load(.formatDescriptions) |
||||
if let desc = descriptions.first { |
||||
if let asbd = CMAudioFormatDescriptionGetStreamBasicDescription(desc) { |
||||
sampleRate = Int(asbd.pointee.mSampleRate) |
||||
} |
||||
} |
||||
} else { |
||||
// No audio track loaded — still attempt the size/duration fallback |
||||
// so we never silently lose the bitrate. |
||||
bitrate = Self.resolveBitrate(estimatedDataRate: 0, |
||||
fileSizeBytes: stats.fileSize, |
||||
durationSeconds: durationSeconds) |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 2: Remove the now-duplicate `TrackFileStats.compute` call** |
||||
|
||||
The original line 162 `let stats = try TrackFileStats.compute(for: url)` (now appearing just above the `return Track(` call) is a duplicate — delete that single line. The `Track(...)` initializer still references `stats.fileSize`, `stats.dateModified`, and `stats.fileHash`, which now come from the moved-up computation. Confirm exactly one `let stats = try TrackFileStats.compute(for: url)` remains in the function. |
||||
|
||||
- [ ] **Step 3: Build to verify it compiles** |
||||
|
||||
Run: |
||||
```bash |
||||
xcodebuild build -project /Users/laurentmorvillier/code/Music/Music.xcodeproj \ |
||||
-scheme Music -destination 'platform=macOS' 2>&1 | tail -15 |
||||
``` |
||||
Expected: **BUILD SUCCEEDED**. |
||||
|
||||
- [ ] **Step 4: Re-run the full test target to confirm no regressions** |
||||
|
||||
Run: |
||||
```bash |
||||
xcodebuild test -project /Users/laurentmorvillier/code/Music/Music.xcodeproj \ |
||||
-scheme Music -destination 'platform=macOS' \ |
||||
-only-testing:MusicTests 2>&1 | tail -20 |
||||
``` |
||||
Expected: **TEST SUCCEEDED** for the whole `MusicTests` target. |
||||
|
||||
- [ ] **Step 5: Commit checkpoint** |
||||
|
||||
Stop and run the `/commit` skill. Suggested message: `fix: importer derives bitrate via resolveBitrate instead of storing 0`. |
||||
|
||||
--- |
||||
|
||||
### Task 3: Backfill script — pure helpers + self-test (Python, TDD) |
||||
|
||||
**Files:** |
||||
- Create: `scripts/backfill_bitrate.py` |
||||
|
||||
- [ ] **Step 1: Create the script with helpers and a failing self-test** |
||||
|
||||
Create `scripts/backfill_bitrate.py` with exactly this content: |
||||
|
||||
```python |
||||
#!/usr/bin/env python3 |
||||
"""One-time backfill of real bitrate onto tracks stored with bitrate 0 or NULL. |
||||
|
||||
ScannerService writes `bitrate = Int(estimatedDataRate / 1000)` at scan time. |
||||
AVFoundation's estimatedDataRate returns 0 for some files (long/VBR MP3s), so a |
||||
literal 0 gets stored; other tracks were imported before bitrate existed and are |
||||
NULL. This script recomputes bitrate for those rows using ffprobe, falling back |
||||
to fileSize*8/duration (the same average the app's importer now uses) when |
||||
ffprobe is unavailable or can't determine a value. |
||||
|
||||
Dry-run by default. Pass --apply to write (a timestamped backup is made first). |
||||
|
||||
Usage: |
||||
python3 backfill_bitrate.py [--db <path>] [--apply] |
||||
python3 backfill_bitrate.py --self-test |
||||
|
||||
Stdlib only; uses ffprobe if present on PATH (optional). |
||||
""" |
||||
|
||||
import argparse |
||||
import os |
||||
import shutil |
||||
import sqlite3 |
||||
import subprocess |
||||
import sys |
||||
import unicodedata |
||||
from datetime import datetime |
||||
from urllib.parse import unquote |
||||
|
||||
# Default DB path for the sandboxed app (bundle id com.staxriver.mu). Computed from |
||||
# $HOME so it resolves to the right user on whichever Mac the script runs on. |
||||
DEFAULT_DB = os.path.expanduser( |
||||
"~/Library/Containers/com.staxriver.mu/Data/Library/" |
||||
"Application Support/Music/db.sqlite" |
||||
) |
||||
|
||||
|
||||
def norm_path(u): |
||||
"""Reduce a file:// URL (or bare path) to a comparable, on-disk POSIX path. |
||||
|
||||
The app stores `fileURL` as Foundation's url.absoluteString (a percent-encoded |
||||
file URL). Decode it, drop the file:// (or file://localhost) prefix, NFC- |
||||
normalize, and strip a trailing slash so it can be stat'd on APFS. |
||||
""" |
||||
s = u |
||||
if s.startswith("file://"): |
||||
s = s[len("file://"):] |
||||
if s.startswith("localhost/"): |
||||
s = s[len("localhost"):] # leaves the leading "/" |
||||
s = unquote(s) |
||||
s = unicodedata.normalize("NFC", s) |
||||
if len(s) > 1 and s.endswith("/"): |
||||
s = s[:-1] |
||||
return s |
||||
|
||||
|
||||
def parse_ffprobe_bitrate(stdout): |
||||
"""Parse ffprobe's bit_rate stdout (bits/sec) into integer kbps, or None. |
||||
|
||||
Returns None for empty output, 'N/A', or any non-integer text so the caller |
||||
falls back to the formula. |
||||
""" |
||||
s = stdout.strip() |
||||
if not s or s == "N/A": |
||||
return None |
||||
try: |
||||
return round(int(s) / 1000) |
||||
except ValueError: |
||||
return None |
||||
|
||||
|
||||
def kbps_from_ffprobe(path): |
||||
"""Return integer kbps from ffprobe's format bit_rate, or None if unavailable. |
||||
|
||||
None on: ffprobe not installed, ffprobe error, or N/A/empty/non-integer output. |
||||
""" |
||||
try: |
||||
out = subprocess.run( |
||||
["ffprobe", "-v", "error", "-show_entries", "format=bit_rate", |
||||
"-of", "default=nw=1:nk=1", path], |
||||
capture_output=True, text=True, timeout=30, |
||||
) |
||||
except (FileNotFoundError, subprocess.SubprocessError): |
||||
return None |
||||
return parse_ffprobe_bitrate(out.stdout) |
||||
|
||||
|
||||
def kbps_from_formula(file_size, duration): |
||||
"""Average kbps from size (bytes) and duration (seconds): size*8/duration/1000. |
||||
|
||||
Returns None when inputs can't yield a meaningful value (missing size, or |
||||
non-positive/missing duration). |
||||
""" |
||||
if not file_size or not duration or duration <= 0: |
||||
return None |
||||
return round(file_size * 8 / duration / 1000) |
||||
|
||||
|
||||
def resolve_bitrate(path, duration): |
||||
"""Best available kbps for an on-disk file: ffprobe first, formula fallback. |
||||
|
||||
`duration` is the DB's stored seconds; file size is read from disk. Returns |
||||
None if neither method can produce a positive value. |
||||
""" |
||||
kbps = kbps_from_ffprobe(path) |
||||
if kbps and kbps > 0: |
||||
return kbps |
||||
try: |
||||
size = os.path.getsize(path) |
||||
except OSError: |
||||
size = None |
||||
return kbps_from_formula(size, duration) |
||||
|
||||
|
||||
def ffprobe_available(): |
||||
return shutil.which("ffprobe") is not None |
||||
|
||||
|
||||
def self_test(): |
||||
"""Fast smoke check of the pure helpers (no DB, no ffprobe needed).""" |
||||
# ffprobe stdout parsing |
||||
assert parse_ffprobe_bitrate("256005\n") == 256 |
||||
assert parse_ffprobe_bitrate("N/A") is None |
||||
assert parse_ffprobe_bitrate("") is None |
||||
assert parse_ffprobe_bitrate("garbage") is None |
||||
|
||||
# formula: 230_358_479 bytes over 7198.54 s -> 256 kbps (matches ffprobe sample) |
||||
assert kbps_from_formula(230_358_479, 7198.5371428571425) == 256 |
||||
assert kbps_from_formula(None, 100) is None |
||||
assert kbps_from_formula(1000, 0) is None |
||||
assert kbps_from_formula(1000, None) is None |
||||
|
||||
# path normalization (NFD vs NFC accents, percent-encoding, localhost host) |
||||
nfc = norm_path("file:///Users/x/Mu%CC%81sica/Cafe%CC%81.mp3") |
||||
nfd = norm_path("file://localhost/Users/x/M%C3%BAsica/Caf%C3%A9.mp3") |
||||
assert nfc == nfd == "/Users/x/Música/Café.mp3", (nfc, nfd) |
||||
assert norm_path("file:///a/b%20c%23d.mp3") == "/a/b c#d.mp3" |
||||
|
||||
print("self-test OK") |
||||
|
||||
|
||||
def main(argv=None): |
||||
p = argparse.ArgumentParser(description=__doc__) |
||||
p.add_argument("--db", default=DEFAULT_DB, help=f"App DB path (default: {DEFAULT_DB})") |
||||
p.add_argument("--apply", action="store_true", help="Write changes (default: dry run).") |
||||
p.add_argument("--self-test", action="store_true", help="Run the built-in smoke test.") |
||||
args = p.parse_args(argv) |
||||
|
||||
if args.self_test: |
||||
self_test() |
||||
return 0 |
||||
|
||||
if not os.path.exists(args.db): |
||||
p.error(f"DB not found: {args.db}") |
||||
|
||||
run(args.db, args.apply) |
||||
return 0 |
||||
|
||||
|
||||
if __name__ == "__main__": |
||||
sys.exit(main()) |
||||
``` |
||||
|
||||
Note: `run(...)` is referenced by `main` but not yet defined — Task 4 adds it. The self-test does not call `run`, so `--self-test` works now. |
||||
|
||||
- [ ] **Step 2: Run the self-test to verify the helpers pass** |
||||
|
||||
Run: |
||||
```bash |
||||
python3 /Users/laurentmorvillier/code/Music/scripts/backfill_bitrate.py --self-test |
||||
``` |
||||
Expected: `self-test OK`. |
||||
|
||||
- [ ] **Step 3: Verify the dry-run path fails cleanly (run undefined)** |
||||
|
||||
Run: |
||||
```bash |
||||
python3 /Users/laurentmorvillier/code/Music/scripts/backfill_bitrate.py --db /nonexistent.sqlite |
||||
``` |
||||
Expected: argparse error `DB not found: /nonexistent.sqlite` (exit 2) — confirms arg handling before `run` is implemented. |
||||
|
||||
- [ ] **Step 4: Commit checkpoint** |
||||
|
||||
Stop and run the `/commit` skill. Suggested message: `feat: add backfill_bitrate.py helpers + self-test`. |
||||
|
||||
--- |
||||
|
||||
### Task 4: Backfill script — DB wiring, dry-run report, `--apply` |
||||
|
||||
**Files:** |
||||
- Modify: `scripts/backfill_bitrate.py` (add `fetch_rows`, `build_updates`, `backup_db`, `apply_updates`, `run`) |
||||
|
||||
- [ ] **Step 1: Add the DB + reporting functions** |
||||
|
||||
Insert these functions into `scripts/backfill_bitrate.py` immediately before `def main(`: |
||||
|
||||
```python |
||||
def fetch_rows(db_path): |
||||
"""Return candidate rows: (id, fileURL, duration, bitrate) where bitrate is 0/NULL.""" |
||||
con = sqlite3.connect(db_path) |
||||
try: |
||||
return con.execute( |
||||
"SELECT id, fileURL, duration, bitrate FROM tracks " |
||||
"WHERE bitrate = 0 OR bitrate IS NULL" |
||||
).fetchall() |
||||
finally: |
||||
con.close() |
||||
|
||||
|
||||
def build_updates(rows): |
||||
"""Resolve a new bitrate for each candidate row. |
||||
|
||||
Returns (updates, missing, undeterminable): |
||||
- updates: list of {id, file_url, old, new} where new is a positive kbps |
||||
- missing: (id, path) for rows whose file is not on disk (left untouched) |
||||
- undeterminable: (id, path) for on-disk files whose bitrate couldn't be found |
||||
""" |
||||
updates, missing, undeterminable = [], [], [] |
||||
for row_id, file_url, duration, old in rows: |
||||
path = norm_path(file_url) |
||||
if not os.path.exists(path): |
||||
missing.append((row_id, path)) |
||||
continue |
||||
new = resolve_bitrate(path, duration) |
||||
if not new or new <= 0: |
||||
undeterminable.append((row_id, path)) |
||||
continue |
||||
updates.append({"id": row_id, "file_url": file_url, "old": old, "new": new}) |
||||
return updates, missing, undeterminable |
||||
|
||||
|
||||
def backup_db(db_path): |
||||
"""Copy db.sqlite (+ -wal, -shm) under backups/<timestamp>/ next to the DB.""" |
||||
stamp = datetime.now().strftime("%Y%m%d-%H%M%S") |
||||
backup_dir = os.path.join(os.path.dirname(db_path), "backups", stamp) |
||||
os.makedirs(backup_dir, exist_ok=True) |
||||
for suffix in ("", "-wal", "-shm"): |
||||
src = db_path + suffix |
||||
if os.path.exists(src): |
||||
shutil.copy2(src, os.path.join(backup_dir, os.path.basename(src))) |
||||
return backup_dir |
||||
|
||||
|
||||
def apply_updates(db_path, updates): |
||||
"""Write bitrate updates in a single transaction, then checkpoint the WAL.""" |
||||
con = sqlite3.connect(db_path) |
||||
try: |
||||
con.execute("BEGIN") |
||||
con.executemany("UPDATE tracks SET bitrate=:new WHERE id=:id", updates) |
||||
con.commit() |
||||
con.execute("PRAGMA wal_checkpoint(TRUNCATE)") |
||||
finally: |
||||
con.close() |
||||
|
||||
|
||||
def run(db_path, apply): |
||||
rows = fetch_rows(db_path) |
||||
updates, missing, undeterminable = build_updates(rows) |
||||
|
||||
print(f"Candidate rows (bitrate 0 or NULL): {len(rows)}") |
||||
print(f"Resolvable (will set): {len(updates)}") |
||||
print(f"Skipped — file missing on disk: {len(missing)}") |
||||
print(f"Skipped — could not determine: {len(undeterminable)}") |
||||
if not ffprobe_available(): |
||||
print("NOTE: ffprobe not on PATH — used the filesize/duration formula for all rows.") |
||||
print() |
||||
|
||||
for u in updates[:15]: |
||||
name = os.path.basename(norm_path(u["file_url"])) |
||||
old = "NULL" if u["old"] is None else u["old"] |
||||
print(f" • {name}") |
||||
print(f" bitrate {old} -> {u['new']} kbps") |
||||
if len(updates) > 15: |
||||
print(f" ... and {len(updates) - 15} more") |
||||
print() |
||||
|
||||
if missing[:5]: |
||||
print("Sample of skipped (file missing on disk, left untouched):") |
||||
for row_id, path in missing[:5]: |
||||
print(f" - [{row_id}] {os.path.basename(path)}") |
||||
print() |
||||
|
||||
if undeterminable[:5]: |
||||
print("Sample of skipped (could not determine bitrate, left untouched):") |
||||
for row_id, path in undeterminable[:5]: |
||||
print(f" - [{row_id}] {os.path.basename(path)}") |
||||
print() |
||||
|
||||
if not apply: |
||||
print("DRY RUN — nothing written. Re-run with --apply to commit these changes.") |
||||
return |
||||
|
||||
if not updates: |
||||
print("Nothing to apply.") |
||||
return |
||||
|
||||
backup_dir = backup_db(db_path) |
||||
print(f"Backup written to: {backup_dir}") |
||||
apply_updates(db_path, updates) |
||||
print(f"Applied {len(updates)} bitrate updates to {db_path}") |
||||
``` |
||||
|
||||
- [ ] **Step 2: Re-run the self-test (ensure the new code didn't break helpers)** |
||||
|
||||
Run: |
||||
```bash |
||||
python3 /Users/laurentmorvillier/code/Music/scripts/backfill_bitrate.py --self-test |
||||
``` |
||||
Expected: `self-test OK`. |
||||
|
||||
- [ ] **Step 3: Dry-run against a temp DB to verify end-to-end wiring** |
||||
|
||||
This builds a tiny throwaway DB with one real bitrate=0 row, so the run path is exercised without touching the app DB: |
||||
|
||||
```bash |
||||
python3 - <<'PY' |
||||
import sqlite3, os, tempfile |
||||
d = tempfile.mkdtemp() |
||||
db = os.path.join(d, "db.sqlite") |
||||
con = sqlite3.connect(db) |
||||
con.execute("CREATE TABLE tracks (id INTEGER PRIMARY KEY, fileURL TEXT, duration REAL, bitrate INTEGER)") |
||||
# A file that does not exist -> should be reported as 'missing', not crash. |
||||
con.execute("INSERT INTO tracks (fileURL, duration, bitrate) VALUES (?,?,?)", |
||||
("file:///no/such/file.mp3", 100.0, 0)) |
||||
con.commit(); con.close() |
||||
print(db) |
||||
PY |
||||
``` |
||||
Take the printed path and run: |
||||
```bash |
||||
python3 /Users/laurentmorvillier/code/Music/scripts/backfill_bitrate.py --db <printed-path> |
||||
``` |
||||
Expected: a summary with `Candidate rows ... : 1`, `Skipped — file missing on disk: 1`, and `DRY RUN — nothing written.` (no traceback). |
||||
|
||||
- [ ] **Step 4: Commit checkpoint** |
||||
|
||||
Stop and run the `/commit` skill. Suggested message: `feat: backfill_bitrate.py DB wiring, dry-run report, --apply`. |
||||
|
||||
--- |
||||
|
||||
### Task 5: Verify against the real library (manual, no code change) |
||||
|
||||
**Files:** none. |
||||
|
||||
- [ ] **Step 1: Dry-run against the real app DB** |
||||
|
||||
Quit the Music app first (avoids WAL/lock contention). Then: |
||||
```bash |
||||
python3 /Users/laurentmorvillier/code/Music/scripts/backfill_bitrate.py |
||||
``` |
||||
Expected: a non-zero `Resolvable (will set)` count and a sample of `bitrate 0 -> NNN kbps` lines. Eyeball a few values for plausibility (typical 128–320 kbps). |
||||
|
||||
- [ ] **Step 2: Apply** |
||||
|
||||
```bash |
||||
python3 /Users/laurentmorvillier/code/Music/scripts/backfill_bitrate.py --apply |
||||
``` |
||||
Expected: `Backup written to: .../backups/<timestamp>` then `Applied N bitrate updates`. |
||||
|
||||
- [ ] **Step 3: Confirm the DB no longer has 0/NULL bitrates (except undeterminable)** |
||||
|
||||
```bash |
||||
DB="$HOME/Library/Containers/com.staxriver.mu/Data/Library/Application Support/Music/db.sqlite" |
||||
sqlite3 "$DB" "SELECT COUNT(*) AS still_zero_or_null FROM tracks WHERE bitrate = 0 OR bitrate IS NULL;" |
||||
``` |
||||
Expected: `0`, or the small count the dry-run reported as "could not determine"/"file missing". |
||||
|
||||
- [ ] **Step 4: Reopen the app and spot-check** |
||||
|
||||
Open a previously-0 track's Get Info (or the Bit Rate column) and confirm it now shows a real value. |
||||
|
||||
--- |
||||
|
||||
## Self-Review Notes |
||||
|
||||
- **Spec coverage:** Root cause + invariant → Tasks 1–2 (`resolveBitrate`, never stores 0). Backfill (0 and NULL, ffprobe→formula, missing-file skip, dry-run/backup/apply) → Tasks 3–4. Manual verification → Task 5. All spec sections covered. |
||||
- **Type consistency:** `resolveBitrate(estimatedDataRate:fileSizeBytes:durationSeconds:)` is defined identically in Task 1 and called identically in Task 2; `stats.fileSize` is `Int64` (matches `fileSizeBytes: Int64?`). Python helper names (`parse_ffprobe_bitrate`, `kbps_from_ffprobe`, `kbps_from_formula`, `resolve_bitrate`, `fetch_rows`, `build_updates`, `backup_db`, `apply_updates`, `run`) are defined once and referenced consistently. |
||||
- **No placeholders:** every code step shows complete code; every run step shows the command and expected output. |
||||
@ -0,0 +1,780 @@ |
||||
# Playing Queue Implementation Plan |
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
||||
|
||||
**Goal:** Add a Spotify-style priority "Up Next" queue: tracks can be pushed to the front ("Play Next") or end ("Add to Queue") via the track context menu, played before the playlist/album context resumes, and managed in a right-docked panel. |
||||
|
||||
**Architecture:** `PlayerViewModel` keeps its existing `queue`/`currentIndex` as the playback **context** and gains a parallel `manualQueue` of `QueueEntry` (a track + stable UUID). `next()` drains the manual queue before advancing the context; a dedicated `playManual(_:)` plays a queued track without moving `currentIndex`, so the context resumes correctly. A new `QueueView` renders the panel; the context menu and `ContentView` are wired up. Local-only for v1 — queue actions are hidden when driving a remote device. |
||||
|
||||
**Tech Stack:** Swift, SwiftUI + AppKit (`NSTableView`), Swift Testing (`import Testing`), Xcode 16 synchronized groups (no `pbxproj` edits for new files). |
||||
|
||||
**Spec:** `docs/superpowers/specs/2026-05-30-playing-queue-design.md` |
||||
|
||||
> **Project rule — commits:** This repo's CLAUDE.md says *never commit unless triggered by the `/commit` skill*. The "Commit" steps below are checkpoints: stage the listed files and ask the user to run `/commit` (or run it when they direct). Do **not** commit autonomously. |
||||
|
||||
> **Test command:** `xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:<TestTarget/Suite> 2>&1 | tail -30`. Full build: `xcodebuild build -scheme Music -destination 'platform=macOS' 2>&1 | tail -20`. |
||||
|
||||
--- |
||||
|
||||
## Task 1: `QueueEntry` model + queue state and add actions |
||||
|
||||
**Files:** |
||||
- Create: `Music/Models/QueueEntry.swift` |
||||
- Modify: `Music/ViewModels/PlayerViewModel.swift` (add state + methods) |
||||
- Test: `MusicTests/PlayerViewModelTests.swift` |
||||
|
||||
- [ ] **Step 1: Write the failing tests** |
||||
|
||||
Add these tests inside the `PlayerViewModelTests` struct in `MusicTests/PlayerViewModelTests.swift` (after the existing tests, before the closing `}` of the struct): |
||||
|
||||
```swift |
||||
// Step 1: context [1,2,3], track 1 playing. |
||||
// Step 2: addToQueue twice → manual queue holds those tracks in arrival order. |
||||
// Step 3: playNext jumps a track to the FRONT of the manual queue. |
||||
@Test func addToQueueAppendsAndPlayNextInsertsFront() { |
||||
let vm = PlayerViewModel(provider: AudioService(), db: nil) |
||||
let tracks = makeTracks(6) |
||||
vm.setQueue(Array(tracks[0..<3])) |
||||
vm.play(tracks[0]) |
||||
|
||||
vm.addToQueue(tracks[3]) // id 4 |
||||
vm.addToQueue(tracks[4]) // id 5 |
||||
#expect(vm.manualQueue.map { $0.track.id } == [4, 5]) |
||||
|
||||
vm.playNext(tracks[5]) // id 6 to the front |
||||
#expect(vm.manualQueue.map { $0.track.id } == [6, 4, 5]) |
||||
} |
||||
|
||||
// Step 1: a view model with nothing playing (idle). |
||||
// Step 2: addToQueue should start playback immediately (queue-while-idle) and |
||||
// leave the manual queue empty because the track was consumed to play. |
||||
@Test func queueWhileIdleStartsPlayback() { |
||||
let vm = PlayerViewModel(provider: AudioService(), db: nil) |
||||
let track = Track.fixture(id: 1, fileURL: "/a.mp3", title: "A") |
||||
|
||||
vm.addToQueue(track) |
||||
|
||||
#expect(vm.currentTrack?.id == 1) |
||||
#expect(vm.manualQueue.isEmpty) |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 2: Run the tests to verify they fail** |
||||
|
||||
Run: `xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:MusicTests/PlayerViewModelTests 2>&1 | tail -30` |
||||
Expected: FAIL to compile — `value of type 'PlayerViewModel' has no member 'manualQueue' / 'addToQueue' / 'playNext'`. |
||||
|
||||
- [ ] **Step 3: Create the `QueueEntry` model** |
||||
|
||||
Create `Music/Models/QueueEntry.swift`: |
||||
|
||||
```swift |
||||
import Foundation |
||||
|
||||
// A single slot in the manual "Up Next" queue. Carries its own stable identity so |
||||
// the same track can be queued more than once without SwiftUI confusing the rows — |
||||
// Track.id alone is not unique across duplicate queue entries. |
||||
nonisolated struct QueueEntry: Identifiable { |
||||
let id = UUID() |
||||
let track: Track |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 4: Add queue state to `PlayerViewModel`** |
||||
|
||||
In `Music/ViewModels/PlayerViewModel.swift`, add the new stored properties immediately after the `originalQueue` line (currently line 20): |
||||
|
||||
```swift |
||||
private var originalQueue: [Track] = [] |
||||
/// The manual "Up Next" queue. Plays ahead of `queue` (the context) and survives |
||||
/// starting a new context. `queue`/`currentIndex` remain the CONTEXT position. |
||||
private(set) var manualQueue: [QueueEntry] = [] |
||||
/// Display label for the panel's "Next from: <name>" section. |
||||
private(set) var contextName: String? |
||||
``` |
||||
|
||||
- [ ] **Step 5: Add the manual-queue methods and `playManual`** |
||||
|
||||
In the same file, add a new section just after the `// MARK: - Queue Management` block (after `setQueue`'s closing brace, currently line 99). Note the `remoteProvider == nil` guard — the manual queue is local-only for v1: |
||||
|
||||
```swift |
||||
// MARK: - Manual Queue |
||||
|
||||
func playNext(_ track: Track) { |
||||
guard remoteProvider == nil else { return } |
||||
manualQueue.insert(QueueEntry(track: track), at: 0) |
||||
startQueuedTrackIfIdle() |
||||
} |
||||
|
||||
func addToQueue(_ track: Track) { |
||||
guard remoteProvider == nil else { return } |
||||
manualQueue.append(QueueEntry(track: track)) |
||||
startQueuedTrackIfIdle() |
||||
} |
||||
|
||||
func removeFromQueue(at offsets: IndexSet) { |
||||
manualQueue.remove(atOffsets: offsets) |
||||
} |
||||
|
||||
func moveInQueue(from source: IndexSet, to destination: Int) { |
||||
manualQueue.move(fromOffsets: source, toOffset: destination) |
||||
} |
||||
|
||||
/// Context tracks after the current context position — the panel's "Next from" |
||||
/// section. Empty when there is no context or we are at its end. |
||||
var upcomingContext: [Track] { |
||||
guard let idx = currentIndex, idx + 1 < queue.count else { return [] } |
||||
return Array(queue[(idx + 1)...]) |
||||
} |
||||
|
||||
// If nothing is playing, start the just-queued track immediately rather than |
||||
// parking it — matches Spotify's "queue while idle starts playback". |
||||
private func startQueuedTrackIfIdle() { |
||||
guard currentTrack == nil, !manualQueue.isEmpty else { return } |
||||
let entry = manualQueue.removeFirst() |
||||
playManual(entry.track) |
||||
} |
||||
|
||||
// Plays a track pulled from the manual queue. Mirrors play(_:) but deliberately |
||||
// does NOT touch currentIndex, so the context position is preserved and resumes |
||||
// once the manual queue drains. |
||||
private func playManual(_ track: Track) { |
||||
currentTrack = track |
||||
halfwayReported = false |
||||
isPlaying = true |
||||
currentTime = 0 |
||||
duration = track.duration |
||||
guard let url = provider.urlForTrack(track) else { return } |
||||
provider.play(url: url) |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 6: Run the tests to verify they pass** |
||||
|
||||
Run: `xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:MusicTests/PlayerViewModelTests 2>&1 | tail -30` |
||||
Expected: PASS (the two new tests plus all existing PlayerViewModel tests). |
||||
|
||||
- [ ] **Step 7: Commit (checkpoint — via `/commit`)** |
||||
|
||||
Stage and request a commit: |
||||
|
||||
```bash |
||||
git add Music/Models/QueueEntry.swift Music/ViewModels/PlayerViewModel.swift MusicTests/PlayerViewModelTests.swift |
||||
# Then ask the user to run /commit (suggested message: "feat: add manual queue state and Play Next / Add to Queue to PlayerViewModel") |
||||
``` |
||||
|
||||
--- |
||||
|
||||
## Task 2: `next()` drains the manual queue, then resumes the context |
||||
|
||||
**Files:** |
||||
- Modify: `Music/ViewModels/PlayerViewModel.swift:160-172` (the `next()` method) |
||||
- Test: `MusicTests/PlayerViewModelTests.swift` |
||||
|
||||
- [ ] **Step 1: Write the failing tests** |
||||
|
||||
Add to `PlayerViewModelTests`: |
||||
|
||||
```swift |
||||
// Step 1: context [1,2,3], track 1 playing (currentIndex 0). |
||||
// Step 2: queue track 5. next() must play the QUEUED track, not context track 2, |
||||
// consume it from the queue, and leave currentIndex at 0 (context held). |
||||
@Test func nextConsumesManualQueueBeforeContext() { |
||||
let vm = PlayerViewModel(provider: AudioService(), db: nil) |
||||
let tracks = makeTracks(6) |
||||
vm.setQueue(Array(tracks[0..<3])) |
||||
vm.play(tracks[0]) |
||||
|
||||
vm.addToQueue(tracks[4]) // id 5 |
||||
vm.next() |
||||
|
||||
#expect(vm.currentTrack?.id == 5) |
||||
#expect(vm.manualQueue.isEmpty) |
||||
#expect(vm.currentIndex == 0) |
||||
} |
||||
|
||||
// Step 1: context [1,2,3], track 1 playing; queue track 5. |
||||
// Step 2: first next() plays the queued track 5 (context still at index 0). |
||||
// Step 3: second next() finds the queue empty and resumes the context at index 1 |
||||
// → track 2. |
||||
@Test func contextResumesAfterManualQueueDrains() { |
||||
let vm = PlayerViewModel(provider: AudioService(), db: nil) |
||||
let tracks = makeTracks(6) |
||||
vm.setQueue(Array(tracks[0..<3])) |
||||
vm.play(tracks[0]) |
||||
|
||||
vm.addToQueue(tracks[4]) // id 5 |
||||
vm.next() // plays queued track 5 |
||||
vm.next() // resumes context |
||||
|
||||
#expect(vm.currentTrack?.id == 2) |
||||
#expect(vm.currentIndex == 1) |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 2: Run the tests to verify they fail** |
||||
|
||||
Run: `xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:MusicTests/PlayerViewModelTests/nextConsumesManualQueueBeforeContext 2>&1 | tail -30` |
||||
Expected: FAIL — `next()` currently advances the context, so `currentTrack?.id` is `2`, not `5`. |
||||
|
||||
- [ ] **Step 3: Update `next()`** |
||||
|
||||
In `Music/ViewModels/PlayerViewModel.swift`, replace the existing `next()` method: |
||||
|
||||
```swift |
||||
func next() { |
||||
if let remote = remoteProvider { |
||||
remote.sendNext() |
||||
return |
||||
} |
||||
guard let idx = currentIndex else { return } |
||||
let nextIdx = idx + 1 |
||||
if nextIdx < queue.count { |
||||
play(queue[nextIdx]) |
||||
} else { |
||||
stop() |
||||
} |
||||
} |
||||
``` |
||||
|
||||
with: |
||||
|
||||
```swift |
||||
func next() { |
||||
if let remote = remoteProvider { |
||||
remote.sendNext() |
||||
return |
||||
} |
||||
// Manual queue takes priority and is consumed as it plays. |
||||
if !manualQueue.isEmpty { |
||||
let entry = manualQueue.removeFirst() |
||||
playManual(entry.track) |
||||
return |
||||
} |
||||
// Otherwise advance the context from the preserved position. |
||||
guard let idx = currentIndex else { return } |
||||
let nextIdx = idx + 1 |
||||
if nextIdx < queue.count { |
||||
play(queue[nextIdx]) |
||||
} else { |
||||
stop() |
||||
} |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 4: Run the tests to verify they pass** |
||||
|
||||
Run: `xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:MusicTests/PlayerViewModelTests 2>&1 | tail -30` |
||||
Expected: PASS (new tests + existing tests, including `nextAtEndStops` and `nextAdvancesToNextTrack`, still green). |
||||
|
||||
- [ ] **Step 5: Commit (checkpoint — via `/commit`)** |
||||
|
||||
```bash |
||||
git add Music/ViewModels/PlayerViewModel.swift MusicTests/PlayerViewModelTests.swift |
||||
# Suggested message: "feat: next() drains the manual queue before advancing the context" |
||||
``` |
||||
|
||||
--- |
||||
|
||||
## Task 3: Edit ops, `upcomingContext`, `setQueue(contextName:)`, shuffle isolation |
||||
|
||||
**Files:** |
||||
- Modify: `Music/ViewModels/PlayerViewModel.swift` (`setQueue`, `setProvider`) |
||||
- Test: `MusicTests/PlayerViewModelTests.swift` |
||||
|
||||
- [ ] **Step 1: Write the failing tests** |
||||
|
||||
Add to `PlayerViewModelTests`: |
||||
|
||||
```swift |
||||
// Step 1: context [1,2,3], track 1 playing; queue tracks 4,5,6. |
||||
// Step 2: removeFromQueue removes the middle entry → [4,6]. |
||||
// Step 3: moveInQueue moves the last entry to the front → [6,4]. |
||||
@Test func removeAndMoveMutateManualQueue() { |
||||
let vm = PlayerViewModel(provider: AudioService(), db: nil) |
||||
let tracks = makeTracks(6) |
||||
vm.setQueue(Array(tracks[0..<3])) |
||||
vm.play(tracks[0]) |
||||
vm.addToQueue(tracks[3]); vm.addToQueue(tracks[4]); vm.addToQueue(tracks[5]) |
||||
#expect(vm.manualQueue.map { $0.track.id } == [4, 5, 6]) |
||||
|
||||
vm.removeFromQueue(at: IndexSet(integer: 1)) |
||||
#expect(vm.manualQueue.map { $0.track.id } == [4, 6]) |
||||
|
||||
vm.moveInQueue(from: IndexSet(integer: 1), to: 0) |
||||
#expect(vm.manualQueue.map { $0.track.id } == [6, 4]) |
||||
} |
||||
|
||||
// Step 1: context [1,2,3,4], track 2 playing (currentIndex 1). |
||||
// Step 2: upcomingContext is the slice after the current position → [3,4]. |
||||
@Test func upcomingContextReturnsTracksAfterCurrent() { |
||||
let vm = PlayerViewModel(provider: AudioService(), db: nil) |
||||
let tracks = makeTracks(4) |
||||
vm.setQueue(tracks) |
||||
vm.play(tracks[1]) |
||||
|
||||
#expect(vm.upcomingContext.map { $0.id } == [3, 4]) |
||||
} |
||||
|
||||
// Step 1: 10-track context, one playing; queue tracks 11 and 12 in order. |
||||
// Step 2: toggling shuffle reorders only the context — the manual queue order |
||||
// must be left exactly as the user arranged it. |
||||
@Test func shuffleLeavesManualQueueIntact() { |
||||
let vm = PlayerViewModel(provider: AudioService(), db: nil) |
||||
let tracks = makeTracks(12) |
||||
vm.setQueue(Array(tracks[0..<10])) |
||||
vm.play(tracks[0]) |
||||
vm.addToQueue(tracks[10]) // id 11 |
||||
vm.addToQueue(tracks[11]) // id 12 |
||||
|
||||
vm.toggleShuffle() |
||||
|
||||
#expect(vm.manualQueue.map { $0.track.id } == [11, 12]) |
||||
} |
||||
|
||||
// Step 1: setQueue accepts an optional context label for the panel header. |
||||
@Test func setQueueStoresContextName() { |
||||
let vm = PlayerViewModel(provider: AudioService(), db: nil) |
||||
vm.setQueue(makeTracks(2), contextName: "Synthwave") |
||||
#expect(vm.contextName == "Synthwave") |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 2: Run the tests to verify they fail** |
||||
|
||||
Run: `xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:MusicTests/PlayerViewModelTests/setQueueStoresContextName 2>&1 | tail -30` |
||||
Expected: FAIL to compile — `setQueue` has no `contextName:` parameter. (`removeAndMoveMutateManualQueue`, `upcomingContext...`, and `shuffleLeaves...` already pass from Task 1's methods, except the `contextName` compile error blocks the whole suite.) |
||||
|
||||
- [ ] **Step 3: Add the `contextName` parameter to `setQueue`** |
||||
|
||||
In `Music/ViewModels/PlayerViewModel.swift`, replace the `setQueue` signature line: |
||||
|
||||
```swift |
||||
func setQueue(_ tracks: [Track]) { |
||||
originalQueue = tracks |
||||
``` |
||||
|
||||
with: |
||||
|
||||
```swift |
||||
func setQueue(_ tracks: [Track], contextName: String? = nil) { |
||||
self.contextName = contextName |
||||
originalQueue = tracks |
||||
``` |
||||
|
||||
(The default `nil` keeps existing callers and tests compiling.) |
||||
|
||||
- [ ] **Step 4: Reset the new state in `setProvider`** |
||||
|
||||
In the `setProvider(_:)` method, add the two resets next to the existing `queue = []` / `originalQueue = []` lines (currently lines 55-56): |
||||
|
||||
```swift |
||||
queue = [] |
||||
originalQueue = [] |
||||
manualQueue = [] |
||||
contextName = nil |
||||
``` |
||||
|
||||
- [ ] **Step 5: Run the tests to verify they pass** |
||||
|
||||
Run: `xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:MusicTests/PlayerViewModelTests 2>&1 | tail -30` |
||||
Expected: PASS (all PlayerViewModel tests, old and new). |
||||
|
||||
- [ ] **Step 6: Commit (checkpoint — via `/commit`)** |
||||
|
||||
```bash |
||||
git add Music/ViewModels/PlayerViewModel.swift MusicTests/PlayerViewModelTests.swift |
||||
# Suggested message: "feat: queue edit ops, upcomingContext, and contextName label" |
||||
``` |
||||
|
||||
--- |
||||
|
||||
## Task 4: Context-menu config — `onPlayNext` / `onAddToQueue` + both menu builders |
||||
|
||||
**Files:** |
||||
- Modify: `Music/Models/TrackContextMenuConfig.swift` |
||||
- Modify: `Music/Views/TrackContextMenuModifier.swift` (SwiftUI menu) |
||||
- Modify: `Music/Views/TrackTableView.swift:328-394` (AppKit menu + actions) |
||||
- Test: `MusicTests/TrackContextMenuConfigTests.swift` |
||||
|
||||
- [ ] **Step 1: Write the failing test** |
||||
|
||||
The two new config fields get `= nil` defaults (Step 3), so the existing |
||||
`TrackContextMenuConfig(...)` constructions in this file and in `ContentView` |
||||
keep compiling untouched. Just add a new test to `TrackContextMenuConfigTests`: |
||||
|
||||
```swift |
||||
// Verifies the queue callbacks fire with the right track. |
||||
@Test func queueCallbacksFire() { |
||||
let track = Track.fixture(id: 7, title: "Q") |
||||
var playNextTrack: Track? = nil |
||||
var addQueueTrack: Track? = nil |
||||
|
||||
let config = TrackContextMenuConfig( |
||||
playlists: [], |
||||
lastUsedPlaylistName: nil, |
||||
selectedPlaylist: nil, |
||||
onAddToPlaylist: { _, _ in }, |
||||
onAddToLastPlaylist: nil, |
||||
onRemoveFromPlaylist: nil, |
||||
onPlayNext: { t in playNextTrack = t }, |
||||
onAddToQueue: { t in addQueueTrack = t } |
||||
) |
||||
|
||||
config.onPlayNext?(track) |
||||
config.onAddToQueue?(track) |
||||
|
||||
#expect(playNextTrack?.id == 7) |
||||
#expect(addQueueTrack?.id == 7) |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 2: Run the test to verify it fails** |
||||
|
||||
Run: `xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:MusicTests/TrackContextMenuConfigTests 2>&1 | tail -30` |
||||
Expected: FAIL to compile — `extra arguments 'onPlayNext', 'onAddToQueue'` (the struct has no such members yet). |
||||
|
||||
- [ ] **Step 3: Add the closures to the config struct** |
||||
|
||||
In `Music/Models/TrackContextMenuConfig.swift`, add the two fields after `onRemoveFromPlaylist`. The `= nil` defaults flow into the synthesized memberwise initializer, so existing callers (ContentView + the other config test) keep compiling and the items stay hidden until wired: |
||||
|
||||
```swift |
||||
let onRemoveFromPlaylist: ((Track) -> Void)? |
||||
// nil hides the corresponding item (e.g. when driving a remote device). |
||||
let onPlayNext: ((Track) -> Void)? = nil |
||||
let onAddToQueue: ((Track) -> Void)? = nil |
||||
``` |
||||
|
||||
- [ ] **Step 4: Run the config test to verify it passes** |
||||
|
||||
Run: `xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing:MusicTests/TrackContextMenuConfigTests 2>&1 | tail -30` |
||||
Expected: PASS, and the app target still builds (existing callers use the `nil` defaults). |
||||
|
||||
> Note: the menu items are wired with real closures in Task 6. Until then `ContentView` passes the default `nil`, so the items stay hidden — expected interim state. |
||||
|
||||
- [ ] **Step 5: Render the items in the SwiftUI menu** |
||||
|
||||
In `Music/Views/TrackContextMenuModifier.swift`, inside `if let track, let config {`, insert this block **before** the existing `lastUsedPlaylistName` block (so Play Next / Add to Queue appear at the top): |
||||
|
||||
```swift |
||||
if let onPlayNext = config.onPlayNext { |
||||
Button("Play Next") { onPlayNext(track) } |
||||
} |
||||
if let onAddToQueue = config.onAddToQueue { |
||||
Button("Add to Queue") { onAddToQueue(track) } |
||||
} |
||||
if config.onPlayNext != nil || config.onAddToQueue != nil { |
||||
Divider() |
||||
} |
||||
|
||||
``` |
||||
|
||||
- [ ] **Step 6: Render the items in the AppKit menu** |
||||
|
||||
In `Music/Views/TrackTableView.swift`, in `menuNeedsUpdate(_:)`, insert this block immediately after the two `guard` lines (after `guard let config = parent.contextMenuConfig else { return }`) and **before** the `if let lastPlaylistName` block: |
||||
|
||||
```swift |
||||
if config.onPlayNext != nil { |
||||
let item = NSMenuItem(title: "Play Next", action: #selector(playNext(_:)), keyEquivalent: "") |
||||
item.target = self |
||||
menu.addItem(item) |
||||
} |
||||
if config.onAddToQueue != nil { |
||||
let item = NSMenuItem(title: "Add to Queue", action: #selector(addToQueue(_:)), keyEquivalent: "") |
||||
item.target = self |
||||
menu.addItem(item) |
||||
} |
||||
if config.onPlayNext != nil || config.onAddToQueue != nil { |
||||
menu.addItem(.separator()) |
||||
} |
||||
``` |
||||
|
||||
Then add the two action handlers next to the existing `addToLastPlaylist` / `removeFromPlaylist` handlers (after `removeFromPlaylist(_:)`'s closing brace, currently line 394): |
||||
|
||||
```swift |
||||
@objc func playNext(_ sender: NSMenuItem) { |
||||
guard let tableView, tableView.clickedRow >= 0, tableView.clickedRow < tracks.count else { return } |
||||
guard let config = parent.contextMenuConfig else { return } |
||||
config.onPlayNext?(tracks[tableView.clickedRow]) |
||||
} |
||||
|
||||
@objc func addToQueue(_ sender: NSMenuItem) { |
||||
guard let tableView, tableView.clickedRow >= 0, tableView.clickedRow < tracks.count else { return } |
||||
guard let config = parent.contextMenuConfig else { return } |
||||
config.onAddToQueue?(tracks[tableView.clickedRow]) |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 7: Commit (checkpoint — via `/commit`)** |
||||
|
||||
```bash |
||||
git add Music/Models/TrackContextMenuConfig.swift Music/Views/TrackContextMenuModifier.swift Music/Views/TrackTableView.swift MusicTests/TrackContextMenuConfigTests.swift |
||||
# Suggested message: "feat: add Play Next / Add to Queue context-menu items" |
||||
``` |
||||
|
||||
--- |
||||
|
||||
## Task 5: `QueueView` panel + `PlayerControlsView` toggle button |
||||
|
||||
**Files:** |
||||
- Create: `Music/Views/QueueView.swift` |
||||
- Modify: `Music/Views/PlayerControlsView.swift` |
||||
|
||||
This task is UI; it is verified by a clean build and (optionally) by running the app, not by unit tests. |
||||
|
||||
- [ ] **Step 1: Create `QueueView`** |
||||
|
||||
Create `Music/Views/QueueView.swift`: |
||||
|
||||
```swift |
||||
import SwiftUI |
||||
|
||||
// The right-docked "Up Next" panel. The manual "Queue" section is reorderable and |
||||
// removable; the "Next from" section is the read-only upcoming context (double-click |
||||
// a row to jump to it). |
||||
struct QueueView: View { |
||||
var player: PlayerViewModel |
||||
|
||||
var body: some View { |
||||
List { |
||||
if player.manualQueue.isEmpty && player.upcomingContext.isEmpty { |
||||
Text("Queue is empty.\nRight-click a track → Add to Queue.") |
||||
.font(.system(size: 12)) |
||||
.foregroundStyle(.secondary) |
||||
.multilineTextAlignment(.center) |
||||
.frame(maxWidth: .infinity, alignment: .center) |
||||
.padding(.vertical, 24) |
||||
.listRowSeparator(.hidden) |
||||
} |
||||
|
||||
if !player.manualQueue.isEmpty { |
||||
Section("Queue") { |
||||
ForEach(player.manualQueue) { entry in |
||||
HStack(spacing: 8) { |
||||
trackRow(entry.track) |
||||
Spacer() |
||||
Button { |
||||
if let idx = player.manualQueue.firstIndex(where: { $0.id == entry.id }) { |
||||
player.removeFromQueue(at: IndexSet(integer: idx)) |
||||
} |
||||
} label: { |
||||
Image(systemName: "xmark.circle.fill") |
||||
.foregroundStyle(.tertiary) |
||||
} |
||||
.buttonStyle(.plain) |
||||
} |
||||
} |
||||
.onMove(perform: player.moveInQueue) |
||||
} |
||||
} |
||||
|
||||
if !player.upcomingContext.isEmpty { |
||||
Section("Next from: \(player.contextName ?? "Library")") { |
||||
ForEach(Array(player.upcomingContext.enumerated()), id: \.offset) { _, track in |
||||
trackRow(track) |
||||
.contentShape(Rectangle()) |
||||
.onTapGesture(count: 2) { player.play(track) } |
||||
} |
||||
} |
||||
} |
||||
} |
||||
.listStyle(.inset) |
||||
.frame(width: 280) |
||||
} |
||||
|
||||
private func trackRow(_ track: Track) -> some View { |
||||
VStack(alignment: .leading, spacing: 2) { |
||||
Text(track.title) |
||||
.font(.system(size: 12, weight: .medium)) |
||||
.lineLimit(1) |
||||
Text(track.artist) |
||||
.font(.system(size: 10)) |
||||
.foregroundStyle(.secondary) |
||||
.lineLimit(1) |
||||
} |
||||
} |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 2: Add toggle inputs to `PlayerControlsView`** |
||||
|
||||
In `Music/Views/PlayerControlsView.swift`, add three properties immediately after `var contextMenuConfig: TrackContextMenuConfig? = nil` (line 23): |
||||
|
||||
```swift |
||||
var isQueueVisible: Bool = false |
||||
var showQueueButton: Bool = true |
||||
var onToggleQueue: (() -> Void)? = nil |
||||
``` |
||||
|
||||
- [ ] **Step 3: Render the queue button** |
||||
|
||||
In the same file, replace the start of `volumeSection`: |
||||
|
||||
```swift |
||||
private var volumeSection: some View { |
||||
HStack(spacing: 8) { |
||||
Image(systemName: volumeIconName) |
||||
``` |
||||
|
||||
with: |
||||
|
||||
```swift |
||||
private var volumeSection: some View { |
||||
HStack(spacing: 8) { |
||||
if showQueueButton { |
||||
Button(action: { onToggleQueue?() }) { |
||||
Image(systemName: "list.bullet") |
||||
.font(.system(size: 13)) |
||||
.foregroundStyle(isQueueVisible ? .blue : .secondary) |
||||
} |
||||
.buttonStyle(.plain) |
||||
} |
||||
|
||||
Image(systemName: volumeIconName) |
||||
``` |
||||
|
||||
- [ ] **Step 4: Build to verify it compiles** |
||||
|
||||
Run: `xcodebuild build -scheme Music -destination 'platform=macOS' 2>&1 | tail -20` |
||||
Expected: `** BUILD SUCCEEDED **` (the new toggle props are unused until Task 6 wires them — defaults keep `ContentView` compiling). |
||||
|
||||
- [ ] **Step 5: Commit (checkpoint — via `/commit`)** |
||||
|
||||
```bash |
||||
git add Music/Views/QueueView.swift Music/Views/PlayerControlsView.swift |
||||
# Suggested message: "feat: add Up Next QueueView panel and transport queue toggle" |
||||
``` |
||||
|
||||
--- |
||||
|
||||
## Task 6: Wire everything into `ContentView` |
||||
|
||||
**Files:** |
||||
- Modify: `Music/ContentView.swift` |
||||
|
||||
UI integration — verified by a full build and the complete test suite. |
||||
|
||||
- [ ] **Step 1: Add panel visibility state** |
||||
|
||||
In `Music/ContentView.swift`, add to the `@State` block (e.g. after `@State private var showHome = false`, line 24): |
||||
|
||||
```swift |
||||
@State private var showQueue = false |
||||
``` |
||||
|
||||
- [ ] **Step 2: Wire the queue closures into the context-menu config** |
||||
|
||||
Replace the whole `trackContextMenuConfig` computed property (lines 358-377) with: |
||||
|
||||
```swift |
||||
private var trackContextMenuConfig: TrackContextMenuConfig { |
||||
// Queue actions are local-only for v1: hidden when driving a remote device. |
||||
let queueEnabled = !(networkStatus?.isRemoteMode ?? false) |
||||
return TrackContextMenuConfig( |
||||
playlists: playlist.playlists, |
||||
lastUsedPlaylistName: playlist.lastUsedPlaylistName, |
||||
selectedPlaylist: playlist.selectedPlaylist, |
||||
onAddToPlaylist: { track, targetPlaylist in |
||||
try? playlist.addTrack(track, to: targetPlaylist) |
||||
}, |
||||
onAddToLastPlaylist: { track in |
||||
try? playlist.addTrackToLastUsedPlaylist(track) |
||||
}, |
||||
// Outer nil hides the "Remove from Playlist" menu item when not in a playlist view. |
||||
// Inner re-check defends against the playlist being deselected between menu display and action. |
||||
onRemoveFromPlaylist: playlist.selectedPlaylist != nil ? { track in |
||||
if let selected = playlist.selectedPlaylist { |
||||
try? playlist.removeTrack(track, from: selected) |
||||
} |
||||
} : nil, |
||||
onPlayNext: queueEnabled ? { track in player.playNext(track) } : nil, |
||||
onAddToQueue: queueEnabled ? { track in player.addToQueue(track) } : nil |
||||
) |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 3: Dock the panel beside the main content** |
||||
|
||||
In `body`, wrap the main-content region in an `HStack` and append the panel. Replace the opening of that region (currently lines 118-119): |
||||
|
||||
```swift |
||||
VStack(spacing: 0) { |
||||
if showHome || playlist.selectedItem != nil { |
||||
``` |
||||
|
||||
with: |
||||
|
||||
```swift |
||||
HStack(spacing: 0) { |
||||
VStack(spacing: 0) { |
||||
if showHome || playlist.selectedItem != nil { |
||||
``` |
||||
|
||||
Then replace its closing `.frame(maxHeight: .infinity)` (currently line 191) with: |
||||
|
||||
```swift |
||||
} |
||||
.frame(maxHeight: .infinity) |
||||
|
||||
if showQueue { |
||||
Divider() |
||||
QueueView(player: player) |
||||
} |
||||
} |
||||
``` |
||||
|
||||
(The first `}` closes the existing inner `VStack`; the new outer `}` closes the added `HStack`.) |
||||
|
||||
- [ ] **Step 4: Pass context labels at every `setQueue` call site** |
||||
|
||||
Make these four edits in `ContentView.swift`: |
||||
|
||||
1. HomeView `onTrackDoubleClick` (line 155): `player.setQueue(recentTracks)` → `player.setQueue(recentTracks, contextName: "Recently Added")` |
||||
2. TrackTableView `onDoubleClick` (line 178): `player.setQueue(trackList)` → `player.setQueue(trackList, contextName: playlist.selectedItem?.name ?? "Library")` |
||||
3. `onPlayPause` empty-state (line 393): `player.setQueue(trackList)` → `player.setQueue(trackList, contextName: playlist.selectedItem?.name ?? "Library")` |
||||
4. Keyboard space handler (line 426): `player.setQueue(trackList)` → `player.setQueue(trackList, contextName: playlist.selectedItem?.name ?? "Library")` |
||||
|
||||
- [ ] **Step 5: Pass toggle props to `PlayerControlsView`** |
||||
|
||||
In the `playerControls` computed property, add these arguments after `onNowPlayingTap:` and before `contextMenuConfig:` (line 408-409): |
||||
|
||||
```swift |
||||
onNowPlayingTap: { scrollToPlayingTrigger = UUID() }, |
||||
isQueueVisible: showQueue, |
||||
showQueueButton: !(networkStatus?.isRemoteMode ?? false), |
||||
onToggleQueue: { showQueue.toggle() }, |
||||
contextMenuConfig: trackContextMenuConfig |
||||
``` |
||||
|
||||
- [ ] **Step 6: Build to verify it compiles** |
||||
|
||||
Run: `xcodebuild build -scheme Music -destination 'platform=macOS' 2>&1 | tail -20` |
||||
Expected: `** BUILD SUCCEEDED **`. |
||||
|
||||
- [ ] **Step 7: Run the full test suite** |
||||
|
||||
Run: `xcodebuild test -scheme Music -destination 'platform=macOS' 2>&1 | tail -30` |
||||
Expected: all tests pass, including the new queue tests and all pre-existing suites. |
||||
|
||||
- [ ] **Step 8: Manual verification (optional but recommended)** |
||||
|
||||
Use the `/run` or `/verify` skill to launch the app and confirm: |
||||
- Right-click a track → "Play Next" and "Add to Queue" appear and work. |
||||
- The transport `list.bullet` button toggles the right panel. |
||||
- Queued tracks show under "Queue", reorder by drag, and remove via the × button. |
||||
- Playing a queued track removes it from the panel; after the queue drains, the original playlist resumes at the right spot. |
||||
|
||||
- [ ] **Step 9: Commit (checkpoint — via `/commit`)** |
||||
|
||||
```bash |
||||
git add Music/ContentView.swift |
||||
# Suggested message: "feat: wire Up Next panel, queue toggle, and queue actions into ContentView" |
||||
``` |
||||
|
||||
--- |
||||
|
||||
## Self-Review Notes (for the implementer) |
||||
|
||||
- **Backward compatibility:** `queue`/`currentIndex` keep meaning the *context*; every pre-existing `PlayerViewModelTests` case must stay green at each task. If one breaks, the new logic touched the context path incorrectly. |
||||
- **Remote gate:** queue methods early-return when `remoteProvider != nil`, and `ContentView` passes `nil` queue closures + hides the button when `networkStatus.isRemoteMode`. Streaming-client mode is *not* gated (it plays locally). |
||||
- **Duplicates:** `QueueEntry.id` (UUID) is the SwiftUI identity, so the same track can be queued multiple times without row glitches; removal looks up the entry by `id`, never by track. |
||||
File diff suppressed because it is too large
Load Diff
@ -0,0 +1,525 @@ |
||||
# Track Context Menu on Bottom Controls — Implementation Plan |
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
||||
|
||||
**Goal:** Right-clicking the now-playing area at bottom-left shows the same Add/Remove playlist context menu as right-clicking a track row in the track table. |
||||
|
||||
**Architecture:** Introduce a `TrackContextMenuConfig` value type that bundles all menu data + callbacks. A new `TrackContextMenuModifier` SwiftUI view modifier applies `.contextMenu` using that config. `TrackTableView` is refactored to accept a single `contextMenuConfig` parameter (replacing six individual playlist params), and `PlayerControlsView` gains the same optional parameter with the modifier applied to `nowPlayingSection`. `ContentView` constructs one config and passes it to both views. |
||||
|
||||
**Tech Stack:** Swift 5.9+, SwiftUI `.contextMenu`, AppKit `NSMenu` (table view keeps existing AppKit path), Swift Testing framework. |
||||
|
||||
--- |
||||
|
||||
## File Map |
||||
|
||||
| File | Action | |
||||
|------|--------| |
||||
| `Music/Models/TrackContextMenuConfig.swift` | **Create** — value type holding playlists + callbacks | |
||||
| `Music/Views/TrackContextMenuModifier.swift` | **Create** — SwiftUI ViewModifier + View extension | |
||||
| `MusicTests/TrackContextMenuConfigTests.swift` | **Create** — unit tests for config struct | |
||||
| `Music/Views/TrackTableView.swift` | **Modify** — replace 6 playlist params with `contextMenuConfig`, update `menuNeedsUpdate` | |
||||
| `Music/Views/PlayerControlsView.swift` | **Modify** — add `contextMenuConfig` param, apply modifier to `nowPlayingSection` | |
||||
| `Music/ContentView.swift` | **Modify** — construct and pass `TrackContextMenuConfig` to both views | |
||||
|
||||
--- |
||||
|
||||
### Task 1: Create `TrackContextMenuConfig` |
||||
|
||||
**Files:** |
||||
- Create: `Music/Models/TrackContextMenuConfig.swift` |
||||
- Test: `MusicTests/TrackContextMenuConfigTests.swift` |
||||
|
||||
- [ ] **Step 1: Write the failing test** |
||||
|
||||
Create `MusicTests/TrackContextMenuConfigTests.swift`: |
||||
|
||||
```swift |
||||
import Testing |
||||
@testable import Music |
||||
|
||||
struct TrackContextMenuConfigTests { |
||||
// Builds a config with all fields set and verifies: |
||||
// - stored playlists, lastUsedPlaylistName, selectedPlaylist match the inputs |
||||
// - onAddToPlaylist callback fires with the correct track and playlist |
||||
// - onAddToLastPlaylist callback fires with the correct track |
||||
// - onRemoveFromPlaylist callback fires with the correct track |
||||
// - when optional callbacks are nil, optionally calling them is safe |
||||
|
||||
@Test func storesPropertiesAndFiresCallbacks() { |
||||
// 1. Create fixture data |
||||
let pl1 = Playlist.fixture(id: 1, name: "Favorites") |
||||
let pl2 = Playlist.fixture(id: 2, name: "Chill") |
||||
let track = Track.fixture(id: 42, title: "Test") |
||||
|
||||
var addedTrack: Track? = nil |
||||
var addedPlaylist: Playlist? = nil |
||||
var lastTrack: Track? = nil |
||||
var removedTrack: Track? = nil |
||||
|
||||
// 2. Build config with all callbacks |
||||
let config = TrackContextMenuConfig( |
||||
playlists: [pl1, pl2], |
||||
lastUsedPlaylistName: "Favorites", |
||||
selectedPlaylist: pl1, |
||||
onAddToPlaylist: { t, p in addedTrack = t; addedPlaylist = p }, |
||||
onAddToLastPlaylist: { t in lastTrack = t }, |
||||
onRemoveFromPlaylist: { t in removedTrack = t } |
||||
) |
||||
|
||||
// 3. Verify stored properties |
||||
#expect(config.playlists.count == 2) |
||||
#expect(config.playlists[0].name == "Favorites") |
||||
#expect(config.lastUsedPlaylistName == "Favorites") |
||||
#expect(config.selectedPlaylist == pl1) |
||||
|
||||
// 4. Invoke callbacks and verify they fire correctly |
||||
config.onAddToPlaylist(track, pl2) |
||||
config.onAddToLastPlaylist?(track) |
||||
config.onRemoveFromPlaylist?(track) |
||||
|
||||
#expect(addedTrack?.id == track.id) |
||||
#expect(addedPlaylist?.id == pl2.id) |
||||
#expect(lastTrack?.id == track.id) |
||||
#expect(removedTrack?.id == track.id) |
||||
} |
||||
|
||||
@Test func nilOptionalCallbacksAreSafe() { |
||||
// Verifies that a config with nil optional callbacks does not crash |
||||
// when you call them via optional chaining (the normal usage pattern) |
||||
let pl = Playlist.fixture(id: 1, name: "Rock") |
||||
let track = Track.fixture() |
||||
|
||||
let config = TrackContextMenuConfig( |
||||
playlists: [pl], |
||||
lastUsedPlaylistName: nil, |
||||
selectedPlaylist: nil, |
||||
onAddToPlaylist: { _, _ in }, |
||||
onAddToLastPlaylist: nil, |
||||
onRemoveFromPlaylist: nil |
||||
) |
||||
|
||||
// These must not crash |
||||
config.onAddToLastPlaylist?(track) |
||||
config.onRemoveFromPlaylist?(track) |
||||
|
||||
#expect(config.lastUsedPlaylistName == nil) |
||||
#expect(config.selectedPlaylist == nil) |
||||
} |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 2: Run tests to confirm they fail (type not found)** |
||||
|
||||
``` |
||||
xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing MusicTests/TrackContextMenuConfigTests 2>&1 | grep -E "error:|FAIL|PASS|warning: cannot" |
||||
``` |
||||
|
||||
Expected: compile error — `TrackContextMenuConfig` not found. |
||||
|
||||
- [ ] **Step 3: Create `Music/Models/TrackContextMenuConfig.swift`** |
||||
|
||||
```swift |
||||
import Foundation |
||||
|
||||
struct TrackContextMenuConfig { |
||||
let playlists: [Playlist] |
||||
let lastUsedPlaylistName: String? |
||||
let selectedPlaylist: Playlist? |
||||
let onAddToPlaylist: (Track, Playlist) -> Void |
||||
let onAddToLastPlaylist: ((Track) -> Void)? |
||||
let onRemoveFromPlaylist: ((Track) -> Void)? |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 4: Add the new file to the Xcode project** |
||||
|
||||
In Xcode, right-click the `Models` group in the project navigator → **Add Files to "Music"** → select `TrackContextMenuConfig.swift`. Make sure "Add to targets: Music" is checked. |
||||
|
||||
- [ ] **Step 5: Run tests to confirm they pass** |
||||
|
||||
``` |
||||
xcodebuild test -scheme Music -destination 'platform=macOS' -only-testing MusicTests/TrackContextMenuConfigTests 2>&1 | grep -E "Test.*passed|Test.*failed|error:" |
||||
``` |
||||
|
||||
Expected: both tests pass. |
||||
|
||||
- [ ] **Step 6: Commit** |
||||
|
||||
```bash |
||||
git add Music/Models/TrackContextMenuConfig.swift MusicTests/TrackContextMenuConfigTests.swift |
||||
git commit -m "feat: add TrackContextMenuConfig value type" |
||||
``` |
||||
|
||||
--- |
||||
|
||||
### Task 2: Create `TrackContextMenuModifier` |
||||
|
||||
**Files:** |
||||
- Create: `Music/Views/TrackContextMenuModifier.swift` |
||||
|
||||
> No unit test for this task — SwiftUI view modifier behaviour requires UI/snapshot testing not set up in this project. Manual verification is in Task 5. |
||||
|
||||
- [ ] **Step 1: Create `Music/Views/TrackContextMenuModifier.swift`** |
||||
|
||||
```swift |
||||
import SwiftUI |
||||
|
||||
// Attaches a context menu matching the track table's right-click menu. |
||||
// No-ops silently when track or config is nil so callers can pass optionals freely. |
||||
struct TrackContextMenuModifier: ViewModifier { |
||||
let track: Track? |
||||
let config: TrackContextMenuConfig? |
||||
|
||||
func body(content: Content) -> some View { |
||||
if let track, let config { |
||||
content.contextMenu { |
||||
if let lastPlaylistName = config.lastUsedPlaylistName, |
||||
let onAddToLastPlaylist = config.onAddToLastPlaylist { |
||||
Button("Add to \(lastPlaylistName)") { |
||||
onAddToLastPlaylist(track) |
||||
} |
||||
Divider() |
||||
} |
||||
|
||||
if !config.playlists.isEmpty { |
||||
Menu("Add to Playlist") { |
||||
ForEach(config.playlists) { playlist in |
||||
Button(playlist.name) { |
||||
config.onAddToPlaylist(track, playlist) |
||||
} |
||||
} |
||||
} |
||||
} |
||||
|
||||
if config.selectedPlaylist != nil, |
||||
let onRemoveFromPlaylist = config.onRemoveFromPlaylist { |
||||
Divider() |
||||
Button("Remove from Playlist") { |
||||
onRemoveFromPlaylist(track) |
||||
} |
||||
} |
||||
} |
||||
} else { |
||||
content |
||||
} |
||||
} |
||||
} |
||||
|
||||
extension View { |
||||
func trackContextMenu(track: Track?, config: TrackContextMenuConfig?) -> some View { |
||||
modifier(TrackContextMenuModifier(track: track, config: config)) |
||||
} |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 2: Add the new file to the Xcode project** |
||||
|
||||
In Xcode, right-click the `Views` group → **Add Files to "Music"** → select `TrackContextMenuModifier.swift`. Make sure "Add to targets: Music" is checked. |
||||
|
||||
- [ ] **Step 3: Build to confirm it compiles** |
||||
|
||||
``` |
||||
xcodebuild build -scheme Music -destination 'platform=macOS' 2>&1 | grep -E "error:|BUILD SUCCEEDED|BUILD FAILED" |
||||
``` |
||||
|
||||
Expected: `BUILD SUCCEEDED`. |
||||
|
||||
- [ ] **Step 4: Commit** |
||||
|
||||
```bash |
||||
git add Music/Views/TrackContextMenuModifier.swift |
||||
git commit -m "feat: add TrackContextMenuModifier SwiftUI view modifier" |
||||
``` |
||||
|
||||
--- |
||||
|
||||
### Task 3: Refactor `TrackTableView` to use `contextMenuConfig` |
||||
|
||||
**Files:** |
||||
- Modify: `Music/Views/TrackTableView.swift:45-50` (replace 6 playlist params) |
||||
- Modify: `Music/Views/TrackTableView.swift:333-394` (update `menuNeedsUpdate` + action handlers) |
||||
|
||||
- [ ] **Step 1: Replace the 6 individual playlist properties with `contextMenuConfig`** |
||||
|
||||
In `Music/Views/TrackTableView.swift`, find lines 45–51: |
||||
|
||||
```swift |
||||
var playlists: [Playlist] |
||||
var lastUsedPlaylistName: String? |
||||
var selectedPlaylist: Playlist? |
||||
var onAddToPlaylist: ((Track, Playlist) -> Void)? |
||||
var onAddToLastPlaylist: ((Track) -> Void)? |
||||
var onRemoveFromPlaylist: ((Track) -> Void)? |
||||
var onReorder: ((Int, Int) -> Void)? |
||||
``` |
||||
|
||||
Replace with: |
||||
|
||||
```swift |
||||
var contextMenuConfig: TrackContextMenuConfig? |
||||
var onReorder: ((Int, Int) -> Void)? |
||||
``` |
||||
|
||||
- [ ] **Step 2: Update `menuNeedsUpdate` to read from `contextMenuConfig`** |
||||
|
||||
Find the entire `menuNeedsUpdate` method (lines ~333–375) and replace it: |
||||
|
||||
```swift |
||||
func menuNeedsUpdate(_ menu: NSMenu) { |
||||
menu.removeAllItems() |
||||
guard let tableView, tableView.clickedRow >= 0, tableView.clickedRow < tracks.count else { return } |
||||
guard let config = parent.contextMenuConfig else { return } |
||||
|
||||
if let lastPlaylistName = config.lastUsedPlaylistName, config.onAddToLastPlaylist != nil { |
||||
let lastItem = NSMenuItem( |
||||
title: "Add to \(lastPlaylistName)", |
||||
action: #selector(addToLastPlaylist(_:)), |
||||
keyEquivalent: "" |
||||
) |
||||
lastItem.target = self |
||||
menu.addItem(lastItem) |
||||
menu.addItem(.separator()) |
||||
} |
||||
|
||||
if !config.playlists.isEmpty { |
||||
let submenu = NSMenu() |
||||
for (index, playlist) in config.playlists.enumerated() { |
||||
let item = NSMenuItem( |
||||
title: playlist.name, |
||||
action: #selector(addToPlaylist(_:)), |
||||
keyEquivalent: "" |
||||
) |
||||
item.target = self |
||||
item.tag = index |
||||
submenu.addItem(item) |
||||
} |
||||
let submenuItem = NSMenuItem(title: "Add to Playlist", action: nil, keyEquivalent: "") |
||||
submenuItem.submenu = submenu |
||||
menu.addItem(submenuItem) |
||||
} |
||||
|
||||
if config.selectedPlaylist != nil, config.onRemoveFromPlaylist != nil { |
||||
menu.addItem(.separator()) |
||||
let removeItem = NSMenuItem( |
||||
title: "Remove from Playlist", |
||||
action: #selector(removeFromPlaylist(_:)), |
||||
keyEquivalent: "" |
||||
) |
||||
removeItem.target = self |
||||
menu.addItem(removeItem) |
||||
} |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 3: Update the three `@objc` menu action methods to use `contextMenuConfig`** |
||||
|
||||
Find `addToPlaylist`, `addToLastPlaylist`, and `removeFromPlaylist` (lines ~377–394) and replace all three: |
||||
|
||||
```swift |
||||
@objc func addToPlaylist(_ sender: NSMenuItem) { |
||||
guard let tableView, tableView.clickedRow >= 0, tableView.clickedRow < tracks.count else { return } |
||||
guard let config = parent.contextMenuConfig else { return } |
||||
let track = tracks[tableView.clickedRow] |
||||
let playlist = config.playlists[sender.tag] |
||||
config.onAddToPlaylist(track, playlist) |
||||
} |
||||
|
||||
@objc func addToLastPlaylist(_ sender: NSMenuItem) { |
||||
guard let tableView, tableView.clickedRow >= 0, tableView.clickedRow < tracks.count else { return } |
||||
guard let config = parent.contextMenuConfig else { return } |
||||
let track = tracks[tableView.clickedRow] |
||||
config.onAddToLastPlaylist?(track) |
||||
} |
||||
|
||||
@objc func removeFromPlaylist(_ sender: NSMenuItem) { |
||||
guard let tableView, tableView.clickedRow >= 0, tableView.clickedRow < tracks.count else { return } |
||||
guard let config = parent.contextMenuConfig else { return } |
||||
let track = tracks[tableView.clickedRow] |
||||
config.onRemoveFromPlaylist?(track) |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 4: Build — expect ContentView compile errors (call site not yet updated)** |
||||
|
||||
``` |
||||
xcodebuild build -scheme Music -destination 'platform=macOS' 2>&1 | grep "error:" |
||||
``` |
||||
|
||||
Expected: errors in `ContentView.swift` about removed parameters. `TrackTableView.swift` itself should be clean. |
||||
|
||||
- [ ] **Step 5: Commit** |
||||
|
||||
```bash |
||||
git add Music/Views/TrackTableView.swift |
||||
git commit -m "refactor: replace TrackTableView playlist params with TrackContextMenuConfig" |
||||
``` |
||||
|
||||
--- |
||||
|
||||
### Task 4: Add `contextMenuConfig` to `PlayerControlsView` |
||||
|
||||
**Files:** |
||||
- Modify: `Music/Views/PlayerControlsView.swift:22` (add param after `onNowPlayingTap`) |
||||
- Modify: `Music/Views/PlayerControlsView.swift:112-118` (apply modifier to `nowPlayingSection`) |
||||
|
||||
- [ ] **Step 1: Add the new parameter to `PlayerControlsView`** |
||||
|
||||
In `Music/Views/PlayerControlsView.swift`, find line 22: |
||||
|
||||
```swift |
||||
let onNowPlayingTap: () -> Void |
||||
``` |
||||
|
||||
Add after it: |
||||
|
||||
```swift |
||||
let onNowPlayingTap: () -> Void |
||||
var contextMenuConfig: TrackContextMenuConfig? = nil |
||||
``` |
||||
|
||||
- [ ] **Step 2: Apply the modifier to `nowPlayingSection`** |
||||
|
||||
In `PlayerControlsView.swift`, find the closing of `nowPlayingSection` (lines ~112–118): |
||||
|
||||
```swift |
||||
.contentShape(Rectangle()) |
||||
.onTapGesture { |
||||
if currentTrack != nil { |
||||
onNowPlayingTap() |
||||
} |
||||
} |
||||
} |
||||
``` |
||||
|
||||
Replace with: |
||||
|
||||
```swift |
||||
.contentShape(Rectangle()) |
||||
.onTapGesture { |
||||
if currentTrack != nil { |
||||
onNowPlayingTap() |
||||
} |
||||
} |
||||
.trackContextMenu(track: currentTrack, config: contextMenuConfig) |
||||
} |
||||
``` |
||||
|
||||
- [ ] **Step 3: Build — expect ContentView errors only** |
||||
|
||||
``` |
||||
xcodebuild build -scheme Music -destination 'platform=macOS' 2>&1 | grep "error:" |
||||
``` |
||||
|
||||
Expected: only `ContentView.swift` errors remain (old params still passed there). |
||||
|
||||
- [ ] **Step 4: Commit** |
||||
|
||||
```bash |
||||
git add Music/Views/PlayerControlsView.swift |
||||
git commit -m "feat: add contextMenuConfig param to PlayerControlsView" |
||||
``` |
||||
|
||||
--- |
||||
|
||||
### Task 5: Update `ContentView` — wire both call sites |
||||
|
||||
**Files:** |
||||
- Modify: `Music/ContentView.swift:181-194` (TrackTableView call site) |
||||
- Modify: `Music/ContentView.swift:333-362` (PlayerControlsView call site) |
||||
|
||||
- [ ] **Step 1: Replace the TrackTableView playlist params with `contextMenuConfig`** |
||||
|
||||
In `Music/ContentView.swift`, find lines 181–200 (inside `TrackTableView(...)`): |
||||
|
||||
```swift |
||||
playlists: playlist.playlists, |
||||
lastUsedPlaylistName: playlist.lastUsedPlaylistName, |
||||
selectedPlaylist: playlist.selectedPlaylist, |
||||
onAddToPlaylist: { track, targetPlaylist in |
||||
try? playlist.addTrack(track, to: targetPlaylist) |
||||
}, |
||||
onAddToLastPlaylist: { track in |
||||
try? playlist.addTrackToLastUsedPlaylist(track) |
||||
}, |
||||
onRemoveFromPlaylist: playlist.selectedPlaylist != nil ? { track in |
||||
if let selected = playlist.selectedPlaylist { |
||||
try? playlist.removeTrack(track, from: selected) |
||||
} |
||||
} : nil, |
||||
``` |
||||
|
||||
Replace with: |
||||
|
||||
```swift |
||||
contextMenuConfig: TrackContextMenuConfig( |
||||
playlists: playlist.playlists, |
||||
lastUsedPlaylistName: playlist.lastUsedPlaylistName, |
||||
selectedPlaylist: playlist.selectedPlaylist, |
||||
onAddToPlaylist: { track, targetPlaylist in |
||||
try? playlist.addTrack(track, to: targetPlaylist) |
||||
}, |
||||
onAddToLastPlaylist: { track in |
||||
try? playlist.addTrackToLastUsedPlaylist(track) |
||||
}, |
||||
onRemoveFromPlaylist: playlist.selectedPlaylist != nil ? { track in |
||||
if let selected = playlist.selectedPlaylist { |
||||
try? playlist.removeTrack(track, from: selected) |
||||
} |
||||
} : nil |
||||
), |
||||
``` |
||||
|
||||
- [ ] **Step 2: Pass `contextMenuConfig` to `PlayerControlsView`** |
||||
|
||||
In `Music/ContentView.swift`, find the `PlayerControlsView(...)` block (lines ~333–362). Add `contextMenuConfig` after `onNowPlayingTap`: |
||||
|
||||
```swift |
||||
onNowPlayingTap: { scrollToPlayingTrigger = UUID() }, |
||||
contextMenuConfig: TrackContextMenuConfig( |
||||
playlists: playlist.playlists, |
||||
lastUsedPlaylistName: playlist.lastUsedPlaylistName, |
||||
selectedPlaylist: playlist.selectedPlaylist, |
||||
onAddToPlaylist: { track, targetPlaylist in |
||||
try? playlist.addTrack(track, to: targetPlaylist) |
||||
}, |
||||
onAddToLastPlaylist: { track in |
||||
try? playlist.addTrackToLastUsedPlaylist(track) |
||||
}, |
||||
onRemoveFromPlaylist: playlist.selectedPlaylist != nil ? { track in |
||||
if let selected = playlist.selectedPlaylist { |
||||
try? playlist.removeTrack(track, from: selected) |
||||
} |
||||
} : nil |
||||
) |
||||
``` |
||||
|
||||
- [ ] **Step 3: Build cleanly** |
||||
|
||||
``` |
||||
xcodebuild build -scheme Music -destination 'platform=macOS' 2>&1 | grep -E "error:|BUILD SUCCEEDED|BUILD FAILED" |
||||
``` |
||||
|
||||
Expected: `BUILD SUCCEEDED` with no errors. |
||||
|
||||
- [ ] **Step 4: Run the full test suite** |
||||
|
||||
``` |
||||
xcodebuild test -scheme Music -destination 'platform=macOS' 2>&1 | grep -E "Test.*passed|Test.*failed|error:|BUILD" |
||||
``` |
||||
|
||||
Expected: all tests pass, including `TrackContextMenuConfigTests`. |
||||
|
||||
- [ ] **Step 5: Manual verification** |
||||
|
||||
Run the app. With at least one playlist created: |
||||
|
||||
1. **Track table:** right-click any track row → confirm the menu shows "Add to [last]" / "Add to Playlist" submenu / "Remove from Playlist" (when in a playlist view). Behaviour must be unchanged. |
||||
2. **Bottom controls:** play a track, then right-click anywhere on the now-playing area (album art + title + artist) → confirm the same menu appears. |
||||
3. **No track playing:** right-click the empty now-playing area → confirm no menu appears (the modifier is a no-op when `currentTrack` is nil). |
||||
|
||||
- [ ] **Step 6: Commit** |
||||
|
||||
```bash |
||||
git add Music/ContentView.swift |
||||
git commit -m "feat: wire TrackContextMenuConfig to bottom controls and track table" |
||||
``` |
||||
File diff suppressed because it is too large
Load Diff
@ -0,0 +1,110 @@ |
||||
# Add "New Playlist…" to the Add-to-Playlist menu |
||||
|
||||
**Date:** 2026-05-30 |
||||
**Status:** Approved design |
||||
|
||||
## Goal |
||||
|
||||
In a track's right-click "Add to Playlist" submenu, let the user create a brand-new |
||||
regular playlist on the fly: pick **New Playlist…**, enter a name, and on save the |
||||
playlist is created and the track is added to it. |
||||
|
||||
## Background |
||||
|
||||
The "Add to Playlist" submenu lives in `TrackContextMenuModifier.swift` and is driven by |
||||
the data-only `TrackContextMenuConfig` struct (a `playlists` array plus action closures), |
||||
built in `ContentView.trackContextMenuConfig` (`ContentView.swift:415`). |
||||
|
||||
The app already creates playlists from the sidebar via an `.alert` + `TextField` |
||||
(`ContentView.swift:273`), calling `PlaylistViewModel.createPlaylist(name:)` → |
||||
`DatabaseService.createPlaylist(name:) -> Playlist`. Adding a track is |
||||
`PlaylistViewModel.addTrack(_:to:)`, which also records the playlist as last-used. |
||||
|
||||
Regular and smart playlists are separate tables; this feature only creates **regular** |
||||
playlists (`Playlist`). |
||||
|
||||
## Approach |
||||
|
||||
Route the name prompt up to `ContentView`, which already owns the new-playlist alert. |
||||
|
||||
SwiftUI alerts do not present reliably when attached *inside* a context menu's content |
||||
(the menu dismisses, and the per-row modifier is re-instantiated). So the menu item only |
||||
signals intent — "create a new playlist for this track" — and `ContentView` owns the |
||||
prompt and the orchestration. This reuses the existing alert pattern rather than |
||||
duplicating it inside the modifier (the rejected alternative, which would also need the |
||||
modifier to reach into `PlaylistViewModel`). |
||||
|
||||
## Behavior decisions |
||||
|
||||
- Adds **only the clicked track** (matches the current single-track "Add to Playlist"). |
||||
- **No navigation** — after create+add the sidebar selection is unchanged. |
||||
- Empty / whitespace-only name → no-op (matches the existing create flow). |
||||
- When the user has **no** existing playlists, the submenu still appears showing just |
||||
"New Playlist…" (today the whole submenu is hidden when `playlists` is empty). |
||||
- Remote mode → wired the same way as the existing "Add to Playlist" action |
||||
(unconditionally), keeping parity with current behavior. |
||||
|
||||
## Changes |
||||
|
||||
### 1. `PlaylistViewModel` (`Music/ViewModels/PlaylistViewModel.swift`) |
||||
|
||||
Add one orchestration method — the unit under test: |
||||
|
||||
```swift |
||||
@discardableResult |
||||
func createPlaylistAndAddTrack(name: String, track: Track) throws -> Playlist { |
||||
let playlist = try db.createPlaylist(name: name) |
||||
try addTrack(track, to: playlist) |
||||
return playlist |
||||
} |
||||
``` |
||||
|
||||
`db.createPlaylist` returns a `Playlist` with its assigned `id`; `addTrack` adds the track |
||||
and sets `lastUsedPlaylistId` to the new playlist (so the "Add to <last>" item updates too). |
||||
|
||||
### 2. `TrackContextMenuConfig` (`Music/Models/TrackContextMenuConfig.swift`) |
||||
|
||||
Add a new optional closure, defaulting to `nil` (so all existing call sites and tests |
||||
compile unchanged): |
||||
|
||||
```swift |
||||
let onAddToNewPlaylist: ((Track) -> Void)? |
||||
``` |
||||
|
||||
Add it to the explicit `init` with a `= nil` default, alongside the other optionals. |
||||
|
||||
### 3. `TrackContextMenuModifier` (`Music/Views/TrackContextMenuModifier.swift`) |
||||
|
||||
Inside the "Add to Playlist" submenu: |
||||
|
||||
- Add a **New Playlist…** button at the top (ellipsis = opens a prompt) when |
||||
`onAddToNewPlaylist != nil`, followed by a `Divider` before the list of existing |
||||
playlists. |
||||
- Relax the visibility guard so the submenu shows when |
||||
`!config.playlists.isEmpty || config.onAddToNewPlaylist != nil` (previously hidden |
||||
whenever `playlists` was empty). |
||||
|
||||
### 4. `ContentView` (`Music/ContentView.swift`) |
||||
|
||||
- Add state: `@State private var newPlaylistTrack: Track?` and a name field |
||||
(reuse/parallel the existing `playlistNameInput` style; a dedicated field is fine). |
||||
- In `trackContextMenuConfig`, wire `onAddToNewPlaylist: { track in newPlaylistTrack = track }`. |
||||
- Present an alert (mirroring the existing New Playlist alert) gated on |
||||
`newPlaylistTrack != nil`. **Create** trims whitespace, and if non-empty calls |
||||
`playlist.createPlaylistAndAddTrack(name:track:)` with the pending track; **Cancel** and |
||||
completion both clear `newPlaylistTrack` and the name field. |
||||
|
||||
## Testing (TDD) |
||||
|
||||
Unit-test `PlaylistViewModel.createPlaylistAndAddTrack(name:track:)` against an in-memory |
||||
database: |
||||
|
||||
1. Seed a track in the DB. |
||||
2. Call `createPlaylistAndAddTrack(name:track:)`. |
||||
3. Assert a regular playlist with that name now exists. |
||||
4. Assert the playlist's tracks contain the seeded track. |
||||
5. Assert `lastUsedPlaylistId` equals the new playlist's id. |
||||
|
||||
The SwiftUI menu/alert rendering is not unit-tested, consistent with the rest of the |
||||
codebase. `TrackContextMenuConfig`'s new optional defaults to `nil`, so existing |
||||
`TrackContextMenuConfigTests` are unaffected. |
||||
@ -0,0 +1,166 @@ |
||||
# Fix `bitrate = 0` Tracks — Design |
||||
|
||||
**Date:** 2026-05-30 |
||||
**Status:** Approved (design) |
||||
|
||||
## Problem |
||||
|
||||
Many tracks in the library have a stored `bitrate` of `0`. Bitrate of `0` is |
||||
displayed literally and is meaningless. Users want these tracks to show their |
||||
real bitrate, and want new imports to stop producing the problem. |
||||
|
||||
## Root Cause |
||||
|
||||
`ScannerService.extractMetadata()` extracts bitrate via AVFoundation: |
||||
|
||||
```swift |
||||
// Music/Services/ScannerService.swift (~line 151) |
||||
let estimatedRate = try await audioTrack.load(.estimatedDataRate) |
||||
bitrate = Int(estimatedRate / 1000) // bits/sec -> kbps |
||||
``` |
||||
|
||||
For some files (observed: long / VBR MP3s such as 2-hour DJ "Essential Mix" |
||||
recordings), `AVAsset.estimatedDataRate` returns `0`. `Int(0 / 1000)` is `0`, so |
||||
a literal `0` is written to the DB. The `duration` field is extracted correctly |
||||
for the same files, which is what makes recovery reliable. |
||||
|
||||
Tracks with `bitrate IS NULL` exist as well — the importer never produced any |
||||
value. These display as "—" and are treated as equally "missing" for this work. |
||||
|
||||
### Evidence |
||||
|
||||
Validated against a real `bitrate = 0` row (a ~120 min MP3): |
||||
|
||||
| Method | Result | |
||||
|------------------------------------------|---------------| |
||||
| `fileSize × 8 ÷ duration ÷ 1000` | 256.0 kbps | |
||||
| `ffprobe -show_entries format=bit_rate` | 256005 → 256.0 kbps | |
||||
|
||||
The two agree to the kbps. For VBR, both methods yield the true **average** |
||||
bitrate, which is the meaningful value. |
||||
|
||||
## Scope |
||||
|
||||
Both halves of the problem: |
||||
|
||||
1. **Backfill** existing rows where `bitrate = 0 OR bitrate IS NULL`. |
||||
2. **Fix the importer** so future imports never store `0` again. |
||||
|
||||
No mass rescan is required: the script repairs today's rows; the importer fix |
||||
only needs to guarantee correctness for new imports. |
||||
|
||||
## Component 1 — Importer Fix (`ScannerService`) |
||||
|
||||
Extract the bitrate decision into a pure, unit-testable function: |
||||
|
||||
```swift |
||||
/// Resolve a track's bitrate (kbps) from the OS estimate, with a |
||||
/// file-size/duration fallback. Returns nil when no value can be derived — |
||||
/// never 0. |
||||
static func resolveBitrate(estimatedDataRate: Double, |
||||
fileSizeBytes: Int64?, |
||||
durationSeconds: Double?) -> Int? |
||||
``` |
||||
|
||||
Logic: |
||||
|
||||
- `estimatedDataRate > 0` |
||||
→ `Int((estimatedDataRate / 1000).rounded())` (current behaviour, now rounded |
||||
rather than truncated). |
||||
- else if `fileSizeBytes != nil` **and** `durationSeconds != nil && > 0` |
||||
→ `Int((Double(fileSizeBytes) * 8 / durationSeconds / 1000).rounded())`. |
||||
- else → `nil`. |
||||
|
||||
**Key invariant: the importer never stores `0`.** When nothing can be derived it |
||||
stores `nil`, which the UI already renders as "—". |
||||
|
||||
`extractMetadata()` is updated to: |
||||
|
||||
- read the file size it can already obtain via `FileManager.attributesOfItem(atPath:)`, |
||||
- pass `estimatedDataRate`, file size, and the loaded `duration` into |
||||
`resolveBitrate`, |
||||
- assign the result to `bitrate`. |
||||
|
||||
This keeps the AVFoundation I/O in `extractMetadata` and the arithmetic in a pure |
||||
function that tests can drive directly. |
||||
|
||||
## Component 2 — Backfill Script (`scripts/backfill_bitrate.py`) |
||||
|
||||
Mirrors the conventions of the existing `scripts/backfill_itunes_dates.py`: |
||||
|
||||
- Same `DEFAULT_DB` resolution (`~/Library/Containers/com.staxriver.mu/Data/ |
||||
Library/Application Support/Music/db.sqlite`), with `--db` override. |
||||
- Reuses the `norm_path` / percent-decoding approach to turn a stored `fileURL` |
||||
into a POSIX path. |
||||
- **Dry-run by default**; `--apply` writes after a timestamped backup of the DB. |
||||
- `--self-test` runs offline unit checks and exits. |
||||
- Stdlib only (`sqlite3`, `subprocess`, `os`, `urllib`, …), plus `ffprobe` as an |
||||
optional external tool. |
||||
|
||||
### Selection |
||||
|
||||
```sql |
||||
SELECT id, fileURL, duration, bitrate |
||||
FROM tracks |
||||
WHERE bitrate = 0 OR bitrate IS NULL; |
||||
``` |
||||
|
||||
### Per-row bitrate determination |
||||
|
||||
1. Resolve `fileURL` → POSIX path. If the file does not exist on disk → |
||||
report as **skipped (missing file)**, do not update. |
||||
2. Run `ffprobe -v error -show_entries format=bit_rate -of default=nw=1:nk=1 <path>`. |
||||
- Parse an integer bps → `round(bps / 1000)` kbps. |
||||
3. If ffprobe is **absent**, **errors**, or returns **`N/A`/empty**, fall back to |
||||
the formula: `round(fileSizeBytes * 8 / durationSeconds / 1000)`. |
||||
- If there is also no usable duration → report as |
||||
**skipped (undeterminable)**, do not update. |
||||
|
||||
### Output |
||||
|
||||
- **Dry-run:** a table of `path · old → new` plus a summary. |
||||
- **`--apply`:** create `db.sqlite.bak-<timestamp>`, then `UPDATE tracks SET |
||||
bitrate = ? WHERE id = ?` per resolved row, in a single transaction. |
||||
- **Summary (both modes):** counts for updated, skipped-missing-file, |
||||
skipped-undeterminable, and whether ffprobe was available. |
||||
|
||||
## Testing (TDD — tests written before implementation) |
||||
|
||||
### Swift (`MusicTests`) |
||||
|
||||
Unit tests for `ScannerService.resolveBitrate`: |
||||
|
||||
1. Positive `estimatedDataRate` → rounded kbps (e.g. `320450.0` → `320`). |
||||
2. `estimatedDataRate == 0` with valid size + duration → formula result |
||||
(e.g. 230_358_479 bytes, 7198.54 s → `256`). |
||||
3. `estimatedDataRate == 0`, valid size, **no/zero duration** → `nil`. |
||||
4. `estimatedDataRate == 0`, **no file size** → `nil`. |
||||
5. Confirms the function never returns `0`. |
||||
|
||||
Each test carries a step-by-step comment describing what it exercises. |
||||
|
||||
### Python (`--self-test`) |
||||
|
||||
1. ffprobe-output parsing: `"256005\n"` → `256`. |
||||
2. `N/A`/empty ffprobe output → triggers formula fallback. |
||||
3. Formula math: `(230_358_479 * 8 / 7198.54 / 1000)` rounds to `256`. |
||||
4. `norm_path` edge cases (percent-encoding, `file://localhost/`, NFC, trailing |
||||
slash) — mirrored from the existing script's expectations. |
||||
|
||||
### Manual verification |
||||
|
||||
Run the script in dry-run against the real library DB and eyeball a sample |
||||
(ffprobe vs formula agreement) before `--apply`. |
||||
|
||||
## Operational Notes |
||||
|
||||
- `--apply` writes directly to the SQLite file. **Quit the app first** to avoid |
||||
WAL/lock contention — same caveat as `backfill_itunes_dates.py`. |
||||
- A timestamped backup is created before any write; restore by copying it back. |
||||
|
||||
## Out of Scope |
||||
|
||||
- No new UI (no in-app "repair bitrates" command); the script covers existing |
||||
rows and the importer covers future ones. |
||||
- No change to how bitrate is displayed. |
||||
- No re-encoding or modification of audio files — read-only analysis only. |
||||
@ -0,0 +1,97 @@ |
||||
# iTunes/Music → app DB date & stats backfill (one-time) |
||||
|
||||
Date: 2026-05-30 |
||||
|
||||
## Problem |
||||
|
||||
`ScannerService.extractMetadata` sets `dateAdded: Date()` at scan time |
||||
(`Music/Services/ScannerService.swift:186`), so every track's "added date" in the |
||||
app DB is really its *scan* date, not the date the user originally added it in |
||||
Apple Music. The user wants the **true `Date Added`** (and, since Music.app tracks |
||||
them too, `Play Count`, `Rating`, and last-played) copied from their Apple Music |
||||
library into the app's SQLite database. |
||||
|
||||
## Context (verified) |
||||
|
||||
- The app is **sandboxed**. Current `PRODUCT_BUNDLE_IDENTIFIER` is `com.staxriver.mu` |
||||
(HEAD and working tree; not part of the uncommitted diff). The live DB is therefore at: |
||||
`~/Library/Containers/com.staxriver.mu/Data/Library/Application Support/Music/db.sqlite` |
||||
- **The real app and real library are on a different computer.** This machine only has a |
||||
3-track dev DB. The script must be portable and is intended to run on the other Mac. |
||||
- The user confirmed the audio files are the **same files** Apple Music references (they |
||||
live inside Apple Music's media folder, e.g. |
||||
`~/Music/Music/Media.localized/Music/...`). So the join key is the **file path**. |
||||
- `tracks.dateAdded` is a GRDB `.datetime` column, stored as the string |
||||
`YYYY-MM-DD HH:MM:SS.SSS` in UTC (confirmed from existing rows, e.g. |
||||
`2026-05-24 06:46:01.713`). GRDB is lenient on read, so `....000` round-trips. |
||||
- App rating scale is **0–5 stars** (`TrackTableView.swift:284` renders |
||||
`String(repeating: "★", count: track.rating)`). Music.app stores 0–100, so map `// 20`. |
||||
|
||||
## Approach |
||||
|
||||
A single **stdlib-only Python 3 script**, run once. Source of truth is the Music.app |
||||
**File ▸ Library ▸ Export Library…** XML plist (chosen over live AppleScript: no |
||||
Automation prompt, no timeouts, trivially parseable with `plistlib`). On matched tracks |
||||
it does a **blunt overwrite** of all four fields, with Music.app as the source of truth. |
||||
|
||||
## Matching (the bug-prone part) |
||||
|
||||
Join Music.app `Location` to `tracks.fileURL` on a **normalized decoded POSIX path**, |
||||
not raw URL strings. `norm_path()`: |
||||
|
||||
1. strip leading `file://`, then optional `localhost` host segment, |
||||
2. percent-decode (`urllib.parse.unquote`), |
||||
3. `unicodedata.normalize("NFC", …)` — neutralizes the accented-filename NFC/NFD mismatch |
||||
between APFS storage and the two URL string sources, |
||||
4. strip a trailing slash. |
||||
|
||||
Music tracks with no `Location` (Apple Music streaming entries) are skipped. |
||||
|
||||
## Field mapping (matched rows only; blunt overwrite) |
||||
|
||||
| Column | XML key | Rule | |
||||
|----------------|------------------|-------------------------------------------------------------| |
||||
| `dateAdded` | `Date Added` | `%Y-%m-%d %H:%M:%S.000` UTC. If absent, keep existing (col is NOT NULL). | |
||||
| `playCount` | `Play Count` | integer, `0` if absent. | |
||||
| `rating` | `Rating` (0–100) | `// 20` → 0–5, `0` if absent. | |
||||
| `lastPlayedAt` | `Play Date UTC` | same date format, or `NULL` if absent. | |
||||
|
||||
## Safety |
||||
|
||||
- **Dry-run by default**: prints match rate, a sample of before→after changes, and the |
||||
counts + samples of unmatched-in-DB and unmatched-in-XML. Writes nothing. |
||||
- `--apply`: first copies `db.sqlite` + `-wal` + `-shm` to a timestamped backup, then |
||||
performs all writes in a single transaction, then `PRAGMA wal_checkpoint(TRUNCATE)`. |
||||
Reversible by restoring the backup. |
||||
- The app must be **quit** before running so the sandbox DB isn't mid-write. |
||||
|
||||
## CLI |
||||
|
||||
``` |
||||
python3 scripts/backfill_itunes_dates.py --xml <Library.xml> [--db <path>] [--apply] [--self-test] |
||||
``` |
||||
|
||||
- Default `--db`: `~/Library/Containers/com.staxriver.mu/Data/Library/Application Support/Music/db.sqlite` |
||||
computed from `$HOME`, so it resolves on the other Mac too. |
||||
|
||||
## Testing |
||||
|
||||
`scripts/test_backfill_itunes_dates.py` (stdlib `unittest`): |
||||
|
||||
- `norm_path`: NFC/NFD equivalence, `file://localhost/` form, percent-encoding, |
||||
filenames with spaces/`#`/parentheses/apostrophes. |
||||
- `build_updates`: date formatting, rating `// 20`, playCount & lastPlayed present/absent, |
||||
unmatched-row handling. |
||||
- Integration: a temp SQLite DB with the real `tracks` schema seeded with the user's actual |
||||
3 track paths + scan dates and a synthetic Library.xml → `apply` → assert rows updated. |
||||
|
||||
## Delivery |
||||
|
||||
Script + test live in the repo under `scripts/`. The user commits (via `/commit`), pushes, |
||||
pulls on the real machine, runs **File ▸ Library ▸ Export Library…** there, then runs the |
||||
dry-run, eyeballs the match rate, and re-runs with `--apply`. |
||||
|
||||
## Out of scope |
||||
|
||||
Scanning the library into the app (the user does that in-app first), ongoing/automatic sync, |
||||
and non-file (streaming) tracks. |
||||
@ -0,0 +1,243 @@ |
||||
# Playing Queue — Design |
||||
|
||||
**Date:** 2026-05-30 |
||||
**Status:** Approved (pending spec review) |
||||
|
||||
## Overview |
||||
|
||||
Add a Spotify-style **priority "Up Next" queue** to the Music app. Users can push |
||||
tracks to the front ("Play Next") or end ("Add to Queue") of a manual queue via the |
||||
track context menu. The manual queue plays before the current playback context |
||||
(playlist/album) resumes, survives starting a new context, and is visible and |
||||
editable in a right-docked "Up Next" panel. |
||||
|
||||
## Goals |
||||
|
||||
- "Play Next" and "Add to Queue" actions on the track context menu. |
||||
- A manual queue that takes priority over the playback context and persists when a |
||||
new context (playlist/album/library) starts playing. |
||||
- A visible, right-docked "Up Next" panel showing the manual queue and the upcoming |
||||
context tracks. |
||||
- Drag-to-reorder and remove **within the manual queue**. |
||||
|
||||
## Non-Goals (v1) |
||||
|
||||
- **Remote/streaming support.** When this app is driving a remote device, queue |
||||
actions are hidden and `next()` continues to delegate over the wire. No |
||||
`RemoteProtocol` changes. (Possible follow-up.) |
||||
- **Persistence across app restart.** The queue is in-memory in `PlayerViewModel`. |
||||
- **Reordering the context** from the panel (that is playlist editing, already |
||||
handled elsewhere). The "Next from" section is read-only. |
||||
- **Clear-all-queue** action (not requested). |
||||
- **Multi-select queueing.** Actions operate on the single right-clicked track, |
||||
consistent with the existing "Add to Playlist". |
||||
|
||||
## Resolved Decisions |
||||
|
||||
| Decision | Choice | Reason | |
||||
|---|---|---| |
||||
| Queue model | Spotify-style priority queue (manual queue distinct from context) | User selection | |
||||
| Panel placement | Right-docked slide-out, toggled from transport bar | User selection (mockup A) | |
||||
| Remote scope | Local-only for v1; hide actions in remote mode | User selection | |
||||
| Persistence | In-memory only | User selection | |
||||
| `queue`/`currentIndex` naming | Keep as the **context**; add doc comments | Backward-compatible; avoids touching remote-sync + existing tests | |
||||
| "Next from" section | Read-only, double-click to jump | Context reordering is out of scope | |
||||
|
||||
## Data Model (`PlayerViewModel`) |
||||
|
||||
Existing `queue` / `originalQueue` / `currentIndex` are **retained and keep their |
||||
current meaning: the playback CONTEXT** (playlist/album, with shuffle applied). New |
||||
state is added alongside: |
||||
|
||||
```swift |
||||
private(set) var queue: [Track] // UNCHANGED — the context (shuffled view) |
||||
private var originalQueue: [Track] // UNCHANGED — context, original order |
||||
var currentIndex: Int? // UNCHANGED — index into `queue` (the CONTEXT |
||||
// position), held even while a manual track plays |
||||
private(set) var manualQueue: [QueueEntry] = [] // NEW — priority "Up Next" entries |
||||
private(set) var contextName: String? // NEW — label for "Next from: <name>" |
||||
``` |
||||
|
||||
Each manual entry carries its own identity so the same track can be queued twice |
||||
without SwiftUI confusing the rows (the codebase has prior id-collision bugs): |
||||
|
||||
```swift |
||||
nonisolated struct QueueEntry: Identifiable { |
||||
let id = UUID() |
||||
let track: Track |
||||
} |
||||
``` |
||||
|
||||
A dedicated `playManual(_:)` path plays a queued track **without** touching |
||||
`currentIndex`, so the context position is preserved automatically — no extra "am I |
||||
playing from the queue?" flag is needed. |
||||
|
||||
Computed for the panel: |
||||
|
||||
```swift |
||||
// Context tracks after the current context position; the "Next from" section. |
||||
var upcomingContext: [Track] { |
||||
guard let idx = currentIndex, idx + 1 < queue.count else { return [] } |
||||
return Array(queue[(idx + 1)...]) |
||||
} |
||||
``` |
||||
|
||||
`currentIndex` deliberately tracks the **context** position, not "what is playing." |
||||
While a manual-queue track plays, `currentIndex` stays put so the context resumes at |
||||
the correct spot once the manual queue drains. |
||||
|
||||
## Behavior |
||||
|
||||
### Adding to the queue |
||||
|
||||
```swift |
||||
func playNext(_ track: Track) // insert at front of manualQueue |
||||
func addToQueue(_ track: Track) // append to end of manualQueue |
||||
``` |
||||
|
||||
Both: if **nothing is currently playing** (`currentTrack == nil`), immediately pop |
||||
and play the just-queued track instead of leaving it parked (queue-while-idle starts |
||||
playback). |
||||
|
||||
### Advancing |
||||
|
||||
```swift |
||||
func next() { |
||||
if remoteProvider != nil { remote.sendNext(); return } // UNCHANGED remote path |
||||
if !manualQueue.isEmpty { |
||||
let entry = manualQueue.removeFirst() // consume-on-play |
||||
playManual(entry.track) // currentIndex unchanged |
||||
} else { |
||||
// existing context-advance logic (currentIndex + 1, stop at end) |
||||
} |
||||
} |
||||
``` |
||||
|
||||
- **Consume-on-play:** `removeFirst()` removes the track from "Up Next" the instant |
||||
it starts. The Queue section only ever shows not-yet-played tracks. |
||||
- **Resume point:** when `manualQueue` empties, `next()` advances the context from |
||||
the preserved `currentIndex`. |
||||
- Triggered identically by user-pressed Next and by auto-advance (`trackDidFinish`). |
||||
|
||||
### Previous |
||||
|
||||
Unchanged: steps back through the **context** (`currentIndex − 1`, clamped at 0). |
||||
It never re-adds consumed queue items and does not consult `manualQueue` (accepted |
||||
v1 simplification). |
||||
|
||||
### Shuffle |
||||
|
||||
Unchanged: only the context (`queue`) is shuffled. `manualQueue` order is always |
||||
preserved. |
||||
|
||||
### Explicit play / new context |
||||
|
||||
`play(_:)` (double-click a row, space-bar, Home) sets a new **context** via |
||||
`setQueue(_:contextName:)` and plays from it (setting `currentIndex`). The manual |
||||
queue is **not** cleared — it survives the new context, matching the chosen Spotify |
||||
model. |
||||
|
||||
### Queue editing |
||||
|
||||
```swift |
||||
func moveInQueue(from: IndexSet, to: Int) // reorder manualQueue (panel drag) |
||||
func removeFromQueue(at: IndexSet) // remove from manualQueue (panel × / swipe) |
||||
``` |
||||
|
||||
## UI |
||||
|
||||
### `setQueue` signature change |
||||
|
||||
```swift |
||||
func setQueue(_ tracks: [Track], contextName: String? = nil) |
||||
``` |
||||
|
||||
Call sites in `ContentView` pass the context label: playlist/smart-playlist name, |
||||
`"Library"`, or `"Recently Added"` (Home). Default `nil` keeps existing tests and |
||||
any unlabeled callers compiling. |
||||
|
||||
### Up Next panel — new `QueueView` |
||||
|
||||
A SwiftUI `List`, docked on the right of the main content: |
||||
|
||||
- **Section "Queue"** — `player.manualQueue`. `.onMove` → `moveInQueue`; row × / |
||||
`.swipeActions` → `removeFromQueue`. |
||||
- **Section "Next from: \<contextName\>"** — `player.upcomingContext`, read-only. |
||||
Double-click a row to jump to it in the context (sets `currentIndex` and plays). |
||||
- **Empty state** — "Queue is empty. Right-click a track → Add to Queue." |
||||
|
||||
### Integration into `ContentView` |
||||
|
||||
- New `@State private var showQueue = false`. |
||||
- Wrap the main-content region (the `VStack` holding `HomeView`/`TrackTableView`) in |
||||
`HStack(spacing: 0) { mainContent; if showQueue { QueueView(player: player) } }`. |
||||
- `PlayerControlsView` gains a queue-toggle button (bottom-right of the transport |
||||
bar) bound to `showQueue`. The button is **hidden when `networkStatus` indicates |
||||
remote-drive mode**. |
||||
|
||||
### Context menu — `TrackContextMenuConfig` |
||||
|
||||
Add two optional closures: |
||||
|
||||
```swift |
||||
let onPlayNext: ((Track) -> Void)? |
||||
let onAddToQueue: ((Track) -> Void)? |
||||
``` |
||||
|
||||
Rendered in **both** menu builders, as a group above "Add to Playlist": |
||||
|
||||
- `TrackTableView.Coordinator.menuNeedsUpdate(_:)` (AppKit `NSMenuItem`s + actions). |
||||
- `TrackContextMenuModifier` (SwiftUI `Button`s). |
||||
|
||||
Each is shown only when its closure is non-nil. In `ContentView.trackContextMenuConfig` |
||||
they are wired to `player.playNext` / `player.addToQueue`, and passed as **`nil` |
||||
when driving a remote device** so the items are hidden. |
||||
|
||||
## Remote / Edge Handling (local-only v1) |
||||
|
||||
- The disable gate is specifically the **`RemotePlaybackProvider`** case (driving a |
||||
separate remote device, `networkStatus.mode == .remote`): `next()` delegates over |
||||
the wire (unchanged) and never consults `manualQueue`; queue menu items hidden (nil |
||||
closures); queue-toggle button hidden. No protocol changes. |
||||
- **Streaming client** mode plays locally through `StreamingPlaybackProvider`, so it |
||||
is **not** gated — the queue works there, as it does for local and streaming-host |
||||
playback. |
||||
- Empty manual queue + empty upcoming context: panel shows empty state; `next()` |
||||
at the end of the context stops, as today. |
||||
|
||||
## Testing (TDD) |
||||
|
||||
New `PlayerViewModelTests` cases (reusing the `AudioService()` / `FakeStreamingProvider` |
||||
pattern already in the file). Each test carries a step-by-step comment: |
||||
|
||||
1. `addToQueue` appends to `manualQueue`; `playNext` inserts at the front. |
||||
2. `next()` plays the front of `manualQueue` before advancing the context, and |
||||
`removeFirst` consumes it. |
||||
3. After `manualQueue` drains, `next()` resumes the context at `currentIndex + 1`. |
||||
4. Queue-while-idle (`currentTrack == nil`) starts playback immediately. |
||||
5. `toggleShuffle()` leaves `manualQueue` order unchanged. |
||||
6. `removeFromQueue` / `moveInQueue` mutate `manualQueue` correctly. |
||||
7. `upcomingContext` returns the correct slice of the context. |
||||
8. Existing PlayerViewModel tests remain green (backward-compatible model). |
||||
|
||||
New `TrackContextMenuConfigTests` case: the `onPlayNext` / `onAddToQueue` closures |
||||
fire with the expected track. |
||||
|
||||
## Files Touched |
||||
|
||||
- `Music/ViewModels/PlayerViewModel.swift` — state, `playNext`/`addToQueue`/ |
||||
`moveInQueue`/`removeFromQueue`, `next()` priority logic, `upcomingContext`, |
||||
`setQueue(contextName:)`. |
||||
- `Music/Models/QueueEntry.swift` — **new** identity wrapper for queued tracks. |
||||
- `Music/Models/TrackContextMenuConfig.swift` — two new closures. |
||||
- `Music/Views/TrackTableView.swift` — AppKit menu items + actions. |
||||
- `Music/Views/TrackContextMenuModifier.swift` — SwiftUI menu buttons. |
||||
- `Music/Views/PlayerControlsView.swift` — queue-toggle button. |
||||
- `Music/Views/QueueView.swift` — **new** Up Next panel. |
||||
- `Music/ContentView.swift` — `showQueue` state, panel layout, wiring, `contextName` |
||||
at `setQueue` call sites. |
||||
- `MusicTests/PlayerViewModelTests.swift`, `MusicTests/TrackContextMenuConfigTests.swift` |
||||
— new tests. |
||||
|
||||
No `project.pbxproj` edit is needed: the project uses Xcode 16 file-system |
||||
synchronized groups, so new files under `Music/` are picked up automatically. |
||||
@ -0,0 +1,166 @@ |
||||
# Smart Playlist Conditions — Design Spec |
||||
|
||||
**Date:** 2026-05-30 |
||||
**Status:** Approved |
||||
|
||||
## Overview |
||||
|
||||
Extend smart playlists to support structured metadata-based conditions (e.g. artist = "Alicia Keys", year > 2015). Multiple conditions combine with AND. The existing FTS free-text smart playlist flow is preserved unchanged. |
||||
|
||||
## Data Model |
||||
|
||||
### SmartPlaylist (extended) |
||||
|
||||
Add one new optional field to the existing `SmartPlaylist` struct: |
||||
|
||||
```swift |
||||
var conditions: [SmartPlaylistCondition]? |
||||
``` |
||||
|
||||
- `nil` → FTS mode (existing behavior, no change) |
||||
- non-nil → structured SQL WHERE mode (new behavior) |
||||
|
||||
### SmartPlaylistCondition |
||||
|
||||
```swift |
||||
struct SmartPlaylistCondition: Codable, Equatable, Sendable { |
||||
var field: TrackField |
||||
var op: ConditionOperator |
||||
var value: ConditionValue |
||||
} |
||||
``` |
||||
|
||||
### TrackField |
||||
|
||||
`String` raw-value enum. Raw value matches the SQLite column name exactly (used directly in query building): |
||||
|
||||
``` |
||||
title, artist, albumArtist, album, genre, composer, |
||||
year, bpm, rating, playCount, trackNumber, discNumber, |
||||
duration, bitrate, sampleRate, fileSize, |
||||
dateAdded, dateModified, lastPlayedAt, fileFormat |
||||
``` |
||||
|
||||
Each field has an associated `FieldType`: `.string`, `.int`, `.double`, `.date`. |
||||
|
||||
### ConditionOperator |
||||
|
||||
```swift |
||||
enum ConditionOperator: String, Codable { |
||||
case equals |
||||
case startsWith // strings only |
||||
case greaterThan // numbers and dates only |
||||
case lessThan // numbers and dates only |
||||
} |
||||
``` |
||||
|
||||
Valid operators per field type: |
||||
- String: `equals`, `startsWith` |
||||
- Number (Int, Double): `equals`, `greaterThan`, `lessThan` |
||||
- Date: `equals`, `greaterThan`, `lessThan` |
||||
|
||||
### ConditionValue |
||||
|
||||
Tagged Codable union: |
||||
|
||||
```swift |
||||
enum ConditionValue: Codable, Equatable, Sendable { |
||||
case string(String) |
||||
case int(Int) |
||||
case double(Double) |
||||
case date(Date) |
||||
} |
||||
``` |
||||
|
||||
## Persistence |
||||
|
||||
### DB Migration (v5) |
||||
|
||||
```sql |
||||
ALTER TABLE smart_playlists ADD COLUMN conditions TEXT; |
||||
``` |
||||
|
||||
Nullable, no default. Existing rows stay `NULL` (FTS mode). |
||||
|
||||
### Encoding |
||||
|
||||
`SmartPlaylist.conditions` is encoded as a JSON string when writing to the `conditions` column and decoded on read. GRDB's `Codable` conformance handles this automatically via a custom `columnEncodingStrategy` or manual encode/decode. |
||||
|
||||
## Query Evaluation |
||||
|
||||
### SQL Generation |
||||
|
||||
New private method in `DatabaseService`: |
||||
|
||||
```swift |
||||
private func buildWhereClause(_ conditions: [SmartPlaylistCondition]) -> (sql: String, args: StatementArguments) |
||||
``` |
||||
|
||||
Mapping: |
||||
|
||||
| Field type | Operator | SQL fragment | |
||||
|------------|--------------|-------------------------------------------| |
||||
| String | equals | `LOWER({col}) = LOWER(?)` | |
||||
| String | startsWith | `LOWER({col}) LIKE LOWER(?) || '%'` | |
||||
| Number | equals | `{col} = ?` | |
||||
| Number | greaterThan | `{col} > ?` | |
||||
| Number | lessThan | `{col} < ?` | |
||||
| Date | equals | `{col} = ?` | |
||||
| Date | greaterThan | `{col} > ?` | |
||||
| Date | lessThan | `{col} < ?` | |
||||
|
||||
Fragments joined with ` AND `. Final query: |
||||
|
||||
```sql |
||||
SELECT * FROM tracks WHERE <clause> ORDER BY <sortCol> COLLATE NOCASE <asc|desc> |
||||
``` |
||||
|
||||
### fetchTracks branch |
||||
|
||||
`PlaylistViewModel.observeSmartPlaylistTracks` branches on `smartPlaylist.conditions`: |
||||
- `nil` → existing FTS `ValueObservation` (unchanged) |
||||
- non-nil → new SQL WHERE `ValueObservation` using `buildWhereClause` |
||||
|
||||
## UI |
||||
|
||||
### Entry Point |
||||
|
||||
New "New Smart Playlist…" item in the app menu (alongside existing "New Playlist…"). Triggers a sheet (not an alert) since the form has multiple fields. |
||||
|
||||
### Condition Builder Sheet |
||||
|
||||
``` |
||||
┌─────────────────────────────────────────┐ |
||||
│ New Smart Playlist │ |
||||
│ │ |
||||
│ NAME │ |
||||
│ [________________________] │ |
||||
│ │ |
||||
│ CONDITIONS (all must match) │ |
||||
│ [Field ▾] [Operator ▾] [Value ] [−] │ |
||||
│ [Field ▾] [Operator ▾] [Value ] [−] │ |
||||
│ │ |
||||
│ [+ Add Condition ] │ |
||||
│ │ |
||||
│ [Cancel] [Save] │ |
||||
└─────────────────────────────────────────┘ |
||||
``` |
||||
|
||||
- One condition row shown by default |
||||
- Operator picker options update when field changes (string → is/starts with; number/date → is/greater than/less than) |
||||
- Value input adapts to field type: `TextField` for strings and numbers, `DatePicker` for date fields |
||||
- Remove (−) button disabled when only one condition remains |
||||
- Save disabled until name is non-empty and all condition values are non-empty |
||||
|
||||
### Edit Flow |
||||
|
||||
- **Structured smart playlists:** context menu shows "Edit…" → reopens the builder sheet pre-populated |
||||
- **FTS smart playlists:** context menu keeps existing "Edit Search Query…" → existing text alert (no change) |
||||
|
||||
## Out of Scope |
||||
|
||||
- OR logic between conditions |
||||
- Nested condition groups |
||||
- Track limit per playlist ("limit to N songs") |
||||
- Migrating existing FTS playlists to structured format |
||||
- Live-updating toggle (not needed — `ValueObservation` already handles this automatically) |
||||
@ -0,0 +1,86 @@ |
||||
--- |
||||
title: Track Context Menu on Bottom Controls |
||||
date: 2026-05-30 |
||||
status: approved |
||||
--- |
||||
|
||||
## Goal |
||||
|
||||
Right-clicking the now-playing area (bottom-left of the window) shows the same context menu as right-clicking a track in the track table: add to last playlist, add to any playlist via submenu, and remove from the current playlist when one is open. |
||||
|
||||
## Shared Config Struct |
||||
|
||||
A new `TrackContextMenuConfig` struct captures everything the menu needs: |
||||
|
||||
```swift |
||||
struct TrackContextMenuConfig { |
||||
let playlists: [Playlist] |
||||
let lastUsedPlaylistName: String? |
||||
let selectedPlaylist: Playlist? |
||||
let onAddToPlaylist: (Track, Playlist) -> Void |
||||
let onAddToLastPlaylist: ((Track) -> Void)? |
||||
let onRemoveFromPlaylist: ((Track) -> Void)? |
||||
} |
||||
``` |
||||
|
||||
This is the single source of truth for menu data. Both `TrackTableView` and `PlayerControlsView` receive one instance, constructed by `ContentView`. |
||||
|
||||
## Shared ViewModifier |
||||
|
||||
`TrackContextMenuModifier` is a SwiftUI `ViewModifier` that takes a `Track?` and `TrackContextMenuConfig?` and applies `.contextMenu` when both are non-nil: |
||||
|
||||
- **"Add to [last]"** button — shown only when `lastUsedPlaylistName` is set and `onAddToLastPlaylist` is non-nil. |
||||
- **"Add to Playlist"** submenu — one `Button` per playlist in `playlists`. Calls `onAddToPlaylist(track, playlist)`. |
||||
- **Divider + "Remove from Playlist"** — shown only when `selectedPlaylist != nil` and `onRemoveFromPlaylist` is non-nil. |
||||
|
||||
Menu is omitted entirely (no empty menu flicker) when `track` or `config` is nil. |
||||
|
||||
## PlayerControlsView Changes |
||||
|
||||
`PlayerControlsView` gains one new optional parameter: |
||||
|
||||
```swift |
||||
let contextMenuConfig: TrackContextMenuConfig? |
||||
``` |
||||
|
||||
The `nowPlayingSection` view applies `.trackContextMenu(track: currentTrack, config: contextMenuConfig)` (a convenience extension wrapping `TrackContextMenuModifier`). |
||||
|
||||
## TrackTableView Refactor |
||||
|
||||
`TrackTableView`'s existing four playlist-related parameters (`playlists`, `lastUsedPlaylistName`, `selectedPlaylist`, `onAddToLastPlaylist`, `onRemoveFromPlaylist`) are replaced by a single `contextMenuConfig: TrackContextMenuConfig?`. The `Coordinator.menuNeedsUpdate` builds its `NSMenu` from this config. This makes both call sites symmetric. |
||||
|
||||
> The AppKit `NSMenu` path in `TrackTableView` is kept — SwiftUI `.contextMenu` does not attach per-row in an `NSTableView`, so the table continues using `menuNeedsUpdate`. |
||||
|
||||
## ContentView Changes |
||||
|
||||
`ContentView` constructs one `TrackContextMenuConfig` and passes it to both views: |
||||
|
||||
```swift |
||||
let menuConfig = TrackContextMenuConfig( |
||||
playlists: playlist.allPlaylists, |
||||
lastUsedPlaylistName: playlist.lastUsedPlaylistName, |
||||
selectedPlaylist: playlist.selectedPlaylist, |
||||
onAddToPlaylist: { track, pl in try? playlist.addTrack(track, to: pl) }, |
||||
onAddToLastPlaylist: { track in try? playlist.addTrackToLastUsedPlaylist(track) }, |
||||
onRemoveFromPlaylist: playlist.selectedPlaylist != nil ? { track in |
||||
if let selected = playlist.selectedPlaylist { |
||||
try? playlist.removeTrack(track, from: selected) |
||||
} |
||||
} : nil |
||||
) |
||||
``` |
||||
|
||||
## Files Affected |
||||
|
||||
| File | Change | |
||||
|------|--------| |
||||
| `Music/Models/TrackContextMenuConfig.swift` | New file — struct definition | |
||||
| `Music/Views/TrackContextMenuModifier.swift` | New file — SwiftUI ViewModifier | |
||||
| `Music/Views/PlayerControlsView.swift` | Add `contextMenuConfig` param, apply modifier to `nowPlayingSection` | |
||||
| `Music/Views/TrackTableView.swift` | Replace individual playlist params with `contextMenuConfig`, adapt `menuNeedsUpdate` | |
||||
| `Music/ContentView.swift` | Construct and pass `TrackContextMenuConfig` to both views | |
||||
|
||||
## Out of Scope |
||||
|
||||
- Keyboard shortcut for the menu |
||||
- Any new menu items not already in the track table menu |
||||
@ -0,0 +1,206 @@ |
||||
# Track "Get Info" — Design Spec |
||||
|
||||
**Date:** 2026-05-30 |
||||
**Status:** Approved (design); pending implementation plan |
||||
**Branch:** feat/music-streaming |
||||
|
||||
## Goal |
||||
|
||||
Replicate the macOS Music app's right-click **Get Info** experience: a dialog that |
||||
shows all of a track's metadata and lets the user edit it. Edits persist to the |
||||
app's library (SQLite DB) **and** are written back into the audio file's embedded |
||||
tags, mirroring how the real Music app mutates files. |
||||
|
||||
## Decisions (locked) |
||||
|
||||
| Question | Decision | |
||||
|---|---| |
||||
| Where edits are saved | **DB + write file tags** (best-effort file writeback) | |
||||
| Single vs multi-track | **Both** — single track shows all fields; multiple selected tracks use mixed-value handling | |
||||
| Field scope | **Existing model fields only** — no schema migration; read-only File info section | |
||||
| Writeback formats (v1) | **mp3** (ID3TagEditor) + **m4a / alac / aac** (AVFoundation). flac/wav/aiff → DB only with a UI note | |
||||
| Tag library strategy | **Lighter path, TagLib-ready** — abstract behind a `TagWriter` protocol so a TagLib writer can be added later for flac/wav/aiff with no rework | |
||||
| Layout | **Tabbed** (Details / File), like macOS Music | |
||||
| Failure model | **DB is always saved; file writeback is best-effort** with a non-blocking warning on failure | |
||||
|
||||
## Non-goals (v1) |
||||
|
||||
- No new metadata fields (no comments, lyrics, sorting, artwork). Editing is limited to |
||||
fields already present on the `Track` model. |
||||
- No album-artwork display or editing. |
||||
- No flac/wav/aiff **file-tag** writeback (those edits save to the DB only for now). |
||||
The architecture leaves a clean seam to add TagLib later. |
||||
|
||||
## Data model context |
||||
|
||||
`Track` (Music/Models/Track.swift) already holds every field we need. No migration. |
||||
|
||||
**Editable fields** (surfaced on the Details tab): |
||||
`title`, `artist`, `albumArtist`, `album`, `genre`, `composer` (String); |
||||
`year`, `trackNumber`, `discNumber`, `bpm` (Int?); `rating` (Int 0–5). |
||||
|
||||
> **rating is DB-only in v1.** It's an app/iTunes concept with format-specific |
||||
> scales (ID3 POPM 0–255, iTunes atom 0–100), so writing it to file tags is |
||||
> deferred. `EditableTrackFields` includes `rating`, but the `TagWriter`s ignore it |
||||
> — rating persists only via `updateTrack`. All other editable fields are written to |
||||
> both file and DB (where a writer exists). |
||||
|
||||
**Read-only fields** (File tab): `fileURL` (path), `fileFormat`, `bitrate`, |
||||
`sampleRate`, `fileSize`, `duration`, `playCount`, `lastPlayedAt`, `dateAdded`, |
||||
`dateModified`. (`playCount`/`lastPlayedAt`/`rating` are app-managed, not file tags.) |
||||
|
||||
> Metadata source today: tracks are read from file tags via AVFoundation **once at |
||||
> import** (ScannerService), then the DB is the source of truth. `insertBatch` uses |
||||
> `.ignore` on conflict, so re-scans do not clobber DB edits. |
||||
|
||||
## Architecture & components |
||||
|
||||
Each unit has one responsibility and a well-defined interface. |
||||
|
||||
### `EditableTrackFields` (new value type) |
||||
A plain struct of the ~11 editable fields. The unit of "what the user can change" |
||||
and "what changed." Decouples the sheet and the writer from the full `Track`. |
||||
|
||||
### `TagWriter` protocol + factory (new) |
||||
``` |
||||
protocol TagWriter { |
||||
func write(_ fields: EditableTrackFields, to fileURL: URL) throws |
||||
} |
||||
``` |
||||
- `TagWriterFactory.writer(for: URL) -> TagWriter?` selects by file extension; |
||||
returns `nil` for unsupported formats (flac/wav/aiff in v1). |
||||
- Implementations: |
||||
- `ID3TagWriter` — mp3, via the **ID3TagEditor** SPM package. |
||||
- `MP4TagWriter` — m4a / alac / aac, via **AVFoundation** `AVAssetExportSession` |
||||
(`AVAssetExportPresetPassthrough`) writing iTunes/common metadata items. |
||||
- **No code outside the writers knows how tags are encoded per format.** |
||||
|
||||
### `TrackEditService` (new) |
||||
Orchestrates one save. For each target track: |
||||
1. Diff `EditableTrackFields` against the original → set of changed fields. |
||||
2. If a `TagWriter` exists for the format: write tags to a **temp copy**, then |
||||
`FileManager.replaceItemAt` to atomically swap the original (never leaves a |
||||
half-written file). |
||||
3. Recompute `fileSize`, `dateModified`, `fileHash` from the new file. |
||||
4. Build the updated `Track` (changed fields + refreshed stats) and persist via |
||||
`DatabaseService.updateTrack`. |
||||
5. On file-write failure (or unsupported format): still persist DB changes; collect |
||||
a warning for that track. |
||||
|
||||
Owns the **single- and multi-track diff logic** as pure, testable functions |
||||
(no UI, no I/O in the diff step). |
||||
|
||||
### `TrackInfoSheet` (new SwiftUI view) |
||||
The Get Info dialog (see Layout). Holds local `@State` for the edited fields, |
||||
prefilled from the target(s). On Save, hands `EditableTrackFields` + the target |
||||
track set to `TrackEditService`. Models the `.sheet` pattern already used by |
||||
`SmartPlaylistBuilderSheet`. |
||||
|
||||
### `DatabaseService.updateTrack(_ track: Track) throws` (new method) |
||||
GRDB `track.update(db)` inside `dbPool.write`. **Implementation plan must verify |
||||
whether the `tracks_ft` FTS5 table is kept in sync automatically (triggers / |
||||
external-content) and, if not, update it here.** |
||||
|
||||
### Context-menu integration (edits) |
||||
- Add **Get Info** (⌘I) to `TrackContextMenuConfig`, `TrackTableView`'s `NSMenu` |
||||
(`menuNeedsUpdate`), and `TrackContextMenuModifier`. |
||||
- Target resolution: the menu operates on the **current selection if the |
||||
right-clicked row is part of it**, otherwise just the clicked row (matches macOS |
||||
Music). This requires the config to expose the current multi-selection (or a |
||||
callback that returns the target set), not just a single `Track`. |
||||
- `ContentView` holds the presented-target state and shows the `.sheet`. |
||||
|
||||
## Save sequence (per track) |
||||
|
||||
``` |
||||
edit fields ─▶ diff vs original ─▶ writer for format? |
||||
│ yes │ no / unsupported |
||||
▼ ▼ |
||||
write tags to temp copy (skip file write, |
||||
─▶ atomic replace original collect "DB-only" note) |
||||
│ |
||||
success? │ fail ─────────┐ |
||||
▼ ▼ |
||||
recompute size/mod/hash keep old stats + warning |
||||
│ │ |
||||
└──────┬────────┘ |
||||
▼ |
||||
updateTrack(...) in DB |
||||
``` |
||||
|
||||
Result: the **DB edit always lands**; file writeback is best-effort. Failures |
||||
surface as a single non-blocking summary alert ("Saved to library. Couldn't write |
||||
tags to N file(s): <reason>"). |
||||
|
||||
## Multi-track behavior |
||||
|
||||
- Prefill: fields with one shared value across all targets prefill normally; |
||||
fields that differ show a **"Mixed"** placeholder and start empty. |
||||
- Apply: **only fields the user actually edits** are applied — to all targets. |
||||
Untouched "Mixed" fields are left per-track unchanged. |
||||
- Saving N tracks runs in a background `Task`, sequentially, with a small progress |
||||
indicator when N is large. Per-track failures aggregate into one summary. |
||||
|
||||
## UI layout (tabbed) |
||||
|
||||
``` |
||||
┌─ Get Info ──────────────────────────────┐ |
||||
│ [ Details ] [ File ] │ |
||||
├──────────────────────────────────────────┤ |
||||
│ Title [_______________________] │ |
||||
│ Artist [_______________________] │ |
||||
│ Album Artist[_______________________] │ |
||||
│ Album [_______________________] │ |
||||
│ Genre [_______________________] │ |
||||
│ Composer [_______________________] │ |
||||
│ Year [____] Track [__]/[__] Disc [__]/[__] |
||||
│ BPM [____] Rating ★★★☆☆ │ |
||||
├──────────────────────────────────────────┤ |
||||
│ File tab: format, bitrate, sample rate, │ |
||||
│ size, duration, path, date added, │ |
||||
│ play count, last played — read-only │ |
||||
├──────────────────────────────────────────┤ |
||||
│ [ Cancel ] [ Save ] │ |
||||
└──────────────────────────────────────────┘ |
||||
``` |
||||
|
||||
- Numeric fields (year / track / disc / bpm) validate on input. |
||||
- flac/wav/aiff targets show a subtle note under the tabs: *"Edits save to your |
||||
library only — tag writing isn't supported for .flac yet."* |
||||
- Cancel = `.cancelAction`, Save = `.defaultAction`. |
||||
|
||||
## Error handling & risks |
||||
|
||||
- **Atomic replace** (temp file + `replaceItemAt`) prevents audio-file corruption on |
||||
a failed/interrupted write. |
||||
- **File-write failure** → DB still saved + non-blocking warning with the reason. |
||||
- **Sandbox / permissions (must verify early):** if the app is sandboxed, writing to |
||||
user audio files requires the appropriate entitlement and/or a security-scoped |
||||
bookmark for the music folder. If writing is blocked, file writeback cannot work |
||||
regardless of library — DB edits still work. Verify before building the writers. |
||||
- **New dependency:** ID3TagEditor added via SPM to the Xcode project. |
||||
- **Hash drift:** writing tags changes file size/mod-date → `fileHash`. Step 3 |
||||
refreshes these so the next scan doesn't treat the file as changed. |
||||
|
||||
## Testing (TDD) |
||||
|
||||
- **TagWriter round-trip:** write `EditableTrackFields` to bundled mp3 and m4a |
||||
fixtures, re-read via AVFoundation, assert each field; assert the file remains a |
||||
valid, playable asset after atomic replace. |
||||
- **Diff / multi-track logic:** pure-function, table-driven tests for "which fields |
||||
changed," "shared vs Mixed across N tracks," and "apply only edited fields to all." |
||||
- **Stat refresh:** `fileSize` / `dateModified` / `fileHash` recomputed after |
||||
writeback. |
||||
- **Factory:** correct writer per extension; `nil` for flac/wav/aiff. |
||||
- **`updateTrack`:** persists edited fields and keeps `tracks_ft` in sync. |
||||
|
||||
## Implementation outline |
||||
|
||||
1. Add ID3TagEditor SPM dependency; confirm sandbox/file-write permissions. |
||||
2. `EditableTrackFields` + diff/multi-track pure logic (+ tests). |
||||
3. `TagWriter` protocol, factory, `ID3TagWriter`, `MP4TagWriter` (+ round-trip tests). |
||||
4. `DatabaseService.updateTrack` (+ FTS sync) (+ test). |
||||
5. `TrackEditService` wiring the save sequence (+ tests). |
||||
6. `TrackInfoSheet` UI (tabs, validation, Mixed handling). |
||||
7. Context-menu "Get Info" (⌘I) + target resolution + `ContentView` sheet presentation. |
||||
8. Manual verification against real mp3/m4a/flac files. |
||||
Binary file not shown.
Binary file not shown.
@ -0,0 +1,272 @@ |
||||
#!/usr/bin/env python3 |
||||
"""One-time backfill of real bitrate onto tracks stored with bitrate 0 or NULL. |
||||
|
||||
ScannerService writes `bitrate = Int(estimatedDataRate / 1000)` at scan time. |
||||
AVFoundation's estimatedDataRate returns 0 for some files (long/VBR MP3s), so a |
||||
literal 0 gets stored; other tracks were imported before bitrate existed and are |
||||
NULL. This script recomputes bitrate for those rows using ffprobe, falling back |
||||
to fileSize*8/duration (the same average the app's importer now uses) when |
||||
ffprobe is unavailable or can't determine a value. |
||||
|
||||
Dry-run by default. Pass --apply to write (a timestamped backup is made first). |
||||
|
||||
Usage: |
||||
python3 backfill_bitrate.py [--db <path>] [--apply] |
||||
python3 backfill_bitrate.py --self-test |
||||
|
||||
Stdlib only; uses ffprobe if present on PATH (optional). |
||||
""" |
||||
|
||||
import argparse |
||||
import os |
||||
import shutil |
||||
import sqlite3 |
||||
import subprocess |
||||
import sys |
||||
import unicodedata |
||||
from datetime import datetime |
||||
from urllib.parse import unquote |
||||
|
||||
# Default DB path for the sandboxed app (bundle id com.staxriver.mu). Computed from |
||||
# $HOME so it resolves to the right user on whichever Mac the script runs on. |
||||
DEFAULT_DB = os.path.expanduser( |
||||
"~/Library/Containers/com.staxriver.mu/Data/Library/" |
||||
"Application Support/Music/db.sqlite" |
||||
) |
||||
|
||||
|
||||
def norm_path(u): |
||||
"""Reduce a file:// URL (or bare path) to a comparable, on-disk POSIX path. |
||||
|
||||
The app stores `fileURL` as Foundation's url.absoluteString (a percent-encoded |
||||
file URL). Decode it, drop the file:// (or file://localhost) prefix, NFC- |
||||
normalize, and strip a trailing slash so it can be stat'd on APFS. |
||||
""" |
||||
s = u |
||||
if s.startswith("file://"): |
||||
s = s[len("file://"):] |
||||
if s.startswith("localhost/"): |
||||
s = s[len("localhost"):] # leaves the leading "/" |
||||
s = unquote(s) |
||||
s = unicodedata.normalize("NFC", s) |
||||
if len(s) > 1 and s.endswith("/"): |
||||
s = s[:-1] |
||||
return s |
||||
|
||||
|
||||
def parse_ffprobe_bitrate(stdout): |
||||
"""Parse ffprobe's bit_rate stdout (bits/sec) into integer kbps, or None. |
||||
|
||||
Returns None for empty output, 'N/A', or any non-integer text so the caller |
||||
falls back to the formula. |
||||
""" |
||||
s = stdout.strip() |
||||
if not s or s == "N/A": |
||||
return None |
||||
try: |
||||
return round(int(s) / 1000) |
||||
except ValueError: |
||||
return None |
||||
|
||||
|
||||
def kbps_from_ffprobe(path): |
||||
"""Return integer kbps from ffprobe's format bit_rate, or None if unavailable. |
||||
|
||||
None on: ffprobe not installed, ffprobe error, or N/A/empty/non-integer output. |
||||
""" |
||||
try: |
||||
out = subprocess.run( |
||||
["ffprobe", "-v", "error", "-show_entries", "format=bit_rate", |
||||
"-of", "default=nw=1:nk=1", path], |
||||
capture_output=True, text=True, timeout=30, |
||||
) |
||||
except (FileNotFoundError, subprocess.SubprocessError): |
||||
return None |
||||
return parse_ffprobe_bitrate(out.stdout) |
||||
|
||||
|
||||
def kbps_from_formula(file_size, duration): |
||||
"""Average kbps from size (bytes) and duration (seconds): size*8/duration/1000. |
||||
|
||||
Returns None when inputs can't yield a meaningful value (missing size, or |
||||
non-positive/missing duration). |
||||
""" |
||||
if file_size is None or file_size <= 0 or duration is None or duration <= 0: |
||||
return None |
||||
return round(file_size * 8 / duration / 1000) |
||||
|
||||
|
||||
def resolve_bitrate(path, duration): |
||||
"""Best available kbps for an on-disk file: ffprobe first, formula fallback. |
||||
|
||||
`duration` is the DB's stored seconds; file size is read from disk. Returns |
||||
None if neither method can produce a positive value. |
||||
""" |
||||
kbps = kbps_from_ffprobe(path) |
||||
if kbps is not None and kbps > 0: |
||||
return kbps |
||||
try: |
||||
size = os.path.getsize(path) |
||||
except OSError: |
||||
size = None |
||||
return kbps_from_formula(size, duration) |
||||
|
||||
|
||||
def ffprobe_available(): |
||||
"""Return True if ffprobe is on PATH.""" |
||||
return shutil.which("ffprobe") is not None |
||||
|
||||
|
||||
def self_test(): |
||||
"""Fast smoke check of the pure helpers (no DB, no ffprobe needed).""" |
||||
# ffprobe stdout parsing |
||||
assert parse_ffprobe_bitrate("256005\n") == 256 |
||||
assert parse_ffprobe_bitrate("N/A") is None |
||||
assert parse_ffprobe_bitrate("") is None |
||||
assert parse_ffprobe_bitrate("garbage") is None |
||||
|
||||
# formula: 230_358_479 bytes over 7198.54 s -> 256 kbps (matches ffprobe sample) |
||||
assert kbps_from_formula(230_358_479, 7198.5371428571425) == 256 |
||||
assert kbps_from_formula(None, 100) is None |
||||
assert kbps_from_formula(1000, 0) is None |
||||
assert kbps_from_formula(1000, None) is None |
||||
|
||||
# path normalization (NFD vs NFC accents, percent-encoding, localhost host) |
||||
nfc = norm_path("file:///Users/x/Mu%CC%81sica/Cafe%CC%81.mp3") |
||||
nfd = norm_path("file://localhost/Users/x/M%C3%BAsica/Caf%C3%A9.mp3") |
||||
assert nfc == nfd == "/Users/x/Música/Café.mp3", (nfc, nfd) |
||||
assert norm_path("file:///a/b%20c%23d.mp3") == "/a/b c#d.mp3" |
||||
|
||||
# resolve_bitrate composition: a missing file yields None regardless of whether |
||||
# ffprobe is installed (ffprobe errors on the path -> None; getsize raises |
||||
# OSError -> formula gets size=None -> None). |
||||
assert resolve_bitrate("/nonexistent/file.mp3", 100) is None |
||||
|
||||
print("self-test OK") |
||||
|
||||
|
||||
def fetch_rows(db_path): |
||||
"""Return candidate rows: (id, fileURL, duration, bitrate) where bitrate is 0/NULL.""" |
||||
con = sqlite3.connect(db_path) |
||||
try: |
||||
return con.execute( |
||||
"SELECT id, fileURL, duration, bitrate FROM tracks " |
||||
"WHERE bitrate = 0 OR bitrate IS NULL" |
||||
).fetchall() |
||||
finally: |
||||
con.close() |
||||
|
||||
|
||||
def build_updates(rows): |
||||
"""Resolve a new bitrate for each candidate row. |
||||
|
||||
Returns (updates, missing, undeterminable): |
||||
- updates: list of {id, file_url, old, new} where new is a positive kbps |
||||
- missing: (id, path) for rows whose file is not on disk (left untouched) |
||||
- undeterminable: (id, path) for on-disk files whose bitrate couldn't be found |
||||
""" |
||||
updates, missing, undeterminable = [], [], [] |
||||
for row_id, file_url, duration, old in rows: |
||||
path = norm_path(file_url) |
||||
if not os.path.exists(path): |
||||
missing.append((row_id, path)) |
||||
continue |
||||
new = resolve_bitrate(path, duration) |
||||
if new is None or new <= 0: |
||||
undeterminable.append((row_id, path)) |
||||
continue |
||||
updates.append({"id": row_id, "file_url": file_url, "old": old, "new": new}) |
||||
return updates, missing, undeterminable |
||||
|
||||
|
||||
def backup_db(db_path): |
||||
"""Copy db.sqlite (+ -wal, -shm) under backups/<timestamp>/ next to the DB.""" |
||||
stamp = datetime.now().strftime("%Y%m%d-%H%M%S") |
||||
backup_dir = os.path.join(os.path.dirname(db_path), "backups", stamp) |
||||
os.makedirs(backup_dir, exist_ok=True) |
||||
for suffix in ("", "-wal", "-shm"): |
||||
src = db_path + suffix |
||||
if os.path.exists(src): |
||||
shutil.copy2(src, os.path.join(backup_dir, os.path.basename(src))) |
||||
return backup_dir |
||||
|
||||
|
||||
def apply_updates(db_path, updates): |
||||
"""Write bitrate updates in a single transaction, then checkpoint the WAL.""" |
||||
con = sqlite3.connect(db_path) |
||||
try: |
||||
con.execute("BEGIN") |
||||
con.executemany("UPDATE tracks SET bitrate=:new WHERE id=:id", updates) |
||||
con.commit() |
||||
con.execute("PRAGMA wal_checkpoint(TRUNCATE)") |
||||
finally: |
||||
con.close() |
||||
|
||||
|
||||
def run(db_path, apply): |
||||
rows = fetch_rows(db_path) |
||||
updates, missing, undeterminable = build_updates(rows) |
||||
|
||||
print(f"Candidate rows (bitrate 0 or NULL): {len(rows)}") |
||||
print(f"Resolvable (will set): {len(updates)}") |
||||
print(f"Skipped — file missing on disk: {len(missing)}") |
||||
print(f"Skipped — could not determine: {len(undeterminable)}") |
||||
if not ffprobe_available(): |
||||
print("NOTE: ffprobe not on PATH — used the filesize/duration formula for all rows.") |
||||
print() |
||||
|
||||
for u in updates[:15]: |
||||
name = os.path.basename(norm_path(u["file_url"])) |
||||
old = "NULL" if u["old"] is None else u["old"] |
||||
print(f" • {name}") |
||||
print(f" bitrate {old} -> {u['new']} kbps") |
||||
if len(updates) > 15: |
||||
print(f" ... and {len(updates) - 15} more") |
||||
print() |
||||
|
||||
if missing[:5]: |
||||
print("Sample of skipped (file missing on disk, left untouched):") |
||||
for row_id, path in missing[:5]: |
||||
print(f" - [{row_id}] {os.path.basename(path)}") |
||||
print() |
||||
|
||||
if undeterminable[:5]: |
||||
print("Sample of skipped (could not determine bitrate, left untouched):") |
||||
for row_id, path in undeterminable[:5]: |
||||
print(f" - [{row_id}] {os.path.basename(path)}") |
||||
print() |
||||
|
||||
if not apply: |
||||
print("DRY RUN — nothing written. Re-run with --apply to commit these changes.") |
||||
return |
||||
|
||||
if not updates: |
||||
print("Nothing to apply.") |
||||
return |
||||
|
||||
backup_dir = backup_db(db_path) |
||||
print(f"Backup written to: {backup_dir}") |
||||
apply_updates(db_path, updates) |
||||
print(f"Applied {len(updates)} bitrate updates to {db_path}") |
||||
|
||||
|
||||
def main(argv=None): |
||||
p = argparse.ArgumentParser(description=__doc__) |
||||
p.add_argument("--db", default=DEFAULT_DB, help=f"App DB path (default: {DEFAULT_DB})") |
||||
p.add_argument("--apply", action="store_true", help="Write changes (default: dry run).") |
||||
p.add_argument("--self-test", action="store_true", help="Run the built-in smoke test.") |
||||
args = p.parse_args(argv) |
||||
|
||||
if args.self_test: |
||||
self_test() |
||||
return 0 |
||||
|
||||
if not os.path.exists(args.db): |
||||
p.error(f"DB not found: {args.db}") |
||||
|
||||
run(args.db, args.apply) |
||||
return 0 |
||||
|
||||
|
||||
if __name__ == "__main__": |
||||
sys.exit(main()) |
||||
@ -0,0 +1,253 @@ |
||||
#!/usr/bin/env python3 |
||||
"""One-time backfill of Date Added / play stats from Apple Music into the app DB. |
||||
|
||||
ScannerService stamps `dateAdded = Date()` at scan time, so the app DB holds scan |
||||
dates rather than the real "date added" from Apple Music. This script reads the |
||||
ground truth from a Music.app library export (File > Library > Export Library...) |
||||
and overwrites dateAdded, playCount, rating and lastPlayedAt on tracks it can match |
||||
by file path. |
||||
|
||||
Dry-run by default. Pass --apply to write (a timestamped backup is made first). |
||||
|
||||
Usage: |
||||
python3 backfill_itunes_dates.py --xml <Library.xml> [--db <path>] [--apply] |
||||
python3 backfill_itunes_dates.py --self-test |
||||
|
||||
Stdlib only; needs python3 (ships with Xcode Command Line Tools). |
||||
""" |
||||
|
||||
import argparse |
||||
import os |
||||
import plistlib |
||||
import shutil |
||||
import sqlite3 |
||||
import sys |
||||
import unicodedata |
||||
from datetime import datetime |
||||
from urllib.parse import unquote |
||||
|
||||
# Default DB path for the sandboxed app (bundle id com.staxriver.mu). Computed from |
||||
# $HOME so it resolves to the right user on whichever Mac the script runs on. |
||||
DEFAULT_DB = os.path.expanduser( |
||||
"~/Library/Containers/com.staxriver.mu/Data/Library/" |
||||
"Application Support/Music/db.sqlite" |
||||
) |
||||
|
||||
|
||||
def norm_path(u): |
||||
"""Reduce a file:// URL (or bare path) to a comparable POSIX path. |
||||
|
||||
Both the app's stored `fileURL` (Foundation's url.absoluteString) and Music.app's |
||||
`Location` are percent-encoded file URLs, but they can differ in host form |
||||
(file:/// vs file://localhost/) and Unicode normalization (APFS keeps filenames |
||||
in one form, the URL encoders may emit another). Normalizing both to a decoded, |
||||
NFC, trailing-slash-free path makes accented filenames compare equal. |
||||
""" |
||||
s = u |
||||
if s.startswith("file://"): |
||||
s = s[len("file://"):] |
||||
if s.startswith("localhost/"): |
||||
s = s[len("localhost"):] # leaves the leading "/" |
||||
s = unquote(s) |
||||
s = unicodedata.normalize("NFC", s) |
||||
if len(s) > 1 and s.endswith("/"): |
||||
s = s[:-1] |
||||
return s |
||||
|
||||
|
||||
def fmt_dt(dt): |
||||
"""Format a datetime as GRDB's .datetime string (UTC), or None. |
||||
|
||||
plistlib parses <date> values into naive datetimes already expressed in UTC, |
||||
which is exactly what GRDB stores (e.g. '2026-05-24 06:46:01.713'). We emit |
||||
millisecond precision (.000) to match the column's existing shape; GRDB reads |
||||
both with and without millis, so this round-trips. |
||||
""" |
||||
if dt is None: |
||||
return None |
||||
return dt.strftime("%Y-%m-%d %H:%M:%S") + ".000" |
||||
|
||||
|
||||
def parse_library(xml_path): |
||||
"""Parse a Music.app library export into {norm_path: fields}. |
||||
|
||||
Only tracks with a Location (i.e. real local files) are included; Apple Music |
||||
streaming entries have no Location and are skipped. |
||||
""" |
||||
with open(xml_path, "rb") as fp: |
||||
plist = plistlib.load(fp) |
||||
|
||||
music = {} |
||||
for track in plist.get("Tracks", {}).values(): |
||||
location = track.get("Location") |
||||
if not location: |
||||
continue |
||||
music[norm_path(location)] = { |
||||
"date_added": track.get("Date Added"), |
||||
"play_count": track.get("Play Count"), |
||||
"rating": track.get("Rating"), |
||||
"play_date_utc": track.get("Play Date UTC"), |
||||
} |
||||
return music |
||||
|
||||
|
||||
def build_updates(db_rows, music_map): |
||||
"""Compute UPDATE tuples for matched tracks (blunt overwrite, Music is truth). |
||||
|
||||
db_rows: iterable of (id, fileURL, current_dateAdded). |
||||
Returns (updates, unmatched) where: |
||||
- updates is a list of dicts: id, file_url, old_date, dateAdded, playCount, |
||||
rating, lastPlayedAt |
||||
- unmatched is a list of (id, fileURL) present in the DB but not the export. |
||||
""" |
||||
updates = [] |
||||
unmatched = [] |
||||
for row_id, file_url, current_date in db_rows: |
||||
m = music_map.get(norm_path(file_url)) |
||||
if m is None: |
||||
unmatched.append((row_id, file_url)) |
||||
continue |
||||
# dateAdded is NOT NULL: if the export somehow lacks it, keep what's there. |
||||
new_date = fmt_dt(m["date_added"]) if m["date_added"] else current_date |
||||
rating = min(5, int(m["rating"] or 0) // 20) # Music 0-100 -> 0-5 stars |
||||
updates.append({ |
||||
"id": row_id, |
||||
"file_url": file_url, |
||||
"old_date": current_date, |
||||
"dateAdded": new_date, |
||||
"playCount": int(m["play_count"] or 0), |
||||
"rating": rating, |
||||
"lastPlayedAt": fmt_dt(m["play_date_utc"]), |
||||
}) |
||||
return updates, unmatched |
||||
|
||||
|
||||
def backup_db(db_path): |
||||
"""Copy db.sqlite (+ -wal, -shm) next to it under backups/<timestamp>/.""" |
||||
stamp = datetime.now().strftime("%Y%m%d-%H%M%S") |
||||
backup_dir = os.path.join(os.path.dirname(db_path), "backups", stamp) |
||||
os.makedirs(backup_dir, exist_ok=True) |
||||
for suffix in ("", "-wal", "-shm"): |
||||
src = db_path + suffix |
||||
if os.path.exists(src): |
||||
shutil.copy2(src, os.path.join(backup_dir, os.path.basename(src))) |
||||
return backup_dir |
||||
|
||||
|
||||
def apply_updates(db_path, updates): |
||||
"""Write updates in a single transaction, then checkpoint the WAL.""" |
||||
con = sqlite3.connect(db_path) |
||||
try: |
||||
con.execute("BEGIN") |
||||
con.executemany( |
||||
"UPDATE tracks SET dateAdded=:dateAdded, playCount=:playCount, " |
||||
"rating=:rating, lastPlayedAt=:lastPlayedAt WHERE id=:id", |
||||
updates, |
||||
) |
||||
con.commit() |
||||
con.execute("PRAGMA wal_checkpoint(TRUNCATE)") |
||||
finally: |
||||
con.close() |
||||
|
||||
|
||||
def fetch_db_rows(db_path): |
||||
con = sqlite3.connect(db_path) |
||||
try: |
||||
return con.execute("SELECT id, fileURL, dateAdded FROM tracks").fetchall() |
||||
finally: |
||||
con.close() |
||||
|
||||
|
||||
def run(xml_path, db_path, apply): |
||||
music_map = parse_library(xml_path) |
||||
db_rows = fetch_db_rows(db_path) |
||||
updates, unmatched = build_updates(db_rows, music_map) |
||||
|
||||
matched_paths = {norm_path(u["file_url"]) for u in updates} |
||||
unmatched_xml = [p for p in music_map if p not in matched_paths] |
||||
|
||||
print(f"DB tracks: {len(db_rows)}") |
||||
print(f"Local files in XML: {len(music_map)}") |
||||
print(f"Matched (will set): {len(updates)}") |
||||
print(f"In DB, not in XML: {len(unmatched)}") |
||||
print(f"In XML, not in DB: {len(unmatched_xml)}") |
||||
print() |
||||
|
||||
for u in updates[:10]: |
||||
name = os.path.basename(norm_path(u["file_url"])) |
||||
print(f" • {name}") |
||||
print(f" dateAdded {u['old_date']} -> {u['dateAdded']}") |
||||
print(f" playCount={u['playCount']} rating={u['rating']} " |
||||
f"lastPlayedAt={u['lastPlayedAt']}") |
||||
if len(updates) > 10: |
||||
print(f" ... and {len(updates) - 10} more") |
||||
print() |
||||
|
||||
if unmatched[:5]: |
||||
print("Sample of DB tracks with no XML match (left untouched):") |
||||
for row_id, file_url in unmatched[:5]: |
||||
print(f" - [{row_id}] {os.path.basename(norm_path(file_url))}") |
||||
print() |
||||
|
||||
if not apply: |
||||
print("DRY RUN — nothing written. Re-run with --apply to commit these changes.") |
||||
return |
||||
|
||||
if not updates: |
||||
print("Nothing to apply.") |
||||
return |
||||
|
||||
backup_dir = backup_db(db_path) |
||||
print(f"Backup written to: {backup_dir}") |
||||
apply_updates(db_path, updates) |
||||
print(f"Applied {len(updates)} updates to {db_path}") |
||||
|
||||
|
||||
def self_test(): |
||||
"""Fast smoke check of the matching + formatting core.""" |
||||
nfc = norm_path("file:///Users/x/Mu%CC%81sica/Cafe%CC%81.mp3") # NFD source |
||||
nfd = norm_path("file://localhost/Users/x/M%C3%BAsica/Caf%C3%A9.mp3") # NFC source |
||||
assert nfc == nfd == "/Users/x/Música/Café.mp3", (nfc, nfd) |
||||
assert norm_path("file:///a/b%20c%23d.mp3") == "/a/b c#d.mp3" |
||||
|
||||
music = {"/a/song.mp3": { |
||||
"date_added": datetime(2021, 3, 14, 9, 26, 53), |
||||
"play_count": 7, "rating": 80, |
||||
"play_date_utc": datetime(2024, 1, 2, 3, 4, 5), |
||||
}} |
||||
rows = [(1, "file:///a/song.mp3", "2026-05-24 06:46:01.713"), |
||||
(2, "file:///a/missing.mp3", "2026-05-24 06:46:01.999")] |
||||
updates, unmatched = build_updates(rows, music) |
||||
assert len(updates) == 1 and len(unmatched) == 1 |
||||
u = updates[0] |
||||
assert u["dateAdded"] == "2021-03-14 09:26:53.000", u["dateAdded"] |
||||
assert u["playCount"] == 7 and u["rating"] == 4 |
||||
assert u["lastPlayedAt"] == "2024-01-02 03:04:05.000" |
||||
print("self-test OK") |
||||
|
||||
|
||||
def main(argv=None): |
||||
p = argparse.ArgumentParser(description=__doc__) |
||||
p.add_argument("--xml", help="Path to Music.app Library export (XML plist).") |
||||
p.add_argument("--db", default=DEFAULT_DB, help=f"App DB path (default: {DEFAULT_DB})") |
||||
p.add_argument("--apply", action="store_true", help="Write changes (default: dry run).") |
||||
p.add_argument("--self-test", action="store_true", help="Run the built-in smoke test.") |
||||
args = p.parse_args(argv) |
||||
|
||||
if args.self_test: |
||||
self_test() |
||||
return 0 |
||||
|
||||
if not args.xml: |
||||
p.error("--xml is required (export it via Music.app: File > Library > Export Library...)") |
||||
if not os.path.exists(args.xml): |
||||
p.error(f"XML not found: {args.xml}") |
||||
if not os.path.exists(args.db): |
||||
p.error(f"DB not found: {args.db}") |
||||
|
||||
run(args.xml, args.db, args.apply) |
||||
return 0 |
||||
|
||||
|
||||
if __name__ == "__main__": |
||||
sys.exit(main()) |
||||
@ -0,0 +1,178 @@ |
||||
#!/usr/bin/env python3 |
||||
"""Tests for backfill_itunes_dates. Run: python3 -m unittest test_backfill_itunes_dates""" |
||||
|
||||
import io |
||||
import os |
||||
import plistlib |
||||
import sqlite3 |
||||
import sys |
||||
import tempfile |
||||
import unittest |
||||
from contextlib import redirect_stdout |
||||
from datetime import datetime |
||||
|
||||
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) |
||||
import backfill_itunes_dates as bf # noqa: E402 |
||||
|
||||
# The three real file paths from the dev DB. The Kevin track exercises spaces (%20), |
||||
# a literal apostrophe, and parentheses; the others share a directory. |
||||
KEVIN = ("file:///Users/laurentmorvillier/Music/Music/Media.localized/Music/" |
||||
"Kevin%20Saunderson%20as%20E-Dancer/GlobalDJMix.com/" |
||||
"Radio%201's%20Essential%20Mix%20(2025-02-08).mp3") |
||||
T1130 = ("file:///Users/laurentmorvillier/Music/Music/Media.localized/Music/" |
||||
"Unknown%20Artist/Unknown%20Album/1130.mp3") |
||||
T1486 = ("file:///Users/laurentmorvillier/Music/Music/Media.localized/Music/" |
||||
"Unknown%20Artist/Unknown%20Album/1486.mp3") |
||||
|
||||
|
||||
class NormPathTests(unittest.TestCase): |
||||
# Step: an NFD-encoded source and an NFC-encoded source for the same accented |
||||
# filename must normalize to the identical path, regardless of file:// host form. |
||||
def test_nfc_nfd_and_host_form_converge(self): |
||||
nfd = bf.norm_path("file:///Users/x/Mu%CC%81sica/Cafe%CC%81.mp3") |
||||
nfc = bf.norm_path("file://localhost/Users/x/M%C3%BAsica/Caf%C3%A9.mp3") |
||||
self.assertEqual(nfd, "/Users/x/Música/Café.mp3") |
||||
self.assertEqual(nfd, nfc) |
||||
|
||||
# Step: percent-encoded space and hash decode to literal characters. |
||||
def test_special_chars_decoded(self): |
||||
self.assertEqual(bf.norm_path("file:///a/b%20c%23d.mp3"), "/a/b c#d.mp3") |
||||
|
||||
# Step: a trailing slash is stripped so dir-ish strings compare cleanly. |
||||
def test_trailing_slash_stripped(self): |
||||
self.assertEqual(bf.norm_path("file:///a/b/"), "/a/b") |
||||
|
||||
# Step: the real Kevin path with apostrophe + parens round-trips to a clean path. |
||||
def test_real_kevin_path(self): |
||||
self.assertTrue( |
||||
bf.norm_path(KEVIN).endswith("/Radio 1's Essential Mix (2025-02-08).mp3")) |
||||
|
||||
|
||||
class BuildUpdatesTests(unittest.TestCase): |
||||
# Step: a matched track gets dateAdded reformatted to GRDB shape, rating mapped |
||||
# 0-100 -> 0-5, playCount copied, and lastPlayedAt formatted; an unmatched DB |
||||
# track is reported separately and produces no update. |
||||
def test_mapping_and_unmatched(self): |
||||
music = {bf.norm_path("file:///a/song.mp3"): { |
||||
"date_added": datetime(2021, 3, 14, 9, 26, 53), |
||||
"play_count": 7, "rating": 80, |
||||
"play_date_utc": datetime(2024, 1, 2, 3, 4, 5), |
||||
}} |
||||
rows = [(1, "file:///a/song.mp3", "2026-05-24 06:46:01.713"), |
||||
(2, "file:///a/missing.mp3", "2026-05-24 06:46:01.999")] |
||||
updates, unmatched = bf.build_updates(rows, music) |
||||
self.assertEqual([u["id"] for u in updates], [1]) |
||||
self.assertEqual([r[0] for r in unmatched], [2]) |
||||
u = updates[0] |
||||
self.assertEqual(u["dateAdded"], "2021-03-14 09:26:53.000") |
||||
self.assertEqual(u["playCount"], 7) |
||||
self.assertEqual(u["rating"], 4) |
||||
self.assertEqual(u["lastPlayedAt"], "2024-01-02 03:04:05.000") |
||||
|
||||
# Step: missing optional fields default safely — playCount 0, rating 0, |
||||
# lastPlayedAt None — while dateAdded still applies. |
||||
def test_absent_optionals(self): |
||||
music = {bf.norm_path("file:///a/x.mp3"): { |
||||
"date_added": datetime(2020, 1, 1, 0, 0, 0), |
||||
"play_count": None, "rating": None, "play_date_utc": None, |
||||
}} |
||||
updates, _ = bf.build_updates([(1, "file:///a/x.mp3", "old")], music) |
||||
u = updates[0] |
||||
self.assertEqual(u["playCount"], 0) |
||||
self.assertEqual(u["rating"], 0) |
||||
self.assertIsNone(u["lastPlayedAt"]) |
||||
|
||||
# Step: if the export lacks Date Added, keep the existing value (column is NOT NULL). |
||||
def test_missing_date_keeps_existing(self): |
||||
music = {bf.norm_path("file:///a/x.mp3"): { |
||||
"date_added": None, "play_count": 1, "rating": 0, "play_date_utc": None}} |
||||
updates, _ = bf.build_updates([(1, "file:///a/x.mp3", "2026-05-24 06:46:01.713")], music) |
||||
self.assertEqual(updates[0]["dateAdded"], "2026-05-24 06:46:01.713") |
||||
|
||||
|
||||
class IntegrationTest(unittest.TestCase): |
||||
def setUp(self): |
||||
self.tmp = tempfile.mkdtemp() |
||||
self.db = os.path.join(self.tmp, "db.sqlite") |
||||
self.xml = os.path.join(self.tmp, "Library.xml") |
||||
|
||||
# Step: build a temp DB mirroring the real tracks columns the script touches, |
||||
# seeded with the three real paths carrying placeholder scan dates. |
||||
con = sqlite3.connect(self.db) |
||||
con.execute( |
||||
"CREATE TABLE tracks (" |
||||
" id INTEGER PRIMARY KEY," |
||||
" fileURL TEXT NOT NULL," |
||||
" dateAdded TEXT NOT NULL," |
||||
" playCount INTEGER NOT NULL DEFAULT 0," |
||||
" rating INTEGER NOT NULL DEFAULT 0," |
||||
" lastPlayedAt TEXT)") |
||||
con.executemany( |
||||
"INSERT INTO tracks (id, fileURL, dateAdded, playCount, rating) VALUES (?,?,?,0,0)", |
||||
[(1, KEVIN, "2026-05-24 06:46:01.713"), |
||||
(2, T1130, "2026-05-24 06:46:01.715"), |
||||
(3, T1486, "2026-05-24 06:46:01.718")]) |
||||
con.commit() |
||||
con.close() |
||||
|
||||
# Step: write a synthetic library export. Kevin is matched via the localhost |
||||
# host form (proving normalization), 1130 matched with no optional stats, 1486 |
||||
# deliberately omitted (stays unmatched), plus a streaming entry with no |
||||
# Location (must be skipped) and an XML-only local file (unmatched-in-DB). |
||||
kevin_localhost = KEVIN.replace("file:///", "file://localhost/") |
||||
plist = {"Tracks": { |
||||
"10": {"Location": kevin_localhost, |
||||
"Date Added": datetime(2025, 2, 9, 12, 0, 0), |
||||
"Play Count": 5, "Rating": 100, |
||||
"Play Date UTC": datetime(2025, 3, 1, 8, 0, 0)}, |
||||
"11": {"Location": T1130, |
||||
"Date Added": datetime(2024, 11, 30, 10, 0, 0)}, |
||||
"12": {"Date Added": datetime(2023, 1, 1, 0, 0, 0)}, # streaming, no Location |
||||
"13": {"Location": "file:///Users/x/only-in-xml.mp3", |
||||
"Date Added": datetime(2022, 6, 6, 6, 6, 6)}, |
||||
}} |
||||
with open(self.xml, "wb") as fp: |
||||
plistlib.dump(plist, fp) |
||||
|
||||
def tearDown(self): |
||||
import shutil |
||||
shutil.rmtree(self.tmp, ignore_errors=True) |
||||
|
||||
# Step: a full --apply run rewrites the two matched rows with Music.app's values |
||||
# (Kevin: 5 plays / 5 stars / real dates; 1130: date only), leaves the unmatched |
||||
# 1486 row untouched, and ignores the streaming + xml-only entries. |
||||
def test_apply_end_to_end(self): |
||||
with redirect_stdout(io.StringIO()): |
||||
bf.run(self.xml, self.db, apply=True) |
||||
|
||||
con = sqlite3.connect(self.db) |
||||
rows = {r[0]: r for r in con.execute( |
||||
"SELECT id, dateAdded, playCount, rating, lastPlayedAt FROM tracks")} |
||||
con.close() |
||||
|
||||
self.assertEqual(rows[1], (1, "2025-02-09 12:00:00.000", 5, 5, "2025-03-01 08:00:00.000")) |
||||
self.assertEqual(rows[2], (2, "2024-11-30 10:00:00.000", 0, 0, None)) |
||||
self.assertEqual(rows[3], (3, "2026-05-24 06:46:01.718", 0, 0, None)) # untouched |
||||
|
||||
# Step: a backup directory containing the db copy is created on apply. |
||||
def test_apply_makes_backup(self): |
||||
with redirect_stdout(io.StringIO()): |
||||
bf.run(self.xml, self.db, apply=True) |
||||
backups_root = os.path.join(self.tmp, "backups") |
||||
self.assertTrue(os.path.isdir(backups_root)) |
||||
stamps = os.listdir(backups_root) |
||||
self.assertEqual(len(stamps), 1) |
||||
self.assertIn("db.sqlite", os.listdir(os.path.join(backups_root, stamps[0]))) |
||||
|
||||
# Step: a dry run reports matches but writes nothing to the DB. |
||||
def test_dry_run_writes_nothing(self): |
||||
with redirect_stdout(io.StringIO()): |
||||
bf.run(self.xml, self.db, apply=False) |
||||
con = sqlite3.connect(self.db) |
||||
unchanged = con.execute("SELECT dateAdded FROM tracks WHERE id=1").fetchone()[0] |
||||
con.close() |
||||
self.assertEqual(unchanged, "2026-05-24 06:46:01.713") |
||||
|
||||
|
||||
if __name__ == "__main__": |
||||
unittest.main() |
||||
Loading…
Reference in new issue