Compare commits

..
Author SHA1 Message Date
logan 905654cc84 feat(explore): demote the album page's version selector to a disclosure
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m26s
CI / e2e (pull_request) Successful in 5m59s
Choosing which pressing you are looking at is an advanced,
metadata-repair task, and it sat directly above the tracklist with a
heading, a `<select>` and a paragraph explaining how our clustering
picks a "standard version" by weighing release count, status and date.
That is a sentence about our own heuristic in the most valuable space
on the page.

It is now "Other versions of this album (N)" below the tracklist: a
real `<button aria-expanded aria-controls>` inside the heading that
names the section, with the body rendered unconditionally and toggled
with `hidden`, because `aria-controls` has to name an element that is
in the DOM. Both rules are `config-section`'s rather than new ones.
It is demoted, not removed — matching the wrong release is a real
problem and this is how it gets fixed.

**Two more blocks shared that slot and neither was guarded.** The
selector at least had `distinctTracklistCount() <= 1`; the
`Versions / Loading releases…` spinner and the `Versions / <error>`
block did not, so both took the primary position on every album
regardless of whether there was ever going to be a choice. The spinner
said what `renderTracklist` was already saying about the same fetch, so
it is gone. The error was the one `catalog-scope-notice` shows at the
top of the page with a retry — every path that sets `errorReleases`
also sets `catalogFailed`, the only route to `unavailable`.

That error is what made this a rewrite rather than a move.
`renderTracklist` returned `nothing` on `errorReleases` and leaned on
the selector's own block to have said it, and a control inside a
collapsed disclosure cannot be a page's error surface. The failure
belongs to the list that is missing because of it, so that is where it
is drawn.

**What must not be lost is which version is on screen.** The default is
what the header already describes, so saying it on every album would be
this issue's own complaint one size smaller. `defaultVersionKey` is the
test: a line appears above the tracklist only once someone has chosen
another, naming it and offering the way back. The ★ and the words "in
your library" survive unchanged inside the panel, and the panel does
not close when the selection changes — a panel that shuts on use cannot
be used twice.

The `<select>` also loses an `aria-label` of "Select release version"
that outranked its own visible `<label>Version</label>`, which is a
label not in the name.

Verified against the running app as well as the suite: the collapsed
page, the open panel, a chosen version and 390px width all read
correctly, and the shell still measures 390 in a 390 viewport.

