From 095e294f4b91aa3e81d9e80c903e670e11ce8140 Mon Sep 17 00:00:00 2001 From: Chris Date: Thu, 1 Oct 2026 12:36:22 -0700 Subject: [PATCH] Revert "Flag slave-only songs whose album or artist the master has" This reverts commit 4bae674. Co-Authored-By: Claude Opus 5.5 --- README.md | 9 +--- internal/api/export.go | 4 -- internal/engine/missing.go | 91 --------------------------------- internal/engine/missing_test.go | 64 ----------------------- internal/engine/scan.go | 9 ---- internal/store/models.go | 44 ++++------------ internal/store/store.go | 3 -- web/src/api.ts | 2 - web/src/pages/Review.tsx | 70 ++----------------------- 9 files changed, 15 insertions(+), 281 deletions(-) delete mode 100644 internal/engine/missing.go delete mode 100644 internal/engine/missing_test.go diff --git a/README.md b/README.md index 06cc339..e8fd3ad 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ path on both sides. The fallbacks below catch small differences, and a scan warn | Same title after ignoring track numbers/punctuation (`01 - Song!.flac` vs `Song.mp3`) | Status `similar` (similar title). Manual review. | | Same top-level folder and title, but a **different subfolder** | Status `similar` (same top folder, other subfolder). Manual review. | | *(opt-in)* Approximately the same song in another folder, e.g. a **single vs. the album version** or a **disc folder** | Status `similar` (approximate match), with the reason shown. Manual review. | -| Only exists in the slave library | Status `unmatched`. Never touched. Flagged as *missing from master* when the master has the album or artist. | +| Only exists in the slave library | Status `unmatched`. Never touched. | | Already the same inode | Status `linked`. | **Approximate matching** is an opt-in setting on each pair. It finds songs whose file name or folder doesn't line up exactly with the @@ -86,13 +86,6 @@ recommended to keep (the album version, then one already linked to the master, t **Remove** moves the selected copies into the quarantine after re-checking that each is unchanged and that another copy is still there; one copy of every song is always kept, emptied folders are removed, dry run applies, and every removal can be restored from History. -**Missing from master.** The Review tab's **Missing from master** filter lists slave-only songs whose artist the master library -has, so you can complete the master. *Album in master* means the master has the song's album folder but not the song (the slave has -`Caravan Palace/Robot Face/Waterguns.mp3`, the master has `Caravan Palace/Robot Face/` without it); *only the artist in master* means the -whole album is missing. Folders are compared ignoring case, disc folders and release labels such as *Deluxe Edition*, and each row names -the master folder it found. Scans fill this in, nothing is changed, and the CSV export includes it. With approximate matching off, songs -filed under a slightly different name or release also land here. - **Listening in build mode.** Every song in the Build tab has a play button. A mini-player along the bottom of the page plays the folder in order, with seeking, previous/next, volume and space to pause. Songs are streamed from the master library as they are (nothing is converted), so they play when the browser supports the format: FLAC, MP3, AAC/M4A, Ogg/Opus and WAV in current browsers, diff --git a/internal/api/export.go b/internal/api/export.go index 744bdc1..d5fc324 100644 --- a/internal/api/export.go +++ b/internal/api/export.go @@ -63,10 +63,6 @@ func recommendation(m store.Match, v probe.Verdict) string { return "review" case m.Status == store.StatusError: return "fix error" - case m.Missing == "album": - return "not in master: master has the album" - case m.Missing == "artist": - return "not in master: master has the artist" } return "" } diff --git a/internal/engine/missing.go b/internal/engine/missing.go deleted file mode 100644 index 7103767..0000000 --- a/internal/engine/missing.go +++ /dev/null @@ -1,91 +0,0 @@ -package engine - -import ( - "fmt" - "path/filepath" - "strings" -) - -// A slave-only song is "missing from the master" when the master has its -// album folder (the song was probably never added there) or at least its -// artist folder (the album is missing). Folders are compared with disc -// folders ignored and release labels such as "Deluxe Edition" stripped, so -// "Robot Face (Deluxe)" in the slave finds "Robot Face" in the master. - -// Missing levels. -const ( - MissingAlbum = "album" - MissingArtist = "artist" -) - -// folderNorm is a folder name reduced to letters and digits, release labels removed. -func folderNorm(name string) string { - if w := words(stripRelease(name)); len(w) > 0 { - return strings.Join(w, "") - } - return strings.ToLower(name) -} - -// folderKeys returns the normalized artist key and album key of a song's -// folders; the album key is empty for a song directly in the artist folder. -func folderKeys(rel string) (artist, album string) { - var parts []string - for _, p := range strings.Split(filepath.Dir(rel), string(filepath.Separator)) { - if p != "." && !discFolder.MatchString(p) { - parts = append(parts, folderNorm(p)) - } - } - if len(parts) == 0 { - return "", "" - } - if len(parts) > 1 { - album = strings.Join(parts, "/") - } - return parts[0], album -} - -// masterFolders indexes the master's artist and album folders by key, to -// their real spelling. -type masterFolders struct { - artists map[string]string - albums map[string]string -} - -func buildMasterFolders(rels []string) masterFolders { - mf := masterFolders{artists: map[string]string{}, albums: map[string]string{}} - for _, rel := range rels { - artist, album := folderKeys(rel) - if artist == "" { - continue - } - if _, ok := mf.artists[artist]; !ok { - mf.artists[artist] = firstComponent(rel) - } - if album != "" { - if _, ok := mf.albums[album]; !ok { - d := filepath.Dir(rel) - for d != "." && discFolder.MatchString(filepath.Base(d)) { - d = filepath.Dir(d) - } - mf.albums[album] = d - } - } - } - return mf -} - -// missing reports whether the master has a slave-only song's album or -// artist, with a reason naming the master's folder. -func (mf masterFolders) missing(relSlave string) (level, reason string) { - artist, album := folderKeys(relSlave) - if artist == "" { - return "", "" - } - if d, ok := mf.albums[album]; ok && album != "" { - return MissingAlbum, fmt.Sprintf("the master has the album %q but not this song", d) - } - if d, ok := mf.artists[artist]; ok { - return MissingArtist, fmt.Sprintf("the master has the artist %q but not this album", d) - } - return "", "" -} diff --git a/internal/engine/missing_test.go b/internal/engine/missing_test.go deleted file mode 100644 index dad0032..0000000 --- a/internal/engine/missing_test.go +++ /dev/null @@ -1,64 +0,0 @@ -package engine - -import ( - "strings" - "testing" - - "github.com/cdnutter/playlink/internal/store" -) - -func TestMissingFromMaster(t *testing.T) { - ev := setup(t, false) - m, s := ev.master, ev.slave - ev.write(m, "Caravan Palace/Robot Face/01 Lone Digger.flac", "a") - ev.write(m, "Caravan Palace/Chronologic/Disc 1/01 Miracle.flac", "b") - ev.write(s, "Caravan Palace/Robot Face/01 Lone Digger.flac", "a") - ev.write(s, "Caravan Palace/Robot Face/05 Waterguns.mp3", "song") // album in master - ev.write(s, "Caravan Palace/robot face (Deluxe Edition)/12 Bonus.mp3", "song") // same album, other spelling - ev.write(s, "Caravan Palace/Chronologic/Disc 2/03 Plume.mp3", "song") // disc folders ignored - ev.write(s, "Caravan Palace/Panic/01 Dragons.mp3", "song") // only the artist - ev.write(s, "Caravan Palace/Loose.mp3", "song") // only the artist - ev.write(s, "Unknown Artist/Album/Song.mp3", "song") // neither - ev.scan() - - want := map[string]string{ - "Caravan Palace/Robot Face/05 Waterguns.mp3": "album", - "Caravan Palace/robot face (Deluxe Edition)/12 Bonus.mp3": "album", - "Caravan Palace/Chronologic/Disc 2/03 Plume.mp3": "album", - "Caravan Palace/Panic/01 Dragons.mp3": "artist", - "Caravan Palace/Loose.mp3": "artist", - "Unknown Artist/Album/Song.mp3": "", - "Caravan Palace/Robot Face/01 Lone Digger.flac": "", - } - for rel, level := range want { - mt, err := ev.st.MatchBySlave(ctx, ev.pair.ID, rel) - if err != nil { - t.Fatal(rel, err) - } - if mt.Missing != level { - t.Errorf("%s: missing %q, want %q", rel, mt.Missing, level) - } - } - mt, _ := ev.st.MatchBySlave(ctx, ev.pair.ID, "Caravan Palace/Chronologic/Disc 2/03 Plume.mp3") - if !strings.Contains(mt.Reason, `"Caravan Palace/Chronologic"`) { - t.Errorf("reason should name the master's album folder: %q", mt.Reason) - } - - sum, err := ev.st.Summary(ctx, ev.pair.ID) - if err != nil || sum.Missing["album"] != 3 || sum.Missing["artist"] != 2 { - t.Fatalf("summary %+v %v", sum.Missing, err) - } - if _, n, _ := ev.st.Matches(ctx, ev.pair.ID, store.MatchFilter{Status: "missing"}); n != 5 { - t.Fatalf("missing filter: %d", n) - } - if _, n, _ := ev.st.Matches(ctx, ev.pair.ID, store.MatchFilter{Status: "missing_album"}); n != 3 { - t.Fatalf("missing_album filter: %d", n) - } - - // Once the master gets the song, it is no longer missing. - ev.write(m, "Caravan Palace/Robot Face/05 Waterguns.mp3", "song") - ev.scan() - if mt, _ := ev.st.MatchBySlave(ctx, ev.pair.ID, "Caravan Palace/Robot Face/05 Waterguns.mp3"); mt.Missing != "" || mt.Status == store.StatusUnmatched { - t.Fatalf("now in master: %+v", mt) - } -} diff --git a/internal/engine/scan.go b/internal/engine/scan.go index b77a7ea..2c6ec15 100644 --- a/internal/engine/scan.go +++ b/internal/engine/scan.go @@ -105,8 +105,6 @@ type masterIndex struct { byDir map[string][]string // lower(dir) -> rels byTitle map[string][]string // lower(top folder)|normTitle -> rels - folders masterFolders // artist and album folders, for songs missing from the master - approx *approxIndex // nil unless the pair enables approximate matching threshold float64 verify VerifyFunc // confirms library-wide approximate matches; nil disables that scope @@ -115,7 +113,6 @@ type masterIndex struct { func buildIndex(files map[string]fsops.Info, p store.Pair) *masterIndex { ix := &masterIndex{files: files, lower: map[string]string{}, byDir: map[string][]string{}, byTitle: map[string][]string{}} rels := sortedKeys(files) - ix.folders = buildMasterFolders(rels) if p.ApproxMatching { ix.approx = buildApproxIndex(rels) ix.threshold = float64(p.ApproxThreshold) / 100 @@ -330,9 +327,6 @@ func (e *Engine) scan(ctx context.Context, j *Job, pairID int64, trigger string) pm = &p } e.classify(ctx, l, &m, masterFiles[m.RelMaster], si, pm) - if m.Status == store.StatusUnmatched { - m.Missing, m.Reason = ix.folders.missing(rel) - } if err := e.st.UpsertMatch(ctx, &m); err != nil { return err } @@ -489,9 +483,6 @@ func (e *Engine) refresh(ctx context.Context, l *libs, relSlave string, hint ... m.Kind = store.KindNone } e.classify(ctx, l, &m, mi, si, pm) - if m.Status == store.StatusUnmatched && pm != nil && pm.Status == store.StatusUnmatched { - m.Missing, m.Reason = pm.Missing, pm.Reason // the next scan re-checks the master's folders - } _ = e.st.UpsertMatch(ctx, &m) } diff --git a/internal/store/models.go b/internal/store/models.go index 23994bd..36700dc 100644 --- a/internal/store/models.go +++ b/internal/store/models.go @@ -286,21 +286,20 @@ type Match struct { SlaveProbe json.RawMessage `json:"slave_probe,omitempty"` Verdict json.RawMessage `json:"verdict,omitempty"` Note string `json:"note"` - Reason string `json:"reason"` // why an approximate match was chosen, or what the master has of a missing song - Missing string `json:"missing"` // slave-only song: the master has its "album" or only its "artist" + Reason string `json:"reason"` // why an approximate match was chosen LinkTarget string `json:"link_target"` // where linking puts the slave file (filled by the API) Ignored bool `json:"ignored"` UpdatedAt int64 `json:"updated_at"` } const matchCols = `id, pair_id, rel_slave, rel_master, kind, status, master_ino, master_size, master_mtime, - slave_ino, slave_size, slave_mtime, master_probe, slave_probe, verdict, note, reason, missing, ignored, updated_at` + slave_ino, slave_size, slave_mtime, master_probe, slave_probe, verdict, note, reason, ignored, updated_at` func scanMatch(r scanner) (Match, error) { var m Match var mp, sp, v string err := r.Scan(&m.ID, &m.PairID, &m.RelSlave, &m.RelMaster, &m.Kind, &m.Status, &m.MasterIno, &m.MasterSize, &m.MasterMtime, - &m.SlaveIno, &m.SlaveSize, &m.SlaveMtime, &mp, &sp, &v, &m.Note, &m.Reason, &m.Missing, &m.Ignored, &m.UpdatedAt) + &m.SlaveIno, &m.SlaveSize, &m.SlaveMtime, &mp, &sp, &v, &m.Note, &m.Reason, &m.Ignored, &m.UpdatedAt) if mp != "" { m.MasterProbe = json.RawMessage(mp) } @@ -349,7 +348,7 @@ func (s *Store) AllMatches(ctx context.Context, pairID int64) (map[string]Match, } type MatchFilter struct { - Status string // a status, "review" (different+similar), "upgrade", "missing[_album|_artist]", "ignored" or "" + Status string // a status, "review" (different+similar), "ignored" or "" Query string IDs []int64 // restrict to these matches (at most maxIDFilter) Limit int @@ -368,11 +367,6 @@ func matchWhere(pairID int64, f MatchFilter) (string, []any) { where = append(where, "ignored = 1") case "upgrade": where = append(where, "status IN ('different','similar') AND ignored = 0 AND "+upgradeCond) - case "missing": - where = append(where, "status = 'unmatched' AND missing != '' AND ignored = 0") - case "missing_album", "missing_artist": - where = append(where, "status = 'unmatched' AND missing = ? AND ignored = 0") - args = append(args, strings.TrimPrefix(f.Status, "missing_")) default: where = append(where, "status = ? AND ignored = 0") args = append(args, f.Status) @@ -514,15 +508,15 @@ func (s *Store) IdenticalIDs(ctx context.Context, pairID int64) ([]int64, error) func (s *Store) UpsertMatch(ctx context.Context, m *Match) error { nullable := func(r json.RawMessage) string { return string(r) } _, err := s.DB.ExecContext(ctx, `INSERT INTO matches (pair_id, rel_slave, rel_master, kind, status, master_ino, master_size, - master_mtime, slave_ino, slave_size, slave_mtime, master_probe, slave_probe, verdict, note, reason, missing, ignored, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + master_mtime, slave_ino, slave_size, slave_mtime, master_probe, slave_probe, verdict, note, reason, ignored, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(pair_id, rel_slave) DO UPDATE SET rel_master = excluded.rel_master, kind = excluded.kind, status = excluded.status, master_ino = excluded.master_ino, master_size = excluded.master_size, master_mtime = excluded.master_mtime, slave_ino = excluded.slave_ino, slave_size = excluded.slave_size, slave_mtime = excluded.slave_mtime, master_probe = excluded.master_probe, slave_probe = excluded.slave_probe, - verdict = excluded.verdict, note = excluded.note, reason = excluded.reason, missing = excluded.missing, ignored = excluded.ignored, updated_at = excluded.updated_at`, + verdict = excluded.verdict, note = excluded.note, reason = excluded.reason, ignored = excluded.ignored, updated_at = excluded.updated_at`, m.PairID, m.RelSlave, m.RelMaster, m.Kind, m.Status, m.MasterIno, m.MasterSize, m.MasterMtime, m.SlaveIno, m.SlaveSize, - m.SlaveMtime, nullable(m.MasterProbe), nullable(m.SlaveProbe), nullable(m.Verdict), m.Note, m.Reason, m.Missing, m.Ignored, now()) + m.SlaveMtime, nullable(m.MasterProbe), nullable(m.SlaveProbe), nullable(m.Verdict), m.Note, m.Reason, m.Ignored, now()) return err } @@ -585,13 +579,12 @@ type Summary struct { Review int `json:"review"` Upgrades int `json:"upgrades"` Ignored int `json:"ignored"` - Missing map[string]int `json:"missing"` // slave-only songs by what the master has: album, artist ReclaimableSize int64 `json:"reclaimable_bytes"` LinkedSize int64 `json:"linked_bytes"` } func (s *Store) Summary(ctx context.Context, pairID int64) (Summary, error) { - sum := Summary{Counts: map[string]int{}, Missing: map[string]int{}} + sum := Summary{Counts: map[string]int{}} rows, err := s.DB.QueryContext(ctx, `SELECT status, ignored, COUNT(*), COALESCE(SUM(slave_size), 0) FROM matches WHERE pair_id = ? GROUP BY status, ignored`, pairID) if err != nil { return sum, err @@ -624,24 +617,7 @@ func (s *Store) Summary(ctx context.Context, pairID int64) (Summary, error) { } err = s.DB.QueryRowContext(ctx, `SELECT COUNT(*) FROM matches WHERE pair_id = ? AND ignored = 0 AND status IN ('different','similar') AND `+upgradeCond, pairID).Scan(&sum.Upgrades) - if err != nil { - return sum, err - } - mrows, err := s.DB.QueryContext(ctx, `SELECT missing, COUNT(*) FROM matches WHERE pair_id = ? AND ignored = 0 - AND status = 'unmatched' AND missing != '' GROUP BY missing`, pairID) - if err != nil { - return sum, err - } - defer mrows.Close() - for mrows.Next() { - var k string - var n int - if err := mrows.Scan(&k, &n); err != nil { - return sum, err - } - sum.Missing[k] = n - } - return sum, mrows.Err() + return sum, err } // ---- operations ---- diff --git a/internal/store/store.go b/internal/store/store.go index ea424c1..0f4a0ea 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -140,9 +140,6 @@ var migrations = []string{ // 10: library type (music or soundtracks) and the step-by-step fixer's progress. `ALTER TABLE pairs ADD COLUMN library_type TEXT NOT NULL DEFAULT 'music'; ALTER TABLE pairs ADD COLUMN fixer_state TEXT NOT NULL DEFAULT '';`, - - // 11: slave-only songs whose album or artist the master has. - `ALTER TABLE matches ADD COLUMN missing TEXT NOT NULL DEFAULT '';`, } // SchemaVersion is the newest database schema this build knows. diff --git a/web/src/api.ts b/web/src/api.ts index c7b386a..a549a15 100644 --- a/web/src/api.ts +++ b/web/src/api.ts @@ -57,7 +57,6 @@ export type Summary = { review: number; upgrades: number; ignored: number; - missing?: { album?: number; artist?: number }; reclaimable_bytes: number; linked_bytes: number; }; @@ -133,7 +132,6 @@ export type Match = { verdict?: Verdict; note: string; reason: string; - missing: "" | "album" | "artist"; link_target: string; ignored: boolean; }; diff --git a/web/src/pages/Review.tsx b/web/src/pages/Review.tsx index da893f7..7dd0913 100644 --- a/web/src/pages/Review.tsx +++ b/web/src/pages/Review.tsx @@ -1,6 +1,6 @@ import { useEffect, useState } from "react"; import { keepPreviousData, useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { AlertTriangle, ArrowRight, ArrowUpCircle, Disc3, Download, FolderSearch, ListMusic, ChevronDown, ChevronRight, CopyCheck, CopyMinus, EyeOff, Eye, Link2, Search, Crown } from "lucide-react"; +import { AlertTriangle, ArrowRight, ArrowUpCircle, Disc3, Download, ListMusic, ChevronDown, ChevronRight, CopyCheck, CopyMinus, EyeOff, Eye, Link2, Search, Crown } from "lucide-react"; import { ApiError, api, type Match, type Page, type Pair, type ProbeInfo } from "../api"; import { Alert, Badge, Button, Card, Empty, Modal, Spinner, cx, formatBytes, formatDuration, inputCls, statusTone } from "../ui"; import { useToast } from "../toast"; @@ -13,7 +13,6 @@ const filters = [ { id: "identical", label: "Identical" }, { id: "linked", label: "Linked" }, { id: "unmatched", label: "Slave only" }, - { id: "missing", label: "Missing from master" }, { id: "error", label: "Errors" }, { id: "ignored", label: "Ignored" }, { id: "", label: "All" }, @@ -87,21 +86,8 @@ export function Review({ pair }: { pair: Pair }) { }); const c = pair.summary.counts; - const miss = pair.summary.missing ?? {}; const count = (id: string) => - id === "review" - ? pair.summary.review - : id === "upgrade" - ? pair.summary.upgrades - : id === "ignored" - ? pair.summary.ignored - : id === "missing" - ? (miss.album ?? 0) + (miss.artist ?? 0) - : id === "" - ? undefined - : c[id] ?? 0; - // The missing filter has two levels; the chip covers all three. - const active = (id: string) => status === id || (id === "missing" && status.startsWith("missing_")); + id === "review" ? pair.summary.review : id === "upgrade" ? pair.summary.upgrades : id === "ignored" ? pair.summary.ignored : id === "" ? undefined : c[id] ?? 0; const identical = c.identical ?? 0; const running = pair.job?.state === "running"; @@ -114,7 +100,7 @@ export function Review({ pair }: { pair: Pair }) { onClick={() => setStatus(f.id)} className={cx( "flex items-center gap-1.5 rounded-full border px-3 py-1 text-xs cursor-pointer", - active(f.id) ? "border-accent bg-accent/15 text-white" : "border-line text-muted hover:text-white", + status === f.id ? "border-accent bg-accent/15 text-white" : "border-line text-muted hover:text-white", )} > {f.label} @@ -153,7 +139,7 @@ export function Review({ pair }: { pair: Pair }) { ))} - active(f.id))?.label ?? "All"} /> + f.id === status)?.label ?? "All"} /> )} @@ -162,7 +148,6 @@ export function Review({ pair }: { pair: Pair }) { ) : ( <> - {status.startsWith("missing") && } {status === "identical" && identical > 0 && ( @@ -248,50 +233,6 @@ export function Review({ pair }: { pair: Pair }) { ); } -function MissingIntro({ pair, status, setStatus }: { pair: Pair; status: string; setStatus: (s: string) => void }) { - const miss = pair.summary.missing ?? {}; - const levels = [ - { id: "missing", label: "All", n: (miss.album ?? 0) + (miss.artist ?? 0) }, - { id: "missing_album", label: "Album in master", n: miss.album ?? 0 }, - { id: "missing_artist", label: "Only the artist in master", n: miss.artist ?? 0 }, - ]; - return ( - -
- -
-
Songs the master library is missing
-
- These songs are only in the slave library, but the master has their artist, and often the album too. For example, the slave has{" "} - Caravan Palace/Robot Face/Waterguns and the master has the Caravan Palace/Robot Face album without that song. Add them to - the master to complete it, then scan again to link them. Nothing here is changed automatically. -
- {!pair.approx_matching && ( -
- Approximate matching is off for this pair, so a song filed under a slightly different name or another release (a single vs. the - album) shows up here too. Turn it on in Edit to find those. -
- )} -
-
-
- {levels.map((l) => ( - - ))} -
- - ); -} - function ExportMenu({ pairId, status, query, viewCount, filterLabel }: { pairId: number; status: string; query: string; viewCount?: number; filterLabel: string }) { const [open, setOpen] = useState(false); useEffect(() => { @@ -394,9 +335,6 @@ export function MatchRow({ m, selected, onToggle }: { m: Match; selected?: boole
{m.status} {m.kind !== "path" && m.kind !== "none" && {kindLabel[m.kind]}} - {m.status === "unmatched" && m.missing && ( - {m.missing === "album" ? "album in master" : "artist in master"} - )}
reviewable && setOpen(!open)}>