diff --git a/.planning/STATE.md b/.planning/STATE.md index 5845de3..1e93ddf 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -26,7 +26,7 @@ Phase: 12 — Library CRUD & Data Integrity (completed), 13 next Plan: 2 of 2 in Phase 12 (complete) Status: Plan 12-02 complete — frontend library management UI verified Progress: ████████████████████ Phase 12 complete (2/2 plans), Phase 14 complete (4/4 plans) -Last activity: 2026-03-15 — Completed 12-02 frontend library management UI +Last activity: 2026-03-16 — Completed quick task 19: fix phantom playlist tracks ### Phase Overview diff --git a/.planning/quick/19-fix-phantom-playlist-tracks/19-PLAN.md b/.planning/quick/19-fix-phantom-playlist-tracks/19-PLAN.md new file mode 100644 index 0000000..37ae3bb --- /dev/null +++ b/.planning/quick/19-fix-phantom-playlist-tracks/19-PLAN.md @@ -0,0 +1,256 @@ +--- +phase: quick-19 +plan: 19 +type: execute +wave: 1 +depends_on: [] +files_modified: + - backend/playlist/playlist.go + - backend/playlist/m3u.go + - backend/playlist/m3u_test.go +autonomous: true +requirements: [] + +must_haves: + truths: + - "Playlist tracks resolve correctly when multiple libraries exist" + - "Playlist tracks resolve correctly after FullRescan with multiple libraries" + - "M3U8 relative paths are resolved against all library roots, not just the first" + - "M3U8 relative paths are saved relative to the correct library root for each track" + artifacts: + - path: "backend/playlist/playlist.go" + provides: "getAllLibraryRoots() replacing getLibraryRoot(), multi-root resolution at all call sites" + - path: "backend/playlist/m3u.go" + provides: "toAbsolutePathMultiRoot() and updated helper functions" + - path: "backend/playlist/m3u_test.go" + provides: "Tests for multi-root path resolution" + key_links: + - from: "backend/playlist/playlist.go mergeTracksForPlaylist()" + to: "backend/playlist/m3u.go toAbsolutePathMultiRoot()" + via: "function call for each M3U8 entry" + pattern: "toAbsolutePathMultiRoot" +--- + + +Fix phantom playlist tracks caused by single-library-root path resolution in a multi-library environment. + +Purpose: The playlist M3U8 merge logic uses `getLibraryRoot()` which returns only the first library's path. When tracks belong to other libraries, their relative M3U8 paths resolve against the wrong root, causing them to appear as phantoms. After `FullRescan`, `RestoreAllPlaylists` also only uses one root, so tracks from non-first libraries fail to re-link. + +Output: Multi-root path resolution across all playlist path operations — tracks from any library resolve correctly. + + + +@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md +@/home/caleb/.config/opencode/get-shit-done/templates/summary.md + + + +@.planning/STATE.md +@backend/playlist/playlist.go +@backend/playlist/m3u.go +@backend/playlist/m3u_test.go + + + + + +From backend/playlist/m3u.go: +```go +func toAbsolutePath(relativePath, libraryRoot string) string +func toRelativePath(absolutePath, libraryRoot string) string +func removeM3UEntries(entries []m3uEntry, targetAbsPaths map[string]struct{}, libraryRoot string) []m3uEntry +func replaceM3UEntryPaths(entries []m3uEntry, replacements map[string]string, libraryRoot string) []m3uEntry +func findM3UEntry(entries []m3uEntry, targetAbsPath string, libraryRoot string) (m3uEntry, int) +``` + +From backend/playlist/playlist.go: +```go +func (s *Service) getLibraryRoot() string // returns single root — THE BUG +// Called at lines: 370, 844, 984, 1304, 1448, 1536, 1616, 1743 +``` + +From backend/database/sql/sqlcgen/libraries.sql.go: +```go +func (q *Queries) GetAllLibraries(ctx context.Context) ([]Library, error) +// Library has: ID int64, Name string, Path string +``` + + + + + + Task 1: Add multi-root path resolution to m3u.go and update playlist.go call sites + + backend/playlist/m3u.go + backend/playlist/playlist.go + + +**Part A — m3u.go: Add multi-root resolution functions** + +1. Add `toAbsolutePathMultiRoot(relativePath string, libraryRoots []string) string`: + - If `filepath.IsAbs(relativePath)`, return as-is (same as current) + - For each root in `libraryRoots`, compute `filepath.Join(root, relativePath)` — return the first result (all roots will produce valid-looking paths; the caller uses the result as a map key, so the first root that matches wins) + - Actually, this function should try each root and check if the resulting path exists as a key in a provided map. BUT that couples m3u.go to the caller's map. Instead, return ALL possible absolute paths and let the caller check. + - **Better approach**: Keep it simple. Add `resolveM3UPath(relativePath string, libraryRoots []string, knownPaths map[string]struct{}) string` that: + - If `filepath.IsAbs(relativePath)` and path is in `knownPaths`, return it + - For each root, compute `absPath := filepath.Join(root, relativePath)` and check `knownPaths[absPath]`; if found, return `absPath` + - If no root matches, fall back to `filepath.Join(libraryRoots[0], relativePath)` if roots non-empty, else return `relativePath` (preserves current behavior for phantoms) + - This keeps resolution in one place and avoids O(n) loops at each call site. + +2. Update `removeM3UEntries` signature: `removeM3UEntries(entries []m3uEntry, targetAbsPaths map[string]struct{}, libraryRoots []string) []m3uEntry` + - Inside, for each entry, try `toAbsolutePath` against each root, check if any resolves into `targetAbsPaths` + +3. Update `replaceM3UEntryPaths` signature: `replaceM3UEntryPaths(entries []m3uEntry, replacements map[string]string, libraryRoots []string) []m3uEntry` + - Inside, for each entry, try each root, check if any resolves into `replacements` + +4. Update `findM3UEntry` signature: `findM3UEntry(entries []m3uEntry, targetAbsPath string, libraryRoots []string) (m3uEntry, int)` + - Inside, for each entry, try each root, compare against target + +5. Add `toRelativePathMultiRoot(absolutePath string, libraryRoots []string) string`: + - Try `filepath.Rel(root, absolutePath)` for each root + - Return the first result that doesn't start with ".." (i.e., the path is under that root) + - If no root matches, return `absolutePath` (keeps absolute, same as current fallback) + +**Part B — playlist.go: Replace getLibraryRoot with getAllLibraryRoots** + +1. Replace `getLibraryRoot() string` with `getAllLibraryRoots() []string`: + ```go + func (s *Service) getAllLibraryRoots() []string { + // Legacy config fallback. + if s.libraryDir != nil { + if dir := s.libraryDir.GetLibraryDirectory(); dir != "" { + return []string{dir} + } + } + libs, err := s.db.Queries.GetAllLibraries(s.db.Ctx) + if err != nil || len(libs) == 0 { + return nil + } + roots := make([]string, len(libs)) + for i, lib := range libs { + roots[i] = lib.Path + } + return roots + } + ``` + +2. Update `mergeTracksForPlaylist` (line ~370): + - Change `libraryRoot := s.getLibraryRoot()` to `libraryRoots := s.getAllLibraryRoots()` + - Build a `knownPaths` set from `dbTracks` keys: `knownPaths := make(map[string]struct{}, len(dbTracks)); for k := range dbTracks { knownPaths[k] = struct{}{} }` + - Replace the loop body to use `resolveM3UPath(entry.RelativePath, libraryRoots, knownPaths)` instead of `toAbsolutePath(entry.RelativePath, libraryRoot)` + +3. Update `importPlaylist` (line ~844): + - Change to `libraryRoots := s.getAllLibraryRoots()` + - For each entry, try resolving against each root: `absPath := toAbsolutePath(entry.RelativePath, root)` then `GetAudioFileByPath(absPath)` + - But that's O(entries × roots) DB queries. Better: just try each root's absolute path, use first that works. Wrap in a helper if cleaner. + - Actually, for import, the current approach makes one DB query per entry. With multi-root, wrap in a loop: try each root until `GetAudioFileByPath` succeeds. + +4. Update `RestoreAllPlaylists` / `restoreSinglePlaylist` (line ~984, ~1018): + - Same pattern: pass `libraryRoots` instead of `libraryRoot` + - `restoreSinglePlaylist` already receives `libraryRoot string` — change to `libraryRoots []string` + - Inside, for each M3U8 entry, try each root + +5. Update `buildM3UEntries` (line ~1304): + - Use `toRelativePathMultiRoot(row.FilePath, libraryRoots)` — this finds the correct root for each track's absolute path + +6. Update `saveImportedPlaylistFile` (line ~1245): + - Change to multi-root, use `toRelativePathMultiRoot` + +7. Update `FindPhantomMatches` (line ~1448): + - Pass `libraryRoots` to `findM3UEntry` and entry resolution + +8. Update `GetPhantomCandidates` (line ~1536): + - Same pattern + +9. Update `ResolvePhantomTracks` (line ~1616): + - Same pattern + +10. Update `RemovePhantomTracks` (line ~1743): + - Same pattern + +**Delete `getLibraryRoot`** after all call sites are migrated. Keep `toAbsolutePath` and `toRelativePath` — they're still useful as single-root primitives called by the multi-root wrappers. + +**IMPORTANT CODEBASE PATTERNS:** +- Mutex-protected setter pattern (lock → write → release → callbacks) +- SAFETY comment convention for hand-crafted SQL +- Do NOT change any SQL queries — this is purely Go-side path resolution + + + go build ./backend/... compiles without errors + go vet ./backend/playlist/... passes + + + - `getLibraryRoot()` replaced with `getAllLibraryRoots()` returning all library paths + - `mergeTracksForPlaylist` resolves M3U8 entries against all roots + - `restoreSinglePlaylist` resolves M3U8 entries against all roots + - `buildM3UEntries` saves relative paths using correct root per track + - All m3u.go helper functions accept `[]string` roots + - All 9 call sites updated + + + + + Task 2: Add multi-root path resolution tests + backend/playlist/m3u_test.go + +Add tests to m3u_test.go for the new multi-root functions: + +1. `TestToAbsolutePathMultiRoot` (or `TestResolveM3UPath`): + - Relative path resolves against first matching root + - Absolute path returned as-is + - Empty roots returns relative path unchanged + - Path under second root resolves correctly (not just first) + +2. `TestToRelativePathMultiRoot`: + - Path under first root returns relative to first root + - Path under second root returns relative to second root + - Path under no root returns absolute + - Empty roots returns absolute + +3. Update existing `TestRemoveM3UEntries` to use `[]string{"/music"}` instead of `"/music"` + +4. Update existing `TestRemoveM3UEntriesAll` similarly + +5. Update existing `TestReplaceM3UEntryPaths` similarly + +6. Update existing `TestFindM3UEntry` similarly + +7. Add a multi-root variant of `TestRemoveM3UEntries`: + - Entries from different roots, targets from mixed roots, correct entries removed + +8. Add a multi-root variant of `TestFindM3UEntry`: + - Entry relative to second root found correctly + +Follow existing test patterns: table-driven, `t.Parallel()`, descriptive names. + + + go test ./backend/playlist/... -run "TestResolveM3UPath|TestToRelativePathMultiRoot|TestRemoveM3UEntries|TestReplaceM3UEntryPaths|TestFindM3UEntry" -v passes + + + - Multi-root resolution tested with multiple library roots + - Edge cases covered (empty roots, absolute paths, no match) + - Existing tests updated for new signatures + - All tests pass + + + + + + +1. `go build ./backend/...` — compiles cleanly +2. `go vet ./backend/playlist/...` — no issues +3. `go test ./backend/playlist/...` — all tests pass (existing + new) +4. `go test ./backend/...` — full backend test suite passes (no regressions) + + + +- Playlist M3U8 path resolution tries all library roots instead of just the first +- M3U8 save uses the correct library root for each track's absolute path +- All existing playlist tests continue to pass +- New tests verify multi-root resolution behavior +- No changes to SQL queries or schema + + + +After completion, create `.planning/quick/19-fix-phantom-playlist-tracks/19-SUMMARY.md` +