From 01b7c86f90cd22ba518abbc9b4bba081ffd1156f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Thu, 2 Jul 2026 09:46:57 -0400 Subject: [PATCH 1/8] fix(scanner): stop logging expected lyrics sniff misses as warnings (#5702) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(scanner): stop logging expected lyrics sniff misses as warnings During a scan, embedded lyrics are parsed with an empty suffix, which puts ParseLyrics into content-sniffing mode: it tries the TTML, SRT and Lyricsfile YAML parsers in turn before falling back to plain text. Every plain-text or LRC lyric therefore fails the structured probes on its way to the fallback, and each failure was logged at warning level with no indication of which file triggered it, flooding the scan log with benign "Error parsing lyrics, falling back to plain text" messages. A probe rejecting content it does not own during sniffing is expected control flow, so it is now logged at trace instead. A parse failure under an explicitly requested suffix (e.g. a malformed .yaml/.srt/.ttml sidecar) still warns, since the user declared that format. ParseLyrics gains ctx and path parameters so any warning names the offending file and carries request context where available; all call sites are updated accordingly. Also fixes a test-isolation bug in the new logging spec: the BeforeEach swapped the process-global default logger via SetDefaultLogger but only restored the log level on cleanup, leaking the null logger and its hook into later specs in the shared model suite. * test: use spec-scoped contexts instead of context.Background in lyrics tests Replace context.Background() with GinkgoT().Context() (and b.Context() in the parse benchmarks) across the lyrics-related tests, so contexts are cancelled when each spec ends. The embeddedLyrics fixture in core/lyrics is now a hand-written literal like its sibling fixtures, removing the construction-time ParseLyrics call that could not use a spec-scoped context. * refactor(model): attach lyrics parse log attribution via context Narrow ParseLyrics back to (ctx, suffix, lang, contents), dropping the path parameter added by the previous commit. Attribution now uses the codebase's existing idiom: callers that know the source attach it with log.NewContext (e.g. "file" for the media file or sidecar), and the plugin adapter tags both the plugin name and the track, fixing probe-miss logs that misattributed plugin-returned content to the file's own tags. This removes three adjacent string parameters that were easy to swap silently, and the "" placeholder most call sites had to pass. Also hardens the logging spec from the previous commit: the null test logger is now swapped in before raising the level (SetLevel forces the current default logger to trace, so the old order left the null logger at info and trace entries never reached the hook), the sniff test now asserts probe misses are observable at trace with file attribution instead of only asserting the absence of warnings, and cleanup restores the actual previous logger — via a new return value on log.SetDefaultLogger — instead of a bare logrus.New() that would discard hooks configured on the process-wide logger. * refactor(lyrics): hoist attributed log contexts out of loops Address review feedback on #5702: build the log-attributed context once per operation instead of per iteration, and reuse it on the surrounding log calls so the error/trace lines around ParseLyrics carry the same attribution fields. In fromExternalFile the sidecar path now rides the context for all log lines in the function, replacing the repeated explicit "path" field. * style(model): pass lyrics parse errors as final log arguments Per the project logging convention, errors go as the last argument (the log package normalizes them via its error case) instead of a keyed "error" pair, which stores the raw error value and bypasses that handling. Flagged by review on #5702; the keyed form was inherited from the original warning line. --- core/lyrics/lyrics_test.go | 15 ++-- core/lyrics/sources.go | 11 +-- core/lyrics/sources_test.go | 10 ++- .../20231209211223_alter_lyric_column.go | 2 +- log/log.go | 6 +- model/lyrics_benchmark_test.go | 2 +- model/lyrics_parse.go | 19 ++++- model/lyrics_parse_test.go | 78 ++++++++++++++++--- model/metadata/map_mediafile.go | 6 +- plugins/lyrics_adapter.go | 8 +- scanner/metadata_old/metadata.go | 5 +- server/subsonic/lyrics_test.go | 6 +- server/subsonic/media_retrieval_test.go | 6 +- 13 files changed, 131 insertions(+), 43 deletions(-) diff --git a/core/lyrics/lyrics_test.go b/core/lyrics/lyrics_test.go index 6baacbe71..b00bcd576 100644 --- a/core/lyrics/lyrics_test.go +++ b/core/lyrics/lyrics_test.go @@ -21,10 +21,15 @@ var _ = Describe("Lyrics", func() { var mf model.MediaFile var ctx context.Context - const badLyrics = "This is a set of lyrics\nThat is not good" - unsyncedList, _ := model.ParseLyrics(".lrc", "xxx", []byte(badLyrics)) - unsynced, _ := unsyncedList.Main() - embeddedLyrics := model.LyricList{unsynced} + embeddedLyrics := model.LyricList{ + model.Lyrics{ + Lang: "xxx", + Line: []model.Line{ + {Value: "This is a set of lyrics"}, + {Value: "That is not good"}, + }, + }, + } syncedLyrics := model.LyricList{ model.Lyrics{ @@ -390,7 +395,7 @@ var _ = Describe("Lyrics", func() { }) It("resolves lyrics from the matched media files", func() { - embeddedList, err := model.ParseLyrics(".lrc", "eng", []byte("Embedded lyrics line")) + embeddedList, err := model.ParseLyrics(ctx, ".lrc", "eng", []byte("Embedded lyrics line")) Expect(err).ToNot(HaveOccurred()) embedded, _ := embeddedList.Main() embeddedJSON, err := json.Marshal(model.LyricList{embedded}) diff --git a/core/lyrics/sources.go b/core/lyrics/sources.go index 9de2f6a18..23c20122d 100644 --- a/core/lyrics/sources.go +++ b/core/lyrics/sources.go @@ -28,6 +28,7 @@ func fromEmbedded(ctx context.Context, mf *model.MediaFile) (model.LyricList, er func fromExternalFile(ctx context.Context, mf *model.MediaFile, suffix string) (model.LyricList, error) { ext := path.Ext(mf.Path) sidecarRelPath := mf.Path[0:len(mf.Path)-len(ext)] + suffix + ctx = log.NewContext(ctx, "file", sidecarRelPath) store, err := storage.For(mf.LibraryPath) if err != nil { @@ -40,7 +41,7 @@ func fromExternalFile(ctx context.Context, mf *model.MediaFile, suffix string) ( f, err := fsys.Open(sidecarRelPath) if errors.Is(err, fs.ErrNotExist) { - log.Trace(ctx, "no lyrics found at path", "path", sidecarRelPath) + log.Trace(ctx, "no lyrics found at path") return nil, nil } else if err != nil { return nil, err @@ -52,18 +53,18 @@ func fromExternalFile(ctx context.Context, mf *model.MediaFile, suffix string) ( return nil, err } - list, err := model.ParseLyrics(suffix, "xxx", contents) + list, err := model.ParseLyrics(ctx, suffix, "xxx", contents) if err != nil { - log.Error(ctx, "error parsing external lyric file", "path", sidecarRelPath, err) + log.Error(ctx, "error parsing external lyric file", err) return nil, err } if len(list) == 0 { - log.Trace(ctx, "empty lyrics from external file", "path", sidecarRelPath) + log.Trace(ctx, "empty lyrics from external file") return nil, nil } - log.Trace(ctx, "retrieved lyrics from external file", "path", sidecarRelPath) + log.Trace(ctx, "retrieved lyrics from external file") return list, nil } diff --git a/core/lyrics/sources_test.go b/core/lyrics/sources_test.go index 68f45424e..7c7922bfd 100644 --- a/core/lyrics/sources_test.go +++ b/core/lyrics/sources_test.go @@ -11,7 +11,11 @@ import ( ) var _ = Describe("sources", func() { - ctx := context.Background() + var ctx context.Context + + BeforeEach(func() { + ctx = GinkgoT().Context() + }) Describe("fromEmbedded", func() { It("should return nothing for a media file with no lyrics", func() { @@ -26,8 +30,8 @@ var _ = Describe("sources", func() { const syncedLyrics = "[00:18.80]We're no strangers to love\n[00:22.801]You know the rules and so do I" const unsyncedLyrics = "We're no strangers to love\nYou know the rules and so do I" - syncedList, _ := model.ParseLyrics(".lrc", "eng", []byte(syncedLyrics)) - unsyncedList, _ := model.ParseLyrics(".lrc", "xxx", []byte(unsyncedLyrics)) + syncedList, _ := model.ParseLyrics(ctx, ".lrc", "eng", []byte(syncedLyrics)) + unsyncedList, _ := model.ParseLyrics(ctx, ".lrc", "xxx", []byte(unsyncedLyrics)) synced, _ := syncedList.Main() unsynced, _ := unsyncedList.Main() diff --git a/db/migrations/20231209211223_alter_lyric_column.go b/db/migrations/20231209211223_alter_lyric_column.go index 259a37745..7f1ad2f38 100644 --- a/db/migrations/20231209211223_alter_lyric_column.go +++ b/db/migrations/20231209211223_alter_lyric_column.go @@ -46,7 +46,7 @@ func upAlterLyricColumn(ctx context.Context, tx *sql.Tx) error { continue } - parsed, err := model.ParseLyrics(".lrc", "xxx", []byte(lyrics.String)) + parsed, err := model.ParseLyrics(ctx, ".lrc", "xxx", []byte(lyrics.String)) if err != nil { return err } diff --git a/log/log.go b/log/log.go index b1d6eee10..eaea75fb9 100644 --- a/log/log.go +++ b/log/log.go @@ -175,10 +175,14 @@ func NewContext(ctx context.Context, keyValuePairs ...any) context.Context { return ctx } -func SetDefaultLogger(l *logrus.Logger) { +// SetDefaultLogger swaps the process-wide logger and returns the previous one, +// so tests can restore the original (with its hooks and formatter) on cleanup. +func SetDefaultLogger(l *logrus.Logger) *logrus.Logger { loggerMu.Lock() defer loggerMu.Unlock() + prev := defaultLogger defaultLogger = l + return prev } func CurrentLevel() Level { diff --git a/model/lyrics_benchmark_test.go b/model/lyrics_benchmark_test.go index 0de2549e7..5a7d7871e 100644 --- a/model/lyrics_benchmark_test.go +++ b/model/lyrics_benchmark_test.go @@ -24,7 +24,7 @@ func benchmarkParse(b *testing.B, suffix, fixture string) { b.ReportAllocs() b.SetBytes(int64(len(contents))) for b.Loop() { - if _, err := ParseLyrics(suffix, "eng", contents); err != nil { + if _, err := ParseLyrics(b.Context(), suffix, "eng", contents); err != nil { b.Fatal(err) } } diff --git a/model/lyrics_parse.go b/model/lyrics_parse.go index 4bfaa29e8..8aa095c50 100644 --- a/model/lyrics_parse.go +++ b/model/lyrics_parse.go @@ -2,6 +2,7 @@ package model import ( "bytes" + "context" "fmt" "slices" "strings" @@ -28,7 +29,10 @@ var lyricFormats = []struct { // ParseLyrics is the single entry point for parsing lyrics. A known suffix routes // to that format's parser; an empty or "auto" suffix content-sniffs. Either way, // a structured parser that does not match falls back to the LRC/plain-text floor. -func ParseLyrics(suffix, lang string, contents []byte) (LyricList, error) { +// +// Parse failures are logged through ctx; callers that know the source should +// attach it for attribution, e.g. log.NewContext(ctx, "file", path). +func ParseLyrics(ctx context.Context, suffix, lang string, contents []byte) (LyricList, error) { contents = stripBOM(contents) suffix = strings.ToLower(suffix) sniff := suffix == "" || suffix == "auto" @@ -41,17 +45,24 @@ func ParseLyrics(suffix, lang string, contents []byte) (LyricList, error) { candidates = append(candidates, f.parse) } } - return parseFirstMatch(lang, contents, candidates...) + return parseFirstMatch(ctx, sniff, lang, contents, candidates...) } -func parseFirstMatch(lang string, contents []byte, candidates ...lyricParser) (LyricList, error) { +func parseFirstMatch(ctx context.Context, sniff bool, lang string, contents []byte, candidates ...lyricParser) (LyricList, error) { for _, parse := range candidates { list, err := parse(lang, contents) if err == nil && len(list) > 0 { return list, nil } if err != nil { - log.Warn("Error parsing lyrics, falling back to plain text", "error", err) + // While sniffing, a probe rejecting content it does not own is expected + // control flow, so keep it at trace. A failure under an explicit suffix + // means the declared format is malformed and deserves a warning. + if sniff { + log.Trace(ctx, "Lyrics probe did not match, trying next format", err) + } else { + log.Warn(ctx, "Error parsing lyrics, falling back to plain text", err) + } } } return plainLRC(lang, contents) diff --git a/model/lyrics_parse_test.go b/model/lyrics_parse_test.go index eb58a29ef..0b47e55e9 100644 --- a/model/lyrics_parse_test.go +++ b/model/lyrics_parse_test.go @@ -3,14 +3,17 @@ package model import ( "strings" + "github.com/navidrome/navidrome/log" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/sirupsen/logrus" + "github.com/sirupsen/logrus/hooks/test" ) var _ = Describe("ParseLyrics", func() { DescribeTable("known suffix routes to the matching parser", func(suffix, contents string, wantSynced bool, wantFirst string) { - list, err := ParseLyrics(suffix, "eng", []byte(contents)) + list, err := ParseLyrics(GinkgoT().Context(), suffix, "eng", []byte(contents)) Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(1)) Expect(list[0].Synced).To(Equal(wantSynced)) @@ -25,7 +28,7 @@ var _ = Describe("ParseLyrics", func() { It("empty suffix content-sniffs (TTML)", func() { ttml := `

auto ttml

` - list, err := ParseLyrics("", "eng", []byte(ttml)) + list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(ttml)) Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(1)) Expect(list[0].Line[0].Value).To(Equal("auto ttml")) @@ -33,19 +36,72 @@ var _ = Describe("ParseLyrics", func() { It("empty suffix content-sniffs (YAML)", func() { yaml := "version: \"1.0\"\nmetadata:\n language: eng\nlines:\n - text: auto yaml\n start_ms: 1000\n" - list, err := ParseLyrics("auto", "eng", []byte(yaml)) + list, err := ParseLyrics(GinkgoT().Context(), "auto", "eng", []byte(yaml)) Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(1)) Expect(list[0].Line[0].Value).To(Equal("auto yaml")) }) It("falls back to plain text when a known suffix fails to parse structurally", func() { - list, err := ParseLyrics(".srt", "eng", []byte("not actually an srt file")) + list, err := ParseLyrics(GinkgoT().Context(), ".srt", "eng", []byte("not actually an srt file")) Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(1)) Expect(list[0].Synced).To(BeFalse()) Expect(list[0].Line[0].Value).To(Equal("not actually an srt file")) }) + + Describe("logging on parser probe failures", func() { + var hook *test.Hook + + BeforeEach(func() { + prevLevel := log.CurrentLevel() + l, h := test.NewNullLogger() + hook = h + // Swap the logger before raising the level: SetLevel also forces the + // current default logger to logrus.TraceLevel, and the null logger would + // otherwise stay at Info and drop Trace entries before the hook sees them. + prevLogger := log.SetDefaultLogger(l) + log.SetLevel(log.LevelTrace) + DeferCleanup(func() { + log.SetDefaultLogger(prevLogger) + log.SetLevel(prevLevel) + }) + }) + + // This is the source of the full-scan log spam: embedded lyrics are parsed + // with an empty suffix (sniff mode), so every plain-text lyric fails the + // YAML/SRT/TTML probes on its way to the plain-text fallback. A probe miss + // during sniffing is expected control flow, not a warning. + It("logs sniff probe misses at trace only, with file attribution", func() { + ctx := log.NewContext(GinkgoT().Context(), "file", "/music/song.mp3") + list, err := ParseLyrics(ctx, "", "eng", []byte("Just a plain\nlyric line\n")) + + Expect(err).ToNot(HaveOccurred()) + Expect(list).To(HaveLen(1)) + Expect(list[0].Line[0].Value).To(Equal("Just a plain")) + entries := hook.AllEntries() + Expect(entries).ToNot(BeEmpty(), "probe misses should be observable at trace") + for _, e := range entries { + Expect(e.Level).To(Equal(logrus.TraceLevel), + "sniff-mode probe misses must not be logged above Trace") + Expect(e.Data).To(HaveKeyWithValue("file", "/music/song.mp3")) + } + }) + + // A specific suffix means the user declared the format, so a structural + // failure is worth surfacing loudly — and it must name the file. + It("warns and names the file when a requested suffix fails to parse", func() { + ctx := log.NewContext(GinkgoT().Context(), "file", "/music/song.yaml") + list, err := ParseLyrics(ctx, ".yaml", "eng", []byte("not: [valid, yaml\n")) + + Expect(err).ToNot(HaveOccurred()) + Expect(list).To(HaveLen(1)) // still falls back to plain text + entry := hook.LastEntry() + Expect(entry).ToNot(BeNil()) + Expect(entry.Level).To(Equal(logrus.WarnLevel)) + Expect(entry.Data).To(HaveKeyWithValue("file", "/music/song.yaml")) + }) + }) }) var _ = Describe("ParseLyrics content-sniffing", func() { @@ -67,7 +123,7 @@ var _ = Describe("ParseLyrics content-sniffing", func() { ` - list, err := ParseLyrics("", "ENG", []byte(content)) + list, err := ParseLyrics(GinkgoT().Context(), "", "ENG", []byte(content)) // ParseLyrics's job is to detect TTML and apply the tag language as the // default; the parser's cue/agent details are covered in lyrics_ttml_test.go. @@ -104,7 +160,7 @@ var _ = Describe("ParseLyrics content-sniffing", func() { ` - list, err := ParseLyrics("", "eng", []byte(content)) + list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(content)) Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(3)) @@ -129,7 +185,7 @@ We're from subtitles 00:00:22,801 --> 00:00:26,000 Another subtitle line` - list, err := ParseLyrics("", "POR", []byte(content)) + list, err := ParseLyrics(GinkgoT().Context(), "", "POR", []byte(content)) Expect(err).ToNot(HaveOccurred()) Expect(list).To(Equal(LyricList{ @@ -155,7 +211,7 @@ Another subtitle line` It("should parse embedded SRT blocks separated by whitespace-only blank lines", func() { content := "1\n00:00:01,000 --> 00:00:02,000\nFirst subtitle\n \n2\n00:00:03,000 --> 00:00:04,000\nSecond subtitle" - list, err := ParseLyrics("", "eng", []byte(content)) + list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(content)) Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(1)) @@ -168,7 +224,7 @@ Another subtitle line` It("should keep embedded enhanced LRC cues", func() { content := "[00:01.00]<00:01.00>Lead <00:01.50>words" - list, err := ParseLyrics("", "eng", []byte(content)) + list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(content)) Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(1)) @@ -185,7 +241,7 @@ Another subtitle line` ` - list, err := ParseLyrics("", "eng", []byte(content)) + list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(content)) Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(1)) @@ -202,7 +258,7 @@ Another subtitle line` It("detects a Lyricsfile YAML payload via content-sniffing", func() { yaml := "version: \"1.0\"\nmetadata:\n title: Song\n language: eng\nlines:\n - text: sniffed yaml line\n start_ms: 1000\n" - list, err := ParseLyrics("", "eng", []byte(yaml)) + list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(yaml)) Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(1)) diff --git a/model/metadata/map_mediafile.go b/model/metadata/map_mediafile.go index de2ba813e..b3ce4ef02 100644 --- a/model/metadata/map_mediafile.go +++ b/model/metadata/map_mediafile.go @@ -2,6 +2,7 @@ package metadata import ( "cmp" + "context" "encoding/json" "maps" "math" @@ -139,13 +140,14 @@ func (md Metadata) mapLyrics() string { lyricList := make(model.LyricList, 0, len(rawLyrics)) + ctx := log.NewContext(context.Background(), "file", md.filePath) for _, raw := range rawLyrics { lang := raw.Key() text := raw.Value() - lyrics, err := model.ParseLyrics("", lang, []byte(text)) + lyrics, err := model.ParseLyrics(ctx, "", lang, []byte(text)) if err != nil { - log.Warn("Unexpected failure occurred when parsing lyrics", "file", md.filePath, err) + log.Warn(ctx, "Unexpected failure occurred when parsing lyrics", err) continue } for _, lyric := range lyrics { diff --git a/plugins/lyrics_adapter.go b/plugins/lyrics_adapter.go index 12d84f60d..281f022fb 100644 --- a/plugins/lyrics_adapter.go +++ b/plugins/lyrics_adapter.go @@ -44,15 +44,19 @@ func (l *LyricsPlugin) GetLyrics(ctx context.Context, mf *model.MediaFile) (mode return nil, err } + // The lyric text comes from the plugin, not the media file's own tags, so + // attribute logs to both the plugin and the track it was fetched for. + ctx = log.NewContext(ctx, "plugin", l.name, "file", mf.Path) + var result model.LyricList for _, lt := range resp.Lyrics { lang := lt.Lang if lang == "" { lang = "xxx" } - parsed, err := model.ParseLyrics("", lang, []byte(lt.Text)) + parsed, err := model.ParseLyrics(ctx, "", lang, []byte(lt.Text)) if err != nil { - log.Warn(ctx, "Error parsing plugin lyrics", "plugin", l.name, err) + log.Warn(ctx, "Error parsing plugin lyrics", err) continue } for _, lyric := range parsed { diff --git a/scanner/metadata_old/metadata.go b/scanner/metadata_old/metadata.go index 8cde586b9..2906a2c09 100644 --- a/scanner/metadata_old/metadata.go +++ b/scanner/metadata_old/metadata.go @@ -1,6 +1,7 @@ package metadata_old import ( + "context" "encoding/json" "fmt" "math" @@ -205,7 +206,7 @@ func (t Tags) Lyrics() string { basicLyrics := t.getAllTagValues("lyrics", "unsynced_lyrics", "unsynced lyrics", "unsyncedlyrics") for _, value := range basicLyrics { - parsed, err := model.ParseLyrics(".lrc", "xxx", []byte(value)) + parsed, err := model.ParseLyrics(context.Background(), ".lrc", "xxx", []byte(value)) if err != nil { log.Warn("Unexpected failure occurred when parsing lyrics", "file", t.filePath, "error", err) continue @@ -224,7 +225,7 @@ func (t Tags) Lyrics() string { } for _, text := range value { - parsed, err := model.ParseLyrics(".lrc", language, []byte(text)) + parsed, err := model.ParseLyrics(context.Background(), ".lrc", language, []byte(text)) if err != nil { log.Warn("Unexpected failure occurred when parsing lyrics", "file", t.filePath, "error", err) continue diff --git a/server/subsonic/lyrics_test.go b/server/subsonic/lyrics_test.go index 3d9881872..8713b7a3b 100644 --- a/server/subsonic/lyrics_test.go +++ b/server/subsonic/lyrics_test.go @@ -100,8 +100,8 @@ var _ = Describe("GetLyricsBySongId", func() { It("should return mixed lyrics", func() { r := newGetRequest("id=1") - syncedList, _ := model.ParseLyrics(".lrc", "eng", []byte(syncedLyrics)) - unsyncedList, _ := model.ParseLyrics(".lrc", "xxx", []byte(unsyncedLyrics)) + syncedList, _ := model.ParseLyrics(GinkgoT().Context(), ".lrc", "eng", []byte(syncedLyrics)) + unsyncedList, _ := model.ParseLyrics(GinkgoT().Context(), ".lrc", "xxx", []byte(unsyncedLyrics)) synced, _ := syncedList.Main() unsynced, _ := unsyncedList.Main() lyricsJson, err := json.Marshal(model.LyricList{ @@ -158,7 +158,7 @@ var _ = Describe("GetLyricsBySongId", func() { It("should parse lrc metadata", func() { r := newGetRequest("id=1") - syncedList, _ := model.ParseLyrics(".lrc", "eng", []byte(metadata+"\n"+syncedLyrics)) + syncedList, _ := model.ParseLyrics(GinkgoT().Context(), ".lrc", "eng", []byte(metadata+"\n"+syncedLyrics)) synced, _ := syncedList.Main() lyricsJson, err := json.Marshal(model.LyricList{ synced, diff --git a/server/subsonic/media_retrieval_test.go b/server/subsonic/media_retrieval_test.go index 228427d5a..9331dfbe4 100644 --- a/server/subsonic/media_retrieval_test.go +++ b/server/subsonic/media_retrieval_test.go @@ -78,7 +78,7 @@ var _ = Describe("MediaRetrievalController", func() { When("client disconnects (context is cancelled)", func() { It("should not call the service if cancelled before the call", func() { - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(GinkgoT().Context()) r := newGetRequest("id=34", "size=128", "square=true") r = r.WithContext(ctx) cancel() @@ -93,7 +93,7 @@ var _ = Describe("MediaRetrievalController", func() { }) It("should not return data if cancelled during the call", func() { - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(GinkgoT().Context()) defer cancel() r := newGetRequest("id=34", "size=128", "square=true") r = r.WithContext(ctx) @@ -113,7 +113,7 @@ var _ = Describe("MediaRetrievalController", func() { Describe("GetLyrics", func() { It("should return data for given artist & title", func() { r := newGetRequest("artist=Rick+Astley", "title=Never+Gonna+Give+You+Up") - lyricsList, _ := model.ParseLyrics(".lrc", "eng", []byte("[00:18.80]We're no strangers to love\n[00:22.80]You know the rules and so do I")) + lyricsList, _ := model.ParseLyrics(GinkgoT().Context(), ".lrc", "eng", []byte("[00:18.80]We're no strangers to love\n[00:22.80]You know the rules and so do I")) lyrics, _ := lyricsList.Main() lyricsJson, err := json.Marshal(model.LyricList{ lyrics, From 427d4b9bce198df09aff18be1beddb85aaa3f725 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Thu, 2 Jul 2026 12:53:10 -0400 Subject: [PATCH 2/8] fix(search): artists with atomic non-ASCII names unfindable after FTS5 migration (#5703) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(scanner): update artist search_normalized when rescanning The FTS5 migration back-fills artist.search_normalized with a SQL punctuation-strip approximation, relying on the next scan to compute the precise value in Go (normalizeForFTS transliterates atomic letters like Ø/æ/ß that FTS5's remove_diacritics cannot fold). But the scanner persisted artists with an explicit column list that omitted search_normalized, so not even a full scan ever repaired it: an artist migrated from a pre-FTS database (e.g. "GØGGS") stayed unfindable by any ASCII search, while their albums and songs, which are saved with all columns, were fixed by a full scan. Add search_normalized to the column list so a full scan re-indexes the artist via the artist_fts trigger. * refactor(persistence): move normalizeForFTS to utils/str Export it as str.NormalizeForFTS so the upcoming migration can reuse the exact index-time normalization. Migrations cannot import the persistence package (persistence -> db -> db/migrations would be an import cycle). * fix(persistence): backfill artist search_normalized via migration Recompute artist.search_normalized with the precise Go normalization for databases migrated from pre-FTS5 versions, where the SQL back-fill could not transliterate atomic letters (Ø/æ/ß) and the scanner never rewrote the column. Only changed rows are updated, so the artist_fts update trigger re-indexes exactly the affected artists, making artists like GØGGS or MØ findable again without requiring a full scan. * refactor(persistence): share FTS punctuation-strip regex via utils/str Index-time normalization (NormalizeForFTS) and query-time processing (buildFTS5Query/ftsQueryDegraded) must produce matching tokens, so keep the punctuation-strip pattern in a single exported symbol instead of two identical private copies that could drift. Also document that derived columns computed in dbArtist.PostMapArgs must be listed in the scanner's artist Put, which is how search_normalized went stale in the first place. * chore(migrations): announce artist search backfill in the log Match the FTS5 migration's notice() pattern so startup isn't silent while the backfill runs on large libraries. * docs: tighten comments added in this branch * docs: describe FTSPunctStrip by what it matches, not one replacement --- ...52457_backfill_artist_search_normalized.go | 58 +++++++++++++++++++ persistence/album_repository.go | 3 +- persistence/artist_repository.go | 5 +- persistence/mediafile_repository.go | 3 +- persistence/sql_search_fts.go | 43 ++------------ persistence/sql_search_fts_test.go | 22 ------- scanner/phase_1_folders.go | 2 +- scanner/scanner_test.go | 28 +++++++++ utils/str/normalize_fts.go | 45 ++++++++++++++ utils/str/normalize_fts_test.go | 29 ++++++++++ 10 files changed, 173 insertions(+), 65 deletions(-) create mode 100644 db/migrations/20260702152457_backfill_artist_search_normalized.go create mode 100644 utils/str/normalize_fts.go create mode 100644 utils/str/normalize_fts_test.go diff --git a/db/migrations/20260702152457_backfill_artist_search_normalized.go b/db/migrations/20260702152457_backfill_artist_search_normalized.go new file mode 100644 index 000000000..93902e230 --- /dev/null +++ b/db/migrations/20260702152457_backfill_artist_search_normalized.go @@ -0,0 +1,58 @@ +package migrations + +import ( + "context" + "database/sql" + "fmt" + + "github.com/navidrome/navidrome/utils/str" + "github.com/pressly/goose/v3" +) + +func init() { + goose.AddMigrationContext(upBackfillArtistSearchNormalized, downBackfillArtistSearchNormalized) +} + +// The FTS5 migration back-filled artist.search_normalized with a SQL approximation that +// cannot transliterate atomic letters (Ø, æ, ß, ...), and the scanner never rewrote the +// column, leaving artists like "GØGGS" unfindable by ASCII searches. Recompute it in Go; +// the artist_fts update trigger re-indexes every row that changes. +func upBackfillArtistSearchNormalized(ctx context.Context, tx *sql.Tx) error { + notice(ctx, tx, "Rebuilding artist search index data. This may take a moment on large libraries.") + + rows, err := tx.QueryContext(ctx, "SELECT id, name, search_normalized FROM artist") + if err != nil { + return fmt.Errorf("querying artists: %w", err) + } + defer rows.Close() + + updates := map[string]string{} + for rows.Next() { + var id, name, current string + if err := rows.Scan(&id, &name, ¤t); err != nil { + return fmt.Errorf("scanning artist: %w", err) + } + if expected := str.NormalizeForFTS(name); expected != current { + updates[id] = expected + } + } + if err := rows.Err(); err != nil { + return fmt.Errorf("iterating artists: %w", err) + } + + stmt, err := tx.PrepareContext(ctx, "UPDATE artist SET search_normalized = ? WHERE id = ?") + if err != nil { + return fmt.Errorf("preparing update: %w", err) + } + defer stmt.Close() + for id, normalized := range updates { + if _, err := stmt.ExecContext(ctx, normalized, id); err != nil { + return fmt.Errorf("updating artist %s: %w", id, err) + } + } + return nil +} + +func downBackfillArtistSearchNormalized(context.Context, *sql.Tx) error { + return nil +} diff --git a/persistence/album_repository.go b/persistence/album_repository.go index 31a0f5c91..34845be15 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -17,6 +17,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/slice" + "github.com/navidrome/navidrome/utils/str" "github.com/pocketbase/dbx" ) @@ -69,7 +70,7 @@ func (a *dbAlbum) PostMapArgs(args map[string]any) error { fullText = append(fullText, a.Album.Tags[model.TagCatalogNumber]...) args["full_text"] = formatFullText(fullText...) args["search_participants"] = strings.Join(participantNames, " ") - args["search_normalized"] = normalizeForFTS(a.Name, a.AlbumArtist) + args["search_normalized"] = str.NormalizeForFTS(a.Name, a.AlbumArtist) args["tags"] = marshalTags(a.Album.Tags) args["participants"] = marshalParticipants(a.Album.Participants) diff --git a/persistence/artist_repository.go b/persistence/artist_repository.go index 5152e774f..f84f410e9 100644 --- a/persistence/artist_repository.go +++ b/persistence/artist_repository.go @@ -19,6 +19,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils" "github.com/navidrome/navidrome/utils/slice" + "github.com/navidrome/navidrome/utils/str" "github.com/pocketbase/dbx" ) @@ -102,8 +103,10 @@ func (a *dbArtist) PostMapArgs(m map[string]any) error { } similarArtists, _ := json.Marshal(sa) m["similar_artists"] = string(similarArtists) + // When adding a derived column here, also add it to the scanner's artist Put column list + // in phase_1_folders.go, or rescans will never update it (how search_normalized went stale). m["full_text"] = formatFullText(a.Name, a.SortArtistName) - m["search_normalized"] = normalizeForFTS(a.Name) + m["search_normalized"] = str.NormalizeForFTS(a.Name) // Do not override the sort_artist_name and mbz_artist_id fields if they are empty // TODO: Better way to handle this? diff --git a/persistence/mediafile_repository.go b/persistence/mediafile_repository.go index d7d892ed1..094268783 100644 --- a/persistence/mediafile_repository.go +++ b/persistence/mediafile_repository.go @@ -16,6 +16,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/slice" + "github.com/navidrome/navidrome/utils/str" "github.com/pocketbase/dbx" ) @@ -62,7 +63,7 @@ func (m *dbMediaFile) PostMapArgs(args map[string]any) error { fullText = append(fullText, participantNames...) args["full_text"] = formatFullText(fullText...) args["search_participants"] = strings.Join(participantNames, " ") - args["search_normalized"] = normalizeForFTS(m.FullTitle(), m.Album, m.Artist, m.AlbumArtist) + args["search_normalized"] = str.NormalizeForFTS(m.FullTitle(), m.Album, m.Artist, m.AlbumArtist) args["tags"] = marshalTags(m.MediaFile.Tags) args["participants"] = marshalParticipants(m.MediaFile.Participants) return nil diff --git a/persistence/sql_search_fts.go b/persistence/sql_search_fts.go index bbae47fe8..999c904af 100644 --- a/persistence/sql_search_fts.go +++ b/persistence/sql_search_fts.go @@ -11,6 +11,7 @@ import ( "github.com/deluan/sanitize" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils/str" ) // containsCJK returns true if the string contains any CJK (Chinese/Japanese/Korean) characters. @@ -35,48 +36,12 @@ func containsCJK(s string) bool { // as unbalanced string delimiters. var fts5SpecialChars = regexp.MustCompile(`[^\p{L}\p{N}\s*"\x00]`) -// fts5PunctStrip strips everything except letters and numbers (no whitespace, wildcards, or quotes). -// Used for normalizing words at index time to create concatenated forms (e.g., "R.E.M." → "REM"). -var fts5PunctStrip = regexp.MustCompile(`[^\p{L}\p{N}]`) - // fts5Operators matches FTS5 boolean operators as whole words (case-insensitive). var fts5Operators = regexp.MustCompile(`(?i)\b(AND|OR|NOT|NEAR)\b`) // fts5LeadingStar matches a * at the start of a token. FTS5 only supports * at the end (prefix queries). var fts5LeadingStar = regexp.MustCompile(`(^|[\s])\*+`) -// normalizeForFTS takes multiple strings and returns a space-separated, deduplicated list of -// alternative searchable forms for each word: punctuation-stripped (R.E.M. → REM, AC/DC → ACDC) -// and ASCII-transliterated (Bjørk → Bjork, œuvre → oeuvre). The transliterated form is needed -// because FTS5's `unicode61 remove_diacritics 2` only handles NFKD-decomposable diacritics — -// atomic letters like ø/æ/œ/ß survive tokenization, so the query side and index side disagree -// without an explicit transliterated entry here. -func normalizeForFTS(values ...string) string { - seen := make(map[string]struct{}) - var result []string - add := func(orig, variant string) { - if variant == "" || variant == orig { - return - } - lower := strings.ToLower(variant) - if _, ok := seen[lower]; ok { - return - } - seen[lower] = struct{}{} - result = append(result, variant) - } - for _, v := range values { - for word := range strings.FieldsSeq(v) { - transliterated := sanitize.Accents(word) - // Concatenated ASCII form: R.E.M. → REM, AC/DC → ACDC, St-Étienne → StEtienne. - add(word, fts5PunctStrip.ReplaceAllString(transliterated, "")) - // Accent-only transliteration for words without name-punctuation (Bjørk → Bjork). - add(word, transliterated) - } - } - return strings.Join(result, " ") -} - // isSingleUnicodeLetter returns true if token is exactly one Unicode letter. func isSingleUnicodeLetter(token string) bool { r, size := utf8.DecodeRuneInString(token) @@ -100,7 +65,7 @@ func processPunctuatedWords(input string, phrases []string) (string, []string) { result = append(result, w) continue } - concat := fts5PunctStrip.ReplaceAllString(w, "") + concat := str.FTSPunctStrip.ReplaceAllString(w, "") if concat == "" || concat == w { result = append(result, w) continue @@ -329,7 +294,7 @@ func ftsQueryDegraded(original, ftsQuery string) bool { // Strip quotes from original for comparison — we want the raw content stripped := strings.ReplaceAll(original, `"`, "") // Extract the alphanumeric content from the original query - alphaNum := fts5PunctStrip.ReplaceAllString(stripped, "") + alphaNum := str.FTSPunctStrip.ReplaceAllString(stripped, "") // If the original is entirely alphanumeric, nothing was stripped — not degraded if len(alphaNum) == len(stripped) { return false @@ -353,7 +318,7 @@ func ftsQueryDegraded(original, ftsQuery string) bool { if strings.HasPrefix(t, `"`) { // Extract content between quotes inner := strings.Trim(t, `"`) - innerAlpha := fts5PunctStrip.ReplaceAllString(inner, " ") + innerAlpha := str.FTSPunctStrip.ReplaceAllString(inner, " ") for it := range strings.FieldsSeq(innerAlpha) { if len(it) > 2 { return false diff --git a/persistence/sql_search_fts_test.go b/persistence/sql_search_fts_test.go index b54e5856a..6c975c601 100644 --- a/persistence/sql_search_fts_test.go +++ b/persistence/sql_search_fts_test.go @@ -74,28 +74,6 @@ var _ = DescribeTable("ftsQueryDegraded", Entry("not degraded for OR groups from processPunctuatedWords", "AC/DC", `("AC DC" OR ACDC*)`, false), ) -var _ = DescribeTable("normalizeForFTS", - func(expected string, values ...string) { - Expect(normalizeForFTS(values...)).To(Equal(expected)) - }, - Entry("strips dots and concatenates", "REM", "R.E.M."), - Entry("strips slash", "ACDC", "AC/DC"), - Entry("strips hyphen", "Aha", "A-ha"), - Entry("skips unchanged ASCII words", "", "The Beatles"), - Entry("handles mixed input", "REM", "R.E.M.", "Automatic for the People"), - Entry("deduplicates", "REM", "R.E.M.", "R.E.M."), - Entry("strips apostrophe from word", "N", "Guns N' Roses"), - Entry("handles multiple values with punctuation", "REM ACDC", "R.E.M.", "AC/DC"), - Entry("transliterates ø to o", "Bjork", "Bjørk"), - Entry("transliterates Ø to O", "Oystein", "Øystein"), - Entry("transliterates œ ligature to oe", "oeuvre", "œuvre"), - Entry("transliterates Latin diacritics", "cafe", "café"), - Entry("transliterates only the non-ASCII words", "Mo Ros", "Mø Rós"), - Entry("combines punctuation strip and transliteration", "StEtienne St-Etienne", "St-Étienne"), - Entry("deduplicates against punctuation form", "Cafe", "Café", "Cafe"), - Entry("transliterates ß to ss", "Strasse", "Straße"), -) - var _ = DescribeTable("containsCJK", func(input string, expected bool) { Expect(containsCJK(input)).To(Equal(expected)) diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go index 38967832c..5e898590b 100644 --- a/scanner/phase_1_folders.go +++ b/scanner/phase_1_folders.go @@ -360,7 +360,7 @@ func (p *phaseFolders) persistChanges(entry *folderEntry) (*folderEntry, error) // Save all new/modified artists to DB. Their information will be incomplete, but they will be refreshed later for i := range entry.artists { err = artistRepo.Put(&entry.artists[i], "name", - "mbz_artist_id", "sort_artist_name", "order_artist_name", "full_text", "updated_at") + "mbz_artist_id", "sort_artist_name", "order_artist_name", "full_text", "search_normalized", "updated_at") if err != nil { log.Error(p.ctx, "Scanner: Error persisting artist to DB", "folder", entry.path, "artist", entry.artists[i].Name, err) return err diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go index cc3732bc3..7f3dca775 100644 --- a/scanner/scanner_test.go +++ b/scanner/scanner_test.go @@ -189,6 +189,34 @@ var _ = Describe("Scanner", Ordered, func() { }) }) + Context("Artist with atomic non-ASCII letters, 'GØGGS'", func() { + BeforeEach(func() { + goggs := template(_t{"albumartist": "GØGGS", "album": "Pre Strike Sweep", "year": 2018}) + createFS(fstest.MapFS{ + "GØGGS/Pre Strike Sweep/01 - Falling For You.mp3": goggs(track(1, "Falling For You")), + }) + }) + + searchNormalized := func() string { + var sn string + Expect(db.Db().QueryRowContext(ctx, + "SELECT search_normalized FROM artist WHERE name = 'GØGGS'").Scan(&sn)).To(Succeed()) + return sn + } + + It("repopulates a stale search_normalized on a full rescan", func() { + Expect(runScanner(ctx, true)).To(Succeed()) + Expect(searchNormalized()).To(Equal("GOGGS")) + + // Simulate the stale value left by the FTS5 migration's SQL back-fill + _, err := db.Db().ExecContext(ctx, "UPDATE artist SET search_normalized = '' WHERE name = 'GØGGS'") + Expect(err).ToNot(HaveOccurred()) + + Expect(runScanner(ctx, true)).To(Succeed()) + Expect(searchNormalized()).To(Equal("GOGGS")) + }) + }) + Context("Ignored entries", func() { BeforeEach(func() { revolver := template(_t{"albumartist": "The Beatles", "album": "Revolver", "year": 1966}) diff --git a/utils/str/normalize_fts.go b/utils/str/normalize_fts.go new file mode 100644 index 000000000..994f77cc9 --- /dev/null +++ b/utils/str/normalize_fts.go @@ -0,0 +1,45 @@ +package str + +import ( + "regexp" + "strings" + + "github.com/deluan/sanitize" +) + +// FTSPunctStrip matches any character that is not a letter or number. Index-time +// normalization (NormalizeForFTS) and query-time processing in persistence share it +// so both sides produce matching tokens. +var FTSPunctStrip = regexp.MustCompile(`[^\p{L}\p{N}]`) + +// NormalizeForFTS takes multiple strings and returns a space-separated, deduplicated list of +// alternative searchable forms for each word: punctuation-stripped (R.E.M. → REM, AC/DC → ACDC) +// and ASCII-transliterated (Bjørk → Bjork, œuvre → oeuvre). The transliterated form is needed +// because FTS5's `unicode61 remove_diacritics 2` only handles NFKD-decomposable diacritics — +// atomic letters like ø/æ/œ/ß survive tokenization, so the query side and index side disagree +// without an explicit transliterated entry here. +func NormalizeForFTS(values ...string) string { + seen := make(map[string]struct{}) + var result []string + add := func(orig, variant string) { + if variant == "" || variant == orig { + return + } + lower := strings.ToLower(variant) + if _, ok := seen[lower]; ok { + return + } + seen[lower] = struct{}{} + result = append(result, variant) + } + for _, v := range values { + for word := range strings.FieldsSeq(v) { + transliterated := sanitize.Accents(word) + // Concatenated ASCII form: R.E.M. → REM, AC/DC → ACDC, St-Étienne → StEtienne. + add(word, FTSPunctStrip.ReplaceAllString(transliterated, "")) + // Accent-only transliteration for words without name-punctuation (Bjørk → Bjork). + add(word, transliterated) + } + } + return strings.Join(result, " ") +} diff --git a/utils/str/normalize_fts_test.go b/utils/str/normalize_fts_test.go new file mode 100644 index 000000000..52387cb1d --- /dev/null +++ b/utils/str/normalize_fts_test.go @@ -0,0 +1,29 @@ +package str_test + +import ( + "github.com/navidrome/navidrome/utils/str" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = DescribeTable("NormalizeForFTS", + func(expected string, values ...string) { + Expect(str.NormalizeForFTS(values...)).To(Equal(expected)) + }, + Entry("strips dots and concatenates", "REM", "R.E.M."), + Entry("strips slash", "ACDC", "AC/DC"), + Entry("strips hyphen", "Aha", "A-ha"), + Entry("skips unchanged ASCII words", "", "The Beatles"), + Entry("handles mixed input", "REM", "R.E.M.", "Automatic for the People"), + Entry("deduplicates", "REM", "R.E.M.", "R.E.M."), + Entry("strips apostrophe from word", "N", "Guns N' Roses"), + Entry("handles multiple values with punctuation", "REM ACDC", "R.E.M.", "AC/DC"), + Entry("transliterates ø to o", "Bjork", "Bjørk"), + Entry("transliterates Ø to O", "Oystein", "Øystein"), + Entry("transliterates œ ligature to oe", "oeuvre", "œuvre"), + Entry("transliterates Latin diacritics", "cafe", "café"), + Entry("transliterates only the non-ASCII words", "Mo Ros", "Mø Rós"), + Entry("combines punctuation strip and transliteration", "StEtienne St-Etienne", "St-Étienne"), + Entry("deduplicates against punctuation form", "Cafe", "Café", "Cafe"), + Entry("transliterates ß to ss", "Strasse", "Straße"), +) From ae9b8a5fe6e43bfe3d4dd07c24efe6c0ddca729c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Thu, 2 Jul 2026 15:51:03 -0400 Subject: [PATCH 3/8] feat(search): rank exact matches above prefix matches (#5704) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(search): boost exact token matches over prefix matches buildFTS5Query now emits (word OR word*) instead of word* for plain tokens. The match set is unchanged (exact is a subset of prefix), but bm25 gives the rare exact token a high-IDF contribution, so rows containing the literal query word rank above prefix-only matches. The degraded-query check keeps evaluating the plain prefix form, preserving the LIKE fallback for queries like "1+" and "C++". * feat(search): weight artist search_normalized equal to name in bm25 For the artist table, search_normalized holds only the artist's name in alternate spelling (transliterated/punctuation-stripped), so a hit there is as meaningful as a name hit. Combined with exact-token boosting, artists like MØ now rank in the top results for the query "MO" instead of dead last. media_file and album keep weight 1.0 because their search_normalized mixes title, album, and artist variants. * test(persistence): add exact-match ranking regression test Seeds MØ, Morrissey, and Modest Mouse and asserts MØ ranks first for the queries "MO" and "MØ": the exact transliterated hit in search_normalized must outrank name-prefix matches. The corpus deliberately has no competing exact-word names, since exact-vs-exact ordering depends on corpus statistics rather than the guaranteed exact-over-prefix property. Rows are inserted per-test (with their library_artist associations) and cleaned up to avoid disturbing the shared seed fixtures and their count assertions. * docs(search): document exact-token OR emission in buildFTS5Query * test(persistence): harden exact-match ranking test fixtures Register the corpus cleanup before the insert loop so a mid-loop assertion failure cannot leak fts-rank-% rows into the shared integration DB, and reuse the existing createArtistWithLibrary helper instead of hand-rolling Put+AddArtist (which also replaces the ad-hoc context.TODO with the helper's GinkgoT().Context). * fix(search): flag multi-word degraded queries for the LIKE fallback The degradation probe was joined with explicit " AND " like the real query, so ftsQueryDegraded counted the literal AND as a long token and never flagged queries where every term degrades to a short token (e.g. "1+ 2+"). This predates this branch (the old code passed the same AND-joined string), but the probe now exists separately, so join it with spaces — it only feeds ftsQueryDegraded, which needs no explicit operators. --- persistence/sql_search_fts.go | 40 ++++++++--- persistence/sql_search_fts_test.go | 107 +++++++++++++++++++++-------- 2 files changed, 107 insertions(+), 40 deletions(-) diff --git a/persistence/sql_search_fts.go b/persistence/sql_search_fts.go index 999c904af..fce77afbb 100644 --- a/persistence/sql_search_fts.go +++ b/persistence/sql_search_fts.go @@ -105,13 +105,15 @@ func isDottedAbbreviation(w string, subTokens []string) bool { } // buildFTS5Query preprocesses user input into a safe FTS5 MATCH expression. +// Plain tokens are emitted as (token OR token*) so bm25 ranks exact-token hits above prefix-only matches. // It preserves quoted phrases and * prefix wildcards, neutralizes FTS5 operators // (by lowercasing them, since FTS5 operators are case-sensitive) and strips // special characters to prevent query injection. -func buildFTS5Query(userInput string) string { +// The second return reports whether tokenization degraded the query (see ftsQueryDegraded). +func buildFTS5Query(userInput string) (string, bool) { q := strings.TrimSpace(userInput) if q == "" || q == `""` { - return "" + return "", false } var phrases []string @@ -151,25 +153,38 @@ func buildFTS5Query(userInput string) string { result = fts5LeadingStar.ReplaceAllString(result, "$1") tokens := strings.Fields(result) - // Append * to plain tokens for prefix matching (e.g., "love" → "love*"). - // Skip tokens that are already wildcarded or are quoted phrase placeholders. + // Two forms per token: a plain prefix form (love*) used only to evaluate query + // degradation, and the final (love OR love*) form. The OR adds no matches + // (exact ⊂ prefix) but gives bm25 a high-IDF exact-term hit, ranking rows that + // contain the literal word above prefix-only matches. Placeholders and + // user-supplied wildcards pass through untouched in both forms. + prefixTokens := make([]string, len(tokens)) + wrappedTokens := make([]string, len(tokens)) for i, t := range tokens { if strings.HasPrefix(t, "\x00") || strings.HasSuffix(t, "*") { + prefixTokens[i], wrappedTokens[i] = t, t continue } - tokens[i] = t + "*" + prefixTokens[i] = t + "*" + wrappedTokens[i] = "(" + t + " OR " + t + "*)" } // Use explicit AND between tokens — FTS5's implicit AND (space-separated) - // doesn't work correctly with parenthesized OR groups from processPunctuatedWords. - result = strings.Join(tokens, " AND ") + // doesn't work correctly with parenthesized OR groups. The prefix form is + // space-joined instead: it only feeds ftsQueryDegraded, which would count a + // literal "AND" as a long token and never flag all-short-token queries. + prefixQuery := strings.Join(prefixTokens, " ") + result = strings.Join(wrappedTokens, " AND ") for i, phrase := range phrases { placeholder := fmt.Sprintf("\x00PHRASE%d\x00", i) + prefixQuery = strings.ReplaceAll(prefixQuery, placeholder, phrase) result = strings.ReplaceAll(result, placeholder, phrase) } - return result + // Degradation is evaluated on the prefix form: ftsQueryDegraded treats + // leading-( tokens as punctuated-word groups and would never flag wrapped ones. + return result, ftsQueryDegraded(userInput, prefixQuery) } // ftsColumn pairs an FTS5 column name with its BM25 relevance weight. @@ -209,7 +224,10 @@ var ftsColumnDefs = map[string][]ftsColumn{ "artist": { {"name", 10.0}, {"sort_artist_name", 1.0}, - {"search_normalized", 1.0}, + // Same weight as name: for artists this column is purely the name in + // alternate spelling (unlike media_file/album, where it mixes + // title/album/artist variants and full weight would distort ranking). + {"search_normalized", 10.0}, }, } @@ -338,8 +356,8 @@ func ftsQueryDegraded(original, ftsQuery string) bool { // tokenization stripped significant content from the query (e.g., "1+" → "1*"). // Returns nil when the query produces no searchable tokens at all. func newFTSSearch(tableName, query string) searchStrategy { - q := buildFTS5Query(query) - if q == "" || ftsQueryDegraded(query, q) { + q, degraded := buildFTS5Query(query) + if q == "" || degraded { // Fallback: try LIKE search with the raw query cleaned := strings.TrimSpace(strings.ReplaceAll(query, `"`, "")) if cleaned != "" { diff --git a/persistence/sql_search_fts_test.go b/persistence/sql_search_fts_test.go index 6c975c601..d0b26e8d5 100644 --- a/persistence/sql_search_fts_test.go +++ b/persistence/sql_search_fts_test.go @@ -12,44 +12,45 @@ import ( var _ = DescribeTable("buildFTS5Query", func(input, expected string) { - Expect(buildFTS5Query(input)).To(Equal(expected)) + q, _ := buildFTS5Query(input) + Expect(q).To(Equal(expected)) }, Entry("returns empty string for empty input", "", ""), Entry("returns empty string for whitespace-only input", " ", ""), - Entry("appends * to a single word for prefix matching", "beatles", "beatles*"), - Entry("appends * to each word for prefix matching", "abbey road", "abbey* AND road*"), - Entry("preserves quoted phrases without appending *", `"the beatles"`, `"the beatles"`), - Entry("does not double-append * to existing prefix wildcard", "beat*", "beat*"), - Entry("strips FTS5 operators and appends * to lowercased words", "AND OR NOT NEAR", "and* AND or* AND not* AND near*"), - Entry("strips special FTS5 syntax characters and appends *", "test^col:val", "test* AND col* AND val*"), - Entry("handles mixed phrases and words", `"the beatles" abbey`, `"the beatles" AND abbey*`), - Entry("handles prefix with multiple words", "beat* abbey", "beat* AND abbey*"), - Entry("collapses multiple spaces", "abbey road", "abbey* AND road*"), - Entry("strips leading * from tokens and appends trailing *", "*livia", "livia*"), - Entry("strips leading * and preserves existing trailing *", "*livia oliv*", "livia* AND oliv*"), + Entry("wraps a single word as exact OR prefix", "beatles", "(beatles OR beatles*)"), + Entry("wraps each word as exact OR prefix", "abbey road", "(abbey OR abbey*) AND (road OR road*)"), + Entry("preserves quoted phrases without wrapping", `"the beatles"`, `"the beatles"`), + Entry("does not wrap user-supplied prefix wildcard", "beat*", "beat*"), + Entry("strips FTS5 operators and wraps lowercased words", "AND OR NOT NEAR", "(and OR and*) AND (or OR or*) AND (not OR not*) AND (near OR near*)"), + Entry("strips special FTS5 syntax characters and wraps", "test^col:val", "(test OR test*) AND (col OR col*) AND (val OR val*)"), + Entry("handles mixed phrases and words", `"the beatles" abbey`, `"the beatles" AND (abbey OR abbey*)`), + Entry("handles prefix with multiple words", "beat* abbey", "beat* AND (abbey OR abbey*)"), + Entry("collapses multiple spaces", "abbey road", "(abbey OR abbey*) AND (road OR road*)"), + Entry("strips leading * from tokens and wraps", "*livia", "(livia OR livia*)"), + Entry("strips leading * and preserves existing trailing *", "*livia oliv*", "(livia OR livia*) AND oliv*"), Entry("strips standalone *", "*", ""), - Entry("strips apostrophe from input", "Guns N' Roses", "Guns* AND N* AND Roses*"), + Entry("strips apostrophe from input", "Guns N' Roses", "(Guns OR Guns*) AND (N OR N*) AND (Roses OR Roses*)"), Entry("converts slashed word to phrase+concat OR", "AC/DC", `("AC DC" OR ACDC*)`), Entry("converts hyphenated word to phrase+concat OR", "a-ha", `("a ha" OR aha*)`), Entry("converts partial hyphenated word to phrase+concat OR", "a-h", `("a h" OR ah*)`), Entry("converts hyphenated name to phrase+concat OR", "Jay-Z", `("Jay Z" OR JayZ*)`), Entry("converts contraction to phrase+concat OR", "it's", `("it s" OR its*)`), - Entry("handles punctuated word mixed with plain words", "best of a-ha", `best* AND of* AND ("a ha" OR aha*)`), - Entry("handles contraction followed by plain words", "you've got", `("you ve" OR youve*) AND got*`), - Entry("strips miscellaneous punctuation", "rock & roll, vol. 2", "rock* AND roll* AND vol* AND 2*"), - Entry("transliterates NFKD-decomposable diacritics", "Björk début", "Bjork* AND debut*"), - Entry("transliterates ø to o", "Øystein", "Oystein*"), - Entry("transliterates œ ligature to oe", "œuvre", "oeuvre*"), - Entry("transliterates æ ligature to ae", "Brennæ", "Brennae*"), - Entry("transliterates mixed unicode words", "Mø Sigur Rós", "Mo* AND Sigur* AND Ros*"), - Entry("transliterates ß to ss", "Straße", "Strasse*"), + Entry("handles punctuated word mixed with plain words", "best of a-ha", `(best OR best*) AND (of OR of*) AND ("a ha" OR aha*)`), + Entry("handles contraction followed by plain words", "you've got", `("you ve" OR youve*) AND (got OR got*)`), + Entry("strips miscellaneous punctuation", "rock & roll, vol. 2", "(rock OR rock*) AND (roll OR roll*) AND (vol OR vol*) AND (2 OR 2*)"), + Entry("transliterates NFKD-decomposable diacritics", "Björk début", "(Bjork OR Bjork*) AND (debut OR debut*)"), + Entry("transliterates ø to o", "Øystein", "(Oystein OR Oystein*)"), + Entry("transliterates œ ligature to oe", "œuvre", "(oeuvre OR oeuvre*)"), + Entry("transliterates æ ligature to ae", "Brennæ", "(Brennae OR Brennae*)"), + Entry("transliterates mixed unicode words", "Mø Sigur Rós", "(Mo OR Mo*) AND (Sigur OR Sigur*) AND (Ros OR Ros*)"), + Entry("transliterates ß to ss", "Straße", "(Strasse OR Strasse*)"), Entry("preserves quoted unicode phrase verbatim", `"Björk"`, `"Björk"`), Entry("collapses dotted abbreviation into phrase", "R.E.M.", `"R E M"`), Entry("collapses abbreviation without trailing dot", "R.E.M", `"R E M"`), - Entry("collapses abbreviation mixed with words", "best of R.E.M.", `best* AND of* AND "R E M"`), + Entry("collapses abbreviation mixed with words", "best of R.E.M.", `(best OR best*) AND (of OR of*) AND "R E M"`), Entry("collapses two-letter abbreviation", "U.K.", `"U K"`), - Entry("does not collapse single letter surrounded by words", "I am fine", "I* AND am* AND fine*"), - Entry("does not collapse single standalone letter", "A test", "A* AND test*"), + Entry("does not collapse single letter surrounded by words", "I am fine", "(I OR I*) AND (am OR am*) AND (fine OR fine*)"), + Entry("does not collapse single standalone letter", "A test", "(A OR A*) AND (test OR test*)"), Entry("preserves quoted phrase with punctuation verbatim", `"ac/dc"`, `"ac/dc"`), Entry("preserves quoted abbreviation verbatim", `"R.E.M."`, `"R.E.M."`), Entry("returns empty string for punctuation-only input", "!!!!!!!", ""), @@ -57,6 +58,20 @@ var _ = DescribeTable("buildFTS5Query", Entry("returns empty string for empty quoted phrase", `""`, ""), ) +var _ = DescribeTable("buildFTS5Query degraded flag", + func(input string, expected bool) { + _, degraded := buildFTS5Query(input) + Expect(degraded).To(Equal(expected)) + }, + Entry("plain words are not degraded", "beatles", false), + Entry("special chars stripped leaving short token is degraded", "1+", true), + Entry("multiple short tokens are degraded", "1+ 2+", true), + Entry("short tokens mixed with a long word are not degraded", "1+ beatles", false), + Entry("quoted short-token phrase is degraded", `"1+"`, true), + Entry("punctuated-name group is not degraded", "AC/DC", false), + Entry("empty input is not degraded", "", false), +) + var _ = DescribeTable("ftsQueryDegraded", func(original, ftsQuery string, expected bool) { Expect(ftsQueryDegraded(original, ftsQuery)).To(Equal(expected)) @@ -143,7 +158,7 @@ var _ = Describe("ftsColumnDefs helpers", func() { It("returns weight CSV for artist", func() { Expect(ftsBM25Weights).To(HaveKeyWithValue("artist", - "10.0, 1.0, 1.0", + "10.0, 1.0, 10.0", )) }) @@ -237,18 +252,18 @@ var _ = Describe("newFTSSearch", func() { Expect(fts.rankExpr).To(Equal("unknown_table_fts.rank")) }) - It("wraps query with column filter for known tables", func() { + It("wraps query with column filter", func() { strategy := newFTSSearch("artist", "Beatles") fts, ok := strategy.(*ftsSearch) Expect(ok).To(BeTrue()) - Expect(fts.matchExpr).To(Equal("{name sort_artist_name search_normalized} : (Beatles*)")) + Expect(fts.matchExpr).To(Equal("{name sort_artist_name search_normalized} : ((Beatles OR Beatles*))")) }) It("passes query without column filter for unknown tables", func() { strategy := newFTSSearch("unknown_table", "test") fts, ok := strategy.(*ftsSearch) Expect(ok).To(BeTrue()) - Expect(fts.matchExpr).To(Equal("test*")) + Expect(fts.matchExpr).To(Equal("(test OR test*)")) }) It("preserves phrase queries inside column filter", func() { @@ -425,4 +440,38 @@ var _ = Describe("FTS5 Integration Search", func() { Expect(results).ToNot(BeEmpty(), "Max=0 should mean no limit, not LIMIT 0") }) }) + + Describe("Exact-match ranking", func() { + BeforeEach(func() { + // Registered before the inserts so a mid-loop failure cannot leak corpus rows. + DeferCleanup(func() { + // library_artist rows are removed by the artist_id ON DELETE CASCADE FK. + _, err := GetDBXBuilder().NewQuery("DELETE FROM artist WHERE id LIKE 'fts-rank-%'").Execute() + Expect(err).ToNot(HaveOccurred()) + }) + // Corpus has no competing exact-word names ("Mo X"): exact-vs-exact order depends + // on corpus statistics; the guaranteed property is exact > prefix. + for _, a := range []model.Artist{ + {ID: "fts-rank-1", Name: "MØ", OrderArtistName: "mø"}, + {ID: "fts-rank-2", Name: "Modest Mouse", OrderArtistName: "modest mouse"}, + {ID: "fts-rank-3", Name: "Morrissey", OrderArtistName: "morrissey"}, + } { + Expect(createArtistWithLibrary(arr, &a, 1)).To(Succeed()) + } + }) + + It("ranks the exact transliterated match first for 'MO'", func() { + results, err := arr.Search("MO", model.QueryOptions{Max: 10}) + Expect(err).ToNot(HaveOccurred()) + Expect(results).To(HaveLen(3)) + Expect(results[0].Name).To(Equal("MØ"), "exact match via search_normalized must outrank prefix matches") + }) + + It("ranks the exact match first for the accented query 'MØ'", func() { + results, err := arr.Search("MØ", model.QueryOptions{Max: 10}) + Expect(err).ToNot(HaveOccurred()) + Expect(results).ToNot(BeEmpty()) + Expect(results[0].Name).To(Equal("MØ")) + }) + }) }) From d4387c550279cf9fcb8f3f5614f694aaa9b3b165 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Fri, 3 Jul 2026 08:58:55 -0400 Subject: [PATCH 4/8] perf(db): index media_file album/artist sort orders (#5706) * perf(db): add composite indexes for song list album/artist sorts The media_file sort mappings for album, artist and albumArtist expand to multi-column ORDER BY clauses that no existing index could satisfy, so SQLite fell back to a full table scan plus a temp B-tree sort of every row (including the large lyrics/tags/full_text columns) even for a single 15-item page. On a 96K-track library this made /api/song?_sort=album take 3.6s on a cold cache. Add composite indexes matching the three sort mappings, allowing the query to walk the index and stop at the page size, in both directions. Drop the now redundant single-column order_album_name/order_artist_name indexes (strict prefixes of the new composites) and three indexes with no query path: birth_time is only read in Go code, and artist/album_artist text column lookups go through the media_file_artists table instead. * fix(ui): make composer and track number columns non-sortable in song list Clicking the Composer header was a silent no-op: composer is not a media_file column, so the native API's sanitizeSort drops the sort and returns rows in table order. Track number sorting across the whole library is not meaningful and cannot use an index (the existing index leads with disc_number). Mark both columns sortable={false}, like quality and mood. * test(persistence): add sort index coverage test for large tables Guard against sort options silently losing index support: every sort mapping on media_file, album and artist is now verified with EXPLAIN QUERY PLAN to be satisfiable by an index (both directions), so adding a mapping or dropping an index that reintroduces a full-table temp B-tree sort fails the test. Sorts that genuinely cannot use an index (random, annotation-join columns, JSON expressions) must be declared in an exceptions list with the reason, keeping the trade-off visible in review. To make the sort mappings the complete declared sort surface, add identity mappings for the media_file columns the UI sorts by without a mapping (year, genre, duration, channels, bpm, path, comment, play_count, play_date, rating). These are behaviorally no-ops: the same ORDER BY was previously produced by the field whitelist fallback. * perf(db): drop PreferSortTags expression indexes from media_file The media_file sort_title/sort_artist_name/sort_album_name expression indexes are only usable when PreferSortTags is enabled - a config reported by ~0.1% of installations (insights, week of 2026-06-22) - yet every install pays their storage (~8.6MB on a 96K-track library) and scanner write overhead. Drop them: PreferSortTags installs fall back to a full sort for title/artist/album orders, everyone else gets smaller DBs and cheaper writes. The order_album_name and order_artist_name collation checks remain valid, now satisfied by the composite sort indexes. Signed-off-by: Deluan --------- Signed-off-by: Deluan --- ...13908_optimize_media_file_sort_indexes.sql | 56 ++++++ persistence/collation_test.go | 3 - persistence/mediafile_repository.go | 10 ++ persistence/sort_index_coverage_test.go | 168 ++++++++++++++++++ ui/src/song/SongList.jsx | 6 +- 5 files changed, 238 insertions(+), 5 deletions(-) create mode 100644 db/migrations/20260703013908_optimize_media_file_sort_indexes.sql create mode 100644 persistence/sort_index_coverage_test.go diff --git a/db/migrations/20260703013908_optimize_media_file_sort_indexes.sql b/db/migrations/20260703013908_optimize_media_file_sort_indexes.sql new file mode 100644 index 000000000..dd36bf4c2 --- /dev/null +++ b/db/migrations/20260703013908_optimize_media_file_sort_indexes.sql @@ -0,0 +1,56 @@ +-- +goose Up +-- +goose StatementBegin + +-- Composite indexes matching the media_file sort mappings for album, artist and +-- albumArtist. Without them, SQLite cannot satisfy the multi-column ORDER BY and +-- falls back to a full scan + temp B-tree sort of the whole table (including all +-- its large columns) even for a small LIMIT. +create index if not exists media_file_album_sort + on media_file(order_album_name, album_id, disc_number, track_number, order_artist_name, title); +create index if not exists media_file_artist_sort + on media_file(order_artist_name, order_album_name, release_date, disc_number, track_number); +create index if not exists media_file_album_artist_sort + on media_file(order_album_artist_name, order_album_name, release_date, disc_number, track_number); + +-- These two are strict prefixes of the composites above, so they are redundant now. +drop index if exists media_file_order_album_name; +drop index if exists media_file_order_artist_name; + +-- No query filters or sorts on these columns: birth_time is only read in Go code; +-- artist/album_artist conditions go through the media_file_artists table. +drop index if exists media_file_birth_time; +drop index if exists media_file_artist; +drop index if exists media_file_album_artist; + +-- These expression indexes are only usable when PreferSortTags is enabled, a +-- config used by ~0.1% of installations (per insights), yet they are maintained +-- on every write of every install. Dropping them means those installs fall back +-- to a full sort; everyone else saves the space and the scanner write overhead. +drop index if exists media_file_sort_title; +drop index if exists media_file_sort_artist_name; +drop index if exists media_file_sort_album_name; +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +drop index if exists media_file_album_sort; +drop index if exists media_file_artist_sort; +drop index if exists media_file_album_artist_sort; + +create index if not exists media_file_order_album_name + on media_file(order_album_name); +create index if not exists media_file_order_artist_name + on media_file(order_artist_name); +create index if not exists media_file_birth_time + on media_file(birth_time); +create index if not exists media_file_artist + on media_file(artist); +create index if not exists media_file_album_artist + on media_file(album_artist); +create index if not exists media_file_sort_title + on media_file (coalesce(nullif(sort_title,''),order_title) collate NOCASE); +create index if not exists media_file_sort_artist_name + on media_file (coalesce(nullif(sort_artist_name,''),order_artist_name) collate NOCASE); +create index if not exists media_file_sort_album_name + on media_file (coalesce(nullif(sort_album_name,''),order_album_name) collate NOCASE); +-- +goose StatementEnd diff --git a/persistence/collation_test.go b/persistence/collation_test.go index bb1276577..dff91148e 100644 --- a/persistence/collation_test.go +++ b/persistence/collation_test.go @@ -50,9 +50,6 @@ var _ = Describe("Collation", func() { Entry("media_file.order_title", "media_file", "order_title collate nocase"), Entry("media_file.order_album_name", "media_file", "order_album_name collate nocase"), Entry("media_file.order_artist_name", "media_file", "order_artist_name collate nocase"), - Entry("media_file.sort_title", "media_file", "coalesce(nullif(sort_title,''),order_title) collate nocase"), - Entry("media_file.sort_album_name", "media_file", "coalesce(nullif(sort_album_name,''),order_album_name) collate nocase"), - Entry("media_file.sort_artist_name", "media_file", "coalesce(nullif(sort_artist_name,''),order_artist_name) collate nocase"), Entry("media_file.path", "media_file", "path collate nocase"), Entry("playlist.name", "playlist", "name collate nocase"), Entry("radio.name", "radio", "name collate nocase"), diff --git a/persistence/mediafile_repository.go b/persistence/mediafile_repository.go index 094268783..b4979ca77 100644 --- a/persistence/mediafile_repository.go +++ b/persistence/mediafile_repository.go @@ -91,6 +91,16 @@ func NewMediaFileRepository(ctx context.Context, db dbx.Builder) model.MediaFile "recently_added": mediaFileRecentlyAddedSort(), "starred_at": "starred, starred_at", "rated_at": "rating, rated_at", + "year": "year", + "genre": "genre", + "duration": "duration", + "channels": "channels", + "bpm": "bpm", + "path": "path", + "comment": "comment", + "play_count": "play_count", + "play_date": "play_date", + "rating": "rating", }) return r } diff --git a/persistence/sort_index_coverage_test.go b/persistence/sort_index_coverage_test.go new file mode 100644 index 000000000..b5dea231d --- /dev/null +++ b/persistence/sort_index_coverage_test.go @@ -0,0 +1,168 @@ +package persistence + +import ( + "context" + "database/sql" + "fmt" + "maps" + "regexp" + "slices" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/db" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// These tests guard against sort options silently losing index support: adding or +// changing a sort mapping, or dropping/renaming an index in a migration, must not +// reintroduce full-table temp B-tree sorts on the large tables. Those are +// catastrophic on big libraries but invisible on dev-sized ones, which is how the +// unindexed album/artist song sorts went unnoticed for years. +// +// Every sort mapping is checked automatically: the real ORDER BY is built via +// buildSortOrder (both directions) and verified with EXPLAIN QUERY PLAN against +// the migrated test schema. The planner's choice is deterministic even on an +// empty table. A sort passes when the plan has no full "USE TEMP B-TREE FOR +// ORDER BY" step; an incremental sort of tie groups ("... FOR LAST TERM OF ORDER +// BY") is fine, as it only sorts rows with equal leading columns. +// +// A new sort mapping therefore fails this test until a matching index is created. +// The only escape hatch is exceptions, for sorts that genuinely cannot be +// served by a table index (random, annotation-join columns, JSON expressions): +// declaring one requires writing down the reason, making the trade-off visible in +// review. The checks run with the default config: PreferSortTags=true rewrites +// mappings to coalesce expressions with no matching indexes (used by ~0.1% of +// installations, per insights), and is out of scope here. +var _ = Describe("Sort index coverage", func() { + conn := db.Db() + + type repoCase struct { + table string + newRepo func(ctx context.Context) *sqlRepository + // sort mapping -> reason it cannot be served by an index + exceptions map[string]string + } + + cases := []repoCase{ + { + table: "media_file", + newRepo: func(ctx context.Context) *sqlRepository { + return &NewMediaFileRepository(ctx, GetDBXBuilder()).(*mediaFileRepository).sqlRepository + }, + exceptions: map[string]string{ + "random": "not a column sort", + "starred_at": "sorts on annotation join columns", + "rated_at": "sorts on annotation join columns", + "play_count": "sorts on annotation join columns", + "play_date": "sorts on annotation join columns", + "rating": "sorts on annotation join columns", + "comment": "UI-sortable but rarely used; not worth an index", + }, + }, + { + table: "album", + newRepo: func(ctx context.Context) *sqlRepository { + return &NewAlbumRepository(ctx, GetDBXBuilder()).(*albumRepository).sqlRepository + }, + exceptions: map[string]string{ + "random": "not a column sort", + "starred_at": "sorts on annotation join columns", + "rated_at": "sorts on annotation join columns", + "max_year": "coalesce expression over original_date/max_year, no expression index", + }, + }, + { + table: "artist", + newRepo: func(ctx context.Context) *sqlRepository { + return &NewArtistRepository(ctx, GetDBXBuilder()).(*artistRepository).sqlRepository + }, + exceptions: map[string]string{ //nolint:gosec // G101 false positive, same as the artist sortMappings + "starred_at": "sorts on annotation join columns", + "rated_at": "sorts on annotation join columns", + "song_count": "JSON expression over stats column", + "album_count": "JSON expression over stats column", + "size": "JSON expression over stats column", + "maincredit_song_count": "aggregate over JSON stats", + "maincredit_album_count": "aggregate over JSON stats", + "maincredit_size": "aggregate over JSON stats", + }, + }, + } + + newCtx := func() context.Context { + ctx := log.NewContext(GinkgoT().Context()) + return request.WithUser(ctx, model.User{ID: "userid"}) + } + + for _, c := range cases { + It(fmt.Sprintf("uses an index for every sort mapping on %s", c.table), func() { + r := c.newRepo(newCtx()) + for _, sort := range slices.Sorted(maps.Keys(r.sortMappings)) { + if _, ok := c.exceptions[sort]; ok { + continue + } + for _, dir := range []string{"asc", "desc"} { + orderBy := r.buildSortOrder(sort, dir) + Expect(checkSortUsesIndex(conn, c.table, orderBy)).To(Succeed(), + "sort %q (%s) on table %q needs an index. Create one matching its ORDER BY, or, if it cannot be served by an index, add it to exceptions with the reason", + sort, dir, c.table) + } + } + }) + + It(fmt.Sprintf("has no stale exceptions entries for %s", c.table), func() { + r := c.newRepo(newCtx()) + for _, sort := range slices.Sorted(maps.Keys(c.exceptions)) { + Expect(r.sortMappings).To(HaveKey(sort), + "exceptions entry %q on table %q does not match any sort mapping - remove it", sort, c.table) + } + }) + } + + It("uses an index for recently_added when RecentlyAddedByModTime is enabled", func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.RecentlyAddedByModTime = true + for _, c := range cases[:2] { // media_file and album + r := c.newRepo(newCtx()) + for _, dir := range []string{"asc", "desc"} { + orderBy := r.buildSortOrder("recently_added", dir) + Expect(checkSortUsesIndex(conn, c.table, orderBy)).To(Succeed(), + "sort recently_added (%s) on table %q", dir, c.table) + } + } + }) +}) + +// Matches the full-sort step only: incremental tie-group sorts are reported as +// "USE TEMP B-TREE FOR LAST TERM OF ORDER BY" (or "LAST N TERMS") and are allowed. +var fullTempBTreeSort = regexp.MustCompile(`USE TEMP B-TREE FOR ORDER BY`) + +func checkSortUsesIndex(conn *sql.DB, table, orderBy string) error { + rows, err := conn.Query(fmt.Sprintf("explain query plan select * from %s order by %s limit 15", table, orderBy)) + if err != nil { + return fmt.Errorf("explain query plan failed for order by %q: %w", orderBy, err) + } + defer rows.Close() + + var details []string + for rows.Next() { + var id, parent, notUsed int + var detail string + if err := rows.Scan(&id, &parent, ¬Used, &detail); err != nil { + return err + } + details = append(details, detail) + } + if err := rows.Err(); err != nil { + return err + } + if slices.ContainsFunc(details, fullTempBTreeSort.MatchString) { + return fmt.Errorf("no index satisfies ORDER BY %s - plan: %v", orderBy, details) + } + return nil +} diff --git a/ui/src/song/SongList.jsx b/ui/src/song/SongList.jsx index d928af549..d44992d0c 100644 --- a/ui/src/song/SongList.jsx +++ b/ui/src/song/SongList.jsx @@ -143,9 +143,11 @@ const SongList = (props) => { return { album: isDesktop && , artist: , - composer: , + composer: , albumArtist: , - trackNumber: isDesktop && , + trackNumber: isDesktop && ( + + ), playCount: isDesktop && ( ), From 89aa58a7137f6dbe6e750ae439902c0beb5d0a39 Mon Sep 17 00:00:00 2001 From: Patrick <52888461+draconivis@users.noreply.github.com> Date: Fri, 3 Jul 2026 23:53:57 +0200 Subject: [PATCH 5/8] feat(ui): add rose pine themes (#5664) * feat(theme): add rose pine themes fix checkboxes * feat(theme): apply review suggestions * feat(theme): fix toolbar background in mobile view * feat(theme): small css improvements --- ui/src/themes/index.js | 6 + ui/src/themes/rosePine.css.js | 148 ++++++++++++++++++++++ ui/src/themes/rosePine.js | 108 ++++++++++++++++ ui/src/themes/rosePineDawn.css.js | 198 ++++++++++++++++++++++++++++++ ui/src/themes/rosePineDawn.js | 108 ++++++++++++++++ ui/src/themes/rosePineMoon.css.js | 148 ++++++++++++++++++++++ ui/src/themes/rosePineMoon.js | 108 ++++++++++++++++ 7 files changed, 824 insertions(+) create mode 100644 ui/src/themes/rosePine.css.js create mode 100644 ui/src/themes/rosePine.js create mode 100644 ui/src/themes/rosePineDawn.css.js create mode 100644 ui/src/themes/rosePineDawn.js create mode 100644 ui/src/themes/rosePineMoon.css.js create mode 100644 ui/src/themes/rosePineMoon.js diff --git a/ui/src/themes/index.js b/ui/src/themes/index.js index f79a6a999..98705da30 100644 --- a/ui/src/themes/index.js +++ b/ui/src/themes/index.js @@ -13,6 +13,9 @@ import CatppuccinLatteTheme from './catppuccinLatte' import DraculaTheme from './dracula' import NuclearTheme from './nuclear' import NutballTheme from './nutball' +import RosePineTheme from './rosePine' +import RosePineDawnTheme from './rosePineDawn' +import RosePineMoonTheme from './rosePineMoon' import AmusicTheme from './amusic' import SquiddiesGlassTheme from './SquiddiesGlass' import NautilineTheme from './nautiline' @@ -43,6 +46,9 @@ export default { NordTheme, NuclearTheme, NutballTheme, + RosePineDawnTheme, + RosePineMoonTheme, + RosePineTheme, SpotifyTheme, SquiddiesGlassTheme, TokyoNightLightTheme, diff --git a/ui/src/themes/rosePine.css.js b/ui/src/themes/rosePine.css.js new file mode 100644 index 000000000..a1c21f562 --- /dev/null +++ b/ui/src/themes/rosePine.css.js @@ -0,0 +1,148 @@ +const stylesheet = ` + +.react-jinke-music-player-main svg:active, .react-jinke-music-player-main svg:hover { + color: #c4a7e7 +} + +.react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle, .react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-track { + background-color: #ebbcba +} + +.react-jinke-music-player-main ::-webkit-scrollbar-thumb { + background-color: #ebbcba; +} + +.react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle:active { + box-shadow: 0 0 2px #ebbcba +} + +.react-jinke-music-player-main .audio-item.playing svg { + color: #ebbcba +} + +.react-jinke-music-player-main .audio-item.playing .player-singer { + color: #ebbcba !important +} + +.react-jinke-music-player-main .loading svg { + color: #ebbcba !important +} + + +.react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle { + border: none; + box-shadow:rgba(25, 23, 36, 0.35) 0px 4px 6px, rgba(25, 23, 36, 0.2) 0px 5px 7px; +} + + +.rc-slider-rail, .rc-slider-track { + height: 6px; +} + +.rc-slider { + padding: 3px 0; +} + +.sound-operation > div:nth-child(4) { + transform: translateX(-50%) translateY(5%) !important; +} + +.sound-operation { + padding: 4px 0; +} + +.react-jinke-music-player-main .music-player-panel { + background-color: #1f1d2e; + color: #e0def4; + box-shadow: 0 0 8px rgba(25, 23, 36, 0.35); +} + +.audio-lists-panel { + background-color: #1f1d2e; + bottom: 6.25rem; + box-shadow:rgba(25, 23, 36, 0.35) 0px 4px 6px, rgba(25, 23, 36, 0.2) 0px 5px 7px; +} + +.audio-lists-panel-content .audio-item.playing { + background-color: rgba(0, 0, 0, 0); +} + +.audio-lists-panel-content .audio-item:nth-child(2n+1) { + background-color: rgba(0, 0, 0, 0); +} + + +.react-jinke-music-player-main .music-player-panel .panel-content .player-content .audio-lists-btn { + background-color:rgba(0,0,0,0); + box-shadow:0 0 0 0; +} + +.audio-lists-panel-content .audio-item { + line-height: 32px; +} + +.react-jinke-music-player-main .music-player-panel .panel-content .img-content { + box-shadow:rgba(25, 23, 36, 0.35) 0px 4px 6px, rgba(25, 23, 36, 0.2) 0px 5px 7px; +} + +.react-jinke-music-player-main .music-player-lyric { + color: #908caa; + -webkit-text-stroke: 0.5px #191724; + font-weight: bolder; +} + +.react-jinke-music-player-main .lyric-btn-active, .react-jinke-music-player-main .lyric-btn-active svg { + color: #908caa !important; +} + +.audio-lists-panel-header { + border-bottom:1px solid #26233a; + box-shadow:none; +} + +.audio-lists-panel-content .audio-item.playing, .audio-lists-panel-content .audio-item.playing svg { + color: #ebbcba +} + +.audio-lists-panel-content .audio-item:active .group:not(.player-delete) svg, .audio-lists-panel-content .audio-item:hover .group:not(.player-delete) svg { + color: #ebbcba +} + +.audio-lists-panel-content .audio-item .player-icons { + scale: 75%; +} + +.audio-lists-panel-content .audio-item:active, +.audio-lists-panel-content .audio-item:hover { + background-color: #26233a; +} + +/* Mobile */ + +.react-jinke-music-player-mobile-cover { + border: none; + box-shadow:rgba(25, 23, 36, 0.35) 0px 4px 6px, rgba(25, 23, 36, 0.2) 0px 5px 7px; +} + +.react-jinke-music-player .music-player-controller { + border: none; + background-color: #1f1d2e; + border-color: #1f1d2e; + box-shadow:rgba(25, 23, 36, 0.35) 0px 4px 6px, rgba(25, 23, 36, 0.2) 0px 5px 7px; + color: #ebbcba; +} + +.react-jinke-music-player .music-player-controller .music-player-controller-setting { + color: rgba(196,167,231,.3); +} + +.react-jinke-music-player-mobile-progress .rc-slider-handle, .react-jinke-music-player-mobile-progress .rc-slider-track { + background-color: #ebbcba; +} + +.react-jinke-music-player-mobile-progress .rc-slider-handle { + border: none; +} +` + +export default stylesheet diff --git a/ui/src/themes/rosePine.js b/ui/src/themes/rosePine.js new file mode 100644 index 000000000..547a1f764 --- /dev/null +++ b/ui/src/themes/rosePine.js @@ -0,0 +1,108 @@ +import stylesheet from './rosePine.css.js' + +export default { + themeName: 'Rosé Pine', + palette: { + primary: { + main: '#ebbcba', + }, + secondary: { + main: '#1f1d2e', + contrastText: '#e0def4', + }, + type: 'dark', + background: { + default: '#191724', + paper: '#1f1d2e', + }, + }, + overrides: { + MuiPaper: { + root: { + color: '#e0def4', + backgroundColor: '#1f1d2e', + }, + }, + MuiButton: { + textPrimary: { + color: '#31748f', + }, + textSecondary: { + color: '#e0def4', + }, + }, + MuiIconButton: { + colorSecondary: { + color: '#6e6a86', + }, + }, + MuiChip: { + clickable: { + background: '#26233a', + }, + }, + MuiCheckbox: { + colorSecondary: { + color: '#6e6a86', + '&$checked': { + color: '#ebbcba', + }, + }, + }, + MuiFormGroup: { + root: { + color: '#e0def4', + }, + }, + MuiFormHelperText: { + root: { + '&$error': { + color: '#eb6f92', + }, + }, + }, + MuiTableHead: { + root: { + color: '#e0def4', + background: '#1f1d2e', + }, + }, + MuiTableCell: { + root: { + color: '#e0def4', + background: '#1f1d2e !important', + }, + head: { + color: '#e0def4', + background: '#1f1d2e !important', + }, + }, + NDLogin: { + systemNameLink: { + color: '#ebbcba', + }, + icon: {}, + welcome: { + color: '#e0def4', + }, + card: { + minWidth: 300, + background: '#191724', + }, + avatar: {}, + button: { + boxShadow: '3px 3px 5px rgba(25, 23, 36, 0.35)', + }, + }, + NDMobileArtistDetails: { + bgContainer: { + background: + 'linear-gradient(to bottom, rgba(25, 23, 36, 0.72), rgb(25, 23, 36))!important', + }, + }, + }, + player: { + theme: 'dark', + stylesheet, + }, +} diff --git a/ui/src/themes/rosePineDawn.css.js b/ui/src/themes/rosePineDawn.css.js new file mode 100644 index 000000000..e3c882815 --- /dev/null +++ b/ui/src/themes/rosePineDawn.css.js @@ -0,0 +1,198 @@ +const stylesheet = ` + .react-jinke-music-player-main.light-theme svg, + .react-jinke-music-player .music-player-controller, + .react-jinke-music-player .audio-circle-process-bar circle[class='stroke'] { + color: #797593; + stroke: #797593; + } + + .react-jinke-music-player-main svg:active, + .react-jinke-music-player-main svg:hover { + color: #907aa9; + } + + .react-jinke-music-player-main.light-theme svg:active, + .react-jinke-music-player-main.light-theme svg:hover { + color: #907aa9; + } + + .react-jinke-music-player-mobile-play-model-tip, + .react-jinke-music-player-main.light-theme .play-mode-title { + background-color: #d7827e; + color: #faf4ed; + } + + .react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle, + .react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-track { + background-color: #d7827e; + } + + .react-jinke-music-player-main ::-webkit-scrollbar-thumb { + background-color: #d7827e; + } + + .react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle:active { + box-shadow: 0 0 2px #d7827e; + } + + .react-jinke-music-player-main .audio-item.playing svg { + color: #d7827e; + } + + .react-jinke-music-player-main .audio-item.playing .player-singer { + color: #d7827e !important; + } + + .react-jinke-music-player-main .loading svg { + color: #d7827e !important; + } + + .react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle { + border: none; + box-shadow: + rgba(70, 66, 97, 0.12) 0px 4px 6px, + rgba(70, 66, 97, 0.08) 0px 5px 7px; + } + + .rc-slider-rail, + .rc-slider-track { + height: 6px; + } + + .rc-slider { + padding: 3px 0; + } + + .react-jinke-music-player-main.light-theme .rc-switch-checked { + background-color: #d7827e !important; + border: 1px solid #d7827e; + } + + .sound-operation > div:nth-child(4) { + transform: translateX(-50%) translateY(5%) !important; + } + + .sound-operation { + padding: 4px 0; + } + + .react-jinke-music-player-main .music-player-panel { + background-color: #fffaf3; + color: #464261; + box-shadow: 0 0 8px rgba(70, 66, 97, 0.12); + } + + .react-jinke-music-player-main.light-theme .music-player-panel { + color: #464261; + } + + .audio-lists-panel { + background-color: #fffaf3; + bottom: 6.25rem; + box-shadow: + rgba(70, 66, 97, 0.12) 0px 4px 6px, + rgba(70, 66, 97, 0.08) 0px 5px 7px; + } + + .audio-lists-panel-content .audio-item.playing { + background-color: rgba(0, 0, 0, 0); + } + + .audio-lists-panel-content .audio-item:nth-child(2n+1) { + background-color: rgba(0, 0, 0, 0); + } + + .audio-lists-panel-header { + border-bottom: 1px solid #f2e9e1; + box-shadow: none; + } + + .react-jinke-music-player-main .music-player-panel .panel-content .player-content .audio-lists-btn { + background-color: rgba(0, 0, 0, 0); + box-shadow: 0 0 0 0; + } + + .react-jinke-music-player-main.light-theme .audio-lists-panel-header { + background-color: #fffaf3; + color: #464261; + } + + .audio-lists-panel-content .audio-item { + line-height: 32px; + color: #464261; + } + + .react-jinke-music-player-main .music-player-panel .panel-content .img-content { + box-shadow: + rgba(70, 66, 97, 0.12) 0px 4px 6px, + rgba(70, 66, 97, 0.08) 0px 5px 7px; + } + + .react-jinke-music-player-main .music-player-lyric { + color: #797593; + -webkit-text-stroke: 0.35px #faf4ed; + font-weight: bolder; + } + + .react-jinke-music-player-main .lyric-btn-active, + .react-jinke-music-player-main .lyric-btn-active svg { + color: #797593 !important; + } + + .audio-lists-panel-content .audio-item.playing, + .audio-lists-panel-content .audio-item.playing svg { + color: #d7827e; + } + + .audio-lists-panel-content .audio-item:active .group:not(.player-delete) svg, + .audio-lists-panel-content .audio-item:hover .group:not(.player-delete) svg { + color: #d7827e; + } + + .audio-lists-panel-content .audio-item .player-icons { + scale: 75%; + } + + .audio-lists-panel-content .audio-item:active, + .audio-lists-panel-content .audio-item:hover { + background-color: #f2e9e1; + } + + /* Mobile */ + .react-jinke-music-player-mobile-cover { + border: none; + box-shadow: + rgba(70, 66, 97, 0.12) 0px 4px 6px, + rgba(70, 66, 97, 0.08) 0px 5px 7px; + } + + .react-jinke-music-player .music-player-controller { + border: none; + background-color: #fffaf3; + border-color: #fffaf3; + box-shadow: + rgba(70, 66, 97, 0.12) 0px 4px 6px, + rgba(70, 66, 97, 0.08) 0px 5px 7px; + color: #d7827e; + } + + .react-jinke-music-player .music-player-controller.music-player-playing:before { + border: 1px solid rgba(70, 66, 97, 0.18); + } + + .react-jinke-music-player .music-player-controller .music-player-controller-setting { + background: rgba(215, 130, 126, 0.2); + color: #faf4ed; + } + + .react-jinke-music-player-mobile-progress .rc-slider-handle, + .react-jinke-music-player-mobile-progress .rc-slider-track { + background-color: #d7827e; + } + + .react-jinke-music-player-mobile-progress .rc-slider-handle { + border: none; + } +` + +export default stylesheet diff --git a/ui/src/themes/rosePineDawn.js b/ui/src/themes/rosePineDawn.js new file mode 100644 index 000000000..aea903c1a --- /dev/null +++ b/ui/src/themes/rosePineDawn.js @@ -0,0 +1,108 @@ +import stylesheet from './rosePineDawn.css.js' + +export default { + themeName: 'Rosé Pine Dawn', + palette: { + primary: { + main: '#d7827e', + }, + secondary: { + main: '#fffaf3', + contrastText: '#464261', + }, + type: 'light', + background: { + default: '#faf4ed', + paper: '#fffaf3', + }, + }, + overrides: { + MuiPaper: { + root: { + color: '#464261', + backgroundColor: '#fffaf3', + }, + }, + MuiButton: { + textPrimary: { + color: '#286983', + }, + textSecondary: { + color: '#464261', + }, + }, + MuiIconButton: { + colorSecondary: { + color: '#9893a5', + }, + }, + MuiChip: { + clickable: { + background: '#f2e9e1', + }, + }, + MuiCheckbox: { + colorSecondary: { + color: '#9893a5', + '&$checked': { + color: '#d7827e', + }, + }, + }, + MuiFormGroup: { + root: { + color: '#464261', + }, + }, + MuiFormHelperText: { + root: { + '&$error': { + color: '#b4637a', + }, + }, + }, + MuiTableHead: { + root: { + color: '#464261', + background: '#fffaf3', + }, + }, + MuiTableCell: { + root: { + color: '#464261', + background: '#fffaf3 !important', + }, + head: { + color: '#464261', + background: '#fffaf3 !important', + }, + }, + NDLogin: { + systemNameLink: { + color: '#d7827e', + }, + icon: {}, + welcome: { + color: '#464261', + }, + card: { + minWidth: 300, + background: '#faf4ed', + }, + avatar: {}, + button: { + boxShadow: '3px 3px 5px rgba(87, 82, 121, 0.12)', + }, + }, + NDMobileArtistDetails: { + bgContainer: { + background: + 'linear-gradient(to bottom, rgba(250, 244, 237, 0.72), rgb(250, 244, 237))!important', + }, + }, + }, + player: { + theme: 'light', + stylesheet, + }, +} diff --git a/ui/src/themes/rosePineMoon.css.js b/ui/src/themes/rosePineMoon.css.js new file mode 100644 index 000000000..0eb7aaf57 --- /dev/null +++ b/ui/src/themes/rosePineMoon.css.js @@ -0,0 +1,148 @@ +const stylesheet = ` + +.react-jinke-music-player-main svg:active, .react-jinke-music-player-main svg:hover { + color: #c4a7e7 +} + +.react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle, .react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-track { + background-color: #ea9a97 +} + +.react-jinke-music-player-main ::-webkit-scrollbar-thumb { + background-color: #ea9a97; +} + +.react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle:active { + box-shadow: 0 0 2px #ea9a97 +} + +.react-jinke-music-player-main .audio-item.playing svg { + color: #ea9a97 +} + +.react-jinke-music-player-main .audio-item.playing .player-singer { + color: #ea9a97 !important +} + +.react-jinke-music-player-main .loading svg { + color: #ea9a97 !important +} + + +.react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle { + border: none; + box-shadow:rgba(35, 33, 54, 0.35) 0px 4px 6px, rgba(35, 33, 54, 0.2) 0px 5px 7px; +} + + +.rc-slider-rail, .rc-slider-track { + height: 6px; +} + +.rc-slider { + padding: 3px 0; +} + +.sound-operation > div:nth-child(4) { + transform: translateX(-50%) translateY(5%) !important; +} + +.sound-operation { + padding: 4px 0; +} + +.react-jinke-music-player-main .music-player-panel { + background-color: #2a273f; + color: #e0def4; + box-shadow: 0 0 8px rgba(35, 33, 54, 0.35); +} + +.audio-lists-panel { + background-color: #2a273f; + bottom: 6.25rem; + box-shadow:rgba(35, 33, 54, 0.35) 0px 4px 6px, rgba(35, 33, 54, 0.2) 0px 5px 7px; +} + +.audio-lists-panel-content .audio-item.playing { + background-color: rgba(0, 0, 0, 0); +} + +.audio-lists-panel-content .audio-item:nth-child(2n+1) { + background-color: rgba(0, 0, 0, 0); +} + + +.react-jinke-music-player-main .music-player-panel .panel-content .player-content .audio-lists-btn { + background-color:rgba(0,0,0,0); + box-shadow:0 0 0 0; +} + +.audio-lists-panel-content .audio-item { + line-height: 32px; +} + +.react-jinke-music-player-main .music-player-panel .panel-content .img-content { + box-shadow:rgba(35, 33, 54, 0.35) 0px 4px 6px, rgba(35, 33, 54, 0.2) 0px 5px 7px; +} + +.react-jinke-music-player-main .music-player-lyric { + color: #908caa; + -webkit-text-stroke: 0.5px #232136; + font-weight: bolder; +} + +.react-jinke-music-player-main .lyric-btn-active, .react-jinke-music-player-main .lyric-btn-active svg { + color: #908caa !important; +} + +.audio-lists-panel-header { + border-bottom:1px solid #393552; + box-shadow:none; +} + +.audio-lists-panel-content .audio-item.playing, .audio-lists-panel-content .audio-item.playing svg { + color: #ea9a97 +} + +.audio-lists-panel-content .audio-item:active .group:not(.player-delete) svg, .audio-lists-panel-content .audio-item:hover .group:not(.player-delete) svg { + color: #ea9a97 +} + +.audio-lists-panel-content .audio-item .player-icons { + scale: 75%; +} + +.audio-lists-panel-content .audio-item:active, +.audio-lists-panel-content .audio-item:hover { + background-color: #393552; +} + +/* Mobile */ + +.react-jinke-music-player-mobile-cover { + border: none; + box-shadow:rgba(35, 33, 54, 0.35) 0px 4px 6px, rgba(35, 33, 54, 0.2) 0px 5px 7px; +} + +.react-jinke-music-player .music-player-controller { + border: none; + background-color: #2a273f; + border-color: #2a273f; + box-shadow:rgba(35, 33, 54, 0.35) 0px 4px 6px, rgba(35, 33, 54, 0.2) 0px 5px 7px; + color: #ea9a97; +} + +.react-jinke-music-player .music-player-controller .music-player-controller-setting { + color: rgba(196,167,231,.3); +} + +.react-jinke-music-player-mobile-progress .rc-slider-handle, .react-jinke-music-player-mobile-progress .rc-slider-track { + background-color: #ea9a97; +} + +.react-jinke-music-player-mobile-progress .rc-slider-handle { + border: none; +} +` + +export default stylesheet diff --git a/ui/src/themes/rosePineMoon.js b/ui/src/themes/rosePineMoon.js new file mode 100644 index 000000000..facf09446 --- /dev/null +++ b/ui/src/themes/rosePineMoon.js @@ -0,0 +1,108 @@ +import stylesheet from './rosePineMoon.css.js' + +export default { + themeName: 'Rosé Pine Moon', + palette: { + primary: { + main: '#ea9a97', + }, + secondary: { + main: '#2a273f', + contrastText: '#e0def4', + }, + type: 'dark', + background: { + default: '#232136', + paper: '#2a273f', + }, + }, + overrides: { + MuiPaper: { + root: { + color: '#e0def4', + backgroundColor: '#2a273f', + }, + }, + MuiButton: { + textPrimary: { + color: '#3e8fb0', + }, + textSecondary: { + color: '#e0def4', + }, + }, + MuiIconButton: { + colorSecondary: { + color: '#6e6a86', + }, + }, + MuiChip: { + clickable: { + background: '#393552', + }, + }, + MuiCheckbox: { + colorSecondary: { + color: '#6e6a86', + '&$checked': { + color: '#ea9a97', + }, + }, + }, + MuiFormGroup: { + root: { + color: '#e0def4', + }, + }, + MuiFormHelperText: { + root: { + '&$error': { + color: '#eb6f92', + }, + }, + }, + MuiTableHead: { + root: { + color: '#e0def4', + background: '#2a273f', + }, + }, + MuiTableCell: { + root: { + color: '#e0def4', + background: '#2a273f !important', + }, + head: { + color: '#e0def4', + background: '#2a273f !important', + }, + }, + NDLogin: { + systemNameLink: { + color: '#ea9a97', + }, + icon: {}, + welcome: { + color: '#e0def4', + }, + card: { + minWidth: 300, + background: '#232136', + }, + avatar: {}, + button: { + boxShadow: '3px 3px 5px rgba(35, 33, 54, 0.35)', + }, + }, + NDMobileArtistDetails: { + bgContainer: { + background: + 'linear-gradient(to bottom, rgba(35, 33, 54, 0.72), rgb(35, 33, 54))!important', + }, + }, + }, + player: { + theme: 'dark', + stylesheet, + }, +} From 37e75c435402f413ff72e3a61cbf222ff6c86b93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sat, 4 Jul 2026 19:55:34 -0400 Subject: [PATCH 6/8] feat(sharing): enable sharing by default (#5714) Flip the EnableSharing default from false to true so new installations have the sharing feature available out of the box. Users can still disable it via the EnableSharing config option. The native API only registers the /share route when sharing is enabled, so the nativeapi tests that build the router without wiring a share service now explicitly disable sharing in their setup to avoid registering a route backed by a nil service. --- conf/configuration.go | 2 +- server/nativeapi/config_test.go | 1 + server/nativeapi/library_test.go | 2 ++ server/nativeapi/native_api_song_test.go | 1 + server/nativeapi/playlists_test.go | 1 + server/nativeapi/plugin_test.go | 1 + 6 files changed, 7 insertions(+), 1 deletion(-) diff --git a/conf/configuration.go b/conf/configuration.go index 17abfe400..8646bf075 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -794,7 +794,7 @@ func setViperDefaults() { viper.SetDefault("uiplaybackreportinterval", consts.DefaultUIPlaybackReportInterval) viper.SetDefault("enableartworkupload", true) viper.SetDefault("maximageuploadsize", consts.DefaultMaxImageUploadSize) - viper.SetDefault("enablesharing", false) + viper.SetDefault("enablesharing", true) viper.SetDefault("shareurl", "") viper.SetDefault("defaultshareexpiration", 8760*time.Hour) viper.SetDefault("defaultdownloadableshare", false) diff --git a/server/nativeapi/config_test.go b/server/nativeapi/config_test.go index 4e6e9e89b..107b01e01 100644 --- a/server/nativeapi/config_test.go +++ b/server/nativeapi/config_test.go @@ -25,6 +25,7 @@ var _ = Describe("Config API", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) + conf.Server.EnableSharing = false conf.Server.DevUIShowConfig = true // Enable config endpoint for tests ds = &tests.MockDataStore{} auth.Init(ds) diff --git a/server/nativeapi/library_test.go b/server/nativeapi/library_test.go index ed5564a41..9b7061845 100644 --- a/server/nativeapi/library_test.go +++ b/server/nativeapi/library_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "strings" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/core/auth" @@ -27,6 +28,7 @@ var _ = Describe("Library API", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) + conf.Server.EnableSharing = false ds = &tests.MockDataStore{} auth.Init(ds) nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil) diff --git a/server/nativeapi/native_api_song_test.go b/server/nativeapi/native_api_song_test.go index f0ee50ebb..b1ed09d65 100644 --- a/server/nativeapi/native_api_song_test.go +++ b/server/nativeapi/native_api_song_test.go @@ -32,6 +32,7 @@ var _ = Describe("Song Endpoints", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) + conf.Server.EnableSharing = false conf.Server.SessionTimeout = time.Minute // Setup mock repositories diff --git a/server/nativeapi/playlists_test.go b/server/nativeapi/playlists_test.go index e1c933709..9bf502687 100644 --- a/server/nativeapi/playlists_test.go +++ b/server/nativeapi/playlists_test.go @@ -76,6 +76,7 @@ var _ = Describe("Playlist Tracks Endpoint", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) + conf.Server.EnableSharing = false conf.Server.SessionTimeout = time.Minute plsSvc = &mockPlaylistsService{} diff --git a/server/nativeapi/plugin_test.go b/server/nativeapi/plugin_test.go index 8fc88e09c..aa91a7951 100644 --- a/server/nativeapi/plugin_test.go +++ b/server/nativeapi/plugin_test.go @@ -29,6 +29,7 @@ var _ = Describe("Plugin API", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) + conf.Server.EnableSharing = false conf.Server.Plugins.Enabled = true ds = &tests.MockDataStore{} mockManager = &tests.MockPluginManager{} From 4f6afbbe671611f4d77db2eac79fd4036d7536e1 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 4 Jul 2026 21:38:01 -0400 Subject: [PATCH 7/8] fix(ci): pin GoReleaser version to 2.16.0 Signed-off-by: Deluan --- .github/workflows/pipeline.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 8d3cd3ad0..8a01dc028 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -491,7 +491,7 @@ jobs: - name: Run GoReleaser uses: goreleaser/goreleaser-action@v7 with: - version: '~> v2' + version: '2.16.0' args: "release --clean -f release/goreleaser.yml ${{ env.RELEASE_FLAGS }}" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} From e3297831125d49593123d0c93cb5cb928e3635ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sun, 5 Jul 2026 00:15:59 -0400 Subject: [PATCH 8/8] fix(build): derive version from reachable git tag (#5711) * fix(build): derive version from reachable git tag * fix(build): fall back gracefully when no tag is reachable git describe --tags --abbrev=0 exits with an error when the checkout has no reachable tag (e.g. tagless forks or pre-first-tag commits). In the Makefile this printed a fatal message and produced a bare -SNAPSHOT version, and in the CI git-version step the non-zero exit would abort the job under bash -e before the empty-tag guard could run. Silence stderr and fall back to v0.0.0 in the Makefile, and to an empty string in the workflow so the existing guard keeps skipping the output as it did before. --- .github/workflows/pipeline.yml | 4 ++-- Makefile | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 8a01dc028..228bac9e7 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -32,7 +32,7 @@ jobs: - name: Show git version info run: | echo "git describe (dirty): $(git describe --dirty --always --tags)" - echo "git describe --tags: $(git describe --tags `git rev-list --tags --max-count=1`)" + echo "git describe --tags --abbrev=0: $(git describe --tags --abbrev=0)" echo "git tag: $(git tag --sort=-committerdate | head -n 1)" echo "github_ref: $GITHUB_REF" echo "github_head_sha: ${{ github.event.pull_request.head.sha }}" @@ -40,7 +40,7 @@ jobs: - name: Determine git current SHA and latest tag id: git-version run: | - GIT_TAG=$(git tag --sort=-committerdate | head -n 1) + GIT_TAG=$(git describe --tags --abbrev=0 2>/dev/null || true) if [ -n "$GIT_TAG" ]; then if [[ "$GITHUB_REF" != refs/tags/* ]]; then GIT_TAG=${GIT_TAG}-SNAPSHOT diff --git a/Makefile b/Makefile index 90a405de8..fa0d10475 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ export ND_ENABLEINSIGHTSCOLLECTOR=false ifneq ("$(wildcard .git/HEAD)","") GIT_SHA=$(shell git rev-parse --short HEAD) -GIT_TAG=$(shell git describe --tags `git rev-list --tags --max-count=1`)-SNAPSHOT +GIT_TAG=$(shell git describe --tags --abbrev=0 2>/dev/null || echo v0.0.0)-SNAPSHOT else GIT_SHA=source_archive GIT_TAG=$(patsubst navidrome-%,v%,$(notdir $(PWD)))-SNAPSHOT