diff --git a/backend/playlist/playlist.go b/backend/playlist/playlist.go index 126b531..983dc40 100644 --- a/backend/playlist/playlist.go +++ b/backend/playlist/playlist.go @@ -3125,6 +3125,19 @@ func (s *Service) EvaluateSmartPlaylist( return tracks, nil } +// SuggestSmartPlaylistValues returns values of field present in the +// library that contain needle, for the rule editor's value box. +func (s *Service) SuggestSmartPlaylistValues( + field, needle string, +) ([]string, error) { + values, err := smartplaylist.SuggestValues(s.db, field, needle) + if err != nil { + return nil, fmt.Errorf("suggest smart playlist values: %w", err) + } + + return values, nil +} + // PreviewSmartPlaylist evaluates a rule set from raw JSON without // requiring a saved playlist. This powers live preview in the rule // editor — the frontend sends rules as they are being edited and diff --git a/backend/smartplaylist/smartplaylist.go b/backend/smartplaylist/smartplaylist.go index 9705419..9b5b5b1 100644 --- a/backend/smartplaylist/smartplaylist.go +++ b/backend/smartplaylist/smartplaylist.go @@ -614,7 +614,12 @@ const leanTrackQuery = `SELECT af.file_size, af.play_count, COALESCE(af.last_played, '') AS last_played -FROM ( +FROM ` + leanTrackSource + +// leanTrackSource is the joined row every rule's column resolves +// against, aliased af. Shared by Evaluate and SuggestValues so a +// suggestion is always a value a rule on the same field can match. +const leanTrackSource = `( SELECT af.id, af.file_path, @@ -1117,3 +1122,79 @@ func splitGenres(concatenated string) []string { return strings.Split(concatenated, genreDelimiter) } + +// suggestLimit bounds one suggestion answer: a combobox shows a +// screenful, and the user narrows by typing. +const suggestLimit = 50 + +// errNoSuggestions is a field the editor does not offer values for. +var errNoSuggestions = errors.New("field has no suggestions") + +// suggestFields are the fields whose values the rule editor suggests. +// Every one is in fieldMap; numeric fields other than the years are +// ranges, where a list of every value present is no help. +var suggestFields = map[string]bool{ + "title": true, "artist": true, "album": true, "genre": true, + "composer": true, "file_type": true, "year": true, "release_year": true, +} + +// SuggestValues returns up to suggestLimit distinct values of field +// present in the library that contain needle (case-insensitively), +// sorted. +// +// The rule editor used to build these lists from libraryStore's +// whole-library arrays, which made the editor one more reason to fetch +// every track at startup (#279) — and gave it an empty list when the +// arrays had not landed. Asking for the values that match what has +// been typed is a few hundred bytes, whatever the library's size. +func SuggestValues(db *database.DB, field, needle string) ([]string, error) { + col, ok := fieldMap[field] + if !ok || !suggestFields[field] { + return nil, fmt.Errorf("%w: %q", errNoSuggestions, field) + } + + pattern := "%" + escapeLike(needle) + "%" + + // SAFETY: col comes from fieldMap, never from the caller; the + // needle is a bound parameter. + query := `SELECT DISTINCT CAST(af.` + col + ` AS TEXT) AS v FROM ` + + leanTrackSource + ` + WHERE v != '' AND v != '0' AND v LIKE ? ESCAPE '\' + ORDER BY v COLLATE NOCASE LIMIT ?` + + if field == "genre" { + query = `SELECT name FROM genres + WHERE name != '' AND name LIKE ? ESCAPE '\' + ORDER BY name COLLATE NOCASE LIMIT ?` + } + + rows, err := db.QueryContext(query, pattern, suggestLimit) + if err != nil { + return nil, fmt.Errorf("suggest %s: %w", field, err) + } + + defer func() { _ = rows.Close() }() + + values := make([]string, 0, suggestLimit) + + for rows.Next() { + var v string + if err := rows.Scan(&v); err != nil { + return nil, fmt.Errorf("suggest %s: %w", field, err) + } + + values = append(values, v) + } + + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("suggest %s: %w", field, err) + } + + return values, nil +} + +// escapeLike makes needle match itself literally inside a LIKE pattern +// with ESCAPE '\'. +func escapeLike(needle string) string { + return strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`).Replace(needle) +} diff --git a/backend/smartplaylist/suggest_test.go b/backend/smartplaylist/suggest_test.go new file mode 100644 index 0000000..2496947 --- /dev/null +++ b/backend/smartplaylist/suggest_test.go @@ -0,0 +1,61 @@ +package smartplaylist + +import ( + "errors" + "slices" + "testing" + + "yellowjacket/backend/database" +) + +// The rule editor's value box asks the backend for what matches the +// text typed (#279), rather than building every list from the whole +// library loaded into the frontend. +func TestSuggestValues(t *testing.T) { + t.Parallel() + + db := database.NewTestDB(t) + seedSmartPlaylistData(t, db) + + cases := []struct { + field, needle string + want []string + }{ + // Case-insensitive substring, distinct, sorted. + {"artist", "q", []string{"QOTSA", "Queen"}}, + {"album", "OPERA", []string{"A Night at the Opera"}}, + // Genres come from the genre table, one value per genre, not + // from a concatenated per-track column. + {"genre", "rock", []string{ + "Funk Rock", "Hard Rock", "Progressive Rock", "Rock", "Stoner Rock", + }}, + // Years are numbers and are suggested as their text. + {"year", "199", []string{"1990", "1991"}}, + // The needle is literal: a LIKE wildcard in it matches only itself. + {"title", "%", []string{}}, + {"composer", "_", []string{}}, + } + + for _, tc := range cases { + got, err := SuggestValues(db, tc.field, tc.needle) + if err != nil { + t.Fatalf("SuggestValues(%q, %q): %v", tc.field, tc.needle, err) + } + + if !slices.Equal(got, tc.want) { + t.Errorf("SuggestValues(%q, %q) = %q, want %q", tc.field, tc.needle, got, tc.want) + } + } +} + +func TestSuggestValuesRefusesFieldsItDoesNotSuggest(t *testing.T) { + t.Parallel() + + db := database.NewTestDB(t) + + for _, field := range []string{"duration", "file_path", "title; DROP TABLE x"} { + if _, err := SuggestValues(db, field, ""); !errors.Is(err, errNoSuggestions) { + t.Errorf("SuggestValues(%q) err = %v, want errNoSuggestions", field, err) + } + } +} diff --git a/frontend/bindings/yellowjacket/backend/playlist/service.ts b/frontend/bindings/yellowjacket/backend/playlist/service.ts index d3cde05..efb7dec 100644 --- a/frontend/bindings/yellowjacket/backend/playlist/service.ts +++ b/frontend/bindings/yellowjacket/backend/playlist/service.ts @@ -309,6 +309,14 @@ export function SearchLibrary(query: string): $CancellablePromise<$models.Candid return $Call.ByID(3912116995, query); } +/** + * SuggestSmartPlaylistValues returns values of field present in the + * library that contain needle, for the rule editor's value box. + */ +export function SuggestSmartPlaylistValues(field: string, needle: string): $CancellablePromise { + return $Call.ByID(3718198115, field, needle); +} + /** * ToggleDefaultPlaylistTrack adds or removes a single track * from the default playlist. Returns true if the track is now diff --git a/frontend/src/components/combobox/combobox.ts b/frontend/src/components/combobox/combobox.ts index 94b166e..9df22c5 100644 --- a/frontend/src/components/combobox/combobox.ts +++ b/frontend/src/components/combobox/combobox.ts @@ -7,6 +7,11 @@ import { designTokens } from '../../styles/tokens.css'; * keyboard navigation. Accepts a flat `options` string array, filters as * the user types, and emits `combobox-change` when a value is selected. * + * It also emits `combobox-input` with `{ text }` whenever the text it + * filters by changes (typing, and the reset to empty on focus), so a + * host whose options are too many to hand over at once can fetch the + * ones matching what was typed instead (#279). + * * Key implementation detail: option `
  • ` elements use `@mousedown` with * `e.preventDefault()` so that the input's `blur` event does not close the * dropdown before the click registers. @@ -196,6 +201,7 @@ export class YjCombobox extends LitElement { this.filterText = input.value; this.open = true; this.highlightedIndex = -1; + this.announceInput(); } private handleFocus() { @@ -203,6 +209,17 @@ export class YjCombobox extends LitElement { this.filterText = ''; this.open = true; this.highlightedIndex = -1; + this.announceInput(); + } + + private announceInput() { + this.dispatchEvent( + new CustomEvent('combobox-input', { + detail: { text: this.filterText }, + bubbles: true, + composed: true, + }), + ); } private handleBlur() { diff --git a/frontend/src/components/smart-playlist-editor/smart-playlist-editor.ts b/frontend/src/components/smart-playlist-editor/smart-playlist-editor.ts index c74d668..c515810 100644 --- a/frontend/src/components/smart-playlist-editor/smart-playlist-editor.ts +++ b/frontend/src/components/smart-playlist-editor/smart-playlist-editor.ts @@ -1,8 +1,12 @@ import { LitElement, html, css, nothing } from 'lit'; import { customElement, property, state } from 'lit/decorators.js'; import * as library from '@go/library/models.js'; -import { PreviewSmartPlaylist } from '@go/playlist/service.js'; -import { libraryStore } from '@store/library-store'; +import { + PreviewSmartPlaylist, + SuggestSmartPlaylistValues, +} from '@go/playlist/service.js'; +import { list } from '@utils/binding'; +import { LRUMap } from '@utils/lru-map'; import { describeError } from '@utils/describe-error'; import { designTokens } from '../../styles/tokens.css'; import '@components/combobox/combobox.ts'; @@ -91,51 +95,23 @@ function formatOperatorLabel(op: string): string { return op.replace(/_/g, ' '); } -/** Returns autocomplete suggestions for a given field from libraryStore. */ -function getAutocompleteOptions(field: string): string[] { - switch (field) { - case 'artist': - return libraryStore.getCachedArtists()?.map((a) => a.Name) ?? []; - case 'genre': - return libraryStore.getCachedGenres()?.map((g) => g.Name) ?? []; - case 'album': - return libraryStore.getCachedAlbums()?.map((a) => a.Name) ?? []; - case 'title': { - const tracks = libraryStore.getCachedTracks(); - if (!tracks) return []; - return [...new Set(tracks.map((t) => t.TrackName).filter(Boolean))]; - } - case 'composer': { - const tracks = libraryStore.getCachedTracks(); - if (!tracks) return []; - return [...new Set(tracks.map((t) => t.Composer).filter(Boolean))]; - } - case 'file_type': { - const tracks = libraryStore.getCachedTracks(); - if (!tracks) return []; - return [...new Set(tracks.map((t) => t.FileType).filter(Boolean))]; - } - case 'year': - case 'release_year': { - // Both year fields draw suggestions from the set of years - // present in the library. The cached Track only carries the - // display (original) year, so it seeds both datalists — the - // list is just a hint, and the real filter runs server-side. - const tracks = libraryStore.getCachedTracks(); - if (!tracks) return []; - return [ - ...new Set( - tracks - .map((t) => t.Year) - .filter((y) => y > 0) - .map(String), - ), - ].sort(); - } - default: - return []; - } -} +/** + * Fields whose value box suggests values present in the library. Must + * match `suggestFields` in backend/smartplaylist; any other field gets + * a plain box. + * + * Suggestions are asked for as the user types (#279): they used to be + * built from libraryStore's whole-library arrays, which made this + * editor one more reason to load every track at startup, and gave it + * empty lists whenever those arrays had not landed. + */ +const SUGGEST_FIELDS = new Set([ + 'title', 'artist', 'album', 'genre', 'composer', 'file_type', + 'year', 'release_year', +]); + +/** How long typing must pause before suggestions are asked for. */ +const SUGGEST_DEBOUNCE_MS = 120; /** * Overrides for fields whose title-cased name would be ambiguous. The @@ -206,6 +182,17 @@ export class SmartPlaylistEditor extends LitElement { private previewTimer: ReturnType | null = null; + /** The value suggestions each rule row is showing, by row index. */ + @state() private suggestions = new Map(); + + private suggestTimer: ReturnType | null = null; + + /** Answers already fetched this session, by field and typed text. */ + private suggestCache = new LRUMap(100); + + /** The latest request per row, so a slow answer cannot overwrite a newer one. */ + private suggestWanted = new Map(); + // ── Styles ────────────────────────────────────────────────────── static override styles = [ @@ -599,6 +586,7 @@ export class SmartPlaylistEditor extends LitElement { // Reset value when field changes to avoid stale autocomplete data row.value = ''; row.value2 = ''; + this.dropSuggestions(); this.ruleRows = [...this.ruleRows]; this.onRulesChanged(); @@ -628,6 +616,56 @@ export class SmartPlaylistEditor extends LitElement { this.onRulesChanged(); } + /** Ask for the values of row `index`'s field that contain `text`. */ + private requestSuggestions(index: number, text: string) { + const field = this.ruleRows[index]?.field ?? ''; + + if (!SUGGEST_FIELDS.has(field)) return; + + const key = `${field}\u0000${text}`; + + this.suggestWanted.set(index, key); + + const cached = this.suggestCache.get(key); + + if (cached) { + this.setSuggestions(index, cached); + + return; + } + + if (this.suggestTimer !== null) clearTimeout(this.suggestTimer); + + this.suggestTimer = setTimeout(() => { + this.suggestTimer = null; + void list(SuggestSmartPlaylistValues(field, text)) + .then((values) => { + this.suggestCache.set(key, values); + + if (this.suggestWanted.get(index) === key) { + this.setSuggestions(index, values); + } + }) + .catch((err: unknown) => { + // A suggestion is a hint; the box still takes any text. + console.error('smart playlist: suggestions failed', err); + }); + }, SUGGEST_DEBOUNCE_MS); + } + + private setSuggestions(index: number, values: readonly string[]) { + const next = new Map(this.suggestions); + + next.set(index, values); + this.suggestions = next; + } + + /** Row indexes and fields changed: what was suggested no longer applies. */ + private dropSuggestions() { + this.suggestions = new Map(); + this.suggestWanted.clear(); + } + private updateValue2(index: number, newValue: string) { const row = this.ruleRows[index]; if (!row) return; @@ -644,6 +682,7 @@ export class SmartPlaylistEditor extends LitElement { private removeRule(index: number) { if (this.ruleRows.length <= 1) return; this.ruleRows = this.ruleRows.filter((_, i) => i !== index); + this.dropSuggestions(); this.onRulesChanged(); } @@ -892,7 +931,9 @@ export class SmartPlaylistEditor extends LitElement { ` : html` ) => + this.requestSuggestions(index, e.detail.text)} .value=${row.value} placeholder=${isAnyOf ? 'Comma-separated values' diff --git a/frontend/test/components/smart-playlist-suggestions.test.ts b/frontend/test/components/smart-playlist-suggestions.test.ts new file mode 100644 index 0000000..0c113da --- /dev/null +++ b/frontend/test/components/smart-playlist-suggestions.test.ts @@ -0,0 +1,86 @@ +/** + * The rule editor's value box suggests values from the library by + * asking for the ones that match what has been typed (#279). + * + * It used to build the lists from `libraryStore`'s whole-library + * arrays: one more reason to load every track at startup, and an empty + * list whenever they had not loaded. These tests stub *no* library + * collection — what is asserted is that the suggestions arrive anyway, + * from the one call made for them. + */ +import { describe, expect, it, beforeEach } from 'vitest'; + +import '@components/smart-playlist-editor/smart-playlist-editor'; +import type { YjCombobox } from '@components/combobox/combobox'; +import { calls, resetHarness, stub } from '@test/support/harness'; +import { fixture, shadow, shadowAll } from '@test/support/render'; +import type { LitElement } from 'lit'; + +const SUGGEST = 'playlist.Service.SuggestSmartPlaylistValues'; + +async function editor(field: string): Promise { + return fixture('smart-playlist-editor', { + rules: JSON.stringify({ rules: [{ field, operator: 'contains', value: '' }] }), + }); +} + +/** The row's second combobox: the value box (the first picks the field). */ +function valueBox(el: LitElement): YjCombobox { + return shadowAll(el, 'yj-combobox')[1]!; +} + +async function type(box: YjCombobox, text: string): Promise { + const input = shadow(box, 'input')!; + + input.focus(); + input.value = text; + input.dispatchEvent(new Event('input', { bubbles: true })); + await box.updateComplete; +} + +describe('smart playlist value suggestions', () => { + beforeEach(() => { + resetHarness(); + stub(SUGGEST, (field: string, needle: string) => + [`${field}:${needle}:1`, `${field}:${needle}:2`], + ); + }); + + it('asks for the values matching what was typed, once typing pauses', async () => { + const el = await editor('artist'); + const box = valueBox(el); + + await type(box, 'q'); + await type(box, 'qu'); + + await expect.poll(() => box.options).toEqual(['artist:qu:1', 'artist:qu:2']); + // Debounced: the pause after "qu" asked, the keystroke before did not. + expect(calls(SUGGEST).map((c) => c.args)).toEqual([['artist', 'qu']]); + }); + + it('answers a repeat from what it already fetched', async () => { + const el = await editor('album'); + const box = valueBox(el); + + await type(box, 'x'); + await expect.poll(() => box.options).toEqual(['album:x:1', 'album:x:2']); + await type(box, 'xy'); + await expect.poll(() => box.options).toEqual(['album:xy:1', 'album:xy:2']); + await type(box, 'x'); + + expect(box.options).toEqual(['album:x:1', 'album:x:2']); + expect(calls(SUGGEST)).toHaveLength(2); + }); + + it('asks nothing for a field that has no suggestions', async () => { + const el = await editor('duration'); + const boxes = shadowAll(el, 'yj-combobox'); + + // A numeric range field may render no value combobox at all; if it + // does, typing in it must not ask. + if (boxes[1]) await type(boxes[1], '3'); + await new Promise((r) => setTimeout(r, 250)); + + expect(calls(SUGGEST)).toHaveLength(0); + }); +});