perf(subsonic): speed up artist search3 deep-offset pagination (#5620)

* perf(subsonic): speed up artist search3 deep-offset pagination Empty-query and FTS artist search (search3/search2) paginated via a CROSS JOIN library_artist + DISTINCT in Phase 1 purely for library access control. The DISTINCT forced a temp b-tree over the whole junction table on every page, making deep offsets O(offset): ~200ms at offset 299k on 300k artists. Replace it with a join-free EXISTS predicate keyed on artist.id, backed by a new covering index on library_artist(artist_id, library_id). EXISTS keeps artist as the ordered driver and never fans out rowids, so Phase 1 stays a plain ordered scan that LIMIT/OFFSET can short-circuit. Admin, headless, and all-libraries users skip the filter entirely (the dominant case) for a flat ordered walk over the primary key. Measured on a 300k-artist / 1M-song library: admin/all-libs pagination is ~4.5-5.4x faster at depth (~180ms to ~33ms at offset 400k); restricted subset users keep correct, gap-free pages while also getting faster. The narrowing artist filter is applied at the subsonic layer only when the request targets a strict subset of the user's libraries, so the common case (and the admin fast-path) is never burdened with a redundant predicate. * fix(subsonic): narrow artist search by library set, not count narrowsArtistLibraries decided whether to add the subsonic-layer artist narrowing filter by comparing len(requested) < len(accessible). musicFolderId is not deduplicated, so duplicate IDs inflated the requested count: a user requesting ?musicFolderId=1&musicFolderId=1&musicFolderId=2 against three accessible libraries produced len([1,1,2])==3, which is not < 3, so the filter was skipped and the user saw artists from the third library too. Compare as set membership instead: the request narrows iff some accessible library is absent from it (requested is always a subset of accessible, validated upstream by selectedMusicFolderIds). This is immune to duplicate IDs. Add a regression test that fails against the old length-based check. Also consolidate the repeated EXISTS/no-DISTINCT/O(page) rationale that the prior commit spread across five sites down to a single authoritative comment on ArtistLibraryFilter, with the call sites referencing it. * perf(subsonic): drop redundant library_artist covering index The migration added an index on library_artist(artist_id, library_id) on the theory that the restricted-subset artist-search EXISTS needed it to seek by artist_id. Benchmarking on a 405k-artist / 5-library dataset showed no benefit: the EXISTS subquery constrains both columns (artist_id = and library_id IN), so SQLite already resolves it as a covering-index seek on the existing (library_id, artist_id) UNIQUE autoindex. With the new index present the planner still picks the autoindex and ignores it. Drop the migration and correct the comment. Removing ~11MB of dead index plus its write-amplification on every library_artist insert/delete, for zero query gain. * fix(scanner): mark artists missing when they lose their last library Artist search Phase 1 filters on artist.missing and Phase 2 inner-joins library_artist, so a non-missing artist with no library_artist row (an orphan) takes a pagination slot in Phase 1 and then vanishes in Phase 2, shortening the page and shifting deep offsets. The admin/headless search fast-path walks artist unfiltered, so it is fully exposed to this. Two paths created such orphans without updating artist.missing: - RefreshStats deletes library_artist rows whose stats are '{}' (artist lost all content in a library) after every scan. This is the common source. - Library deletion cascades away the library's library_artist rows. Mark newly-orphaned artists missing at both sources, so the shared 'missing = false' search filter excludes them immediately instead of waiting for a later scan. In RefreshStats the update only runs when the cleanup actually removed rows (the only way a new orphan can appear), so steady-state scans pay nothing; measured ~160ms on 300k artists only when orphans can exist. * refactor(subsonic): address review feedback on artist search filter Code-review follow-ups to the artist search pagination change: - ArtistLibraryFilter: short-circuit to a constant-false predicate when no library IDs are given, avoiding a degenerate empty IN () subquery. - ArtistLibraryFilter: add an inner LIMIT 1 to the correlated EXISTS so SQLite cannot flatten it into a fan-out join (an artist in multiple of the user's libraries would otherwise yield duplicate rowids and corrupt pagination). - narrowsArtistLibraries: compare accessible-vs-requested as a set lookup instead of slices.Contains in a loop. - searchConfig.LibraryFilter: document that a join-free filter is now a correctness requirement (DISTINCT was removed), not just a performance one. * docs: trim verbose comments in artist search/orphan code Condense the over-explained comments added in this PR to the essential 'why', removing repeated cross-references and restatements of the adjacent code. * fix(scanner): heal pre-existing orphan artists on full refresh The orphan-marking added to RefreshStats only ran when its empty-stats cleanup deleted rows, so it reconciled newly-created orphans but not ones already left in the database by older versions (whose library_artist row was deleted before this fix existed). Such legacy orphans would surface in the admin/headless search fast-path as short/gappy pages. Also run the orphan-marking on a full refresh (allArtists), so a full scan — which upgrades commonly trigger and users can run manually — reconciles the backlog. No migration needed; the runtime fixes prevent recurrence. * perf(subsonic): extend artist search fast-path to all-library users applyLibraryFilterToSearchQuery only skipped the library filter for admin and headless processes. A regular (non-admin) user who can access every library has the same result set as an admin, but was still given the EXISTS filter — an O(offset) cost for a predicate that matches every non-missing artist anyway. Skip the filter for them too, using a cheap library CountAll() (a count over the tiny library table) compared against the user's library count. On any error it falls back to the filtered path, which is correct, just slower. * fix(scanner): log error as trailing arg, not explicit error key Signed-off-by: Deluan <deluan@navidrome.org> * test(scanner): e2e guard for orphan artists under PurgeMissing Adds an end-to-end scanner test for the orphan-artist invariant fixed in RefreshStats: with Scanner.PurgeMissing enabled, removing all of an artist's files hard-deletes them, cascades away their media_file_artists rows, and RefreshStats then drops the artist's emptied library_artist row. The test asserts no non-missing artist is left without a library_artist row. Verified it fails without the RefreshStats orphan-marking and passes with it. * test(scanner): assert the orphaned artist is marked missing The orphan e2e test only checked the aggregate no-orphan invariant (orphanCount == 0), which a fully-deleted artist or an un-cleaned row would also satisfy — so it could pass without exercising the fix. Assert Pink Floyd's row specifically: missing=false before, missing=true after, and absent from the non-missing results. Verified it fails without the RefreshStats orphan-marking. * test(scanner): drop misleading non-missing-list assertion for orphan GetAll has no default missing filter, but selectArtist inner-joins library_artist, so an orphaned artist (no junction row) is excluded from the results whether or not it is marked missing. The Not(ContainElement) check therefore passed for the wrong reason. The direct floydMissing() == 1 query is the assertion that actually validates the missing flag; keep that plus the orphan-count invariant and an over-marking guard on The Beatles. * test(scanner): document why orphan check reads the artist row directly Clarify that GetAll cannot observe the orphan: selectArtist inner-joins library_artist, so an artist with no junction row is excluded from results whether or not it is marked missing. Asserting on GetAll would pass even without the fix, so the test reads the artist row directly to check the missing flag. * test(scanner): return descriptive artist state for clearer failures floydState returns PRESENT/MISSING/NOT_FOUND instead of 0/1/-1, so a failure reads '<string>: PRESENT to equal MISSING' rather than '0 to equal 1'. * refactor(subsonic): keep artist library scoping in the repository The search endpoint built a persistence-layer EXISTS predicate (persistence.ArtistLibraryFilter) and injected it into artistOpts.Filters — the only place the subsonic package reached into persistence, leaking a storage detail up two layers. Pass the same Eq{"library_id": ids} filter used for albums and songs, and let the artist repository translate it to the join-free library_artist predicate (scopeSearchToLibraries), where the junction knowledge belongs. The subset-vs- fast-path decision moves there too, so narrowsArtistLibraries and the persistence import are gone from the subsonic layer. Behavior is unchanged; coverage for the translation moves to artist_repository_test. * refactor(persistence): extract canonical markOrphansMissing helper The 'mark non-missing artists with no library_artist row as missing' invariant was hand-written as SQL in two places (RefreshStats and libraryRepository.Delete), in two slightly different dialects (not exists vs id not in). Extract a single artistRepository.markOrphansMissing method next to markMissing and call it from both sites, so the invariant has one definition. * fix(persistence): apply scoped library filter in both search phases Two bugs from moving the artist library-scoping into the repository: - Search() scoped opts.Filters for Phase 1 but still passed the original (unscoped) options to selectArtist, so Phase 2 re-applied the raw Eq{library_id} against the wrong columns and a restricted user's search returned nothing. Pass the scoped opts to both phases. - scopeSearchToLibraries dropped the filter unconditionally for admins, so an admin explicitly narrowing via musicFolderId (e.g. search3?musicFolderId=2) leaked content from other libraries. Compare the request against the user's visible library set (all libraries for admin/headless), narrowing whenever it is a strict subset. Both regressions were caught by the server/e2e multi-library suite. * fix(core): delete library and reconcile orphans in one transaction libraryRepository.Delete runs the FK-cascade delete and the orphaned-artist reconciliation (markOrphansMissing) as two writes on r.db. Called directly they autocommit separately, so an interruption between them could leave non-missing artists with no library_artist row — the orphan state the artist search fast-path forbids. Wrap the deletion in ds.WithTx at the core wrapper so both writes commit atomically; the watcher/scanner/broker side-effects stay post-commit. * refactor(persistence): unify artist search library scoping into one filter Phase 1 previously applied two overlapping library predicates: cfg.LibraryFilter (scoped to the user's libraries) AND options.Filters (the requested subset), producing two correlated EXISTS subqueries per rowid even though the request is always a subset of the user's libraries. And the 'does this user see everything' decision was implemented twice (userHasAllLibraries via CountAll vs scopeSearchToLibraries via set-membership), with applyLibraryFilterToSearchQuery as a third scoping path. Resolve the effective library scope once in Search() via searchScope (intersect the requested set with the user's visible libraries; nil = fast-path), clear opts.Filters, and realize that single scope as the only Phase-1 LibraryFilter. The visibility logic is now one pipeline: requestedLibraryIDs + visibleLibraryIDs + userSeesAllLibraries. Behavior unchanged; one EXISTS instead of two on the hot path, one source of truth for library visibility. * fix(persistence): harden artist search against malformed library_id filter Search consumed only an Eq{"library_id": []int} filter; an Eq whose library_id value wasn't []int slipped through unconsumed and would reach Phase 1's bare artist table (no library_id column) → SQL error. Recognize any Eq carrying a library_id key (isLibraryIDFilter) and always consume it, falling back to the user's visible scope for a malformed value. Non-library filters are still left in place for doSearch. * refactor(persistence): trim redundant comments and unexport artist library filter The artist-search-pagination work left dense explanatory comments, with the join-free / LIMIT-1 anti-flatten rationale and the orphan-artist mechanics each restated in several places. Consolidate each rationale into one canonical home (artistLibraryFilter for the EXISTS/LIMIT-1 trick, markOrphansMissing for the orphan lifecycle) and have the other sites reference it instead of repeating it. Also unexport ArtistLibraryFilter to artistLibraryFilter: its only caller is searchCfg in the same package and no test references it, so it never needed to be part of the package's exported surface. Comments only plus the rename; no behavior change. * refactor: add slice.ToSet and use it for the artist search subset check searchScope's subset test compared the requested libraries against the visible set with a nested slices.Contains, which is O(visible * requested). On an instance with many libraries (e.g. 100 libraries, a user granted 99) and an explicit musicFolderId request, that is ~9.8k comparisons; with a set it is ~200. Add a small reusable slice.ToSet helper (a slice -> map[T]struct{} set, collapsing duplicates) and use it to make the membership lookups O(1), restoring O(n+m) without the throwaway struct{}{} literal that an inline ToMap would need. No behavior change. * refactor(artist): move artistLibraryFilter to artist_repository Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>
2026-06-19 07:37:15 +00:00 · 2026-06-16 21:47:15 -04:00
parent f0625ff709
commit 838ceee26d
11 changed files with 515 additions and 55 deletions
@@ -253,7 +253,11 @@ func (r *libraryRepositoryWrapper) Delete(id string) error {
 		return r.mapError(err)
 	}

