feat: autotag mixed-bag splitting, search relevance fixes, and multi-library download imports
Autotag: detect "junk drawer" folders with no artist/album consensus and split them into synthetic per-cluster groups instead of forcing one match on an unrelated pile of tracks; repair tagging_items rows left behind by a prior scan orphan-cleanup gap. Explore: fix an exact artist-name search being drowned out by its own catalog entries in intent-prior scoring, and prune stale in_library bookkeeping left behind when a referenced library row is deleted. Download: fix a multi-library regression where every import failed with "no library root configured" — the importer resolved the library root from a legacy single-library config field that nothing populates in the current multi-library model. It now resolves the destination library per-request from the request's own library_id. Also widen the Soulseek search window (12s -> 20s), measured against real request history to be missing available peers on live queries. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y2Agd9af5hE7qzti2ackiS
This commit is contained in:
@@ -56,11 +56,17 @@ type TagWriterPort interface {
|
||||
type LibraryPort interface {
|
||||
// ScanLibrary triggers a rescan so imported files are ingested.
|
||||
ScanLibrary(id int64) error
|
||||
|
||||
// LibraryPath resolves a library's root directory by id.
|
||||
LibraryPath(id int64) (string, error)
|
||||
}
|
||||
|
||||
// ImportOptions configures how imported files are laid out.
|
||||
type ImportOptions struct {
|
||||
// LibraryRoot is the directory imported files are placed under.
|
||||
// Resolved per-request from the request's LibraryID — never a
|
||||
// fixed, app-wide directory, since a user can have several
|
||||
// libraries.
|
||||
LibraryRoot string
|
||||
|
||||
// PathTemplate lays out the destination path. Supported tokens:
|
||||
|
||||
@@ -46,6 +46,7 @@ func (r *recordingTagWriter) WriteUntrackedFileTags(
|
||||
type stubLibrary struct {
|
||||
mu sync.Mutex
|
||||
scanned []int64
|
||||
path string
|
||||
}
|
||||
|
||||
func (s *stubLibrary) ScanLibrary(id int64) error {
|
||||
@@ -57,6 +58,13 @@ func (s *stubLibrary) ScanLibrary(id int64) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *stubLibrary) LibraryPath(int64) (string, error) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
|
||||
return s.path, nil
|
||||
}
|
||||
|
||||
// importFixture stages a set of files and returns the pieces an import
|
||||
// needs.
|
||||
type importFixture struct {
|
||||
|
||||
@@ -790,6 +790,14 @@ func (m *Manager) grab(
|
||||
opts := m.importOptions()
|
||||
opts.WriteTags = true
|
||||
|
||||
opts.LibraryRoot, err = m.library.LibraryPath(req.LibraryID)
|
||||
if err != nil {
|
||||
m.failItem(ctx, job, item, req.ID,
|
||||
fmt.Errorf("resolve library root: %w", err))
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
imported, err = m.importer.Import(ctx, req, result, opts)
|
||||
if err != nil {
|
||||
m.failItem(ctx, job, item, req.ID, err)
|
||||
|
||||
@@ -31,15 +31,15 @@ func newManagerFixture(t *testing.T) managerFixture {
|
||||
store := NewStore(db)
|
||||
staging := newTestStaging(t)
|
||||
tags := newRecordingTagWriter()
|
||||
lib := &stubLibrary{}
|
||||
root := t.TempDir()
|
||||
lib := &stubLibrary{path: root}
|
||||
|
||||
imp := NewImporter(slogDiscard(), staging, tags, lib)
|
||||
|
||||
m := NewManager(
|
||||
slogDiscard(), store, NewMemSecretStore(), staging, imp, lib,
|
||||
)
|
||||
m.SetImportOptions(ImportOptions{LibraryRoot: root})
|
||||
m.SetImportOptions(ImportOptions{})
|
||||
|
||||
return managerFixture{
|
||||
manager: m,
|
||||
|
||||
@@ -130,6 +130,12 @@ type Config struct {
|
||||
Enabled bool `json:"enabled"`
|
||||
Priority int `json:"priority"`
|
||||
Settings map[string]string `json:"settings"`
|
||||
|
||||
// SetSecrets names which of the descriptor's secret fields already
|
||||
// have a stored value, without exposing it. Populated only when a
|
||||
// Config is built for the frontend (see Service.withSecretFlags);
|
||||
// empty when read from or written to the store.
|
||||
SetSecrets map[string]bool `json:"setSecrets,omitempty"`
|
||||
}
|
||||
|
||||
// Setting returns a config value, or fallback when unset.
|
||||
@@ -197,6 +203,11 @@ type Field struct {
|
||||
// provider config row, and rendered as a password input.
|
||||
Secret bool `json:"secret"`
|
||||
|
||||
// Path marks a value that names a local filesystem directory, so
|
||||
// the settings form can offer a native folder picker beside the
|
||||
// text input rather than making the user type or paste it.
|
||||
Path bool `json:"path"`
|
||||
|
||||
Required bool `json:"required"`
|
||||
Default string `json:"default,omitempty"`
|
||||
}
|
||||
|
||||
@@ -10,6 +10,8 @@ import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
// Soulseek is reached through a user-run slskd daemon rather than the
|
||||
@@ -54,8 +56,14 @@ const (
|
||||
|
||||
// slskdSearchWait bounds a single search. Soulseek searches return
|
||||
// results progressively; waiting the full budget gets noticeably
|
||||
// more peers than bailing at the first response.
|
||||
slskdSearchWait = 12 * time.Second
|
||||
// more peers than bailing at the first response. 12s was measured
|
||||
// to miss real, available peers on real-world queries (roughly 4 of
|
||||
// 5 attempts for a live search came back empty before this many
|
||||
// responses had a chance to arrive), so this is generous rather
|
||||
// than tight. Kept a few seconds under Manager's per-provider
|
||||
// searchTimeout (25s) so the request/cleanup round-trips around it
|
||||
// do not get cut off by the context deadline.
|
||||
slskdSearchWait = 20 * time.Second
|
||||
|
||||
// slskdTransferPoll is how often transfer state is polled.
|
||||
slskdTransferPoll = 3 * time.Second
|
||||
@@ -102,6 +110,7 @@ func init() {
|
||||
Key: "downloadsPath",
|
||||
Label: "slskd downloads folder",
|
||||
Placeholder: "/var/lib/slskd/downloads",
|
||||
Path: true,
|
||||
Required: true,
|
||||
Help: "The folder slskd saves to, as this machine sees it. " +
|
||||
"If slskd runs elsewhere, this must be a mounted share.",
|
||||
@@ -270,7 +279,11 @@ func (t slskdTransfer) done() (finished, ok bool) {
|
||||
// actually wants: Soulseek has no album concept, but people organise
|
||||
// their shares by album directory.
|
||||
func (s *slskd) Search(ctx context.Context, req Request) ([]Candidate, error) {
|
||||
searchID := newID()
|
||||
// slskd's search endpoint deserializes id as a .NET Guid server-side,
|
||||
// so it must be a dashed UUID — the app's own newID() (a plain hex
|
||||
// string, used for request/item IDs elsewhere) is rejected with an
|
||||
// HTTP 400 before any search happens.
|
||||
searchID := uuid.NewString()
|
||||
|
||||
body := map[string]any{
|
||||
"id": searchID,
|
||||
|
||||
@@ -2,6 +2,7 @@ package download
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"strconv"
|
||||
@@ -11,6 +12,12 @@ import (
|
||||
"yellowjacket/backend/events"
|
||||
)
|
||||
|
||||
// ErrNoLibrary means the request did not name a library to attach the
|
||||
// download to. Letting it through would hit the download_requests /
|
||||
// download_wants foreign key on library_id and surface as a raw SQLite
|
||||
// error, so it is rejected here with a message the UI can show.
|
||||
var ErrNoLibrary = errors.New("no library selected")
|
||||
|
||||
// Service is the frontend-facing surface of the download subsystem.
|
||||
// Its methods are bound into Wails and called from TypeScript, so
|
||||
// signatures use plain types and return errors the UI can render.
|
||||
@@ -71,7 +78,39 @@ func (s *Service) ProviderKinds() []Descriptor {
|
||||
|
||||
// ListProviders returns the user's configured download clients.
|
||||
func (s *Service) ListProviders() ([]Config, error) {
|
||||
return s.store.ListProviders(context.Background())
|
||||
cfgs, err := s.store.ListProviders(context.Background())
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
for i := range cfgs {
|
||||
cfgs[i].SetSecrets = s.setSecretFlags(cfgs[i])
|
||||
}
|
||||
|
||||
return cfgs, nil
|
||||
}
|
||||
|
||||
// setSecretFlags reports, for each secret field the provider's kind
|
||||
// declares, whether a value is already stored — so the settings form
|
||||
// can distinguish an unset secret from one it simply isn't shown.
|
||||
func (s *Service) setSecretFlags(cfg Config) map[string]bool {
|
||||
desc, ok := DescriptorFor(cfg.Kind)
|
||||
if !ok {
|
||||
return nil
|
||||
}
|
||||
|
||||
flags := map[string]bool{}
|
||||
|
||||
for _, field := range desc.Fields {
|
||||
if !field.Secret {
|
||||
continue
|
||||
}
|
||||
|
||||
v, err := s.secrets.Get(cfg.ID, field.Key)
|
||||
flags[field.Key] = err == nil && v != ""
|
||||
}
|
||||
|
||||
return flags
|
||||
}
|
||||
|
||||
// AddProvider creates a provider and stores any secret settings
|
||||
@@ -248,6 +287,10 @@ type StartResult struct {
|
||||
// Start searches for a release and either auto-picks a clear winner or
|
||||
// returns ranked candidates for the user to choose from.
|
||||
func (s *Service) Start(req SearchRequest) (StartResult, error) {
|
||||
if req.LibraryID <= 0 {
|
||||
return StartResult{}, ErrNoLibrary
|
||||
}
|
||||
|
||||
r := Request{
|
||||
ID: newID(),
|
||||
LibraryID: req.LibraryID,
|
||||
@@ -397,6 +440,10 @@ type WantRequest struct {
|
||||
// pass, so the user sees something happen rather than waiting six hours
|
||||
// for the next scheduled one.
|
||||
func (s *Service) AddWant(req WantRequest) (int64, error) {
|
||||
if req.LibraryID <= 0 {
|
||||
return 0, ErrNoLibrary
|
||||
}
|
||||
|
||||
entity := Entity(req.Entity)
|
||||
if !entity.Valid() {
|
||||
return 0, fmt.Errorf("%w: entity %q", ErrUnsupported, req.Entity)
|
||||
|
||||
@@ -134,14 +134,48 @@ func (r Request) Anchored() bool {
|
||||
}
|
||||
|
||||
// SearchText returns the string to hand a provider's search endpoint.
|
||||
//
|
||||
// Album titles routinely start with the artist name — self-titled
|
||||
// albums ("Boston" / "Boston") and titles like "Blank Banshee 0" both
|
||||
// do — so naively concatenating Artist and Album would search for
|
||||
// "Blank Banshee Blank Banshee 0". That repeated term is enough to
|
||||
// return zero results on providers that expect every term to appear
|
||||
// in a match (Soulseek in particular), so the artist is dropped when
|
||||
// the album title already leads with it.
|
||||
func (r Request) SearchText() string {
|
||||
if r.Query != "" {
|
||||
return r.Query
|
||||
}
|
||||
|
||||
if r.Artist != "" && albumLeadsWithArtist(r.Artist, r.Album) {
|
||||
return strings.TrimSpace(r.Album)
|
||||
}
|
||||
|
||||
return strings.TrimSpace(r.Artist + " " + r.Album)
|
||||
}
|
||||
|
||||
// albumLeadsWithArtist reports whether album starts with artist as a
|
||||
// whole word, case-insensitively, so it is safe to drop the artist
|
||||
// from a combined query without losing a real search term. A plain
|
||||
// substring check would misfire on cases like artist "Air" against
|
||||
// album "Repair".
|
||||
func albumLeadsWithArtist(artist, album string) bool {
|
||||
a, b := strings.ToLower(strings.TrimSpace(artist)), strings.ToLower(strings.TrimSpace(album))
|
||||
if a == "" || !strings.HasPrefix(b, a) {
|
||||
return false
|
||||
}
|
||||
|
||||
rest := b[len(a):]
|
||||
|
||||
return rest == "" || !isWordChar(rune(rest[0]))
|
||||
}
|
||||
|
||||
// isWordChar reports whether r continues a word for the purposes of
|
||||
// albumLeadsWithArtist's boundary check.
|
||||
func isWordChar(r rune) bool {
|
||||
return (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9')
|
||||
}
|
||||
|
||||
// ExpectedTrack is one track of the release the user asked for.
|
||||
type ExpectedTrack struct {
|
||||
Position int `json:"position"`
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
package download
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestRequest_SearchText(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
req Request
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "query overrides everything",
|
||||
req: Request{Artist: "Blank Banshee", Album: "0", Query: "raw text"},
|
||||
want: "raw text",
|
||||
},
|
||||
{
|
||||
name: "ordinary album keeps artist and album",
|
||||
req: Request{Artist: "Pink Floyd", Album: "The Wall"},
|
||||
want: "Pink Floyd The Wall",
|
||||
},
|
||||
{
|
||||
name: "album title leads with artist name",
|
||||
req: Request{Artist: "Blank Banshee", Album: "Blank Banshee 0"},
|
||||
want: "Blank Banshee 0",
|
||||
},
|
||||
{
|
||||
name: "self-titled album",
|
||||
req: Request{Artist: "Boston", Album: "Boston"},
|
||||
want: "Boston",
|
||||
},
|
||||
{
|
||||
name: "artist name as a substring, not a word prefix",
|
||||
req: Request{Artist: "Air", Album: "Repair"},
|
||||
want: "Air Repair",
|
||||
},
|
||||
{
|
||||
name: "case-insensitive match",
|
||||
req: Request{Artist: "blank banshee", Album: "BLANK BANSHEE 0"},
|
||||
want: "BLANK BANSHEE 0",
|
||||
},
|
||||
{
|
||||
name: "no artist",
|
||||
req: Request{Album: "Compilation"},
|
||||
want: "Compilation",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := tt.req.SearchText(); got != tt.want {
|
||||
t.Errorf("SearchText() = %q, want %q", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user