Upstream navidrome/navidrome state of the touched files before b3c16e5fa1

persistence/playlist_repository.go

@@ -0,0 +1,502 @@
+package persistence
+
+import (
+	"context"
+	"database/sql"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"slices"
+	"time"
+
+	. "github.com/Masterminds/squirrel"
+	"github.com/deluan/rest"
+	"github.com/navidrome/navidrome/log"
+	"github.com/navidrome/navidrome/model"
+	"github.com/navidrome/navidrome/utils/slice"
+	"github.com/pocketbase/dbx"
+)
+
+type playlistRepository struct {
+	sqlRepository
+}
+
+type dbPlaylist struct {
+	model.Playlist `structs:",flatten"`
+	Rules          sql.NullString `structs:"-"`
+}
+
+func (p *dbPlaylist) PostScan() error {
+	if p.Rules.String != "" {
+		return json.Unmarshal([]byte(p.Rules.String), &p.Playlist.Rules)
+	}
+	return nil
+}
+
+func (p dbPlaylist) PostMapArgs(args map[string]any) error {
+	var err error
+	if p.Playlist.IsSmartPlaylist() {
+		args["rules"], err = json.Marshal(p.Playlist.Rules)
+		if err != nil {
+			return fmt.Errorf("invalid criteria expression: %w", err)
+		}
+		// Smart playlist counters are owned by refreshCounters (evaluation), never by callers
+		delete(args, "song_count")
+		delete(args, "duration")
+		delete(args, "size")
+		return nil
+	}
+	delete(args, "rules")
+	return nil
+}
+
+func NewPlaylistRepository(db dbx.Builder) model.PlaylistRepository {
+	r := &playlistRepository{}
+	r.db = db
+	r.registerModel(&model.Playlist{}, map[string]filterFunc{
+		"id":      idFilter("playlist"),
+		"q":       playlistFilter,
+		"smart":   smartPlaylistFilter,
+		"starred": annotationBoolFilter("starred"),
+	})
+	r.setSortMappings(map[string]string{
+		"name":       naturalSort("playlist.name"),
+		"owner_name": naturalSort("owner_name"),
+	})
+	return r
+}
+
+func playlistFilter(_ string, value any) Sqlizer {
+	return Or{
+		substringFilter("playlist.name", value),
+		substringFilter("playlist.comment", value),
+	}
+}
+
+func smartPlaylistFilter(string, any) Sqlizer {
+	return Or{
+		Eq{"rules": ""},
+		Eq{"rules": nil},
+	}
+}
+
+func (r *playlistRepository) userFilter(ctx context.Context) Sqlizer {
+	user := loggedUser(ctx)
+	if user.IsAdmin {
+		return And{}
+	}
+	return Or{
+		Eq{"public": true},
+		Eq{"owner_id": user.ID},
+	}
+}
+
+func (r *playlistRepository) CountAll(ctx context.Context, options ...model.QueryOptions) (int64, error) {
+	query := Select().Where(r.userFilter(ctx))
+	if filtersNeedAnnotation(r.applyFilters(query, options...)) {
+		query = r.withAnnotation(ctx, query, "playlist.id")
+	}
+	return r.count(ctx, query, options...)
+}
+
+func (r *playlistRepository) Exists(ctx context.Context, id string) (bool, error) {
+	return r.exists(ctx, And{Eq{"id": id}, r.userFilter(ctx)})
+}
+
+func (r *playlistRepository) Delete(ctx context.Context, ids ...string) error {
+	return r.delete(ctx, And{Eq{"id": ids}, r.userFilter(ctx)})
+}
+
+func (r *playlistRepository) Put(ctx context.Context, p *model.Playlist, cols ...string) error {
+	pls := dbPlaylist{Playlist: *p}
+	if len(cols) > 0 {
+		if pls.ID == "" {
+			return errors.New("playlist id is required for partial update")
+		}
+		_, err := r.put(ctx, pls.ID, pls, cols...)
+		return err
+	}
+	isNew := pls.ID == ""
+	if isNew {
+		pls.CreatedAt = time.Now()
+	}
+	pls.UpdatedAt = time.Now()
+
+	id, err := r.put(ctx, pls.ID, pls)
+	if err != nil {
+		return err
+	}
+	p.ID = id
+
+	if p.IsSmartPlaylist() {
+		// Do not update tracks at this point, as it may take a long time and lock the DB, breaking the scan process
+		return nil
+	}
+	// Only update tracks if they were specified
+	if len(pls.Tracks) > 0 {
+		return r.updateTracks(ctx, id, p.MediaFiles())
+	}
+	pls.ID = id // r.put assigns the generated id to p, not to this copy
+	if isNew {
+		// Even a trackless new playlist has art to find (an imported m3u can carry an
+		// ExternalImageURL); an update landing here changed only metadata, so leave its cover be.
+		r.enqueueCoverRebuild(ctx, id)
+	}
+	return r.refreshCounters(ctx, &pls.Playlist)
+}
+
+func (r *playlistRepository) Get(ctx context.Context, id string) (*model.Playlist, error) {
+	return r.findBy(ctx, And{Eq{"playlist.id": id}, r.userFilter(ctx)})
+}
+
+func (r *playlistRepository) GetWithTracks(ctx context.Context, id string, refreshSmartPlaylist, includeMissing bool) (*model.Playlist, error) {
+	pls, err := r.Get(ctx, id)
+	if err != nil {
+		return nil, err
+	}
+	if refreshSmartPlaylist {
+		r.refreshSmartPlaylist(ctx, pls)
+	}
+	tracks, err := r.loadTracks(ctx, Select().From("playlist_tracks").
+		Where(Eq{"missing": false}).
+		OrderBy("playlist_tracks.id"), id)
+	if err != nil {
+		log.Error(ctx, "Error loading playlist tracks ", "playlist", pls.Name, "id", pls.ID, err)
+		return nil, err
+	}
+	pls.SetTracks(tracks)
+	return pls, nil
+}
+
+func (r *playlistRepository) FindByPath(ctx context.Context, path string) (*model.Playlist, error) {
+	return r.findBy(ctx, Eq{"path": path})
+}
+
+func (r *playlistRepository) findBy(ctx context.Context, sql Sqlizer) (*model.Playlist, error) {
+	sel := r.selectPlaylist(ctx).Where(sql)
+	var pls []dbPlaylist
+	err := r.queryAll(ctx, sel, &pls)
+	if err != nil {
+		return nil, err
+	}
+	if len(pls) == 0 {
+		return nil, model.ErrNotFound
+	}
+
+	list := model.Playlists{pls[0].Playlist}
+	r.hydrateArtwork(ctx, list)
+	return &list[0], nil
+}
+
+func (r *playlistRepository) hydrateArtwork(ctx context.Context, playlists model.Playlists) {
+	hydrateItems(ctx, r.db, model.KindPlaylistArtwork, playlists,
+		func(p *model.Playlist) (string, *model.ItemImage) { return p.ID, &p.ItemImage })
+}
+
+func (r *playlistRepository) GetAll(ctx context.Context, options ...model.QueryOptions) (model.Playlists, error) {
+	sel := r.selectPlaylist(ctx, options...).Where(r.userFilter(ctx))
+	var res []dbPlaylist
+	err := r.queryAll(ctx, sel, &res)
+	if err != nil {
+		return nil, err
+	}
+	playlists := make(model.Playlists, len(res))
+	for i, p := range res {
+		playlists[i] = p.Playlist
+	}
+	r.hydrateArtwork(ctx, playlists)
+	return playlists, err
+}
+
+// getAllIDs returns the IDs of GetAll's row set, skipping its per-row processing.
+func (r *playlistRepository) getAllIDs(ctx context.Context, options ...model.QueryOptions) ([]string, error) {
+	// Joins a projection of user, not the table: its name/created_at columns would make an ORDER BY
+	// on the playlist's own ambiguous.
+	sq := r.newSelect(ctx, options...).Columns("playlist.id", "user.user_name as owner_name").
+		Join("(select id, user_name from user) user on user.id = owner_id").Where(r.userFilter(ctx))
+	if filtersNeedAnnotation(sq) {
+		sq = r.withAnnotation(ctx, sq, "playlist.id")
+	}
+	ids := []string{}
+	err := r.queryAllSlice(ctx, sq, &ids)
+	return ids, err
+}
+
+func (r *playlistRepository) GetCursor(ctx context.Context, options ...model.QueryOptions) (model.PlaylistCursor, error) {
+	// Both passes apply userFilter, so a visibility change between them cannot widen the cursor.
+	ids, err := r.getAllIDs(ctx, options...)
+	if err != nil {
+		return nil, err
+	}
+	opts := chunkOptions(options, "playlist.id")
+	return model.PlaylistCursor(streamByIDs(ids, func(chunk []string) (model.Playlists, error) {
+		return r.GetAll(ctx, opts(chunk))
+	})), nil
+}
+
+func (r *playlistRepository) GetPlaylists(ctx context.Context, mediaFileId string) (model.Playlists, error) {
+	sel := r.selectPlaylist(ctx, model.QueryOptions{Sort: "name"}).
+		Join("playlist_tracks on playlist.id = playlist_tracks.playlist_id").
+		Where(And{Eq{"playlist_tracks.media_file_id": mediaFileId}, r.userFilter(ctx)})
+	var res []dbPlaylist
+	err := r.queryAll(ctx, sel, &res)
+	if err != nil {
+		if errors.Is(err, model.ErrNotFound) {
+			return model.Playlists{}, nil
+		}
+		return nil, err
+	}
+	playlists := make(model.Playlists, len(res))
+	for i, p := range res {
+		playlists[i] = p.Playlist
+	}
+	r.hydrateArtwork(ctx, playlists)
+	return playlists, nil
+}
+
+func (r *playlistRepository) selectPlaylist(ctx context.Context, options ...model.QueryOptions) SelectBuilder {
+	sel := r.newSelect(ctx, options...).Join("user on user.id = owner_id").
+		Columns(r.tableName+".*", "user.user_name as owner_name")
+	return r.withAnnotation(ctx, sel, r.tableName+".id")
+}
+
+func (r *playlistRepository) updateTracks(ctx context.Context, id string, tracks model.MediaFiles) error {
+	ids := make([]string, len(tracks))
+	for i := range tracks {
+		ids[i] = tracks[i].ID
+	}
+	return r.updatePlaylist(ctx, id, ids)
+}
+
+func (r *playlistRepository) updatePlaylist(ctx context.Context, playlistId string, mediaFileIds []string) error {
+	// Remove old tracks
+	del := Delete("playlist_tracks").Where(Eq{"playlist_id": playlistId})
+	_, err := r.executeSQL(ctx, del)
+	if err != nil {
+		return err
+	}
+
+	_, err = r.addTracks(ctx, playlistId, 1, mediaFileIds)
+	return err
+}
+
+// addTracks is the only path that writes playlist_tracks rows (smart playlists aside), so it owns
+// the library and existence check: every caller, including a full replace through Put, goes through it.
+func (r *playlistRepository) addTracks(ctx context.Context, playlistId string, startingPos int, mediaFileIds []string) (int, error) {
+	mediaFileIds, err := r.keepAccessible(ctx, mediaFileIds)
+	if err != nil {
+		return 0, err
+	}
+	// Break the track list in chunks to avoid hitting SQLITE_MAX_VARIABLE_NUMBER limit
+	// Add new tracks, chunk by chunk
+	pos := startingPos
+	for chunk := range slices.Chunk(mediaFileIds, 200) {
+		ins := Insert("playlist_tracks").Columns("playlist_id", "media_file_id", "id")
+		for _, t := range chunk {
+			ins = ins.Values(playlistId, t, pos)
+			pos++
+		}
+		if _, err := r.executeSQL(ctx, ins); err != nil {
+			return 0, err
+		}
+	}
+
+	r.enqueueCoverRebuild(ctx, playlistId)
+	return len(mediaFileIds), r.refreshCounters(ctx, &model.Playlist{ID: playlistId})
+}
+
+// keepAccessible drops ids the caller cannot read or that match no track, preserving order and duplicates.
+// Chunked because callers pass unbounded id lists (M3U import), well past SQLITE_MAX_VARIABLE_NUMBER.
+func (r *playlistRepository) keepAccessible(ctx context.Context, mediaFileIds []string) ([]string, error) {
+	accessible := make(map[string]struct{}, len(mediaFileIds))
+	for chunk := range slices.Chunk(slice.Unique(mediaFileIds), 200) {
+		sq := r.applyLibraryFilter(ctx, Select("id").From("media_file").Where(Eq{"id": chunk}), "media_file")
+		var found []string
+		if err := r.queryAllSlice(ctx, sq, &found); err != nil {
+			return nil, err
+		}
+		for _, id := range found {
+			accessible[id] = struct{}{}
+		}
+	}
+	return slice.Filter(mediaFileIds, func(id string) bool {
+		_, ok := accessible[id]
+		return ok
+	}), nil
+}
+
+// refreshCounters updates total playlist duration, size and count
+func (r *playlistRepository) refreshCounters(ctx context.Context, pls *model.Playlist) error {
+	statsSql := Select(
+		"coalesce(sum(duration), 0) as duration",
+		"coalesce(sum(size), 0) as size",
+		"count(*) as count",
+	).
+		From("media_file").
+		Join("playlist_tracks f on f.media_file_id = media_file.id").
+		Where(Eq{"playlist_id": pls.ID})
+	var res struct{ Duration, Size, Count float32 }
+	err := r.queryOne(ctx, statsSql, &res)
+	if err != nil {
+		return err
+	}
+
+	// Update playlist's total duration, size and count
+	now := time.Now()
+	upd := Update("playlist").
+		Set("duration", res.Duration).
+		Set("size", res.Size).
+		Set("song_count", res.Count).
+		Set("updated_at", now).
+		Where(Eq{"id": pls.ID})
+	_, err = r.executeSQL(ctx, upd)
+	if err != nil {
+		return err
+	}
+	pls.SongCount = int(res.Count)
+	pls.Duration = res.Duration
+	pls.Size = int64(res.Size)
+	pls.UpdatedAt = now
+	return nil
+}
+
+// enqueueCoverRebuild re-resolves the generated 2x2 grid. Call it only when the track set changes:
+// the grid samples albums at random, so rebuilding after a mere rename would change the cover.
+func (r *playlistRepository) enqueueCoverRebuild(ctx context.Context, id string) {
+	item := model.ArtworkQueueItem{ItemKind: model.KindPlaylistArtwork.Prefix(), ItemID: id,
+		ImageType: model.ImageTypePrimary, Priority: model.ArtworkPriorityScan}
+	if err := NewArtworkQueueRepository(r.db).Enqueue(ctx, item); err != nil {
+		log.Warn(ctx, "could not enqueue playlist artwork after content change", "id", id, err)
+	}
+}
+
+// tracksQuery is shared by loadTracks and GetCursor, so both hydrate rows identically.
+func (r *playlistRepository) tracksQuery(ctx context.Context, query SelectBuilder, id string) SelectBuilder {
+	query = r.applyLibraryFilter(ctx, query, "f")
+	userID := loggedUser(ctx).ID
+	return query.
+		Columns(
+			"coalesce(starred, 0) as starred",
+			"starred_at",
+			"coalesce(play_count, 0) as play_count",
+			"play_date",
+			"coalesce(rating, 0) as rating",
+			"rated_at",
+			"f.*",
+			"playlist_tracks.*",
+			"library.path as library_path",
+			"library.name as library_name",
+		).
+		LeftJoin("annotation on (" +
+			"annotation.item_id = media_file_id" +
+			" AND annotation.item_type = 'media_file'" +
+			" AND annotation.user_id = '" + userID + "')").
+		Join("media_file f on f.id = media_file_id").
+		Join("library on f.library_id = library.id").
+		Where(Eq{"playlist_id": id})
+}
+
+func (r *playlistRepository) loadTracks(ctx context.Context, query SelectBuilder, id string) (model.PlaylistTracks, error) {
+	tracks := dbPlaylistTracks{}
+	err := r.queryAll(ctx, r.tracksQuery(ctx, query, id), &tracks)
+	if err != nil {
+		return nil, err
+	}
+	res := tracks.toModels()
+	hydratePlaylistTrackArtwork(ctx, r.db, res)
+	return res, err
+}
+
+func (r *playlistRepository) Count(ctx context.Context, options ...rest.QueryOptions) (int64, error) {
+	return r.CountAll(ctx, r.parseRestOptions(ctx, options...))
+}
+
+func (r *playlistRepository) Read(ctx context.Context, id string) (*model.Playlist, error) {
+	return r.Get(ctx, id)
+}
+
+func (r *playlistRepository) ReadAll(ctx context.Context, options ...rest.QueryOptions) ([]model.Playlist, error) {
+	return r.GetAll(ctx, r.parseRestOptions(ctx, options...))
+}
+
+func (r *playlistRepository) Save(ctx context.Context, pls *model.Playlist) (string, error) {
+	pls.ID = "" // Force new creation
+	err := r.Put(ctx, pls)
+	if err != nil {
+		return "", err
+	}
+	return pls.ID, err
+}
+
+func (r *playlistRepository) Update(ctx context.Context, id string, entity model.Playlist, cols ...string) error {
+	pls := dbPlaylist{Playlist: entity}
+	pls.ID = id
+	pls.UpdatedAt = time.Now()
+	_, err := r.put(ctx, id, pls, append(cols, "updatedAt")...)
+	return err
+}
+
+func (r *playlistRepository) removeOrphans(ctx context.Context) error {
+	sel := Select("playlist_tracks.playlist_id as id", "p.name").From("playlist_tracks").
+		Join("playlist p on playlist_tracks.playlist_id = p.id").
+		LeftJoin("media_file mf on playlist_tracks.media_file_id = mf.id").
+		Where(Eq{"mf.id": nil}).
+		GroupBy("playlist_tracks.playlist_id")
+
+	var pls []struct{ Id, Name string }
+	err := r.queryAll(ctx, sel, &pls)
+	if err != nil {
+		return fmt.Errorf("fetching playlists with orphan tracks: %w", err)
+	}
+
+	for _, pl := range pls {
+		log.Debug(ctx, "Cleaning-up orphan tracks from playlist", "id", pl.Id, "name", pl.Name)
+		del := Delete("playlist_tracks").Where(And{
+			ConcatExpr("media_file_id not in (select id from media_file)"),
+			Eq{"playlist_id": pl.Id},
+		})
+		n, err := r.executeSQL(ctx, del)
+		if n == 0 || err != nil {
+			return fmt.Errorf("deleting orphan tracks from playlist %s: %w", pl.Name, err)
+		}
+		log.Debug(ctx, "Deleted tracks, now reordering", "id", pl.Id, "name", pl.Name, "deleted", n)
+
+		// Renumber the playlist if any track was removed
+		if err := r.renumber(ctx, pl.Id); err != nil {
+			return fmt.Errorf("renumbering playlist %s: %w", pl.Name, err)
+		}
+	}
+	return nil
+}
+
+// renumber updates the position of all tracks in the playlist to be sequential starting from 1, ordered by their
+// current position. This is needed after removing orphan tracks, to ensure there are no gaps in the track numbering.
+// The two-step approach (negate then reassign via CTE) avoids UNIQUE constraint violations on (playlist_id, id).
+func (r *playlistRepository) renumber(ctx context.Context, id string) error {
+	// Step 1: Negate all IDs to clear the positive ID space
+	_, err := r.executeSQL(ctx, Expr(
+		`UPDATE playlist_tracks SET id = -id WHERE playlist_id = ? AND id > 0`, id))
+	if err != nil {
+		return err
+	}
+	// Step 2: Assign new sequential positive IDs using UPDATE...FROM with a CTE.
+	// The CTE is fully materialized before the UPDATE begins, avoiding self-referencing issues.
+	// ORDER BY id DESC restores original order since IDs are now negative.
+	_, err = r.executeSQL(ctx, Expr(
+		`WITH new_ids AS (
+			SELECT rowid as rid, ROW_NUMBER() OVER (ORDER BY id DESC) as new_id
+			FROM playlist_tracks WHERE playlist_id = ?
+		)
+		UPDATE playlist_tracks SET id = new_ids.new_id
+		FROM new_ids
+		WHERE playlist_tracks.rowid = new_ids.rid AND playlist_tracks.playlist_id = ?`, id, id))
+	if err != nil {
+		return err
+	}
+	r.enqueueCoverRebuild(ctx, id)
+	return r.refreshCounters(ctx, &model.Playlist{ID: id})
+}
+
+var _ model.PlaylistRepository = (*playlistRepository)(nil)
+var _ rest.Repository[model.Playlist] = (*playlistRepository)(nil)
+var _ rest.Persistable[model.Playlist] = (*playlistRepository)(nil)

