Compare commits

..
Author SHA1 Message Date
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
logan e897364a73 fix(loop): worktree provisioning, fresh fetch, corruption halt
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m18s
CI / e2e (pull_request) Successful in 11m3s
Three P1 adoption-wave findings, all of which break an unattended run.
The worktree needs make build-frontend and make testdata once before its
first push — the pre-push go-test hook embeds frontend/dist and the
fixture library and refused the push without them. The merge leg now
says to fetch origin/main in the same breath as the refresh merge: a
cached origin/main merged against the wrong base, CI went green on it,
and the merge came back 405 "behind base" — a full cycle wasted. And a
killed fetch leaves 0-byte objects in the shared store whose signature
(error: object file … is empty / unpack-objects failed) must halt the
loop for a human instead of churning, with the repair recipe written
down.

Closes #240
2026-09-03 13:56:58 -04:00
logan 5fa68fcf43 Merge pull request 'ci(skill-check): scan the docs a contributor reads' (#229) from docs/220-skill-check-scope into main
CI / check (push) Successful in 3m6s
CI / e2e (push) Successful in 10m59s
default
2026-09-03 17:35:40 +00:00
logan b1368bbc7e Merge remote-tracking branch 'origin/main' into docs/220-skill-check-scope
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m10s
CI / e2e (pull_request) Successful in 11m6s
2026-09-03 13:20:23 -04:00
logan 9432f68c8b Merge remote-tracking branch 'origin/main' into docs/220-skill-check-scope
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m11s
CI / e2e (pull_request) Successful in 10m44s
2026-09-03 13:04:17 -04:00
logan 4c921ed1ba Merge pull request 'fix(config): put the old value back when a setter is rejected' (#233) from fix/231-setter-rollback into main
CI / check (push) Successful in 3m52s
CI / e2e (push) Successful in 11m13s
default
2026-09-03 16:48:05 +00:00
logan 26251badda ci(skill-check): scan the docs a contributor reads
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m0s
CI / e2e (pull_request) Successful in 10m18s
The check asserts that every make target named in a doc exists, and its
scanned set was .pi/ plus CLAUDE.md.  Since #50, CONTRIBUTING.md is the
document a *human* goes to for a build command, and it names 21 targets
that nothing verified; README.md names none today and is in for the same
reason.  The script's own header sentence is the argument — a renamed
target sends a person off the same cliff it sends an agent off.

The file list is now one `docs` variable used twice, because the failure
message carried a second copy of it and a second list is a second thing
to forget.  The `[ -d .pi ]` guard went with it: gating the whole run on
.pi/ would make the human-facing half conditional on the agent-facing
one, and an empty list is the same "nothing to scan" exit without the
coupling.

The lefthook glob is that scanned set now rather than
{Makefile,.pi/**/*.md} — #220's smaller half, and it did not fire on
CLAUDE.md either, which the script had read for months.

Verified by planting a bad target rather than by reading the diff: both
matched forms in each of the four scanned surfaces, each naming the
right file; the same two plants pass on the pre-change script; unfenced
prose still does not match; and the hook fires on a staged
CONTRIBUTING.md under the new glob where the old one skipped it.  The
count is unchanged at 47 — the set is a union — so coverage is the only
thing that moved.

Closes #220
2026-08-28 03:38:28 -04:00
16 changed files with 975 additions and 50 deletions
+37 -4
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 |
| 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,
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**
@@ -168,8 +186,12 @@ Merge when, and only when, **all** hold:
`block_on_outdated_branch: true` refuses it anyway; never
`force_manually_merged` around it.
**Refresh before every merge.** In the loop worktree: fetch, then
`git merge origin/main` on the PR branch, push. A textual conflict
**Refresh before every merge.** In the loop worktree: `git fetch origin`
in the same breath, then `git merge origin/main` on the PR branch,
push. The fetch must be immediate — a cached `origin/main` merges
against the wrong base, CI goes green on it, and the merge comes back
405 "behind base", one whole CI cycle wasted (measured on the adoption
wave). A textual conflict
stops the leg there — as diff text, not as a failed merge click: hunks
the loop authored are resolved by the loop; anything else is left with
`⟦loop⟧` comment for a human, never forced. After any refresh push,
@@ -247,7 +269,10 @@ AVD), then `make android-emulator` per session.
- **Worktree:** `git worktree add ~/.paseo/worktrees/loop/jumpy-hound
origin/main` (from any clone; branch from origin/main in the loop
tree, never `git checkout main`).
tree, never `git checkout main`). **Provision it once before the
first push:** `make build-frontend` and `make testdata` — the pre-push
`go-test` hook needs `frontend/dist` (the `//go:embed` in `main.go`)
and the fixture library, and refuses the push without them.
- **Session:** pi in that worktree, `/name loop`. Add the job via
`/schedule-prompt` (name `yj-loop`, cron
`0 0 10-18 * * 1-5`, prompt: "Read `.pi/skills/yj-loop/SKILL.md` and
@@ -270,4 +295,12 @@ AVD), then `make android-emulator` per session.
which leg it really is.
- The job did not fire — the scheduler fires only while a session is
open in its directory (documented); "the loop is off" is the correct
reading, not a bug.
reading, not a bug.
- `error: object file … is empty` / `unpack-objects failed` / `bad
object refs/heads/…` during a fetch or checkout — the shared object
store was corrupted (a killed fetch leaves 0-byte object files, and a
local ref can end up pointing at the dead sha1). **Halt and report**;
do not retry, the churn only deepens it. Human repair: delete the
0-byte objects, `git fetch origin --prune`, delete any ref that
still dangles (`git update-ref -d refs/heads/<b>`), re-checkout the
worktree at `origin/main`, then `git fsck --full`.
@@ -112,7 +112,9 @@ in the job's directory — that limitation is the switch:
- **Worktree:** `git worktree add` a dedicated clone at
`~/.paseo/worktrees/loop/jumpy-hound`. Loop edits happen only there; a
dirty tree there is the loop's business and nobody else's.
dirty tree there is the loop's business and nobody else's. **Provision
it once before its first push:** `make build-frontend` + `make testdata`
— the pre-push `go-test` hook needs both and refuses without them.
- **Session:** pi in that worktree, `/name loop`. The job is bound to that
session, so another pi elsewhere in the same directory does not
double-fire it.
+1 -1
View File
@@ -172,7 +172,7 @@ make ui-test # Vitest component/store suite in a real browser (no app)
make ui-visual # Same, including toMatchScreenshot comparisons
make ui-setup # Install the Vitest provider's own Chromium (once)
make bindings-check # Fail if frontend/bindings is stale vs the Go bindings
make skill-check # Fail if .pi/ documents a make target that doesn't exist
make skill-check # Fail if a doc names a make target that doesn't exist
make commit-check # Fail if a commit subject is not a Conventional Commit
make lint # golangci-lint v2 (strict), all three build configurations
make test # All tests with race detector, all three build configurations
+1 -1
View File
@@ -192,7 +192,7 @@ css-check: ## Fail on a css`` literal ended early by a backtick, or a nested rul
# Every command in them is a make target on purpose, so this is
# checkable. It also asserts AGENTS.md is a symlink to CLAUDE.md, so the
# two harnesses cannot drift onto two descriptions of one project.
skill-check: ## Fail if the agent docs name a missing make target, or AGENTS.md is not a symlink
skill-check: ## Fail if the docs name a missing make target, or AGENTS.md is not a symlink
@./scripts/skill-check.sh
# Conventional Commits, which CLAUDE.md claimed CI enforced for a long
+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
* box holding it) is not in the accessibility tree at all.
*/
const headerFit = (page: import('@playwright/test').Page) =>
page.evaluate(() => {
const headerFit = (page: import('@playwright/test').Page, view = 'playlist-view') =>
page.evaluate((tag) => {
const root = document
.querySelector('[data-testid="main-content"] playlist-view')
.querySelector(`[data-testid="main-content"] ${tag}`)
?.shadowRoot?.querySelector('page-header')?.shadowRoot;
if (!root) return null;
@@ -76,7 +76,7 @@ const headerFit = (page: import('@playwright/test').Page) =>
...root.querySelectorAll('#page-header-overflow wa-dropdown-item'),
].map((i) => i.textContent?.trim() ?? ''),
};
});
}, view);
test.describe('the page header never clips an action', () => {
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([]);
});
});
/**
* 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 }) => {
const shuffle = app.getByRole('button', { name: 'Shuffle' });
const shuffle = app.getByRole('button', { name: 'Shuffle', exact: true });
await resetEvents(app);
await shuffle.click();
@@ -11,11 +11,22 @@ import {
GetArtistImageCachedPath,
GetArtistMBID,
} 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 '@components/cover-grid/cover-grid.js';
import '../notifications/inline-notice';
import { designTokens } from '../../styles/tokens.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')
export class ArtistDetails extends LitElement {
@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
* ==================================== */
@@ -145,6 +189,23 @@ export class ArtistDetails extends LitElement {
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() {
@@ -302,6 +363,45 @@ export class ArtistDetails extends LitElement {
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
* ================================================================ */
@@ -351,12 +451,38 @@ export class ArtistDetails extends LitElement {
`
: ''}
</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 class="content">
<cover-grid
.externalAlbums=${this.albums}
></cover-grid>
</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 { queueStore } from '../../store/queue-store';
import type { QueueSource } from '../../store/queue-store';
import { playAll } from '@utils/play-all';
import { notificationStore } from '../../store/notification-store';
import '../notifications/inline-notice';
import {
@@ -2771,20 +2772,11 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
/** Play what the user owns of this release, optionally shuffled. */
private playOwned(shuffle: boolean): void {
const paths = this.ownedFilePaths();
// The button is only rendered when there is something to play,
// so an empty set here is not a state the user can reach.
if (paths.length === 0) return;
// `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.
if (shuffle && !queueStore.getState().shuffleMode) {
queueStore.toggleShuffle();
}
queueStore.setQueue(paths, 0, shuffle, this.queueSource());
// so an empty set here is not a state the user can reach. The
// shuffle-mode semantics live in `playAll`, shared with the
// play-all/shuffle-all pair on every track list.
playAll(this.ownedFilePaths(), this.queueSource(), shuffle);
}
/** Append what the user owns of this release to the queue. */
@@ -85,7 +85,9 @@ import {
ICON_PLAYLIST,
ICON_QUEUE,
ICON_REMOVE,
ICON_SHUFFLE,
} from '@utils/icon-language';
import { playAll } from '@utils/play-all';
/** One playlist row: the track and its position in the *playlist*,
* which is not its position in the filtered view. */
@@ -355,14 +357,32 @@ export class PlaylistDetails
// Track interactions
// =================================================================
private handlePlayAll() {
const filePaths = this.tracks
private playableFilePaths(): string[] {
return this.tracks
.filter((t) => !t.Phantom)
.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(
@@ -1599,9 +1619,16 @@ export class PlaylistDetails
class="play-all-button"
@click=${() => this.handlePlayAll()}
>
<wa-icon name="play"></wa-icon>
<wa-icon name=${ICON_PLAY}></wa-icon>
Play All
</button>
<button
class="play-all-button"
@click=${() => this.handleShuffleAll()}
>
<wa-icon name=${ICON_SHUFFLE}></wa-icon>
Shuffle All
</button>
</div>
<div class="track-header">
<div class="header-cell col-number">#</div>
@@ -15,6 +15,7 @@ import {
import { EventsOn } from '@runtime/runtime';
import { Events } from '../../events';
import { queueStore } from '@store/queue-store';
import { playAll } from '@utils/play-all';
import { creditStore } from '@store/credit-store';
import { PlayerController } from '@store/controllers/player-controller';
import { SearchController } from '@store/controllers/search-controller';
@@ -771,24 +772,29 @@ export class SmartPlaylistDetails
// Actions
// =================================================================
private handlePlay() {
const filePaths = this.tracks
private playableFilePaths(): string[] {
return this.tracks
.filter((t) => !t.Phantom)
.map((t) => t.FilePath);
}
if (filePaths.length === 0) return;
queueStore.setQueue(filePaths, 0, false, { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName });
private handlePlay() {
playAll(
this.playableFilePaths(),
{ type: 'smartPlaylist', id: this.playlistId, label: this.playlistName },
false,
);
}
private handleShuffle() {
const filePaths = this.tracks
.filter((t) => !t.Phantom)
.map((t) => t.FilePath);
if (filePaths.length === 0) return;
queueStore.setQueue(filePaths, 0, true, { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName });
// This used to be a no-op when shuffle mode was off: it passed
// `shuffleStart` without turning the mode on, so the queue
// started at track 1 in order. `playAll` sets the mode first.
playAll(
this.playableFilePaths(),
{ type: 'smartPlaylist', id: this.playlistId, label: this.playlistName },
true,
);
}
private async handleRefresh() {
@@ -26,7 +26,10 @@ import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller
import { PlayerController } from '@store/controllers/player-controller';
import { SearchController } from '@store/controllers/search-controller';
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 { FavoritesController } from '@store/controllers/favorites-controller';
import { queueStore } from '@store/queue-store';
@@ -87,7 +90,9 @@ import {
ICON_PLAYLIST,
ICON_PLAY_NEXT,
ICON_QUEUE,
ICON_SHUFFLE,
} from '@utils/icon-language';
import { playAll } from '@utils/play-all';
const COLUMN_STORAGE_KEY = 'track-list-column-widths';
const SORT_FIELD_KEY = 'track-list-sort-field';
@@ -2393,6 +2398,27 @@ export class TrackList
.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`
<page-header
heading=${this.externalTracks === undefined ? 'Tracks' : ''}
@@ -2404,11 +2430,29 @@ export class TrackList
sort-field=${this.sortField ?? ''}
sort-direction=${this.sortDirection}
search-term=${this.searchCtrl.term}
.actions=${actions}
@sort-change=${this.onPageHeaderSort}
></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 = (
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' },
});
});
});
+7 -3
View File
@@ -38,10 +38,14 @@ pre-commit:
glob: "*.go"
run: ./scripts/bindings-check.sh
# .pi/ documents make targets; a stale one sends an agent off a
# cliff with total confidence. Instant.
# The docs document make targets; a stale one sends an agent — or a
# contributor reading CONTRIBUTING.md — off a cliff with total
# confidence. The glob is the script's own scanned set, because a
# hook that does not fire on a file the check reads is the drift the
# check exists to prevent: it was `{Makefile,.pi/**/*.md}` while the
# script already read CLAUDE.md. Instant.
skill-check:
glob: "{Makefile,.pi/**/*.md}"
glob: "{Makefile,.pi/**/*.md,AGENTS.md,CLAUDE.md,README.md,CONTRIBUTING.md}"
run: ./scripts/skill-check.sh
frontend-typecheck:
+21 -4
View File
@@ -14,6 +14,11 @@
# missing: CLAUDE.md names 27 targets and nothing verified one of them,
# so the file the agents trust most was the file least checked.
#
# README.md and CONTRIBUTING.md are in it too, and the header sentence
# above is why: a person who has *not* read the Makefile goes looking in
# the contributor-facing doc, so a renamed target sends them off the
# same cliff it sends an agent off. CONTRIBUTING.md names 21 targets.
#
# **AGENTS.md is a symlink to CLAUDE.md.** This repo is worked on by
# two agent harnesses that read different files by convention — Claude
# Code reads CLAUDE.md, others read AGENTS.md — and two harnesses
@@ -43,7 +48,19 @@ if [ -e AGENTS.md ] || [ -L AGENTS.md ]; then
fi
fi
[ -d .pi ] || exit 0
# The scan is over the docs that are actually there: a checkout without
# .pi/ still has README.md and CONTRIBUTING.md to check, and gating the
# whole run on .pi/ would have made the human-facing half conditional on
# the agent-facing one. This list is used twice — once to read the
# mentions out and once to say which file a missing target came from —
# because a second list is a second thing to forget.
# `ls` exits non-zero when *any* of its arguments is missing while still
# printing the ones that are there, and under `set -e` that would sink
# the assignment rather than scanning what exists, so swallow it.
docs="$({ find .pi -name '*.md' 2>/dev/null
ls CLAUDE.md README.md CONTRIBUTING.md 2>/dev/null || true; })"
[ -n "$docs" ] || exit 0
# `make -pq` prints the database including every rule, without running
# anything. It exits non-zero when a target is out of date, and under
@@ -68,7 +85,7 @@ targets="$({ make -pqRr 2>/dev/null || true; } |
# AGENTS.md is deliberately not in this list: it is a symlink to
# CLAUDE.md, asserted above, so scanning it would report every failure
# twice under two names.
mentioned="$({ find .pi -name '*.md' 2>/dev/null; echo CLAUDE.md; } |
mentioned="$(printf '%s\n' "$docs" |
xargs awk '
FNR == 1 { fence = 0 }
/^```/ { fence = !fence; next }
@@ -93,10 +110,10 @@ for t in $mentioned; do
done
if [ -n "$missing" ]; then
echo "skill-check: the agent docs name make targets that do not exist:" >&2
echo "skill-check: the docs name make targets that do not exist:" >&2
for t in $missing; do
echo " make $t" >&2
grep -rln "make $t" .pi CLAUDE.md --include='*.md' | sed 's/^/ /' >&2
printf '%s\n' "$docs" | xargs grep -ln "make $t" | sed 's/^/ /' >&2
done
echo "Fix the docs, or restore the target." >&2
exit 1