Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
aa59773d22 |
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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}
|
||||
></catalog-scope-notice>
|
||||
${this.renderVersionSelector()}
|
||||
${this.renderTracklistScope()}
|
||||
${this.renderTracklist()}
|
||||
</div>
|
||||
<track-details></track-details>
|
||||
@@ -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`
|
||||
<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.
|
||||
*
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user