Genre index: sort A–Z, and repair casing damage at scan time #124

Merged
bvandeusen merged 2 commits from dev into main 2026-08-07 21:40:33 -04:00
5 changed files with 302 additions and 11 deletions
+96 -3
View File
@@ -89,7 +89,7 @@ func normaliseGenreValue(v string) []string {
for strings.HasPrefix(v, "(") { for strings.HasPrefix(v, "(") {
// "((" is the spec's escape for a literal "(" — the rest is plain text. // "((" is the spec's escape for a literal "(" — the rest is plain text.
if strings.HasPrefix(v, "((") { if strings.HasPrefix(v, "((") {
return append(out, strings.TrimSpace(v[1:])) return append(out, trueUpCasing(strings.TrimSpace(v[1:])))
} }
end := strings.IndexByte(v, ')') end := strings.IndexByte(v, ')')
if end < 0 { if end < 0 {
@@ -106,7 +106,7 @@ func normaliseGenreValue(v string) []string {
if err != nil { if err != nil {
// Parenthesised but not a reference, e.g. "(Live)". Keep the // Parenthesised but not a reference, e.g. "(Live)". Keep the
// whole remainder as written. // whole remainder as written.
return append(out, v) return append(out, trueUpCasing(v))
} }
if name, ok := id3v1GenreName(n); ok { if name, ok := id3v1GenreName(n); ok {
out = append(out, name) out = append(out, name)
@@ -120,11 +120,104 @@ func normaliseGenreValue(v string) []string {
} }
if n, err := strconv.Atoi(v); err == nil { if n, err := strconv.Atoi(v); err == nil {
if name, ok := id3v1GenreName(n); ok { if name, ok := id3v1GenreName(n); ok {
// Canonical table name, deliberately NOT re-cased — see id3v1Genres.
return append(out, name) return append(out, name)
} }
return out 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) { func id3v1GenreName(n int) (string, bool) {
+99
View File
@@ -419,3 +419,102 @@ func equalStrings(a, b []string) bool {
} }
return true 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)
}
}
+5 -1
View File
@@ -52,7 +52,11 @@ var audioExtensions = map[string]bool{
// ("Alternative Rock" + "Rock" -> "Alternative RockRock"), which corrupted // ("Alternative Rock" + "Rock" -> "Alternative RockRock"), which corrupted
// the genre browse axis and polluted the taste profile's tag vocabulary, // the genre browse axis and polluted the taste profile's tag vocabulary,
// and left bare ID3v1 numeric references unresolved (#2499). // 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 { type Stats struct {
Scanned int `json:"scanned"` Scanned int `json:"scanned"`
+49 -5
View File
@@ -30,6 +30,31 @@
return genres.filter((g: GenreCount) => g.genre.toLowerCase().includes(q)); 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 albums = $state<AlbumRef[]>([]);
let total = $state(0); let total = $state(0);
let loading = $state(false); let loading = $state(false);
@@ -153,12 +178,30 @@
<h1 class="font-display text-2xl font-medium text-text-primary">Genres</h1> <h1 class="font-display text-2xl font-medium text-text-primary">Genres</h1>
{#if !index.isPending && !index.isError} {#if !index.isPending && !index.isError}
<p class="text-sm text-text-secondary"> <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 {genres.length} {genres.length === 1 ? 'genre' : 'genres'}, straight from your file tags
{/if}
</p> </p>
{/if} {/if}
</div> </div>
{#if genres.length > 0} {#if genres.length > 0}
<div class="flex flex-wrap items-center gap-2">
<QuickFilter bind:value={filter} placeholder="Filter genres" /> <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">AZ</option>
</select>
</label>
</div>
{/if} {/if}
</header> </header>
@@ -180,16 +223,17 @@
</a> </a>
{/snippet} {/snippet}
</EmptyState> </EmptyState>
{:else if filter.trim() && filteredGenres.length === 0} {:else if filter.trim() && visibleGenres.length === 0}
<p class="text-text-secondary"> <p class="text-text-secondary">
No genres match <span class="font-medium">'{filter.trim()}'</span>. No genres match <span class="font-medium">'{filter.trim()}'</span>.
</p> </p>
{:else} {:else}
<!-- Ordered by track count, not alphabetically: raw tags carry a long <!-- Defaults to track count rather than alphabetical: raw tags carry a
tail of one-offs, so alphabetical would bury the handful of genres long tail of one-offs, so A-Z would bury the handful of genres you
you actually have a library's worth of. --> 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"> <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> <li>
<a <a
href={genreHref(g.genre)} href={genreHref(g.genre)}
@@ -98,6 +98,57 @@ describe('/library/genres index', () => {
expect(screen.getByRole('link', { name: /^rock 2$/ })).toBeInTheDocument(); 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. AZ is the ask.
test('AZ 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', () => { test('empty library explains where genres come from', () => {
asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] })); asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] }));
render(GenresPage); render(GenresPage);