persistence/smart_playlist_repository_test.go

@@ -0,0 +1,810 @@
+package persistence
+
+import (
+	"context"
+	"path/filepath"
+	"time"
+
+	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/conf/configtest"
+	"github.com/navidrome/navidrome/log"
+	"github.com/navidrome/navidrome/model"
+	"github.com/navidrome/navidrome/model/criteria"
+	"github.com/navidrome/navidrome/model/request"
+	"github.com/navidrome/navidrome/utils/slice"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/pocketbase/dbx"
+)
+
+var _ = Describe("PlaylistRepository - Smart Playlists", func() {
+	var repo model.PlaylistRepository
+	var ctx context.Context
+
+	BeforeEach(func() {
+		ctx = request.WithUser(log.NewContext(GinkgoT().Context()), model.User{ID: "userid", UserName: "userid", IsAdmin: true})
+		repo = NewPlaylistRepository(GetDBXBuilder())
+	})
+
+	Context("Smart Playlists", func() {
+		var rules *criteria.Criteria
+		BeforeEach(func() {
+			rules = &criteria.Criteria{
+				Expression: criteria.All{
+					criteria.Contains{"title": "love"},
+				},
+			}
+		})
+		Context("valid rules", func() {
+			Specify("Put/Get", func() {
+				newPls := model.Playlist{Name: "Great!", OwnerID: "userid", Rules: rules}
+				Expect(repo.Put(ctx, &newPls)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(ctx, newPls.ID) })
+
+				savedPls, err := repo.Get(ctx, newPls.ID)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(savedPls.Rules).To(Equal(rules))
+			})
+		})
+
+		Context("after an evaluation", func() {
+			It("stamps updated_at and evaluated_at with the same instant", func() {
+				newPls := model.Playlist{Name: "Evaluated", OwnerID: "userid", Rules: rules}
+				Expect(repo.Put(ctx, &newPls)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(ctx, newPls.ID) })
+
+				refreshed, err := repo.GetWithTracks(ctx, newPls.ID, true, false)
+				Expect(err).ToNot(HaveOccurred())
+
+				stored, err := repo.Get(ctx, newPls.ID)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(stored.EvaluatedAt).ToNot(BeNil())
+				Expect(stored.UpdatedAt).To(BeTemporally("==", *stored.EvaluatedAt))
+				Expect(refreshed.UpdatedAt).To(BeTemporally("==", stored.UpdatedAt))
+			})
+		})
+
+		Context("invalid rules", func() {
+			It("fails to Put it in the DB", func() {
+				rules = &criteria.Criteria{
+					// This is invalid because "contains" cannot have multiple fields
+					Expression: criteria.All{
+						criteria.Contains{"genre": "Hardcore", "filetype": "mp3"},
+					},
+				}
+				newPls := model.Playlist{Name: "Great!", OwnerID: "userid", Rules: rules}
+				Expect(repo.Put(ctx, &newPls)).To(MatchError(ContainSubstring("invalid criteria expression")))
+			})
+		})
+
+		Context("re-imported from disk", func() {
+			// The scanner re-imports every playlist in a touched folder, and a freshly parsed
+			// .nsp carries no counters — saving it must not wipe the ones already evaluated.
+			It("keeps the stored counters when a freshly parsed playlist is saved over it", func() {
+				rules = &criteria.Criteria{
+					Expression: criteria.All{
+						criteria.Contains{"title": "Antenna"},
+					},
+				}
+				pls := model.Playlist{Name: "Smart", OwnerID: "userid", Rules: rules, Path: "/music/smart.nsp", Sync: true}
+				Expect(repo.Put(ctx, &pls)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(ctx, pls.ID) })
+
+				evaluated, err := repo.GetWithTracks(ctx, pls.ID, true, false)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(evaluated.SongCount).To(BeNumerically(">", 0))
+
+				stored, err := repo.Get(ctx, pls.ID)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(stored.SongCount).To(Equal(evaluated.SongCount))
+
+				reimported := model.Playlist{
+					ID: pls.ID, Name: pls.Name, OwnerID: "userid", Rules: rules,
+					Path: pls.Path, Sync: true,
+				}
+				Expect(repo.Put(ctx, &reimported)).To(Succeed())
+
+				afterImport, err := repo.Get(ctx, pls.ID)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(afterImport.SongCount).To(Equal(stored.SongCount))
+				Expect(afterImport.Duration).To(Equal(stored.Duration))
+				Expect(afterImport.Size).To(Equal(stored.Size))
+			})
+		})
+
+		Context("child smart playlists", func() {
+			BeforeEach(func() {
+				DeferCleanup(configtest.SetupConfig())
+			})
+
+			When("refresh delay has expired", func() {
+				It("should refresh tracks for smart playlist referenced in parent smart playlist criteria", func() {
+					conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+
+					childRules := &criteria.Criteria{
+						Expression: criteria.All{
+							criteria.Contains{"title": "Day"},
+						},
+					}
+					nestedPls := model.Playlist{Name: "Nested [ID]", OwnerID: "userid", Public: true, Rules: childRules}
+					Expect(repo.Put(ctx, &nestedPls)).To(Succeed())
+					DeferCleanup(func() { _ = repo.Delete(ctx, nestedPls.ID) })
+
+					childRules = &criteria.Criteria{
+						Expression: criteria.All{
+							criteria.Eq{"artist": "シートベルツ"},
+						},
+					}
+					nestedPathPls := model.Playlist{Name: "Nested [Path]", OwnerID: "userid", Path: "test.nsp", Public: true, Rules: childRules}
+					Expect(repo.Put(ctx, &nestedPathPls)).To(Succeed())
+					DeferCleanup(func() { _ = repo.Delete(ctx, nestedPathPls.ID) })
+
+					parentPls := model.Playlist{Name: "Parent", OwnerID: "userid", Rules: &criteria.Criteria{
+						Expression: criteria.Any{
+							criteria.InPlaylist{"id": nestedPls.ID},
+							criteria.InPlaylist{"path": nestedPathPls.Path},
+						},
+					}}
+					Expect(repo.Put(ctx, &parentPls)).To(Succeed())
+					DeferCleanup(func() { _ = repo.Delete(ctx, parentPls.ID) })
+
+					// Nested playlist has not been evaluated yet
+					nestedPlsRead, err := repo.Get(ctx, nestedPls.ID)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(nestedPlsRead.EvaluatedAt).To(BeNil())
+
+					// Getting parent with refresh should recursively refresh the nested playlist
+					pls, err := repo.GetWithTracks(ctx, parentPls.ID, true, false)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(pls.EvaluatedAt).ToNot(BeNil())
+					Expect(*pls.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
+
+					// Parent should have tracks from the nested playlist
+					Expect(pls.Tracks).To(HaveLen(2))
+					Expect(pls.Tracks[0].MediaFileID).To(Equal(songDayInALife.ID))
+
+					// Nested playlists should now have been refreshed (EvaluatedAt set)
+					nestedPlsAfterParentGet, err := repo.Get(ctx, nestedPls.ID)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(nestedPlsAfterParentGet.EvaluatedAt).ToNot(BeNil())
+					Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
+
+					nestedPlsAfterParentGet, err = repo.Get(ctx, nestedPathPls.ID)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(nestedPlsAfterParentGet.EvaluatedAt).ToNot(BeNil())
+					Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
+				})
+			})
+
+			It("does not recurse forever when two smart playlists reference each other", func() {
+				conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+
+				plsA := model.Playlist{Name: "Cycle A", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{
+					Expression: criteria.All{criteria.Contains{"title": "Day"}},
+				}}
+				Expect(repo.Put(ctx, &plsA)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(ctx, plsA.ID) })
+
+				plsB := model.Playlist{Name: "Cycle B", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{
+					Expression: criteria.All{criteria.InPlaylist{"id": plsA.ID}},
+				}}
+				Expect(repo.Put(ctx, &plsB)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(ctx, plsB.ID) })
+
+				plsA.Rules = &criteria.Criteria{Expression: criteria.All{criteria.InPlaylist{"id": plsB.ID}}}
+				Expect(repo.Put(ctx, &plsA)).To(Succeed())
+
+				_, err := repo.GetWithTracks(ctx, plsA.ID, true, false)
+				Expect(err).ToNot(HaveOccurred())
+			})
+
+			It("does not treat an empty path as a reference to every playlist without a path", func() {
+				conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+
+				bystander := model.Playlist{Name: "Bystander", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{
+					Expression: criteria.All{criteria.Contains{"title": "Day"}},
+				}}
+				Expect(repo.Put(ctx, &bystander)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(ctx, bystander.ID) })
+
+				parent := model.Playlist{Name: "Empty Path", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{
+					Expression: criteria.All{criteria.InPlaylist{"path": ""}},
+				}}
+				Expect(repo.Put(ctx, &parent)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(ctx, parent.ID) })
+
+				_, err := repo.GetWithTracks(ctx, parent.ID, true, false)
+				Expect(err).ToNot(HaveOccurred())
+
+				reloaded, err := repo.Get(ctx, bystander.ID)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reloaded.EvaluatedAt).To(BeNil())
+			})
+
+			It("matches a child path stored in a different Unicode normalization form", func() {
+				conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+
+				child := model.Playlist{Name: "NFD Child", OwnerID: "userid", Public: true, Path: filepath.FromSlash("/mu\u0301sica/child.nsp"), Rules: &criteria.Criteria{
+					Expression: criteria.All{criteria.Contains{"title": "Day"}},
+				}}
+				Expect(repo.Put(ctx, &child)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(ctx, child.ID) })
+
+				parent := model.Playlist{Name: "NFC Parent", OwnerID: "userid", Rules: &criteria.Criteria{
+					Expression: criteria.All{criteria.InPlaylist{"path": "/m\u00fasica/child.nsp"}},
+				}}
+				Expect(repo.Put(ctx, &parent)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(ctx, parent.ID) })
+
+				pls, err := repo.GetWithTracks(ctx, parent.ID, true, false)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(pls.Tracks).To(HaveLen(1))
+				Expect(pls.Tracks[0].MediaFileID).To(Equal(songDayInALife.ID))
+			})
+
+			When("refresh delay has not expired", func() {
+				It("should NOT refresh tracks for smart playlist referenced in parent smart playlist criteria", func() {
+					conf.Server.SmartPlaylistRefreshDelay = 1 * time.Hour
+					childEvaluatedAt := time.Now().Add(-30 * time.Minute)
+
+					childRules := &criteria.Criteria{
+						Expression: criteria.All{
+							criteria.Contains{"title": "Day"},
+						},
+					}
+					nestedPls := model.Playlist{Name: "Nested", OwnerID: "userid", Public: true, Rules: childRules, EvaluatedAt: &childEvaluatedAt}
+					Expect(repo.Put(ctx, &nestedPls)).To(Succeed())
+					DeferCleanup(func() { _ = repo.Delete(ctx, nestedPls.ID) })
+
+					// Parent has no EvaluatedAt, so it WILL refresh, but the child should not
+					parentPls := model.Playlist{Name: "Parent", OwnerID: "userid", Rules: &criteria.Criteria{
+						Expression: criteria.All{
+							criteria.InPlaylist{"id": nestedPls.ID},
+						},
+					}}
+					Expect(repo.Put(ctx, &parentPls)).To(Succeed())
+					DeferCleanup(func() { _ = repo.Delete(ctx, parentPls.ID) })
+
+					nestedPlsRead, err := repo.Get(ctx, nestedPls.ID)
+					Expect(err).ToNot(HaveOccurred())
+
+					// Getting parent with refresh should NOT recursively refresh the nested playlist
+					parent, err := repo.GetWithTracks(ctx, parentPls.ID, true, false)
+					Expect(err).ToNot(HaveOccurred())
+
+					// Parent should have been refreshed (its EvaluatedAt was nil)
+					Expect(parent.EvaluatedAt).ToNot(BeNil())
+					Expect(*parent.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
+
+					// Nested playlist should NOT have been refreshed (still within delay window)
+					nestedPlsAfterParentGet, err := repo.Get(ctx, nestedPls.ID)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(BeTemporally("~", childEvaluatedAt, time.Second))
+					Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(Equal(*nestedPlsRead.EvaluatedAt))
+				})
+			})
+
+			Context("per-playlist refreshDelay", func() {
+				BeforeEach(func() {
+					DeferCleanup(configtest.SetupConfig())
+				})
+
+				It("does NOT refresh when the per-playlist delay has not elapsed, even if global has", func() {
+					conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+					evaluatedAt := time.Now().Add(-1 * time.Hour)
+
+					rules := &criteria.Criteria{
+						Expression:   criteria.All{criteria.Contains{"title": "Day"}},
+						RefreshDelay: 24 * time.Hour,
+					}
+					pls := model.Playlist{Name: "Frozen Daily", OwnerID: "userid", Rules: rules, EvaluatedAt: &evaluatedAt}
+					Expect(repo.Put(ctx, &pls)).To(Succeed())
+					DeferCleanup(func() { _ = repo.Delete(ctx, pls.ID) })
+
+					got, err := repo.GetWithTracks(ctx, pls.ID, true, false)
+					Expect(err).ToNot(HaveOccurred())
+					// Not re-evaluated: EvaluatedAt unchanged, no tracks materialized
+					Expect(*got.EvaluatedAt).To(BeTemporally("~", evaluatedAt, time.Second))
+					Expect(got.Tracks).To(BeEmpty())
+				})
+
+				It("refreshes when the per-playlist delay has elapsed, even if global has not", func() {
+					conf.Server.SmartPlaylistRefreshDelay = 1 * time.Hour
+					evaluatedAt := time.Now().Add(-10 * time.Minute)
+
+					rules := &criteria.Criteria{
+						Expression:   criteria.All{criteria.Contains{"title": "Day"}},
+						RefreshDelay: 5 * time.Minute,
+					}
+					pls := model.Playlist{Name: "Fast Refresh", OwnerID: "userid", Rules: rules, EvaluatedAt: &evaluatedAt}
+					Expect(repo.Put(ctx, &pls)).To(Succeed())
+					DeferCleanup(func() { _ = repo.Delete(ctx, pls.ID) })
+
+					got, err := repo.GetWithTracks(ctx, pls.ID, true, false)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(*got.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
+					Expect(got.Tracks).To(HaveLen(1))
+					Expect(got.Tracks[0].MediaFileID).To(Equal(songDayInALife.ID))
+				})
+			})
+		})
+	})
+
+	Describe("Playlist Track Sorting", func() {
+		var testPlaylistID string
+
+		AfterEach(func() {
+			if testPlaylistID != "" {
+				Expect(repo.Delete(ctx, testPlaylistID)).To(BeNil())
+				testPlaylistID = ""
+			}
+		})
+
+		It("sorts tracks correctly by album (disc and track number)", func() {
+			By("creating a playlist with multi-disc album tracks in arbitrary order")
+			newPls := model.Playlist{Name: "Multi-Disc Test", OwnerID: "userid"}
+			// Add tracks in intentionally scrambled order
+			newPls.AddMediaFilesByID([]string{"2001", "2002", "2003", "2004"})
+			Expect(repo.Put(ctx, &newPls)).To(Succeed())
+			testPlaylistID = newPls.ID
+
+			By("retrieving tracks sorted by album")
+			tracksRepo := repo.Tracks(ctx, newPls.ID, false)
+			tracks, err := tracksRepo.GetAll(ctx, model.QueryOptions{Sort: "album", Order: "asc"})
+			Expect(err).ToNot(HaveOccurred())
+
+			By("verifying tracks are sorted by disc number then track number")
+			Expect(tracks).To(HaveLen(4))
+			// Expected order: Disc 1 Track 1, Disc 1 Track 2, Disc 2 Track 1, Disc 2 Track 11
+			Expect(tracks[0].MediaFileID).To(Equal("2002")) // Disc 1, Track 1
+			Expect(tracks[1].MediaFileID).To(Equal("2004")) // Disc 1, Track 2
+			Expect(tracks[2].MediaFileID).To(Equal("2003")) // Disc 2, Track 1
+			Expect(tracks[3].MediaFileID).To(Equal("2001")) // Disc 2, Track 11
+		})
+	})
+
+	Describe("Smart Playlists with Album/Artist Annotation Criteria", func() {
+		var testPlaylistID string
+
+		AfterEach(func() {
+			if testPlaylistID != "" {
+				_ = repo.Delete(ctx, testPlaylistID)
+				testPlaylistID = ""
+			}
+		})
+
+		It("matches tracks from starred albums using albumLoved", func() {
+			// albumRadioactivity (ID "103") is starred in test fixtures
+			// Songs in album 103: 1003, 1004, 1005, 1006
+			rules := &criteria.Criteria{
+				Expression: criteria.All{
+					criteria.Is{"albumLoved": true},
+				},
+			}
+			newPls := model.Playlist{Name: "Starred Album Songs", OwnerID: "userid", Rules: rules}
+			Expect(repo.Put(ctx, &newPls)).To(Succeed())
+			testPlaylistID = newPls.ID
+
+			conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+			pls, err := repo.GetWithTracks(ctx, newPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+
+			trackIDs := make([]string, len(pls.Tracks))
+			for i, t := range pls.Tracks {
+				trackIDs[i] = t.MediaFileID
+			}
+			Expect(trackIDs).To(ConsistOf("1003", "1004", "1005", "1006"))
+		})
+
+		It("matches tracks from starred artists using artistLoved", func() {
+			// artistBeatles (ID "3") is starred in test fixtures
+			// Songs with ArtistID "3": 1001, 1002, 3002
+			rules := &criteria.Criteria{
+				Expression: criteria.All{
+					criteria.Is{"artistLoved": true},
+				},
+			}
+			newPls := model.Playlist{Name: "Starred Artist Songs", OwnerID: "userid", Rules: rules}
+			Expect(repo.Put(ctx, &newPls)).To(Succeed())
+			testPlaylistID = newPls.ID
+
+			conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+			pls, err := repo.GetWithTracks(ctx, newPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+
+			trackIDs := make([]string, len(pls.Tracks))
+			for i, t := range pls.Tracks {
+				trackIDs[i] = t.MediaFileID
+			}
+			Expect(trackIDs).To(ConsistOf("1001", "1002", "3002"))
+		})
+
+		It("matches tracks with combined album and artist criteria", func() {
+			// albumLoved=true → songs from album 103 (1003, 1004, 1005, 1006)
+			// artistLoved=true → songs with artist 3 (1001, 1002)
+			// Using Any: union of both sets
+			rules := &criteria.Criteria{
+				Expression: criteria.Any{
+					criteria.Is{"albumLoved": true},
+					criteria.Is{"artistLoved": true},
+				},
+			}
+			newPls := model.Playlist{Name: "Combined Album+Artist", OwnerID: "userid", Rules: rules}
+			Expect(repo.Put(ctx, &newPls)).To(Succeed())
+			testPlaylistID = newPls.ID
+
+			conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+			pls, err := repo.GetWithTracks(ctx, newPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+
+			trackIDs := make([]string, len(pls.Tracks))
+			for i, t := range pls.Tracks {
+				trackIDs[i] = t.MediaFileID
+			}
+			Expect(trackIDs).To(ConsistOf("1001", "1002", "1003", "1004", "1005", "1006", "3002"))
+		})
+
+		It("returns no tracks when no albums/artists match", func() {
+			// No album has rating 5 in fixtures
+			rules := &criteria.Criteria{
+				Expression: criteria.All{
+					criteria.Is{"albumRating": 5},
+				},
+			}
+			newPls := model.Playlist{Name: "No Match", OwnerID: "userid", Rules: rules}
+			Expect(repo.Put(ctx, &newPls)).To(Succeed())
+			testPlaylistID = newPls.ID
+
+			conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+			pls, err := repo.GetWithTracks(ctx, newPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(pls.Tracks).To(BeEmpty())
+		})
+
+		It("matches loved tracks when loved value is a string in nested group (issue #4826)", func() {
+			// songComeTogether (ID "1002") is starred in test fixtures
+			rules := &criteria.Criteria{
+				Expression: criteria.All{
+					criteria.Any{
+						criteria.Is{"loved": "true"},
+					},
+				},
+			}
+			newPls := model.Playlist{Name: "String Loved Nested", OwnerID: "userid", Rules: rules}
+			Expect(repo.Put(ctx, &newPls)).To(Succeed())
+			testPlaylistID = newPls.ID
+
+			conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+			pls, err := repo.GetWithTracks(ctx, newPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+
+			trackIDs := make([]string, len(pls.Tracks))
+			for i, t := range pls.Tracks {
+				trackIDs[i] = t.MediaFileID
+			}
+			Expect(trackIDs).To(ContainElement("1002"))
+			Expect(len(pls.Tracks)).To(BeNumerically(">=", 1))
+		})
+
+		It("returns same results for string and bool loved values (issue #4826)", func() {
+			boolRules := &criteria.Criteria{
+				Expression: criteria.All{
+					criteria.Any{
+						criteria.Is{"loved": true},
+					},
+				},
+			}
+			boolPls := model.Playlist{Name: "Bool Loved", OwnerID: "userid", Rules: boolRules}
+			Expect(repo.Put(ctx, &boolPls)).To(Succeed())
+			DeferCleanup(func() { _ = repo.Delete(ctx, boolPls.ID) })
+
+			stringRules := &criteria.Criteria{
+				Expression: criteria.All{
+					criteria.Any{
+						criteria.Is{"loved": "true"},
+					},
+				},
+			}
+			stringPls := model.Playlist{Name: "String Loved", OwnerID: "userid", Rules: stringRules}
+			Expect(repo.Put(ctx, &stringPls)).To(Succeed())
+			testPlaylistID = stringPls.ID
+
+			conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+			boolResult, err := repo.GetWithTracks(ctx, boolPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+			stringResult, err := repo.GetWithTracks(ctx, stringPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+
+			boolIDs := make([]string, len(boolResult.Tracks))
+			for i, t := range boolResult.Tracks {
+				boolIDs[i] = t.MediaFileID
+			}
+			stringIDs := make([]string, len(stringResult.Tracks))
+			for i, t := range stringResult.Tracks {
+				stringIDs[i] = t.MediaFileID
+			}
+			Expect(stringIDs).To(ConsistOf(boolIDs))
+		})
+	})
+
+	Describe("Smart Playlists with Album Aggregate Criteria", func() {
+		BeforeEach(func() {
+			DeferCleanup(configtest.SetupConfig())
+			conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+		})
+
+		trackIDsOf := func(rules *criteria.Criteria) []string {
+			newPls := model.Playlist{Name: "Album Aggregates", OwnerID: "userid", Rules: rules}
+			Expect(repo.Put(ctx, &newPls)).To(Succeed())
+			DeferCleanup(func() { _ = repo.Delete(ctx, newPls.ID) })
+
+			pls, err := repo.GetWithTracks(ctx, newPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+			return slice.Map(pls.Tracks, func(t model.PlaylistTrack) string { return t.MediaFileID })
+		}
+
+		It("filters on albumSongCount", func() {
+			// albumMultiDisc (ID "104") is the only fixture album with SongCount > 3
+			rules := &criteria.Criteria{Expression: criteria.All{criteria.Gt{"albumSongCount": 3}}}
+
+			Expect(trackIDsOf(rules)).To(ConsistOf("2001", "2002", "2003", "2004"))
+		})
+
+		It("sorts by an album field not referenced in the expression (issue #5347)", func() {
+			// All four tracks share album 104, so the album date ties and disc/track number decide.
+			rules := &criteria.Criteria{
+				Expression: criteria.All{criteria.Is{"album": "Multi Disc Album"}},
+				Sort:       "-albumDateAdded,discNumber,trackNumber",
+			}
+
+			Expect(trackIDsOf(rules)).To(HaveExactElements("2002", "2004", "2003", "2001"))
+		})
+	})
+
+	Describe("Smart Playlists with Tag Criteria", func() {
+		var mfRepo model.MediaFileRepository
+		var testPlaylistID string
+		var songWithGrouping, songWithoutGrouping model.MediaFile
+
+		BeforeEach(func() {
+			ctx := log.NewContext(GinkgoT().Context())
+			ctx = request.WithUser(ctx, model.User{ID: "userid", UserName: "userid", IsAdmin: true})
+			mfRepo = NewMediaFileRepository(GetDBXBuilder())
+
+			// Register 'grouping' as a valid tag for smart playlists
+			criteria.AddTagNames([]string{"grouping"})
+
+			// Create a song with the grouping tag
+			songWithGrouping = model.MediaFile{
+				ID:       "test-grouping-1",
+				Title:    "Song With Grouping",
+				Artist:   "Test Artist",
+				ArtistID: "1",
+				Album:    "Test Album",
+				AlbumID:  "101",
+				Path:     "test/grouping/song1.mp3",
+				Tags: model.Tags{
+					"grouping": []string{"My Crate"},
+				},
+				Participants: model.Participants{},
+				LibraryID:    1,
+				Lyrics:       "[]",
+			}
+			Expect(mfRepo.Put(ctx, &songWithGrouping)).To(Succeed())
+
+			// Create a song without the grouping tag
+			songWithoutGrouping = model.MediaFile{
+				ID:           "test-grouping-2",
+				Title:        "Song Without Grouping",
+				Artist:       "Test Artist",
+				ArtistID:     "1",
+				Album:        "Test Album",
+				AlbumID:      "101",
+				Path:         "test/grouping/song2.mp3",
+				Tags:         model.Tags{},
+				Participants: model.Participants{},
+				LibraryID:    1,
+				Lyrics:       "[]",
+			}
+			Expect(mfRepo.Put(ctx, &songWithoutGrouping)).To(Succeed())
+		})
+
+		AfterEach(func() {
+			if testPlaylistID != "" {
+				_ = repo.Delete(ctx, testPlaylistID)
+				testPlaylistID = ""
+			}
+			// Clean up test media files
+			_, _ = GetDBXBuilder().Delete("media_file", dbx.HashExp{"id": "test-grouping-1"}).Execute()
+			_, _ = GetDBXBuilder().Delete("media_file", dbx.HashExp{"id": "test-grouping-2"}).Execute()
+		})
+
+		It("matches tracks with a tag value using 'contains' with empty string (issue #4728 workaround)", func() {
+			By("creating a smart playlist that checks if grouping tag has any value")
+			// This is the workaround for issue #4728: using 'contains' with empty string
+			// generates SQL: value LIKE '%%' which matches any non-empty string
+			rules := &criteria.Criteria{
+				Expression: criteria.All{
+					criteria.Contains{"grouping": ""},
+				},
+			}
+			newPls := model.Playlist{Name: "Tracks with Grouping", OwnerID: "userid", Rules: rules}
+			Expect(repo.Put(ctx, &newPls)).To(Succeed())
+			testPlaylistID = newPls.ID
+
+			By("refreshing the smart playlist")
+			conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second // Force refresh
+			pls, err := repo.GetWithTracks(ctx, newPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("verifying only the track with grouping tag is matched")
+			Expect(pls.Tracks).To(HaveLen(1))
+			Expect(pls.Tracks[0].MediaFileID).To(Equal(songWithGrouping.ID))
+		})
+
+		It("excludes tracks with a tag value using 'notContains' with empty string", func() {
+			By("creating a smart playlist that checks if grouping tag is NOT set")
+			rules := &criteria.Criteria{
+				Expression: criteria.All{
+					criteria.NotContains{"grouping": ""},
+				},
+			}
+			newPls := model.Playlist{Name: "Tracks without Grouping", OwnerID: "userid", Rules: rules}
+			Expect(repo.Put(ctx, &newPls)).To(Succeed())
+			testPlaylistID = newPls.ID
+
+			By("refreshing the smart playlist")
+			conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second // Force refresh
+			pls, err := repo.GetWithTracks(ctx, newPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("verifying the track with grouping is NOT in the playlist")
+			for _, track := range pls.Tracks {
+				Expect(track.MediaFileID).ToNot(Equal(songWithGrouping.ID))
+			}
+
+			By("verifying the track without grouping IS in the playlist")
+			var foundWithoutGrouping bool
+			for _, track := range pls.Tracks {
+				if track.MediaFileID == songWithoutGrouping.ID {
+					foundWithoutGrouping = true
+					break
+				}
+			}
+			Expect(foundWithoutGrouping).To(BeTrue())
+		})
+	})
+
+	Describe("Smart Playlists Library Filtering", func() {
+		var mfRepo model.MediaFileRepository
+		var testPlaylistID string
+		var lib2ID int
+		var restrictedUserID string
+		var uniqueLibPath string
+
+		BeforeEach(func() {
+			db := GetDBXBuilder()
+
+			// Generate unique IDs for this test run
+			uniqueSuffix := time.Now().Format("20060102150405.000")
+			restrictedUserID = "restricted-user-" + uniqueSuffix
+			uniqueLibPath = "/music/lib2-" + uniqueSuffix
+
+			// Create a second library with unique name and path to avoid conflicts with other tests
+			_, err := db.DB().Exec("INSERT INTO library (name, path, created_at, updated_at) VALUES (?, ?, datetime('now'), datetime('now'))", "Library 2-"+uniqueSuffix, uniqueLibPath)
+			Expect(err).ToNot(HaveOccurred())
+			err = db.DB().QueryRow("SELECT last_insert_rowid()").Scan(&lib2ID)
+			Expect(err).ToNot(HaveOccurred())
+
+			// Create a restricted user with access only to library 1
+			_, err = db.DB().Exec("INSERT INTO user (id, user_name, name, is_admin, password, created_at, updated_at) VALUES (?, ?, 'Restricted User', false, 'pass', datetime('now'), datetime('now'))", restrictedUserID, restrictedUserID)
+			Expect(err).ToNot(HaveOccurred())
+			_, err = db.DB().Exec("INSERT INTO user_library (user_id, library_id) VALUES (?, 1)", restrictedUserID)
+			Expect(err).ToNot(HaveOccurred())
+
+			// Create test media files in each library
+			ctx := log.NewContext(GinkgoT().Context())
+			ctx = request.WithUser(ctx, model.User{ID: "userid", UserName: "userid", IsAdmin: true})
+			mfRepo = NewMediaFileRepository(db)
+
+			// Song in library 1 (accessible by restricted user)
+			songLib1 := model.MediaFile{
+				ID:           "lib1-song",
+				Title:        "Song in Lib1",
+				Artist:       "Test Artist",
+				ArtistID:     "1",
+				Album:        "Test Album",
+				AlbumID:      "101",
+				Path:         "lib1/song.mp3",
+				LibraryID:    1,
+				Participants: model.Participants{},
+				Tags:         model.Tags{},
+				Lyrics:       "[]",
+			}
+			Expect(mfRepo.Put(ctx, &songLib1)).To(Succeed())
+
+			// Song in library 2 (NOT accessible by restricted user)
+			songLib2 := model.MediaFile{
+				ID:           "lib2-song",
+				Title:        "Song in Lib2",
+				Artist:       "Test Artist",
+				ArtistID:     "1",
+				Album:        "Test Album",
+				AlbumID:      "101",
+				Path:         "lib2/song.mp3",
+				LibraryID:    lib2ID,
+				Participants: model.Participants{},
+				Tags:         model.Tags{},
+				Lyrics:       "[]",
+			}
+			Expect(mfRepo.Put(ctx, &songLib2)).To(Succeed())
+		})
+
+		AfterEach(func() {
+			db := GetDBXBuilder()
+			if testPlaylistID != "" {
+				_ = repo.Delete(ctx, testPlaylistID)
+				testPlaylistID = ""
+			}
+			// Clean up test data
+			_, _ = db.Delete("media_file", dbx.HashExp{"id": "lib1-song"}).Execute()
+			_, _ = db.Delete("media_file", dbx.HashExp{"id": "lib2-song"}).Execute()
+			_, _ = db.Delete("user_library", dbx.HashExp{"user_id": restrictedUserID}).Execute()
+			_, _ = db.Delete("user", dbx.HashExp{"id": restrictedUserID}).Execute()
+			_, _ = db.DB().Exec("DELETE FROM library WHERE id = ?", lib2ID)
+		})
+
+		It("should only include tracks from libraries the user has access to (issue #4738)", func() {
+			db := GetDBXBuilder()
+			ctx := log.NewContext(GinkgoT().Context())
+
+			// Create the smart playlist as the restricted user
+			restrictedUser := model.User{ID: restrictedUserID, UserName: restrictedUserID, IsAdmin: false}
+			ctx = request.WithUser(ctx, restrictedUser)
+			restrictedRepo := NewPlaylistRepository(db)
+
+			// Create a smart playlist that matches all songs
+			rules := &criteria.Criteria{
+				Expression: criteria.All{
+					criteria.Gt{"playCount": -1}, // Matches everything
+				},
+			}
+			newPls := model.Playlist{Name: "All Songs", OwnerID: restrictedUserID, Rules: rules}
+			Expect(restrictedRepo.Put(ctx, &newPls)).To(Succeed())
+			testPlaylistID = newPls.ID
+
+			By("refreshing the smart playlist")
+			conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second // Force refresh
+			pls, err := restrictedRepo.GetWithTracks(ctx, newPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("verifying only the track from library 1 is in the playlist")
+			var foundLib1Song, foundLib2Song bool
+			for _, track := range pls.Tracks {
+				if track.MediaFileID == "lib1-song" {
+					foundLib1Song = true
+				}
+				if track.MediaFileID == "lib2-song" {
+					foundLib2Song = true
+				}
+			}
+			Expect(foundLib1Song).To(BeTrue(), "Song from library 1 should be in the playlist")
+			Expect(foundLib2Song).To(BeFalse(), "Song from library 2 should NOT be in the playlist")
+
+			By("verifying playlist_tracks table only contains the accessible track")
+			var playlistTracksCount int
+			err = db.DB().QueryRow("SELECT count(*) FROM playlist_tracks WHERE playlist_id = ?", newPls.ID).Scan(&playlistTracksCount)
+			Expect(err).ToNot(HaveOccurred())
+			// Count should only include tracks visible to the user (lib1-song)
+			// The count may include other test songs from library 1, but NOT lib2-song
+			var lib2TrackCount int
+			err = db.DB().QueryRow("SELECT count(*) FROM playlist_tracks WHERE playlist_id = ? AND media_file_id = 'lib2-song'", newPls.ID).Scan(&lib2TrackCount)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(lib2TrackCount).To(Equal(0), "lib2-song should not be in playlist_tracks")
+
+			By("verifying SongCount matches visible tracks")
+			Expect(pls.SongCount).To(Equal(len(pls.Tracks)), "SongCount should match the number of visible tracks")
+		})
+	})
+})