From ef89707bb14d48cdd6256d7c6bdfb3f1b36ea920 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Sat, 26 Sep 2026 22:11:03 -0400 Subject: [PATCH] feat(download): fill in the tracks a nearly complete album is missing A grab that delivers nine of twelve tracks clears the completeness floor and is imported, and the other three were never looked for. On Soulseek that is the commonest way an album ends up almost right: one peer's folder lacks a track, or one file fails. After a successful import the manager now compares the tracks the files were aligned to (ImportResult.Matched, new) with the expected tracklist. When one to three are missing, and fewer than half, it searches for each one on its own, as the track's artist and title with the album kept for ranking. It grabs the first auto-acceptable copy that is not from the source that already failed to supply it, and is not a delegate, which would place it in its own library. The candidate is trimmed to the one file aligned to the track. The import uses the album's own request with ImportOptions.Only, which skips the completeness check and imports only a file aligned to the missing track, so it is tagged and placed as part of the album, and anything else the folder brought is left out. One attempt per track, and nothing here fails the download: the album is already imported, so a track that cannot be found is logged on the job and left. slskd accepts one-file folders for a request that expects one track, which the per-track search needs. Closes #276 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT --- backend/download/fillin.go | 190 +++++++++++++++++++++++++++++ backend/download/fillin_test.go | 168 +++++++++++++++++++++++++ backend/download/importer.go | 63 +++++++++- backend/download/manager.go | 5 +- backend/download/provider_slskd.go | 4 +- 5 files changed, 426 insertions(+), 4 deletions(-) create mode 100644 backend/download/fillin.go create mode 100644 backend/download/fillin_test.go diff --git a/backend/download/fillin.go b/backend/download/fillin.go new file mode 100644 index 0000000..d645c6c --- /dev/null +++ b/backend/download/fillin.go @@ -0,0 +1,190 @@ +package download + +import ( + "cmp" + "context" + "fmt" + "strings" + + "yellowjacket/backend/jobs" +) + +// Filling in an almost-complete album (#276). +// +// A grab that delivers nine of twelve tracks clears the completeness +// floor and is imported, and before this the other three were never +// looked for. On Soulseek that is the commonest way an album ends up +// almost right: one peer's folder is missing a track, or one file +// failed. So after an import, each missing track is searched for on +// its own and fetched from somewhere else, into the same album. + +// maxFillInTracks bounds how many tracks are fetched one by one. An +// album missing more than a few is a different candidate's job, not a +// dozen single-track grabs. +const maxFillInTracks = 3 + +// fillIn fetches the tracks a successful import did not deliver. It +// never fails the download: the album is already imported, and a track +// it cannot find is logged and left. +func (m *Manager) fillIn( + ctx context.Context, + dl Download, + main Candidate, + imported ImportResult, + job *jobs.Handle, +) { + missing := missingTracks(dl, imported.Matched) + if len(missing) == 0 { + return + } + + for _, t := range missing { + if ctx.Err() != nil { + return + } + + paths, err := m.fillInTrack(ctx, dl, main, t, job) + if err != nil { + m.logger.Info( + "could not fill in a missing track", + "download", dl.ID, + "track", t.Title, + "error", err, + ) + + if job != nil { + job.Logf(jobs.LevelWarn, fmt.Sprintf( + "Could not find %q elsewhere: %v", t.Title, err, + )) + } + + continue + } + + if job != nil { + job.Logf(jobs.LevelInfo, fmt.Sprintf( + "Filled in %q from another source (%d file)", t.Title, len(paths), + )) + } + } +} + +// missingTracks is what an import left out, when filling it in is +// worth trying: an album with a tracklist, a few tracks short. +func missingTracks(dl Download, matched []ExpectedTrack) []ExpectedTrack { + // A recording request is one track; there is no album to complete. + if dl.RecordingMBID != "" || len(dl.Expected) < 2 { + return nil + } + + have := make(map[trackKey]bool, len(matched)) + for _, t := range matched { + have[keyOf(t)] = true + } + + var missing []ExpectedTrack + + for _, t := range dl.Expected { + if !have[keyOf(t)] { + missing = append(missing, t) + } + } + + // A half-empty album was a poor copy, not a nearly complete one. + if len(missing) > maxFillInTracks || 2*len(missing) >= len(dl.Expected) { + return nil + } + + return missing +} + +// fillInTrack searches for one track and grabs the first acceptable +// copy that is not from the source that already failed to supply it. +// One attempt: a fill-in that walks a candidate list per track would +// multiply a download's grabs by the number of gaps. +func (m *Manager) fillInTrack( + ctx context.Context, + dl Download, + main Candidate, + t ExpectedTrack, + job *jobs.Handle, +) ([]string, error) { + want := trackRequest(dl, t) + + ranked, err := m.Search(ctx, want) + if err != nil { + return nil, err + } + + prefs := m.preferences() + + for _, c := range ranked { + if ruledOutBy(c, []Candidate{main}) || !autoAcceptable(want, c, prefs) { + continue + } + + // The track is being put into an album this app placed; a + // delegate would put it in its own library instead. + if plan, err := m.planTransfer(want, c); err != nil || plan.delegated() { + continue + } + + narrowed, ok := narrowTo(c, t) + if !ok { + continue + } + + out := m.attemptGrab(ctx, dl, narrowed, job, []ExpectedTrack{t}) + + if out.item.StagingDir != "" { + if err := m.staging.Release(out.item.StagingDir); err != nil { + m.logger.Warn("could not release staging dir", "error", err) + } + } + + if out.err != nil { + return nil, out.err + } + + if err := m.store.SetItemImported( + ctx, out.item.ID, out.imported.Paths, + ); err != nil { + m.logger.Warn("could not record imported paths", "error", err) + } + + return out.imported.Paths, nil + } + + return nil, ErrNoCandidates +} + +// trackRequest is the search for one track of an album: the track's +// artist and title as the query, the album kept so a copy from that +// album outranks the same song off a compilation, and one expected +// track so a single file is a complete answer. +func trackRequest(dl Download, t ExpectedTrack) Download { + artist := cmp.Or(t.Artist, dl.Artist) + + want := dl + want.Query = strings.TrimSpace(artist + " " + t.Title) + want.Expected = []ExpectedTrack{t} + + return want +} + +// narrowTo trims a candidate to the one file that aligns to t, so the +// grab fetches a track rather than whatever else the folder offered. +func narrowTo(c Candidate, t ExpectedTrack) (Candidate, bool) { + aligned, _ := matchFiles(c.Files, []ExpectedTrack{t}) + + for _, f := range aligned { + if f.IsAudio && f.MatchedTo == t.Position { + c.Files = []CandidateFile{f} + c.TotalSize = f.Size + + return c, true + } + } + + return Candidate{}, false +} diff --git a/backend/download/fillin_test.go b/backend/download/fillin_test.go new file mode 100644 index 0000000..7b79d47 --- /dev/null +++ b/backend/download/fillin_test.go @@ -0,0 +1,168 @@ +package download + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" +) + +// Filling in the tracks an almost-complete album is missing (#276). + +func fiveTrackTitles() []string { + return append(allTitles(), "Let Down") +} + +func fiveTrackDownload() Download { + dl := fourTrackDownload() + dl.Expected = append(dl.Expected, ExpectedTrack{Position: 5, Title: "Let Down"}) + + return dl +} + +func TestMissingTracks(t *testing.T) { + t.Parallel() + + dl := fiveTrackDownload() + got := func(positions ...int) []ExpectedTrack { + out := make([]ExpectedTrack, 0, len(positions)) + for _, p := range positions { + out = append(out, dl.Expected[p-1]) + } + + return out + } + + cases := []struct { + name string + dl Download + matched []ExpectedTrack + want int + }{ + {"complete", dl, got(1, 2, 3, 4, 5), 0}, + {"one short", dl, got(1, 2, 3, 4), 1}, + {"two short", dl, got(1, 2, 3), 2}, + {"half gone is a poor copy", dl, got(1, 2), 0}, + {"a recording is not an album", func() Download { + d := dl + d.RecordingMBID = "rec" + + return d + }(), got(1, 2, 3, 4), 0}, + } + + for _, tc := range cases { + if n := len(missingTracks(tc.dl, tc.matched)); n != tc.want { + t.Errorf("%s: %d missing, want %d", tc.name, n, tc.want) + } + } + + big := fiveTrackDownload() + for i := 6; i <= 20; i++ { + big.Expected = append(big.Expected, ExpectedTrack{Position: i, Title: "T" + itoa(i)}) + } + + if n := len(missingTracks(big, big.Expected[:16])); n != 0 { + t.Errorf("four of twenty missing: %d filled in, want none past %d", n, maxFillInTracks) + } +} + +// A fill-in import takes only the file that is the missing track, and +// places it in the album with the album's tags; anything else the grab +// brought is left out. +func TestImportOnlyTakesTheMissingTrack(t *testing.T) { + t.Parallel() + + f := newImportFixture(t, + "05 - Let Down.flac", + "02 - Paranoid Android.flac", + ) + + dl := fiveTrackDownload() + + got, err := f.importer.Import( + context.Background(), dl, + Result{Dir: f.dir, Files: f.files}, + ImportOptions{LibraryRoot: f.root, WriteTags: true, Only: dl.Expected[4:]}, + ) + if err != nil { + t.Fatalf("Import: %v", err) + } + + want := filepath.Join(f.root, "Radiohead", "OK Computer", "05 Let Down.flac") + if len(got.Paths) != 1 || got.Paths[0] != want { + t.Errorf("imported %q, want only %s", got.Paths, want) + } + + // A file that is not the missing track is not imported at all. + g := newImportFixture(t, "02 - Paranoid Android.flac") + + if _, err := g.importer.Import( + context.Background(), dl, + Result{Dir: g.dir, Files: g.files}, + ImportOptions{LibraryRoot: g.root, WriteTags: true, Only: dl.Expected[4:]}, + ); !errors.Is(err, ErrTooIncomplete) { + t.Errorf("Import = %v, want nothing matched", err) + } +} + +// The album comes from one source missing its fifth track; the fifth is +// then found on its own at another and lands in the same album. +func TestManagerFillsInAMissingTrack(t *testing.T) { + t.Parallel() + + f := newManagerFixture(t) + + titles := fiveTrackTitles() + + album := NewFakeProvider(1, "album", Caps{CanSearch: true, CanTransport: true}) + ac := candidateFor("album-cand", titles, ".flac", 30_000_000) + ac.ProviderID = 1 + album.Candidates = []Candidate{ac} + + for i, tt := range titles[:4] { + album.Written[trackToken(i+1)+" - "+tt+".flac"] = []byte("audio-data") + } + + single := NewFakeProvider(2, "single", Caps{CanSearch: true, CanTransport: true}) + sc := Candidate{ + ID: "single-cand", + Protocol: ProtocolDirect, + Title: "Radiohead - OK Computer", + Artist: "Radiohead", + Files: []CandidateFile{{ + Path: "Radiohead - OK Computer/05 - Let Down.flac", + Size: 30_000_000, + }}, + Health: 0.5, + ProviderID: 2, + } + single.Candidates = []Candidate{sc} + single.Written["05 - Let Down.flac"] = []byte("audio-data") + + f.manager.installProvider(Config{ID: 1, Priority: 90}, album) + f.manager.installProvider(Config{ID: 2, Priority: 10}, single) + + dl := fiveTrackDownload() + + if _, err := f.manager.Start(context.Background(), dl); err != nil { + t.Fatalf("Start: %v", err) + } + + waitForDownloadState(t, f.store, dl.ID, StateComplete) + + if album.GrabCallCount() != 1 || single.GrabCallCount() != 1 { + t.Errorf( + "grabs: album=%d single=%d, want 1 and 1", + album.GrabCallCount(), single.GrabCallCount(), + ) + } + + for i, tt := range titles { + p := filepath.Join(f.root, "Radiohead", "OK Computer", trackToken(i+1)+" "+tt+".flac") + if _, err := os.Stat(p); err != nil { + t.Errorf("track %d not in the library: %v", i+1, err) + } + } +} diff --git a/backend/download/importer.go b/backend/download/importer.go index 45020a5..c4c2022 100644 --- a/backend/download/importer.go +++ b/backend/download/importer.go @@ -79,6 +79,12 @@ type ImportOptions struct { // them. Off for delegate providers, which have already imported // and tagged the files themselves. WriteTags bool + + // Only, when set, imports just the files that align to these + // tracks of the download and skips the completeness check: it is a + // fill-in for tracks an earlier grab of the same album did not + // deliver (#276), tagged and placed as part of that album. + Only []ExpectedTrack } // DefaultPathTemplate is the layout used when none is configured. @@ -118,6 +124,10 @@ type ImportResult struct { // Skipped counts non-audio files left in staging (logs, cue sheets, // scene .nfo files) — deliberately not imported. Skipped int + + // Matched are the expected tracks an imported file was aligned to, + // which is how a caller learns what the grab did not deliver. + Matched []ExpectedTrack } // Import verifies, tags and moves a completed grab into the library. @@ -140,14 +150,25 @@ func (i *Importer) Import( return ImportResult{}, ErrNoAudio } - if err := checkCompleteness(len(audio), dl); err != nil { - return ImportResult{}, err + if len(opts.Only) == 0 { + if err := checkCompleteness(len(audio), dl); err != nil { + return ImportResult{}, err + } } // Align staged files to the expected tracklist so tags and // filenames reflect the release, not the uploader's naming. plan := i.planFiles(audio, dl) + if len(opts.Only) > 0 { + plan = onlyTracks(plan, opts.Only) + if len(plan) == 0 { + return ImportResult{}, fmt.Errorf( + "%w: no file matched the missing track", ErrTooIncomplete, + ) + } + } + out := ImportResult{ Paths: make([]string, 0, len(plan)), Skipped: skipped, @@ -184,11 +205,49 @@ func (i *Importer) Import( } out.Paths = append(out.Paths, dest) + + if p.Matched { + out.Matched = append(out.Matched, p.Track) + } } return out, nil } +// trackKey identifies an expected track within a release. +type trackKey struct{ disc, position int } + +func keyOf(t ExpectedTrack) trackKey { + return trackKey{disc: t.DiscNumber, position: t.Position} +} + +// onlyTracks keeps the planned files aligned to one of want. A fill-in +// grab can bring more than the one file it was after — a folder where +// the title also matched a live take — and anything else would land in +// the album as a duplicate or a stranger. +func onlyTracks(plan []plannedFile, want []ExpectedTrack) []plannedFile { + keys := make(map[trackKey]bool, len(want)) + for _, t := range want { + keys[keyOf(t)] = true + } + + out := make([]plannedFile, 0, len(want)) + seen := map[trackKey]bool{} + + for _, p := range plan { + k := keyOf(p.Track) + if !p.Matched || !keys[k] || seen[k] { + continue + } + + seen[k] = true + + out = append(out, p) + } + + return out +} + // plannedFile pairs a staged file with the expected track it matched. type plannedFile struct { Source string diff --git a/backend/download/manager.go b/backend/download/manager.go index 3b429de..d41bb28 100644 --- a/backend/download/manager.go +++ b/backend/download/manager.go @@ -747,8 +747,9 @@ func (m *Manager) grab( var failed []Candidate for { - out := m.attemptGrab(ctx, dl, c, job) + out := m.attemptGrab(ctx, dl, c, job, nil) if out.err == nil { + m.fillIn(ctx, dl, c, out.imported, job) m.finishGrab(ctx, dl, out.item, out.imported, job) return @@ -836,6 +837,7 @@ func (m *Manager) attemptGrab( dl Download, c Candidate, job *jobs.Handle, + only []ExpectedTrack, ) grabOutcome { // Who will move the bytes is decided before any slot is taken, so // the transfer waits in its own provider's queue rather than in a @@ -942,6 +944,7 @@ func (m *Manager) attemptGrab( opts := m.importOptions() opts.WriteTags = true + opts.Only = only opts.LibraryRoot, err = m.library.LibraryPath(dl.LibraryID) if err != nil { diff --git a/backend/download/provider_slskd.go b/backend/download/provider_slskd.go index a0fda64..bb62a5e 100644 --- a/backend/download/provider_slskd.go +++ b/backend/download/provider_slskd.go @@ -573,8 +573,10 @@ func isVariousArtists(artist string) bool { // usually matches one file per folder. The two-file floor that filters // out one-file noise for an album therefore filtered out every result // for a track, and a single-track request could never be served here. +// A request expecting one track — a recording, or the fill-in for one +// missing from an album (#276) — takes a one-file folder. func minFilesFor(dl Download) int { - if dl.RecordingMBID != "" { + if dl.RecordingMBID != "" || len(dl.Expected) == 1 { return 1 }