fix(mix): rating filter across mixes and Lucky Mix queue fill (#714)

* fix(mix): apply rating filter across mixes and fix Lucky Mix queue fill

Invalidate entity rating cache on setRating and stop negative-caching
unrated artists/albums so filters see fresh stars. Honor UI rating
overrides, wire Instant Mix and CLI paths, fix Random Mix filter order,
and align Lucky Mix progress with the real player queue length.

* docs(changelog): document mix rating filter and Lucky Mix queue fix

src/utils/componentHelpers/contextMenuActions.ts

@@ -1,6 +1,7 @@
 import { join } from '@tauri-apps/api/path';
 import { invoke } from '@tauri-apps/api/core';
 import { getSimilarSongs2, getSimilarSongs, getTopSongs } from '../../api/subsonicArtists';
+import { filterSongsForLuckyMixRatings, getMixMinRatingsConfigFromAuth } from '../mix/mixRatingFilter';
 import { buildDownloadUrl } from '../../api/subsonicStreamUrl';
 import { useAuthStore } from '../../store/authStore';
 import { usePlayerStore } from '../../store/playerStore';
@@ -107,10 +108,13 @@ export async function startInstantMix(
   try {
     const similar = await getSimilarSongs(song.id, 50);
     if (serverId) useAuthStore.getState().setAudiomuseNavidromeIssue(serverId, false);
+    const mixCfg = getMixMinRatingsConfigFromAuth();
+    const ratedFiltered = await filterSongsForLuckyMixRatings(
+      similar.filter(s => s.id !== song.id),
+      mixCfg,
+    );
     const shuffled = shuffleArray(
-      similar
-        .filter(s => s.id !== song.id)
-        .map(s => ({ ...songToTrack(s), radioAdded: true as const })),
+      ratedFiltered.map(s => ({ ...songToTrack(s), radioAdded: true as const })),
     );
     if (shuffled.length > 0) {
       const aid = song.artistId?.trim() || undefined;

src/utils/componentHelpers/randomMixHelpers.test.ts

@@ -0,0 +1,35 @@
+import { describe, expect, it } from 'vitest';
+import type { SubsonicSong } from '../../api/subsonicTypes';
+import { filterRandomMixSongs } from './randomMixHelpers';
+
+function song(id: string): SubsonicSong {
+  return {
+    id,
+    title: 't',
+    artist: 'A',
+    album: 'Al',
+    albumId: 'alb',
+    duration: 1,
+    artistUserRating: 1,
+  };
+}
+
+describe('filterRandomMixSongs', () => {
+  it('applies mix rating filter even when audiobook exclusion is off', () => {
+    const cfg = { enabled: true, minSong: 0, minAlbum: 0, minArtist: 2 };
+    const out = filterRandomMixSongs([song('1'), song('2')], {
+      excludeAudiobooks: false,
+      customGenreBlacklist: [],
+      mixRatingCfg: { ...cfg, minArtist: 2 },
+    });
+    expect(out).toHaveLength(0);
+
+    const kept = filterRandomMixSongs([song('1'), { ...song('2'), artistUserRating: 4 }], {
+      excludeAudiobooks: false,
+      customGenreBlacklist: [],
+      mixRatingCfg: cfg,
+    });
+    expect(kept).toHaveLength(1);
+    expect(kept[0].id).toBe('2');
+  });
+});

src/utils/componentHelpers/randomMixHelpers.ts

@@ -24,6 +24,7 @@ interface FilterArgs {
 export function filterRandomMixSongs(songs: SubsonicSong[], args: FilterArgs): SubsonicSong[] {
   const { excludeAudiobooks, customGenreBlacklist, mixRatingCfg } = args;
   return songs.filter(song => {
+    if (!passesMixMinRatings(song, mixRatingCfg)) return false;
     if (!excludeAudiobooks) return true;
     const checkText = (text: string) => {
       const t = text.toLowerCase();
@@ -35,7 +36,6 @@ export function filterRandomMixSongs(songs: SubsonicSong[], args: FilterArgs): S
     if (song.title && checkText(song.title)) return false;
     if (song.album && checkText(song.album)) return false;
     if (song.artist && checkText(song.artist)) return false;
-    if (!passesMixMinRatings(song, mixRatingCfg)) return false;
     return true;
   });
 }

src/utils/mix/luckyMix.ts

@@ -12,6 +12,7 @@ import { isLuckyMixAvailable } from '../../hooks/useLuckyMixAvailable';
 import { showToast } from '../ui/toast';
 import {
   filterSongsForLuckyMixRatings,
+  filterTopArtistsForMixRatings,
   getMixMinRatingsConfigFromAuth,
 } from './mixRatingFilter';
 import {
@@ -99,13 +100,15 @@ export async function buildAndPlayLuckyMix(): Promise<void> {
   let unsubPlayer: (() => void) | null = null;
   let startedPlayback = false;
   try {
-    const queuedIds = new Set<string>();
     let allSeedSongs: SubsonicSong[] = [];
 
+    const mixQueueSize = () => usePlayerStore.getState().queue.length;
+    const mixQueueTrackIds = () => new Set(usePlayerStore.getState().queue.map(t => t.id));
+
     const bailIfCancelled = () => {
       if (useLuckyMixStore.getState().cancelRequested) throw new LuckyMixCancelled();
     };
-    const reachedTarget = () => queuedIds.size >= MIX_TARGET_SIZE;
+    const reachedTarget = () => mixQueueSize() >= MIX_TARGET_SIZE;
 
     const startImmediatePlayback = async (song: SubsonicSong, source: string) => {
       if (startedPlayback || !song?.id) return;
@@ -113,26 +116,24 @@ export async function buildAndPlayLuckyMix(): Promise<void> {
       if (!allowed.length) return;
       const play = allowed[0];
       startedPlayback = true;
-      queuedIds.add(play.id);
       const track = songToTrack(play);
       usePlayerStore.getState().playTrack(track, [track], true);
       logStep('start_immediate_playback', {
         source,
         song: songDebug([play])[0],
-        queuedCount: queuedIds.size,
+        queuedCount: mixQueueSize(),
       });
 
       // Auto-cancel: once we're playing, watch the player store. If the
       // current track switches to something the user picked themselves (not
-      // in our queuedIds set), treat that as "user moved on" and cancel the
-      // build so we don't later overwrite their choice with our finalised mix.
+      // in the mix queue), treat that as "user moved on" and cancel the build.
       if (!unsubPlayer) {
         unsubPlayer = usePlayerStore.subscribe((state, prev) => {
           const prevId = prev.currentTrack?.id ?? null;
           const nextId = state.currentTrack?.id ?? null;
           if (nextId === prevId) return;
           if (!nextId) return;
-          if (queuedIds.has(nextId)) return;
+          if (state.queue.some(t => t.id === nextId)) return;
           useLuckyMixStore.getState().cancel();
         });
       }
@@ -142,7 +143,8 @@ export async function buildAndPlayLuckyMix(): Promise<void> {
       if (useLuckyMixStore.getState().cancelRequested) return 0;
       if (reachedTarget()) return 0;
       if (!songs.length) return 0;
-      const unique = uniqueBySongId(songs).filter(s => !queuedIds.has(s.id));
+      const knownIds = mixQueueTrackIds();
+      const unique = uniqueBySongId(songs).filter(s => !knownIds.has(s.id));
       const deduped = await filterSongsForLuckyMixRatings(unique, mixRatingCfg);
       if (!deduped.length) return 0;
 
@@ -153,19 +155,20 @@ export async function buildAndPlayLuckyMix(): Promise<void> {
       }
 
       if (!candidates.length) return 0;
-      const remaining = Math.max(0, MIX_TARGET_SIZE - queuedIds.size);
+      const remaining = Math.max(0, MIX_TARGET_SIZE - mixQueueSize());
       if (remaining <= 0) return 0;
       const toAdd = sampleRandom(candidates, Math.min(remaining, candidates.length));
       if (!toAdd.length) return 0;
-      toAdd.forEach(s => queuedIds.add(s.id));
-      usePlayerStore.getState().enqueue(toAdd.map(songToTrack));
+      const before = mixQueueSize();
+      usePlayerStore.getState().enqueue(toAdd.map(songToTrack), true);
+      const added = mixQueueSize() - before;
       logStep('append_queue_batch', {
         reason,
-        added: toAdd.length,
-        queuedCount: queuedIds.size,
+        added,
+        queuedCount: mixQueueSize(),
         songs: songDebug(toAdd),
       });
-      return toAdd.length;
+      return added;
     };
 
     const frequentAlbums = await fetchFrequentAlbumsPool();
@@ -175,7 +178,10 @@ export async function buildAndPlayLuckyMix(): Promise<void> {
       fetched: frequentAlbums.length,
       withPlays: albumsWithPlays.length,
     });
-    const topArtists = deriveTopArtistsFromFrequentAlbums(albumsWithPlays);
+    const topArtists = await filterTopArtistsForMixRatings(
+      deriveTopArtistsFromFrequentAlbums(albumsWithPlays),
+      mixRatingCfg,
+    );
     const pickedArtists = sampleRandom(topArtists, 2);
     logStep('pick_top_artists', {
       topArtistsCount: topArtists.length,
@@ -280,7 +286,8 @@ export async function buildAndPlayLuckyMix(): Promise<void> {
       initialPoolCount: pool.length,
     });
 
-    for (let i = 0; i < 10 && pool.length < MIX_TARGET_SIZE; i++) {
+    const poolFillMaxBatches = mixRatingCfg.enabled ? 25 : 10;
+    for (let i = 0; i < poolFillMaxBatches && !reachedTarget(); i++) {
       bailIfCancelled();
       const rnd = await filterSongsToActiveLibrary(await getRandomSongs(120));
       pool = uniqueAppend(pool, rnd);
@@ -289,24 +296,39 @@ export async function buildAndPlayLuckyMix(): Promise<void> {
         batch: i + 1,
         fetched: rnd.length,
         poolCount: pool.length,
+        queueCount: mixQueueSize(),
       });
-      if (reachedTarget()) break;
     }
 
     bailIfCancelled();
-    const poolFiltered = await filterSongsForLuckyMixRatings(pool, mixRatingCfg);
-    const finalSongs = sampleRandom(poolFiltered, MIX_TARGET_SIZE).filter(s => !queuedIds.has(s.id));
-    await appendSongsToQueue(finalSongs, 'finalize-randomized');
+    if (!reachedTarget()) {
+      const poolFiltered = await filterSongsForLuckyMixRatings(pool, mixRatingCfg);
+      const need = MIX_TARGET_SIZE - mixQueueSize();
+      const finalSongs = sampleRandom(
+        poolFiltered.filter(s => !mixQueueTrackIds().has(s.id)),
+        need,
+      );
+      await appendSongsToQueue(finalSongs, 'finalize-randomized');
+    }
+
+    for (let i = 0; i < 20 && !reachedTarget(); i++) {
+      bailIfCancelled();
+      const rnd = await filterSongsToActiveLibrary(await getRandomSongs(120));
+      const added = await appendSongsToQueue(rnd, `topup-${i + 1}`);
+      if (added === 0 && i >= 8) break;
+    }
+
+    const finalQueueCount = mixQueueSize();
     logStep('final_queue_state', {
       poolCount: pool.length,
-      queuedCount: queuedIds.size,
+      queuedCount: finalQueueCount,
       queuedTarget: MIX_TARGET_SIZE,
     });
-    if (queuedIds.size === 0) {
+    if (finalQueueCount === 0) {
       throw new Error('empty-mix');
     }
-    showToast(i18n.t('luckyMix.done', { count: queuedIds.size }), 3500, 'success');
-    logStep('done', { queueCount: queuedIds.size });
+    showToast(i18n.t('luckyMix.done', { count: finalQueueCount }), 3500, 'success');
+    logStep('done', { queueCount: finalQueueCount });
     if (debugEnabled) {
       console.debug('[psysonic][lucky-mix] full-steps', debugSteps);
       void invoke('frontend_debug_log', {

src/utils/mix/mixRatingFilter.test.ts

@@ -0,0 +1,111 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import type { SubsonicSong } from '../../api/subsonicTypes';
+import { usePlayerStore } from '../../store/playerStore';
+import { resetPlayerStore } from '../../test/helpers/storeReset';
+
+vi.mock('../../api/subsonicRatings', () => ({
+  prefetchArtistUserRatings: vi.fn(),
+  prefetchAlbumUserRatings: vi.fn(),
+  parseSubsonicEntityStarRating: vi.fn(),
+}));
+
+import { prefetchArtistUserRatings, prefetchAlbumUserRatings } from '../../api/subsonicRatings';
+import {
+  enrichSongsForMixRatingFilter,
+  filterTopArtistsForMixRatings,
+  passesMixMinRatings,
+} from './mixRatingFilter';
+
+const enabledArtist2: { enabled: true; minSong: 0; minAlbum: 0; minArtist: 2 } = {
+  enabled: true,
+  minSong: 0,
+  minAlbum: 0,
+  minArtist: 2,
+};
+
+function song(partial: Partial<SubsonicSong> & Pick<SubsonicSong, 'id'>): SubsonicSong {
+  return {
+    title: 't',
+    artist: 'A',
+    album: 'Al',
+    albumId: 'alb-1',
+    artistId: 'art-1',
+    duration: 180,
+    ...partial,
+  };
+}
+
+beforeEach(() => {
+  resetPlayerStore();
+  vi.mocked(prefetchArtistUserRatings).mockReset();
+  vi.mocked(prefetchAlbumUserRatings).mockReset();
+  vi.mocked(prefetchAlbumUserRatings).mockResolvedValue(new Map());
+});
+
+describe('passesMixMinRatings — artist axis', () => {
+  it('excludes when artistUserRating is at or below threshold', () => {
+    expect(passesMixMinRatings(song({ id: '1', artistUserRating: 1 }), enabledArtist2)).toBe(false);
+    expect(passesMixMinRatings(song({ id: '2', artistUserRating: 2 }), enabledArtist2)).toBe(false);
+    expect(passesMixMinRatings(song({ id: '3', artistUserRating: 3 }), enabledArtist2)).toBe(true);
+  });
+
+  it('keeps unrated artists (missing or zero)', () => {
+    expect(passesMixMinRatings(song({ id: '1' }), enabledArtist2)).toBe(true);
+    expect(passesMixMinRatings(song({ id: '2', artistUserRating: 0 }), enabledArtist2)).toBe(true);
+  });
+
+  it('uses playerStore userRatingOverrides before API fields', () => {
+    usePlayerStore.getState().setUserRatingOverride('art-1', 1);
+    expect(
+      passesMixMinRatings(song({ id: '1', artistUserRating: 5 }), enabledArtist2),
+    ).toBe(false);
+  });
+
+  it('uses OpenSubsonic artists[] ref when artistUserRating is absent', () => {
+    const low = song({
+      id: '1',
+      artists: [{ id: 'art-1', userRating: 1 }],
+    });
+    expect(passesMixMinRatings(low, enabledArtist2)).toBe(false);
+  });
+});
+
+describe('enrichSongsForMixRatingFilter', () => {
+  it('prefetches entity artist rating even when song carries a misleading artists[] ref', async () => {
+    vi.mocked(prefetchArtistUserRatings).mockResolvedValue(new Map([['art-1', 1]]));
+
+    const input = [
+      song({
+        id: '1',
+        artists: [{ id: 'art-1', userRating: 5 }],
+      }),
+    ];
+    const out = await enrichSongsForMixRatingFilter(input, enabledArtist2);
+
+    expect(prefetchArtistUserRatings).toHaveBeenCalledWith(['art-1']);
+    expect(out[0].artistUserRating).toBe(1);
+    expect(passesMixMinRatings(out[0], enabledArtist2)).toBe(false);
+  });
+});
+
+describe('filterTopArtistsForMixRatings', () => {
+  it('drops artists rated at or below the threshold', async () => {
+    vi.mocked(prefetchArtistUserRatings).mockResolvedValue(
+      new Map([
+        ['a1', 1],
+        ['a2', 3],
+      ]),
+    );
+
+    const out = await filterTopArtistsForMixRatings(
+      [
+        { id: 'a1', name: 'Low' },
+        { id: 'a2', name: 'Ok' },
+        { id: 'a3', name: 'Unrated' },
+      ],
+      enabledArtist2,
+    );
+
+    expect(out.map(a => a.id)).toEqual(['a2', 'a3']);
+  });
+});

src/utils/mix/mixRatingFilter.ts

@@ -2,6 +2,7 @@ import { parseSubsonicEntityStarRating, prefetchAlbumUserRatings, prefetchArtist
 import { getRandomSongs } from '../../api/subsonicLibrary';
 import type { SubsonicAlbum, SubsonicSong } from '../../api/subsonicTypes';
 import { useAuthStore } from '../../store/authStore';
+import { usePlayerStore } from '../../store/playerStore';
 
 /** Default target list size for Random Mix; per-call override via `fetchRandomMixSongsUntilFull(c, { targetSize })`. */
 export const RANDOM_MIX_TARGET_SIZE = 50;
@@ -59,6 +60,14 @@ function numRating(v: unknown): number | undefined {
   return n;
 }
 
+/** Optimistic stars from the UI (`setUserRatingOverride`) take precedence over API payloads. */
+function mixRatingOverrideForEntity(entityId: string | undefined): number | undefined {
+  if (!entityId) return undefined;
+  const o = usePlayerStore.getState().userRatingOverrides[entityId];
+  if (o === undefined || o <= 0) return undefined;
+  return o;
+}
+
 type OpenArtistRefLike = { id?: string; userRating?: unknown; rating?: unknown };
 
 function refStarRating(a: OpenArtistRefLike | undefined): number | undefined {
@@ -107,9 +116,11 @@ function artistEntityIdForMixRating(song: SubsonicSong): string | undefined {
 
 /** Song-level artist rating: explicit field, then OpenSubsonic `artists` / `albumArtists` on the child. */
 function effectiveArtistRatingForFilter(song: SubsonicSong): number | undefined {
+  const prefer = artistEntityIdForMixRating(song);
+  const fromOverride = mixRatingOverrideForEntity(prefer);
+  if (fromOverride !== undefined) return fromOverride;
   const d = numRating(song.artistUserRating);
   if (d !== undefined) return d;
-  const prefer = artistEntityIdForMixRating(song);
   const fromArtists = ratingFromArtistRefs(song.artists, prefer);
   if (fromArtists !== undefined) return fromArtists;
   return ratingFromArtistRefs(song.albumArtists, prefer);
@@ -117,11 +128,15 @@ function effectiveArtistRatingForFilter(song: SubsonicSong): number | undefined
 
 /** Song-level album (parent) rating when the server puts it on the child payload. */
 function effectiveAlbumRatingOnSong(song: SubsonicSong): number | undefined {
+  const fromOverride = mixRatingOverrideForEntity(song.albumId);
+  if (fromOverride !== undefined) return fromOverride;
   const x = song as SubsonicSong & { albumRating?: unknown };
   return numRating(song.albumUserRating ?? x.albumRating);
 }
 
 function songTrackStarRatingForMix(song: SubsonicSong): number | undefined {
+  const fromOverride = mixRatingOverrideForEntity(song.id);
+  if (fromOverride !== undefined) return fromOverride;
   const x = song as SubsonicSong & { rating?: unknown };
   return numRating(song.userRating ?? x.rating);
 }
@@ -220,6 +235,20 @@ export async function filterSongsForLuckyMixRatings(
  * Merge `getArtist` / `getAlbum` ratings into songs when list payloads omit them,
  * so `passesMixMinRatings` / Lucky Mix filtering see album and artist stars.
  */
+/** Drop low-rated seed artists before Lucky Mix picks from listening history. */
+export async function filterTopArtistsForMixRatings<T extends { id: string }>(
+  artists: T[],
+  c: MixMinRatingsConfig,
+): Promise<T[]> {
+  if (!c.enabled || c.minArtist <= 0 || !artists.length) return artists;
+  const byArtist = await prefetchArtistUserRatings(artists.map(a => a.id));
+  return artists.filter(a => {
+    const r = mixRatingOverrideForEntity(a.id) ?? byArtist.get(a.id);
+    if (r === undefined || r <= 0) return true;
+    return r > c.minArtist;
+  });
+}
+
 export async function enrichSongsForMixRatingFilter(
   songs: SubsonicSong[],
   c: MixMinRatingsConfig,
@@ -227,22 +256,11 @@ export async function enrichSongsForMixRatingFilter(
   if (!c.enabled || (c.minArtist <= 0 && c.minAlbum <= 0)) return songs;
   const artistIds =
     c.minArtist > 0
-      ? [
-          ...new Set(
-            songs
-              .filter(
-                s =>
-                  s.artistUserRating === undefined
-                  && effectiveArtistRatingForFilter(s) === undefined
-                  && artistEntityIdForMixRating(s),
-              )
-              .map(s => artistEntityIdForMixRating(s)!),
-          ),
-        ]
+      ? [...new Set(songs.map(s => artistEntityIdForMixRating(s)).filter((id): id is string => !!id))]
       : [];
   const albumIds =
     c.minAlbum > 0
-      ? [...new Set(songs.filter(s => s.albumUserRating === undefined && s.albumId).map(s => s.albumId!))]
+      ? [...new Set(songs.filter(s => s.albumId).map(s => s.albumId!))]
       : [];
   const [byArtist, byAlbum] = await Promise.all([
     artistIds.length ? prefetchArtistUserRatings(artistIds) : Promise.resolve(new Map<string, number>()),
@@ -252,13 +270,9 @@ export async function enrichSongsForMixRatingFilter(
   return songs.map(s => {
     const aid = artistEntityIdForMixRating(s);
     const artistPatch =
-      s.artistUserRating === undefined && aid && byArtist.has(aid)
-        ? { artistUserRating: byArtist.get(aid)! }
-        : {};
+      aid && byArtist.has(aid) ? { artistUserRating: byArtist.get(aid)! } : {};
     const albumPatch =
-      s.albumUserRating === undefined && s.albumId && byAlbum.has(s.albumId)
-        ? { albumUserRating: byAlbum.get(s.albumId)! }
-        : {};
+      s.albumId && byAlbum.has(s.albumId) ? { albumUserRating: byAlbum.get(s.albumId)! } : {};
     return { ...s, ...artistPatch, ...albumPatch };
   });
 }