Compare commits

...
Author SHA1 Message Date
logan 5fae61cdf1 Merge pull request 'fix(loop): document the model fallback chain and foreground launches' (#244) from fix/243-model-fallback into main
CI / check (push) Successful in 3m26s
CI / e2e (push) Successful in 11m7s
default
2026-09-04 03:37:29 +00:00
logan 1f43234b80 fix(loop): document the model fallback chain and foreground launches
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m23s
CI / e2e (pull_request) Successful in 12m13s
Two #31-tick findings that would strand an unattended run. The qwen
worker hit its weekly 429 mid-tick; the obvious fallback
deepseek/deepseek-v4-pro is wrong because the deepseek provider has no
models (only catalog overrides) and fails silently — the model lives on
the go gateway as go/deepseek-v4-pro, with go/glm-5.3-flash the next
rung. And the async subagent runner has died without persisting a
session, so legs launch in the foreground and a dead worker is recovered
by completing, never re-implementing.

Closes #243
2026-09-03 23:20:33 -04:00
logan ca00f8a803 Merge pull request 'feat(library): play all and shuffle all on every track list' (#242) from feat/31-play-all-shuffle-all into main
CI / check (push) Successful in 3m12s
CI / e2e (push) Successful in 11m24s
default
2026-09-04 02:48:27 +00:00
logan 0be7b4fdc8 feat(library): play all and shuffle all on every track list
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m38s
CI / e2e (pull_request) Successful in 12m3s
Four pages that list tracks — Tracks, genre, artist, and both playlist
views — had no way to start the whole list, or had a broken one. One
shared helper (utils/play-all.ts) now owns what "shuffle this
collection" means: SetQueue's shuffleStart only picks a random first
track when shuffle mode is already on, it does not turn it on, so the
mode is toggled before the queue is set. Each host passes an honest
queue Source (#14): anything that builds a queue names what it built it
from, so "Playing from" stops lying.

Two behaviour changes ride along, both flagged: smart-playlist-details'
Shuffle was a live no-op (shuffleStart without enabling mode played
track 1 in order) and is fixed; playlist-details' Play all drops its
shuffleStart:true, so with shuffle mode already on it now starts at the
first row instead of a random one — the album page's existing
semantics.

Verified: make ui-test (1147, incl. a case that fails when the
smart-playlist fix is reverted), npx tsc --noEmit, make e2e (255,
incl. new play-all and header-fit specs), make lint, make test,
make bindings-check, make css-check; artist header read from
screenshots at 424/320/900 (the pair wraps below the name on a phone).

Closes #31
2026-09-03 17:06:55 -04:00
logan 18f10e966b Merge pull request 'fix(loop): worktree provisioning, fresh fetch, corruption halt' (#241) from fix/240-loop-operational-fixes into main
CI / check (push) Successful in 3m13s
CI / e2e (push) Successful in 10m44s
default
2026-09-03 18:17:02 +00:00
11 changed files with 923 additions and 36 deletions
+18
View File
@@ -81,6 +81,24 @@ rules. Never "go fix it" — the leg contract is in this file.
| escalation | `yj-loop.escalate` | go/kimi-k3 (T3) | same leg re-run, seeded with failure summary | | escalation | `yj-loop.escalate` | go/kimi-k3 (T3) | same leg re-run, seeded with failure summary |
| prose (PR body, commit msgs, journal) | `yj-loop.scribe` | go/mimo-v2.5 (T0) | text only, from supplied facts | | prose (PR body, commit msgs, journal) | `yj-loop.scribe` | go/mimo-v2.5 (T0) | text only, from supplied facts |
**Model fallback on quota exhaustion.** The pinned models are the
intent, not a guarantee. The qwen token plan is a weekly pool and has
run dry mid-tick (`429 … 1-week quota exhausted`). When a leg's launch
fails with a 429, re-run it with a per-run `model` override one rung
down and journal the substitution — never spend the T3 escalation
model on a quota substitution. The qwen-pinned legs (`work`,
`diffreview`) fall back `qwen/deepseek-v4-pro-0813``go/deepseek-v4-pro`
`go/glm-5.3-flash`. Do **not** use the `deepseek/...` provider: it has
no models, only catalog overrides, and fails silently (empty artifact,
no session) — the model lives on the `go` gateway.
**Launch legs in the foreground.** The async subagent runner has died
without persisting a child session (nothing to resume) and emits
spurious "needs attention" nudges on runs that are already complete.
Foreground `subagent` calls are the reliable mode here. A worker that
dies mid-leg leaves uncommitted work: inspect the tree, then relaunch
to *complete* — never to re-implement.
Orchestrator-only legs: **claim** (`issue.sh claim --branch` — atomic, Orchestrator-only legs: **claim** (`issue.sh claim --branch` — atomic,
refuses if held), **ship's PR/CI polling** (REST API below — `gitea_ci` refuses if held), **ship's PR/CI polling** (REST API below — `gitea_ci`
job_logs 404s on this Gitea; the REST endpoints are the way), **merge** job_logs 404s on this Gitea; the REST endpoints are the way), **merge**
+59 -4
View File
@@ -42,10 +42,10 @@ const ACTIONS = ['Import', 'New Playlist', 'New Smart Playlist'];
* because the number this issue is about (a button 48px wider than the * because the number this issue is about (a button 48px wider than the
* box holding it) is not in the accessibility tree at all. * box holding it) is not in the accessibility tree at all.
*/ */
const headerFit = (page: import('@playwright/test').Page) => const headerFit = (page: import('@playwright/test').Page, view = 'playlist-view') =>
page.evaluate(() => { page.evaluate((tag) => {
const root = document const root = document
.querySelector('[data-testid="main-content"] playlist-view') .querySelector(`[data-testid="main-content"] ${tag}`)
?.shadowRoot?.querySelector('page-header')?.shadowRoot; ?.shadowRoot?.querySelector('page-header')?.shadowRoot;
if (!root) return null; if (!root) return null;
@@ -76,7 +76,7 @@ const headerFit = (page: import('@playwright/test').Page) =>
...root.querySelectorAll('#page-header-overflow wa-dropdown-item'), ...root.querySelectorAll('#page-header-overflow wa-dropdown-item'),
].map((i) => i.textContent?.trim() ?? ''), ].map((i) => i.textContent?.trim() ?? ''),
}; };
}); }, view);
test.describe('the page header never clips an action', () => { test.describe('the page header never clips an action', () => {
test.beforeEach(async ({ app }) => { test.beforeEach(async ({ app }) => {
@@ -316,3 +316,58 @@ test.describe('the page header never clips an action', () => {
await expect.poll(async () => (await headerFit(app))?.menu).toEqual([]); await expect.poll(async () => (await headerFit(app))?.menu).toEqual([]);
}); });
}); });
/**
* The Tracks header carries the play-all/shuffle-all pair (#31), so
* the promise above has to hold for it too — the same per-button
* measurement, one view over. Its two actions are the whole of the
* header's declared set, and the pair is what plays the list the row
* is in, so a button rendered 20px of its 90px is a queue of nothing.
*/
const TRACK_ACTIONS = ['Play all', 'Shuffle all'];
test.describe('the Tracks header never clips an action', () => {
test.beforeEach(async ({ app }) => {
await app.getByTestId('nav-tracks').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'tracks',
);
});
test.afterEach(async ({ app }) => {
await app.setViewportSize({ width: 1280, height: 800 });
});
for (const vp of VIEWPORTS) {
test(`every action is reachable at ${vp.name}`, async ({ app }) => {
await app.setViewportSize({ width: vp.width, height: vp.height });
await expect
.poll(async () => (await headerFit(app, 'track-list'))?.clipped)
.toEqual([]);
const fit = (await headerFit(app, 'track-list'))!;
expect(fit.overflow).toBeLessThanOrEqual(0);
// Between them, buttons and menu account for both actions —
// not "it fits" but "nothing was dropped to make it fit".
expect([...fit.buttons, ...fit.menu].sort()).toEqual(
[...TRACK_ACTIONS].sort(),
);
});
}
/**
* The pair's names, through the accessibility tree — a shadow query
* measures, but it cannot say what a screen reader is offered.
*/
test('both actions are named controls', async ({ app }) => {
for (const label of TRACK_ACTIONS) {
await expect(
app.getByRole('button', { name: label, exact: true }),
).toBeVisible();
}
});
});
+220
View File
@@ -0,0 +1,220 @@
import { test, expect, callBinding, resetEvents, waitForEvent } from '../support/fixtures.js';
type Page = import('@playwright/test').Page;
/**
* Play-all/Shuffle-all, asserted on what the backend queued rather than
* on playback pixels.
*
* `SetQueue` reports the queue through `QueueChanged`, and `GetState`
* says exactly what it holds: the tracks in order, whether shuffle is
* on, and the `Source` the "Playing from" link is built from. That is
* the honest contract here — the buttons are only as good as the queue
* they build, and the queue is only as good as the source it names.
*/
interface QueueState {
tracks: { filePath: string; title: string }[];
currentIndex: number;
shuffleMode: boolean;
source: { type: string; id: number; label: string };
}
const TRACKS_SOURCE = { type: 'tracks', id: 0, label: 'All Tracks' };
const getQueue = (app: Page) =>
callBinding<QueueState>(app, 'queue.Queue.GetState');
/** The track paths a rendered track list shows, in row order. */
function displayedPaths(app: Page, scope: string): Promise<string[]> {
return app
.locator(`${scope} [data-testid="track-row"]`)
.evaluateAll((els) =>
els.map((el) => el.getAttribute('data-file-path') ?? ''),
);
}
/** Leave shuffle in a known state. The mode persists across specs in
* one backend process, so a test that asserts on it has to set it. */
async function setShuffleMode(app: Page, on: boolean): Promise<void> {
const state = await getQueue(app);
if (state.shuffleMode !== on) {
await resetEvents(app);
await callBinding(app, 'queue.Queue.ToggleShuffle');
await waitForEvent(app, 'QueueModeChanged');
}
}
test.describe('play-all/shuffle-all on the track list', () => {
test.beforeEach(async ({ app }) => {
await callBinding(app, 'queue.Queue.Clear').catch(() => {
/* the queue is clearable on every build these specs run against */
});
await setShuffleMode(app, false);
});
test('Tracks Play all queues the displayed list with an honest source', async ({
app,
}) => {
await app.getByTestId('nav-tracks').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'tracks',
);
await expect(
app.locator('track-list [data-testid="track-row"]').first(),
).toBeVisible();
const paths = await displayedPaths(app, 'track-list');
await resetEvents(app);
await app.getByTestId('page-action-play-all').click();
await waitForEvent(app, 'QueueChanged');
const state = await getQueue(app);
expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
expect(state.currentIndex).toBe(0);
expect(state.shuffleMode).toBe(false);
expect(state.source).toEqual(TRACKS_SOURCE);
});
test('Tracks Shuffle all turns shuffle on and keeps the source', async ({
app,
}) => {
await app.getByTestId('nav-tracks').click();
await expect(
app.locator('track-list [data-testid="track-row"]').first(),
).toBeVisible();
const paths = await displayedPaths(app, 'track-list');
await resetEvents(app);
await app.getByTestId('page-action-shuffle-all').click();
await waitForEvent(app, 'QueueChanged');
const state = await getQueue(app);
expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
expect(state.shuffleMode).toBe(true);
expect(state.source).toEqual(TRACKS_SOURCE);
});
});
test.describe('play-all on an embedded track list', () => {
test.beforeEach(async ({ app }) => {
await callBinding(app, 'queue.Queue.Clear').catch(() => {});
await setShuffleMode(app, false);
});
test('a genre page queues the genre with its name as the source', async ({
app,
}) => {
await app.getByTestId('nav-genres').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'genres',
);
const first = app.locator('genres-view .genre-card').first();
await expect(first).toBeVisible();
await first.click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'genre-details',
);
await expect(
app.locator('genre-details [data-testid="track-row"]').first(),
).toBeVisible();
const genreName = (await app
.locator('genre-details .genre-title')
.textContent())?.trim();
const paths = await displayedPaths(app, 'genre-details');
await resetEvents(app);
await app
.locator('genre-details [data-testid="page-action-play-all"]')
.click();
await waitForEvent(app, 'QueueChanged');
const state = await getQueue(app);
expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
expect(state.currentIndex).toBe(0);
expect(state.source).toEqual({ type: 'genre', id: 0, label: genreName });
});
});
test.describe('play-all on the library artist page', () => {
test.beforeEach(async ({ app }) => {
await callBinding(app, 'queue.Queue.Clear').catch(() => {});
await setShuffleMode(app, false);
});
test('an artist page queues album paths in album order with the artist source', async ({
app,
}) => {
const artists = await callBinding<{ ID: number; Name: string }[]>(
app,
'library.Library.GetArtists',
[0],
);
const first = artists[0]!;
await app.evaluate(
([id, name]) => {
document.dispatchEvent(
new CustomEvent('navigate', {
detail: {
view: 'artist-details',
artistId: id,
artistName: name,
},
bubbles: true,
composed: true,
}),
);
},
[first.ID, first.Name] as const,
);
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'artist-details',
);
await expect(app.getByTestId('artist-play-all')).toBeEnabled();
const albums = await callBinding<{ ID: number }[]>(
app,
'library.Library.GetAlbumsByArtist',
[first.Name, 0],
);
const byAlbum = await callBinding<Record<string, string[]>>(
app,
'library.Library.GetFilePathsByAlbums',
[albums.map((a) => a.ID), 0],
);
const expected: string[] = [];
for (const album of albums) {
expected.push(...(byAlbum[String(album.ID)] ?? []));
}
await resetEvents(app);
await app.getByTestId('artist-play-all').click();
await waitForEvent(app, 'QueueChanged');
const state = await getQueue(app);
expect(state.tracks.map((t) => t.filePath)).toEqual(expected);
expect(state.source).toEqual({
type: 'artist',
id: first.ID,
label: first.Name,
});
});
});
+1 -1
View File
@@ -137,7 +137,7 @@ test.describe('queue', () => {
}); });
test('shuffle and repeat toggles report their state', async ({ app }) => { test('shuffle and repeat toggles report their state', async ({ app }) => {
const shuffle = app.getByRole('button', { name: 'Shuffle' }); const shuffle = app.getByRole('button', { name: 'Shuffle', exact: true });
await resetEvents(app); await resetEvents(app);
await shuffle.click(); await shuffle.click();
@@ -11,11 +11,22 @@ import {
GetArtistImageCachedPath, GetArtistImageCachedPath,
GetArtistMBID, GetArtistMBID,
} from '@go/explore/service.js'; } from '@go/explore/service.js';
import { GetFilePathsByAlbums } from '@go/library/library.js';
import { libraryStore } from '@store/library-store';
import { notificationStore } from '@store/notification-store';
import { dict } from '@utils/binding';
import { playAll } from '@utils/play-all';
import { describeError } from '@utils/describe-error';
import { ICON_PLAY, ICON_SHUFFLE } from '@utils/icon-language';
import '@awesome.me/webawesome/dist/components/icon/icon.js'; import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@components/cover-grid/cover-grid.js'; import '@components/cover-grid/cover-grid.js';
import '../notifications/inline-notice';
import { designTokens } from '../../styles/tokens.css'; import { designTokens } from '../../styles/tokens.css';
import { backButton } from '../../styles/back-button.css'; import { backButton } from '../../styles/back-button.css';
/** The region the artist header's own failures are rendered in. */
const ArtistRegion = 'library-artist';
@customElement('artist-details') @customElement('artist-details')
export class ArtistDetails extends LitElement { export class ArtistDetails extends LitElement {
@property({ type: Number, attribute: 'artist-id' }) @property({ type: Number, attribute: 'artist-id' })
@@ -131,6 +142,39 @@ export class ArtistDetails extends LitElement {
); );
} }
.header-actions {
margin-left: auto;
display: flex;
align-items: center;
gap: 8px;
flex-shrink: 0;
}
.header-action {
background: none;
border: 1px solid var(--yj-border-subtle, #555);
border-radius: 4px;
color: var(--yj-text-primary, #fff);
padding: 6px 12px;
font-size: var(--yj-text-md, 13px);
font-family: inherit;
cursor: pointer;
display: flex;
align-items: center;
gap: 6px;
white-space: nowrap;
}
.header-action:hover {
border-color: var(--yj-accent, #ffd43b);
color: var(--yj-accent-text, #ffd43b);
}
.header-action:disabled {
opacity: 0.5;
cursor: default;
}
/* ==================================== /* ====================================
* Content * Content
* ==================================== */ * ==================================== */
@@ -145,6 +189,23 @@ export class ArtistDetails extends LitElement {
height: 100%; height: 100%;
} }
/* Phone widths: the header's flex row squeezed .artist-info to
* nothing, so the title ellipsised away entirely and the
* actions clipped against the host's own overflow — the album
* page's fault one detail view over (#66). The pair takes its
* own row instead. Written last, because a media query adds no
* specificity and a rule placed above the plain ones it
* overrides is silently dead. */
@media (max-width: 599px) {
.artist-header {
flex-wrap: wrap;
}
.header-actions {
flex-basis: 100%;
margin-left: 0;
}
}
`]; `];
override connectedCallback() { override connectedCallback() {
@@ -302,6 +363,45 @@ export class ArtistDetails extends LitElement {
return name.charAt(0).toUpperCase(); return name.charAt(0).toUpperCase();
} }
/**
* Play every track on this artist's albums, in album order.
*
* One `GetFilePathsByAlbums` call returns the paths grouped by
* album id; the caller owns the ordering, so they are flattened in
* `this.albums` order rather than by id.
*/
private async playAllTracks(shuffle: boolean): Promise<void> {
if (this.albums.length === 0) return;
try {
const libId = libraryStore.getSelectedLibraryId() ?? 0;
const ids = this.albums.map((a) => a.ID);
const byAlbum = await dict(
GetFilePathsByAlbums(ids, libId),
);
const paths: string[] = [];
for (const id of ids) {
paths.push(...(byAlbum[id] ?? []));
}
playAll(
paths,
{
type: 'artist',
id: this.artistId,
label: this.artistName,
},
shuffle,
);
} catch (error) {
console.error('Could not play artist:', error);
notificationStore.inline(ArtistRegion, {
text: describeError(error, 'Could not play this artists tracks.'),
});
}
}
/* ================================================================ /* ================================================================
* Rendering * Rendering
* ================================================================ */ * ================================================================ */
@@ -351,12 +451,38 @@ export class ArtistDetails extends LitElement {
` `
: ''} : ''}
</div> </div>
<div class="header-actions">
<button
class="header-action"
data-testid="artist-play-all"
?disabled=${this.albums.length === 0}
@click=${() =>
void this.playAllTracks(false)}
>
<wa-icon name=${ICON_PLAY}></wa-icon>
Play all
</button>
<button
class="header-action"
data-testid="artist-shuffle-all"
?disabled=${this.albums.length === 0}
@click=${() =>
void this.playAllTracks(true)}
>
<wa-icon name=${ICON_SHUFFLE}></wa-icon>
Shuffle all
</button>
</div>
</div> </div>
<div class="content"> <div class="content">
<cover-grid <cover-grid
.externalAlbums=${this.albums} .externalAlbums=${this.albums}
></cover-grid> ></cover-grid>
</div> </div>
<inline-notice
region=${ArtistRegion}
testid="artist-play-message"
></inline-notice>
`; `;
} }
} }
@@ -43,6 +43,7 @@ import type * as autotagservice from '@go/autotagservice/models.js';
import { confirmAction } from '../confirm-dialog/confirm-dialog'; import { confirmAction } from '../confirm-dialog/confirm-dialog';
import { queueStore } from '../../store/queue-store'; import { queueStore } from '../../store/queue-store';
import type { QueueSource } from '../../store/queue-store'; import type { QueueSource } from '../../store/queue-store';
import { playAll } from '@utils/play-all';
import { notificationStore } from '../../store/notification-store'; import { notificationStore } from '../../store/notification-store';
import '../notifications/inline-notice'; import '../notifications/inline-notice';
import { import {
@@ -2771,20 +2772,11 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
/** Play what the user owns of this release, optionally shuffled. */ /** Play what the user owns of this release, optionally shuffled. */
private playOwned(shuffle: boolean): void { private playOwned(shuffle: boolean): void {
const paths = this.ownedFilePaths();
// The button is only rendered when there is something to play, // The button is only rendered when there is something to play,
// so an empty set here is not a state the user can reach. // so an empty set here is not a state the user can reach. The
if (paths.length === 0) return; // shuffle-mode semantics live in `playAll`, shared with the
// play-all/shuffle-all pair on every track list.
// `shuffleStart` only picks a random first track when shuffle playAll(this.ownedFilePaths(), this.queueSource(), shuffle);
// mode is *already* on — it does not turn it on — so the mode
// has to be set before the queue, not after.
if (shuffle && !queueStore.getState().shuffleMode) {
queueStore.toggleShuffle();
}
queueStore.setQueue(paths, 0, shuffle, this.queueSource());
} }
/** Append what the user owns of this release to the queue. */ /** Append what the user owns of this release to the queue. */
@@ -85,7 +85,9 @@ import {
ICON_PLAYLIST, ICON_PLAYLIST,
ICON_QUEUE, ICON_QUEUE,
ICON_REMOVE, ICON_REMOVE,
ICON_SHUFFLE,
} from '@utils/icon-language'; } from '@utils/icon-language';
import { playAll } from '@utils/play-all';
/** One playlist row: the track and its position in the *playlist*, /** One playlist row: the track and its position in the *playlist*,
* which is not its position in the filtered view. */ * which is not its position in the filtered view. */
@@ -355,14 +357,32 @@ export class PlaylistDetails
// Track interactions // Track interactions
// ================================================================= // =================================================================
private handlePlayAll() { private playableFilePaths(): string[] {
const filePaths = this.tracks return this.tracks
.filter((t) => !t.Phantom) .filter((t) => !t.Phantom)
.map((t) => t.FilePath); .map((t) => t.FilePath);
}
if (filePaths.length === 0) return; private handlePlayAll() {
// Start at the first row, not at a random one: the old `true`
// was `shuffleStart`, which only picks a random first track
// when shuffle mode is already on — so "Play All" quietly did
// "play from the top" while leaving the mode as it was. The
// mode semantics now live in `playAll`, shared with the other
// track lists.
playAll(
this.playableFilePaths(),
{ type: 'playlist', id: this.playlistId, label: this.playlistName },
false,
);
}
queueStore.setQueue(filePaths, 0, true, { type: 'playlist', id: this.playlistId, label: this.playlistName }); private handleShuffleAll() {
playAll(
this.playableFilePaths(),
{ type: 'playlist', id: this.playlistId, label: this.playlistName },
true,
);
} }
private handleTrackClick( private handleTrackClick(
@@ -1599,9 +1619,16 @@ export class PlaylistDetails
class="play-all-button" class="play-all-button"
@click=${() => this.handlePlayAll()} @click=${() => this.handlePlayAll()}
> >
<wa-icon name="play"></wa-icon> <wa-icon name=${ICON_PLAY}></wa-icon>
Play All Play All
</button> </button>
<button
class="play-all-button"
@click=${() => this.handleShuffleAll()}
>
<wa-icon name=${ICON_SHUFFLE}></wa-icon>
Shuffle All
</button>
</div> </div>
<div class="track-header"> <div class="track-header">
<div class="header-cell col-number">#</div> <div class="header-cell col-number">#</div>
@@ -15,6 +15,7 @@ import {
import { EventsOn } from '@runtime/runtime'; import { EventsOn } from '@runtime/runtime';
import { Events } from '../../events'; import { Events } from '../../events';
import { queueStore } from '@store/queue-store'; import { queueStore } from '@store/queue-store';
import { playAll } from '@utils/play-all';
import { creditStore } from '@store/credit-store'; import { creditStore } from '@store/credit-store';
import { PlayerController } from '@store/controllers/player-controller'; import { PlayerController } from '@store/controllers/player-controller';
import { SearchController } from '@store/controllers/search-controller'; import { SearchController } from '@store/controllers/search-controller';
@@ -771,24 +772,29 @@ export class SmartPlaylistDetails
// Actions // Actions
// ================================================================= // =================================================================
private handlePlay() { private playableFilePaths(): string[] {
const filePaths = this.tracks return this.tracks
.filter((t) => !t.Phantom) .filter((t) => !t.Phantom)
.map((t) => t.FilePath); .map((t) => t.FilePath);
}
if (filePaths.length === 0) return; private handlePlay() {
playAll(
queueStore.setQueue(filePaths, 0, false, { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName }); this.playableFilePaths(),
{ type: 'smartPlaylist', id: this.playlistId, label: this.playlistName },
false,
);
} }
private handleShuffle() { private handleShuffle() {
const filePaths = this.tracks // This used to be a no-op when shuffle mode was off: it passed
.filter((t) => !t.Phantom) // `shuffleStart` without turning the mode on, so the queue
.map((t) => t.FilePath); // started at track 1 in order. `playAll` sets the mode first.
playAll(
if (filePaths.length === 0) return; this.playableFilePaths(),
{ type: 'smartPlaylist', id: this.playlistId, label: this.playlistName },
queueStore.setQueue(filePaths, 0, true, { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName }); true,
);
} }
private async handleRefresh() { private async handleRefresh() {
@@ -26,7 +26,10 @@ import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller
import { PlayerController } from '@store/controllers/player-controller'; import { PlayerController } from '@store/controllers/player-controller';
import { SearchController } from '@store/controllers/search-controller'; import { SearchController } from '@store/controllers/search-controller';
import '@components/page-header/page-header'; import '@components/page-header/page-header';
import type { SortOption } from '@components/page-header/page-header'; import type {
SortOption,
PageAction,
} from '@components/page-header/page-header';
import { TrackListController } from '@store/controllers/tracklist-controller'; import { TrackListController } from '@store/controllers/tracklist-controller';
import { FavoritesController } from '@store/controllers/favorites-controller'; import { FavoritesController } from '@store/controllers/favorites-controller';
import { queueStore } from '@store/queue-store'; import { queueStore } from '@store/queue-store';
@@ -87,7 +90,9 @@ import {
ICON_PLAYLIST, ICON_PLAYLIST,
ICON_PLAY_NEXT, ICON_PLAY_NEXT,
ICON_QUEUE, ICON_QUEUE,
ICON_SHUFFLE,
} from '@utils/icon-language'; } from '@utils/icon-language';
import { playAll } from '@utils/play-all';
const COLUMN_STORAGE_KEY = 'track-list-column-widths'; const COLUMN_STORAGE_KEY = 'track-list-column-widths';
const SORT_FIELD_KEY = 'track-list-sort-field'; const SORT_FIELD_KEY = 'track-list-sort-field';
@@ -2393,6 +2398,27 @@ export class TrackList
.map((c) => ({ id: c.id, label: c.label })), .map((c) => ({ id: c.id, label: c.label })),
]; ];
const hasTracks = this.cachedSortedTracks.length > 0;
const actions: PageAction[] = [
{
id: 'play-all',
label: 'Play all',
icon: ICON_PLAY,
priority: 1,
disabled: !hasTracks,
onSelect: this.handlePlayAll,
},
{
id: 'shuffle-all',
label: 'Shuffle all',
icon: ICON_SHUFFLE,
priority: 0,
disabled: !hasTracks,
onSelect: this.handleShuffleAll,
},
];
return html` return html`
<page-header <page-header
heading=${this.externalTracks === undefined ? 'Tracks' : ''} heading=${this.externalTracks === undefined ? 'Tracks' : ''}
@@ -2404,11 +2430,29 @@ export class TrackList
sort-field=${this.sortField ?? ''} sort-field=${this.sortField ?? ''}
sort-direction=${this.sortDirection} sort-direction=${this.sortDirection}
search-term=${this.searchCtrl.term} search-term=${this.searchCtrl.term}
.actions=${actions}
@sort-change=${this.onPageHeaderSort} @sort-change=${this.onPageHeaderSort}
></page-header> ></page-header>
`; `;
} }
/** The queue is the list as displayed, in the order the user sees. */
private handlePlayAll = (): void => {
playAll(
this.cachedSortedTracks.map((t) => t.FilePath),
this.effectiveQueueSource,
false,
);
};
private handleShuffleAll = (): void => {
playAll(
this.cachedSortedTracks.map((t) => t.FilePath),
this.effectiveQueueSource,
true,
);
};
private onPageHeaderSort = ( private onPageHeaderSort = (
e: CustomEvent<{ field: string; direction: 'asc' | 'desc' }>, e: CustomEvent<{ field: string; direction: 'asc' | 'desc' }>,
) => { ) => {
+26
View File
@@ -0,0 +1,26 @@
import { queueStore } from '@store/queue-store';
import type { QueueSource } from '@store/queue-store';
/**
* Queue a list and start it, optionally shuffled.
*
* This is the one place that owns what "shuffle this collection" means.
* `SetQueue`'s `shuffleStart` only picks a random first track when
* shuffle mode is *already* on — it does not turn it on — so the mode
* has to be set before the queue, not after. The album page used to
* carry that rule privately; the play-all/shuffle-all pair on every
* track list now shares it.
*/
export function playAll(
paths: string[],
source: QueueSource | undefined,
shuffle: boolean,
): void {
if (paths.length === 0) return;
if (shuffle && !queueStore.getState().shuffleMode) {
queueStore.toggleShuffle();
}
queueStore.setQueue(paths, 0, shuffle, source);
}
+373
View File
@@ -0,0 +1,373 @@
/**
* The play-all/shuffle-all pair on every page that lists tracks.
*
* The pair is driven by one helper (`utils/play-all`) that owns the
* one rule the album page already carried: `shuffleStart` does not turn
* shuffle on, it only picks a random first track once the mode is on —
* so the mode has to be set *before* the queue, not after. The hosts
* differ only in where their paths come from and what `Source` they
* hand over.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import '@components/track-list/track-list';
import '@components/artist-details/artist-details';
import '@components/playlist-details/playlist-details';
import '@components/smart-playlist-details/smart-playlist-details';
import {
stub,
flush,
resetHarness,
calls,
lastArgs,
emit,
} from '@test/support/harness';
import {
fixture,
shadowAll,
deepShadow,
} from '@test/support/render';
/** The action button rendered by `<page-header>`, through the nested
* shadow roots (track-list → page-header). */
function pageAction(
host: LitElement,
id: string,
): HTMLElement | null {
return deepShadow<HTMLElement>(host, `[data-testid="page-action-${id}"]`);
}
function setShuffleMode(on: boolean): void {
emit('QueueModeChanged', { shuffleMode: on, repeatMode: 'off' });
}
/** The queue's `SetQueue` args, with the shuffle flag and source. */
function queued(): {
paths: string[];
startIndex: number;
shuffleStart: boolean;
source: unknown;
} {
const args = lastArgs('queue.Queue.SetQueue');
if (!args) throw new Error('nothing was queued');
return {
paths: args[0] as string[],
startIndex: args[1] as number,
shuffleStart: args[2] as boolean,
source: args[3],
};
}
// =====================================================================
// The track list (Tracks, and every embedding detail view)
// =====================================================================
const PATHS = Array.from({ length: 12 }, (_, i) => `/music/track-${i}.mp3`);
const LIST = PATHS.map((FilePath, i) => ({
FilePath,
TrackName: `Track ${i}`,
ArtistName: 'An Artist',
Album: 'An Album',
Duration: 180,
}));
const GENRE_SOURCE = { type: 'genre', id: 0, label: 'Dream Pop' };
async function embeddedTrackList(): Promise<LitElement> {
resetHarness();
localStorage.removeItem('track-list-column-widths');
const el = await fixture<LitElement>('track-list', {
externalTracks: LIST,
queueSource: GENRE_SOURCE,
});
// Say which order is being asserted rather than inheriting a
// persisted sort. See play-in-context.test.ts for the same trap.
(el as unknown as { sortField: string | null }).sortField = null;
el.style.display = 'block';
el.style.height = '600px';
await flush();
await el.updateComplete;
await new Promise((r) => setTimeout(r, 60));
return el;
}
describe('the track-list play-all/shuffle-all pair', () => {
beforeEach(() => {
resetHarness();
localStorage.removeItem('track-list-column-widths');
});
it('renders in the primary Tracks header and is disabled while empty', async () => {
const el = await fixture<LitElement>('track-list', {});
const play = pageAction(el, 'play-all');
const shuffle = pageAction(el, 'shuffle-all');
expect(play).not.toBeNull();
expect(shuffle).not.toBeNull();
expect(play?.hasAttribute('disabled')).toBe(true);
expect(shuffle?.hasAttribute('disabled')).toBe(true);
});
it('renders in an embedded track-list header too', async () => {
const el = await embeddedTrackList();
expect(pageAction(el, 'play-all')).not.toBeNull();
expect(pageAction(el, 'shuffle-all')).not.toBeNull();
expect(pageAction(el, 'play-all')?.hasAttribute('disabled')).toBe(false);
expect(pageAction(el, 'shuffle-all')?.hasAttribute('disabled')).toBe(false);
});
it('Play all queues the displayed list in order, unshuffled', async () => {
const el = await embeddedTrackList();
pageAction(el, 'play-all')!.click();
await flush();
expect(queued()).toEqual({
paths: PATHS,
startIndex: 0,
shuffleStart: false,
source: GENRE_SOURCE,
});
});
it('Shuffle all turns shuffle on before queueing when the mode is off', async () => {
const el = await embeddedTrackList();
setShuffleMode(false);
await flush();
pageAction(el, 'shuffle-all')!.click();
await flush();
const all = calls();
const toggle = all.findLastIndex(
(c) => c.path === 'queue.Queue.ToggleShuffle',
);
const setQueue = all.findLastIndex(
(c) => c.path === 'queue.Queue.SetQueue',
);
expect(toggle).toBeGreaterThanOrEqual(0);
expect(toggle).toBeLessThan(setQueue);
expect(queued()).toEqual({
paths: PATHS,
startIndex: 0,
shuffleStart: true,
source: GENRE_SOURCE,
});
});
it('Shuffle all does not toggle when the mode is already on', async () => {
const el = await embeddedTrackList();
setShuffleMode(true);
await flush();
pageAction(el, 'shuffle-all')!.click();
await flush();
expect(calls('queue.Queue.ToggleShuffle')).toHaveLength(0);
expect(queued()).toEqual({
paths: PATHS,
startIndex: 0,
shuffleStart: true,
source: GENRE_SOURCE,
});
});
});
// =====================================================================
// The library artist page
// =====================================================================
const ALBUMS = [
{ ID: 3, Name: 'Third', ArtistName: 'Aurora Fields', ArtistMBID: '', MBID: '', CoverArtPath: '', CoverArtSmall: '', CoverArtMedium: '', CoverArtLarge: '', Year: 0, ReleaseYear: 0 },
{ ID: 1, Name: 'First', ArtistName: 'Aurora Fields', ArtistMBID: '', MBID: '', CoverArtPath: '', CoverArtSmall: '', CoverArtMedium: '', CoverArtLarge: '', Year: 0, ReleaseYear: 0 },
{ ID: 2, Name: 'Second', ArtistName: 'Aurora Fields', ArtistMBID: '', MBID: '', CoverArtPath: '', CoverArtSmall: '', CoverArtMedium: '', CoverArtLarge: '', Year: 0, ReleaseYear: 0 },
];
describe('the artist page play-all pair', () => {
beforeEach(() => {
resetHarness();
stub('explore.Service.GetArtistMBID', '');
stub('library.Library.GetAlbumsByArtist', ALBUMS);
stub('library.Library.GetFilePathsByAlbums', {
'3': ['/a3-1', '/a3-2'],
'1': ['/a1'],
'2': ['/a2-1', '/a2-2', '/a2-3'],
});
});
it('flattens album paths in the album list order', async () => {
const el = await fixture<LitElement>('artist-details', {
artistId: 7,
artistName: 'Aurora Fields',
artistMBID: '',
});
await flush();
await el.updateComplete;
const play = shadowAll<HTMLElement>(el, '[data-testid="artist-play-all"]')[0];
play!.click();
await flush();
expect(queued()).toEqual({
paths: ['/a3-1', '/a3-2', '/a1', '/a2-1', '/a2-2', '/a2-3'],
startIndex: 0,
shuffleStart: false,
source: { type: 'artist', id: 7, label: 'Aurora Fields' },
});
});
});
// =====================================================================
// A smart playlist
// =====================================================================
function smartPlaylistTracks(n: number) {
return Array.from({ length: n }, (_, i) => ({
ID: i + 1,
FilePath: `/music/track-${i}.mp3`,
Title: `Track ${i}`,
Artist: 'An Artist',
Album: 'An Album',
Duration: 180000,
CoverArtSmall: `/covers/${i}_sm.jpg`,
CoverArtMedium: `/covers/${i}_md.jpg`,
CoverArtPath: `/covers/${i}.jpg`,
Phantom: false,
}));
}
describe('the smart-playlist play-all pair', () => {
beforeEach(() => {
resetHarness();
stub('playlist.Service.GetSmartPlaylistTracks', smartPlaylistTracks(8));
stub('playlist.Service.GetSmartPlaylistRules', '{"rules":[]}');
stub('playlist.Service.GetAllPlaylists', []);
});
/** The details header's own action row, not a page-header action. */
function actionButton(el: LitElement, label: string): HTMLElement {
const button = shadowAll<HTMLElement>(el, '.action-button').find(
(b) => b.textContent?.trim() === label,
);
if (!button) throw new Error(`no "${label}" action button rendered`);
return button;
}
it('Shuffle turns the mode on before queueing, so the queue starts shuffled', async () => {
const el = await fixture<LitElement>('smart-playlist-details', {
playlistId: 1,
playlistName: 'A smart playlist',
});
el.style.display = 'block';
el.style.height = '600px';
await flush();
await el.updateComplete;
await new Promise((r) => setTimeout(r, 60));
setShuffleMode(false);
await flush();
actionButton(el, 'Shuffle').click();
await flush();
// The issue's headline fix. `SetQueue`'s `shuffleStart` only picks
// a random first track when shuffle mode is already on — it does
// not turn it on — so reverting the mode toggle puts the queue back
// to track 1 in order while every assertion about the queue's
// contents still passes. Order of the calls is the assertion.
const all = calls();
const toggle = all.findLastIndex(
(c) => c.path === 'queue.Queue.ToggleShuffle',
);
const setQueue = all.findLastIndex(
(c) => c.path === 'queue.Queue.SetQueue',
);
expect(toggle).toBeGreaterThanOrEqual(0);
expect(toggle).toBeLessThan(setQueue);
expect(queued()).toEqual({
paths: Array.from({ length: 8 }, (_, i) => `/music/track-${i}.mp3`),
startIndex: 0,
shuffleStart: true,
source: { type: 'smartPlaylist', id: 1, label: 'A smart playlist' },
});
});
});
// =====================================================================
// A regular playlist
// =====================================================================
function playlistTracks(n: number) {
return Array.from({ length: n }, (_, i) => ({
ID: i + 1,
FilePath: `/music/track-${i}.mp3`,
Title: `Track ${i}`,
Artist: 'An Artist',
Album: 'An Album',
Duration: 180000,
Phantom: false,
}));
}
describe('the playlist play-all pair', () => {
beforeEach(() => {
resetHarness();
stub('playlist.Service.GetPlaylistTracks', playlistTracks(8));
stub('playlist.Service.GetAllPlaylists', []);
});
it('offers Shuffle All beside Play All, both through the helper', async () => {
const el = await fixture<LitElement>('playlist-details', {
playlistId: 1,
playlistName: 'A playlist',
});
el.style.display = 'block';
el.style.height = '600px';
await flush();
await el.updateComplete;
await new Promise((r) => setTimeout(r, 60));
const buttons = shadowAll<HTMLElement>(el, '.play-all-button');
expect(buttons.map((b) => b.textContent?.trim())).toEqual([
'Play All',
'Shuffle All',
]);
setShuffleMode(false);
await flush();
buttons[1]!.click();
await flush();
expect(queued()).toEqual({
paths: Array.from({ length: 8 }, (_, i) => `/music/track-${i}.mp3`),
startIndex: 0,
shuffleStart: true,
source: { type: 'playlist', id: 1, label: 'A playlist' },
});
});
});