-	err = r.LibraryRepository.Delete(libID)
+	// Run the deletion in a transaction so the cascade delete and the orphaned-artist
+	// reconciliation it triggers (see libraryRepository.Delete) commit atomically.
+	err = r.ds.WithTx(func(tx model.DataStore) error {
+		return tx.Library(r.ctx).Delete(libID)
+	}, "delete library")
 	if err != nil {
 		return r.mapError(err)
 	}
@@ -353,6 +353,19 @@ func (r *artistRepository) purgeEmpty() error {
 	return nil
 }

+// markOrphansMissing flags as missing any non-missing artist with no library_artist row, keeping the
+// search fast-path's `missing = false` filter correct (see searchCfg). Called wherever such a row can
+// be dropped: RefreshStats cleanup and library deletion cascade.
+func (r *artistRepository) markOrphansMissing() error {
+	_, err := r.executeSQL(Expr(
+		"update artist set missing = true where missing = false " +
+			"and not exists (select 1 from library_artist where library_artist.artist_id = artist.id)"))
+	if err != nil {
+		return fmt.Errorf("marking orphaned artists missing: %w", err)
+	}
+	return nil
+}
+
 // markMissing marks artists as missing if all their albums are missing.
 func (r *artistRepository) markMissing() error {
 	q := Expr(`
@@ -527,42 +540,62 @@ func (r *artistRepository) RefreshStats(allArtists bool) (int64, error) {
 		totalRowsAffected += rowsAffected
 	}

-	// // Remove library_artist entries for artists that no longer have any content in any library
+	// Remove library_artist entries for artists that no longer have any content in a library.
 	cleanupSQL := Delete("library_artist").Where("stats = '{}'")
 	cleanupRows, err := r.executeSQL(cleanupSQL)
 	if err != nil {
-		log.Warn(r.ctx, "Failed to cleanup empty library_artist entries", "error", err)
-	} else if cleanupRows > 0 {
+		log.Warn(r.ctx, "Failed to cleanup empty library_artist entries", err)
+	} else {
+		if cleanupRows > 0 {
 			log.Debug(r.ctx, "Cleaned up empty library_artist entries", "rowsDeleted", cleanupRows)
 		}
+		// Reconcile orphans whenever the cleanup removed rows, and on a full refresh so a full scan
+		// also heals any left by older versions.
+		if cleanupRows > 0 || allArtists {
+			if err := r.markOrphansMissing(); err != nil {
+				log.Warn(r.ctx, "Failed to mark orphaned artists missing after library_artist cleanup", err)
+			}
+		}
+	}

 	log.Debug(r.ctx, "RefreshStats: Successfully updated stats.", "totalArtistsProcessed", len(allTouchedArtistIDs), "totalDBRowsAffected", totalRowsAffected)
 	return totalRowsAffected, nil
 }

-// applyLibraryFilterToSearchQuery is applyLibraryFilterToArtistQuery with the join order
-// pinned via CROSS JOIN (SQLite's explicit join-order override): the search Phase 1 paginates
-// rowids by artist.id, and when the planner drives from library_artist it must sort every
-// junction row on every page (temp b-tree over the whole table). Keeping artist as the outer
-// table streams rows in artist.id order from its primary key index, so LIMIT/OFFSET
-// short-circuits. Search-only: other artist queries keep the planner's freedom.
-func (r *artistRepository) applyLibraryFilterToSearchQuery(query SelectBuilder) SelectBuilder {
-	user := loggedUser(r.ctx)
-	query = query.CrossJoin("library_artist on library_artist.artist_id = artist.id")
-	if user.ID != invalidUserId && !user.IsAdmin {
-		query = query.Join("user_library on user_library.library_id = library_artist.library_id AND user_library.user_id = ?", user.ID)
-	}
-	return query
-}
-
-func (r *artistRepository) searchCfg() searchConfig {
+// searchCfg builds the per-search config. scope is the set of library IDs the rowid Phase 1 must
+// restrict artists to, or nil to skip the filter (fast-path). See [artistRepository.searchScope].
+func (r *artistRepository) searchCfg(scope []int) searchConfig {
 	return searchConfig{
 		// Natural order for artists is more performant by ID, due to GROUP BY clause in selectArtist
 		NaturalOrder: "artist.id",
 		OrderBy:      []string{"sum(json_extract(stats, '$.total.m')) desc", "name"},
 		MBIDFields:   []string{"mbz_artist_id"},
-		LibraryFilter: r.applyLibraryFilterToSearchQuery,
+		// scope==nil is the fast-path: no filter (and orphans must not exist — see markOrphansMissing).
+		// Otherwise the join-free [artistLibraryFilter].
+		LibraryFilter: func(query SelectBuilder) SelectBuilder {
+			if scope == nil {
+				return query
 			}
+			return query.Where(artistLibraryFilter(scope))
+		},
+	}
+}
+
+// artistLibraryFilter restricts artists to the given libraries via a correlated EXISTS over the
+// library_artist junction, staying join-free so it can scope the join-free search Phase 1 (a JOIN
+// would fan out rowids and corrupt offset pagination). The inner LIMIT 1 is load-bearing: it stops
+// SQLite from flattening the EXISTS back into a fan-out join, while still using the
+// (library_id, artist_id) UNIQUE autoindex.
+func artistLibraryFilter(libraryIDs []int) Sqlizer {
+	if len(libraryIDs) == 0 {
+		return Eq{"1": 2} // match nothing, without a degenerate `IN ()` subquery
+	}
+	sub, args, _ := Select("1").From("library_artist").
+		Where(And{
+			Expr("library_artist.artist_id = artist.id"),
+			Eq{"library_artist.library_id": libraryIDs},
+		}).Limit(1).ToSql()
+	return Expr("EXISTS ("+sub+")", args...)
 }

 func (r *artistRepository) Search(q string, options ...model.QueryOptions) (model.Artists, error) {
@@ -570,14 +603,93 @@ func (r *artistRepository) Search(q string, options ...model.QueryOptions) (mode
 	if len(options) > 0 {
 		opts = options[0]
 	}
+	// Artists have no library_id column, so the library_id filter callers pass (same as albums/songs)
+	// can't be applied directly: consume it and realize it as a join-free Phase-1 scope (searchCfg).
+	scope := r.searchScope(opts.Filters)
+	if isLibraryIDFilter(opts.Filters) {
+		opts.Filters = nil
+	}
 	var res dbArtists
-	err := r.doSearch(r.selectArtist(options...), q, &res, r.searchCfg(), opts)
+	err := r.doSearch(r.selectArtist(opts), q, &res, r.searchCfg(scope), opts)
 	if err != nil {
 		return nil, fmt.Errorf("searching artist %q: %w", q, err)
 	}
 	return res.toModels(), nil
 }

+// searchScope returns the library IDs the search must be restricted to, or nil to skip the filter
+// entirely (the fast-path: the user sees everything the search could return, so a filter would be
+// pure O(offset) overhead). It intersects the requested libraries with what the user can see.
+func (r *artistRepository) searchScope(filter Sqlizer) []int {
+	visible, err := r.visibleLibraryIDs()
+	if err != nil {
+		return r.requestedLibraryIDs(filter) // fail safe: narrow to the request rather than widen
+	}
+	requested := r.requestedLibraryIDs(filter)
+	if requested == nil {
+		// No explicit request: scope to the visible set, unless the user sees everything.
+		if r.userSeesAllLibraries(visible) {
+			return nil
+		}
+		return visible
+	}
+	// Narrow unless the request already covers everything the user can see. Compare by membership,
+	// not length: the requested IDs may contain duplicates.
+	requestedSet := slice.ToSet(requested)
+	if slices.ContainsFunc(visible, func(id int) bool { _, ok := requestedSet[id]; return !ok }) {
+		return requested
+	}
+	return nil
+}
+
+// requestedLibraryIDs extracts the []int from an Eq{"library_id": ids} filter, or nil if filter is
+// not that shape.
+func (r *artistRepository) requestedLibraryIDs(filter Sqlizer) []int {
+	eq, ok := filter.(Eq)
+	if !ok {
+		return nil
+	}
+	ids, _ := eq["library_id"].([]int)
+	return ids
+}
+
+// isLibraryIDFilter reports whether the filter is an Eq carrying a library_id key, so Search can
+// consume it before it reaches the bare artist table (which has no library_id column).
+func isLibraryIDFilter(filter Sqlizer) bool {
+	eq, ok := filter.(Eq)
+	if !ok {
+		return false
+	}
+	_, ok = eq["library_id"]
+	return ok
+}
+
+// userSeesAllLibraries reports whether the visible set already covers every library, so a search
+// needs no library filter at all.
+func (r *artistRepository) userSeesAllLibraries(visible []int) bool {
+	user := loggedUser(r.ctx)
+	if user.IsAdmin || user.ID == invalidUserId {
+		return true // visible is the whole library table
+	}
+	total, err := NewLibraryRepository(r.ctx, r.db).CountAll()
+	if err != nil || total == 0 {
+		return false
+	}
+	return int64(len(visible)) >= total
+}
+
+// visibleLibraryIDs returns the libraries the current user can see: all libraries for admin and
+// headless processes, otherwise the user's granted libraries.
+func (r *artistRepository) visibleLibraryIDs() ([]int, error) {
+	user := loggedUser(r.ctx)
+	if user.IsAdmin || user.ID == invalidUserId {
+		var ids []int
+		err := r.queryAllSlice(Select("id").From("library"), &ids)
+		return ids, err
+	}
+	return slice.Map(user.Libraries, func(lib model.Library) int { return lib.ID }), nil
+}
+
 func (r *artistRepository) Count(options ...rest.QueryOptions) (int64, error) {
 	return r.CountAll(r.parseRestOptions(r.ctx, options...))
 }
@@ -111,6 +111,80 @@ var _ = Describe("ArtistRepository", func() {
 			})
 		})

+		Describe("searchScope", func() {
+			// Resolves the library IDs a search must be restricted to (nil = fast-path / no filter),
+			// the way Search() does, for a repo whose context carries the given user.
+			scope := func(user model.User, filter squirrel.Sqlizer) []int {
+				ctx := request.WithUser(GinkgoT().Context(), user)
+				r := NewArtistRepository(ctx, GetDBXBuilder()).(*artistRepository)
+				return r.searchScope(filter)
+			}
+			subsetUser := model.User{ID: "u", Libraries: model.Libraries{{ID: 1}, {ID: 2}, {ID: 3}}}
+
+			It("scopes to a strict subset of the user's libraries", func() {
+				Expect(scope(subsetUser, squirrel.Eq{"library_id": []int{1, 2}})).To(Equal([]int{1, 2}))
+			})
+
+			It("treats duplicate IDs as a set so a real subset still narrows", func() {
+				// {1,1,2} has 3 entries but is a strict subset of the user's 3 libraries.
+				Expect(scope(subsetUser, squirrel.Eq{"library_id": []int{1, 1, 2}})).To(Equal([]int{1, 1, 2}))
+			})
+
+			It("returns nil (fast-path) when the request covers all the user's libraries", func() {
+				Expect(scope(subsetUser, squirrel.Eq{"library_id": []int{1, 2, 3}})).To(BeNil())
+			})
+
+			It("scopes to the user's libraries when no library filter is given", func() {
+				// A restricted user (strictly fewer libs than exist) with no musicFolderId is still
+				// confined to their granted libs. Build the user with total-1 libraries derived from
+				// the real DB total, so the "sees all" fast-path can't kick in regardless of count.
+				total, err := NewLibraryRepository(GinkgoT().Context(), GetDBXBuilder()).CountAll()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(total).To(BeNumerically(">", 0))
+				libs := make(model.Libraries, 0, total-1)
+				for i := int64(1); i < total; i++ { // total-1 distinct libraries → a strict subset
+					libs = append(libs, model.Library{ID: int(i)})
+				}
+				restricted := model.User{ID: "r", Libraries: libs}
+				got := scope(restricted, nil)
+				Expect(got).To(HaveLen(int(total) - 1))
+			})
+
+			It("returns nil (fast-path) for an admin requesting all existing libraries", func() {
+				// Admins see every library, so the visible set is the whole library table — derive
+				// it from the DB rather than assuming a count.
+				var allLibs []int
+				Expect(NewLibraryRepository(GinkgoT().Context(), GetDBXBuilder()).(*libraryRepository).
+					queryAllSlice(squirrel.Select("id").From("library"), &allLibs)).To(Succeed())
+				admin := model.User{ID: "a", IsAdmin: true}
+				Expect(scope(admin, squirrel.Eq{"library_id": allLibs})).To(BeNil())
+				Expect(scope(admin, nil)).To(BeNil())
+			})
+
+			It("narrows for an admin explicitly requesting a subset via musicFolderId", func() {
+				// An admin scoping to a single, non-existent-as-the-whole-set library must still be
+				// narrowed (regression: search3?musicFolderId=lib2 was leaking lib1 content).
+				admin := model.User{ID: "a", IsAdmin: true}
+				Expect(scope(admin, squirrel.Eq{"library_id": []int{-1}})).To(Equal([]int{-1}))
+			})
+
+			It("returns nil for a non-library_id filter (no library scoping requested)", func() {
+				// Such a filter carries no library intent; for this fully-granted-style user the
+				// search needs no extra library restriction.
+				allUser := model.User{ID: "u2", IsAdmin: true}
+				Expect(scope(allUser, squirrel.Eq{"name": "x"})).To(BeNil())
+			})
+
+			It("falls back to the visible scope for a malformed library_id value (no crash)", func() {
+				// A library_id filter whose value isn't []int is still recognized as a library
+				// filter (so Search consumes it and it never reaches the bare artist table), and
+				// searchScope falls back to exactly the no-filter behavior rather than crashing.
+				malformed := squirrel.Eq{"library_id": "not-a-slice"}
+				Expect(isLibraryIDFilter(malformed)).To(BeTrue())
+				Expect(scope(subsetUser, malformed)).To(Equal(scope(subsetUser, nil)))
+			})
+		})
+
 		Describe("dbArtist mapping", func() {
 			var (
 				artist *model.Artist
@@ -653,6 +727,38 @@ var _ = Describe("ArtistRepository", func() {
 						_, _ = raw.executeSQL(squirrel.Delete(raw.tableName).Where(squirrel.Eq{"id": lib2Artist.ID}))
 					}
 				})
+
+				It("paginates a restricted user's visible artists without gaps", func() {
+					// ID "25" sorts between base fixtures "2" and "3", so this lib2-only artist lands
+					// inside the restricted user's visible range — exercising the no-gap guarantee.
+					lib2Artist := model.Artist{ID: "25", Name: "Restricted Lib2 Artist"}
+					Expect(repo.Put(&lib2Artist)).To(Succeed())
+					Expect(lr.AddArtist(lib2.ID, lib2Artist.ID)).To(Succeed())
+					DeferCleanup(func() {
+						if raw, ok := repo.(*artistRepository); ok {
+							_, _ = raw.executeSQL(squirrel.Delete(raw.tableName).Where(squirrel.Eq{"id": lib2Artist.ID}))
+						}
+					})
+
+					all, err := restrictedRepo.Search("", model.QueryOptions{Max: 1000})
+					Expect(err).ToNot(HaveOccurred())
+					Expect(len(all)).To(BeNumerically(">", 1))
+					for _, a := range all {
+						Expect(a.ID).ToNot(Equal(lib2Artist.ID))
+					}
+
+					var paged model.Artists
+					for offset := range len(all) {
+						page, err := restrictedRepo.Search("", model.QueryOptions{Max: 1, Offset: offset})
+						Expect(err).ToNot(HaveOccurred())
+						Expect(page).To(HaveLen(1), fmt.Sprintf("page at offset %d should be full", offset))
+						paged = append(paged, page...)
+					}
+					Expect(paged).To(HaveLen(len(all)))
+					for i := range all {
+						Expect(paged[i].ID).To(Equal(all[i].ID))
+					}
+				})
 			})

 			Context("Headless Processes (No User Context)", func() {
@@ -891,6 +997,45 @@ var _ = Describe("ArtistRepository", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(idx).To(HaveLen(0))
 			})
+
+			It("takes the unfiltered fast-path when the user can access every library", func() {
+				// The fixture DB has a single library and the user was granted it, so it has access
+				// to all libraries: search results must match what an admin sees.
+				adminRepo := NewArtistRepository(request.WithUser(GinkgoT().Context(), adminUser), GetDBXBuilder())
+				adminAll, err := adminRepo.Search("", model.QueryOptions{Max: 1000})
+				Expect(err).ToNot(HaveOccurred())
+
+				userAll, err := restrictedRepo.Search("", model.QueryOptions{Max: 1000})
+				Expect(err).ToNot(HaveOccurred())
+
+				ids := func(artists model.Artists) []string {
+					out := make([]string, len(artists))
+					for i, a := range artists {
+						out[i] = a.ID
+					}
+					return out
+				}
+				Expect(ids(userAll)).To(Equal(ids(adminAll)))
+				Expect(userAll).ToNot(BeEmpty())
+			})
+
+			It("detects all-library access regardless of result equivalence", func() {
+				// userSeesAllLibraries drives the search fast-path for a non-admin: true when the
+				// visible-library count reaches the DB total. Derive the total from the DB so the
+				// assertion doesn't depend on how many libraries other specs left behind.
+				raw := restrictedRepo.(*artistRepository) // context carries a non-admin user
+				total, err := NewLibraryRepository(GinkgoT().Context(), GetDBXBuilder()).CountAll()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(total).To(BeNumerically(">", 0))
+
+				allLibs := make([]int, total)
+				for i := range allLibs {
+					allLibs[i] = i + 1
+				}
+				Expect(raw.userSeesAllLibraries(allLibs)).To(BeTrue())
+				Expect(raw.userSeesAllLibraries(allLibs[:total-1])).To(BeFalse())
+				Expect(raw.userSeesAllLibraries([]int{})).To(BeFalse())
+			})
 		})
 	})

@@ -976,6 +1121,66 @@ var _ = Describe("ArtistRepository", func() {
 			Expect(err).ToNot(HaveOccurred())
 		})
 	})
+
+	Describe("RefreshStats", func() {
+		var repo *artistRepository
+
+		missing := func(id string) bool {
+			var vals []bool
+			Expect(repo.queryAllSlice(squirrel.Select("missing").From("artist").Where(squirrel.Eq{"id": id}), &vals)).To(Succeed())
+			Expect(vals).To(HaveLen(1))
+			return vals[0]
+		}
+
+		BeforeEach(func() {
+			ctx := request.WithUser(GinkgoT().Context(), adminUser)
+			repo = NewArtistRepository(ctx, GetDBXBuilder()).(*artistRepository)
+		})
+
+		It("marks artists missing when the empty-stats cleanup drops their last library_artist row", func() {
+			// A library_artist row with stats '{}' (no content) gets deleted by the cleanup,
+			// which would orphan this non-missing artist.
+			emptyArtist := model.Artist{ID: "refresh-empty", Name: "No Content Artist"}
+			Expect(repo.Put(&emptyArtist)).To(Succeed())
+			_, err := repo.executeSQL(squirrel.Insert("library_artist").
+				SetMap(map[string]any{"library_id": 1, "artist_id": emptyArtist.ID, "stats": "{}"}))
+			Expect(err).ToNot(HaveOccurred())
+			DeferCleanup(func() {
+				_, _ = repo.executeSQL(squirrel.Delete("library_artist").Where(squirrel.Eq{"artist_id": emptyArtist.ID}))
+				_ = repo.delete(squirrel.Eq{"id": emptyArtist.ID})
+			})
+
+			Expect(missing(emptyArtist.ID)).To(BeFalse())
+
+			_, err = repo.RefreshStats(true)
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(missing(emptyArtist.ID)).To(BeTrue())
+			var orphanIDs []string
+			Expect(repo.queryAllSlice(squirrel.Select("id").From("artist").
+				Where("missing = false").
+				Where("id not in (select artist_id from library_artist)"), &orphanIDs)).To(Succeed())
+			Expect(orphanIDs).ToNot(ContainElement(emptyArtist.ID))
+		})
+
+		It("heals a pre-existing orphan (no library_artist row) on a full refresh", func() {
+			// A legacy orphan left by an older version: non-missing, with no library_artist row at
+			// all. The cleanup deletes nothing for it, so a full refresh (allArtists) must still
+			// reconcile it.
+			legacyOrphan := model.Artist{ID: "refresh-legacy-orphan", Name: "Legacy Orphan"}
+			Expect(repo.Put(&legacyOrphan)).To(Succeed())
+			DeferCleanup(func() {
+				_ = repo.delete(squirrel.Eq{"id": legacyOrphan.ID})
+			})
+
+			Expect(missing(legacyOrphan.ID)).To(BeFalse())
+
+			_, err := repo.RefreshStats(true)
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(missing(legacyOrphan.ID)).To(BeTrue())
+		})
+	})
 })

 // Helper function to create an artist with proper library association.
@@ -261,6 +261,11 @@ func (r *libraryRepository) Delete(id int) error {
 		return err
 	}

+	// The cascade above can drop an artist's last library_artist row; reconcile any such orphans.
+	if err := NewArtistRepository(r.ctx, r.db).(*artistRepository).markOrphansMissing(); err != nil {
+		return fmt.Errorf("marking orphaned artists missing after deleting library %d: %w", id, err)
+	}
+
 	// Clear cache entry for this library only if DB operation was successful
 	libLock.Lock()
 	defer libLock.Unlock()
@@ -4,6 +4,7 @@ import (
 	"context"
 	"time"

+	"github.com/Masterminds/squirrel"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/request"
@@ -206,4 +207,49 @@ var _ = Describe("LibraryRepository", func() {
 			})
 		})
 	})
+
+	Describe("Delete", func() {
+		var adminRepo model.LibraryRepository
+		var artistRepo model.ArtistRepository
+
+		artistMissing := func(id string) bool {
+			var missing bool
+			err := conn.NewQuery("SELECT missing FROM artist WHERE id = {:id}").
+				Bind(dbx.Params{"id": id}).Row(&missing)
+			Expect(err).ToNot(HaveOccurred())
+			return missing
+		}
+
+		BeforeEach(func() {
+			adminCtx := request.WithUser(log.NewContext(context.TODO()), adminUser)
+			adminRepo = NewLibraryRepository(adminCtx, conn)
+			artistRepo = NewArtistRepository(adminCtx, conn)
+		})
+
+		It("marks artists orphaned by the delete as missing", func() {
+			lib := model.Library{Name: "Doomed Library", Path: "/doomed"}
+			Expect(adminRepo.Put(&lib)).To(Succeed())
+
+			orphanArtist := model.Artist{ID: "delete-orphan", Name: "Orphan To Be"}
+			sharedArtist := model.Artist{ID: "delete-shared", Name: "Shared Artist"}
+			Expect(artistRepo.Put(&orphanArtist)).To(Succeed())
+			Expect(artistRepo.Put(&sharedArtist)).To(Succeed())
+			Expect(adminRepo.AddArtist(lib.ID, orphanArtist.ID)).To(Succeed())
+			Expect(adminRepo.AddArtist(lib.ID, sharedArtist.ID)).To(Succeed())
+			Expect(adminRepo.AddArtist(1, sharedArtist.ID)).To(Succeed())
+			DeferCleanup(func() {
+				if raw, ok := artistRepo.(*artistRepository); ok {
+					_, _ = raw.executeSQL(squirrel.Delete("artist").
+						Where(squirrel.Eq{"id": []string{orphanArtist.ID, sharedArtist.ID}}))
+				}
+			})
+
+			Expect(artistMissing(orphanArtist.ID)).To(BeFalse())
+
+			Expect(adminRepo.Delete(lib.ID)).To(Succeed())
+
+			Expect(artistMissing(orphanArtist.ID)).To(BeTrue(), "orphaned artist should be marked missing")
+			Expect(artistMissing(sharedArtist.ID)).To(BeFalse(), "artist still in another library must stay visible")
+		})
+	})
 })
@@ -21,10 +21,9 @@ type searchConfig struct {
 	NaturalOrder string   // ORDER BY for empty-query results (e.g. "album.rowid")
 	OrderBy      []string // ORDER BY for text search results (e.g. ["name"])
 	MBIDFields   []string // columns to match when query is a UUID
-	// LibraryFilter overrides the default applyLibraryFilter for the rowid Phase 1 of
-	// two-phase searches (FTS and empty-query). Needed when library access goes through a
-	// junction table (e.g. artist → library_artist), whose JOIN can fan out rowids for
-	// entities in multiple libraries — Phase 1 dedups whenever this is set.
+	// LibraryFilter overrides the default applyLibraryFilter for the rowid Phase 1, for entities whose
+	// library access goes through a junction table (e.g. artist → library_artist). It MUST be join-free
+	// (Phase 1 has no DISTINCT, so a fan-out JOIN would corrupt offset pagination). See [artistLibraryFilter].
 	LibraryFilter func(sq SelectBuilder) SelectBuilder
 }

@@ -102,10 +101,7 @@ func (r sqlRepository) executeTwoPhase(sq SelectBuilder, results any, rowidCore
 		rowidQuery = rowidQuery.Offset(uint64(options.Offset))
 	}
 	if cfg.LibraryFilter != nil {
-		// Junction-table library filters can repeat rowids for entities in multiple
-		// libraries, which would corrupt offset-based pagination — dedup before paginating.
-		// (DISTINCT, not GROUP BY: bm25() can't be evaluated in a grouped query.)
-		rowidQuery = cfg.LibraryFilter(rowidQuery).Distinct()
+		rowidQuery = cfg.LibraryFilter(rowidQuery)
 	} else {
 		rowidQuery = r.applyLibraryFilter(rowidQuery)
 	}
@@ -2,6 +2,7 @@ package scanner_test

 import (
 	"context"
+	"database/sql"
 	"errors"
 	"path/filepath"
 	"testing/fstest"
@@ -531,6 +532,69 @@ var _ = Describe("Scanner", Ordered, func() {
 			})).To(Equal(int64(2)))
 		})

+		It("leaves no non-missing orphan artist after purging an artist's only content", func() {
+			// Guards the orphan case: with PurgeMissing on, removing an artist's last file hard-deletes
+			// its media_file_artists rows, RefreshStats recomputes its stats to '{}', and the cleanup
+			// drops its last library_artist row — leaving the artist row alive but orphaned. RefreshStats
+			// must then mark it missing (see markOrphansMissing).
+			DeferCleanup(configtest.SetupConfig())
+			conf.Server.Scanner.PurgeMissing = consts.PurgeMissingAlways
+
+			By("Starting from a library where Pink Floyd has its own single album")
+			floyd := template(_t{"artist": "Pink Floyd", "album": "The Wall", "year": 1979})
+			fsys = createFS(fstest.MapFS{
+				"The Beatles/Help!/01 - Help!.mp3":            help(track(1, "Help!")),
+				"The Beatles/Help!/02 - The Night Before.mp3": help(track(2, "The Night Before")),
+				"The Beatles/Revolver/01 - Taxman.mp3":        revolver(track(1, "Taxman")),
+				"The Beatles/Revolver/02 - Eleanor Rigby.mp3": revolver(track(2, "Eleanor Rigby")),
+				"Pink Floyd/The Wall/01 - Another Brick.mp3":  floyd(track(1, "Another Brick in the Wall")),
+			})
+			Expect(runScanner(ctx, true)).To(Succeed())
+
+			nonMissingArtists := func() []string {
+				aa, err := ds.Artist(ctx).GetAll(model.QueryOptions{Filters: squirrel.Eq{"missing": false}})
+				Expect(err).ToNot(HaveOccurred())
+				return slice.Map(aa, func(a model.Artist) string { return a.Name })
+			}
+			orphanCount := func() int64 {
+				var n int64
+				Expect(db.Db().QueryRowContext(ctx,
+					"SELECT count(*) FROM artist WHERE missing = false "+
+						"AND id NOT IN (SELECT artist_id FROM library_artist)").Scan(&n)).To(Succeed())
+				return n
+			}
+			// Read the artist row directly: selectArtist inner-joins library_artist, so an orphan never
+			// surfaces through the repository. Returns a descriptive string for clear test failures.
+			floydState := func() string {
+				var m bool
+				err := db.Db().QueryRowContext(ctx,
+					"SELECT missing FROM artist WHERE name = 'Pink Floyd'").Scan(&m)
+				if errors.Is(err, sql.ErrNoRows) {
+					return "NOT_FOUND"
+				}
+				Expect(err).ToNot(HaveOccurred())
+				if m {
+					return "MISSING"
+				}
+				return "PRESENT"
+			}
+
+			By("Confirming Pink Floyd is visible after the import, with no orphan")
+			Expect(nonMissingArtists()).To(ContainElement("Pink Floyd"))
+			Expect(floydState()).To(Equal("PRESENT"))
+			Expect(orphanCount()).To(BeZero())
+
+			By("Removing all of Pink Floyd's files and rescanning")
+			fsys.Remove("Pink Floyd/The Wall/01 - Another Brick.mp3")
+			Expect(runScanner(ctx, true)).To(Succeed())
+
+			By("Checking Pink Floyd's row survives but is marked missing, leaving no orphan")
+			Expect(floydState()).To(Equal("MISSING"))
+			Expect(orphanCount()).To(BeZero())
+			// The Beatles keep their content, so the fix must not over-mark them.
+			Expect(nonMissingArtists()).To(ContainElement("The Beatles"))
+		})
+
 		It("does not override artist fields when importing an undertagged file", func() {
 			By("Making sure artist in the DB contains MBID and sort name")
 			aa, err := ds.Artist(ctx).GetAll(model.QueryOptions{
@@ -74,7 +74,7 @@ func (api *Router) searchAll(ctx context.Context, sp *searchParams, musicFolderI
 	if len(musicFolderIds) > 0 {
 		songOpts.Filters = Eq{"library_id": musicFolderIds}
 		albumOpts.Filters = Eq{"library_id": musicFolderIds}
-		artistOpts.Filters = Eq{"library_artist.library_id": musicFolderIds}
+		artistOpts.Filters = Eq{"library_id": musicFolderIds}
 	}

 	// Run searches in parallel
@@ -39,12 +39,17 @@ var _ = Describe("Search", func() {
 		}

 		Describe("Search2", func() {
-			It("should accept musicFolderId parameter", func() {
+			It("scopes all entity types to the requested libraries", func() {
+				// The subsonic layer passes the same library_id filter to all three repos; the
+				// artist repository translates it to the join-free library_artist predicate itself.
 				r := newGetRequest("query=test", "musicFolderId=1")
 				ctx := request.WithUser(r.Context(), model.User{
 					ID:       "user1",
 					UserName: "testuser",
-					Libraries: []model.Library{{ID: 1, Name: "Library 1"}},
+					Libraries: []model.Library{
+						{ID: 1, Name: "Library 1"},
+						{ID: 2, Name: "Library 2"},
+					},
 				})
 				r = r.WithContext(ctx)

@@ -54,14 +59,13 @@ var _ = Describe("Search", func() {
 				Expect(resp).ToNot(BeNil())
 				Expect(resp.SearchResult2).ToNot(BeNil())

-				// Verify that library filter was applied to all repositories
 				assertQueryOptions(mockAlbumRepo.Options.Filters, "library_id IN (?)", 1)
-				assertQueryOptions(mockArtistRepo.Options.Filters, "library_id IN (?)", 1)
 				assertQueryOptions(mockMediaFileRepo.Options.Filters, "library_id IN (?)", 1)
+				assertQueryOptions(mockArtistRepo.Options.Filters, "library_id IN (?)", 1)
 			})

-			It("should return results from all accessible libraries when musicFolderId is not provided", func() {
-				r := newGetRequest("query=test")
+			It("applies no library filter when musicFolderId is not provided", func() {
+				r := newGetRequest("query=test") // no musicFolderId → all accessible libraries
 				ctx := request.WithUser(r.Context(), model.User{
 					ID:       "user1",
 					UserName: "testuser",
@@ -79,10 +83,9 @@ var _ = Describe("Search", func() {
 				Expect(resp).ToNot(BeNil())
 				Expect(resp.SearchResult2).ToNot(BeNil())

-				// Verify that library filter was applied to all repositories with all accessible libraries
 				assertQueryOptions(mockAlbumRepo.Options.Filters, "library_id IN (?,?,?)", 1, 2, 3)
-				assertQueryOptions(mockArtistRepo.Options.Filters, "library_id IN (?,?,?)", 1, 2, 3)
 				assertQueryOptions(mockMediaFileRepo.Options.Filters, "library_id IN (?,?,?)", 1, 2, 3)
+				assertQueryOptions(mockArtistRepo.Options.Filters, "library_id IN (?,?,?)", 1, 2, 3)
 			})

 			It("should return empty results when user has no accessible libraries", func() {
@@ -122,12 +125,15 @@ var _ = Describe("Search", func() {
 		})

 		Describe("Search3", func() {
-			It("should accept musicFolderId parameter", func() {
+			It("scopes all entity types to the requested libraries", func() {
 				r := newGetRequest("query=test", "musicFolderId=1")
 				ctx := request.WithUser(r.Context(), model.User{
 					ID:       "user1",
 					UserName: "testuser",
-					Libraries: []model.Library{{ID: 1, Name: "Library 1"}},
+					Libraries: []model.Library{
+						{ID: 1, Name: "Library 1"},
+						{ID: 2, Name: "Library 2"},
+					},
 				})
 				r = r.WithContext(ctx)

@@ -137,14 +143,13 @@ var _ = Describe("Search", func() {
 				Expect(resp).ToNot(BeNil())
 				Expect(resp.SearchResult3).ToNot(BeNil())

-				// Verify that library filter was applied to all repositories
 				assertQueryOptions(mockAlbumRepo.Options.Filters, "library_id IN (?)", 1)
-				assertQueryOptions(mockArtistRepo.Options.Filters, "library_id IN (?)", 1)
 				assertQueryOptions(mockMediaFileRepo.Options.Filters, "library_id IN (?)", 1)
+				assertQueryOptions(mockArtistRepo.Options.Filters, "library_id IN (?)", 1)
 			})

-			It("should return results from all accessible libraries when musicFolderId is not provided", func() {
-				r := newGetRequest("query=test")
+			It("applies no library filter when musicFolderId is not provided", func() {
+				r := newGetRequest("query=test") // no musicFolderId → all accessible libraries
 				ctx := request.WithUser(r.Context(), model.User{
 					ID:       "user1",
 					UserName: "testuser",
@@ -162,10 +167,9 @@ var _ = Describe("Search", func() {
 				Expect(resp).ToNot(BeNil())
 				Expect(resp.SearchResult3).ToNot(BeNil())

-				// Verify that library filter was applied to all repositories with all accessible libraries
 				assertQueryOptions(mockAlbumRepo.Options.Filters, "library_id IN (?,?,?)", 1, 2, 3)
-				assertQueryOptions(mockArtistRepo.Options.Filters, "library_id IN (?,?,?)", 1, 2, 3)
 				assertQueryOptions(mockMediaFileRepo.Options.Filters, "library_id IN (?,?,?)", 1, 2, 3)
+				assertQueryOptions(mockArtistRepo.Options.Filters, "library_id IN (?,?,?)", 1, 2, 3)
 			})

 			It("should return empty results when user has no accessible libraries", func() {
@@ -42,6 +42,16 @@ func ToMap[T any, K comparable, V any](s []T, transformFunc func(T) (K, V)) map[
 	return m
 }

+// ToSet builds a set (a map keyed by the slice's elements) for O(1) membership tests. Duplicate
+// elements collapse to a single key.
+func ToSet[T comparable](s []T) map[T]struct{} {
+	m := make(map[T]struct{}, len(s))
+	for _, item := range s {
+		m[item] = struct{}{}
+	}
+	return m
+}
+
 func CompactByFrequency[T comparable](list []T) []T {
 	counters := make(map[T]int)
 	for _, item := range list {
@@ -81,6 +81,20 @@ var _ = Describe("Slice Utils", func() {
 		})
 	})

+	Describe("ToSet", func() {
+		It("returns empty set for an empty input", func() {
+			Expect(slice.ToSet([]int{})).To(BeEmpty())
+		})
+
+		It("builds a set with one key per distinct element", func() {
+			result := slice.ToSet([]int{1, 2, 2, 3, 3, 3})
+			Expect(result).To(HaveLen(3))
+			Expect(result).To(HaveKey(1))
+			Expect(result).To(HaveKey(2))
+			Expect(result).To(HaveKey(3))
+		})
+	})
+
 	Describe("CompactByFrequency", func() {
 		It("returns empty slice for an empty input", func() {
 			Expect(slice.CompactByFrequency([]int{})).To(BeEmpty())