Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
aa9f534f3c | ||
|
|
8e1d25a772 | ||
|
|
4509f740f8 |
@@ -89,7 +89,7 @@ func normaliseGenreValue(v string) []string {
|
||||
for strings.HasPrefix(v, "(") {
|
||||
// "((" is the spec's escape for a literal "(" — the rest is plain text.
|
||||
if strings.HasPrefix(v, "((") {
|
||||
return append(out, strings.TrimSpace(v[1:]))
|
||||
return append(out, trueUpCasing(strings.TrimSpace(v[1:])))
|
||||
}
|
||||
end := strings.IndexByte(v, ')')
|
||||
if end < 0 {
|
||||
@@ -106,7 +106,7 @@ func normaliseGenreValue(v string) []string {
|
||||
if err != nil {
|
||||
// Parenthesised but not a reference, e.g. "(Live)". Keep the
|
||||
// whole remainder as written.
|
||||
return append(out, v)
|
||||
return append(out, trueUpCasing(v))
|
||||
}
|
||||
if name, ok := id3v1GenreName(n); ok {
|
||||
out = append(out, name)
|
||||
@@ -120,11 +120,104 @@ func normaliseGenreValue(v string) []string {
|
||||
}
|
||||
if n, err := strconv.Atoi(v); err == nil {
|
||||
if name, ok := id3v1GenreName(n); ok {
|
||||
// Canonical table name, deliberately NOT re-cased — see id3v1Genres.
|
||||
return append(out, name)
|
||||
}
|
||||
return out
|
||||
}
|
||||
return append(out, v)
|
||||
return append(out, trueUpCasing(v))
|
||||
}
|
||||
|
||||
// genreAcronyms are tokens that belong in caps. Tag editors that title-case the
|
||||
// whole genre field turn "EDM" into "Edm" and "UK Garage" into "Uk Garage", and
|
||||
// the operator's library carries all of these (#2468).
|
||||
//
|
||||
// Matched case-INSENSITIVELY, so "edm", "Edm" and "EDM" all land on "EDM". That
|
||||
// is a deliberate, narrow exception to the project's rule that genre case is
|
||||
// exposed as the file says it: "Rock" and "rock" still stay separate rows,
|
||||
// because folding those would be a judgement about labels. Fixing a token that
|
||||
// is unambiguously an initialism is not — there is no genre named "Edm".
|
||||
//
|
||||
// Kept short and evidence-led. Add a token here only when a real library shows
|
||||
// it damaged; a speculative list risks mangling a word that legitimately looks
|
||||
// like an acronym.
|
||||
var genreAcronyms = map[string]string{
|
||||
"edm": "EDM",
|
||||
"idm": "IDM",
|
||||
"aor": "AOR",
|
||||
"uk": "UK",
|
||||
"us": "US",
|
||||
"ebm": "EBM",
|
||||
}
|
||||
|
||||
// contractionSuffixes are the word-endings that follow an apostrophe in normal
|
||||
// English. Title-casing capitalises the letter after ANY non-letter, which is
|
||||
// how "Children's Music" became "Children'S Music".
|
||||
//
|
||||
// Deliberately a fixed list rather than "lowercase whatever follows an
|
||||
// apostrophe": that broader rule would break "O'Brien" and "D'Angelo", which are
|
||||
// correctly capitalised after the apostrophe.
|
||||
var contractionSuffixes = map[string]bool{
|
||||
"s": true, "t": true, "re": true, "ll": true, "ve": true, "d": true, "m": true,
|
||||
}
|
||||
|
||||
// trueUpCasing repairs casing damage done by tag editors that title-case the
|
||||
// genre field. It only ever changes case, never the letters — so it cannot
|
||||
// silently turn one genre into a different one, which is what separates it from
|
||||
// the label-remapping idea #2468 rejected.
|
||||
func trueUpCasing(s string) string {
|
||||
if s == "" {
|
||||
return s
|
||||
}
|
||||
words := strings.Split(s, " ")
|
||||
for i, w := range words {
|
||||
if w == "" {
|
||||
continue
|
||||
}
|
||||
// Match the word's letter core, not the raw word, so surrounding
|
||||
// punctuation doesn't hide the acronym: "(Edm)" and "Edm," both need
|
||||
// fixing, and re-attaching the trimmed edges keeps them intact.
|
||||
lead, core, trail := splitWordCore(w)
|
||||
if fixed, ok := genreAcronyms[strings.ToLower(core)]; ok {
|
||||
words[i] = lead + fixed + trail
|
||||
continue
|
||||
}
|
||||
words[i] = fixApostrophe(w)
|
||||
}
|
||||
return strings.Join(words, " ")
|
||||
}
|
||||
|
||||
// splitWordCore peels leading and trailing non-alphanumerics off a word.
|
||||
// Interior punctuation stays in the core, so "Lo-Fi" and "R&B" are compared
|
||||
// whole rather than being split into fragments that might match by accident.
|
||||
func splitWordCore(w string) (lead, core, trail string) {
|
||||
isCore := func(r rune) bool {
|
||||
return (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9')
|
||||
}
|
||||
start := 0
|
||||
for start < len(w) && !isCore(rune(w[start])) {
|
||||
start++
|
||||
}
|
||||
end := len(w)
|
||||
for end > start && !isCore(rune(w[end-1])) {
|
||||
end--
|
||||
}
|
||||
return w[:start], w[start:end], w[end:]
|
||||
}
|
||||
|
||||
// fixApostrophe lowercases a capitalised contraction or possessive suffix:
|
||||
// "Children'S" -> "Children's". Leaves "O'Brien" alone, since "Brien" is not a
|
||||
// contraction suffix.
|
||||
func fixApostrophe(w string) string {
|
||||
idx := strings.LastIndexByte(w, '\'')
|
||||
if idx <= 0 || idx == len(w)-1 {
|
||||
return w
|
||||
}
|
||||
suffix := w[idx+1:]
|
||||
if !contractionSuffixes[strings.ToLower(suffix)] {
|
||||
return w
|
||||
}
|
||||
return w[:idx+1] + strings.ToLower(suffix)
|
||||
}
|
||||
|
||||
func id3v1GenreName(n int) (string, bool) {
|
||||
|
||||
@@ -419,3 +419,102 @@ func equalStrings(a, b []string) bool {
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func TestTrueUpCasing(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
in string
|
||||
want string
|
||||
}{
|
||||
// The damage actually present in the operator's library (#2468).
|
||||
{"edm acronym", "Edm", "EDM"},
|
||||
{"idm acronym", "Idm", "IDM"},
|
||||
{"aor acronym", "Aor", "AOR"},
|
||||
{"uk prefix", "Uk Garage", "UK Garage"},
|
||||
{"uk hardcore", "Uk Hardcore", "UK Hardcore"},
|
||||
{"acronym mid-phrase", "Trap Edm", "Trap EDM"},
|
||||
{"acronym at the end", "Glitch Hop Edm", "Glitch Hop EDM"},
|
||||
{"possessive", "Children'S Music", "Children's Music"},
|
||||
|
||||
// Already correct input must be left exactly alone.
|
||||
{"correct acronym", "EDM", "EDM"},
|
||||
{"correct possessive", "Children's Music", "Children's Music"},
|
||||
|
||||
// Case-insensitive, so a lower-cased tag also lands on the canonical
|
||||
// form rather than becoming a third variant.
|
||||
{"lowercase acronym", "edm", "EDM"},
|
||||
|
||||
// Names with an apostrophe followed by a real word are NOT contractions
|
||||
// and must keep their capital — this is why the suffix list is fixed
|
||||
// rather than "lowercase anything after an apostrophe".
|
||||
{"irish surname", "O'Brien Core", "O'Brien Core"},
|
||||
{"french elision", "D'Angelo Soul", "D'Angelo Soul"},
|
||||
|
||||
// Ordinary genres pass through untouched. Case is otherwise exposed as
|
||||
// the file says it — "Rock" vs "rock" stays a real distinction.
|
||||
{"plain", "Alternative Rock", "Alternative Rock"},
|
||||
{"lowercase plain", "rock", "rock"},
|
||||
{"hyphenated", "Lo-Fi Hip Hop", "Lo-Fi Hip Hop"},
|
||||
{"ampersand", "R&B", "R&B"},
|
||||
{"empty", "", ""},
|
||||
|
||||
// Punctuation around a word must not hide the acronym inside it.
|
||||
{"parenthesised acronym", "Hip Hop (Edm)", "Hip Hop (EDM)"},
|
||||
{"acronym with comma", "Edm, Trap", "EDM, Trap"},
|
||||
|
||||
// Interior punctuation stays in the core, so these are compared whole
|
||||
// and cannot match a fragment by accident.
|
||||
{"hyphenated stays whole", "Lo-Fi", "Lo-Fi"},
|
||||
{"ampersand stays whole", "Drum & Bass", "Drum & Bass"},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := trueUpCasing(tc.in); got != tc.want {
|
||||
t.Errorf("trueUpCasing(%q) = %q, want %q", tc.in, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// The casing repair must never touch a name resolved from the ID3v1 table. The
|
||||
// operator chose to keep that table canonical, so entry 40's 1990s spelling
|
||||
// "AlternRock" stays as-is even though it reads like damage.
|
||||
func TestNormaliseGenreValue_CanonicalTableNamesNotRecased(t *testing.T) {
|
||||
if got := normaliseGenreValue("40"); !equalStrings(got, []string{"AlternRock"}) {
|
||||
t.Errorf("bare 40 = %q, want [AlternRock]", got)
|
||||
}
|
||||
if got := normaliseGenreValue("(40)"); !equalStrings(got, []string{"AlternRock"}) {
|
||||
t.Errorf("(40) = %q, want [AlternRock]", got)
|
||||
}
|
||||
}
|
||||
|
||||
// Casing runs on values that came from the file, including the parenthesised
|
||||
// and refinement paths.
|
||||
func TestNormaliseGenreValue_CasingAppliedToFileText(t *testing.T) {
|
||||
tests := []struct {
|
||||
in string
|
||||
want []string
|
||||
}{
|
||||
{"Edm", []string{"EDM"}},
|
||||
{"(17)Uk Garage", []string{"Rock", "UK Garage"}},
|
||||
// Punctuation around the acronym must not hide it — the letter core is
|
||||
// what gets matched, and the trimmed edges are re-attached.
|
||||
{"(Live Edm)", []string{"(Live EDM)"}},
|
||||
{"((Children'S Music", []string{"(Children's Music"}},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.in, func(t *testing.T) {
|
||||
if got := normaliseGenreValue(tc.in); !equalStrings(got, tc.want) {
|
||||
t.Errorf("normaliseGenreValue(%q) = %q, want %q", tc.in, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Two spellings of one acronym in the same file collapse to a single tag rather
|
||||
// than surviving as near-duplicates.
|
||||
func TestNormaliseGenres_AcronymVariantsDedupe(t *testing.T) {
|
||||
if got := normaliseGenres([]string{"Edm", "EDM", "edm"}); !equalStrings(got, []string{"EDM"}) {
|
||||
t.Errorf("genres = %q, want [EDM]", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -52,7 +52,11 @@ var audioExtensions = map[string]bool{
|
||||
// ("Alternative Rock" + "Rock" -> "Alternative RockRock"), which corrupted
|
||||
// the genre browse axis and polluted the taste profile's tag vocabulary,
|
||||
// and left bare ID3v1 numeric references unresolved (#2499).
|
||||
const tagReadVersion int16 = 1
|
||||
// 2: acronym and apostrophe casing repaired on genre values (#2468) — "Edm" ->
|
||||
// "EDM", "Children'S Music" -> "Children's Music". Bumped rather than left
|
||||
// to new files only because taste_profile.sql reads tracks.genre directly,
|
||||
// so a half-repaired library would carry both spellings as separate tags.
|
||||
const tagReadVersion int16 = 2
|
||||
|
||||
type Stats struct {
|
||||
Scanned int `json:"scanned"`
|
||||
|
||||
@@ -30,6 +30,31 @@
|
||||
return genres.filter((g: GenreCount) => g.genre.toLowerCase().includes(q));
|
||||
});
|
||||
|
||||
// Count-first by default because that's what the server returns and it's the
|
||||
// right default: the head of this list is genuinely where you're going. But a
|
||||
// real library runs to several hundred genres with a long tail of one-offs
|
||||
// (391 / ~3.7 per track on the operator's), and at that size "I know roughly
|
||||
// what it's called" needs A-Z as much as filtering does.
|
||||
//
|
||||
// View state only, not a query parameter — same as `filter` above. Neither is
|
||||
// worth making shareable, and putting one in the URL and not the other would
|
||||
// be the inconsistent choice.
|
||||
let sortMode = $state<'count' | 'name'>('count');
|
||||
|
||||
const visibleGenres = $derived.by(() => {
|
||||
// Copy before sorting. With no filter applied `filteredGenres` IS the array
|
||||
// held by the query cache, and Array.sort mutates in place — sorting it
|
||||
// directly would reorder cached data for every other consumer.
|
||||
const list = [...filteredGenres];
|
||||
if (sortMode === 'name') {
|
||||
// sensitivity 'base' so case and accents don't split neighbours apart.
|
||||
return list.sort((a: GenreCount, b: GenreCount) =>
|
||||
a.genre.localeCompare(b.genre, undefined, { sensitivity: 'base' })
|
||||
);
|
||||
}
|
||||
return list; // server order: count DESC, then name
|
||||
});
|
||||
|
||||
let albums = $state<AlbumRef[]>([]);
|
||||
let total = $state(0);
|
||||
let loading = $state(false);
|
||||
@@ -153,12 +178,30 @@
|
||||
<h1 class="font-display text-2xl font-medium text-text-primary">Genres</h1>
|
||||
{#if !index.isPending && !index.isError}
|
||||
<p class="text-sm text-text-secondary">
|
||||
{#if filter.trim()}
|
||||
{visibleGenres.length} of {genres.length} genres
|
||||
{:else}
|
||||
{genres.length} {genres.length === 1 ? 'genre' : 'genres'}, straight from your file tags
|
||||
{/if}
|
||||
</p>
|
||||
{/if}
|
||||
</div>
|
||||
{#if genres.length > 0}
|
||||
<div class="flex flex-wrap items-center gap-2">
|
||||
<QuickFilter bind:value={filter} placeholder="Filter genres" />
|
||||
<label class="flex items-center gap-1.5 text-xs text-text-secondary">
|
||||
Sort
|
||||
<select
|
||||
bind:value={sortMode}
|
||||
aria-label="Sort genres"
|
||||
class="rounded border border-border bg-surface px-2 py-1 text-sm text-text-primary
|
||||
focus-visible:outline focus-visible:outline-2 focus-visible:outline-accent"
|
||||
>
|
||||
<option value="count">Most tracks</option>
|
||||
<option value="name">A–Z</option>
|
||||
</select>
|
||||
</label>
|
||||
</div>
|
||||
{/if}
|
||||
</header>
|
||||
|
||||
@@ -180,16 +223,17 @@
|
||||
</a>
|
||||
{/snippet}
|
||||
</EmptyState>
|
||||
{:else if filter.trim() && filteredGenres.length === 0}
|
||||
{:else if filter.trim() && visibleGenres.length === 0}
|
||||
<p class="text-text-secondary">
|
||||
No genres match <span class="font-medium">'{filter.trim()}'</span>.
|
||||
</p>
|
||||
{:else}
|
||||
<!-- Ordered by track count, not alphabetically: raw tags carry a long
|
||||
tail of one-offs, so alphabetical would bury the handful of genres
|
||||
you actually have a library's worth of. -->
|
||||
<!-- Defaults to track count rather than alphabetical: raw tags carry a
|
||||
long tail of one-offs, so A-Z would bury the handful of genres you
|
||||
actually have a library's worth of. The Sort control lets you ask for
|
||||
A-Z when you already know roughly what you're looking for. -->
|
||||
<ul class="grid grid-cols-1 gap-2 sm:grid-cols-2 lg:grid-cols-3">
|
||||
{#each filteredGenres as g (g.genre)}
|
||||
{#each visibleGenres as g (g.genre)}
|
||||
<li>
|
||||
<a
|
||||
href={genreHref(g.genre)}
|
||||
|
||||
@@ -98,6 +98,57 @@ describe('/library/genres index', () => {
|
||||
expect(screen.getByRole('link', { name: /^rock 2$/ })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// #2468: the operator's real library is 391 genres at ~3.7 per track, so the
|
||||
// count-first default buries anything you can already name. A–Z is the ask.
|
||||
test('A–Z sort reorders the list without mutating the source array', async () => {
|
||||
const data = [
|
||||
{ genre: 'Rock', track_count: 8974 },
|
||||
{ genre: 'Ambient', track_count: 646 },
|
||||
{ genre: 'jazz', track_count: 1406 }
|
||||
];
|
||||
asMock(createGenresQuery).mockReturnValue(mockQuery({ data }));
|
||||
render(GenresPage);
|
||||
|
||||
const order = () =>
|
||||
screen.getAllByRole('link').map((a) => a.textContent?.trim().split(/\s+/)[0]);
|
||||
|
||||
// Count mode PRESERVES the server's order rather than re-sorting client
|
||||
// side — the server already returns count DESC, name ASC, and duplicating
|
||||
// that here would be two orderings to keep in step. The fixture is
|
||||
// deliberately not in count order so this asserts pass-through, not luck.
|
||||
expect(order()).toEqual(['Rock', 'Ambient', 'jazz']);
|
||||
|
||||
await fireEvent.change(screen.getByLabelText('Sort genres'), { target: { value: 'name' } });
|
||||
|
||||
// Case-insensitive, so 'jazz' sorts between Ambient and Rock rather than
|
||||
// after both.
|
||||
expect(order()).toEqual(['Ambient', 'jazz', 'Rock']);
|
||||
|
||||
// The query cache's own array must not have been reordered in place —
|
||||
// Array.sort mutates, and with no filter applied the derived list IS that
|
||||
// array.
|
||||
expect(data.map((d) => d.genre)).toEqual(['Rock', 'Ambient', 'jazz']);
|
||||
});
|
||||
|
||||
test('filtering reports the visible subset against the total', async () => {
|
||||
asMock(createGenresQuery).mockReturnValue(
|
||||
mockQuery({
|
||||
data: [
|
||||
{ genre: 'Rock', track_count: 10 },
|
||||
{ genre: 'Ska Punk', track_count: 5 },
|
||||
{ genre: 'Jazz', track_count: 3 }
|
||||
]
|
||||
})
|
||||
);
|
||||
render(GenresPage);
|
||||
expect(screen.getByText(/3 genres, straight from your file tags/)).toBeInTheDocument();
|
||||
|
||||
await fireEvent.input(screen.getByLabelText('Filter genres'), { target: { value: 'punk' } });
|
||||
|
||||
// QuickFilter debounces by 120ms.
|
||||
await waitFor(() => expect(screen.getByText('1 of 3 genres')).toBeInTheDocument());
|
||||
});
|
||||
|
||||
test('empty library explains where genres come from', () => {
|
||||
asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] }));
|
||||
render(GenresPage);
|
||||
|
||||
Reference in New Issue
Block a user