Merge pull request 'Genre index: sort A–Z, and repair casing damage at scan time' (#124) from dev into main

Operator decision: keep the ID3v1 table canonical, fix the casing.

The operator's library carries "Edm", "Idm", "Aor", "Uk Garage", "Uk
Hardcore", "Trap Edm", "Glitch Hop Edm" and "Children'S Music" — an external
tag editor title-cased the whole genre field. The "'S" is the giveaway.

Fixed at SCAN time, not in the display layer: taste_profile.sql reads
tracks.genre directly, so a cosmetic-only fix would leave the taste
vocabulary holding "Edm" while the UI showed "EDM", and any correctly
tagged file would contribute a second, separate tag.

trueUpCasing only ever changes case, never letters, so it cannot silently
turn one genre into a different one — that is what separates it from the
label-remapping idea this task rejected. Two narrow rules:

- A short, evidence-led acronym list, matched case-insensitively so "edm",
  "Edm" and "EDM" all land on "EDM". This is a deliberate 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 is a judgement about
  labels, whereas there is no genre named "Edm".
- Apostrophe suffixes from a FIXED contraction list, so "Children'S" is
  repaired while "O'Brien" and "D'Angelo" keep their capital. A blanket
  "lowercase after an apostrophe" would have broken both.

Matching uses the word's letter core rather than the raw word, so "(Edm)"
and "Edm," are repaired and their punctuation re-attached. Interior
punctuation stays in the core, so "Lo-Fi" and "R&B" are compared whole and
cannot match a fragment by accident. My first version missed this and a test
expecting "(Live EDM)" caught it.

Names resolved from the ID3v1 table are deliberately NOT re-cased, per the
operator's call — entry 40's "AlternRock" stays as the table spells it, with
a test pinning that so a later tidy-up doesn't quietly "fix" it.

tagReadVersion 1 -> 2, so this reaches the existing library on the next scan
rather than new files only. That re-read reuses stored durations, so it costs
tag reads and no ffprobe.

.gitea/workflows/release.yml

@@ -98,6 +98,31 @@ jobs:
           echo "code=${COMMIT_COUNT}" >> "$GITHUB_OUTPUT"
           echo "::notice::APK version: ${VERSION_NAME} (code=${COMMIT_COUNT})"
 
+      # Checked BEFORE the expensive work, not after it. "Attach APK to gitea
+      # Release" below resolves the release by tag and fails if it is absent —
+      # but that is the final step, so a tag pushed without a release built an
+      # APK for several minutes first and only then discovered it had nowhere to
+      # put it. Same check, seconds in instead of minutes.
+      #
+      # Releases are normally created through the API (which creates the tag and
+      # the release together, so this passes). A bare `git push origin vX` is the
+      # case this catches.
+      - name: Release must exist for this tag
+        shell: bash
+        working-directory: ${{ github.workspace }}
+        env:
+          CI_TOKEN: ${{ secrets.CI_TOKEN }}
+        run: |
+          set -euo pipefail
+          TAG="${GITHUB_REF#refs/tags/}"
+          if ! curl -fsSL -o /dev/null \
+            -H "Authorization: token ${CI_TOKEN}" \
+            "https://git.fabledsword.com/api/v1/repos/${GITHUB_REPOSITORY}/releases/tags/${TAG}"; then
+            echo "::error::no release exists for ${TAG}. Create the release (which creates the tag) rather than pushing a bare tag — otherwise there is nothing to attach the APK to."
+            exit 1
+          fi
+          echo "::notice::release found for ${TAG}"
+
       - name: Cache Gradle dirs
         uses: actions/cache@v4
         with:
@@ -322,3 +347,79 @@ jobs:
           docker buildx build \
             --build-arg MINSTREL_VERSION="${{ steps.tags.outputs.version }}" \
             --push ${{ steps.tags.outputs.args }} .
+
+  # Verifies a tag release actually ended up complete, and names the specific
+  # thing that's missing if not.
+  #
+  # Added 2026-08-07 after v2026.08.07 was re-cut. The android-release job never
+  # started — no log was written at all — so all eight of its steps reported
+  # `failure` with none executed and image-release showed `skipped`. The run was
+  # red, but the *release page rendered fine*, and `main`'s own push build had
+  # already moved `:latest`, so the code was deployable and nothing looked
+  # obviously wrong. The release was simply missing its APK and its immutable
+  # `:vYYYY.MM.DD` image, which is easy to skim past.
+  #
+  # This job cannot prevent that (the cause was a runner failing to launch, not
+  # anything in this file). What it does is turn an incomplete release into an
+  # explicit, named error instead of eight mystery step failures — so the
+  # consequence is legible without having to infer it.
+  #
+  # `if: always()` is the whole point: it has to report precisely when the jobs
+  # above did NOT succeed.
+  verify-release:
+    name: Verify release artifacts (tag releases only)
+    needs: [android-release, image-release]
+    if: ${{ always() && startsWith(github.ref, 'refs/tags/v') }}
+    runs-on: go-ci
+    container:
+      image: git.fabledsword.com/bvandeusen/ci-go:1.26
+
+    steps:
+      - name: Release must have an APK attached
+        shell: bash
+        env:
+          CI_TOKEN: ${{ secrets.CI_TOKEN }}
+        run: |
+          set -euo pipefail
+          TAG="${GITHUB_REF#refs/tags/}"
+          REPO="${GITHUB_REPOSITORY}"
+
+          REL_JSON="$(curl -fsSL \
+            -H "Authorization: token ${CI_TOKEN}" \
+            "https://git.fabledsword.com/api/v1/repos/${REPO}/releases/tags/${TAG}" || true)"
+          if [ -z "${REL_JSON}" ]; then
+            echo "::error::no release found for ${TAG} — the tag exists but nothing was published"
+            exit 1
+          fi
+
+          APK="$(printf '%s' "${REL_JSON}" \
+            | grep -oP '"browser_download_url":\s*"\K[^"]+' \
+            | grep -E '\.apk$' | head -1 || true)"
+          if [ -z "${APK}" ]; then
+            echo "::error::release ${TAG} has NO APK attached — in-app update will offer nothing, and the bundled-APK path on future :latest builds has no source."
+            echo "::error::Fix by RE-RUNNING this workflow run. Do NOT delete and re-create the tag; if it fails again the runner never started the container, and the evidence is in act_runner on the host (Gitea will hold no job log)."
+            exit 1
+          fi
+
+          echo "::notice::APK attached: ${APK}"
+
+      # The other half. Checking only the APK would report success on a release
+      # whose image push failed — which is precisely the second thing that was
+      # missing when v2026.08.07 had to be re-cut. `always()` on this job means
+      # it runs even when image-release failed, so without this the guard would
+      # cheerfully verify an incomplete release.
+      - name: Immutable image tag must exist
+        shell: bash
+        run: |
+          set -euo pipefail
+          TAG="${GITHUB_REF#refs/tags/}"
+          IMAGE="git.fabledsword.com/bvandeusen/minstrel"
+
+          echo "${{ secrets.CI_TOKEN }}" \
+            | docker login git.fabledsword.com -u "${{ github.actor }}" --password-stdin
+
+          if ! docker manifest inspect "${IMAGE}:${TAG}" > /dev/null 2>&1; then
+            echo "::error::image ${IMAGE}:${TAG} was never pushed — the release tag has no immutable image, so there is nothing to pin or roll back to. Re-run this workflow run."
+            exit 1
+          fi
+          echo "::notice::image verified: ${IMAGE}:${TAG}"

internal/library/genre.go

@@ -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) {

internal/library/genre_test.go

@@ -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)
+	}
+}

internal/library/scanner.go

@@ -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"`

web/src/routes/library/genres/+page.svelte

@@ -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)}

web/src/routes/library/genres/genres.test.ts

@@ -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);