diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 86e4dcd..e2687de 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -3506,3 +3506,31 @@ 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. diff --git a/CLAUDE.md b/CLAUDE.md index 7d861bc..0e603b7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1623,6 +1623,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 diff --git a/frontend/src/components/explore-album-details/explore-album-details.ts b/frontend/src/components/explore-album-details/explore-album-details.ts index f447e96..bb48aac 100644 --- a/frontend/src/components/explore-album-details/explore-album-details.ts +++ b/frontend/src/components/explore-album-details/explore-album-details.ts @@ -193,6 +193,25 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost { @state() private selectedVersionKey: string = ''; @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 @@ -559,6 +578,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; @@ -914,6 +946,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost { this.releases = []; this.versionEntries = []; this.selectedVersionKey = ''; + this.showFullTracklist = null; this.localTracks = []; this.filePaths = new Map(); this.askedFor = new Set(); @@ -1817,17 +1850,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 +1900,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 @@ -2238,6 +2296,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost { @catalog-retry=${this.retryCatalog} > ${this.renderVersionSelector()} + ${this.renderTracklistScope()} ${this.renderTracklist()} @@ -3036,6 +3095,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` +
+ + Show the whole album + + + ${showing + ? `${this.localTracks.length} of ${fullCount} tracks are in your library` + : `${fullCount - this.localTracks.length} more tracks are on this release`} + +
+ `; + } + + /** + * 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. * diff --git a/frontend/test/components/album-full-tracklist.test.ts b/frontend/test/components/album-full-tracklist.test.ts new file mode 100644 index 0000000..c7ffdee --- /dev/null +++ b/frontend/test/components/album-full-tracklist.test.ts @@ -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, + local: unknown[] = [], +): Promise { + stub('explore.Service.BrowseReleases', releases); + stub('library.Library.GetAlbumCompleteness', completeness); + stub('library.Library.GetAlbumTracks', local); + + const el = await fixture('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 `