Closes #17
2026-08-19 01:24:49 -04:00
logan 219fa3c615 Merge pull request 'Make it obvious everywhere when you are looking at things you do not own' (#117) from feat/38-ownership-visibility into main
CI / check (push) Successful in 2m28s
CI / e2e (push) Successful in 6m3s
Owned is plain; unowned is dimmed, named and requestable; a partly-held
album says how partly. Ownership is a file (`localId`), never the
`in_library` ratchet.

Closes #38
2026-08-19 05:11:43 +00:00
logan c4e055ce51 docs: write down which of the two ownership columns to read
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m26s
CI / e2e (pull_request) Successful in 6m15s
The `localId` / `inLibrary` choice outlives #38 — every future catalog
surface has to make it, and the code read them as an OR at eight call
sites precisely because nothing said they were different kinds of
thing. CLAUDE.md gets the rule and its four load-bearing details;
NOTES.md gets the measurement, the card that used both answers at once,
and the alternative that was rejected.
2026-08-19 00:50:53 -04:00
logan 10eca353ab fix(explore): gate playback on the same answer the row is drawn from
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m53s
CI / e2e (pull_request) Successful in 6m24s
Two play paths still accepted `inLibrary`, so a row drawn dimmed and
`aria-disabled` by the new rule would still attempt to play and fail
with "this track could not be found in your library" — the disagreement
this pass exists to remove, one layer down from the badge.
2026-08-19 00:39:18 -04:00
logan 88fc50afb8 feat(explore): mark what is not owned, everywhere it can be shown
`explore-album-details` had the rule right for one tracklist and
nothing else did: Explore's cards, `top-results-row` and the artist
page's three card shapes all mixed owned and unowned with a small badge
as the only difference, and drew a green tick on the *common* case —
which is the treatment that tracklist's own green ticks were removed
for.

`utils/ownership.ts` is the rule written once, so eight call sites
stop each holding their own version:

- owned is plain, and draws no badge at all;
- unowned is dimmed *and* says so in its accessible name, because
  dimming is a colour and cannot be the only signal;
- a partly-held album says how partly.

**Ownership is a file, and `localId` is the flag that says so.** The
album page answers with `filePaths`, a real file per displayed track; a
card grid cannot afford that and does not need to, because
`local_*_id` is built by queries that all join `audio_files` and
cleared by a prune whose existence test is a file test in every case.
`inLibrary` is written by the same pass, so the two agree in a healthy
database — but it is a one-way ratchet (`MAX(in_library, excluded)`)
whose only clearing pass is gated on a non-null local id, so it cannot
be un-set on its own.

Where they already diverged was the client. Both `explore-view` and
`explore-artist-details` kept a `libraryMBIDs` set that accumulated
every MBID ever seen with `inLibrary` and cleared it never, in views
that never unmount. Both are deleted.

And one card answered the question twice and got two answers:
`renderReleaseMenuItems` gates Play on `localId > 0` while the badge
and `albumTarget.owned` used `inLibrary`, so an album with the flag and
no local row drew a tick saying it was in your library, offered no
Play, and — the request item being gated on *not* owned — offered no
way to ask for it either.

The count comes from `completenessStore`, shaped like `credit-store`:
`request()` is per-card and coalesces a screenful into one
`GetAlbumsCompleteness`, absence is cached as an answer, and the whole
cache is dropped on a scan, a retag or a removal rather than aged.

`aria-disabled` goes on rows that cannot be activated and deliberately
not on cards: an unowned card still navigates to the catalog page for
it, which is a perfectly good thing to do with something you do not
own.

Audited and unchanged: `home-view`, `downloads-view`, `cover-grid`,
`artist-details` and `genre-details` cannot show catalog content, so
everything on them is owned and "owned is plain" is already what they
do. The album page's own header badge stays, because that page is about
one entity and the badge is its answer rather than a mark on one of
many.

Closes #38
2026-08-19 00:38:22 -04:00
logan 19c68d73a7 fix(ui): keep the count in a partial badge that can act
A control is named after what activating it does, so an actionable
badge said "Request album X" — and `partial` is actionable, because an
album you hold nine of twelve tracks of has three left to ask for.
That made the one state the ring exists for the one state whose name
did not mention it.

The argument the `partial` branch already carries does not stop
applying because the badge became clickable: a ring says "some" to a
sighted user and nothing to anyone else. The name is now the action and
the count.
2026-08-19 00:38:05 -04:00
logan 41c41a860e feat(explore): carry the local row id on a top result
`TopResult` was the one projection here that shipped `inLibrary` and no
local id, so the top-results cards had no choice but to read the weaker
flag. Every sibling model — `MBArtist`, `MBReleaseGroup`, `MBRecording`
— already carries `LocalID`, and the candidate builders had the value
in hand at every construction site.

`LocalID` is set and cleared by a test against `audio_files`, so it
means "there is something of mine here". `InLibrary` is written by the
same pass but is a one-way ratchet the prune can only clear alongside a
local id; it stays for scoring, which is where an approximate answer is
fine.
2026-08-19 00:37:57 -04:00
logan 4bf59b45b7 feat(library): answer album completeness for a screenful in one query
A card grid has to know how much of an album is here — an album held 2
tracks of 10 wearing the same green tick as one held whole is the
complaint the badge-accuracy work was filed about — and
`GetAlbumCompleteness` is one query per album, which is fifty round
trips for a grid of fifty.

`GetAlbumsCompleteness` is the same question over a slice. It is two
grouping levels rather than the single-album form's correlated
subqueries, because a correlated subquery in the FROM clause is not
something SQLite will reliably do, and because the slice may only be
spelled once or sqlc expands it twice with independently numbered
placeholders.

An album with no files is absent from the result rather than zeroed:
"I have none of this" and "I have no idea" are the third state `Known`
exists to keep apart.

The test that matters is that the two spellings never disagree — they
are genuinely different SQL, so the risk is a drift in meaning (a
disc's total counted once per file, a duplicate counted twice) rather
than a typo.
2026-08-19 00:37:46 -04:00
logan fc99d9e0d7 Merge pull request 'Make a release a shipment rather than a merge' (#116) from ci/115-manual-release into main
CI / check (push) Successful in 2m24s
CI / e2e (push) Successful in 6m11s
Closes #115
2026-08-19 02:36:01 +00:00
logan 90f1239fba ci: make a release a shipment rather than a merge
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m25s
CI / e2e (pull_request) Successful in 6m13s
release.yml fired on every push to main, so the trigger was "a PR was
merged" and nothing else decided. That is a version per unit of *work*
rather than per *shipment*: eight releases in twenty-two hours, v0.0.1
through v0.3.1, for one session -- each fanning out to four publishers on
a runner with capacity 1, so roughly forty packaging jobs shipped three
issues while ordinary PR CI queued behind them. pacman, Homebrew and
Obtainium see every one.

The push trigger is gone and workflow_dispatch, which was already there
and already worked, is the whole mechanism. Nothing else had to change to
batch releases, because semantic-release already reads every commit since
the last tag: five fixes and two feats become one minor release with all
seven in the notes. Release frequency was only ever how often this file
fired.

This is the rule index-artifact.yml states and is the other instance of:
a job that mutates state which cannot be rebuilt in ten minutes is
triggered deliberately, not by a push. A release here is a tag, a Gitea
release, an Arch package, a Homebrew formula, a signed APK and desktop
assets -- and an Android version going backwards costs the user their
library.

`dry_run` is what makes a manual trigger usable: the point of pulling a
lever by hand is being able to look first, so the input runs
semantic-release --dry-run -- the version and the notes, no tag, no
release, no publishers. Anything but the literal string "true" releases
for real, because a typo in a dispatch box must not silently turn a
shipment into a green no-op.

Two alternatives were considered and rejected, both recorded on the
issue. A `beta` integration branch relocates the trigger rather than
removing one: it needs a second protected branch carrying the same
required checks, and it *adds* a full check + e2e run per batch on the
very runner whose queue is the complaint. A schedule batches without
anyone having to remember, but puts the decision back on a timer, which
is the thing being removed.

Closes #115
2026-08-18 22:25:11 -04:00
logan b2fe1cb1e0 ci: skip a prerelease tag in all four publishers
Their trigger is `v*`, which matches `v0.4.0-beta.1`. They guarded
`v0.0.0` -- the version floor -- and nothing else, so the first
prerelease tag would have published a beta everywhere.

Nothing produces one today. The guard is here because the thing that
would is `prerelease: true` in .releaserc.yml, a one-line change whose
blast radius is four public channels and which nothing in those four
files mentions. That is the same argument release.yml's `chore(release):`
guard is kept on: cheap, against something a future edit turns on
somewhere else entirely.

android-apk is the worst of the four twice over. Its APK goes to the
*generic* registry, which is readable without credentials so Obtainium
can poll a plain URL, so a beta would be offered to every device on it.
And its versionCode maths splits on dots: it would read "1" out of
"0-beta" and produce a wrong number rather than a failed build, which
matters because Android orders releases by that integer and refuses
anything not greater than what is installed.

Each is a clean skip rather than a failure, matching the v0.0.0 guard
beside it: a red run against a tag that was never meant to ship is noise.
2026-08-18 22:25:11 -04:00
logan 065a879190 Merge pull request 'Give the icons one vocabulary and sweep the call sites' (#114) from feat/34-icon-language into main
Release / release (push) Successful in 32s
Build & publish Arch package / arch-package (push) Successful in 2m35s
Attach the desktop build to the release / linux (push) Successful in 56s
Sync Homebrew formula / sync-formula (push) Successful in 6s
CI / e2e (push) Successful in 6m17s
CI / check (push) Successful in 3m8s
Build & publish the Android APK / apk (push) Successful in 1m29s
Closes #34
2026-08-19 01:37:34 +00:00
logan 89882b4863 refactor(ui): give the icons one vocabulary and sweep the call sites
CI / check (pull_request) Successful in 2m27s
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / e2e (pull_request) Successful in 6m42s
`plus` meant "add to the queue", "add to a playlist", "make a new
playlist" and "you do not own this" -- the first two adjacent in the
same context menu, so two neighbouring items were the same glyph doing
different things. `list` meant the queue (the button that opens it), the
Playlists destination, and adding to the queue in `queue-panel` alone.
Two icons carrying seven meanings is not a vocabulary, and nothing
catches it: a wrong-but-real icon renders perfectly.

`utils/icon-language.ts` is the table, beside `library-status.ts` as the
issue suggested. The rule it is built on is that an icon names the
**noun** it acts on, not the verb: "add to queue" and "add to playlist"
are one verb on two nouns, so the noun is what differs -- which is why
adding to a playlist wears the Playlists destination's own icon, and why
the queue took `bars-staggered` and stopped wearing Playlists'. `plus`
keeps the one meaning it is unambiguous about, making something that is
not there yet, which covers New Playlist and the drop zones.

`bars-staggered` is the only new glyph, vendored through names.txt and
fetch-icons.mjs after confirming it is in Font Awesome **Free** 7.3.1.

Two things this found rather than changed:

- The request toggle's outline/solid pair was already in the app and
  already right -- `explore-album-details`'s "Request this" button has
  used `regular/bookmark` -> `solid/bookmark` since it was written --
  while the badge forty pixels away showed a **plus** for the same
  state. That is `utils/library-status.ts`'s fault one layer down: it
  made the two surfaces agree on what wanting *means* and left them
  disagreeing on what it looks like.
- `explore-artist-details`'s Follow button was `bookmark-check`, which
  is Font Awesome **Pro** and has never been bundled, so it has drawn
  the missing-icon fallback -- a circled question mark -- for every
  followed artist since it was written. `requested-badge.spec.ts` was
  written for exactly this bug on the album button and says so in its
  docstring; this is the same bug one component over, still live,
  because `offline-icons.spec.ts` sweeps `__yjIconMisses` and no spec
  had ever followed an artist.

So the test does what reaching the state cannot. `icon-language.test.ts`
reads every `src/**/*.ts` as raw text and fails on a governed name
written outside the table, and separately asserts every `ICON_*` is a
*bundled* name -- which is what makes a Pro name a failing test rather
than a runtime report from a state something has to reach first. Its
first assertion is that it read any source at all, because a sweep over
an empty glob passes.

`chrome.test.ts` asserted `['check', 'bookmark', 'plus']` and so pinned
the badge's glyphs against the vocabulary they were meant to follow; it
names them from the table now, and keeps the assertion that the three
differ, which is the property the states actually need.

Downloads keeps the solid bookmark on purpose. That is one word twice,
not two words: the badge says the entity is on your list and the nav
item is that list.

Closes #34
2026-08-18 21:18:36 -04:00
logan 18a08daa91 Merge pull request 'Let the album page be asked for the whole tracklist' (#113) from feat/7-full-tracklist-toggle into main
Release / release (push) Successful in 33s
Build & publish Arch package / arch-package (push) Successful in 2m45s
Attach the desktop build to the release / linux (push) Successful in 1m2s
Sync Homebrew formula / sync-formula (push) Successful in 7s
CI / e2e (push) Successful in 6m18s
CI / check (push) Successful in 2m25s
Build & publish the Android APK / apk (push) Successful in 1m34s
Closes #7
2026-08-19 01:10:34 +00:00
logan aa59773d22 feat(explore): let the album page be asked for the whole tracklist
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m25s
CI / e2e (pull_request) Successful in 6m12s
An album the user holds part of showed only the tracks on disk, with
nothing to say the rest existed. The page could already draw the full
release with the missing rows dimmed -- it just could not be asked: the
automatic rule fires on `completeness.known`, which depends on the files
declaring a per-disc total, or failing that on the catalog's own
`total_tracks`.

Neither reaches most albums. #16 fixed the first input for anything
tagged from now on, and the second is worse than it looks: the published
artifact is from 2026-08-10 and the column landed on 08-16, so
`completenessAnswer()`'s catalog fallback answers 0 for every user until
the index job republishes. Measured, and noted on #88, which is the
publish that carries it.

So the control is explicit. A "Show the whole album" switch flips the
synthetic "Your Library" entry between the local files and the release,
which is the same rendering, reached deliberately rather than inferred.

Three things about it are load-bearing:

- `showFullTracklist` is a tri-state, `null` meaning "follow the
  automatic rule". The rule is right when it fires, and the switch has
  to agree with the page it is sitting on rather than starting out
  contradicting it -- a plain boolean would need its default recomputed
  every time the completeness answer moved underneath it. The user
  outranks the rule in both directions.
- `fullReleaseCluster()` falls back to the highest-scoring cluster.
  `findLibraryCluster` is a guess over the `inLibrary` flags and returns
  nothing at all when none are set, which is exactly the untagged
  library this exists for -- without the fallback the control would be
  absent precisely where it is needed. The sublabel names the release
  either way rather than leaving the user to wonder whose tracklist they
  are reading.
- It appears only where it can change what is on screen: against the
  library entry, with a release to switch to, and only when the two
  tracklists differ. A complete album's release has the same rows as its
  files, so the switch would redraw the same list and read as broken --
  the same test the version dropdown one section up already answers.

The accessible name is asserted rather than assumed, through the
browser's own computation. `wa-switch` happens to get it right, and for
a third reason again: its `<input role="switch">` sits inside a native
`<label>` that also holds the `<slot>`, so the name is computed across
the flattened tree from light-DOM text. This app has shipped the
opposite twice.

Closes #7
2026-08-18 20:49:08 -04:00
logan a4777f26b6 Merge pull request 'Declare the track and disc totals when tagging' (#105) from fix/16-tagwriter-totals into main
Release / release (push) Successful in 30s
CI / e2e (push) Successful in 6m8s
CI / check (push) Successful in 2m23s
Build & publish the Android APK / apk (push) Successful in 1m25s
Build & publish Arch package / arch-package (push) Successful in 2m31s
Attach the desktop build to the release / linux (push) Successful in 54s
Sync Homebrew formula / sync-formula (push) Successful in 6s
Closes #16
2026-08-19 00:45:07 +00:00
logan 92faa9741b Merge branch 'main' into fix/16-tagwriter-totals
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m22s
CI / e2e (pull_request) Successful in 6m8s
2026-08-18 20:32:08 -04:00
yonlu bf0a53e64c Merge pull request 'Give the unclaim step a CA bundle' (#110) from fix/unclaim-ca-certs into main
Release / release (push) Successful in 31s
CI / e2e (push) Successful in 6m3s
CI / check (push) Successful in 2m22s
Build & publish the Android APK / apk (push) Successful in 1m24s
Build & publish Arch package / arch-package (push) Successful in 2m29s
Attach the desktop build to the release / linux (push) Successful in 52s
Sync Homebrew formula / sync-formula (push) Successful in 6s
Reviewed-on: #110
2026-08-18 23:30:31 +00:00
yonlu 7be4a02e31 fix(ci): give the unclaim step a CA bundle
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m27s
CI / e2e (pull_request) Successful in 6m14s
Second defect in the same workflow. The shell fix took -- the step ran
under `bash --noprofile --norc -e -o pipefail` -- and got one layer
further before failing:

  curl: (77) error setting certificate file: /etc/ssl/certs/ca-certificates.crt

ubuntu:24.04 ships no CA bundle, and --no-install-recommends skips the
ca-certificates that curl recommends, so curl came up unable to verify
TLS against our own Gitea.

This was avoidable by reading the repo rather than reasoning about it:
ci.yml (twice), desktop-assets.yml, android-apk.yml and release.yml all
spell out `ca-certificates curl ... jq` for exactly this reason. The
convention was written down five times already.

Validated in the real image this time rather than by extracting the
script and running it on the host, which is what missed this: the step
now succeeds inside `docker run ubuntu:24.04` against a scratch issue --
label present, 204, label gone -- and the previous version reproduces
`curl: (77)` in the same image. Both checked, then the scratch issue was
deleted.

The DELETE also keeps its response body now and prints it on a non-204.
Whether the automatic token carries issue-write scope is still unproven,
because both failures happened before the API call, and "403" without
Gitea's own sentence would cost another merge to interpret.

Closes #102
2026-08-18 19:08:50 -04:00
yonlu ad9c25a5a2 Merge pull request 'Run the unclaim step under bash' (#108) from fix/unclaim-shell into main
Release / release (push) Successful in 32s
CI / e2e (push) Successful in 6m10s
CI / check (push) Successful in 2m23s
Build & publish the Android APK / apk (push) Successful in 1m23s
Build & publish Arch package / arch-package (push) Successful in 2m37s
Attach the desktop build to the release / linux (push) Successful in 52s
Sync Homebrew formula / sync-formula (push) Successful in 7s
Reviewed-on: #108
2026-08-18 23:05:38 +00:00
yonlu a83a127e31 fix(ci): run the unclaim step under bash
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m28s
CI / e2e (pull_request) Successful in 6m14s
The workflow shipped in #103 and failed on every close, on its second
line, before reaching the API:

  shell: sh -e {0}
  /var/run/act/workflow/0.sh: 2: set: Illegal option -o pipefail

Inside `container:` the act runner selects sh, not bash, and
`set -o pipefail` is a bashism. homebrew-formula.yml carries the same
line without trouble because it runs with no container, on the host
image where bash is the default -- so "another workflow does it" was
not the evidence it looked like, and the comment now says so where the
next person will read it.

pipefail is kept rather than dropped for POSIX's sake: the lookup is
`curl -sSf ... | jq`, so without it an API error yields empty output,
an empty label id, and a cheerful "nothing to do" on every close. A
silent no-op is the one outcome worse than a failing job here.

Validated end to end against scratch issues rather than by reading it:
with the label present the step returns 204 and the label is gone, and
against an issue that never carried it the step also returns 204 and
exits 0 -- which is what makes it safe to run on every close rather
than only claimed ones.

Still untested: whether secrets.GITEA_TOKEN carries issue-write scope.
The old run never got far enough to find out. If it 403s, the fix is
one line -- secrets.PACKAGE_TOKEN, which is a user PAT.

Closes #102
2026-08-18 18:54:39 -04:00
logan 4b9114fd8d fix(tagwriter): declare the track and disc totals when tagging
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m46s
CI / e2e (pull_request) Successful in 6m18s
An album the user holds 2 of 10 tracks of showed a green tick reading
"is in your library", and the mechanism was our own writer. tagwriter
wrote track and disc *numbers* and dropped the totals, so autotagging a
folder made the release MBID-matched -- which is what earns the tick --
while erasing the one field GetAlbumCompleteness reads. The evidence
for "2 of 10" was destroyed by the act that produced the tick.

FieldTotalTracks and FieldTotalDiscs are written as the ID3 "n/N" form
and as Vorbis TRACKTOTAL/DISCTOTAL; the autotag apply pass and the
download importer fill them from the release's own tracklist; and
dbsync persists the track total to audio_files.total_tracks so the
album page agrees with the file without waiting for a rescan.

Five things about it are load-bearing, and four fail silently:

- The total is per *disc*, not per release, because that is what the
  tag form declares and what GetAlbumCompleteness sums per disc. A
  release total on every file multiplies a two-disc album's expectation
  by two, which no library can satisfy. backend/tagtotals is that
  derivation once, since the two callers must not import the writer or
  each other.
- The Vorbis names are TRACKTOTAL and DISCTOTAL and no other spelling.
  dhowden/tag reads exactly those two keys, so TOTALTRACKS -- which
  xiph lists and several taggers write -- or a "1/12" packed into
  TRACKNUMBER writes successfully and reads back as no total at all.
  The tests therefore assert the round trip through the reader the scan
  uses, not through the bytes.
- ID3's number and total share one frame, so writing either alone must
  read the other off the existing tag or discard it. A total with no
  number is not written: "/12" parses as track 0.
- The totals are written unconditionally rather than on a diff. The
  case this exists for is a file declaring no total at all, which
  compares equal to nothing and is exactly what a "only if it changed"
  guard skips.
- A single-track download is not totalled. A RecordingMBID anchor
  resolves Expected to that one track, so the same code would tag a
  track off a twelve-track album "1 of 1" -- and a declared total
  outranks the catalog total that would have answered correctly.

autotag's field constants are a second copy of tagwriter's, deliberately
so autotag stays out of the write pipeline's import graph. A key that
drifts neither fails to compile nor fails to write -- the writer simply
finds nothing under the name it looks for -- so autotagservice, the one
package importing both, now pins them.

Steps 2 and 3 of the issue stay open under #38: the catalog fallback
already landed as completenessAnswer(), and the badge call-site audit is
the part that overlaps it.

Closes #16
2026-08-18 18:19:54 -04:00
yonlu e049a71458 Merge pull request 'Drop the claim label when an issue closes' (#103) from ci/unclaim-on-close into main
CI / check (push) Successful in 2m30s
Release / release (push) Successful in 31s
CI / e2e (push) Successful in 6m6s
Reviewed-on: #103
2026-08-18 21:57:11 +00:00
yonlu 0c944f2382 ci: drop the claim label when an issue closes
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m27s
CI / e2e (pull_request) Successful in 6m17s
A `Closes #N` footer closes the issue on merge and leaves
`Status/In Progress` on it, because Gitea's auto-close touches state
and nothing else. #100 was closed and simultaneously marked as being
actively worked on. `scripts/issue.sh close` does drop the label, and
is exactly the call the footer exists to avoid making.

This hooks the close rather than the merge. Stripping the label in the
PR would work and would be a per-PR habit, which is what the footer
removed in the first place; `issues: [closed]` covers the footer,
issue.sh close and a click in the web UI alike, and asks nothing of
anyone at any of them.

Reopening deliberately does not restore the label: reopening says the
work was not finished, not that somebody is at a keyboard now.

Two costs, both stated in the file rather than discovered later. The
runner has capacity 1 and is shared with an index build that can hold
it for three hours, so this is not instant -- stale for an afternoon
beats stale forever, which is what it was. And it is an eighth
workflow, so CLAUDE.md's count moves with it.

The audit stays, because a workflow that silently stops firing is the
failure mode this area has already produced once:

  ./scripts/issue.sh list --state closed --label "Status/In Progress"

Closes #102
2026-08-18 17:36:05 -04:00
yonlu 75525b67e4 Merge pull request 'Put the closing keyword where Gitea will actually read it' (#101) from docs/closing-keyword into main
CI / check (push) Successful in 2m27s
Release / release (push) Successful in 31s
CI / e2e (push) Successful in 6m15s
Reviewed-on: #101
2026-08-18 21:30:35 +00:00
yonlu 85768dc489 docs: put the closing keyword where Gitea will actually read it
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m27s
CI / e2e (pull_request) Successful in 6m7s
CLAUDE.md said the Closes list was unreliable and to close by hand. It
is unreliable for a specific reason, and the rule can say what works.

Gitea parses commit messages that reach main. It does not parse the PR
body, which closes something only if the merge happens to copy it into
the merge commit message. Both halves were measured here: #83's merge
commit carried "Closes #9, #13, #14, ..." and closed five of the ten,
because a comma list is only partially matched; #93's merge commit body
was a lone Reviewed-on: trailer, so #92 stayed open behind a perfectly
correct Closes line in the PR description.

So the keyword goes in the commit body as a footer, one issue per line.
That costs nothing elsewhere -- Conventional Commits allows a footer,
commit-check only regexes the subject, and semantic-release reads the
type from the subject, so no release decision changes. The existing
rule that the issue number stays out of the subject is untouched and
was never about the body.

The verification step stays, because a squash or a hand-edited merge
message still drops the footer.

This commit is the experiment: if #98 and #100 close when this branch
merges without anyone touching them, the mechanism is confirmed.

Closes #98
Closes #100
2026-08-18 17:15:28 -04:00
yonlu 1a221a40d3 Merge pull request 'Delete four documents that contradict the code' (#99) from docs/retire-stale-planning-docs into main
CI / check (push) Successful in 2m26s
Release / release (push) Successful in 29s
CI / e2e (push) Successful in 6m14s
Reviewed-on: #99
2026-08-18 21:06:59 +00:00
66 changed files with 4017 additions and 314 deletions
+17
View File
@@ -139,6 +139,23 @@ jobs:
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
fi
# Nor is a prerelease, and this trigger is `v*`, which matches
# `v0.4.0-beta.1`. Two reasons it is worst here. The APK goes
# to the *generic* registry, which is readable without
# credentials so Obtainium can poll a plain URL — a beta would
# be offered to every device on it. And the versionCode maths
# below splits on dots and would read "1" out of "0-beta",
# producing a code that is wrong rather than a build that
# fails: Android orders releases by that integer and refuses
# anything not greater than what is installed.
case "$v" in
*-*)
echo "v$v is a prerelease; not publishing an APK for it"
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
;;
esac
echo "skip=false" >> "$GITHUB_OUTPUT"
# Android orders releases by an integer and refuses anything
+16
View File
@@ -72,6 +72,22 @@ jobs:
exit 0
fi
# A prerelease is not a shipment either, and this trigger is
# `v*` — which matches `v0.4.0-beta.1`. Nothing produces one
# today; the guard is here because the thing that would is
# semantic-release's `prerelease: true` channel, a one-line
# change in .releaserc.yml whose blast radius is four public
# package channels. Same argument as release.yml's
# `chore(release):` guard: cheap, against something a future
# edit turns on somewhere else entirely.
case "$v" in
*-*)
echo "$v is a prerelease; not packaging it for pacman"
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
;;
esac
echo "skip=false" >> "$GITHUB_OUTPUT"
echo "tag=$v" >> "$GITHUB_OUTPUT"
echo "building $v"
+13
View File
@@ -95,6 +95,19 @@ jobs:
exit 0
fi
# Nor is a prerelease, and this trigger is `v*`, which matches
# `v0.4.0-beta.1`. The mildest of the four — assets attach to
# the prerelease's own Gitea release and no package manager
# reads them — but four workflows sharing one trigger should
# share one answer about what a shipment is.
case "$v" in
*-*)
echo "$v is a prerelease; not attaching desktop assets"
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
;;
esac
echo "skip=false" >> "$GITHUB_OUTPUT"
echo "tag=$v" >> "$GITHUB_OUTPUT"
echo "version=${v#v}" >> "$GITHUB_OUTPUT"
+12
View File
@@ -56,6 +56,18 @@ jobs:
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
fi
# Nor is a prerelease, and this trigger is `v*`, which matches
# `v0.4.0-beta.1`. It matters most here of the four: the tap
# is public, and `brew upgrade` would offer a beta to everyone
# on it.
case "$VERSION" in
*-*)
echo "$TAG is a prerelease; not syncing it to a public tap"
echo "skip=true" >> "$GITHUB_OUTPUT"
exit 0
;;
esac
echo "skip=false" >> "$GITHUB_OUTPUT"
TARBALL="${SOURCE_TARBALL_BASE}/${TAG}.tar.gz"
+54 -8
View File
@@ -1,11 +1,36 @@
name: Release
# The sixth workflow, and the one that decides whether the other three
# run at all. On every push to main it reads the Conventional Commits
# since the last tag, and if any of them is releasable it writes the
# changelog, pushes the tag, and creates the Gitea release whose body is
# that changelog section. The publishing workflows are keyed on `v*`, so
# the tag push is what starts them.
# run at all. It reads the Conventional Commits since the last tag, and
# if any of them is releasable it writes the changelog, pushes the tag,
# and creates the Gitea release whose body is that changelog section.
# The publishing workflows are keyed on `v*`, so the tag push is what
# starts them.
#
# **It is triggered by hand, and there is deliberately no `push`
# trigger.** There was one, on `main`, which made the trigger "a PR was
# merged" and nothing else: eight releases in twenty-two hours
# (v0.0.1 -> v0.3.1) for one session's work, each fanning out to four
# publishers on a runner with capacity 1, so ~40 packaging jobs shipped
# three issues and ordinary PR CI queued behind them. A version per
# merged PR is a version per unit of *work*, not per *shipment*, and
# pacman, Homebrew and Obtainium see every one.
#
# Nothing else had to change to batch them: semantic-release already
# reads every commit since the last tag, so five fixes and two feats
# become one minor release with all seven in the notes. Release
# frequency was only ever how often this file fired.
#
# This is the rule `index-artifact.yml` states and is the other instance
# of: **a job that mutates state which cannot be rebuilt in ten minutes
# is triggered deliberately, not by a push.** A release here is a tag,
# a Gitea release, an Arch package, a Homebrew formula, a signed APK and
# desktop assets — and an Android version going backwards costs the user
# their library (docs/android-release.md).
#
# A schedule was considered and rejected: a cron batches without anyone
# having to remember, but it puts the decision back on a timer, which is
# the thing being removed.
#
# **Why the tag is pushed with PACKAGE_TOKEN and not the Actions token.**
# Gitea, like GitHub, does not start a workflow from a ref pushed by a
@@ -19,9 +44,12 @@ name: Release
# instead.
on:
push:
branches: [main]
workflow_dispatch:
inputs:
dry_run:
description: "Report what would be released and stop"
required: false
default: "false"
# Cutting a tag is not a thing to cancel halfway: a superseded run must
# finish, not be killed between `git push --tags` and the release POST.
@@ -163,14 +191,32 @@ jobs:
# been right, the tag would have been right, every job would have
# been green, and the release body would have been empty. Check the
# notes, not the exit code, before moving any of these.
# The point of a manual trigger is deliberateness, and deliberate
# means being able to look before pulling the lever. `--dry-run`
# reports the version and the notes and writes nothing: no tag, no
# release, no publishers. `make release-dry` is the same answer
# locally; this is it from the runner, against the same commit and
# the same tag history, which is what actually decides.
- name: Run semantic-release
if: steps.guard.outputs.skip == 'false'
working-directory: /src
env:
DRY_RUN: ${{ inputs.dry_run }}
run: |
set -eu
git config user.name "yellowjacket-ci"
git config user.email "yj@yellowjacket.app"
# Anything but a literal "true" releases for real. A typo in a
# dispatch box must not silently turn a shipment into a no-op
# that reports success — the failure worth avoiding is the one
# where nothing happens and the run is green.
dry=""
if [ "${DRY_RUN:-false}" = "true" ]; then
echo "DRY RUN — no tag will be pushed and no release created"
dry="--dry-run"
fi
npx --yes \
-p semantic-release@25 \
-p @semantic-release/commit-analyzer@13 \
@@ -178,5 +224,5 @@ jobs:
-p @semantic-release/changelog@7 \
-p @semantic-release/exec@7 \
-p conventional-changelog-conventionalcommits@9 \
semantic-release \
semantic-release $dry \
--repository-url "https://x-access-token:${PACKAGE_TOKEN}@${SERVER_URL#https://}/${REPO}.git"
+107
View File
@@ -0,0 +1,107 @@
name: Unclaim
# A `Closes #N` footer in a commit body closes the issue on merge — and
# leaves `Status/In Progress` on it, because Gitea's auto-close touches
# state and nothing else. So #100 was closed and simultaneously marked
# as being actively worked on, and `scripts/issue.sh close` (which does
# drop the label) is exactly the thing the footer exists to avoid
# calling.
#
# **This hooks the close, not the merge.** Stripping the label in the
# PR would work and would be a per-PR habit; habits are what the footer
# removed. `issues: [closed]` covers every path an issue can close by —
# the footer on merge, `issue.sh close`, someone clicking Close in the
# web UI — and asks nothing of anyone at any of them.
#
# **Reopening deliberately does not restore it.** Reopening says the
# work was not finished, not that somebody is at a keyboard doing it
# now; the claim gets re-made by whoever picks it up.
#
# **This is not instant, and should not be described as it.** The
# runner has capacity 1 and is shared with an index build that can hold
# it for three hours, so a label tweak can queue behind one. Stale for
# an afternoon beats stale forever, which is what it was.
#
# The audit that answers "is this still firing" stays in CLAUDE.md and
# is one command:
#
# ./scripts/issue.sh list --state closed --label "Status/In Progress"
#
# A workflow that silently stops working is the failure mode this whole
# area has already produced once.
on:
issues:
types: [closed]
jobs:
unclaim:
runs-on: ubuntu-latest
container:
image: ubuntu:24.04
steps:
- name: Drop the claim label
# **Inside a container the act runner selects `sh`, not bash**, so
# `set -o pipefail` fails the job on its second line with "Illegal
# option" and the step never reaches the API. `homebrew-formula.yml`
# carries the same `set -euo pipefail` without trouble because it
# runs with **no container**, on the host image where bash is the
# default — so "another workflow does it" is not evidence here.
shell: bash
env:
# The automatic Actions token, as release.yml uses for the
# floor tag. It needs no more than write access to this repo.
TOKEN: ${{ secrets.GITEA_TOKEN }}
API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }}
ISSUE: ${{ github.event.issue.number }}
run: |
set -euo pipefail
# `ca-certificates` is named because `--no-install-recommends`
# skips it, and `ubuntu:24.04` ships no CA bundle of its own —
# so curl comes up unable to verify TLS against our own Gitea
# and fails with "error setting certificate file" (exit 77).
# Every other containerised workflow here spells it out for the
# same reason; this one did not, and cost a release cycle.
apt-get update -qq
apt-get install -y -qq --no-install-recommends \
ca-certificates curl jq >/dev/null
label_id=$(
curl -sSf -H "Authorization: token $TOKEN" "$API/labels?limit=100" |
jq -r '.[] | select(.name == "Status/In Progress") | .id'
)
# The label not existing is a repo somebody reorganised, not a
# failure of this run — say so and stop, rather than failing a
# job on every close from then on.
if [ -z "$label_id" ]; then
echo "unclaim: no 'Status/In Progress' label in this repo; nothing to do"
exit 0
fi
# DELETE is idempotent here: an issue that never carried the
# label answers the same as one that did, which is what makes
# this safe to run on *every* close rather than only the ones
# that were claimed.
# The body is captured, not discarded, so a refusal is
# diagnosable from this log alone. Whether the automatic
# token carries issue-write scope is still unproven, and
# "DELETE returned 403" without Gitea's own sentence costs
# another merge to find out which of the two it is.
body=$(mktemp)
code=$(
curl -sS -o "$body" -w '%{http_code}' -X DELETE \
-H "Authorization: token $TOKEN" \
"$API/issues/$ISSUE/labels/$label_id"
)
case "$code" in
204) echo "unclaim: #$ISSUE is closed and unclaimed" ;;
*)
echo "unclaim: DELETE returned $code for #$ISSUE" >&2
cat "$body" >&2
exit 1
;;
esac
+97
View File
@@ -3483,3 +3483,100 @@ public tap.
A guard added today does not protect a tag that points at yesterday. When
re-pointing a tag, check what the workflows looked like *there*.
## A tag reader looks at exactly one spelling of "total" (measured 2026-08-18)
Writing #16's totals means matching the reader, which is
`dhowden/tag`, and it is narrower than the specs are:
- **Vorbis (FLAC, OGG): `TRACKTOTAL` and `DISCTOTAL` only.**
`vorbis.go`'s `Track()` reads `tracknumber` and `tracktotal` and
nothing else, so `TOTALTRACKS` — which several taggers write and
which xiph lists — and a `1/12` packed into `TRACKNUMBER` both read
back as *no total*. They write successfully. Nothing errors.
- **ID3v2 (MP3): `TRCK`/`TPOS` as `n/N`**, via `parseXofN`. That is one
frame carrying two facts, which is why `applyPositionFrame` reads the
existing frame before writing either half.
- **WAV: nothing at all.** There is no RIFF reader in the module, so a
WAV's `id3 ` chunk is invisible to `metadata.ExtractTags` — every
field, not just the totals. Filed as #104.
The general shape, and the reason this is written down: a tag written
under a name the reader does not look at is indistinguishable from one
never written. So the tests assert the round trip through
`metadata.ExtractTags` — the reader the *scan* uses — rather than
through the bytes the writer produced.
## The published catalog artifact predates `total_tracks` (measured 2026-08-18)
```
$ curl -sSI .../generic/yellowjacket-core-index/latest/core-index.db.zst
last-modified: Mon, 10 Aug 2026 04:38:16 GMT
content-length: 75417037
$ sqlite3 core-index.db \
"SELECT COUNT(*) FROM pragma_table_info('explore_index') WHERE name='total_tracks';"
0
$ sqlite3 core-index.db "SELECT COUNT(*) FROM explore_index;"
1079667
```
The column landed in the schema on 2026-08-16; the artifact is from
08-10, and `index-artifact.yml` is a weekly cron, not a push trigger.
So `completenessAnswer()`'s catalog fallback answers 0 for **every**
user today — the machinery is correct and `artifactHasTotals()` is
doing precisely its job, there is just no data behind it. Same position
the credit tables are in; both ride on the next publish (#88).
The general point, which is why this is written down rather than just
fixed: **a probe that makes a column optional also makes its absence
silent.** `artifactHasTotals` and `artifactHasCredits` are both correct
and both mean a feature can ship, pass every test, and produce nothing
for anybody without a single failure anywhere. Checking the *published
file* is one query and is not implied by any tick in CI.
## "Do I own this" has two answers in the schema, and one of them is a flag (2026-08-19)
Decided while doing #38, and it outlives it because every future
catalog surface has to pick one.
`explore_index` carries both `in_library` and `local_artist_id` /
`local_release_group_id` / `local_recording_id`. They are written by
the same pass (`collectLibraryEntities`), so on a healthy database they
agree, and the code read them as an OR — `inLibrary || localId > 0` —
at eight call sites.
They are not the same kind of thing:
- **`local_*_id` is a fact with an owner.** Every query that sets one
joins `audio_files`, and `pruneStaleLocalCrossReferences` clears it
with an existence test that is a file test in all three cases. It is
the same rule `explore-album-details`'s `filePaths` implements, one
layer down and computed once per scan.
- **`in_library` is a ratchet.** `upsertBatch` raises it with
`MAX(in_library, excluded.in_library)` and the prune is the only
thing that lowers it — gated on the local id being non-null, so a row
holding the flag *without* an id is a fixed point nothing can clear.
Filed as #118; it still drives search scoring, the popularity-floor
bypass and two Explore shelves, so routing the UI around it was not a
fix.
What made the choice concrete rather than theoretical: on
`explore-artist-details` the *same card* used both. The context menu
gated Play on `localId > 0`; the badge used `inLibrary`. An album with
the flag and no local row drew a green tick saying it was in your
library, offered no Play, and — the request item being gated on *not*
owned — offered no way to ask for it either.
The rejected alternative is worth keeping: batching a real file lookup
per screenful, the way `credit-store` coalesces. It would have answered
for **recordings** (`GetFilePathsByRecordingMBIDs`) and most of the
cards on these surfaces are release groups, so it would have made track
rows strong, left album cards exactly where they were, and cost a new
store. The batch that *was* worth adding is a different question —
`GetAlbumsCompleteness`, "how much of this album is here", which no
per-card flag can answer at all.
The general point: **two columns that agree today are not one column.**
Which of them a new surface reads should be decided by which one has
something that can un-set it.
+14 -3
View File
@@ -1,8 +1,19 @@
# semantic-release configuration.
#
# Runs on pushes to main from .gitea/workflows/release.yml: determine the
# version from the Conventional Commits since the last tag, write the
# changelog, commit it, push the tag, and create the Gitea release.
# Run by hand from .gitea/workflows/release.yml, which has no push
# trigger: determine the version from the Conventional Commits since the
# last tag, write the changelog, push the tag, and create the Gitea
# release. A release is a shipment rather than a merge, and the commits
# accumulate until someone says so -- this file needs to know nothing
# about that, because reading everything since the last tag is what it
# already did.
#
# `branches` is main and only main. A `prerelease: true` channel is the
# obvious next edit here and is the one to think twice about: all four
# publishing workflows trigger on `v*`, which matches `v0.4.0-beta.1`.
# They carry a prerelease guard now, so the failure is a clean skip
# rather than a beta in a public tap -- but they are four separate files
# and this is the line that would turn them on.
#
# **There is no `@semantic-release/github` plugin here and there must not
# be.** Gitea's API is `/api/v1` and is not GitHub's surface. The Gitea
+10 -5
View File
@@ -5,15 +5,20 @@ The changelog is the releases page:
<https://git.ljones.me/yonlu/yellowjacket/releases>
Every release there is generated from the Conventional Commits it
contains, by `.gitea/workflows/release.yml` on merge to `main`. Each one
carries its notes as its body, grouped by change type, with a link to the
commit behind every line.
contains, by `.gitea/workflows/release.yml`. Each one carries its notes
as its body, grouped by change type, with a link to the commit behind
every line.
That workflow is **run by hand**, so a release holds everything merged
since the last one rather than one PR's worth. It used to fire on every
push to `main`, which made a version per merged PR (issue #115).
**This file is not generated and is not a copy of that.** `main` is a
protected branch, so nothing pushes a changelog commit back to it — and a
file that claimed to be a changelog while silently never updating would
be worse than no file at all. `make release-dry` prints what the next
merge would release.
be worse than no file at all. `make release-dry` prints what a release
run would cut right now, and the workflow's own `dry_run` input answers
the same question from CI.
History before `v0.0.1` is in `git log`. The versions before it were cut
by hand and are not on the releases page; the entries this file used to
+233 -13
View File
@@ -85,8 +85,18 @@ release decision. The rule that the issue number stays out of the
still drops the footer. `./scripts/issue.sh list --state open` after a
merge, looking for what you just shipped; `./scripts/issue.sh close
<n>` for whatever did not take, with a comment naming the commit.
`close` also drops `Status/In Progress`, because a claim outlives the
work if nothing takes the label off.
**Unclaiming is automatic, and it is hooked to the close rather than
to the merge.** Gitea's auto-close changes state and nothing else, so a
footer left `Status/In Progress` on a closed issue — #100 was closed
and marked as being actively worked on at the same time.
`.gitea/workflows/unclaim.yml` runs on `issues: [closed]`, which covers
the footer, `issue.sh close` and a click in the web UI alike; stripping
the label in the PR instead would have been a per-PR habit, and habits
are what the footer removed. It is not instant — the runner has
capacity 1 — and reopening deliberately does not restore the label.
`./scripts/issue.sh list --state closed --label "Status/In Progress"`
is how you find out it has stopped firing.
## Planning
@@ -1438,6 +1448,52 @@ is therefore **reported at runtime** to `window.__yjIconMisses` and
drawn as a fallback — an e2e sweep asserts there are none — since a
missing icon used to be impossible, the CDN having had everything.
**What each icon *means* is a second table, and it is
`utils/icon-language.ts`.** Bundling answers "does this name resolve";
nothing answered "does this name mean what the one next to it means",
and a wrong-but-real icon renders perfectly. So `plus` came to mean add
to the queue, add to a playlist, make a new playlist **and** you do not
own this — the first two *adjacent in the same context menu* — while
`list` meant the queue, the Playlists destination and adding to the
queue.
The rule the table is built on: **an icon names the noun it acts on,
not the verb.** "Add to queue" and "add to playlist" are one verb on
two nouns, so the noun is what has to differ — which is why adding to a
playlist wears the Playlists destination's own icon, and why the queue
got `bars-staggered` and stopped wearing Playlists'. `plus` keeps the
one meaning it is unambiguous about, making something that is not there
yet.
Four things about it are load-bearing:
- **The request toggle is one glyph in two weights**
(`regular/bookmark``solid/bookmark`), because two states of a
toggle have to read as each other's opposite and a plus against a
bookmark does not. The pair was *already in the app and already
right* on `explore-album-details`'s "Request this" button while the
badge forty pixels away showed a plus — `utils/library-status.ts`'s
fault one layer down, having made the two agree on what wanting means
and left them disagreeing on what it looks like.
- **Downloads keeps the solid bookmark, deliberately.** That is the
same word twice, not two words: the badge says "this is on your
list" and the nav item is that list.
- **`icon-language.test.ts` sweeps the source**, because the rule is
about every call site and checking one checks nothing — the same
shape as `TestNoDirectRuntimeEmits`. It reads every `src/**/*.ts` as
raw text and fails on a literal `name="plus"` or `icon: 'list'`
outside the table, and its **first assertion is that it read
anything at all**, since a sweep over an empty glob passes.
- **It also asserts every `ICON_*` is bundled**, which closes the loop
the runtime cannot: `bookmark-check` is Font Awesome **Pro** and sat
on `explore-artist-details`'s Follow button, drawn for every followed
artist as a circled question mark. `offline-icons.spec.ts` sweeps
`__yjIconMisses` and could not see it, because no spec had ever
followed an artist — the same fault `requested-badge.spec.ts` was
written for, one component over, still live. A name computed from
state was only checkable from the state; now it is checkable from the
table.
**An album page says how much of the album is yours.**
`explore-album-details` is a *catalog* page and there is no
library-side album detail page at all, so the album on it may be
@@ -1537,11 +1593,55 @@ shape as the encoding probe beside it.
What neither side can give is *which* tracks are missing, only how many
— so an incomplete album still browses, and that is now the exception
rather than every album load. Two smaller consequences: existing databases
read "unknown" until a rescan repopulates the column (which degrades to
exactly the old behaviour, so nothing breaks), and our own `tagwriter`
writes track and disc *numbers* but not totals, so autotagging a folder
currently degrades the field this rests on.
rather than every album load. One smaller consequence: existing databases
read "unknown" until a rescan repopulates the column, which degrades to
exactly the old behaviour, so nothing breaks.
**And our own writers declare the total, because for a long time they
did not.** `tagwriter` wrote track and disc *numbers* and dropped the
totals, so autotagging an album actively **erased** the evidence this
rests on: the release became MBID-matched — a green tick — while the
field `GetAlbumCompleteness` reads stayed absent, which is exactly the
"2 of 10 tracks, reported as in your library" the report described.
`FieldTotalTracks` / `FieldTotalDiscs` are written by the autotag apply
pass and by the download importer, and `dbsync` persists the track
total to the row so the album page agrees with the file without waiting
for a rescan.
Five things about it are load-bearing, and four of them fail silently:
- **The total is per *disc*, not per release**, because that is what
the tag form declares and what `GetAlbumCompleteness` **sums** per
disc — a release total written on every file multiplies a two-disc
album's expectation by two, and no library can then satisfy it.
`backend/tagtotals` is that derivation, once, because the two callers
must not import each other or the writer.
- **The Vorbis names are `TRACKTOTAL` and `DISCTOTAL` and no other
spelling.** `dhowden/tag`'s Vorbis reader looks at exactly those two
keys, so a perfectly reasonable `TOTALTRACKS`, or a `1/12` inside
`TRACKNUMBER`, is written successfully and reads back as no total at
all. The tests assert the round trip through the reader the *scan*
uses rather than through the bytes, for that reason.
- **ID3's number and total share one frame**, so writing either alone
has to read the other off the existing tag or it silently discards
it. A total with no number is not written: `/12` is what a reader
parses as track 0.
- **The totals are written unconditionally, not on a diff.** The case
this exists for is a file that declares *no* total, which compares
equal to nothing and is exactly what a "only if it changed" guard
skips.
- **A single-track download must not be totalled.** A `RecordingMBID`
anchor resolves `Expected` to that one track, so the same code would
tag a track off a twelve-track album "1 of 1" — and a declared total
outranks the catalog total that would otherwise have answered
correctly. Confidently wrong is worse than absent here, which is the
same rule `Known` exists for.
One gap this did not close, and it is older: **`dhowden/tag` has no
RIFF reader**, so nothing the tag writer puts in a WAV's `id3 ` chunk
is visible to `metadata.ExtractTags` — not the totals and not the title
either. `wav_test.go` reads that chunk itself, which is why no test
ever noticed.
**The absence is what gets marked, not the presence.** The tracklist
put a green tick against every owned track and a legend underneath
@@ -1558,6 +1658,59 @@ not about plumbing — it says rows may be missing from the page
altogether, which nothing on screen can show. (`explore-artist-details`
still uses `loading`; it has no equivalent per-row signal.)
**And that treatment is the app's, not the page's.**
`utils/ownership.ts` is the rule written once, because it was written
at eight call sites and so none of them had the whole of it: Explore's
cards, `top-results-row` and the artist page's three card shapes all
mixed owned and unowned with a small badge as the only difference, and
the badge on the *owned* ones was a green tick — the mark on the common
case this tracklist removed. Owned is plain and draws no badge at all;
unowned is dimmed, says so in its accessible name, and keeps its
request affordance; a partly-held album says how partly.
Four things about it are load-bearing.
**Ownership is `localId`, and `inLibrary` is deliberately not
consulted.** The album page answers with `filePaths`, a real file per
displayed track, and a card grid cannot afford that — but it does not
need to, because `explore_index.local_*_id` is built by
`collectLibraryEntities` from queries that every one join `audio_files`
and cleared by `pruneStaleLocalCrossReferences`, whose existence test
is a file test in all three cases. That is the same "ownership is a
file" rule computed once per scan instead of once per screenful.
`in_library` is written by the same pass, so the two agree in a healthy
database, but it is a one-way ratchet
(`MAX(in_library, excluded.in_library)`) whose only clearing pass is
gated on a non-null local id: it cannot be un-set on its own (#118).
One is a fact with an owner; the other is a flag that happens to agree.
Both `explore-view` and `explore-artist-details` additionally kept a
`libraryMBIDs` set that accumulated every MBID ever seen with the flag
and cleared it never, in views that never unmount; both are gone.
**The two answers used to sit on one card.**
`renderReleaseMenuItems` gates Play on `release.localId > 0` while the
badge used `inLibrary`, so an album with the flag and no local row drew
a tick saying it was in your library, offered no Play, and — the
request item being gated on *not* owned — offered no way to ask for it
either. Any new surface that asks the question twice will reproduce it.
**`aria-disabled` goes on rows and not on cards.** An unowned *row*
cannot be activated; an unowned *card* still navigates to the catalog
page for it, which is a perfectly good thing to do with something you
do not own. The accessible name carries the state either way, which is
why it is one helper and not a class.
**The count is batched, not looked up.** `store/completeness-store.ts`
is `credit-store` one question over: `request()` is per-card and
coalesces a screenful into one `GetAlbumsCompleteness`, absence is
cached as an answer (or the albums with no totals re-ask forever), and
the whole cache is dropped on a scan, a retag or a removal rather than
aged. `library-status.ts`'s `albumBadgeFor` is where that meets
`Known`: a total that was never declared is a plain `in-library`, never
a ring at 0%. One consequence in the badge itself — a `partial` badge
is *actionable*, and a control named after its action alone dropped the
count from the one state the ring exists for, so its name is both.
**A partly-owned album draws the release, not the part.** Once the tags
say nine of twelve, `buildLibraryEntry` shows the *catalog's* twelve
with three dimmed, rather than the nine on disk — the missing tracks
@@ -1569,6 +1722,32 @@ side-effect worth knowing: this is what finally makes `ownership()`
say something true here, since counting the displayed tracklist of a
library-only entry could only ever produce "9 of 9".
**And it can be asked, because the rule alone reaches too few albums.**
That guard depends on two inputs the user does not control: the files
declaring a per-disc total, and the catalog's own `total_tracks`. Where
neither says — which is a great deal of any library, and *every* library
until an artifact carrying the column is published — a partly-owned
album showed only the tracks on disk with nothing to say the rest
existed. `renderTracklistScope()` is the explicit route: a
"Show the whole album" switch that flips the synthetic "Your Library"
entry between the local files and the release, which is the rendering
the page could already do and could only be *triggered* automatically.
Three things about it are load-bearing. **`showFullTracklist` is a
tri-state**, `null` meaning "follow the automatic rule": the rule is
right when it fires and the switch has to be able to agree with the page
it sits on rather than starting out contradicting it, which a plain
boolean would need recomputed every time the completeness answer moved
underneath it. **`fullReleaseCluster()` falls back to the
highest-scoring cluster**, because `findLibraryCluster` is a guess over
the `inLibrary` flags and returns *nothing* when none are set — which is
exactly the untagged library the switch exists for, so without the
fallback the control would be absent precisely where it is needed. And
**it is shown only where it can change what is on screen**: against the
library entry, with a release to switch to, and only when the two
tracklists differ — the same test the version dropdown answers, one
control over.
**A dropdown is only a choice if the choices differ.** The version
selector tested `versionEntries.length`, but a release group routinely
has several releases — reissues, regional pressings, a remaster — whose
@@ -2176,19 +2355,60 @@ Pre-commit runs vet, lint, codegen check, and frontend typecheck in parallel. Pr
## CI
Seven workflows in `.gitea/workflows/`. Five of them package and
Eight workflows in `.gitea/workflows/`. Five of them package and
publish (`arch-package`, `homebrew-formula`, `index-artifact`,
`android-apk`, `desktop-assets`); `release.yml` decides *whether* four of
those run at all; only `ci.yml` gates, and it is the one to look at when
those run at all; `unclaim.yml` is housekeeping on the tracker and
touches no code; only `ci.yml` gates, and it is the one to look at when
deciding whether a push was healthy.
**`release.yml` is the entry point for all of it.** On every push to
`main` it reads the Conventional Commits since the last tag and, if any
**`release.yml` is the entry point for all of it, and it is triggered by
hand.** It reads the Conventional Commits since the last tag and, if any
is releasable, writes the changelog, pushes the tag and creates the Gitea
release whose body is that changelog section. `arch-package`,
`homebrew-formula`, `android-apk` and `desktop-assets` are all keyed on
`v*`, so **the tag push is what starts them**nothing is released by
hand any more.
`v*`, so **the tag push is what starts them**the version, the notes
and the packaging are still nobody's manual work; *when* is the only
decision left to a person.
**It used to fire on every push to `main`, which made the trigger "a PR
was merged".** That is a version per unit of *work* rather than per
*shipment*: eight releases in twenty-two hours (`v0.0.1``v0.3.1`) for
one session, each fanning out to four publishers on a runner with
capacity 1 — ~40 packaging jobs to ship three issues, with ordinary PR CI
queued behind them. Nothing else had to change to batch them, because
**semantic-release already reads every commit since the last tag**: five
`fix`es and two `feat`s become one minor release with all seven in the
notes. Release frequency was only ever how often the workflow fired.
This is the same rule `index-artifact.yml` states — *a job that mutates
state which cannot be rebuilt in ten minutes is triggered deliberately,
not by a push* — and the two are now the only workflows with no push
trigger. A schedule was considered and rejected: a cron batches without
anyone having to remember, but it puts the decision back on a timer,
which is the thing being removed. A `beta` integration branch was
considered and rejected too (#115): it relocates the trigger rather than
removing one, needs a second protected branch carrying the same required
checks, and *adds* a full `check` + `e2e` run per batch on the very
runner whose queue is the complaint.
**`dry_run` is why the manual trigger is usable.** The point of pulling
a lever by hand is being able to look first, so the dispatch takes a
flag that runs `semantic-release --dry-run`: the version and the notes,
no tag, no release, no publishers. Anything but the literal string
`true` releases for real — a typo in a dispatch box must not silently
turn a shipment into a green no-op.
**A prerelease tag is not a shipment, and all four publishers now say
so.** Their trigger is `v*`, which matches `v0.4.0-beta.1`; they guarded
`v0.0.0` and nothing else. Nothing produces a prerelease today — the
guard is there because the thing that would is `prerelease: true` in
`.releaserc.yml`, one line whose blast radius is a public Homebrew tap
and a credential-free APK registry that Obtainium polls. `android-apk`
is the worst of the four twice over, since its `versionCode` maths
splits on dots and would read `1` out of `0-beta` — a wrong number
rather than a failed build, and Android refuses anything not greater
than what is installed.
Four things about it are load-bearing:
+11 -5
View File
@@ -192,15 +192,21 @@ skill-check: ## Fail if the agent docs name a missing make target, or AGENTS.md
commit-check: ## Fail if a commit subject is not a Conventional Commit
@./scripts/commit-check.sh $(if $(RANGE),--range $(RANGE))
# What a merge to main would release, without releasing it. Reads the
# same .releaserc.yml CI does, so "why did that not cut a version" is
# answerable locally instead of by pushing and watching. Needs no
# credentials: --dry-run neither tags nor publishes.
# What running the release workflow now would ship, without shipping it.
# Reads the same .releaserc.yml CI does, so "why did that not cut a
# version" is answerable locally instead of by pushing and watching.
# Needs no credentials: --dry-run neither tags nor publishes.
#
# release.yml is dispatch-only, so this answers the question that
# actually gets asked now -- what has accumulated since the last tag --
# rather than what one merge would have done. The workflow's own
# `dry_run` input is the same answer from the runner, against whatever
# main points at rather than the working tree.
#
# The pins must stay identical to release.yml's, which is where the note
# on holding the conventionalcommits preset at 9 lives -- at 10 the
# release notes come out empty with everything green.
release-dry: ## Print the version a merge to main would release
release-dry: ## Print the version a release run would cut right now
@npx --yes \
-p semantic-release@25 \
-p @semantic-release/commit-analyzer@13 \
+30
View File
@@ -8,6 +8,7 @@ import (
"log/slog"
"yellowjacket/backend/database/sql/sqlcgen"
"yellowjacket/backend/tagtotals"
)
// TagChanges mirrors tagwriter.TagChanges — redefined here so the
@@ -28,6 +29,8 @@ const (
FieldYear = "year"
FieldTrackNumber = "track_number"
FieldDiscNumber = "disc_number"
FieldTotalTracks = "total_tracks"
FieldTotalDiscs = "total_discs"
FieldCoverArt = "cover_art"
)
@@ -418,5 +421,32 @@ func buildChanges(
changes[FieldDiscNumber] = track.DiscNumber
}
// The totals are what says "2 of 10" rather than a bare tick, and
// dropping them here is what made autotagging an album *erase* the
// evidence: the release becomes MBID-matched while the field
// GetAlbumCompleteness reads stays absent.
//
// They are written unconditionally where the candidate has a
// tracklist, not only when they differ from the local value, because
// the common case is a file that declares no total at all -- which
// compares equal to nothing and would be skipped by a diff guard.
if tracks, discs := tagtotals.For(
candidatePositions(cand), track.DiscNumber,
); tracks > 0 {
changes[FieldTotalTracks] = tracks
changes[FieldTotalDiscs] = discs
}
return changes
}
// candidatePositions is the candidate's tracklist as bare positions.
func candidatePositions(cand Candidate) []tagtotals.Position {
out := make([]tagtotals.Position, 0, len(cand.Tracks))
for _, t := range cand.Tracks {
out = append(out, tagtotals.Position{Disc: t.DiscNumber, Track: t.Position})
}
return out
}
+90
View File
@@ -0,0 +1,90 @@
package autotag
import "testing"
// Autotagging an album used to *erase* the evidence that says "2 of 10":
// the release became MBID-matched while the totals the files declared
// went unwritten, so the album page showed a plain tick. These pin the
// two halves of the fix that are easy to get wrong silently.
func TestBuildChanges_Totals(t *testing.T) {
t.Parallel()
twoDiscs := Candidate{
Tracks: []CandidateTrack{
{DiscNumber: 1, Position: 1},
{DiscNumber: 1, Position: 2},
{DiscNumber: 2, Position: 1},
{DiscNumber: 2, Position: 2},
{DiscNumber: 2, Position: 3},
},
}
tests := []struct {
name string
cand Candidate
local LocalTrack
track CandidateTrack
wantTracks any
wantDiscs any
}{
{
// The common case, and the one a diff guard would skip: the
// file declares no total at all, so the total "has not
// changed" and would never be written.
name: "a file with no total gets one",
cand: Candidate{Tracks: []CandidateTrack{
{Position: 1}, {Position: 2}, {Position: 3},
}},
local: LocalTrack{TrackNumber: 1},
track: CandidateTrack{Position: 1},
wantTracks: 3,
wantDiscs: 1,
},
{
// 5 here would be the release's track count. Summed once
// per disc by GetAlbumCompleteness that claims a ten-track
// expectation for a five-track album, which no library can
// ever satisfy.
name: "a multi-disc release totals the track's own disc",
cand: twoDiscs,
local: LocalTrack{},
track: CandidateTrack{DiscNumber: 2, Position: 1},
wantTracks: 3,
wantDiscs: 2,
},
{
name: "the other disc gets its own total",
cand: twoDiscs,
local: LocalTrack{},
track: CandidateTrack{DiscNumber: 1, Position: 1},
wantTracks: 2,
wantDiscs: 2,
},
{
// A candidate with no tracklist knows nothing, and writing
// a zero would claim it did.
name: "a candidate with no tracklist writes no total",
cand: Candidate{},
local: LocalTrack{},
track: CandidateTrack{Position: 1},
wantTracks: nil,
wantDiscs: nil,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
changes := buildChanges(tc.local, tc.cand, tc.track)
if got := changes[FieldTotalTracks]; got != tc.wantTracks {
t.Errorf("%s: got %v, want %v", FieldTotalTracks, got, tc.wantTracks)
}
if got := changes[FieldTotalDiscs]; got != tc.wantDiscs {
t.Errorf("%s: got %v, want %v", FieldTotalDiscs, got, tc.wantDiscs)
}
})
}
}
+38
View File
@@ -0,0 +1,38 @@
package autotagservice
import (
"testing"
"yellowjacket/backend/autotag"
"yellowjacket/backend/tagwriter"
)
// twAdapter passes the diff map through unchanged, so autotag's field
// constants and tagwriter's are the same keys written down twice --
// deliberately, to keep autotag out of the write pipeline's import
// graph. A key that drifts does not fail to compile and does not fail
// to write: the writer simply finds no entry under the name it looks
// for, and the field is silently dropped. That is what this pins, and
// this package is the one place that imports both.
func TestAutotagAndTagwriterAgreeOnFieldNames(t *testing.T) {
t.Parallel()
pairs := map[string][2]string{
"title": {autotag.FieldTitle, tagwriter.FieldTitle},
"artist": {autotag.FieldArtist, tagwriter.FieldArtist},
"album": {autotag.FieldAlbum, tagwriter.FieldAlbum},
"album artist": {autotag.FieldAlbumArtist, tagwriter.FieldAlbumArtist},
"year": {autotag.FieldYear, tagwriter.FieldYear},
"track number": {autotag.FieldTrackNumber, tagwriter.FieldTrackNumber},
"disc number": {autotag.FieldDiscNumber, tagwriter.FieldDiscNumber},
"total tracks": {autotag.FieldTotalTracks, tagwriter.FieldTotalTracks},
"total discs": {autotag.FieldTotalDiscs, tagwriter.FieldTotalDiscs},
"cover art": {autotag.FieldCoverArt, tagwriter.FieldCoverArt},
}
for name, pair := range pairs {
if pair[0] != pair[1] {
t.Errorf("%s: autotag says %q, tagwriter says %q", name, pair[0], pair[1])
}
}
}
+46
View File
@@ -135,3 +135,49 @@ SELECT
) AS INTEGER) AS known
FROM audio_files a
WHERE a.album_id = sqlc.arg(album_id);
-- name: GetAlbumsCompleteness :many
-- The same question as GetAlbumCompleteness, asked of a screenful of
-- albums at once.
--
-- A card grid cannot afford one query per card, and the answer it wants
-- is the one thing a badge cannot guess: an album held 9 tracks of 12
-- must show the count, never a bare tick. So this is one query for the
-- whole grid, asked only of the cards that have a local album id.
--
-- It is two grouping levels rather than the single-album form's
-- correlated subqueries, because a correlated subquery in the FROM
-- clause is not something SQLite will reliably do -- and because the
-- slice may only be spelled once, or sqlc expands it twice with
-- independently numbered placeholders.
--
-- The per-disc level is where the meaning is, and it is the same
-- meaning as the single-album query. `owned` counts DISTINCT track
-- numbers within a disc (this app detects duplicates, and counting two
-- files of track 3 twice would report a short album as complete), with
-- a file that declares no track number falling back to its own id
-- because three untagged files are three tracks and not one.
-- `expected` takes each disc's declared total and sums over discs,
-- since a total is declared per disc and a release total written on
-- every file of a two-disc album would double its expectation. A disc
-- whose files declared nothing contributes a NULL that SUM ignores,
-- and `known` is what says the album is therefore unanswerable.
WITH per_disc AS (
SELECT
album_id AS album_id,
COUNT(DISTINCT COALESCE(CAST(track_number AS TEXT), 'f' || id))
AS owned_on_disc,
MAX(total_tracks) AS disc_total,
SUM(CASE WHEN total_tracks IS NULL THEN 1 ELSE 0 END)
AS discs_without_a_total
FROM audio_files
WHERE album_id IN (sqlc.slice('album_ids'))
GROUP BY album_id, COALESCE(disc_number, 1)
)
SELECT
CAST(album_id AS INTEGER) AS album_id,
CAST(SUM(owned_on_disc) AS INTEGER) AS owned,
CAST(COALESCE(SUM(disc_total), 0) AS INTEGER) AS expected,
CAST(SUM(discs_without_a_total) = 0 AS INTEGER) AS known
FROM per_disc
GROUP BY album_id;
@@ -8,6 +8,7 @@ package sqlcgen
import (
"context"
"database/sql"
"strings"
)
const deleteAlbum = `-- name: DeleteAlbum :exec
@@ -234,6 +235,98 @@ func (q *Queries) GetAlbumsByArtistName(ctx context.Context, arg GetAlbumsByArti
return items, nil
}
const getAlbumsCompleteness = `-- name: GetAlbumsCompleteness :many
WITH per_disc AS (
SELECT
album_id AS album_id,
COUNT(DISTINCT COALESCE(CAST(track_number AS TEXT), 'f' || id))
AS owned_on_disc,
MAX(total_tracks) AS disc_total,
SUM(CASE WHEN total_tracks IS NULL THEN 1 ELSE 0 END)
AS discs_without_a_total
FROM audio_files
WHERE album_id IN (/*SLICE:album_ids*/?)
GROUP BY album_id, COALESCE(disc_number, 1)
)
SELECT
CAST(album_id AS INTEGER) AS album_id,
CAST(SUM(owned_on_disc) AS INTEGER) AS owned,
CAST(COALESCE(SUM(disc_total), 0) AS INTEGER) AS expected,
CAST(SUM(discs_without_a_total) = 0 AS INTEGER) AS known
FROM per_disc
GROUP BY album_id
`
type GetAlbumsCompletenessRow struct {
AlbumID int64
Owned int64
Expected int64
Known int64
}
// The same question as GetAlbumCompleteness, asked of a screenful of
// albums at once.
//
// A card grid cannot afford one query per card, and the answer it wants
// is the one thing a badge cannot guess: an album held 9 tracks of 12
// must show the count, never a bare tick. So this is one query for the
// whole grid, asked only of the cards that have a local album id.
//
// It is two grouping levels rather than the single-album form's
// correlated subqueries, because a correlated subquery in the FROM
// clause is not something SQLite will reliably do -- and because the
// slice may only be spelled once, or sqlc expands it twice with
// independently numbered placeholders.
//
// The per-disc level is where the meaning is, and it is the same
// meaning as the single-album query. `owned` counts DISTINCT track
// numbers within a disc (this app detects duplicates, and counting two
// files of track 3 twice would report a short album as complete), with
// a file that declares no track number falling back to its own id
// because three untagged files are three tracks and not one.
// `expected` takes each disc's declared total and sums over discs,
// since a total is declared per disc and a release total written on
// every file of a two-disc album would double its expectation. A disc
// whose files declared nothing contributes a NULL that SUM ignores,
// and `known` is what says the album is therefore unanswerable.
func (q *Queries) GetAlbumsCompleteness(ctx context.Context, albumIds []sql.NullInt64) ([]GetAlbumsCompletenessRow, error) {
query := getAlbumsCompleteness
var queryParams []interface{}
if len(albumIds) > 0 {
for _, v := range albumIds {
queryParams = append(queryParams, v)
}
query = strings.Replace(query, "/*SLICE:album_ids*/?", strings.Repeat(",?", len(albumIds))[1:], 1)
} else {
query = strings.Replace(query, "/*SLICE:album_ids*/?", "NULL", 1)
}
rows, err := q.db.QueryContext(ctx, query, queryParams...)
if err != nil {
return nil, err
}
defer rows.Close()
var items []GetAlbumsCompletenessRow
for rows.Next() {
var i GetAlbumsCompletenessRow
if err := rows.Scan(
&i.AlbumID,
&i.Owned,
&i.Expected,
&i.Known,
); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Close(); err != nil {
return nil, err
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const getAlbumsWithPendingReleaseMBID = `-- name: GetAlbumsWithPendingReleaseMBID :many
SELECT id, pending_release_mbid FROM albums
WHERE pending_release_mbid IS NOT NULL AND pending_release_mbid != ''
+32
View File
@@ -12,6 +12,7 @@ import (
"strconv"
"strings"
"yellowjacket/backend/tagtotals"
"yellowjacket/backend/tagwriter"
)
@@ -275,6 +276,25 @@ func (i *Importer) tagFile(p plannedFile, dl Download) error {
changes[tagwriter.FieldDiscNumber] = p.Track.DiscNumber
}
// An imported file should arrive knowing how much of the album it
// is one of, or the album reads as "in your library" from its first
// imported track onward.
//
// A *track* download is the case this must not touch: a
// RecordingMBID anchor resolves Expected to exactly that one track,
// so totalling it would write "1 of 1" onto a track off a
// twelve-track album -- a confident lie, and one that outranks the
// catalog's own total, which is the fallback that would otherwise
// have answered correctly.
if dl.RecordingMBID == "" {
if tracks, discs := tagtotals.For(
expectedPositions(dl.Expected), p.Track.DiscNumber,
); tracks > 0 {
changes[tagwriter.FieldTotalTracks] = tracks
changes[tagwriter.FieldTotalDiscs] = discs
}
}
if err := i.tags.WriteUntrackedFileTags(p.Source, changes); err != nil {
return fmt.Errorf("write tags: %w", err)
}
@@ -282,6 +302,18 @@ func (i *Importer) tagFile(p plannedFile, dl Download) error {
return nil
}
// expectedPositions is the download's resolved tracklist as bare
// positions.
func expectedPositions(expected []ExpectedTrack) []tagtotals.Position {
out := make([]tagtotals.Position, 0, len(expected))
for _, t := range expected {
out = append(out, tagtotals.Position{Disc: t.DiscNumber, Track: t.Position})
}
return out
}
// destinationFor computes a file's library path from the template.
func (i *Importer) destinationFor(
p plannedFile,
+74
View File
@@ -446,3 +446,77 @@ func keysOf(m map[string]tagwriter.TagChanges) []string {
return out
}
// An imported album should arrive knowing its own size, or the album
// page reads "in your library" from its first imported track onward --
// which is the badge complaint this exists to answer.
func TestImportWritesTheAlbumTotals(t *testing.T) {
t.Parallel()
f := newImportFixture(t,
"01 - Airbag.flac",
"02 - Paranoid Android.flac",
"03 - Subterranean Homesick Alien.flac",
"04 - Exit Music (For a Film).flac",
)
if _, err := f.importer.Import(
context.Background(),
fourTrackDownload(),
Result{Dir: f.dir, Files: f.files},
ImportOptions{LibraryRoot: f.root, WriteTags: true},
); err != nil {
t.Fatalf("Import: %v", err)
}
changes := f.tags.writes["01 - Airbag.flac"]
if changes == nil {
t.Fatal("no tag write recorded for the first track")
}
if got := changes[tagwriter.FieldTotalTracks]; got != 4 {
t.Errorf("%s: got %v, want 4", tagwriter.FieldTotalTracks, got)
}
if got := changes[tagwriter.FieldTotalDiscs]; got != 1 {
t.Errorf("%s: got %v, want 1", tagwriter.FieldTotalDiscs, got)
}
}
// A RecordingMBID anchor resolves Expected to exactly the one track it
// asked for, so totalling it would tag a track off a twelve-track album
// as "1 of 1" -- worse than saying nothing, because a declared total
// outranks the catalog total that would have answered correctly.
func TestImportWritesNoTotalsForATrackDownload(t *testing.T) {
t.Parallel()
f := newImportFixture(t, "01 - Airbag.flac")
dl := Download{
ID: "dl-track",
LibraryID: 1,
RecordingMBID: "mbid-recording",
Artist: "Radiohead",
Album: "OK Computer",
Expected: []ExpectedTrack{{Position: 1, Title: "Airbag"}},
}
if _, err := f.importer.Import(
context.Background(),
dl,
Result{Dir: f.dir, Files: f.files},
ImportOptions{LibraryRoot: f.root, WriteTags: true},
); err != nil {
t.Fatalf("Import: %v", err)
}
changes := f.tags.writes["01 - Airbag.flac"]
if changes == nil {
t.Fatal("no tag write recorded")
}
if _, ok := changes[tagwriter.FieldTotalTracks]; ok {
t.Errorf("%s written for a single-track download: %v",
tagwriter.FieldTotalTracks, changes[tagwriter.FieldTotalTracks])
}
}
+9
View File
@@ -2212,6 +2212,7 @@ func (e *Service) gatherTopCandidates(
ArtistType: a.Type,
Country: a.Country,
InLibrary: a.InLibrary,
LocalID: a.LocalID,
},
category: "artist",
qualityScore: quality,
@@ -2243,6 +2244,7 @@ func (e *Service) gatherTopCandidates(
ArtistType: a.Type,
Country: a.Country,
InLibrary: a.InLibrary,
LocalID: a.LocalID,
},
category: "artist",
qualityScore: quality,
@@ -2275,6 +2277,7 @@ func (e *Service) gatherTopCandidates(
PrimaryType: rg.PrimaryType,
Year: year,
InLibrary: rg.InLibrary,
LocalID: rg.LocalID,
},
category: "release_group",
qualityScore: quality,
@@ -2320,6 +2323,7 @@ func (e *Service) gatherTopCandidates(
PrimaryType: rg.PrimaryType,
Year: year,
InLibrary: rg.InLibrary,
LocalID: rg.LocalID,
},
category: "release_group",
qualityScore: quality,
@@ -2347,6 +2351,7 @@ func (e *Service) gatherTopCandidates(
CAAReleaseMBID: r.CAAReleaseMBID,
ReleaseName: r.ReleaseName,
InLibrary: r.InLibrary,
LocalID: r.LocalID,
},
category: "recording",
qualityScore: quality,
@@ -2403,6 +2408,7 @@ func (e *Service) gatherTopCandidates(
CAAReleaseMBID: r.CAAReleaseMBID,
ReleaseName: r.ReleaseName,
InLibrary: r.InLibrary,
LocalID: r.LocalID,
},
category: "recording",
qualityScore: quality,
@@ -2425,6 +2431,7 @@ func (e *Service) gatherTopCandidates(
ArtistType: m.ArtistType,
Country: m.Country,
InLibrary: m.InLibrary || m.LocalArtistID > 0,
LocalID: m.LocalArtistID,
},
category: "artist",
qualityScore: quality,
@@ -2445,6 +2452,7 @@ func (e *Service) gatherTopCandidates(
PrimaryType: m.PrimaryType,
Year: year,
InLibrary: m.InLibrary || m.LocalReleaseGroupID > 0,
LocalID: m.LocalReleaseGroupID,
},
category: "release_group",
qualityScore: quality,
@@ -2459,6 +2467,7 @@ func (e *Service) gatherTopCandidates(
ArtistMBID: m.ArtistMBID,
Length: m.Duration,
InLibrary: m.InLibrary || m.LocalRecordingID > 0,
LocalID: m.LocalRecordingID,
},
category: "recording",
qualityScore: quality,
+10 -1
View File
@@ -41,7 +41,16 @@ type TopResult struct {
ReleaseGroupMBID string `json:"releaseGroupMbid,omitempty"`
ReleaseName string `json:"releaseName,omitempty"`
// Library status — populated from index cross-reference columns.
InLibrary bool `json:"inLibrary"`
//
// LocalID is the one the cards read. It is the local row behind
// this entity — an album, a file, an artist — and it is set and
// cleared by a test against `audio_files`, so it means "there is
// something of mine here". InLibrary is written by the same pass
// but is a one-way ratchet the prune can only clear alongside a
// local id, so it is the weaker of the two and stays for scoring
// (`fwInLibrary`), which is where an approximate answer is fine.
InLibrary bool `json:"inLibrary"`
LocalID int64 `json:"localId,omitempty"`
}
// MBArtist is a Wails-friendly projection of a MusicBrainz artist.
+109
View File
@@ -232,3 +232,112 @@ func TestGetAlbumCompleteness_EmptyAlbum(t *testing.T) {
t.Errorf("empty album reported %+v, want zero and unknown", got)
}
}
// The batch and the single-album query are two spellings of one
// question, and the thing worth pinning is that they never disagree.
//
// They are genuinely different SQL — the single-album form is
// correlated subqueries over one album, the batch is two grouping
// levels over a slice — so the risk is not a typo but a drift in
// meaning: a disc's total counted once per file, a duplicate counted
// twice, a disc with no total silently covered by one that had one.
// Every shape the table above cares about is staged here at once,
// because a batch that is only ever asked about one album is not being
// asked the question that can go wrong.
func TestGetAlbumsCompletenessAgreesWithTheSingleAlbumQuery(t *testing.T) {
t.Parallel()
lib, _ := setupTestLibrary(t)
shapes := map[int][]track{
1: disc(1, 100, 12, 12),
2: disc(1, 200, 9, 12),
3: disc(1, 300, 13, 12),
4: {{recordingID: 400, disc: 1, number: 1}},
5: append(disc(1, 500, 10, 10), disc(2, 600, 2, 5)...),
6: append(
disc(1, 700, 10, 10),
track{recordingID: 750, disc: 2, number: 1},
),
7: append(
disc(1, 800, 5, 6),
track{recordingID: 899, disc: 1, number: 3, total: 6},
),
}
ids := make([]int64, 0, len(shapes))
for albumID, tracks := range shapes {
stageAlbum(t, lib, albumID, tracks)
ids = append(ids, albumIDFor(t, lib, albumID))
}
batch, err := lib.GetAlbumsCompleteness(ids)
if err != nil {
t.Fatalf("GetAlbumsCompleteness: %v", err)
}
if len(batch) != len(ids) {
t.Fatalf("batch answered for %d albums, want %d", len(batch), len(ids))
}
for _, id := range ids {
one, err := lib.GetAlbumCompleteness(id)
if err != nil {
t.Fatalf("GetAlbumCompleteness(%d): %v", id, err)
}
if got := batch[id]; got != one {
t.Errorf("album %d: batch says %+v, single says %+v", id, got, one)
}
}
}
// An album with no files is absent from the batch, not zeroed.
//
// "I have none of this" and "I have no idea" are the third state Known
// exists to keep apart, and a caller reading a missing key gets nothing
// rather than a confident zero it would have to know to distrust.
func TestGetAlbumsCompletenessOmitsAnAlbumWithNoFiles(t *testing.T) {
t.Parallel()
lib, _ := setupTestLibrary(t)
stageAlbum(t, lib, 1, disc(1, 100, 3, 3))
held := albumIDFor(t, lib, 1)
got, err := lib.GetAlbumsCompleteness([]int64{held, 4242})
if err != nil {
t.Fatalf("GetAlbumsCompleteness: %v", err)
}
if _, ok := got[4242]; ok {
t.Errorf("an album with no files answered %+v, want absent", got[4242])
}
if !got[held].Complete {
t.Errorf("held album reported %+v, want complete", got[held])
}
}
// A caller with nothing to ask about must not issue a query at all —
// sqlc's empty-slice branch rewrites the placeholder to NULL, which is
// a perfectly valid query returning nothing, so this is about the round
// trip rather than the answer.
func TestGetAlbumsCompletenessAsksNothingForAnEmptyList(t *testing.T) {
t.Parallel()
lib, _ := setupTestLibrary(t)
for _, ids := range [][]int64{nil, {}, {0}, {-1, 0}} {
got, err := lib.GetAlbumsCompleteness(ids)
if err != nil {
t.Fatalf("GetAlbumsCompleteness(%v): %v", ids, err)
}
if len(got) != 0 {
t.Errorf("GetAlbumsCompleteness(%v) = %+v, want empty", ids, got)
}
}
}
+59
View File
@@ -244,6 +244,65 @@ func (l *Library) GetAlbumCompleteness(albumID int64) (AlbumCompleteness, error)
}, nil
}
// GetAlbumsCompleteness answers the same question for a screenful of
// albums in one query, keyed by album id.
//
// A card grid asks this about every card that has a local album behind
// it, and one query per card is how a grid of fifty albums becomes
// fifty round trips. The answer matters there for the reason it
// matters on the album page: an album held 9 tracks of 12 has to show
// the count, and a bare tick saying "in your library" is the complaint
// this whole rule came from.
//
// An album with no row in the result is one with no files, and it is
// absent rather than zeroed — "I have none of this" and "I have no
// idea" are the same third state `Known` exists to keep apart, and a
// caller reading a missing key gets nothing rather than a confident 0.
func (l *Library) GetAlbumsCompleteness(
albumIDs []int64,
) (map[int64]AlbumCompleteness, error) {
out := make(map[int64]AlbumCompleteness, len(albumIDs))
if len(albumIDs) == 0 {
return out, nil
}
keys := make([]sql.NullInt64, 0, len(albumIDs))
for _, id := range albumIDs {
if id <= 0 {
continue
}
keys = append(keys, sql.NullInt64{Int64: id, Valid: true})
}
if len(keys) == 0 {
return out, nil
}
rows, err := l.db.ReadQueries.GetAlbumsCompleteness(l.ctx, keys)
if err != nil {
l.logger.Error("could not get album completeness in batch",
"albums", len(keys), "error", err)
return nil, fmt.Errorf("could not get album completeness: %w", err)
}
for _, row := range rows {
known := row.Known != 0 && row.Expected > 0
out[row.AlbumID] = AlbumCompleteness{
Owned: int(row.Owned),
Expected: int(row.Expected),
Known: known,
Complete: known && row.Owned >= row.Expected,
}
}
return out, nil
}
// GetAlbumTracks returns one album's tracks in disc/track order.
func (l *Library) GetAlbumTracks(albumID, libraryID int64) ([]Track, error) {
rows, err := l.db.ReadQueries.GetTracksByAlbum(
+56
View File
@@ -0,0 +1,56 @@
// Package tagtotals derives the totals a tag's "5/12" form declares.
//
// It exists because the two writers that know a release's full
// tracklist -- the autotag apply pass and the download importer --
// must not import each other or the tag writer, and because getting
// the denominator wrong is invisible: a total that is too large marks
// a complete album incomplete forever, and nothing fails.
package tagtotals
// Position is one track's place in a release. A zero Disc means the
// release did not say, which is disc 1.
type Position struct {
Disc int
Track int
}
// For returns the totals to write on a file sitting on disc `disc`:
// how many tracks that disc has, and how many discs the release has.
//
// The track total is **per disc** and not the release's track count,
// because that is what the tag form means and what
// GetAlbumCompleteness sums -- summing a release total once per disc
// would multiply a two-disc album's expectation by two.
//
// Tracks are counted by distinct position rather than by row: a
// tracklist that lists a position twice is a defect in the source, and
// counting it twice would put an album permanently out of reach of its
// own total.
func For(all []Position, disc int) (tracks, discs int) {
disc = normaliseDisc(disc)
seenTracks := make(map[int]struct{}, len(all))
seenDiscs := make(map[int]struct{}, 1)
for _, p := range all {
d := normaliseDisc(p.Disc)
seenDiscs[d] = struct{}{}
if d != disc || p.Track <= 0 {
continue
}
seenTracks[p.Track] = struct{}{}
}
return len(seenTracks), len(seenDiscs)
}
// normaliseDisc treats an undeclared disc as disc 1.
func normaliseDisc(d int) int {
if d <= 0 {
return 1
}
return d
}
+92
View File
@@ -0,0 +1,92 @@
package tagtotals_test
import (
"testing"
"yellowjacket/backend/tagtotals"
)
func TestFor(t *testing.T) {
t.Parallel()
singleDisc := []tagtotals.Position{
{Disc: 0, Track: 1}, {Disc: 0, Track: 2}, {Disc: 0, Track: 3},
}
twoDiscs := []tagtotals.Position{
{Disc: 1, Track: 1},
{Disc: 1, Track: 2},
{Disc: 2, Track: 1},
{Disc: 2, Track: 2},
{Disc: 2, Track: 3},
}
tests := []struct {
name string
all []tagtotals.Position
disc int
wantTracks int
wantDiscs int
}{
{
name: "a single-disc release totals its own tracks",
all: singleDisc, disc: 0, wantTracks: 3, wantDiscs: 1,
},
{
// An undeclared disc is disc 1, on both sides of the
// question -- a file tagged "disc 1" and a tracklist that
// declares no disc describe the same disc.
name: "an undeclared disc is disc 1",
all: singleDisc, disc: 1, wantTracks: 3, wantDiscs: 1,
},
{
// The whole point: 5 here would be the release's track
// count, which summed once per disc claims a ten-track
// expectation for a five-track album.
name: "a multi-disc release totals the file's own disc",
all: twoDiscs, disc: 2, wantTracks: 3, wantDiscs: 2,
},
{
name: "the other disc gets its own total",
all: twoDiscs, disc: 1, wantTracks: 2, wantDiscs: 2,
},
{
// A disc the tracklist does not mention cannot be totalled,
// and 0 is how the caller is told to write nothing.
name: "a disc with no tracks totals nothing",
all: twoDiscs, disc: 3, wantTracks: 0, wantDiscs: 2,
},
{
name: "an empty tracklist totals nothing",
all: nil, disc: 1, wantTracks: 0, wantDiscs: 0,
},
{
// A source that lists a position twice would otherwise put
// the album permanently one track short of its own total.
name: "a repeated position counts once",
all: []tagtotals.Position{
{Disc: 1, Track: 1}, {Disc: 1, Track: 1}, {Disc: 1, Track: 2},
},
disc: 1, wantTracks: 2, wantDiscs: 1,
},
{
name: "a track with no position is not counted",
all: []tagtotals.Position{
{Disc: 1, Track: 0}, {Disc: 1, Track: 1},
},
disc: 1, wantTracks: 1, wantDiscs: 1,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
tracks, discs := tagtotals.For(tc.all, tc.disc)
if tracks != tc.wantTracks || discs != tc.wantDiscs {
t.Errorf("For(%v, %d) = (%d, %d), want (%d, %d)",
tc.all, tc.disc, tracks, discs, tc.wantTracks, tc.wantDiscs)
}
})
}
}
+10 -1
View File
@@ -183,6 +183,15 @@ func syncDatabase(
discNum = toNullInt64(v)
}
// The completeness evidence. Without this the row keeps whatever
// the last scan read while the file on disk now declares a total,
// so the album stays "unknown" until a full rescan -- which is the
// state the report describes.
totalTracks := old.TotalTracks
if v, ok := asInt(params.changes[FieldTotalTracks]); ok {
totalTracks = toNullInt64(v)
}
composer := old.Composer
if v, ok := params.changes[FieldComposer].(string); ok {
composer = v
@@ -207,7 +216,7 @@ func syncDatabase(
AlbumID: albumID,
TrackNumber: trackNum,
DiscNumber: discNum,
TotalTracks: old.TotalTracks,
TotalTracks: totalTracks,
Year: year,
Composer: composer,
Comment: old.Comment,
+5
View File
@@ -101,6 +101,11 @@ func applyFlacTextChanges(cmt *flacvorbis.MetaDataBlockVorbisComment, changes Ta
{FieldYear, flacvorbis.FIELD_DATE, true},
{FieldTrackNumber, flacvorbis.FIELD_TRACKNUMBER, true},
{FieldDiscNumber, "DISCNUMBER", true},
// TRACKTOTAL/DISCTOTAL and no other spelling: dhowden/tag's
// Vorbis reader looks at exactly these two keys, so TOTALTRACKS
// or a "1/12" inside TRACKNUMBER reads back as no total at all.
{FieldTotalTracks, "TRACKTOTAL", true},
{FieldTotalDiscs, "DISCTOTAL", true},
{FieldComposer, "COMPOSER", false},
}
+63 -11
View File
@@ -6,6 +6,7 @@ import (
"log/slog"
"os"
"strconv"
"strings"
id3v2 "github.com/bogem/id3v2/v2"
@@ -66,17 +67,10 @@ func applyTextChanges(tag *id3v2.Tag, changes TagChanges) {
tag.SetYear(strconv.Itoa(v))
}
if v, ok := asInt(changes[FieldTrackNumber]); ok {
trckID := tag.CommonID("Track number/Position in set")
tag.DeleteFrames(trckID)
tag.AddTextFrame(trckID, id3v2.EncodingUTF8, strconv.Itoa(v))
}
if v, ok := asInt(changes[FieldDiscNumber]); ok {
tposID := tag.CommonID("Part of a set")
tag.DeleteFrames(tposID)
tag.AddTextFrame(tposID, id3v2.EncodingUTF8, strconv.Itoa(v))
}
applyPositionFrame(tag, "Track number/Position in set", changes,
FieldTrackNumber, FieldTotalTracks)
applyPositionFrame(tag, "Part of a set", changes,
FieldDiscNumber, FieldTotalDiscs)
if v, ok := changes[FieldComposer].(string); ok {
tag.DeleteFrames("TCOM")
@@ -90,6 +84,64 @@ func applyTextChanges(tag *id3v2.Tag, changes TagChanges) {
}
}
// applyPositionFrame writes an ID3v2 position frame (TRCK or TPOS) in
// the "n/N" form the readers parse.
//
// The number and the total are separate diff entries and either may be
// absent, so the frame's *existing* value is the base: writing a total
// alone must not discard the number that is already there, and writing
// a number alone must not discard a total the file already declared.
// A total with no number at all is not written, since "/12" says
// nothing a reader can use.
func applyPositionFrame(
tag *id3v2.Tag, description string, changes TagChanges, numKey, totalKey string,
) {
_, hasNum := changes[numKey]
_, hasTotal := changes[totalKey]
if !hasNum && !hasTotal {
return
}
frameID := tag.CommonID(description)
num, total := parseXofN(
strings.TrimRight(tag.GetTextFrame(frameID).Text, "\x00 \t\n\r"),
)
if v, ok := asInt(changes[numKey]); ok {
num = v
}
if v, ok := asInt(changes[totalKey]); ok {
total = v
}
if num <= 0 {
return
}
value := strconv.Itoa(num)
if total > 0 {
value += "/" + strconv.Itoa(total)
}
tag.DeleteFrames(frameID)
tag.AddTextFrame(frameID, id3v2.EncodingUTF8, value)
}
// parseXofN splits an ID3v2 "n/N" position value. A bare "n" yields a
// zero total, and anything unparseable yields zeros — the same reading
// dhowden/tag gives the frame.
func parseXofN(s string) (int, int) {
numText, totalText, _ := strings.Cut(s, "/")
num, _ := strconv.Atoi(strings.TrimSpace(numText))
total, _ := strconv.Atoi(strings.TrimSpace(totalText))
return num, total
}
// applyCoverArtChanges handles the FieldCoverArt entry in the diff map.
//
// - []byte with len > 0: embed the given image as front cover.
+2
View File
@@ -166,6 +166,8 @@ var oggFieldMappings = []struct { //nolint:gochecknoglobals // field mapping tab
{FieldYear, "DATE", true},
{FieldTrackNumber, "TRACKNUMBER", true},
{FieldDiscNumber, "DISCNUMBER", true},
{FieldTotalTracks, "TRACKTOTAL", true},
{FieldTotalDiscs, "DISCTOTAL", true},
{FieldComposer, "COMPOSER", false},
}
+28
View File
@@ -333,3 +333,31 @@ func TestWriteTrackTags_DBSync(t *testing.T) {
t.Error("expected FTS5 result for 'New Title'")
}
}
// The row is what the album page reads, and it is only refreshed by a
// scan. Leaving total_tracks at whatever the last scan saw means an
// album autotagged just now stays "unknown" -- a plain tick on an album
// the user holds two tracks of -- until a full rescan happens to run.
func TestWriteTrackTags_PersistsTheTotal(t *testing.T) {
db := database.NewTestDB(t)
dir := t.TempDir()
trackID := seedTestTrack(t, db, createPipelineTestMP3(t, dir))
tw := NewTagWriter(testLogger(), db, &mockPlayer{}, &mockPipelineLocker{})
if err := tw.WriteTrackTags(trackID, TagChanges{
FieldTrackNumber: 2,
FieldTotalTracks: 10,
}); err != nil {
t.Fatalf("WriteTrackTags: %v", err)
}
af, err := db.Queries.GetAudioFile(context.Background(), trackID)
if err != nil {
t.Fatalf("get audio file: %v", err)
}
if !af.TotalTracks.Valid || af.TotalTracks.Int64 != 10 {
t.Errorf("total_tracks: got %v, want 10", af.TotalTracks)
}
}
+9
View File
@@ -26,6 +26,15 @@ const (
FieldDiscNumber = "disc_number"
FieldComposer = "composer"
FieldCoverArt = "cover_art" // []byte for set, nil for clear
// FieldTotalTracks is how many tracks are on *this file's disc*, not
// in the whole release. That is what the "5/12" form declares and
// what GetAlbumCompleteness sums per disc; a release total written
// here would multiply the expectation by the number of discs.
FieldTotalTracks = "total_tracks"
// FieldTotalDiscs is how many discs the release has.
FieldTotalDiscs = "total_discs"
)
// AudioFormat represents a supported audio file format.
+199
View File
@@ -0,0 +1,199 @@
package tagwriter
import (
"path/filepath"
"testing"
"yellowjacket/backend/metadata"
)
// The totals are the evidence GetAlbumCompleteness reads, and every way
// of getting them wrong is silent: a tag written under a name the
// reader does not look at reads back as no total at all, which is
// indistinguishable from never having written one. So these assert the
// round trip through the *reader the scan uses*, not the bytes.
//
// WAV is the exception and it is not this change's: dhowden/tag has no
// RIFF reader at all, so metadata.ExtractTags cannot see a WAV's ID3
// chunk -- which is why every other test here reads that chunk itself.
func TestWriteTotals_RoundTripsInEveryFormat(t *testing.T) {
t.Parallel()
changes := TagChanges{
FieldTitle: "Some Song",
FieldTrackNumber: 2,
FieldTotalTracks: 10,
FieldDiscNumber: 1,
FieldTotalDiscs: 2,
}
viaScanner := func(t *testing.T, path string) *metadata.TrackMetadata {
t.Helper()
meta, err := metadata.ExtractTags(path)
if err != nil {
t.Fatalf("ExtractTags: %v", err)
}
return meta
}
tests := []struct {
name string
write func(t *testing.T, dir string) string
read func(t *testing.T, path string) *metadata.TrackMetadata
}{
{
name: "mp3",
read: viaScanner,
write: func(t *testing.T, dir string) string {
t.Helper()
path := createTestMP3(t, dir, "totals.mp3", nil)
if err := writeMp3Tags(testLogger(), path, changes); err != nil {
t.Fatalf("writeMp3Tags: %v", err)
}
return path
},
},
{
name: "flac",
read: viaScanner,
write: func(t *testing.T, dir string) string {
t.Helper()
path := filepath.Join(dir, "totals.flac")
makeMinimalFLAC(t, path)
if err := writeFlacTags(testLogger(), path, changes); err != nil {
t.Fatalf("writeFlacTags: %v", err)
}
return path
},
},
{
name: "ogg",
read: viaScanner,
write: func(t *testing.T, dir string) string {
t.Helper()
path := filepath.Join(dir, "totals.ogg")
createTestOGG(t, path)
if err := writeOggTags(testLogger(), path, changes); err != nil {
t.Fatalf("writeOggTags: %v", err)
}
return path
},
},
{
name: "wav",
read: readWavID3Tags,
write: func(t *testing.T, dir string) string {
t.Helper()
path := createTestWAV(t, dir, "totals.wav", nil)
if err := writeWavTags(testLogger(), path, changes); err != nil {
t.Fatalf("writeWavTags: %v", err)
}
return path
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
meta := tc.read(t, tc.write(t, t.TempDir()))
assertIntField(t, "TrackNumber", meta.TrackNumber, 2)
assertIntField(t, "TotalTracks", meta.TotalTracks, 10)
assertIntField(t, "DiscNumber", meta.DiscNumber, 1)
assertIntField(t, "TotalDiscs", meta.TotalDiscs, 2)
})
}
}
// A number and a total are separate diff entries, so writing one must
// not discard the other. For ID3v2 they share a single "n/N" frame,
// which is the only place this can go wrong -- and it goes wrong by
// silently zeroing a total the file already declared.
func TestWriteMp3Totals_PartialUpdateKeepsTheOtherHalf(t *testing.T) {
t.Parallel()
t.Run("writing the number keeps the total", func(t *testing.T) {
t.Parallel()
dir := t.TempDir()
path := createTestMP3(t, dir, "seeded.mp3", TagChanges{
FieldTrackNumber: 2,
FieldTotalTracks: 10,
})
if err := writeMp3Tags(testLogger(), path, TagChanges{
FieldTrackNumber: 4,
}); err != nil {
t.Fatalf("writeMp3Tags: %v", err)
}
meta, err := metadata.ExtractTags(path)
if err != nil {
t.Fatalf("ExtractTags: %v", err)
}
assertIntField(t, "TrackNumber", meta.TrackNumber, 4)
assertIntField(t, "TotalTracks", meta.TotalTracks, 10)
})
t.Run("writing the total keeps the number", func(t *testing.T) {
t.Parallel()
dir := t.TempDir()
path := createTestMP3(t, dir, "seeded.mp3", TagChanges{
FieldTrackNumber: 7,
})
if err := writeMp3Tags(testLogger(), path, TagChanges{
FieldTotalTracks: 12,
}); err != nil {
t.Fatalf("writeMp3Tags: %v", err)
}
meta, err := metadata.ExtractTags(path)
if err != nil {
t.Fatalf("ExtractTags: %v", err)
}
assertIntField(t, "TrackNumber", meta.TrackNumber, 7)
assertIntField(t, "TotalTracks", meta.TotalTracks, 12)
})
// "/12" says nothing a reader can use, and dhowden/tag reads it as
// track 0 -- which the scan would store as a real track number.
t.Run("a total with no number writes nothing", func(t *testing.T) {
t.Parallel()
dir := t.TempDir()
path := createTestMP3(t, dir, "bare.mp3", nil)
if err := writeMp3Tags(testLogger(), path, TagChanges{
FieldTotalTracks: 12,
}); err != nil {
t.Fatalf("writeMp3Tags: %v", err)
}
meta, err := metadata.ExtractTags(path)
if err != nil {
t.Fatalf("ExtractTags: %v", err)
}
assertIntField(t, "TrackNumber", meta.TrackNumber, 0)
assertIntField(t, "TotalTracks", meta.TotalTracks, 0)
})
}
+5 -4
View File
@@ -522,19 +522,20 @@ func readWavID3Tags(
}
}
// Track number (TRCK).
// Track number and total (TRCK), disc number and total (TPOS).
// Both carry the "n/N" form, so they are read the way a reader
// reads them rather than with Atoi -- which sees "2/10" as 0.
trckID := parsed.CommonID("Track number/Position in set")
if frames := parsed.GetFrames(trckID); len(frames) > 0 {
if tf, ok := frames[0].(id3v2.TextFrame); ok {
meta.TrackNumber = atoiSafe(tf.Text)
meta.TrackNumber, meta.TotalTracks = parseXofN(tf.Text)
}
}
// Disc number (TPOS).
tposID := parsed.CommonID("Part of a set")
if frames := parsed.GetFrames(tposID); len(frames) > 0 {
if tf, ok := frames[0].(id3v2.TextFrame); ok {
meta.DiscNumber = atoiSafe(tf.Text)
meta.DiscNumber, meta.TotalDiscs = parseXofN(tf.Text)
}
}
+16 -6
View File
@@ -7,12 +7,22 @@ which is what lets Obtainium poll a plain URL with no token. It also
attaches the same file to the Gitea release, which is what a person
looking at the release page downloads.
**Tags are not pushed by hand any more.** `.gitea/workflows/release.yml`
reads the Conventional Commits on every merge to `main`, decides the
version, and pushes the tag this workflow is keyed on — so releasing the
APK means merging a `fix:` or `feat:` commit, not running `git tag`. The
`workflow_dispatch` path below remains, for rebuilding a tag that already
exists.
**Tags are not pushed by hand any more, but releasing is a decision.**
`.gitea/workflows/release.yml` reads the Conventional Commits since the
last tag, decides the version, and pushes the tag this workflow is keyed
on — so releasing the APK means **running that workflow**, not running
`git tag`. It has no push trigger: merging a `fix:` or `feat:` used to
be enough and produced a version per merged PR (issue #115). Run it with
`dry_run` first to see what the accumulated commits would ship. The
`workflow_dispatch` path below is a different thing and remains, for
rebuilding a tag that already exists.
**A prerelease tag is skipped here**, cleanly. This workflow triggers on
`v*`, which matches `v0.4.0-beta.1`, and it is the one where that would
hurt most: the APK goes to the credential-free generic registry that
Obtainium polls, and the `versionCode` maths below splits on dots — it
would read `1` out of `0-beta` and produce a wrong number rather than a
failed build.
## The 1.x installs cannot be upgraded to 0.0.x
@@ -431,8 +431,17 @@ export interface TopResult {
/**
* Library status populated from index cross-reference columns.
*
* LocalID is the one the cards read. It is the local row behind
* this entity an album, a file, an artist and it is set and
* cleared by a test against `audio_files`, so it means "there is
* something of mine here". InLibrary is written by the same pass
* but is a one-way ratchet the prune can only clear alongside a
* local id, so it is the weaker of the two and stays for scoring
* (`fwInLibrary`), which is where an approximate answer is fine.
*/
"inLibrary": boolean;
"localId"?: number;
}
/**
@@ -98,6 +98,26 @@ export function GetAlbumsByArtist(artist: string, libraryID: number): $Cancellab
return $Call.ByID(1456840721, artist, libraryID);
}
/**
* GetAlbumsCompleteness answers the same question for a screenful of
* albums in one query, keyed by album id.
*
* A card grid asks this about every card that has a local album behind
* it, and one query per card is how a grid of fifty albums becomes
* fifty round trips. The answer matters there for the reason it
* matters on the album page: an album held 9 tracks of 12 has to show
* the count, and a bare tick saying "in your library" is the complaint
* this whole rule came from.
*
* An album with no row in the result is one with no files, and it is
* absent rather than zeroed "I have none of this" and "I have no
* idea" are the same third state `Known` exists to keep apart, and a
* caller reading a missing key gets nothing rather than a confident 0.
*/
export function GetAlbumsCompleteness(albumIDs: number[] | null): $CancellablePromise<{ [_ in `${number}`]?: $models.AlbumCompleteness } | null> {
return $Call.ByID(531636827, albumIDs);
}
/**
* GetAllLibrariesWithTrackCounts lists the libraries and their sizes.
*/
+4 -1
View File
@@ -39,7 +39,10 @@
<audio-player></audio-player>
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
id="queue-button">
<wa-icon name="list"></wa-icon>
<!-- ICON_QUEUE in src/utils/icon-language.ts, written out
because this file has no module scope. It was `list`,
which is the Playlists destination's icon. -->
<wa-icon name="bars-staggered"></wa-icon>
</button>
</footer>
<!-- The phone's primary navigation, hidden above 600px by
@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><!--! Font Awesome Free 7.3.1 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License) Copyright 2026 Fonticons, Inc. --><path fill="currentColor" d="M0 96C0 78.3 14.3 64 32 64l384 0c17.7 0 32 14.3 32 32s-14.3 32-32 32L32 128C14.3 128 0 113.7 0 96zM64 256c0-17.7 14.3-32 32-32l384 0c17.7 0 32 14.3 32 32s-14.3 32-32 32L96 288c-17.7 0-32-14.3-32-32zM448 416c0 17.7-14.3 32-32 32L32 448c-17.7 0-32-14.3-32-32s14.3-32 32-32l384 0c17.7 0 32 14.3 32 32z"/></svg>

After

Width:  |  Height:  |  Size: 609 B

@@ -37,6 +37,10 @@ import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js'
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import '@components/playlist-picker/playlist-picker.js';
import { dict, list } from '@utils/binding';
import {
ICON_PLAYLIST,
ICON_QUEUE,
} from '@utils/icon-language';
/** Pixels to change card width per scroll tick. */
const ZOOM_STEP = 16;
@@ -1371,7 +1375,7 @@ export class ArtistsView
>
<wa-icon
slot="icon"
name="plus"
name=${ICON_QUEUE}
></wa-icon>
Add to Queue
</wa-dropdown-item>
@@ -1407,7 +1411,7 @@ export class ArtistsView
>
<wa-icon
slot="icon"
name="plus"
name=${ICON_PLAYLIST}
></wa-icon>
Add to Playlist
<span
@@ -6,6 +6,7 @@ import type WaDrawer from '@awesome.me/webawesome/dist/components/drawer/drawer.
import { designTokens } from '../../styles/tokens.css';
import '../sidebar/app-sidebar.js';
import { nameDialog } from '@utils/name-dialog';
import { ICON_PLAYLIST } from '@utils/icon-language';
type View = 'home' | 'albums' | 'tracks' | 'playlists';
@@ -138,7 +139,7 @@ export class BottomNav extends LitElement {
{ id: 'home', label: 'Home', icon: 'house' },
{ id: 'albums', label: 'Albums', icon: 'compact-disc' },
{ id: 'tracks', label: 'Tracks', icon: 'music' },
{ id: 'playlists', label: 'Playlists', icon: 'list' },
{ id: 'playlists', label: 'Playlists', icon: ICON_PLAYLIST },
];
override connectedCallback() {
@@ -76,6 +76,10 @@ import type {
SortDirection,
} from './cover-grid-types.js';
import { list } from '@utils/binding';
import {
ICON_PLAYLIST,
ICON_QUEUE,
} from '@utils/icon-language';
@customElement('cover-grid')
export class CoverGrid
@@ -2123,7 +2127,7 @@ export class CoverGrid
>
<wa-icon
slot="icon"
name="plus"
name=${ICON_QUEUE}
></wa-icon>
Add to Queue
</wa-dropdown-item>
@@ -2156,7 +2160,7 @@ export class CoverGrid
>
<wa-icon
slot="icon"
name="plus"
name=${ICON_PLAYLIST}
></wa-icon>
Add to Playlist
<span
@@ -3,6 +3,7 @@ import { customElement, property, state, query } from 'lit/decorators.js';
import { classMap } from 'lit/directives/class-map.js';
import { designTokens } from '../../styles/tokens.css';
import { srOnly } from '../../styles/sr-only.css';
import { unownedLabel, unownedStyles } from '@utils/ownership';
import {
LookupReleaseGroup,
BrowseReleases,
@@ -52,6 +53,12 @@ import { dictByName } from '@utils/binding';
import type { TrackDetails } from '@components/track-details/track-details.js';
import { showTrackDetailsForPath } from '@utils/track-details-opener.js';
import '@components/playlist-picker/playlist-picker.js';
import {
ICON_CAN_REQUEST,
ICON_PLAYLIST,
ICON_QUEUE,
ICON_REQUESTED,
} from '@utils/icon-language';
/**
* The region the album header's own failures are rendered in.
@@ -191,8 +198,49 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
@state() private versionEntries: VersionEntry[] = [];
/** Currently-selected dropdown entry (by VersionEntry.key). */
@state() private selectedVersionKey: string = '';
/**
* The key `buildClusters` defaulted to, kept so the page can tell
* "this is what we picked for you" from "you went and chose this".
*
* Only the second needs saying out loud. With the selector demoted
* to a disclosure below the tracklist, a chosen version is the one
* case where the list on screen is not the one the header
* describes, and nothing else on the page would say so.
*/
@state() private defaultVersionKey: string = '';
/**
* Whether the "Other versions" disclosure is open.
*
* Collapsed by default choosing which pressing you are looking at
* is a metadata-repair task and does not belong above the
* tracklist. It is deliberately *not* closed when the selection
* changes: the user opened it to change something, and a panel that
* shuts on use cannot be used twice.
*/
@state() private versionsOpen = false;
@state() private coverArtURL = '';
/**
* Whether to draw the whole release rather than only the files on
* disk `null` while nobody has said, which is the automatic rule
* (`buildLibraryEntry`: show the release once the tags say the album
* is incomplete).
*
* It is a *tri-state* on purpose. The automatic rule is right when
* it fires and the switch has to be able to agree with it, or the
* control would start out contradicting the page it is sitting on;
* a plain boolean would need its default recomputed every time the
* completeness answer changed underneath it.
*
* The rule alone was not enough, which is the report: it depends on
* the files declaring a per-disc total, so a library whose tags
* never said sat permanently on "only my tracks" with no way to ask
* for the rest and no way to tell that there was a rest.
*/
@state() private showFullTracklist: boolean | null = null;
/**
* The local album's own tracks the authoritative answer to "what
* is actually on disk," independent of `this.releases`, which
@@ -291,6 +339,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
exploreLinkStyles,
contextMenuStyles,
srOnly,
unownedStyles,
css`
:host {
display: flex;
@@ -496,7 +545,93 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
flex-shrink: 0;
}
/* ── Version selector ── */
/* ── Other versions (a disclosure, below the tracklist) ── */
.versions {
margin-top: 24px;
border-top: 1px solid
var(--yj-border-subtle, rgba(255, 255, 255, 0.08));
padding-top: 8px;
}
/* The heading exists so the section is reachable by heading
* navigation; the button inside it is the control. Its own
* type scale is the section header's, reduced this is a
* footnote to the page, not a peer of the tracklist. */
.versions-heading {
margin: 0;
font-size: var(--yj-text-sm);
font-weight: 500;
}
.versions-toggle {
display: flex;
align-items: center;
gap: 8px;
width: 100%;
padding: 8px 2px;
background: none;
border: none;
color: var(--yj-text-secondary, #b3b3b3);
font: inherit;
text-align: left;
cursor: pointer;
}
.versions-toggle:hover {
color: var(--yj-text-primary, #fff);
}
.versions-toggle:focus-visible {
outline: 2px solid var(--yj-accent-text, #ffd43b);
outline-offset: 2px;
border-radius: 4px;
}
.versions-toggle wa-icon {
font-size: var(--yj-icon-xs, 11px);
transition: transform 0.2s ease;
}
.versions-toggle[aria-expanded='false'] wa-icon {
transform: rotate(-90deg);
}
.versions-intro {
margin: 0 0 10px;
font-size: var(--yj-text-xs);
color: var(--yj-text-tertiary, #888);
line-height: 1.4;
}
/* A line above the tracklist, and only after a deliberate
* choice see renderChosenVersion. */
.chosen-version {
margin: 0 0 10px;
font-size: var(--yj-text-sm);
color: var(--yj-text-secondary, #b3b3b3);
}
.chosen-version strong {
color: var(--yj-text-primary, #fff);
font-weight: 600;
}
.chosen-version-reset {
background: none;
border: none;
padding: 0;
font: inherit;
color: var(--yj-accent-text, #ffd43b);
text-decoration: underline;
cursor: pointer;
}
.chosen-version-reset:focus-visible {
outline: 2px solid var(--yj-accent-text, #ffd43b);
outline-offset: 2px;
border-radius: 2px;
}
.version-selector {
display: flex;
flex-direction: column;
@@ -559,6 +694,19 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
}
/* ── Tracklist ── */
.tracklist-scope {
display: flex;
align-items: center;
flex-wrap: wrap;
gap: 6px 12px;
margin-bottom: 8px;
}
.tracklist-scope-hint {
font-size: var(--yj-text-xs);
color: var(--yj-text-tertiary, #888);
}
.tracklist {
display: flex;
flex-direction: column;
@@ -649,20 +797,14 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
white-space: nowrap;
}
/* A track the library does not have, on the pattern a
* streaming service uses for something it cannot play: the
* row stays, dimmed, so the album reads as the album rather
* than as the subset that happens to be here.
*
* The dimming is a colour, so it cannot be the only signal
* the row also carries aria-disabled, which is what
* reaches anyone not seeing it. Secondary rather than
* tertiary because the row's hover background is
* bgOverlay, which tertiary does not clear. */
.track-row.unowned .track-title {
color: var(--yj-text-secondary, #b3b3b3);
font-weight: 400;
}
/* The dimming itself is unownedStyles, from
* utils/ownership.ts, imported above. It was written here
* first this tracklist is where the treatment came from
* and moved out when seven other surfaces had to draw the
* same thing, because two of them would otherwise have
* ended up drawing it slightly differently. (No backticks or
* apostrophes-as-quotes here: this is inside a tagged
* template literal.) */
/* The request control is offered on every row that has
* something to request, and is not revealed on hover.
@@ -914,6 +1056,9 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
this.releases = [];
this.versionEntries = [];
this.selectedVersionKey = '';
this.defaultVersionKey = '';
this.versionsOpen = false;
this.showFullTracklist = null;
this.localTracks = [];
this.filePaths = new Map();
this.askedFor = new Set();
@@ -1604,6 +1749,14 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|| standardEntry?.key
|| this.versionEntries[0]?.key
|| '';
// Recorded here rather than derived later: this is the one
// place that knows what "the version we picked" means, and
// recomputing the preference order at the render site would be
// a second copy of it. `handleTracklistScopeChange` rebuilds
// through here too, so the switch does not read as a choice of
// version.
this.defaultVersionKey = this.selectedVersionKey;
}
/**
@@ -1817,17 +1970,23 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
// Guarded on `known` rather than on "fewer tracks than the
// cluster", which would swap in a catalog tracklist for
// every album whose tags simply never declared a total.
//
// And guarded on the *user's* answer first, because the
// automatic rule can only fire where the tags declared a
// total: an album that says nothing is not an album that is
// complete, and it used to be shown as one.
const answer = this.completenessAnswer();
const incomplete = answer?.known && !answer.complete;
if (incomplete) {
const fullRelease = this.findLibraryCluster(clusters);
if (this.showFullTracklist ?? (answer?.known && !answer.complete)) {
const fullRelease = this.fullReleaseCluster(clusters);
if (fullRelease) {
return {
key: 'synthetic:library',
label: 'Your Library',
sublabel: `${answer?.owned ?? 0} of ${answer?.expected ?? 0} tracks · ${this.clusterLabel(fullRelease)}`,
sublabel: answer?.known
? `${answer.owned} of ${answer.expected} tracks · ${this.clusterLabel(fullRelease)}`
: `${this.clusterLabel(fullRelease)} · full tracklist`,
group: 'aggregate',
syntheticKind: 'library',
tracks: fullRelease.representative.tracks ?? [],
@@ -1861,6 +2020,25 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
};
}
/**
* The release to draw when the whole album is wanted rather than
* the files on disk.
*
* `findLibraryCluster` is the right answer where it has one the
* release the user's tracks overlap most but it is a guess over
* the `inLibrary` flags and returns nothing at all when none of
* them are set, which is every untagged library. Falling back to
* the highest-scoring cluster is what makes the switch work there;
* that is the same release the page would call "Standard", and the
* sublabel names it either way rather than leaving the user to
* wonder whose tracklist they are reading.
*/
private fullReleaseCluster(
clusters: ReleaseCluster[],
): ReleaseCluster | undefined {
return this.findLibraryCluster(clusters) ?? clusters[0];
}
/**
* Fallback only: used when there's no local album to anchor on
* (see `buildLibraryEntry`). Finds the cluster with the highest
@@ -2237,8 +2415,10 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
entity-type="album"
@catalog-retry=${this.retryCatalog}
></catalog-scope-notice>
${this.renderVersionSelector()}
${this.renderChosenVersion()}
${this.renderTracklistScope()}
${this.renderTracklist()}
${this.renderVersionSelector()}
</div>
<track-details></track-details>
`;
@@ -2378,7 +2558,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
data-testid="album-queue"
@click=${() => this.queueOwned()}
>
<wa-icon slot="start" name="list"></wa-icon>
<wa-icon slot="start" name=${ICON_QUEUE}></wa-icon>
Add to queue
</wa-button>
${partial
@@ -2681,7 +2861,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
of the same Free glyph carry the toggle instead. -->
<wa-icon
slot="start"
name=${this.isRequested ? 'solid/bookmark' : 'regular/bookmark'}
name=${this.isRequested ? ICON_REQUESTED : ICON_CAN_REQUEST}
></wa-icon>
${this.isRequested ? 'Requested' : 'Request this'}
</wa-button>
@@ -2889,26 +3069,81 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
/* ── Version Selector (R025, R026, R027) ── */
/**
* Which pressing is on screen said only when the user chose it.
*
* The selector is a disclosure below the tracklist now, so nothing
* above the list names the version it came from. That is right for
* the default, which is what the header already describes; it is
* wrong the moment someone picks a different one, because then the
* tracklist and the page disagree and the control that explains it
* is off the bottom of the screen.
*
* `defaultVersionKey` is the whole test. A quiet line that appears
* on every album would be the thing this issue removed, one size
* smaller.
*/
private renderChosenVersion() {
if (this.loadingReleases || this.errorReleases) return nothing;
if (!this.selectedVersionKey) return nothing;
if (this.selectedVersionKey === this.defaultVersionKey) return nothing;
const current = this.currentVersion();
if (!current) return nothing;
return html`
<p class="chosen-version">
Showing <strong>${current.label}</strong>
${current.sublabel}.
<button
type="button"
class="chosen-version-reset"
@click=${this.resetVersion}
>
Use the default version
</button>
</p>
`;
}
/** Back to what `buildClusters` picked, without opening the panel. */
private resetVersion = () => {
if (!this.defaultVersionKey) return;
this.selectedVersionKey = this.defaultVersionKey;
};
/**
* "Other versions" a disclosure, below the tracklist.
*
* Choosing which pressing you are looking at is an advanced,
* metadata-repair task, and it used to sit directly above the
* tracklist with a heading and a paragraph of prose explaining our
* clustering heuristic. It is not removed matching the wrong
* release is a real problem and this is how it gets fixed it is
* demoted (#17).
*
* Two things about the shape are load-bearing, and both are
* `config-section`'s rules rather than new ones. The header is a
* real `<button aria-expanded aria-controls>` inside the heading
* that names the section, so it is reachable by Tab and by heading
* navigation alike. And the body **renders unconditionally and is
* toggled with `hidden`**, because `aria-controls` has to name an
* element that is in the DOM.
*
* The loading and error states this used to own are gone rather
* than moved. Both were unguarded, so they took the primary slot on
* every album regardless of whether there was ever going to be a
* choice: the spinner said the same thing `renderTracklist` was
* already saying about the same fetch, and the error is the one
* `catalog-scope-notice` shows at the top of the page with a retry
* every path that sets `errorReleases` also sets `catalogFailed`,
* which is the only route to `unavailable`. What the tracklist does
* with a failure is now the tracklist's own business.
*/
private renderVersionSelector() {
if (this.loadingReleases) {
return html`
<section>
<h3 class="section-header">Versions</h3>
<div class="section-loading">Loading releases\u2026</div>
</section>
`;
}
if (this.errorReleases) {
return html`
<section>
<h3 class="section-header">Versions</h3>
<div class="section-error">
<wa-icon name="triangle-exclamation"></wa-icon>
${this.errorReleases}
</div>
</section>
`;
}
if (this.loadingReleases || this.errorReleases) return nothing;
// A dropdown is only a choice if the choices differ. Counting
// *entries* is the wrong test: a release group routinely has
@@ -2919,7 +3154,9 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
//
// Distinct *tracklists* is the real question, and it is already
// computed: clusters are keyed by tracklist fingerprint.
if (this.distinctTracklistCount() <= 1) return nothing;
const choices = this.distinctTracklistCount();
if (choices <= 1) return nothing;
const aggregateEntries = this.versionEntries.filter(
(e) => e.group === 'aggregate',
@@ -2929,35 +3166,59 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
);
return html`
<div class="version-selector">
<div class="version-selector-row">
<label for="version-select">Version</label>
<select
id="version-select"
@change=${this.handleVersionChange}
aria-label="Select release version"
<section class="versions">
<h3 class="versions-heading">
<button
type="button"
class="versions-toggle"
aria-expanded=${this.versionsOpen ? 'true' : 'false'}
aria-controls="versions-body"
@click=${this.toggleVersions}
>
${aggregateEntries.length > 0
? html`
<optgroup label="Aggregate">
${aggregateEntries.map((e) =>
this.renderVersionOption(e),
)}
</optgroup>
`
: nothing}
<optgroup label="Versions">
${clusterEntries.map((e) =>
this.renderVersionOption(e),
)}
</optgroup>
</select>
<wa-icon name="chevron-down" aria-hidden="true"></wa-icon>
Other versions of this album (${choices})
</button>
</h3>
<div id="versions-body" ?hidden=${!this.versionsOpen}>
<p class="versions-intro">
A release group can have several pressings with
different tracklists. Pick another if the one
above does not match your copy.
</p>
<div class="version-selector">
<div class="version-selector-row">
<label for="version-select">Version</label>
<select
id="version-select"
@change=${this.handleVersionChange}
>
${aggregateEntries.length > 0
? html`
<optgroup label="Aggregate">
${aggregateEntries.map((e) =>
this.renderVersionOption(e),
)}
</optgroup>
`
: nothing}
<optgroup label="Versions">
${clusterEntries.map((e) =>
this.renderVersionOption(e),
)}
</optgroup>
</select>
</div>
${this.renderVersionMeta()}
</div>
</div>
${this.renderVersionMeta()}
</div>
</section>
`;
}
private toggleVersions = () => {
this.versionsOpen = !this.versionsOpen;
};
/**
* One option in the version list.
*
@@ -3036,6 +3297,86 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
/* ── Tracklist ── */
/**
* "Show the whole album" the switch between the files on disk and
* the release they are part of.
*
* The page could already draw the full release with the missing
* rows dimmed, and did so automatically once the tags said the
* album was incomplete. What it could not do was be *asked*: where
* the files declare no per-disc total and the catalog has none
* either, the rule never fires, so a partly-owned album showed only
* the tracks the user had and nothing said the rest existed.
*
* Three things about when it appears, all of them the same rule
* a control that cannot change what is on screen is worse than no
* control, which is what the version dropdown's own guard is for:
*
* - Only against the synthetic "Your Library" entry. Every other
* entry *is* a catalog tracklist already.
* - Only when a catalog release exists to switch to.
* - Only when the two differ. A complete album's release has the
* same rows as its files, so the switch would redraw the same
* list and read as broken.
*/
private renderTracklistScope() {
const current = this.currentVersion();
if (current?.syntheticKind !== 'library') return nothing;
if (this.localTracks.length === 0) return nothing;
const full = this.fullReleaseCluster(this.clustersOf(this.versionEntries));
const fullCount = full?.representative.tracks?.length ?? 0;
if (fullCount === 0 || fullCount <= this.localTracks.length) {
return nothing;
}
const showing = current.tracks.length > this.localTracks.length;
return html`
<div class="tracklist-scope">
<wa-switch
size="small"
?checked=${showing}
@change=${this.handleTracklistScopeChange}
>
Show the whole album
</wa-switch>
<span class="tracklist-scope-hint">
${showing
? `${this.localTracks.length} of ${fullCount} tracks are in your library`
: `${fullCount - this.localTracks.length} more tracks are on this release`}
</span>
</div>
`;
}
/**
* The clusters behind the current entries.
*
* `buildClusters` computes them and keeps only the entries, so this
* recovers them rather than storing the array twice two copies of
* a list rebuilt on four different events is how they come to
* disagree.
*/
private clustersOf(entries: VersionEntry[]): ReleaseCluster[] {
return entries
.filter((e) => e.group === 'cluster')
.map((e) => e.cluster)
.filter((c): c is ReleaseCluster => !!c);
}
private handleTracklistScopeChange = (e: Event) => {
this.showFullTracklist = (e.target as HTMLInputElement).checked;
// The entries are derived, so the switch rebuilds them rather
// than patching the one it changed. `buildClusters` re-defaults
// the selection, which lands back on "Your Library" — the only
// entry this control is ever shown against.
this.buildClusters();
};
/**
* The heading is there and is not drawn.
*
@@ -3056,8 +3397,21 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
`;
}
if (this.errorReleases) {
// Error already shown in version selector section
return nothing;
// The failure belongs to the list that is missing because
// of it. This used to return `nothing` and lean on the
// version selector's own error block to have said it, which
// is precisely the coupling that made demoting the selector
// a rewrite rather than a move: a control in a collapsed
// disclosure cannot be the page's error surface.
return html`
<section>
<h3 class="sr-only">Tracklist</h3>
<div class="section-error">
<wa-icon name="triangle-exclamation"></wa-icon>
${this.errorReleases}
</div>
</section>
`;
}
const current = this.currentVersion();
if (!current) {
@@ -3128,7 +3482,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
aria-disabled=${owned ? 'false' : 'true'}
aria-label=${owned
? `Play “${track.title}`
: `${track.title} — not in your library`}
: unownedLabel(track.title, 'track')}
@dblclick=${() => this.onTrackRowDblClick(track)}
@contextmenu=${(e: MouseEvent) => this.onTrackContextMenu(e, track)}
@keydown=${(e: KeyboardEvent) => this.onTrackRowKeydown(e, track)}
@@ -3199,7 +3553,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
@click=${() => this.onContextMenuAction('add-to-queue')}
@mouseenter=${() => this.ctxMenu.closePlaylistSubmenu()}
>
<wa-icon slot="icon" name="plus"></wa-icon>
<wa-icon slot="icon" name=${ICON_QUEUE}></wa-icon>
Add to Queue
</wa-dropdown-item>
<wa-dropdown-item
@@ -3218,7 +3572,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
this.openPlaylistSubmenu();
}}
>
<wa-icon slot="icon" name="plus"></wa-icon>
<wa-icon slot="icon" name=${ICON_PLAYLIST}></wa-icon>
Add to Playlist
<span class="submenu-arrow">&#9654;</span>
</wa-dropdown-item>
@@ -39,7 +39,17 @@ import { EventsOn } from '@runtime/runtime';
import { Events } from '../../events';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '../library-status-indicator/library-status-indicator.js';
import { libraryStatusFor, toggleRequest } from '@utils/library-status';
import {
albumBadgeFor,
libraryStatusFor,
toggleRequest,
} from '@utils/library-status';
import {
isOwned,
ownershipLabel,
unownedStyles,
} from '@utils/ownership';
import { completenessStore } from '@store/completeness-store';
import '../catalog-scope-notice/catalog-scope-notice.js';
import type { CatalogScope } from '../catalog-scope-notice/catalog-scope-notice.js';
import { queueStore } from '../../store/queue-store';
@@ -59,6 +69,12 @@ import { dict, dictByName } from '@utils/binding';
import type { TrackDetails } from '@components/track-details/track-details.js';
import { showTrackDetailsForPath } from '@utils/track-details-opener.js';
import '@components/playlist-picker/playlist-picker.js';
import {
ICON_CAN_REQUEST,
ICON_PLAYLIST,
ICON_QUEUE,
ICON_REQUESTED,
} from '@utils/icon-language';
/* ── Constants ── */
@@ -172,7 +188,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
@state() private discoRowSize = 5;
private discoObserver?: ResizeObserver;
@state() private similarExpanded = false;
private libraryMBIDs = new Set<string>();
/* ── Release prefetch ── */
@@ -251,6 +266,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
designTokens,
exploreLinkStyles,
contextMenuStyles,
unownedStyles,
css`
:host {
display: flex;
@@ -989,6 +1005,9 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
/** Unsubscribe handle for the requests list. */
private unsubRequests: (() => void) | null = null;
/** Unsubscribes the "how much of this album is here" repaint. */
private unsubCompleteness: (() => void) | null = null;
override connectedCallback() {
super.connectedCallback();
if (this.artistMBID || this.localArtistId) {
@@ -1001,6 +1020,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
this.unsubRequests = downloadStore.subscribe(() => this.requestUpdate());
void downloadStore.init().then(() => this.requestUpdate());
// The count behind a partly-held album lands a frame after the
// cards do, since the store batches a screenful into one query.
this.unsubCompleteness = completenessStore.subscribe(() =>
this.requestUpdate(),
);
// A background discography fetch (top tracks / top releases for an
// artist that wasn't indexed yet) finished — re-fetch those two
// sections, once per artist, so they fill in without the initial
@@ -1039,6 +1064,8 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
super.disconnectedCallback();
this.unsubRequests?.();
this.unsubRequests = null;
this.unsubCompleteness?.();
this.unsubCompleteness = null;
this.unsubDiscogReady?.();
this.unsubSimilarReady?.();
if (this.discogFallbackTimer) clearTimeout(this.discogFallbackTimer);
@@ -1567,10 +1594,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
this.catalogPending = false;
}
// Populate libraryMBIDs from the inLibrary flag (already
// set by the backend via local_release_group_id cross-ref).
this.checkLibrary();
// Batch-resolve cover art for discography (lower priority — loaded after top sections).
void this.batchResolveThumbnails(
rgs?.map((r) => ({ mbid: r.mbid, albumName: r.title, artistName: r.artistCredit }))
@@ -1897,23 +1920,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
}
}
private checkLibrary() {
// Backend now populates `inLibrary` directly on each MBReleaseGroup
// via the local_release_group_id cross-reference column. Just read it.
let updated = false;
for (const rg of this.releaseGroups) {
if (rg.mbid && rg.inLibrary && !this.libraryMBIDs.has(rg.mbid)) {
this.libraryMBIDs.add(rg.mbid);
updated = true;
}
}
if (updated) {
this.requestUpdate();
}
}
/* ── Playback ── */
/**
@@ -2009,11 +2015,14 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
/**
* File path for one top track, resolved by recording MBID the
* same key `inLibrary`/`localId` were set from. Works whether or
* not the containing release itself matched a local album.
* same key `localId` was set from. Works whether or not the
* containing release itself matched a local album.
*
* Gated on the same answer the row is drawn from, or a row drawn
* dimmed and `aria-disabled` would still try to play and fail.
*/
private async trackFilePath(track: LBTopRecording): Promise<string | null> {
if (!(track.inLibrary || track.localId) || !track.recordingMbid) return null;
if (!isOwned(track) || !track.recordingMbid) return null;
const libraryID = libraryStore.getSelectedLibraryId() ?? 0;
const byMBID = await dictByName(
@@ -2057,7 +2066,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
}
private isTrackOwned(track: LBTopRecording): boolean {
return Boolean(track.inLibrary || track.localId);
return isOwned(track);
}
private onTrackRowDblClick(track: LBTopRecording): void {
@@ -2100,7 +2109,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
mbid: rg.releaseGroupMbid || '',
localId: rg.localId ?? 0,
title: rg.title,
owned: Boolean(rg.inLibrary || rg.localId),
owned: isOwned(rg),
};
}
@@ -2120,10 +2129,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
mbid: isLocal ? '' : rg.mbid || '',
localId: Number.isFinite(localId) ? localId : 0,
title: rg.title,
owned:
this.libraryMBIDs.has(rg.mbid) ||
Boolean(rg.inLibrary) ||
localId > 0,
// The same answer the menu gates Play on, which is the
// point: this used to be `inLibrary` too, so a card could
// report itself owned, be offered no Play (that item is
// gated on the local id) and be offered no request either
// (that one is gated on *not* owned).
owned: localId > 0,
};
}
@@ -2674,7 +2685,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
@click=${() => this.onContextMenuAction('add-to-queue')}
@mouseenter=${() => this.ctxMenu.closePlaylistSubmenu()}
>
<wa-icon slot="icon" name="plus"></wa-icon>
<wa-icon slot="icon" name=${ICON_QUEUE}></wa-icon>
Add to Queue
</wa-dropdown-item>
<wa-dropdown-item
@@ -2693,7 +2704,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
void this.openPlaylistSubmenu(true);
}}
>
<wa-icon slot="icon" name="plus"></wa-icon>
<wa-icon slot="icon" name=${ICON_PLAYLIST}></wa-icon>
Add to Playlist
<span class="submenu-arrow">&#9654;</span>
</wa-dropdown-item>
@@ -2737,7 +2748,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
Play
</wa-dropdown-item>
<wa-dropdown-item @click=${() => void this.onReleaseAction('add-to-queue')}>
<wa-icon slot="icon" name="plus"></wa-icon>
<wa-icon slot="icon" name=${ICON_QUEUE}></wa-icon>
Add to Queue
</wa-dropdown-item>
<wa-dropdown-item @click=${() => void this.onReleaseAction('play-next')}>
@@ -2751,7 +2762,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
<wa-dropdown-item @click=${() => void this.onReleaseRequestToggle()}>
<wa-icon
slot="icon"
name=${requested ? 'xmark' : 'bookmark'}
name=${requested ? ICON_REQUESTED : ICON_CAN_REQUEST}
></wa-icon>
${requested ? 'Cancel Request' : 'Request This'}
</wa-dropdown-item>
@@ -2789,9 +2800,15 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
appearance=${request ? 'filled' : 'outlined'}
@click=${() => void this.toggleFollow(request?.id)}
>
<!-- This was bookmark-check, which is not in
names.txt and so has rendered the missing-icon
fallback a circled question mark on every
followed artist since it was written. A
backtick around that name would end this
template literal, which is why there is none. -->
<wa-icon
slot="start"
name=${request ? 'bookmark-check' : 'bookmark'}
name=${request ? ICON_REQUESTED : ICON_CAN_REQUEST}
></wa-icon>
${request ? 'Following' : 'Follow for new releases'}
</wa-button>
@@ -2932,15 +2949,20 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
<div class="top-section-col top-section-col-tracks">
<h3 class="section-header">Top Tracks</h3>
<div class="track-list">
${tracks.map(
(t, i) => html`
${tracks.map((t, i) => {
const owned = this.isTrackOwned(t);
return html`
<div
class=${classMap({ 'track-item': true, owned: this.isTrackOwned(t) })}
class=${classMap({
'track-item': true,
owned,
unowned: !owned,
})}
tabindex="0"
role="button"
aria-label=${this.isTrackOwned(t)
? `Play “${t.trackName}`
: `${t.trackName} — not in your library`}
aria-disabled=${owned ? 'false' : 'true'}
aria-label=${ownershipLabel(owned, 'Play', t.trackName, 'track')}
@dblclick=${() => this.onTrackRowDblClick(t)}
@contextmenu=${(e: MouseEvent) => this.onTrackContextMenu(e, t)}
@keydown=${(e: KeyboardEvent) => this.onTrackRowKeydown(e, t)}
@@ -2965,16 +2987,18 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
<span class="track-listens">
${formatListenCount(t.totalListenCount)} plays
</span>
<library-status-indicator
status=${libraryStatusFor(Boolean(t.inLibrary || t.localId), t.recordingMbid)}
entity-type="track"
label=${t.trackName}
request-mbid=${t.recordingMbid}
request-artist=${t.artistName ?? ''}
></library-status-indicator>
${owned
? nothing
: html`<library-status-indicator
status=${libraryStatusFor(false, t.recordingMbid)}
entity-type="track"
label=${t.trackName}
request-mbid=${t.recordingMbid}
request-artist=${t.artistName ?? ''}
></library-status-indicator>`}
</div>
`,
)}
`;
})}
</div>
${canExpandTracks
? html`
@@ -3045,10 +3069,16 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
private renderTopReleaseCard(rg: LBTopReleaseGroup) {
const artURL = this.thumbnailURLs.get(rg.releaseGroupMbid) || '';
const target = this.topReleaseTarget(rg);
const owned = target.owned;
const badge = albumBadgeFor(
{ localId: target.localId },
rg.releaseGroupMbid,
);
return html`
<div
class="top-release-card"
class=${classMap({ 'top-release-card': true, unowned: !owned })}
aria-label=${ownershipLabel(owned, 'Album', rg.title, 'album')}
@click=${() => this.navigateToTopRelease(rg)}
role="button"
tabindex="0"
@@ -3080,14 +3110,18 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
<div class="top-release-meta-text">
${rg.date ? html`<span>${extractYear(rg.date)}</span>` : nothing}
</div>
<library-status-indicator
status=${libraryStatusFor(Boolean(rg.inLibrary || rg.localId), rg.releaseGroupMbid)}
entity-type="album"
label=${rg.title}
request-mbid=${rg.releaseGroupMbid}
request-artist=${this.artist?.name ?? ''}
size="18"
></library-status-indicator>
${badge.status === 'in-library'
? nothing
: html`<library-status-indicator
status=${badge.status}
owned=${badge.owned}
expected=${badge.expected}
entity-type="album"
label=${rg.title}
request-mbid=${rg.releaseGroupMbid}
request-artist=${this.artist?.name ?? ''}
size="18"
></library-status-indicator>`}
</div>
</div>
</div>
@@ -3177,14 +3211,15 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
private renderAlbumCard(rg: MBReleaseGroup) {
const artURL = this.thumbnailURLs.get(rg.mbid) || '';
const year = extractYear(rg.firstReleaseDate);
const inLibrary = this.libraryMBIDs.has(rg.mbid) || Boolean(rg.inLibrary);
const status = libraryStatusFor(inLibrary, rg.mbid);
const target = this.albumTarget(rg);
const owned = target.owned;
const badge = albumBadgeFor({ localId: target.localId }, target.mbid);
return html`
<div
class="album-card"
class=${classMap({ 'album-card': true, unowned: !owned })}
aria-label=${ownershipLabel(owned, 'Album', rg.title, 'album')}
@click=${() => this.navigateToAlbum(rg)}
role="button"
tabindex="0"
@@ -3213,13 +3248,17 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
<div class="album-meta-text">
${year ? html`<span>${year}</span>` : nothing}
</div>
<library-status-indicator
status=${status}
entity-type="album"
label=${rg.title}
request-mbid=${rg.mbid}
request-artist=${this.artist?.name ?? ''}
></library-status-indicator>
${badge.status === 'in-library'
? nothing
: html`<library-status-indicator
status=${badge.status}
owned=${badge.owned}
expected=${badge.expected}
entity-type="album"
label=${rg.title}
request-mbid=${rg.mbid}
request-artist=${this.artist?.name ?? ''}
></library-status-indicator>`}
</div>
</div>
`;
@@ -1,5 +1,11 @@
import { avatarBackground } from '@utils/avatar-color';
import { libraryStatusFor } from '@utils/library-status';
import { albumBadgeFor, libraryStatusFor } from '@utils/library-status';
import {
isOwned,
ownershipLabel,
unownedStyles,
} from '@utils/ownership';
import { completenessStore } from '@store/completeness-store';
import { downloadStore } from '@store/download-store';
import { LitElement, html, css, nothing } from 'lit';
import { customElement, state, query as litQuery } from 'lit/decorators.js';
@@ -36,6 +42,7 @@ import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import { dict, dictByName } from '@utils/binding';
import { ICON_QUEUE } from '@utils/icon-language';
/** The region explore's own action failures (play/queue) are rendered in. */
export const ExploreRegion = 'explore';
@@ -170,7 +177,6 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
private searchDebounceTimer?: ReturnType<typeof setTimeout>;
private thumbnailCache = new LRUMap<string, string>(THUMBNAIL_CACHE_LIMIT);
private artistImageCache = new LRUMap<string, string>(ARTIST_IMAGE_CACHE_LIMIT);
private libraryMBIDs = new Set<string>();
constructor() {
super();
@@ -225,6 +231,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
srOnly,
exploreLinkStyles,
contextMenuStyles,
unownedStyles,
css`
:host {
display: block;
@@ -811,6 +818,13 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
// Explore should not pay for it.
this.whileActive(downloadStore.subscribe(() => this.requestUpdate()));
void downloadStore.init().then(() => this.requestUpdate());
// How much of an owned album is here arrives a frame after the
// cards do — the store coalesces a screenful into one query —
// so a card that turns out to be 9 of 12 repaints when the
// answer lands rather than showing a plain tick until something
// else happens to re-render the grid.
this.whileActive(completenessStore.subscribe(() => this.requestUpdate()));
}
/** A debounced search that lands after the user has left the page is
@@ -1082,7 +1096,6 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
this.results?.artists ?? [],
this.results?.releaseGroups ?? [],
);
this.checkLibrary();
} catch (err) {
if (version !== this.searchVersion) return;
@@ -1228,8 +1241,11 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
void this.playAlbum(rg, false);
}
private onRecordingRowDblClick(r: { mbid: string; inLibrary: boolean; localId?: number }): void {
if (!r.inLibrary && !r.localId) return;
// The same answer the row is drawn from. It used to accept
// `inLibrary` as well, so a row drawn dimmed and `aria-disabled`
// would still try to play and fail with a notification.
private onRecordingRowDblClick(r: { mbid: string; localId?: number }): void {
if (!isOwned(r)) return;
void this.playRecording(r.mbid);
}
@@ -1342,7 +1358,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
Play
</wa-dropdown-item>
<wa-dropdown-item @click=${() => this.onContextMenuAction('add-to-queue')}>
<wa-icon slot="icon" name="plus"></wa-icon>
<wa-icon slot="icon" name=${ICON_QUEUE}></wa-icon>
Add to Queue
</wa-dropdown-item>
<wa-dropdown-item @click=${() => this.onContextMenuAction('play-next')}>
@@ -1664,42 +1680,6 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
}
}
/**
* Check which result MBIDs exist in the local library.
*/
private checkLibrary() {
if (!this.results) return;
// Backend now populates `inLibrary` directly on each MB result
// via the local_*_id cross-reference columns. Just read those.
let updated = false;
for (const a of this.results.artists ?? []) {
if (a.mbid && a.inLibrary && !this.libraryMBIDs.has(a.mbid)) {
this.libraryMBIDs.add(a.mbid);
updated = true;
}
}
for (const rg of this.results.releaseGroups ?? []) {
if (rg.mbid && rg.inLibrary && !this.libraryMBIDs.has(rg.mbid)) {
this.libraryMBIDs.add(rg.mbid);
updated = true;
}
}
for (const r of this.results.recordings ?? []) {
if (r.mbid && r.inLibrary && !this.libraryMBIDs.has(r.mbid)) {
this.libraryMBIDs.add(r.mbid);
updated = true;
}
}
if (updated) {
this.requestUpdate();
}
}
/* ── Navigation ── */
private navigateToArtist(artist: MBArtist) {
@@ -2106,12 +2086,16 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
: nothing}
<div class="horizontal-row">
${artists.map((a) => {
const owned = isOwned(a);
const name = a.englishName || a.name;
return html`
<div
class="artist-card"
class=${classMap({ 'artist-card': true, unowned: !owned })}
@click=${() => this.navigateToArtist(a)}
role="button"
tabindex="0"
aria-label=${ownershipLabel(owned, 'Artist', name, 'artist')}
@keydown=${(e: KeyboardEvent) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
@@ -2170,11 +2154,17 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
const artURL = this.thumbnailCache.get(rg.mbid) || '';
const year = extractYear(rg.firstReleaseDate);
const owned = Boolean(rg.localId);
const owned = isOwned(rg);
const badge = albumBadgeFor(rg, rg.mbid);
return html`
<div
class=${classMap({ 'album-card': true, owned })}
class=${classMap({
'album-card': true,
owned,
unowned: !owned,
})}
aria-label=${ownershipLabel(owned, 'Album', rg.title, 'album')}
@click=${() => this.navigateToAlbum(rg)}
@dblclick=${() => this.onAlbumCardDblClick(rg)}
@contextmenu=${(e: MouseEvent) =>
@@ -2227,13 +2217,17 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
: nothing}
${year ? html`<span>${year}</span>` : nothing}
</div>
<library-status-indicator
status=${libraryStatusFor(this.libraryMBIDs.has(rg.mbid) || Boolean(rg.inLibrary), rg.mbid)}
entity-type="album"
label=${rg.title}
request-mbid=${rg.mbid}
request-artist=${rg.artistCredit ?? ''}
></library-status-indicator>
${badge.status === 'in-library'
? nothing
: html`<library-status-indicator
status=${badge.status}
owned=${badge.owned}
expected=${badge.expected}
entity-type="album"
label=${rg.title}
request-mbid=${rg.mbid}
request-artist=${rg.artistCredit ?? ''}
></library-status-indicator>`}
</div>
</div>
`;
@@ -2248,12 +2242,20 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
<section>
<h3 class="section-header">Tracks</h3>
<div class="track-list">
${recordings.map(
(r) => html`
${recordings.map((r) => {
const owned = isOwned(r);
return html`
<div
class=${classMap({ 'track-item': true, owned: Boolean(r.inLibrary || r.localId) })}
class=${classMap({
'track-item': true,
owned,
unowned: !owned,
})}
role="button"
tabindex="0"
aria-disabled=${owned ? 'false' : 'true'}
aria-label=${ownershipLabel(owned, 'Play', r.title, 'track')}
@dblclick=${() => this.onRecordingRowDblClick(r)}
@contextmenu=${(e: MouseEvent) =>
this.onExploreContextMenu(e, {
@@ -2290,16 +2292,18 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
? html`<span class="track-duration">${formatDuration(r.length)}</span>`
: nothing}
</div>
<library-status-indicator
status=${libraryStatusFor(this.libraryMBIDs.has(r.mbid) || Boolean(r.inLibrary), r.mbid)}
entity-type="track"
label=${r.title}
request-mbid=${r.mbid}
request-artist=${r.artistCredit ?? ''}
></library-status-indicator>
${owned
? nothing
: html`<library-status-indicator
status=${libraryStatusFor(false, r.mbid)}
entity-type="track"
label=${r.title}
request-mbid=${r.mbid}
request-artist=${r.artistCredit ?? ''}
></library-status-indicator>`}
</div>
`,
)}
`;
})}
</div>
</section>
`;
@@ -35,6 +35,10 @@ import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js'
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import '@components/playlist-picker/playlist-picker.js';
import { dictByName } from '@utils/binding';
import {
ICON_PLAYLIST,
ICON_QUEUE,
} from '@utils/icon-language';
/** Pixels to change card width per scroll tick. */
const ZOOM_STEP = 16;
@@ -1211,7 +1215,7 @@ export class GenresView
>
<wa-icon
slot="icon"
name="plus"
name=${ICON_QUEUE}
></wa-icon>
Add to Queue
</wa-dropdown-item>
@@ -1257,7 +1261,7 @@ export class GenresView
>
<wa-icon
slot="icon"
name="plus"
name=${ICON_PLAYLIST}
></wa-icon>
Add to Playlist
<span
@@ -4,6 +4,11 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
import { toggleRequest } from '@utils/library-status';
import { notificationStore } from '@store/notification-store';
import { describeError } from '@utils/describe-error';
import {
ICON_CAN_REQUEST,
ICON_IN_LIBRARY,
ICON_REQUESTED,
} from '@utils/icon-language';
/**
* Library status for an entity (artist, album, or track).
@@ -248,18 +253,25 @@ export class LibraryStatusIndicator extends LitElement {
* hourglass says "wait, this is under way", which overstates what a
* request is: nothing may be downloading, nothing may ever be found,
* and the user can leave one sitting on the list indefinitely. A
* bookmark says the honest thing it is on your list and reads as
* the opposite of the plus that put it there, which is what a
* toggle's two states have to do.
* bookmark says the honest thing it is on your list.
*
* The *other* state is the outline of that same bookmark, not a
* plus. Two states of one toggle have to read as each other's
* opposite, and a plus and a bookmark do not this badge showed a
* plus on the same page as a "Request this" button already using
* the outline/solid pair, forty pixels away. That is the fault
* `utils/library-status.ts` was written for, one layer down: it
* made the two agree on what wanting *means* and left them
* disagreeing on what it looks like.
*/
private iconName(): string {
switch (this.status) {
case 'in-library':
return 'check';
return ICON_IN_LIBRARY;
case 'queued':
return 'bookmark';
return ICON_REQUESTED;
default:
return 'plus';
return ICON_CAN_REQUEST;
}
}
@@ -285,9 +297,23 @@ export class LibraryStatusIndicator extends LitElement {
// row to one and nothing to the other, and "Add … to library"
// was the old button's promise written into the copy.
if (this.actionable) {
return this.status === 'queued'
? `Cancel the request for ${kind}${name}`
: `Request ${kind}${name}`;
if (this.status === 'queued') {
return `Cancel the request for ${kind}${name}`;
}
// A partly-held album is actionable *and* has a count, and
// the count does not survive being named after the action
// alone. The `partial` case below says why it matters — a
// ring says "some" to a sighted user and nothing to anyone
// else — and that argument does not stop applying because
// the badge became clickable. This branch used to say only
// "Request album X", so the one state the ring exists for
// was the one state whose name did not mention it.
if (this.status === 'partial') {
return `Request the rest of ${kind}${name}${this.owned} of ${this.expected} tracks are in your library`;
}
return `Request ${kind}${name}`;
}
switch (this.status) {
@@ -14,6 +14,7 @@ import { creditStore } from '@store/credit-store';
import { FavoritesController } from '@store/controllers/favorites-controller';
import { designTokens } from '../../styles/tokens.css';
import { srOnly } from '../../styles/sr-only.css';
import { ICON_QUEUE } from '@utils/icon-language';
/**
* What is playing, at the size a phone has room for (plan 016 B2,
@@ -339,7 +340,7 @@ export class NowPlayingView extends LitElement {
aria-label="Show the queue"
@click=${this.openQueue}
>
<wa-icon name="list"></wa-icon>
<wa-icon name=${ICON_QUEUE}></wa-icon>
</button>
</header>
`;
@@ -70,6 +70,10 @@ import {
} from '@utils/explore-link';
import { designTokens } from '../../styles/tokens.css';
import { list } from '@utils/binding';
import {
ICON_PLAYLIST,
ICON_QUEUE,
} from '@utils/icon-language';
/** One playlist row: the track and its position in the *playlist*,
* which is not its position in the filtered view. */
@@ -1358,7 +1362,7 @@ export class PlaylistDetails
</button>
<div class="playlist-avatar">
<wa-icon
name="list"
name=${ICON_PLAYLIST}
></wa-icon>
</div>
<div class="playlist-info">
@@ -1665,7 +1669,7 @@ export class PlaylistDetails
>
<wa-icon
slot="icon"
name="plus"
name=${ICON_QUEUE}
></wa-icon>
Add to Queue
</wa-dropdown-item>
@@ -1720,7 +1724,7 @@ export class PlaylistDetails
>
<wa-icon
slot="icon"
name="plus"
name=${ICON_PLAYLIST}
></wa-icon>
Add to Playlist
<span
@@ -18,6 +18,7 @@ import { notificationStore } from '@store/notification-store';
import { describeError } from '@utils/describe-error';
import type { DuplicateTracksDialog } from '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js';
import { list } from '@utils/binding';
import { ICON_NEW } from '@utils/icon-language';
/**
* A reusable playlist picker that displays existing playlists
@@ -307,7 +308,7 @@ export class PlaylistPicker extends LitElement {
`
: nothing}
<wa-dropdown-item @click=${this.handleShowCreate}>
<wa-icon slot="icon" name="plus"></wa-icon>
<wa-icon slot="icon" name=${ICON_NEW}></wa-icon>
New Playlist
</wa-dropdown-item>
</div>
@@ -37,6 +37,10 @@ import { ViewLifecycleMixin } from '@utils/view-lifecycle';
import { FavoritesController } from '@store/controllers/favorites-controller';
import '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js';
import type { DuplicateTracksDialog } from '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js';
import {
ICON_NEW,
ICON_PLAYLIST,
} from '@utils/icon-language';
const SCROLL_DEBOUNCE_MS = 100;
@@ -1496,7 +1500,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
@dragleave=${this.onNewButtonDragLeave}
@drop=${this.onNewButtonDrop}
>
<wa-icon name="plus"></wa-icon>
<wa-icon name=${ICON_NEW}></wa-icon>
New Playlist
</button>
<button
@@ -1641,10 +1645,10 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
>
<div class="drop-zone-icon">
<wa-icon
name="plus"
name=${ICON_NEW}
></wa-icon>
</div>
<wa-icon name="list"></wa-icon>
<wa-icon name=${ICON_PLAYLIST}></wa-icon>
<p>No playlists yet</p>
<p style="font-size: 12px;">
Create a playlist or drop
@@ -1666,7 +1670,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
>
<div class="drop-zone-icon">
<wa-icon
name="plus"
name=${ICON_NEW}
></wa-icon>
</div>
<p>
@@ -1701,7 +1705,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
>
<div class="drop-zone-icon">
<wa-icon
name="plus"
name=${ICON_NEW}
></wa-icon>
</div>
</li>
@@ -60,6 +60,11 @@ import {
trackLink,
exploreLinkStyles,
} from '@utils/explore-link';
import {
ICON_NEW,
ICON_PLAYLIST,
ICON_QUEUE,
} from '@utils/icon-language';
/** Above this many tracks, clearing the queue asks first. */
const CLEAR_CONFIRM_THRESHOLD = 20;
@@ -1755,7 +1760,7 @@ export class QueuePanel
title="Add queue to playlist"
>
<wa-icon
name="plus"
name=${ICON_PLAYLIST}
></wa-icon>
</button>
</div>
@@ -1794,11 +1799,11 @@ export class QueuePanel
? html`<div class="empty-state">
<div class="drop-zone-icon">
<wa-icon
name="plus"
name=${ICON_NEW}
></wa-icon>
</div>
<wa-icon
name="list"
name=${ICON_QUEUE}
></wa-icon>
<p>Queue is empty</p>
<p style="font-size: 12px;">
@@ -1875,7 +1880,7 @@ export class QueuePanel
>
<wa-icon
slot="icon"
name="plus"
name=${ICON_PLAYLIST}
></wa-icon>
Add to Playlist
<span
@@ -4,6 +4,10 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
import { designTokens } from '../../styles/tokens.css';
import type { DragActiveDetail } from '@utils/drag-controller';
import {
ICON_PLAYLIST,
ICON_REQUESTED,
} from '@utils/icon-language';
type View = 'home' | 'playlists' | 'artists' | 'genres' | 'albums' | 'tracks' | 'explore' | 'downloads' | 'autotag' | 'jobs' | 'settings';
@@ -197,13 +201,13 @@ export class AppSidebar extends LitElement {
private navItems: NavItem[] = [
{ id: 'home', label: 'Home', icon: 'house' },
{ id: 'playlists', label: 'Playlists', icon: 'list' },
{ id: 'playlists', label: 'Playlists', icon: ICON_PLAYLIST },
{ id: 'artists', label: 'Artists', icon: 'user-group' },
{ id: 'genres', label: 'Genres', icon: 'masks-theater' },
{ id: 'albums', label: 'Albums', icon: 'compact-disc' },
{ id: 'tracks', label: 'Tracks', icon: 'music' },
{ id: 'explore', label: 'Explore', icon: 'globe' },
{ id: 'downloads', label: 'Downloads', icon: 'bookmark' },
{ id: 'downloads', label: 'Downloads', icon: ICON_REQUESTED },
{ id: 'autotag', label: 'Autotag', icon: 'tag' },
{ id: 'jobs', label: 'Jobs', icon: 'list-check' },
{ id: 'settings', label: 'Settings', icon: 'gear' },
@@ -61,6 +61,10 @@ import {
import '@components/smart-playlist-editor/smart-playlist-editor.js';
import { designTokens } from '../../styles/tokens.css';
import { list } from '@utils/binding';
import {
ICON_PLAYLIST,
ICON_QUEUE,
} from '@utils/icon-language';
/**
@@ -1481,7 +1485,7 @@ export class SmartPlaylistDetails
>
<wa-icon
slot="icon"
name="plus"
name=${ICON_QUEUE}
></wa-icon>
Add to Queue
</wa-dropdown-item>
@@ -1521,7 +1525,7 @@ export class SmartPlaylistDetails
>
<wa-icon
slot="icon"
name="plus"
name=${ICON_PLAYLIST}
></wa-icon>
Add to Playlist
<span
@@ -11,8 +11,16 @@ import '../library-status-indicator/library-status-indicator.js';
import type { LibraryStatus } from '../library-status-indicator/library-status-indicator.js';
import { creditLink, exploreLinkStyles } from '../../utils/explore-link';
import { creditStore } from '@store/credit-store';
import { libraryStatusFor } from '../../utils/library-status';
import { albumBadgeFor, libraryStatusFor } from '../../utils/library-status';
import {
isOwned,
ownershipLabel,
unownedStyles,
type OwnableKind,
} from '../../utils/ownership';
import { completenessStore } from '../../store/completeness-store';
import { downloadStore } from '../../store/download-store';
import { classMap } from 'lit/directives/class-map.js';
/** Format milliseconds as m:ss. */
function formatDuration(ms: number | undefined): string {
@@ -65,6 +73,9 @@ export class TopResultsRow extends LitElement {
/** Unsubscribes the credit-arrival repaint. */
private creditsUnsub?: () => void;
/** Unsubscribes the "how much of this album is here" repaint. */
private unsubCompleteness?: () => void;
override connectedCallback(): void {
super.connectedCallback();
@@ -74,6 +85,9 @@ export class TopResultsRow extends LitElement {
this.unsubRequests = downloadStore.subscribe(() =>
this.requestUpdate(),
);
this.unsubCompleteness = completenessStore.subscribe(() =>
this.requestUpdate(),
);
}
override disconnectedCallback(): void {
@@ -81,12 +95,15 @@ export class TopResultsRow extends LitElement {
this.creditsUnsub = undefined;
this.unsubRequests?.();
this.unsubRequests = undefined;
this.unsubCompleteness?.();
this.unsubCompleteness = undefined;
super.disconnectedCallback();
}
static override styles = [
designTokens,
exploreLinkStyles,
unownedStyles,
css`
:host {
display: block;
@@ -289,23 +306,41 @@ export class TopResultsRow extends LitElement {
? r.year || ''
: formatDuration(r.length) || '';
const status: LibraryStatus = libraryStatusFor(
Boolean(r.inLibrary),
r.mbid,
);
const entityType: 'artist' | 'album' | 'track' =
const entityType: OwnableKind =
r.entityType === 'artist'
? 'artist'
: r.entityType === 'release_group'
? 'album'
: 'track';
// Ownership is the local row, not the catalog's flag — see
// `utils/ownership.ts`. An album additionally says *how much*
// of it is here, which is the one thing a tick cannot.
const owned = isOwned(r);
const badge =
entityType === 'album'
? albumBadgeFor(r, r.mbid)
: {
status: libraryStatusFor(owned, r.mbid) as LibraryStatus,
owned: 0,
expected: 0,
};
// A card navigates whether or not the entity is owned, so it is
// not `aria-disabled` the way an unplayable track row is — the
// name is what carries the state to anyone not seeing the
// dimming.
return html`
<div
class="card"
class=${classMap({ card: true, unowned: !owned })}
role="button"
tabindex="0"
aria-label=${`${badgeLabel(r.entityType)}: ${r.name}`}
aria-label=${ownershipLabel(
owned,
`${badgeLabel(r.entityType)}:`,
r.name,
entityType,
)}
@click=${() => this.handleClick(r)}
@keydown=${(e: KeyboardEvent) => {
if (e.key !== 'Enter' && e.key !== ' ') return;
@@ -345,10 +380,12 @@ export class TopResultsRow extends LitElement {
: nothing}
</div>
</div>
${isArtist
${isArtist || badge.status === 'in-library'
? nothing
: html`<library-status-indicator
status=${status}
status=${badge.status}
owned=${badge.owned}
expected=${badge.expected}
entity-type=${entityType}
label=${r.name}
request-mbid=${r.mbid}
@@ -74,6 +74,10 @@ import { tracksByFilePath, tracksForPaths } from '@utils/track-index.js';
import '@components/playlist-picker/playlist-picker.js';
import type { TrackDetails } from '@components/track-details/track-details.js';
import type { CoverArtUrls } from '@components/track-details/track-details.js';
import {
ICON_PLAYLIST,
ICON_QUEUE,
} from '@utils/icon-language';
const COLUMN_STORAGE_KEY = 'track-list-column-widths';
const SORT_FIELD_KEY = 'track-list-sort-field';
@@ -2342,7 +2346,7 @@ export class TrackList
@click=${() => this.onContextMenuAction('add-to-queue')}
@mouseenter=${() => this.ctxMenu.closePlaylistSubmenu()}
>
<wa-icon slot="icon" name="plus"></wa-icon>
<wa-icon slot="icon" name=${ICON_QUEUE}></wa-icon>
Add to Queue
</wa-dropdown-item>
<wa-dropdown-item
@@ -2364,7 +2368,7 @@ export class TrackList
void this.ctxMenu.showPlaylistSubmenu(this.selection.getSelectedKeysOrdered());
}}
>
<wa-icon slot="icon" name="plus"></wa-icon>
<wa-icon slot="icon" name=${ICON_PLAYLIST}></wa-icon>
Add to Playlist
<span class="submenu-arrow">&#9654;</span>
</wa-dropdown-item>
+1
View File
@@ -24,6 +24,7 @@ solid/arrows-rotate
solid/arrow-up-short-wide
solid/backward-step
solid/bars
solid/bars-staggered
regular/bookmark
solid/bookmark
solid/box-open
+208
View File
@@ -0,0 +1,208 @@
/**
* How much of an album is here, keyed by local album id.
*
* The album page has always been able to say "9 of 12"; a card could
* not, so an album held two tracks of ten wore a plain green tick on
* every grid in the app which is the complaint the badge-accuracy
* issue was filed about, one surface over. A ring with a count needs a
* numerator and a denominator per album, and one `GetAlbumCompleteness`
* per card is fifty round trips for a grid of fifty.
*
* So this is `credit-store` one question over, and for the same three
* reasons.
*
* **The lookup is batched and coalesced across callers.** `request()`
* is per-card, cheap and safe to call from a render: it collects into a
* pending set and flushes once on the next frame, which turns a
* screenful into exactly one `GetAlbumsCompleteness`. A frame rather
* than a microtask because the point is to collect every card an update
* pass renders, and those do not land inside one microtask checkpoint.
*
* **Absence is cached as an answer.** An album with no files at all is
* omitted by the backend deliberately "I have none of this" and "I
* have no idea" are the third state `known` exists to keep apart so a
* miss stored as a hit would re-ask about it forever. What is cached is
* *asked*, and the unknown answer is a first-class value.
*
* **It is invalidated rather than aged.** Completeness is a fact about
* files on disk, and the two events that change it a scan finishing
* and tags being rewritten are exactly the ones `library-store`
* already discards everything on. `TracksRemovedFromLibrary` is the
* third: it changes a numerator without a rescan.
*/
import { GetAlbumsCompleteness } from '@go/library/library.js';
import type * as library from '@go/library/models.js';
import { EventsOn } from '@runtime/runtime';
import { Events } from '../events';
import { LRUMap } from '../utils/lru-map';
import { compact } from '../utils/binding';
import { registerCacheProbe } from '../utils/cache-stats';
/**
* Entries retained. Four small numbers each, so the bound is about
* unbounded growth rather than bytes and it is set well above any one
* grid, since a cap below the visible count evicts cards that are still
* on screen and the re-render fetches them straight back.
*/
export const COMPLETENESS_CACHE_LIMIT = 20_000;
/** What the app asks of a cached entry. */
export type Completeness = library.AlbumCompleteness;
/**
* The answer for an album nobody has a file of.
*
* Not a zero-of-zero: `known` false is what every consumer already
* reads as "say nothing", and it is the same value the album page's
* `completenessAnswer()` treats as unknowable.
*/
const NOTHING_HERE: Completeness = {
owned: 0,
expected: 0,
known: false,
complete: false,
} as Completeness;
class CompletenessStore {
private cache = new LRUMap<number, Completeness>(COMPLETENESS_CACHE_LIMIT);
/** Ids with a request in flight, so a re-render does not refetch. */
private inFlight = new Set<number>();
private listeners = new Set<() => void>();
/** Collected by request(), flushed as one batch on the next frame. */
private pending = new Set<number>();
private flushHandle: number | null = null;
constructor() {
registerCacheProbe('albumCompleteness', () => ({
entries: this.cache.size,
chars: this.cache.size * 4,
limit: COMPLETENESS_CACHE_LIMIT,
}));
EventsOn(Events.LibraryScanComplete, () => this.invalidate());
EventsOn(Events.TrackMetadataChanged, () => this.invalidate());
EventsOn(Events.TracksRemovedFromLibrary, () => this.invalidate());
}
/**
* Subscribe to "some answers arrived".
*
* Deliberately not per-album, for `credit-store`'s reason: a grid
* fetches its cards in one call and re-renders once, so a
* fine-grained signal would buy nothing and cost a listener a card.
*/
subscribe(fn: () => void): () => void {
this.listeners.add(fn);
return () => this.listeners.delete(fn);
}
/** The answer for one album, or undefined until it has been asked. */
get(albumID: number | undefined | null): Completeness | undefined {
if (!albumID || albumID <= 0) return undefined;
return this.cache.get(albumID);
}
/**
* Ask about one album, joining whatever batch is forming.
*
* Safe from inside a render: a set insert and a scheduled flush,
* with anything cached or in flight dropped. It does not loop
* after a flush every id asked for is cached, so the re-render's
* requests are all dropped and nothing notifies again.
*/
request(albumID: number | undefined | null): void {
if (!albumID || albumID <= 0) return;
if (this.cache.has(albumID)) return;
if (this.inFlight.has(albumID)) return;
if (this.pending.has(albumID)) return;
this.pending.add(albumID);
if (this.flushHandle !== null) return;
this.flushHandle = requestAnimationFrame(() => {
this.flushHandle = null;
const batch = [...this.pending];
this.pending.clear();
void this.ensure(batch);
});
}
/**
* Ask and read in one call, for use inside a template.
*
* A getter with a side effect, deliberately `credit-store` makes
* the same trade and for the same reason: the alternative is every
* call site writing `request(x)` beside `get(x)` and one of them
* eventually forgetting, which renders a permanently unknown
* completeness that looks exactly like an album with no totals.
*/
completeness(albumID: number | undefined | null): Completeness | undefined {
this.request(albumID);
return this.get(albumID);
}
/** Fetch for a list, skipping anything known or already in flight. */
async ensure(albumIDs: readonly number[]): Promise<void> {
const wanted = new Set<number>();
for (const id of albumIDs) {
if (!id || id <= 0) continue;
// `has` rather than `get`: probing must not mark an entry
// recently-used, or scrolling past a card would keep it
// alive ahead of one actually being rendered.
if (this.cache.has(id)) continue;
if (this.inFlight.has(id)) continue;
wanted.add(id);
}
if (wanted.size === 0) return;
const batch = [...wanted];
for (const id of batch) this.inFlight.add(id);
try {
const found = compact(await GetAlbumsCompleteness(batch));
for (const id of batch) {
this.cache.set(id, found[String(id)] ?? NOTHING_HERE);
}
this.notify();
} catch (err) {
// A count is an enrichment: without it a card shows the
// plain "you have this", which is what it showed before and
// is a weaker answer rather than a broken one.
console.error('Failed to load album completeness', err);
} finally {
for (const id of batch) this.inFlight.delete(id);
}
}
/** Drop everything: the files on disk changed. */
invalidate(): void {
this.cache = new LRUMap<number, Completeness>(
COMPLETENESS_CACHE_LIMIT,
);
this.notify();
}
private notify(): void {
for (const fn of this.listeners) fn();
}
}
export const completenessStore = new CompletenessStore();
+100
View File
@@ -0,0 +1,100 @@
/**
* What each icon in this app means, once.
*
* The set was a mix: `plus` meant "add to the queue", "add to a
* playlist", "make a new playlist" and "you do not own this" the
* first two *adjacent in the same context menu* while `list` meant
* the queue, the Playlists destination, and (in `queue-panel` alone)
* adding to the queue. Two icons carrying seven meanings between them
* is not a vocabulary, and a user cannot learn one that says four
* things.
*
* The rule these are chosen by: **an icon names the noun it acts on,
* not the verb.** "Add to queue" and "add to playlist" are the same
* verb on different nouns, so the noun is what has to differ which is
* also why adding to a playlist wears the Playlists destination's own
* icon rather than a generic plus. `plus` survives for exactly the one
* thing it is unambiguous about, making something that did not exist.
*
* Import these rather than writing a name inline. A literal string is
* how the last set drifted, and nothing catches it: a wrong-but-real
* icon renders perfectly.
*/
/** Start playing this now. */
export const ICON_PLAY = 'play';
/** Start playing this now, in a shuffled order. */
export const ICON_SHUFFLE = 'shuffle';
/**
* The queue, and putting something into it.
*
* One glyph for the noun and the action, so the button that opens the
* queue and the menu item that adds to it are visibly the same subject.
* The queue used to wear `list`, which is the Playlists destination.
*/
export const ICON_QUEUE = 'bars-staggered';
/** Put this next in the queue rather than at the end. */
export const ICON_PLAY_NEXT = 'forward-step';
/**
* A playlist, and adding something to one.
*
* The same icon as the Playlists destination in the sidebar, which is
* the point: the menu item says where the thing is going.
*/
export const ICON_PLAYLIST = 'list';
/**
* Make a new thing that did not exist a playlist, a rule, a library.
*
* This is the only meaning `plus` keeps. It used to carry four.
*/
export const ICON_NEW = 'plus';
/**
* The request ("want") toggle, as an outline/solid pair.
*
* Two states of one control have to read as each other's opposite,
* which a plus and a bookmark do not. The pair was already in the app
* and already correct `explore-album-details`'s "Want this" button
* has used it since it was written, and `favorites-controller` uses the
* same shape for `regular/heart` `heart` while the badge forty
* pixels away showed a plus for the same state.
*
* That is `utils/library-status.ts`'s fault one layer down: it made the
* two surfaces agree on *what wanting means* and left them disagreeing
* on what it looks like.
*/
export const ICON_CAN_REQUEST = 'regular/bookmark';
export const ICON_REQUESTED = 'solid/bookmark';
/**
* You have this.
*
* Deliberately not drawn on the common case see the tracklist, where
* absence is what gets marked. This is for the places that answer the
* question directly, like the badge on a catalog card.
*/
export const ICON_IN_LIBRARY = 'check';
/**
* Something is being fetched right now.
*
* Distinct from `ICON_REQUESTED`: a request may sit on the list
* forever without anything happening, which is exactly why the badge's
* "queued" state stopped being an hourglass.
*/
export const ICON_DOWNLOADING = 'download';
/**
* Take this away.
*
* One icon for removing from a playlist, from the queue and from the
* library, because the difference that matters is stated in the words
* beside it and in the confirmation "Remove from Library" says in its
* impact line that the files are not deleted.
*/
export const ICON_REMOVE = 'trash';
+57
View File
@@ -1,7 +1,9 @@
import { completenessStore } from '@store/completeness-store';
import { downloadStore } from '@store/download-store';
import { libraryStore } from '@store/library-store';
import type * as download from '@go/download/models.js';
import type { LibraryStatus } from '../components/library-status-indicator/library-status-indicator';
import { isOwned, type Ownable } from './ownership';
/**
* What the tick/hourglass/plus badge should say about one entity.
@@ -46,6 +48,61 @@ export function libraryStatusFor(
return 'not-in-library';
}
/**
* Everything a badge needs about one entity, decided in one place.
*
* `status` is the state; `owned`/`expected` are the counts behind
* `partial` and are zero for every other state, which is what the badge
* requires it documents that a caller with no total must not pass a
* ring at 0%.
*/
export interface BadgeState {
status: LibraryStatus;
owned: number;
expected: number;
}
/**
* What the badge on an album card should say.
*
* Three rules, and the middle one is the whole point of this issue.
*
* **Ownership is the local album id**, per `utils/ownership.ts` a
* file, not the catalog's `inLibrary` ratchet.
*
* **A partly-held album says how partly.** The count comes from
* `completenessStore`, which batches a screenful into one query;
* reading it is what asks for it. Before this, an album held 2 tracks
* of 10 wore the same green tick as one held whole on every grid in
* the app.
*
* **A total that was never declared is not a total of zero.** Where
* `known` is false most of an untagged library, and every album until
* a rescan repopulates `audio_files.total_tracks` this is a plain
* `in-library` and says nothing, which is the rule the badge's own
* documentation states and the reason `Known` exists at all.
*/
export function albumBadgeFor(
album: Ownable | null | undefined,
mbid?: string | null,
): BadgeState {
if (!isOwned(album)) {
return { status: libraryStatusFor(false, mbid), owned: 0, expected: 0 };
}
const held = completenessStore.completeness(album?.localId);
if (held?.known && !held.complete) {
return {
status: 'partial',
owned: held.owned,
expected: held.expected,
};
}
return { status: 'in-library', owned: 0, expected: 0 };
}
/** What a badge can ask for. Artists are deliberately absent: a
* discography subscription is `explore-artist-details`'s Follow
* button, which can say what it is committing to. */
+134
View File
@@ -0,0 +1,134 @@
/**
* What "I do not own this" looks like, and how the app decides it.
*
* The rule the user asked for, in their words: *owned content is the
* default, normal, unadorned presentation; unowned content is what gets
* marked*. `explore-album-details` implemented it for one tracklist
* dimmed in place, `aria-disabled` because dimming is a colour and
* cannot be the only signal, and nothing at all drawn on the owned rows
* and every other catalog surface still mixed the two with a small
* badge as the only difference. This is that rule, written once, so
* eight surfaces cannot each keep their own version of it.
*
* ## Ownership is a file, and `localId` is the flag that says so
*
* The album page answers "do I own this row" with `filePaths`, a map
* from a displayed track to a real file. A card grid cannot afford a
* lookup per card and does not need one, because the answer is
* already on every model.
*
* `explore_index.local_artist_id` / `local_release_group_id` /
* `local_recording_id` are built by `collectLibraryEntities` from
* queries that every one join `audio_files`, and cleared by
* `pruneStaleLocalCrossReferences` whose existence test is a file test
* in all three cases. That is the same "ownership is a file" rule,
* computed once per scan instead of once per screenful.
*
* **`inLibrary` is the weaker one and is deliberately not consulted.**
* It is written by the same pass, so today the two agree but it is a
* one-way ratchet (`in_library = MAX(in_library, excluded.in_library)`)
* whose only clearing pass is gated on a non-null `local_*_id`, so it
* cannot be un-set on its own. One of the two is a fact with an owner;
* the other is a flag that happens to agree with it.
*
* The divergence was observable before this: both `explore-view` and
* `explore-artist-details` kept a `libraryMBIDs` set that accumulated
* every MBID ever seen with `inLibrary` and cleared it never, in a view
* that never unmounts. And on one artist-detail card the two answers
* were used side by side the context menu gated Play on
* `localId > 0` while the badge said "in your library" from
* `inLibrary`, so a card could claim to be owned, offer no Play, and
* (the request item being gated on *not* owned) offer no way to ask for
* it either.
*/
import { css } from 'lit';
/** Anything a card or row can be drawn from, as far as this is concerned. */
export interface Ownable {
/** The local row id behind this entity: an album, a file, an artist. */
localId?: number | null;
}
/**
* Whether there is something of the user's behind this entity.
*
* Deliberately narrow: a local id and nothing else. Passing the model
* straight in is the point a call site that has to remember which of
* two fields to read is a call site that will eventually read the other
* one, which is exactly how the two answers came to sit on one card.
*/
export function isOwned(entity: Ownable | null | undefined): boolean {
return (entity?.localId ?? 0) > 0;
}
/** The kinds of thing a catalog surface can draw. */
export type OwnableKind = 'album' | 'track' | 'artist';
/**
* The sentence an unowned thing says, once.
*
* It reaches whoever is not seeing the dimming, so it has to name the
* thing as well as the state "not in your library" alone, repeated
* down a grid, identifies nothing. The em dash matches the album
* tracklist's existing phrasing, which is where this came from.
*/
export function unownedLabel(name: string, kind: OwnableKind): string {
return `${name} — not in your library, ${
kind === 'artist' ? 'browsing the catalog' : 'available to request'
}`;
}
/**
* The accessible name for a card or row, owned or not.
*
* `activates` is what the thing does when it is yours: "Play", "Album",
* whatever the surface's own verb is. An unowned one does not get that
* verb, because it cannot do it.
*/
export function ownershipLabel(
owned: boolean,
activates: string,
name: string,
kind: OwnableKind,
): string {
return owned ? `${activates} ${name}` : unownedLabel(name, kind);
}
/**
* The dimming, shared so it cannot drift across surfaces.
*
* Two things about it are load-bearing.
*
* **The text dims to a token, not with `opacity`.** `theme-store`'s
* ramps are checked by `theme-contrast.test.ts` against every surface
* text can sit on; an opacity multiplier is outside that check and
* would quietly drop a dimmed title under 4.5:1 on the light ramps.
* Secondary rather than tertiary for the reason the album tracklist
* gives: these rows and cards have a `bgOverlay` hover background,
* which tertiary does not clear.
*
* **Only the artwork takes an `opacity`.** A cover is not text, so it
* is outside the contrast rule entirely, and it is the part of a card
* that carries the most weight dimming it is what makes a grid read
* as catalog at a glance rather than needing the badge to be found.
*/
export const unownedStyles = css`
.unowned .album-title,
.unowned .track-title,
.unowned .card-name,
.unowned .top-release-title,
.unowned .artist-name {
color: var(--yj-text-secondary, #b3b3b3);
font-weight: 400;
}
.unowned .album-art-container,
.unowned .top-release-art,
.unowned .track-art,
.unowned .card-image,
.unowned .card-image-placeholder,
.unowned .artist-avatar {
opacity: 0.55;
}
`;
@@ -0,0 +1,238 @@
/**
* Asking to see the whole album.
*
* The page could already draw the full release with the missing rows
* dimmed, and did so automatically once the tags said the album was
* incomplete. What it could not do was be *asked*: the rule depends on
* the files declaring a per-disc total, so where they declare none
* which is a great deal of any library a partly-owned album showed
* only the tracks on disk and nothing said the rest existed.
*
* The switch is the explicit route. Its rules are all one rule: a
* control that cannot change what is on screen is worse than no
* control, which is the same test the version dropdown answers.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import { page } from 'vitest/browser';
import type { LitElement } from 'lit';
import '@components/explore-album-details/explore-album-details';
import { stub, flush, resetHarness } from '@test/support/harness';
import { fixture, shadow, shadowAll } from '@test/support/render';
const MBID = 'rg-0001';
function track(n: number, owned = false) {
return {
position: n,
discNumber: 1,
title: `Track ${n}`,
length: 200000,
mbid: `rec-${n}`,
inLibrary: owned,
};
}
function release(mbid: string, date: string, trackCount: number, owned = 0) {
return {
mbid,
title: 'Glass Harbour',
date,
status: 'Official',
tracks: Array.from({ length: trackCount }, (_, i) =>
track(i + 1, i < owned),
),
};
}
/** Local files with no recording MBIDs an untagged rip, which is the
* case the automatic rule cannot see. */
function localTracks(count: number) {
return Array.from({ length: count }, (_, i) => ({
TrackName: `Track ${i + 1}`,
TrackNumber: i + 1,
DiscNumber: 1,
TrackLength: '210000',
RecordingMBID: '',
}));
}
const UNKNOWN = { owned: 0, expected: 0, known: false, complete: false };
async function albumWith(
releases: unknown[],
completeness: Record<string, unknown>,
local: unknown[] = [],
): Promise<LitElement> {
stub('explore.Service.BrowseReleases', releases);
stub('library.Library.GetAlbumCompleteness', completeness);
stub('library.Library.GetAlbumTracks', local);
const el = await fixture<LitElement>('explore-album-details', {
releaseGroupMBID: MBID,
localAlbumId: 7,
albumName: 'Glass Harbour',
});
await flush();
await el.updateComplete;
return el;
}
const scopeSwitch = (el: LitElement) => shadow(el, '.tracklist-scope wa-switch');
async function toggle(el: LitElement) {
const sw = scopeSwitch(el) as HTMLInputElement | null;
if (!sw) throw new Error('no tracklist scope switch on the page');
sw.checked = !sw.checked;
sw.dispatchEvent(new Event('change'));
await flush();
await el.updateComplete;
}
describe('the "show the whole album" switch', () => {
beforeEach(() => {
resetHarness();
stub('explore.Service.LookupReleaseGroup', {
mbid: MBID,
title: 'Glass Harbour',
artistCredit: 'Tideline',
});
stub('explore.Service.GetThumbnail', '');
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
stub('library.Library.GetFilePathsByRecordingMBIDs', {});
});
/**
* The report, exactly: two tracks on disk, twelve on the release,
* and nothing to say so because the tags declared no total.
*/
it('reveals the rest of the release when the total is unknown', async () => {
const el = await albumWith(
[release('rel-1', '2019-04-01', 12, 2)],
UNKNOWN,
localTracks(2),
);
expect(shadowAll(el, '.track-row')).toHaveLength(2);
await toggle(el);
const rows = shadowAll(el, '.track-row');
expect(rows).toHaveLength(12);
// Nothing resolves to a file, so every row is marked unowned —
// the dimming is the signal, and it is not this switch's job to
// invent ownership it cannot prove.
expect(rows.filter((r) => r.classList.contains('unowned'))).toHaveLength(12);
});
/** And back again — a switch that only goes one way is a button. */
it('goes back to the files on disk', async () => {
const el = await albumWith(
[release('rel-1', '2019-04-01', 12, 2)],
UNKNOWN,
localTracks(2),
);
await toggle(el);
expect(shadowAll(el, '.track-row')).toHaveLength(12);
await toggle(el);
expect(shadowAll(el, '.track-row')).toHaveLength(2);
});
/**
* The automatic rule still fires, and the control has to agree with
* the page it is sitting on rather than starting out contradicting
* it. This is what the tri-state is for.
*/
it('starts checked when the tags already said the album is short', async () => {
stub(
'library.Library.GetFilePathsByRecordingMBIDs',
Object.fromEntries(
Array.from({ length: 9 }, (_, i) => [
`rec-${i + 1}`,
[`/music/0${i + 1}.mp3`],
]),
),
);
const el = await albumWith(
[release('rel-1', '2019-04-01', 12, 9)],
{ owned: 9, expected: 12, known: true, complete: false },
localTracks(9),
);
expect(shadowAll(el, '.track-row')).toHaveLength(12);
expect((scopeSwitch(el) as HTMLInputElement).checked).toBe(true);
});
/** And the user outranks it: turning it off asks for the files. */
it('lets the automatic answer be overridden', async () => {
const el = await albumWith(
[release('rel-1', '2019-04-01', 12, 9)],
{ owned: 9, expected: 12, known: true, complete: false },
localTracks(9),
);
await toggle(el);
expect(shadowAll(el, '.track-row')).toHaveLength(9);
});
/**
* A control the accessibility tree cannot name is not a control, and
* this app has shipped that fault twice `wa-slider` pointed
* `aria-labelledby` at an empty internal label, and `config-field`
* rendered a `<label>` as a sibling with no `for`.
*
* `wa-switch` gets it right for a *different* reason than either:
* its `<input role="switch">` sits inside a native `<label>` that also
* holds the `<slot>`, so the name is computed across the flattened
* tree from light-DOM text. That is worth an assertion rather than an
* assumption and it has to be the browser's own answer, since
* querying shadow roots cannot compute a name.
*/
it('is named for anyone not looking at it', async () => {
await albumWith(
[release('rel-1', '2019-04-01', 12, 2)],
UNKNOWN,
localTracks(2),
);
await expect
.element(page.getByRole('switch', { name: 'Show the whole album' }))
.toBeInTheDocument();
});
describe('is absent where it could not change anything', () => {
it('when the album is entirely owned', async () => {
// Ten files, a ten-track release: the switch would redraw the
// same list, which reads as broken.
const el = await albumWith(
[release('rel-1', '2019-04-01', 10, 10)],
{ owned: 10, expected: 10, known: true, complete: true },
localTracks(10),
);
expect(scopeSwitch(el)).toBeNull();
});
it('when there is no catalog release to switch to', async () => {
const el = await albumWith([], UNKNOWN, localTracks(4));
expect(scopeSwitch(el)).toBeNull();
});
it('when the album is not in the library at all', async () => {
// Every entry here is already a catalog tracklist; there is no
// "only my tracks" to go back to.
const el = await albumWith([release('rel-1', '2019-04-01', 12)], UNKNOWN);
expect(scopeSwitch(el)).toBeNull();
});
});
});
@@ -0,0 +1,278 @@
/**
* Choosing a pressing is a repair job, not the album page's headline.
*
* The version selector sat directly above the tracklist with a heading,
* a `<select>` and a paragraph explaining how our clustering picks a
* "standard version" the most valuable space on the page spent on a
* control a normal user never touches (#17). Two more blocks shared
* that slot and were not even guarded by "is there a choice": a
* `Versions / Loading releases…` spinner about the same fetch
* `renderTracklist` was already reporting, and a `Versions / <error>`
* block duplicating what `catalog-scope-notice` shows at the top of the
* page with a retry.
*
* What is pinned here is the demotion and the three things that must
* survive it: the control is still reachable, the page still says which
* version you are looking at once you have chosen one, and a failed
* fetch still says so somewhere a collapsed panel is not.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import { page } from 'vitest/browser';
import '@components/explore-album-details/explore-album-details';
import { stub, stubFailure, flush, resetHarness } from '@test/support/harness';
import { fixture, shadow, shadowAll } from '@test/support/render';
const MBID = 'rg-0001';
function track(n: number) {
return {
position: n,
discNumber: 1,
title: `Track ${n}`,
length: 200000,
mbid: `rec-${n}`,
inLibrary: false,
};
}
function release(mbid: string, date: string, trackCount: number) {
return {
mbid,
title: 'Glass Harbour',
date,
status: 'Official',
tracks: Array.from({ length: trackCount }, (_, i) => track(i + 1)),
};
}
const UNKNOWN = { owned: 0, expected: 0, known: false, complete: false };
/** Two releases whose tracklists genuinely differ, so there is a choice. */
const TWO = [release('rel-1', '2019-04-01', 10), release('rel-2', '2020-09-01', 14)];
async function album(releases: unknown[] = TWO): Promise<LitElement> {
stub('explore.Service.BrowseReleases', releases);
stub('library.Library.GetAlbumCompleteness', UNKNOWN);
stub('library.Library.GetAlbumTracks', []);
const el = await fixture<LitElement>('explore-album-details', {
releaseGroupMBID: MBID,
localAlbumId: 7,
albumName: 'Glass Harbour',
});
await flush();
await el.updateComplete;
return el;
}
/** Positions of two selectors within the shadow root, in document order. */
function order(el: Element, first: string, second: string): [number, number] {
const all = [...(el.shadowRoot?.querySelectorAll('*') ?? [])];
const a = all.findIndex((n) => n.matches(first));
const b = all.findIndex((n) => n.matches(second));
return [a, b];
}
beforeEach(() => {
resetHarness();
stub('explore.Service.LookupReleaseGroup', {
mbid: MBID,
title: 'Glass Harbour',
artistCredit: 'Tideline',
});
stub('explore.Service.GetThumbnail', '');
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
stub('library.Library.GetFilePathsByRecordingMBIDs', {});
});
describe('the version selector is no longer the headline', () => {
it('renders after the tracklist, not before it', async () => {
const el = await album();
const [tracklist, versions] = order(el, '.tracklist', '.versions');
expect(tracklist).toBeGreaterThan(-1);
expect(versions).toBeGreaterThan(tracklist);
});
it('starts collapsed', async () => {
const el = await album();
expect(shadow(el, '#versions-body')?.hasAttribute('hidden')).toBe(true);
});
/**
* `aria-controls` has to name an element that is in the DOM, so the
* body renders unconditionally and is toggled with `hidden` the
* rule `config-section` states and the reason a conditional body
* would be wrong here too.
*/
it('keeps the panel in the DOM while it is shut', async () => {
const el = await album();
expect(shadow(el, '#versions-body')).not.toBeNull();
expect(
shadow(el, '.versions-toggle')?.getAttribute('aria-controls'),
).toBe('versions-body');
});
/**
* The browser's own answer: a disclosure that cannot be tabbed to is
* the fault `config-section` shipped for every setting in the app,
* and a shadow-root query cannot tell you a control has a name.
*/
it('is a named, expandable button', async () => {
await album();
await expect
.element(
page.getByRole('button', { name: /Other versions of this album \(2\)/ }),
)
.toBeInTheDocument();
});
it('opens when the button is pressed', async () => {
const el = await album();
const toggle = shadow<HTMLButtonElement>(el, '.versions-toggle');
expect(toggle?.getAttribute('aria-expanded')).toBe('false');
toggle?.click();
await el.updateComplete;
expect(toggle?.getAttribute('aria-expanded')).toBe('true');
expect(shadow(el, '#versions-body')?.hasAttribute('hidden')).toBe(false);
});
it('says nothing at all when there is only one tracklist', async () => {
const el = await album([release('rel-1', '2019-04-01', 10)]);
expect(shadow(el, '.versions')).toBeNull();
});
});
describe('the two blocks that shared that slot', () => {
/**
* The spinner was unguarded, so `Versions / Loading releases…` took
* the primary position on *every* album load including the ones
* that would never offer a choice beside `renderTracklist`'s own
* "Loading tracks…" about the same fetch.
*/
it('no longer reports the same fetch twice while loading', async () => {
stub('library.Library.GetAlbumCompleteness', UNKNOWN);
stub('library.Library.GetAlbumTracks', []);
stub('explore.Service.BrowseReleases', () => new Promise(() => {}));
const el = await fixture<LitElement>('explore-album-details', {
releaseGroupMBID: MBID,
albumName: 'Glass Harbour',
});
const loading = shadowAll(el, '.section-loading');
expect(loading).toHaveLength(1);
expect(loading[0]?.textContent).toContain('Loading tracks');
});
/**
* A failed browse must still be visible, and it cannot be visible
* from inside a collapsed disclosure. It belongs to the list that is
* missing because of it `renderTracklist` used to return `nothing`
* here and lean on the selector's own error block, which is exactly
* the coupling that made this a rewrite rather than a move.
*/
it('reports a failed fetch in the tracklist, once', async () => {
stub('library.Library.GetAlbumCompleteness', UNKNOWN);
stub('library.Library.GetAlbumTracks', []);
stubFailure('explore.Service.BrowseReleases', 'the catalog said no');
const el = await fixture<LitElement>('explore-album-details', {
releaseGroupMBID: MBID,
albumName: 'Glass Harbour',
});
await flush();
await el.updateComplete;
const errors = shadowAll(el, '.section-error');
expect(errors).toHaveLength(1);
expect(errors[0]?.textContent).toContain('versions');
expect(shadow(el, '.versions')).toBeNull();
});
});
describe('which version is on screen', () => {
/**
* The default is what the header already describes, so a line saying
* so on every album would be the thing this issue removed, one size
* smaller.
*/
it('is not stated while the page picked it', async () => {
const el = await album();
expect(shadow(el, '.chosen-version')).toBeNull();
});
/**
* The moment someone chooses another, the tracklist and the header
* disagree and the control that explains it is now off the bottom
* of the page.
*/
it('is stated above the tracklist once the user chooses', async () => {
const el = await album();
const select = shadow<HTMLSelectElement>(el, '#version-select')!;
const other = [...select.options].find((o) => o.value !== select.value)!;
select.value = other.value;
select.dispatchEvent(new Event('change'));
await el.updateComplete;
const line = shadow(el, '.chosen-version');
expect(line).not.toBeNull();
const [chosen, tracklist] = order(el, '.chosen-version', '.tracklist');
expect(chosen).toBeGreaterThan(-1);
expect(tracklist).toBeGreaterThan(chosen);
});
it('offers a way back, which clears the line', async () => {
const el = await album();
const select = shadow<HTMLSelectElement>(el, '#version-select')!;
const first = select.value;
const other = [...select.options].find((o) => o.value !== first)!;
select.value = other.value;
select.dispatchEvent(new Event('change'));
await el.updateComplete;
shadow<HTMLButtonElement>(el, '.chosen-version-reset')?.click();
await el.updateComplete;
expect(shadow<HTMLSelectElement>(el, '#version-select')?.value).toBe(first);
expect(shadow(el, '.chosen-version')).toBeNull();
});
/** A panel that shuts on use cannot be used twice. */
it('leaves the disclosure open after a choice', async () => {
const el = await album();
shadow<HTMLButtonElement>(el, '.versions-toggle')?.click();
await el.updateComplete;
const select = shadow<HTMLSelectElement>(el, '#version-select')!;
const other = [...select.options].find((o) => o.value !== select.value)!;
select.value = other.value;
select.dispatchEvent(new Event('change'));
await el.updateComplete;
expect(shadow(el, '#versions-body')?.hasAttribute('hidden')).toBe(false);
});
});
+19 -2
View File
@@ -19,6 +19,11 @@ import {
update,
visual,
} from '@test/support/render';
import {
ICON_CAN_REQUEST,
ICON_IN_LIBRARY,
ICON_REQUESTED,
} from '@utils/icon-language';
describe('<app-sidebar>', () => {
it('renders a testid per destination, which is how e2e navigates', async () => {
@@ -154,9 +159,20 @@ describe('<library-status-indicator>', () => {
it('defaults to "not in library"', async () => {
const el = await fixture('library-status-indicator');
expect(shadow(el, 'wa-icon')?.getAttribute('name')).toBe('plus');
expect(shadow(el, 'wa-icon')?.getAttribute('name')).toBe(ICON_CAN_REQUEST);
});
/**
* Named from the vocabulary rather than written out, or this test
* pins the glyphs *against* the table it is supposed to follow
* which is what it did: it asserted `plus` for the un-owned state,
* the same glyph two adjacent menu items were using for two other
* meanings, and passing was the reason nobody looked.
*
* What is still worth asserting is that the three differ, which is
* the property the states need and the one the table cannot state
* about itself here.
*/
it('uses a distinct glyph per state', async () => {
const glyphs: (string | null | undefined)[] = [];
@@ -166,7 +182,8 @@ describe('<library-status-indicator>', () => {
glyphs.push(shadow(el, 'wa-icon')?.getAttribute('name'));
}
expect(glyphs).toEqual(['check', 'bookmark', 'plus']);
expect(glyphs).toEqual([ICON_IN_LIBRARY, ICON_REQUESTED, ICON_CAN_REQUEST]);
expect(new Set(glyphs).size).toBe(3);
});
it('phrases its label around the entity it describes', async () => {
@@ -0,0 +1,140 @@
/**
* The icon vocabulary is one table, and nothing writes around it.
*
* A wrong-but-real icon name renders perfectly: no error, no fallback,
* no failing assertion anywhere. That is how `plus` came to mean "add
* to the queue", "add to a playlist", "make a new playlist" and "you do
* not own this" the first two adjacent in the same context menu
* while `list` meant the queue, the Playlists destination *and* adding
* to the queue.
*
* `src/icons/index.ts` catches a name that is not *bundled*. Nothing
* catches a name that is bundled and means something else, so this
* sweeps the source for the governed ones. It is the same shape as
* `TestNoDirectRuntimeEmits` and `TestNoWritesOnTheReadPool` in the
* backend, and exists for the same reason: the rule is about every call
* site, so checking one is checking nothing.
*/
import { describe, expect, it } from 'vitest';
import { bundledIconNames } from '../../src/icons';
import * as icons from '@utils/icon-language';
/** Every component source, as text. */
const SOURCES = import.meta.glob<string>('../../src/**/*.ts', {
eager: true,
query: '?raw',
import: 'default',
});
/**
* The names that carry a meaning the table owns.
*
* Deliberately not every bundled name. `check` is `ICON_IN_LIBRARY`
* here and also the "Copied" confirmation in `job-log-view`, which is
* a different, perfectly good meaning governing it would force a
* false rename. What belongs on this list is a name that was actually
* overloaded.
*/
const GOVERNED = [
'plus',
'list',
'bookmark',
'solid/bookmark',
'regular/bookmark',
'bars-staggered',
];
/** The one file allowed to say them, plus its own test. */
const DEFINITION = /icon-language\.(ts|test\.ts)$/;
describe('the icon vocabulary', () => {
/**
* A sweep over nothing passes. This is the assertion that makes the
* rest of the file mean something, and it is the first thing that
* breaks if the glob pattern stops matching after a move.
*/
it('actually reads the source', () => {
const paths = Object.keys(SOURCES);
expect(paths.length).toBeGreaterThan(100);
expect(paths.some((p) => p.endsWith('/track-list.ts'))).toBe(true);
expect(SOURCES[paths[0]!]).toContain('import');
});
it.each(GOVERNED)('is not written around for %s', (name) => {
const offenders: string[] = [];
for (const [path, source] of Object.entries(SOURCES)) {
if (DEFINITION.test(path)) continue;
// Both spellings: an icon in a template, and an icon name in a
// data table (which is how the sidebar and bottom-nav carry
// theirs).
const literal = new RegExp(
`(name="${name}"|icon: '${name}'|name=\\$\\{[^}]*'${name}')`,
);
if (literal.test(source)) offenders.push(path);
}
expect(offenders).toEqual([]);
});
/**
* A meaning with no icon behind it is the state the badge's `queued`
* spent a year in declared, styled, and produced by nothing.
*/
it('gives every meaning a name', () => {
const values = Object.entries(icons).filter(([k]) => k.startsWith('ICON_'));
expect(values.length).toBeGreaterThan(0);
for (const [key, value] of values) {
expect(`${key}=${value}`).toMatch(/^ICON_[A-Z_]+=[a-z]+[a-z/-]*$/);
}
});
/**
* Every name in the table is a name the app actually ships.
*
* This is the loop the vocabulary closes. A name that is not bundled
* renders a circled question mark and reports itself to
* `__yjIconMisses` at *runtime*, from a state something has to
* reach first. `bookmark-check` is Font Awesome **Pro**, and it was
* on `explore-artist-details`'s Follow button, drawn for every
* followed artist, invisible to `offline-icons.spec.ts` because no
* spec had ever followed one. Reaching the state is no longer how
* this is found.
*/
it('names only icons that are bundled', () => {
const bundled = new Set(bundledIconNames());
const missing = Object.entries(icons)
.filter(([k]) => k.startsWith('ICON_'))
.filter(([, v]) => !bundled.has(v as string))
.map(([k, v]) => `${k} (${v})`);
expect(missing).toEqual([]);
});
/**
* The two states of the request toggle have to be the same glyph in
* two weights, or they do not read as each other's opposite which
* is what a plus against a bookmark was.
*/
it('makes the request toggle an outline/solid pair', () => {
expect(icons.ICON_CAN_REQUEST).toBe(`regular/${icons.ICON_REQUESTED.replace('solid/', '')}`);
});
/**
* The queue and the Playlists destination wore the same icon, and
* "add to queue" and "add to playlist" sat next to each other wearing
* a third same one. Whatever the table says, these three have to
* differ from each other.
*/
it('keeps the queue, playlists and creating something apart', () => {
const three = [icons.ICON_QUEUE, icons.ICON_PLAYLIST, icons.ICON_NEW];
expect(new Set(three).size).toBe(3);
});
});
@@ -77,3 +77,30 @@ describe('the library status badge', () => {
expect(name).toContain('Glass Harbour');
});
});
/**
* A partly-held album is the one state that is *actionable and
* counted*: there are tracks left to ask for, so the badge is a button
* and a control is named after what activating it does, which is how
* the count came to be dropped from exactly the state the ring exists
* for. Both, or the ring says "some" to an eye and nothing to anyone
* else.
*/
describe('a partial badge that can act', () => {
it('names the action and keeps the count', async () => {
const el = await fixture('library-status-indicator', {
status: 'partial',
owned: 9,
expected: 12,
entityType: 'album',
label: 'Glass Harbour',
requestMbid: 'rg-1',
});
const name = shadow(el, '.badge')?.getAttribute('aria-label') ?? '';
expect(shadow(el, 'button.badge')).not.toBeNull();
expect(name).toContain('Request the rest of');
expect(name).toContain('9 of 12');
});
});
@@ -0,0 +1,363 @@
/**
* Owned is plain; unowned is what gets marked.
*
* `explore-album-details` had this right for one tracklist and nothing
* else did: Explore's cards, the top-results row and the artist page's
* three card shapes all mixed owned and unowned with a small badge as
* the only difference and drew a green tick on the *common* case,
* which is the treatment the album page's own green ticks were removed
* for.
*
* What is pinned here is the rule rather than any one surface, because
* the fault this replaced was eight call sites each holding their own
* version of it:
*
* - an owned thing draws **no badge at all**;
* - an unowned one is dimmed *and* says so in its accessible name,
* because dimming is a colour and cannot be the only signal;
* - ownership is a **file** (`localId`), never the catalog's
* `inLibrary` ratchet, which is a flag that happens to agree;
* - and a partly-held album says *how* partly, which is the one thing
* a tick cannot.
*/
import { beforeEach, describe, expect, it } from 'vitest';
import { page } from 'vitest/browser';
import '@components/explore-view/explore-view';
import '@components/top-results-row/top-results-row';
import { flush, stub, resetHarness } from '@test/support/harness';
import { fixture, shadow, shadowAll, update } from '@test/support/render';
import { completenessStore } from '@store/completeness-store';
const SEARCH = 'explore.Service.SearchLocal';
const SHELVES = 'explore.Service.GetExploreShelves';
const COMPLETENESS = 'library.Library.GetAlbumsCompleteness';
/** A release group as the backend projects one. */
function album(
title: string,
{ localId = 0, inLibrary = false }: { localId?: number; inLibrary?: boolean },
) {
return {
mbid: `rg-${title}`,
title,
artistCredit: 'An Artist',
artistMbid: 'ar-1',
primaryType: 'Album',
firstReleaseDate: '1994-05-01',
popularity: 100,
listenerCount: 10,
secondaryTypes: [],
inLibrary,
localId,
};
}
/** A recording as the backend projects one. */
function recording(
title: string,
{ localId = 0, inLibrary = false }: { localId?: number; inLibrary?: boolean },
) {
return {
mbid: `rec-${title}`,
title,
artistCredit: 'An Artist',
artistMbid: 'ar-1',
length: 200000,
popularity: 0,
listenerCount: 0,
inLibrary,
localId,
};
}
/** Mount Explore showing one page of results. */
async function exploreShowing(results: {
releaseGroups?: unknown[];
recordings?: unknown[];
artists?: unknown[];
}) {
stub(SHELVES, { shelves: [], state: 'ready' });
stub(SEARCH, {
artists: [],
releaseGroups: [],
recordings: [],
...results,
});
const el = await fixture('explore-view');
// A cached primary view only fetches on arrival, and the search is
// what these cards come from.
(el as unknown as { onViewActivate: () => void }).onViewActivate?.();
await update(el, { results: { artists: [], releaseGroups: [], recordings: [], ...results } });
await flush();
await el.updateComplete;
return el;
}
beforeEach(() => {
resetHarness();
stub(COMPLETENESS, {});
// The store is a singleton and caches an *answer*, including the
// absent one — which is the point, or 87% of a grid re-asks forever.
// Two tests in one file are two sessions as far as it is concerned,
// so a stale entry from the test above would otherwise decide the
// one below. Found by writing the assertion the wrong way round.
completenessStore.invalidate();
});
describe('an owned thing is plain', () => {
it('draws no badge on an album card it has files for', async () => {
const el = await exploreShowing({
releaseGroups: [album('Held', { localId: 7 })],
});
expect(shadowAll(el, '.album-card')).toHaveLength(1);
expect(shadow(el, '.album-card library-status-indicator')).toBeNull();
});
it('draws no badge on a track row it has a file for', async () => {
const el = await exploreShowing({
recordings: [recording('Held', { localId: 9 })],
});
expect(shadowAll(el, '.track-item')).toHaveLength(1);
expect(shadow(el, '.track-item library-status-indicator')).toBeNull();
});
it('does not dim it', async () => {
const el = await exploreShowing({
releaseGroups: [album('Held', { localId: 7 })],
});
expect(shadow(el, '.album-card')?.classList.contains('unowned')).toBe(
false,
);
});
});
describe('an unowned thing is marked', () => {
it('dims the card and keeps its request badge', async () => {
const el = await exploreShowing({
releaseGroups: [album('Absent', {})],
});
expect(shadow(el, '.album-card')?.classList.contains('unowned')).toBe(true);
expect(shadow(el, '.album-card library-status-indicator')).not.toBeNull();
});
/**
* The name is the half of this that reaches anyone not seeing the
* dimming, so it has to be the browser's own answer a shadow-root
* query cannot compute a name, and this repo has shipped a nameless
* control three times.
*/
it('says so in the name the browser computes', async () => {
await exploreShowing({ releaseGroups: [album('Absent', {})] });
await expect
.element(page.getByRole('button', { name: /Absent — not in your library/ }))
.toBeInTheDocument();
});
/**
* A track row is `aria-disabled` and a card is not, and the
* difference is not cosmetic: activating an unowned row does nothing
* (`onRecordingRowDblClick` returns early), while a card navigates to
* the catalog page for it, which is a perfectly good thing to do with
* something you do not own.
*/
it('marks a row that cannot be played as disabled', async () => {
const el = await exploreShowing({
recordings: [recording('Absent', {})],
});
expect(shadow(el, '.track-item')?.getAttribute('aria-disabled')).toBe(
'true',
);
});
it('leaves a card that still navigates enabled', async () => {
const el = await exploreShowing({
releaseGroups: [album('Absent', {})],
});
expect(shadow(el, '.album-card')?.getAttribute('aria-disabled')).toBeNull();
});
});
/**
* The decision this issue turned on.
*
* `inLibrary` is written by the same pass that writes the local ids, so
* the two agree in a healthy database but it is a one-way ratchet
* (`MAX(in_library, excluded.in_library)`) whose only clearing pass is
* gated on a non-null `local_*_id`, so it cannot be un-set on its own.
* A row carrying it with no local id behind it is a claim of ownership
* with no file, which is exactly what the album page refuses to trust.
*/
describe('ownership is a file, not a flag', () => {
it('treats a card flagged inLibrary with no local row as unowned', async () => {
const el = await exploreShowing({
releaseGroups: [album('Phantom', { inLibrary: true, localId: 0 })],
});
expect(shadow(el, '.album-card')?.classList.contains('unowned')).toBe(true);
expect(shadow(el, '.album-card library-status-indicator')).not.toBeNull();
});
it('does the same for a track row', async () => {
const el = await exploreShowing({
recordings: [recording('Phantom', { inLibrary: true, localId: 0 })],
});
expect(shadow(el, '.track-item')?.getAttribute('aria-disabled')).toBe(
'true',
);
});
});
/**
* The count, which is what `#16`'s deferred third step asked for: an
* album held 2 tracks of 10 wore the same green tick as one held whole,
* on every grid in the app.
*/
describe('a partly-held album says how partly', () => {
it('draws the ring and puts the count in the badge name', async () => {
stub(COMPLETENESS, {
'7': { owned: 9, expected: 12, known: true, complete: false },
});
const el = await exploreShowing({
releaseGroups: [album('Partly', { localId: 7 })],
});
// The store batches into the next frame, so the answer lands one
// repaint after the cards do — which is the thing the subscription
// exists for.
await new Promise((r) => requestAnimationFrame(() => r(null)));
await flush();
await el.updateComplete;
const badge = shadow(el, '.album-card library-status-indicator');
expect(badge?.getAttribute('status')).toBe('partial');
// A partly-held album is *actionable* — it has three tracks left to
// ask for — so the badge is a button, and the name has to carry the
// action and the count. Naming it after the action alone left the
// one state the ring exists for as the one state whose name did not
// mention it.
await expect
.element(
page.getByRole('button', {
name: /Request the rest of album .*Partly.* — 9 of 12 tracks/,
}),
)
.toBeInTheDocument();
});
/**
* Where the tags never declared a total, `known` is false and the
* card must say nothing most of an untagged library is in that
* state, and a ring drawn from its absence would mark all of it
* incomplete on no evidence. That is the rule `Known` exists for.
*/
it('says nothing when the total was never declared', async () => {
stub(COMPLETENESS, {
'7': { owned: 3, expected: 0, known: false, complete: false },
});
const el = await exploreShowing({
releaseGroups: [album('Untotalled', { localId: 7 })],
});
await new Promise((r) => requestAnimationFrame(() => r(null)));
await flush();
await el.updateComplete;
expect(shadow(el, '.album-card library-status-indicator')).toBeNull();
});
it('asks about the owned albums only, in one call', async () => {
const seen: unknown[][] = [];
stub(COMPLETENESS, (...args: unknown[]) => {
seen.push(args);
return {};
});
await exploreShowing({
releaseGroups: [
album('Held', { localId: 7 }),
album('Also held', { localId: 8 }),
album('Absent', {}),
album('Phantom', { inLibrary: true, localId: 0 }),
],
});
await new Promise((r) => requestAnimationFrame(() => r(null)));
await flush();
expect(seen).toHaveLength(1);
expect(seen[0]?.[0]).toEqual([7, 8]);
});
});
describe('the top-results row follows the same rule', () => {
const result = (
name: string,
entityType: string,
extra: Record<string, unknown> = {},
) => ({
entityType,
mbid: `top-${name}`,
name,
artistCredit: 'An Artist',
intentScore: 1,
inLibrary: false,
...extra,
});
it('draws no badge on something it owns', async () => {
const el = await fixture('top-results-row', {
results: [result('Held', 'release_group', { localId: 7 })],
query: 'held',
});
expect(shadow(el, '.card library-status-indicator')).toBeNull();
expect(shadow(el, '.card')?.classList.contains('unowned')).toBe(false);
});
it('dims and names something it does not', async () => {
const el = await fixture('top-results-row', {
results: [result('Absent', 'release_group')],
query: 'absent',
});
expect(shadow(el, '.card')?.classList.contains('unowned')).toBe(true);
await expect
.element(page.getByRole('button', { name: /Absent — not in your library/ }))
.toBeInTheDocument();
});
/**
* An artist card has never had a badge a discography subscription
* is the artist page's Follow button, which can say what it commits
* to so the dimming and the name are the whole signal there.
*/
it('marks an unowned artist without offering a request', async () => {
const el = await fixture('top-results-row', {
results: [result('An Artist', 'artist')],
query: 'an artist',
});
expect(shadow(el, '.card')?.classList.contains('unowned')).toBe(true);
expect(shadow(el, '.card library-status-indicator')).toBeNull();
});
});
+7 -2
View File
@@ -1,10 +1,15 @@
# YellowJacket Arch package
This directory holds the `PKGBUILD` and desktop entry used to build the
Arch Linux package. CI builds it on every push to `main` (see
Arch Linux package. CI builds it on every **release tag** (see
`.gitea/workflows/arch-package.yml`) and publishes it to the Gitea Arch
package registry, from which pacman can install it directly.
Tags come from `.gitea/workflows/release.yml`, which is run by hand — it
used to fire on every push to `main`, which meant a new package per
merged PR (issue #115). A prerelease tag (`v0.4.0-beta.1`) is skipped:
the workflow's trigger is `v*` and matches one.
## Installing from the registry
The registry is public — no login required.
@@ -58,7 +63,7 @@ this is not recommended.
- Only the runtime package is published; the `-debug` package makepkg
produces (detached symbols) is skipped by the workflow.
- Package versions come from `pkgver()` in the PKGBUILD, derived from git
(e.g. `1.3.0.r173.g4ae5ffc-1`), so every push to `main` yields a new
(e.g. `1.3.0.r173.g4ae5ffc-1`), so every release tag yields a new
version.
- Repo priority: if another configured repo ever provides a package named
`yellowjacket`, the repo listed **first** in `pacman.conf` wins. Force a