perf(smart-playlist): suggest rule values for what was typed
The rule editor's value box built its suggestions from libraryStore's whole-library arrays, which made it one more reason to load every track at startup, and left it empty when they had not loaded. SuggestSmartPlaylistValues answers with up to 50 distinct values of a field that contain the typed text, from the same joined row the rules match against, so a suggestion is always something a rule can match. yj-combobox announces its filter text so the editor can ask, debounced and cached per field and text. Refs #279
This commit is contained in:
1 parent
3f23bb4396
commit
3cddf70c4d
7 files changed
+356
-49
No files matched your search
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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<string[] | null> {
|
||||
return $Call.ByID(3718198115, field, needle);
|
||||
}
|
||||
|
||||
/**
|
||||
* ToggleDefaultPlaylistTrack adds or removes a single track
|
||||
* from the default playlist. Returns true if the track is now
|
||||
|
||||
@@ -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 `<li>` 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() {
|
||||
|
||||
@@ -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<typeof setTimeout> | null = null;
|
||||
|
||||
/** The value suggestions each rule row is showing, by row index. */
|
||||
@state() private suggestions = new Map<number, readonly string[]>();
|
||||
|
||||
private suggestTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
|
||||
/** Answers already fetched this session, by field and typed text. */
|
||||
private suggestCache = new LRUMap<string, readonly string[]>(100);
|
||||
|
||||
/** The latest request per row, so a slow answer cannot overwrite a newer one. */
|
||||
private suggestWanted = new Map<number, string>();
|
||||
|
||||
// ── 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`
|
||||
<yj-combobox
|
||||
.options=${getAutocompleteOptions(row.field)}
|
||||
.options=${this.suggestions.get(index) ?? []}
|
||||
@combobox-input=${(e: CustomEvent<{ text: string }>) =>
|
||||
this.requestSuggestions(index, e.detail.text)}
|
||||
.value=${row.value}
|
||||
placeholder=${isAnyOf
|
||||
? 'Comma-separated values'
|
||||
|
||||
@@ -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<LitElement> {
|
||||
return fixture<LitElement>('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<YjCombobox>(el, 'yj-combobox')[1]!;
|
||||
}
|
||||
|
||||
async function type(box: YjCombobox, text: string): Promise<void> {
|
||||
const input = shadow<HTMLInputElement>(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<YjCombobox>(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);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user