Compare commits

..
Author SHA1 Message Date
yonluandClaude Opus 5.5 399dcc05b3 docs(notes): record slskd's API as read from its source
CI / check (push) Skipped
CI / e2e (push) Skipped
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
2026-09-26 22:11:30 -04:00
yonluandClaude Opus 5.5 ef89707bb1 feat(download): fill in the tracks a nearly complete album is missing
A grab that delivers nine of twelve tracks clears the completeness floor
and is imported, and the other three were never looked for. On Soulseek
that is the commonest way an album ends up almost right: one peer's
folder lacks a track, or one file fails.

After a successful import the manager now compares the tracks the files
were aligned to (ImportResult.Matched, new) with the expected tracklist.
When one to three are missing, and fewer than half, it searches for each
one on its own, as the track's artist and title with the album kept for
ranking. It grabs the first auto-acceptable copy that is not from the
source that already failed to supply it, and is not a delegate, which
would place it in its own library. The candidate is trimmed to the one
file aligned to the track. The import uses the album's own request with
ImportOptions.Only, which skips the completeness check and imports only
a file aligned to the missing track, so it is tagged and placed as part
of the album, and anything else the folder brought is left out.

One attempt per track, and nothing here fails the download: the album
is already imported, so a track that cannot be found is logged on the
job and left. slskd accepts one-file folders for a request that expects
one track, which the per-track search needs.

Closes #276

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
2026-09-26 22:11:03 -04:00
yonluandClaude Opus 5.5 c88fc1c7c7 feat(download): judge a queued slskd peer by its queue position
No bytes for ten minutes usually means a peer has queued us, and the
stall timer could not tell position 2 from position 400: the first was
abandoned while it was about to start, the second was waited on for ten
minutes for nothing.

While a requested file is "Queued, Remotely", the grab asks slskd for
its place (GET .../downloads/{user}/{id}/position, which asks the peer)
once a minute. A place that improved counts as progress and restarts
the stall clock. A place beyond 50 twice running gives the peer up at
once, and the manager moves to the next copy; two readings because
slskd documents the figure as possibly inaccurate. Waiting in a queue
has an overall ceiling of an hour without a byte, since a queue moving
one place an hour would otherwise hold the grab all day. As with a
stall, files that already arrived still go forward.

Closes #275

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
2026-09-26 22:07:18 -04:00
yonluandClaude Opus 5.5 7a9dd69d30 fix(download): own folder per slskd grab, search timeout in seconds
Checked against slskd 0.26.0's source rather than a live daemon, which
#267 never had.

slskd reads a search's searchTimeout in seconds, counted from the last
response. We sent milliseconds, telling it a search may idle for five
hours, so a search never completed on its own. It now sends seconds,
with slskd's floor of 5.

slskd writes a finished file to <downloads>/<remote leaf folder>/, and
when a name is taken it writes name_<ticks>.ext beside it. collect found
files by name there, so a file left by an earlier failed attempt, or by
the user's own download, was collected in place of this grab's. A user
who changed slskd's destination setting got nothing collected at all.

slskd 0.26 takes a batch download with an explicit destination. Each
grab now enqueues batches into yellowjacket/<uuid>/ (one per disc, since
a batch's files land flat), collects from exactly there, and removes the
folder afterwards, including after a failure. An older daemon answers
the batch route with 400, which is remembered, and the per-user enqueue
is used. There, collect skips files that were already present,
unchanged, before the enqueue, and takes the renamed copy slskd wrote
instead. The comparison is against a snapshot, not a clock, because
slskd may run on another machine. The per-folder lock from #272 is kept
only while batches are not known to work.

The test stub now writes files when they are enqueued, as slskd does,
including the rename, so tests no longer stage files before a grab.
An opt-in TestSlskdLive runs against a real daemon when YJ_SLSKD_URL,
YJ_SLSKD_API_KEY and YJ_SLSKD_DOWNLOADS are set.

Closes #274

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
2026-09-26 22:04:50 -04:00
yonlu 3f23bb4396 Merge pull request 'Batch: explore catalog + UI, Go 1.26, Soulseek downloads, CI fixes' (#273) from batch/258-272 into main
CI / check (push) Successful in 4m8s
CI / e2e (push) Successful in 14m37s
Reviewed-on: #273
2026-09-27 01:45:18 +00:00
yonlu af2ff17342 Merge remote-tracking branch 'origin/fix/263-slskd-transfer-lifecycle' into batch/258-272
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 4m53s
CI / e2e (pull_request) Successful in 13m34s
2026-09-26 20:58:54 -04:00
yonlu fc19ca54b7 Merge remote-tracking branch 'origin/build/265-go-1.26' into batch/258-272 2026-09-26 20:58:54 -04:00
yonlu 613901847a Merge remote-tracking branch 'origin/feat/264-explore-cards' into batch/258-272 2026-09-26 20:58:54 -04:00
yonlu ecf0109331 Merge remote-tracking branch 'origin/fix/258-artifact-blob-cursor' into batch/258-272 2026-09-26 20:58:53 -04:00
yonluandClaude Opus 5.5 792c2d9fbc build: require Go 1.26 everywhere at once
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 8m21s
CI / e2e (pull_request) Successful in 15m56s
The newest go-json-experiment/json, which wails/v3's application
package imports, declares go 1.26, so taking it raises our go
directive with it.  Every other place that names a Go version moves in
the same commit: GO_VERSION in ci, desktop-assets and android-apk, the
Arch makedepends, and CONTRIBUTING's table.

index-artifact's container image moves too, and it is the one that
matters most.  The official golang images set GOTOOLCHAIN=local, so a
golang:1.25 container refuses a go 1.26 module outright rather than
fetching a newer toolchain, and that job owns the ~205 GB checkpoint.

Closes #265

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
2026-09-26 17:00:47 -04:00
yonluandClaude Opus 5.5 bb26d5f289 feat(explore): arrows on every sideways row, and one album card size
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 4m25s
CI / e2e (pull_request) Successful in 13m43s
The shelves, the top results and the artist page's discography and
similar-artists rows scrolled sideways only by a horizontal wheel or a
trackpad, so a plain mouse could not reach anything past the fold.
<scroll-row> wraps each of them with previous/next arrows that are
hidden at the end they cannot move from, revealed on hover where there
is hover, and always shown where there is not.

The album card is defined once, in albumCardStyles, instead of twice.
explore-view clamped its cards to 130-150px, and with square artwork
that made cards in one row different heights as well as widths.  The
width is fixed now and each line under the art reserves its own space.
Covers are inset rather than cropped (contain, not cover).

The ownership badge sits over the artwork for owned and unowned alike,
and catalog cards no longer dim: a grid of dimmed covers read as a page
that had failed to load.  The album page's tracklist still dims unowned
rows, which says something different about something different.

Every MusicBrainz link now goes through openMusicBrainz, which pins the
origin, including the one on the album page that still called
window.open directly.  solid/chevron-left is bundled for the left arrow;
without it the arrow drew the missing-icon fallback.

Closes #264

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
2026-09-26 16:59:28 -04:00
yonluandClaude Opus 5.5 7b42b9ce56 feat(explore): sample the one-album-owned shelf instead of ranking it
"More from artists you own one album by" drew its artists and their
albums most-popular first, so it was a second leaderboard: the same
handful of big names every time the page opened, which is not what the
shelf is saying.  The pool is still bounded, but which artists and
which albums fill it is RANDOM(), so each visit is a different sample.
The test asserts the set rather than the order.

Refs #264

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
2026-09-26 16:58:57 -04:00
yonlu 924097b246 Merge branch 'docs/256-claude-md-split' into fix/258-artifact-blob-cursor
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m57s
CI / e2e (pull_request) Successful in 14m49s
Both branches rewrite CLAUDE.md. Mine appended a note about #258 to the
"the catalog stores ids as bytes" section; theirs cuts the file from
4,046 lines to 376 and deletes those write-ups, on the grounds that each
one already exists as a comment beside the code it describes.

Resolved in theirs. Nothing is lost by it: the refactor already states
both halves of the note as broad engineering rules, and cites #258 —
"Encodings are converted at one boundary … a Go `string` cursor against
a BLOB column made the catalog merge loop forever while merging nothing"
and "A loop that must make progress checks that it did … a merge that
lands fewer rows than it read". The detail stays in `artifactKey`'s doc
comment and in `publishedartifact_test.go`'s, which is where the new
file says this class belongs.
2026-09-26 15:56:44 -04:00
yonluandClaude Opus 5.5 1997276def docs: cut CLAUDE.md to the rules it is for
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 4m22s
CI / e2e (pull_request) Successful in 14m24s
CLAUDE.md had grown to 4,046 lines, ~3,300 of them per-component
write-ups: why a breakpoint is 500px, why a cap is a quarter, what a
spec once missed. Every session loaded all of it, and the rules that
apply to every change were buried among decisions that apply to one.

Those write-ups were already duplicated as comments beside the code
they describe -- every issue number they cite also appears in a code
comment -- so they are deleted rather than moved. What is left is the
tracker workflow, the commands, the verification tiers and a set of
broad engineering rules distilled from them, each pointing at where
its example lives. Three declined decisions recorded nowhere else go
to NOTES.md, and three code comments that cited CLAUDE.md sections by
name no longer do.

Closes #256

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-26 15:47:13 -04:00
yonlu 5d9c677cf7 ci(index-artifact): import the exported artifact before publishing it
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m59s
CI / e2e (pull_request) Successful in 12m51s
Nothing should be published until the code that imports it on a user's
machine has imported it here. The exporter and the importer are two
descriptions of one storage format, and every other tier tests the
importer against a *fixture* rather than against the file being shipped —
a second description free to be wrong in the same direction as the code
reading it.

That is how #258 reached everyone: the importer positioned its batch walk
with a Go `string` cursor against this file's 16-byte `mbid` column, and
SQLite neither coerces between TEXT and BLOB nor complains about the
comparison, so the walk merged no rows and never advanced. The fixture
guarding that walk writes the old text encoding, and the only compact
fixture is one row, below the batch size, so the bound query never ran.
Both were green throughout, and no install could finish a first index
build.

`TestImportPublishedArtifact` takes the published file and runs the
client's own path over it — checksum, decompress, merge — and asserts
that what the artifact holds is what the client ends up with: the row
count, the rows carrying a listen count, an FTS index in step with the
table, and one row read back through the app's own MBID conversions. It
skips without `YJ_CORE_INDEX_ARTIFACT`, so an ordinary run pays nothing.

It needs no Wails, which is why the step runs it under the indexbuild
tag: that container has no GTK. Measured on the current artifact, 64.8 MB
compressed: 39 seconds including the decompress.

Verified against the pre-fix comparison behaviour, the step goes red in
about 3 seconds — the strictly-advancing guard fails the import with a
named reason rather than the day-long spin it used to produce.

Refs #258
2026-09-25 11:03:52 -04:00
yonlu 1e3a490c12 fix(explore): refuse a catalog merge that does not land every row
The walk's predicates partition the artifact's key space, so a merge that
ends with fewer rows than the artifact declares does not mean the artifact
was smaller than it said — it means a predicate filtered rows out, and
the catalog is quietly partial while reporting complete.

Equality rather than a lower bound: RowsAffected counts an upsert that
changes nothing, and a row already merged locally is counted again here.

One reachable case, so this is not merely a tripwire. A row whose mbid is
empty is excluded by `mbid > ?` in both encodings, so an artifact
carrying one imports as a success with a row missing — which is the shape
#258 had, one cause over. The test covers exactly that artifact.

Refs #258
2026-09-25 11:03:33 -04:00
yonlu d4ea14ca5c fix(explore): merge the catalog artifact in its own mbid encoding
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 4m4s
CI / e2e (pull_request) Successful in 14m14s
The prebuilt catalog never merged. `mergeArtifactRows` positions itself
with `WHERE mbid > ? ORDER BY mbid LIMIT 1 OFFSET ?` against the
attached artifact, and it bound that cursor as a Go `string` while
`cmd/indexexport` publishes `explore_index.mbid` as 16 raw bytes — the
storage change that took the table from 677 MB to 389 MB.

SQLite does not coerce between TEXT and BLOB and orders every blob after
every text value, so against a byte column the predicate was not wrong
but unconditional: `mbid > <text>` matched the whole artifact, so the
bound the walk looked up was the same row every time and the cursor
never advanced, and `mbid <= <text>` matched nothing, so no batch
merged. No error, no rows, no state change — a fresh install sat at "0
of 1,077,893 rows" burning a core indefinitely, which is what it did
here for a day, while Explore showed only the rows the library scan and
the lazy artist enrichment had produced and popularity for none of the
catalog.

The cursor is now an `artifactKey`, typed to the encoding
`artifactStoresText` reports for the file it is attached to, so the
comparison is made in the same type as the column it is made against.
Two things guard the class rather than the instance: a nil key binds as
an empty value instead of SQL NULL, because `mbid > NULL` agrees with
nothing and would import nothing just as silently; and the walk returns
an error when its bound does not strictly advance, because the failure
here is silence and the next one should be a failed job with a reason.

It was never caught because the fixture that guards the walk writes the
old text encoding, and the only compact fixture is a single row — below
`artifactMergeBatch`, so the bound query never ran at all. The walk is
now covered on both encodings, across several batch boundaries.

Closes #258
2026-09-25 10:18:51 -04:00
yonlu 62c1a95ead test(e2e): give the job specs their own state back
Both specs stage a job through `/__test/emit` and neither cleared it.
Nothing resets those stores, so the spec that staged it is the one that
should put it back, and `JobsChanged` with `[]` is the whole cleanup --
`JobStore` replaces its list from every snapshot, so `testctl` needs no
special case.

**The leak as reported did not reproduce, and that is worth recording
rather than quietly fixing.**  Measured with a temporary probe: a
positive control confirmed a staged job really does move the shell at a
phone width (`job-band` renders a row, `.main-panel`'s top goes 0 to
55), and the very next page had no job at all.  The reason is that
every test gets a fresh page and `JobStore.init()` refetches `GetJobs()`
from a backend registry `/__test/emit` never writes to -- it calls
`events.Deliver`, which touches frontends and no state.  So the state
cannot cross a spec boundary as described, and the 55px offset the
draft assertion saw in that suite run has another cause that is not in
evidence.

The cleanup stays, because it costs a line and the leak would need only
one spec that keeps a page alive, and the comments say what was
measured rather than asserting the mechanism.

The durable half is the rule, now in the harness reference: measure
against the element next to you, not an absolute coordinate.  An
absolute number in a shell measurement is also a claim about everything
above it -- `contentTop === 0` asserts "and no background job is
running", which that spec could not arrange.

Closes #168
2026-09-23 07:51:07 -04:00
yonlu e67462ab53 ci: lint every commit a PR would merge, not just its tip
Gitea leaves `github.event.before` empty on a `pull_request`, so the
Commit messages step fell through to bare `make commit-check`, which
lints `git log -1` -- the tip alone.  Every other commit the branch
would bring was first examined by *main's* post-merge run, so a green
PR stopped being true after the merge, and it happened twice: PR #245
merged a 75-char subject its own CI never saw.

The PR's base is the stand-in.  `base.sha..head` lints the PR's own
commits because base advances on main, so the commits the branch shares
with it stay reachable from it and drop out of the range.

Both payload fields are handed to the shell rather than chosen in an
expression: `github.event.issue.number` in unclaim.yml is this repo's
proof that payload fields resolve, and `github.event` is the webhook
body unmarshalled into a map, so `pull_request.base.sha` comes from
Gitea's own `PRBranchInfo.Sha`.  The shell then falls back to today's
behaviour for a dispatch run, an all-zeros push, or a base commit the
clone does not have -- so the worst case is the fix not taking effect
rather than a broken job.

Verified locally against the report's own evidence: at 68e7edb8 the old
invocation passes ("HEAD is well-formed") while the range catches
a3b5b437 at 75 chars, which is what main's post-merge run did.  All
four event shapes were exercised against the new snippet.  The
end-to-end proof is the next PR with an over-length commit that is not
its tip.

Closes #254
2026-09-23 07:50:59 -04:00
yonlu e5dc54d0ec ci(skill-check): find a make target inside a hard-wrapped span
`scripts/skill-check.sh` matched one regex against one line, so a
mention the file hard-wraps -- `` `make `` at the end of one line and
the target at the start of the next -- was invisible to it.  These docs
are mostly hard-wrapped prose, so the wrap is what the author does not
think about, and `CONTRIBUTING.md:80` is already that shape.

Lines are now joined while the inline span is still open, which an odd
number of backticks means.  The fence and line-start halves are
untouched: a fenced command is already whole, and joining inside one
would break the rule that made this awk rather than a grep.  Joining is
bounded three ways -- a fence, a blank line (CommonMark allows no blank
line inside a code span) and a file boundary -- so a stray backtick
costs one paragraph of over-matching rather than the rest of the file.

The reporting loop needed the other half of the same fix: it named the
offending file with `grep -ln "make $t"`, which cannot see a wrapped
mention either, so a target the new parser found reported no file at
all and `set -o pipefail` turned the empty grep into exit 123 before
the line telling the author what to do.  It falls back to the bare
name.

Verified by planting the report's own wrapped `make
no-such-wrapped-target` into `CONTRIBUTING.md`: the old script reports
48 targets and exits 0, the new one names the target and the file and
exits 1.  Plant removed afterwards.

Closes #228
2026-09-23 07:50:51 -04:00
49 changed files with 4230 additions and 4813 deletions
+1 -1
View File
@@ -68,7 +68,7 @@ jobs:
SHA: ${{ github.sha }}
REF_NAME: ${{ github.ref_name }}
DEBIAN_FRONTEND: noninteractive
GO_VERSION: '1.25.0'
GO_VERSION: '1.26.0'
npm_config_store_dir: /cache/pnpm-store
# The Go half wants the NDK; the Gradle half wants a platform.
ANDROID_HOME: /cache/android-sdk
+23 -6
View File
@@ -36,7 +36,7 @@ concurrency:
cancel-in-progress: true
env:
GO_VERSION: '1.25.0'
GO_VERSION: '1.26.0'
# Shared by all three Playwright consumers (@playwright/cli, e2e/'s
# @playwright/test, frontend/'s Vitest provider). See the browsers
# step in job 2 for why that is not the whole story.
@@ -53,7 +53,7 @@ jobs:
check:
runs-on: ubuntu-latest
container:
# Not golang:1.25 — this job runs `make ui-test`, which is Vitest
# Not golang:1.26 — this job runs `make ui-test`, which is Vitest
# *browser* mode and needs a Chromium and its system libraries
# anyway, so the "fast job needs no browser" split does not hold.
# Not the Playwright image either: e2e/ pins @playwright/test
@@ -107,15 +107,32 @@ jobs:
# Conventional Commits. `.releaserc.yml` has always derived the
# version from the commit type; until now nothing checked that the
# type was one it recognises, so a malformed subject silently meant
# "no release". BEFORE is the push's previous tip and is absent or
# all-zeros for a new branch, in which case only the tip is linted.
# "no release".
#
# **On a `pull_request` there is no `before`.** Gitea leaves
# `github.event.before` empty for one, so this step fell through to
# bare `make commit-check`, which lints `git log -1` — the tip
# alone. Every other commit the branch would bring was first
# examined by *main's* post-merge run, which is a green PR that
# stops being true after the merge, and which happened twice (#254).
# The PR's base is the stand-in: the range below already excludes
# what the base shares with the branch, because base advances on
# main and those commits stay reachable from it.
#
# Both are handed to the shell rather than chosen in an expression:
# `github.event.issue.number` in unclaim.yml is this repo's proof
# that payload fields resolve, and the shell then falls back to
# today's behaviour for a dispatch run or a missing field instead of
# depending on how `&&`/`||` treat an absent context.
- name: Commit messages
working-directory: /src
env:
BEFORE: ${{ github.event.before }}
PR_BASE: ${{ github.event.pull_request.base.sha }}
PUSH_BEFORE: ${{ github.event.before }}
run: |
set -eu
if [ -n "${BEFORE:-}" ] && [ "${BEFORE#0000000}" = "$BEFORE" ] \
BEFORE="${PR_BASE:-${PUSH_BEFORE:-}}"
if [ -n "$BEFORE" ] && [ "${BEFORE#0000000}" = "$BEFORE" ] \
&& git cat-file -e "$BEFORE^{commit}" 2>/dev/null; then
make commit-check RANGE="$BEFORE..$SHA"
else
+1 -1
View File
@@ -48,7 +48,7 @@ jobs:
SHA: ${{ github.sha }}
REF_NAME: ${{ github.ref_name }}
DEBIAN_FRONTEND: noninteractive
GO_VERSION: '1.25.0'
GO_VERSION: '1.26.0'
npm_config_store_dir: /cache/pnpm-store
steps:
# The same set ci.yml's check job installs: the app is cgo, and
+44 -1
View File
@@ -68,7 +68,7 @@ jobs:
# claim with a test behind it now (cmd/indexbuild/deps_test.go),
# because the v3 migration quietly broke it and this job was where
# that surfaced.
image: golang:1.25
image: golang:1.26
# This host path must exist on the runner and be listed verbatim in
# act_runner's container.valid_volumes. It holds explore-staging/
# (counts.bin + state.json) and yj.db — the checkpoint that makes
@@ -148,6 +148,49 @@ jobs:
sha256sum /tmp/core-index.db.zst | tee /tmp/core-index.db.zst.sha256
ls -lh /tmp/core-index.db.zst
# Nothing is published until it has been imported by the code that
# imports it on a user's machine. The exporter and the importer are
# two descriptions of one storage format, and every other tier tests
# the importer against a *fixture* rather than against the file being
# shipped — a second description free to be wrong in the same
# direction as the code reading it.
#
# That is how #258 reached everyone: the importer positioned its batch
# walk with a Go `string` cursor against this file's 16-byte `mbid`
# column, and SQLite neither coerces between TEXT and BLOB nor
# complains about the comparison — so the walk merged no rows and
# never advanced, and no install could finish its first index build.
# The fixture guarding that walk writes the old text encoding, and the
# only compact fixture is one row, below the batch size, so the bound
# query never ran. Both were green throughout.
#
# Running it here is also what keeps the failure cheap: the previous
# artifact stays published while this runs, so a failure costs one
# stale catalog rather than an empty one for every install.
#
# `-tags indexbuild` because this container has no GTK and the default
# tag set links the app through Wails. The `--- PASS` grep is not
# decoration — the test skips without the path, and a skip is
# indistinguishable from a pass in a summary line.
- name: Import the exported artifact as a client does
if: steps.maintain.outputs.complete == 'true' && steps.maintain.outputs.changed == 'true'
working-directory: /src
env:
YJ_CORE_INDEX_ARTIFACT: /tmp/core-index.db.zst
run: |
set -eu
log=/tmp/import-check.log
if ! go test -tags indexbuild -count=1 -timeout 30m -v \
-run TestImportPublishedArtifact ./backend/explore/ > "$log" 2>&1;
then
tail -60 "$log"
echo "::error::The artifact does not import; not publishing it."
exit 1
fi
cat "$log"
grep -qF -- 'PASS: TestImportPublishedArtifact' "$log"
echo "::notice::The artifact imports as a client would merge it."
- name: Publish to the Gitea package registry
if: steps.maintain.outputs.complete == 'true' && steps.maintain.outputs.changed == 'true'
run: |
@@ -57,6 +57,21 @@ behind `YJ_TESTCTL=1`, which `scripts/dev-headless.sh` sets and
staging the work that would produce it — job progress, download
progress, scan progress. It calls `events.Deliver`, which *errors*
when the event reaches nobody, so a `200` means it really arrived.
- **State you stage, you own** (#168). Nothing resets those stores, so
clear yours in `test.afterEach` with the same event that staged it
(`emit('JobsChanged', [])`) — the store replaces its list from every
snapshot, so `testctl` needs no special case. **Measured: this does
not currently cross a spec boundary**, because every test gets a fresh
page and `JobStore.init()` refetches `GetJobs()` from a backend
registry that `/__test/emit` never writes to. Stated anyway, because
it costs one line and the leak needs only one spec that keeps a page
alive — but do not cite #168 for a symptom you have not reproduced.
- **Measure against the thing next to you, not an absolute
coordinate.** An absolute number in a shell measurement is also a
claim about everything above it — `contentTop === 0` quietly asserts
"and no background job is running", which is not what that spec was
about or could arrange, while `contentTop === jobBandBottom` is true
either way. This is the half of #168 that stands on its own.
- **`restore` is slow** (~40 s in the suite) because it copies every
table. Prefer snapshotting once and restoring only when a spec
genuinely mutates state.
+52
View File
@@ -5078,3 +5078,55 @@ last rendered card.
("ask the virtualizer for a larger overscan") is therefore not
available without patching a private, which is why the request is
issued ahead of the element instead.
## Declined, and recorded nowhere else (2026-09-25, #256)
When CLAUDE.md was cut down to rules, most of its "considered and
declined" paragraphs already had a home in a code or config comment
beside what they explain. These three did not:
- **`touch-action: manipulation` was declined** (#54). The 300ms tap
delay it is offered for is already absent on a `width=device-width`
viewport; what it would actually change is the gesture stack #63 tuned
by measurement on the reference device.
- **There is no "Go to Genre"** in the phone row menu (#67). That menu
replaces name links a phone cannot use, and there has never been a
genre link to replace — it would be new navigation, which wants its
own issue.
- **The overlaid queue has no tap-outside gutter on a phone** (#171).
The drawer-style gutter would buy the affordance by taking width off a
full-screen surface on a 424px viewport; back and a 44px close button
answer it instead.
## slskd's API, read from its source rather than a live daemon (2026-09-26)
`backend/download/provider_slskd.go` had never run against a real slskd
when #263–#272 shipped, so its assumptions were checked against slskd
0.26.0's source. One was wrong, and one design was only safe by luck.
These are properties of someone else's server; re-check on an upgrade.
`TestSlskdLive` (env-gated, see its comment) is the way to confirm them
against a running one.
- **`searchTimeout` is seconds, from the last response**, minimum 5
(`SearchRequest.cs`). We sent milliseconds (#274). The other search
options — `responseLimit`, `fileLimit`, `filterResponses`,
`minimumResponseFileCount`, `maximumPeerQueueLength` — are named as we
send them; slskd's defaults are 100 responses, 10 000 files, queue
1 000 000.
- **`GET /searches/{id}/responses` exists**, and `DELETE
/transfers/downloads/{user}/{id}?remove=true` cancels and removes.
- **A finished download is moved to `<downloads>/<Subdirectory>/`**,
where `Destination.Subdirectory` defaults to `${SOURCE_DIRECTORY}`
(the remote leaf folder) and is user-configurable. A taken name is
written as `name_<ticks>.ext` (`Destination.Exists = rename`, the
default). No transfer record says where the file went.
- **Batch enqueue (`POST /transfers/downloads/batches`) is new in 0.26.0**
and is the only way to choose where a file lands: `options.destination`
overrides the subdirectory pattern. A batch's files land flat in it,
so one batch per disc. On an older daemon that path is routed to the
per-user enqueue as username "batches" and the object body is
rejected with 400 — which is why 400 means "no batches" here.
- **`GET .../downloads/{user}/{id}/position` asks the peer** and returns
a bare integer. slskd's own comment on `PlaceInQueue` is "may be
wildly innacurate to the point of uselessness", which is why #275 acts
only on two readings in a row.
+331 -4001
View File
File diff suppressed because it is too large Load Diff
+1 -1
View File
@@ -13,7 +13,7 @@ frontend, bridged by [Wails v3](https://wails.io/).
| Tool | Version |
|------|---------|
| Go | 1.25+ |
| Go | 1.26+ |
| Node.js | 22+ |
| pnpm | 10+ |
| Wails CLI | v3 — vendored, no install needed (`go tool wails3`) |
+26 -6
View File
@@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
@@ -144,17 +145,36 @@ func (c *apiClient) checkStatus(resp *http.Response) error {
case resp.StatusCode >= 400:
snippet, _ := io.ReadAll(io.LimitReader(resp.Body, 512))
return fmt.Errorf(
"%w: HTTP %d: %s",
c.errUnreachable,
resp.StatusCode,
strings.TrimSpace(string(snippet)),
)
return fmt.Errorf("%w: %w", c.errUnreachable, &httpStatusError{
code: resp.StatusCode,
body: strings.TrimSpace(string(snippet)),
})
default:
return nil
}
}
// httpStatusError is a non-2xx answer, kept typed so a caller can tell
// a missing endpoint from a daemon that is down.
type httpStatusError struct {
code int
body string
}
func (e *httpStatusError) Error() string {
return fmt.Sprintf("HTTP %d: %s", e.code, e.body)
}
// statusCode returns the HTTP status an error carries, or 0.
func statusCode(err error) int {
var se *httpStatusError
if errors.As(err, &se) {
return se.code
}
return 0
}
// decodeJSON decodes a JSON string into out. Providers whose auth or
// response handling does not fit apiClient still parse bodies the same
// way, so the helper lives here rather than being repeated.
+1 -1
View File
@@ -159,7 +159,7 @@ func TestSlskdCollectKeepsDiscFolders(t *testing.T) {
got, err := s.collect(Candidate{Files: []CandidateFile{
{Path: `\m\Album\CD1\01 Intro.flac`, IsAudio: true},
{Path: `\m\Album\CD2\01 Intro.flac`, IsAudio: true},
}}, dst)
}}, dst, "", nil)
if err != nil {
t.Fatalf("collect: %v", err)
}
+190
View File
@@ -0,0 +1,190 @@
package download
import (
"cmp"
"context"
"fmt"
"strings"
"yellowjacket/backend/jobs"
)
// Filling in an almost-complete album (#276).
//
// A grab that delivers nine of twelve tracks clears the completeness
// floor and is imported, and before this the other three were never
// looked for. On Soulseek that is the commonest way an album ends up
// almost right: one peer's folder is missing a track, or one file
// failed. So after an import, each missing track is searched for on
// its own and fetched from somewhere else, into the same album.
// maxFillInTracks bounds how many tracks are fetched one by one. An
// album missing more than a few is a different candidate's job, not a
// dozen single-track grabs.
const maxFillInTracks = 3
// fillIn fetches the tracks a successful import did not deliver. It
// never fails the download: the album is already imported, and a track
// it cannot find is logged and left.
func (m *Manager) fillIn(
ctx context.Context,
dl Download,
main Candidate,
imported ImportResult,
job *jobs.Handle,
) {
missing := missingTracks(dl, imported.Matched)
if len(missing) == 0 {
return
}
for _, t := range missing {
if ctx.Err() != nil {
return
}
paths, err := m.fillInTrack(ctx, dl, main, t, job)
if err != nil {
m.logger.Info(
"could not fill in a missing track",
"download", dl.ID,
"track", t.Title,
"error", err,
)
if job != nil {
job.Logf(jobs.LevelWarn, fmt.Sprintf(
"Could not find %q elsewhere: %v", t.Title, err,
))
}
continue
}
if job != nil {
job.Logf(jobs.LevelInfo, fmt.Sprintf(
"Filled in %q from another source (%d file)", t.Title, len(paths),
))
}
}
}
// missingTracks is what an import left out, when filling it in is
// worth trying: an album with a tracklist, a few tracks short.
func missingTracks(dl Download, matched []ExpectedTrack) []ExpectedTrack {
// A recording request is one track; there is no album to complete.
if dl.RecordingMBID != "" || len(dl.Expected) < 2 {
return nil
}
have := make(map[trackKey]bool, len(matched))
for _, t := range matched {
have[keyOf(t)] = true
}
var missing []ExpectedTrack
for _, t := range dl.Expected {
if !have[keyOf(t)] {
missing = append(missing, t)
}
}
// A half-empty album was a poor copy, not a nearly complete one.
if len(missing) > maxFillInTracks || 2*len(missing) >= len(dl.Expected) {
return nil
}
return missing
}
// fillInTrack searches for one track and grabs the first acceptable
// copy that is not from the source that already failed to supply it.
// One attempt: a fill-in that walks a candidate list per track would
// multiply a download's grabs by the number of gaps.
func (m *Manager) fillInTrack(
ctx context.Context,
dl Download,
main Candidate,
t ExpectedTrack,
job *jobs.Handle,
) ([]string, error) {
want := trackRequest(dl, t)
ranked, err := m.Search(ctx, want)
if err != nil {
return nil, err
}
prefs := m.preferences()
for _, c := range ranked {
if ruledOutBy(c, []Candidate{main}) || !autoAcceptable(want, c, prefs) {
continue
}
// The track is being put into an album this app placed; a
// delegate would put it in its own library instead.
if plan, err := m.planTransfer(want, c); err != nil || plan.delegated() {
continue
}
narrowed, ok := narrowTo(c, t)
if !ok {
continue
}
out := m.attemptGrab(ctx, dl, narrowed, job, []ExpectedTrack{t})
if out.item.StagingDir != "" {
if err := m.staging.Release(out.item.StagingDir); err != nil {
m.logger.Warn("could not release staging dir", "error", err)
}
}
if out.err != nil {
return nil, out.err
}
if err := m.store.SetItemImported(
ctx, out.item.ID, out.imported.Paths,
); err != nil {
m.logger.Warn("could not record imported paths", "error", err)
}
return out.imported.Paths, nil
}
return nil, ErrNoCandidates
}
// trackRequest is the search for one track of an album: the track's
// artist and title as the query, the album kept so a copy from that
// album outranks the same song off a compilation, and one expected
// track so a single file is a complete answer.
func trackRequest(dl Download, t ExpectedTrack) Download {
artist := cmp.Or(t.Artist, dl.Artist)
want := dl
want.Query = strings.TrimSpace(artist + " " + t.Title)
want.Expected = []ExpectedTrack{t}
return want
}
// narrowTo trims a candidate to the one file that aligns to t, so the
// grab fetches a track rather than whatever else the folder offered.
func narrowTo(c Candidate, t ExpectedTrack) (Candidate, bool) {
aligned, _ := matchFiles(c.Files, []ExpectedTrack{t})
for _, f := range aligned {
if f.IsAudio && f.MatchedTo == t.Position {
c.Files = []CandidateFile{f}
c.TotalSize = f.Size
return c, true
}
}
return Candidate{}, false
}
+168
View File
@@ -0,0 +1,168 @@
package download
import (
"context"
"errors"
"os"
"path/filepath"
"testing"
)
// Filling in the tracks an almost-complete album is missing (#276).
func fiveTrackTitles() []string {
return append(allTitles(), "Let Down")
}
func fiveTrackDownload() Download {
dl := fourTrackDownload()
dl.Expected = append(dl.Expected, ExpectedTrack{Position: 5, Title: "Let Down"})
return dl
}
func TestMissingTracks(t *testing.T) {
t.Parallel()
dl := fiveTrackDownload()
got := func(positions ...int) []ExpectedTrack {
out := make([]ExpectedTrack, 0, len(positions))
for _, p := range positions {
out = append(out, dl.Expected[p-1])
}
return out
}
cases := []struct {
name string
dl Download
matched []ExpectedTrack
want int
}{
{"complete", dl, got(1, 2, 3, 4, 5), 0},
{"one short", dl, got(1, 2, 3, 4), 1},
{"two short", dl, got(1, 2, 3), 2},
{"half gone is a poor copy", dl, got(1, 2), 0},
{"a recording is not an album", func() Download {
d := dl
d.RecordingMBID = "rec"
return d
}(), got(1, 2, 3, 4), 0},
}
for _, tc := range cases {
if n := len(missingTracks(tc.dl, tc.matched)); n != tc.want {
t.Errorf("%s: %d missing, want %d", tc.name, n, tc.want)
}
}
big := fiveTrackDownload()
for i := 6; i <= 20; i++ {
big.Expected = append(big.Expected, ExpectedTrack{Position: i, Title: "T" + itoa(i)})
}
if n := len(missingTracks(big, big.Expected[:16])); n != 0 {
t.Errorf("four of twenty missing: %d filled in, want none past %d", n, maxFillInTracks)
}
}
// A fill-in import takes only the file that is the missing track, and
// places it in the album with the album's tags; anything else the grab
// brought is left out.
func TestImportOnlyTakesTheMissingTrack(t *testing.T) {
t.Parallel()
f := newImportFixture(t,
"05 - Let Down.flac",
"02 - Paranoid Android.flac",
)
dl := fiveTrackDownload()
got, err := f.importer.Import(
context.Background(), dl,
Result{Dir: f.dir, Files: f.files},
ImportOptions{LibraryRoot: f.root, WriteTags: true, Only: dl.Expected[4:]},
)
if err != nil {
t.Fatalf("Import: %v", err)
}
want := filepath.Join(f.root, "Radiohead", "OK Computer", "05 Let Down.flac")
if len(got.Paths) != 1 || got.Paths[0] != want {
t.Errorf("imported %q, want only %s", got.Paths, want)
}
// A file that is not the missing track is not imported at all.
g := newImportFixture(t, "02 - Paranoid Android.flac")
if _, err := g.importer.Import(
context.Background(), dl,
Result{Dir: g.dir, Files: g.files},
ImportOptions{LibraryRoot: g.root, WriteTags: true, Only: dl.Expected[4:]},
); !errors.Is(err, ErrTooIncomplete) {
t.Errorf("Import = %v, want nothing matched", err)
}
}
// The album comes from one source missing its fifth track; the fifth is
// then found on its own at another and lands in the same album.
func TestManagerFillsInAMissingTrack(t *testing.T) {
t.Parallel()
f := newManagerFixture(t)
titles := fiveTrackTitles()
album := NewFakeProvider(1, "album", Caps{CanSearch: true, CanTransport: true})
ac := candidateFor("album-cand", titles, ".flac", 30_000_000)
ac.ProviderID = 1
album.Candidates = []Candidate{ac}
for i, tt := range titles[:4] {
album.Written[trackToken(i+1)+" - "+tt+".flac"] = []byte("audio-data")
}
single := NewFakeProvider(2, "single", Caps{CanSearch: true, CanTransport: true})
sc := Candidate{
ID: "single-cand",
Protocol: ProtocolDirect,
Title: "Radiohead - OK Computer",
Artist: "Radiohead",
Files: []CandidateFile{{
Path: "Radiohead - OK Computer/05 - Let Down.flac",
Size: 30_000_000,
}},
Health: 0.5,
ProviderID: 2,
}
single.Candidates = []Candidate{sc}
single.Written["05 - Let Down.flac"] = []byte("audio-data")
f.manager.installProvider(Config{ID: 1, Priority: 90}, album)
f.manager.installProvider(Config{ID: 2, Priority: 10}, single)
dl := fiveTrackDownload()
if _, err := f.manager.Start(context.Background(), dl); err != nil {
t.Fatalf("Start: %v", err)
}
waitForDownloadState(t, f.store, dl.ID, StateComplete)
if album.GrabCallCount() != 1 || single.GrabCallCount() != 1 {
t.Errorf(
"grabs: album=%d single=%d, want 1 and 1",
album.GrabCallCount(), single.GrabCallCount(),
)
}
for i, tt := range titles {
p := filepath.Join(f.root, "Radiohead", "OK Computer", trackToken(i+1)+" "+tt+".flac")
if _, err := os.Stat(p); err != nil {
t.Errorf("track %d not in the library: %v", i+1, err)
}
}
}
+61 -2
View File
@@ -79,6 +79,12 @@ type ImportOptions struct {
// them. Off for delegate providers, which have already imported
// and tagged the files themselves.
WriteTags bool
// Only, when set, imports just the files that align to these
// tracks of the download and skips the completeness check: it is a
// fill-in for tracks an earlier grab of the same album did not
// deliver (#276), tagged and placed as part of that album.
Only []ExpectedTrack
}
// DefaultPathTemplate is the layout used when none is configured.
@@ -118,6 +124,10 @@ type ImportResult struct {
// Skipped counts non-audio files left in staging (logs, cue sheets,
// scene .nfo files) — deliberately not imported.
Skipped int
// Matched are the expected tracks an imported file was aligned to,
// which is how a caller learns what the grab did not deliver.
Matched []ExpectedTrack
}
// Import verifies, tags and moves a completed grab into the library.
@@ -140,14 +150,25 @@ func (i *Importer) Import(
return ImportResult{}, ErrNoAudio
}
if err := checkCompleteness(len(audio), dl); err != nil {
return ImportResult{}, err
if len(opts.Only) == 0 {
if err := checkCompleteness(len(audio), dl); err != nil {
return ImportResult{}, err
}
}
// Align staged files to the expected tracklist so tags and
// filenames reflect the release, not the uploader's naming.
plan := i.planFiles(audio, dl)
if len(opts.Only) > 0 {
plan = onlyTracks(plan, opts.Only)
if len(plan) == 0 {
return ImportResult{}, fmt.Errorf(
"%w: no file matched the missing track", ErrTooIncomplete,
)
}
}
out := ImportResult{
Paths: make([]string, 0, len(plan)),
Skipped: skipped,
@@ -184,11 +205,49 @@ func (i *Importer) Import(
}
out.Paths = append(out.Paths, dest)
if p.Matched {
out.Matched = append(out.Matched, p.Track)
}
}
return out, nil
}
// trackKey identifies an expected track within a release.
type trackKey struct{ disc, position int }
func keyOf(t ExpectedTrack) trackKey {
return trackKey{disc: t.DiscNumber, position: t.Position}
}
// onlyTracks keeps the planned files aligned to one of want. A fill-in
// grab can bring more than the one file it was after — a folder where
// the title also matched a live take — and anything else would land in
// the album as a duplicate or a stranger.
func onlyTracks(plan []plannedFile, want []ExpectedTrack) []plannedFile {
keys := make(map[trackKey]bool, len(want))
for _, t := range want {
keys[keyOf(t)] = true
}
out := make([]plannedFile, 0, len(want))
seen := map[trackKey]bool{}
for _, p := range plan {
k := keyOf(p.Track)
if !p.Matched || !keys[k] || seen[k] {
continue
}
seen[k] = true
out = append(out, p)
}
return out
}
// plannedFile pairs a staged file with the expected track it matched.
type plannedFile struct {
Source string
+4 -1
View File
@@ -747,8 +747,9 @@ func (m *Manager) grab(
var failed []Candidate
for {
out := m.attemptGrab(ctx, dl, c, job)
out := m.attemptGrab(ctx, dl, c, job, nil)
if out.err == nil {
m.fillIn(ctx, dl, c, out.imported, job)
m.finishGrab(ctx, dl, out.item, out.imported, job)
return
@@ -836,6 +837,7 @@ func (m *Manager) attemptGrab(
dl Download,
c Candidate,
job *jobs.Handle,
only []ExpectedTrack,
) grabOutcome {
// Who will move the bytes is decided before any slot is taken, so
// the transfer waits in its own provider's queue rather than in a
@@ -942,6 +944,7 @@ func (m *Manager) attemptGrab(
opts := m.importOptions()
opts.WriteTags = true
opts.Only = only
opts.LibraryRoot, err = m.library.LibraryPath(dl.LibraryID)
if err != nil {
+458 -29
View File
@@ -5,13 +5,16 @@ import (
"errors"
"fmt"
"log/slog"
"net/http"
"net/url"
"os"
"path"
"path/filepath"
"regexp"
"slices"
"strconv"
"strings"
"sync/atomic"
"time"
"github.com/google/uuid"
@@ -68,6 +71,10 @@ const (
// do not get cut off by the context deadline.
slskdSearchWait = 20 * time.Second
// slskdMinSearchTimeout is the smallest searchTimeout slskd accepts,
// in seconds.
slskdMinSearchTimeout = 5
// slskdTransferPoll is how often transfer state is polled.
slskdTransferPoll = 3 * time.Second
@@ -88,7 +95,8 @@ const (
// it covers a peer that queues us and never starts as well as one
// that starts and stops. Ten minutes is long enough for a short
// queue ahead of us to clear and short enough that one unresponsive
// peer does not hold slskd's single transfer slot for an evening.
// peer does not hold a transfer slot for an evening. A queue whose
// position improves restarts it (see queueWatch).
slskdStallAfter = 10 * time.Minute
// slskdAbsentGrace is how long a requested file may be missing from
@@ -97,6 +105,23 @@ const (
// a few polls was refused.
slskdAbsentGrace = 30 * time.Second
// slskdPositionPoll is how often a peer that has queued us is asked
// where we are in its queue. Each ask is a message to the peer, so
// it is far slower than the transfer poll.
slskdPositionPoll = time.Minute
// slskdMaxQueuePosition is the queue position past which a peer is
// not worth waiting for: at a few minutes a track, fifty albums
// ahead of us is days. slskd warns the figure can be inaccurate, so
// it takes two readings in a row to act on (see awaitTransfers).
slskdMaxQueuePosition = 50
// slskdQueueCeiling is the longest a grab waits in a peer's queue
// without a byte arriving, however steadily the queue moves. An
// improving position restarts the stall clock, so without this a
// queue moving one place an hour would hold the grab all day.
slskdQueueCeiling = time.Hour
// slskdCancelTimeout bounds the cleanup that cancels abandoned
// transfers.
slskdCancelTimeout = 15 * time.Second
@@ -161,6 +186,12 @@ type slskd struct {
transferPoll time.Duration
stallAfter time.Duration
absentGrace time.Duration
positionPoll time.Duration
queueCeiling time.Duration
// batches is whether the daemon takes batch downloads, which is
// how a grab gets a folder of its own (see enqueue).
batches atomic.Int32
}
// newSlskd builds the provider from config.
@@ -217,6 +248,8 @@ func newSlskd(
transferPoll: slskdTransferPoll,
stallAfter: slskdStallAfter,
absentGrace: slskdAbsentGrace,
positionPoll: slskdPositionPoll,
queueCeiling: slskdQueueCeiling,
}, nil
}
@@ -289,6 +322,12 @@ type slskdTransfer struct {
BytesTransferred int64 `json:"bytesTransferred"`
}
// remotelyQueued reports whether the peer has accepted the request and
// put it in its upload queue, where it waits for a slot.
func (t slskdTransfer) remotelyQueued() bool {
return strings.Contains(t.State, "Queued") && strings.Contains(t.State, "Remotely")
}
// done reports whether the transfer reached a terminal state, and
// whether it succeeded. slskd reports compound states such as
// "Completed, Succeeded" and "Completed, Errored".
@@ -431,14 +470,18 @@ func (s *slskd) searchRequest(id, text string, minFiles int) map[string]any {
maximumPeerQueueLength = 100
)
// A tenth of the wait is left for the last poll and the responses
// fetch.
timeout := s.searchWait - s.searchWait/10
// slskd reads this in whole seconds, counted from the last response
// rather than from the start, with a floor of 5 (#274). A tenth of
// our own wait is left for the last poll and the responses fetch.
timeout := max(
int((s.searchWait-s.searchWait/10)/time.Second),
slskdMinSearchTimeout,
)
return map[string]any{
"id": id,
"searchText": text,
"searchTimeout": timeout.Milliseconds(),
"searchTimeout": timeout,
"responseLimit": responseLimit,
"fileLimit": fileLimit,
"filterResponses": true,
@@ -530,8 +573,10 @@ func isVariousArtists(artist string) bool {
// usually matches one file per folder. The two-file floor that filters
// out one-file noise for an album therefore filtered out every result
// for a track, and a single-track request could never be served here.
// A request expecting one track — a recording, or the fill-in for one
// missing from an album (#276) — takes a one-file folder.
func minFilesFor(dl Download) int {
if dl.RecordingMBID != "" {
if dl.RecordingMBID != "" || len(dl.Expected) == 1 {
return 1
}
@@ -736,20 +781,99 @@ func (s *slskd) Grab(
)
}
// Only the per-user enqueue writes into folders other grabs share;
// a batch has a folder of its own. Until the daemon has answered a
// batch either way, take the locks anyway.
if s.batches.Load() != batchesSupported {
release, err := lockSlskdFolders(ctx, s.localFolders(c))
if err != nil {
return Result{}, err
}
defer release()
}
// slskd keeps finished transfers listed until someone removes them,
// and a transfer is matched to the request by filename. A record
// left by an earlier attempt at the same file from the same peer
// would otherwise be read as this attempt's answer the moment the
// first poll came back — an old failure failing a transfer that has
// not started. So what is already terminal is noted before enqueueing
// and ignored after.
release, err := lockSlskdFolders(ctx, s.localFolders(c))
// and ignored after. The files already on disk are noted for the
// same reason (see arrivedFile).
stale := s.terminalTransferIDs(ctx, username)
existing := s.snapshotFolders(c)
dest, err := s.enqueue(ctx, username, c)
if err != nil {
return Result{}, err
}
defer release()
stale := s.terminalTransferIDs(ctx, username)
if err := s.awaitTransfers(
ctx, username, stale, c, onProgress,
); err != nil {
s.discardDestination(dest)
return Result{}, err
}
result, err := s.collect(c, dst, dest, existing)
s.discardDestination(dest)
return result, err
}
// Whether the daemon has the batch endpoint, learned from the first
// grab that asks.
const (
batchesUnknown int32 = iota
batchesSupported
batchesUnsupported
)
// slskdDestRoot is the folder under slskd's downloads directory that
// batch destinations are made in, so everything this app asked slskd to
// write is in one place and nothing else is.
const slskdDestRoot = "yellowjacket"
// enqueue asks slskd for a candidate's files and returns the folder,
// relative to the downloads directory, they will be written to — or ""
// when slskd will choose, which is its per-user enqueue.
//
// slskd 0.26 takes a batch with an explicit destination, which is the
// only way to know for certain where a file lands. Without one it is
// `<downloads>/<remote leaf folder>/`, shared with every other download
// of a same-named folder and with the user's own, renamed with a
// `_<ticks>` suffix when a name is taken, and moved by the user's
// `Destination.Subdirectory` setting (#274).
func (s *slskd) enqueue(
ctx context.Context,
username string,
c Candidate,
) (string, error) {
if s.batches.Load() != batchesUnsupported {
dest := slskdDestRoot + "/" + uuid.NewString()
err := s.enqueueBatches(ctx, username, c, dest)
if err == nil {
s.batches.Store(batchesSupported)
return dest, nil
}
if !batchEndpointMissing(err) {
return "", err
}
// An older daemon routes this path to the per-user enqueue with
// "batches" as the username and rejects the body, which is the
// 400; 404 and 405 are a daemon that routes it nowhere.
s.batches.Store(batchesUnsupported)
s.logger.Info(
"slskd has no batch downloads; files will be found by name",
"error", err,
)
}
wanted := make([]map[string]any, 0, len(c.Files))
for _, f := range c.Files {
@@ -762,16 +886,88 @@ func (s *slskd) Grab(
if err := s.client.post(
ctx, slskdDownloadsPath(username), wanted, nil,
); err != nil {
return Result{}, err
return "", err
}
if err := s.awaitTransfers(
ctx, username, stale, c, onProgress,
); err != nil {
return Result{}, err
return "", nil
}
func batchEndpointMissing(err error) bool {
switch statusCode(err) {
case http.StatusBadRequest, http.StatusNotFound, http.StatusMethodNotAllowed:
return true
default:
return false
}
}
// enqueueBatches enqueues one batch per destination folder. A batch's
// files all land directly in its destination, so a multi-disc rip needs
// one per disc or disc 2's "01" is renamed out of the way of disc 1's.
func (s *slskd) enqueueBatches(
ctx context.Context,
username string,
c Candidate,
dest string,
) error {
groups := map[string][]map[string]any{}
for _, f := range c.Files {
sub := batchSubfolder(f.Path)
groups[sub] = append(groups[sub], map[string]any{
"filename": f.Path,
"size": f.Size,
})
}
return s.collect(c, dst)
subs := make([]string, 0, len(groups))
for sub := range groups {
subs = append(subs, sub)
}
slices.Sort(subs)
for _, sub := range subs {
body := map[string]any{
"username": username,
"files": groups[sub],
"options": map[string]any{"destination": path.Join(dest, sub)},
}
if err := s.client.post(
ctx, "/api/v0/transfers/downloads/batches", body, nil,
); err != nil {
return err
}
}
return nil
}
// batchSubfolder is where under a batch's destination a file goes: its
// disc folder, renamed to a form slskd's path sanitising leaves alone
// and ParsePath still reads a disc number from, or nothing.
func batchSubfolder(remote string) string {
norm := strings.ReplaceAll(remote, `\`, "/")
if n, ok := discFolder(path.Base(path.Dir(norm))); ok {
return "Disc " + strconv.Itoa(n)
}
return ""
}
// discardDestination removes a batch's folder once its files have been
// collected or the grab abandoned. It is this grab's own folder under
// slskdDestRoot, so nothing in it belongs to anyone else.
func (s *slskd) discardDestination(dest string) {
if dest == "" || !strings.HasPrefix(dest, slskdDestRoot+"/") {
return
}
if err := os.RemoveAll(filepath.Join(s.downloadsPath, filepath.FromSlash(dest))); err != nil {
s.logger.Debug("could not remove slskd batch folder", "dest", dest, "error", err)
}
}
// slskdFolders serialises grabs that land in the same local folder.
@@ -862,11 +1058,12 @@ func (s *slskd) terminalTransferIDs(
// state, the transfer stalls, or the caller gives up.
//
// Soulseek queues are measured in hours, so there is no deadline on the
// transfer as a whole — but there is one on *progress*. slskd's
// transfer limit is one, so a peer that holds us in its queue without
// sending a byte is not only failing this download, it is holding every
// other Soulseek download behind it. After stallAfter with nothing
// moving the peer is given up on, and the manager tries another.
// transfer as a whole — but there is one on *progress*. A peer that
// holds us without sending a byte is failing this download and holding
// one of the daemon's few transfer slots. After stallAfter with nothing
// moving the peer is given up on, and the manager tries another; a peer
// that has queued us is judged by its queue position as well (see
// queueWatch).
//
// Whatever way this ends short of every file finishing, the transfers
// still live in slskd are cancelled there. Returning without doing so
@@ -889,6 +1086,7 @@ func (s *slskd) awaitTransfers(
lastProgress = started
lastBytes int64
live []slskdTransfer
queue queueWatch
)
for {
@@ -930,6 +1128,28 @@ func (s *slskd) awaitTransfers(
lastProgress = time.Now()
}
if err := queue.observe(ctx, s, username, tally.live, &lastProgress); err != nil {
s.cancelTransfers(username, live)
// As with a stall: what already arrived goes forward.
if tally.done > 0 {
s.logger.Info("slskd queue too long; keeping what arrived", "error", err)
return nil
}
return err
}
if tally.bytes == 0 && time.Since(started) >= s.queueCeiling {
s.cancelTransfers(username, live)
return fmt.Errorf(
"%w: %s sent nothing in %s",
ErrSlskdTimeout, username, s.queueCeiling,
)
}
if onProgress != nil {
onProgress(Progress{
Current: tally.bytes,
@@ -982,6 +1202,77 @@ func (s *slskd) awaitTransfers(
}
}
// queueWatch follows our place in a peer's upload queue while nothing is
// arriving (#275).
//
// No bytes for stallAfter usually means the peer has queued us, and the
// timer alone cannot tell position 2 from position 400. So a queued
// grab asks where it stands every positionPoll: a place that improved is
// progress and restarts the stall clock, and a place past
// slskdMaxQueuePosition twice running gives the peer up at once, so the
// manager moves to the next copy without waiting out the timer.
type queueWatch struct {
lastAsk time.Time
lastPlace int
far int
}
func (q *queueWatch) observe(
ctx context.Context,
s *slskd,
username string,
live []slskdTransfer,
lastProgress *time.Time,
) error {
var queued *slskdTransfer
for i := range live {
if live[i].remotelyQueued() && live[i].ID != "" {
queued = &live[i]
break
}
}
if queued == nil || time.Since(q.lastAsk) < s.positionPoll {
return nil
}
q.lastAsk = time.Now()
var place int
if err := s.client.get(
ctx, slskdDownloadsPath(username)+"/"+url.PathEscape(queued.ID)+"/position", &place,
); err != nil {
// The peer may simply not answer; the stall timer still applies.
s.logger.Debug("slskd queue position unavailable", "peer", username, "error", err)
return nil
}
if q.lastPlace > 0 && place > 0 && place < q.lastPlace {
*lastProgress = time.Now()
}
q.lastPlace = place
if place > slskdMaxQueuePosition {
q.far++
} else {
q.far = 0
}
if q.far >= 2 {
return fmt.Errorf(
"%w: %s has us at position %d in its queue",
ErrSlskdTimeout, username, place,
)
}
return nil
}
// transferTally is one poll's reading of the files a grab asked for.
type transferTally struct {
done, failed int
@@ -1102,10 +1393,17 @@ func (s *slskd) transfersFor(
}
// collect moves finished files out of slskd's download directory into
// staging. slskd lays them out as <downloads>/<folder>/<file>, so each
// wanted file is looked up by its base name under the folder slskd
// derived from the remote path.
func (s *slskd) collect(c Candidate, dst string) (Result, error) {
// staging.
//
// With a batch destination each file is exactly where it was asked to
// go. Without one slskd lays files out as <downloads>/<folder>/<file>,
// and arrivedFile has to tell this grab's file from whatever else has
// that name there.
func (s *slskd) collect(
c Candidate,
dst, dest string,
existing map[string]fileStamp,
) (Result, error) {
result := Result{Dir: dst, Files: make([]string, 0, len(c.Files))}
for _, f := range c.Files {
@@ -1113,10 +1411,28 @@ func (s *slskd) collect(c Candidate, dst string) (Result, error) {
folder := path.Base(path.Dir(norm))
base := path.Base(norm)
src := filepath.Join(s.downloadsPath, folder, base)
var (
src string
info os.FileInfo
ok bool
)
info, err := os.Stat(src)
if err != nil || info.Size() == 0 {
if dest != "" {
src = filepath.Join(
s.downloadsPath, filepath.FromSlash(dest), batchSubfolder(f.Path), base,
)
var err error
info, err = os.Stat(src)
ok = err == nil && info.Size() > 0
} else {
src, info, ok = arrivedFile(
filepath.Join(s.downloadsPath, folder), base, existing,
)
}
if !ok {
// Not every requested file arrives; that is expected and
// handled by completeness scoring downstream.
continue
@@ -1126,7 +1442,7 @@ func (s *slskd) collect(c Candidate, dst string) (Result, error) {
// Flattened, disc 2's "01 Intro.flac" overwrites disc 1's, and
// the importer loses the folder it reads the disc number from.
target := filepath.Join(dst, base)
if _, ok := discFolder(folder); ok {
if _, disc := discFolder(folder); disc {
target = filepath.Join(dst, folder, base)
}
@@ -1147,3 +1463,116 @@ func (s *slskd) collect(c Candidate, dst string) (Result, error) {
return result, nil
}
// fileStamp is enough of a file to tell whether it has been replaced.
type fileStamp struct {
size int64
modTime time.Time
}
// snapshotFolders records the files already in the folders a per-user
// enqueue will write to, so collect does not take one of them for the
// file this grab asked for. A batch writes to a new folder and needs
// none.
func (s *slskd) snapshotFolders(c Candidate) map[string]fileStamp {
if s.batches.Load() == batchesSupported {
return nil
}
out := map[string]fileStamp{}
for _, dir := range s.localFolders(c) {
entries, err := os.ReadDir(dir)
if err != nil {
continue
}
for _, e := range entries {
info, err := e.Info()
if err != nil || !info.Mode().IsRegular() {
continue
}
out[filepath.Join(dir, e.Name())] = fileStamp{
size: info.Size(),
modTime: info.ModTime(),
}
}
}
return out
}
// arrivedFile finds the file slskd wrote for base in dir.
//
// slskd's default when a name is taken is to write `name_<ticks>.ext`
// beside it, so a file with that name left by an earlier failed attempt
// — or by the user's own download of the same folder — would otherwise
// be collected while this grab's copy sat beside it under another name.
// A candidate is the name itself or a renamed form of it that was not
// already there, unchanged, before the grab enqueued; the newest wins.
// Comparing against the snapshot rather than a clock matters because
// slskd may run on another machine whose clock is not ours.
func arrivedFile(
dir, base string,
existing map[string]fileStamp,
) (string, os.FileInfo, bool) {
entries, err := os.ReadDir(dir)
if err != nil {
return "", nil, false
}
ext := filepath.Ext(base)
stem := strings.TrimSuffix(base, ext)
var (
best string
bestInfo os.FileInfo
)
for _, e := range entries {
name := e.Name()
if name != base && !isRenamedCopy(name, stem, ext) {
continue
}
info, err := e.Info()
if err != nil || !info.Mode().IsRegular() || info.Size() == 0 {
continue
}
full := filepath.Join(dir, name)
if was, ok := existing[full]; ok &&
was.size == info.Size() && was.modTime.Equal(info.ModTime()) {
continue
}
if bestInfo == nil || info.ModTime().After(bestInfo.ModTime()) {
best, bestInfo = full, info
}
}
return best, bestInfo, bestInfo != nil
}
// isRenamedCopy reports whether name is stem_<digits>ext, which is how
// slskd names a download whose name was taken.
func isRenamedCopy(name, stem, ext string) bool {
rest, ok := strings.CutPrefix(name, stem+"_")
if !ok {
return false
}
digits, ok := strings.CutSuffix(rest, ext)
if !ok || digits == "" {
return false
}
for _, r := range digits {
if r < '0' || r > '9' {
return false
}
}
return true
}
@@ -0,0 +1,130 @@
package download
import (
"context"
"os"
"path/filepath"
"slices"
"testing"
"time"
)
// TestSlskdLive runs the provider against a real slskd daemon. It is
// skipped unless YJ_SLSKD_URL, YJ_SLSKD_API_KEY and YJ_SLSKD_DOWNLOADS
// are set, and it downloads something only when YJ_SLSKD_GRAB=1 — then
// the smallest candidate the search returns, from whichever stranger
// is sharing it.
//
// Everything else here tests the provider against a stub written from
// reading slskd's source. This is where those readings are checked:
// the search options, the responses endpoint, the batch destination,
// the cancel.
//
// YJ_SLSKD_URL=http://localhost:5030 YJ_SLSKD_API_KEY=… \
// YJ_SLSKD_DOWNLOADS=/path/to/slskd/downloads YJ_SLSKD_GRAB=1 \
// go test -run TestSlskdLive -v ./backend/download/
func TestSlskdLive(t *testing.T) {
base, key, downloads := os.Getenv("YJ_SLSKD_URL"),
os.Getenv("YJ_SLSKD_API_KEY"), os.Getenv("YJ_SLSKD_DOWNLOADS")
if base == "" || key == "" || downloads == "" {
t.Skip(
"set YJ_SLSKD_URL, YJ_SLSKD_API_KEY and YJ_SLSKD_DOWNLOADS to run against a real slskd",
)
}
query := os.Getenv("YJ_SLSKD_QUERY")
if query == "" {
query = "Radiohead OK Computer"
}
p, err := newSlskd(
Config{
ID: 1, Kind: KindSlskd, Name: "live", Enabled: true,
Settings: map[string]string{"url": base, "downloadsPath": downloads},
},
func(string) (string, error) { return key, nil },
slogDiscard(),
)
if err != nil {
t.Fatalf("newSlskd: %v", err)
}
s, ok := p.(*slskd)
if !ok {
t.Fatalf("provider is %T", p)
}
ctx := context.Background()
if err := s.Check(ctx); err != nil {
t.Fatalf("Check: %v", err)
}
started := time.Now()
got, err := s.Search(ctx, Download{Query: query})
if err != nil {
t.Fatalf("Search: %v", err)
}
t.Logf(
"search %q: %d candidates in %s",
query,
len(got),
time.Since(started).Round(time.Millisecond),
)
if len(got) == 0 {
t.Fatal("no candidates; try a more common YJ_SLSKD_QUERY")
}
timed := 0
for _, c := range got {
for _, f := range c.Files {
if f.LengthMillis > 0 {
timed++
}
}
}
t.Logf("%d files carry a length", timed)
if os.Getenv("YJ_SLSKD_GRAB") != "1" {
return
}
smallest := slices.MinFunc(got, func(a, b Candidate) int {
return int(a.TotalSize - b.TotalSize)
})
t.Logf("grabbing %q from %s (%d files, %d bytes)",
smallest.Title, smallest.Origin, len(smallest.Files), smallest.TotalSize)
s.stallAfter = 3 * time.Minute
gctx, cancel := context.WithTimeout(ctx, 15*time.Minute)
defer cancel()
res, err := s.Grab(gctx, smallest, t.TempDir(), func(p Progress) {
t.Logf("%s: %d/%d bytes", p.Phase, p.Current, p.Total)
})
if err != nil {
// A stranger going offline is not a defect; what matters is
// that the transfers were cancelled, which slskd's UI shows.
t.Fatalf("Grab: %v", err)
}
t.Logf("batches: %v", s.batches.Load() == batchesSupported)
for _, f := range res.Files {
info, err := os.Stat(f)
if err != nil || info.Size() == 0 {
t.Errorf("collected %s is missing or empty: %v", f, err)
}
}
if entries, _ := os.ReadDir(filepath.Join(downloads, slskdDestRoot)); len(entries) != 0 {
t.Errorf("%d batch folders left in slskd's downloads", len(entries))
}
}
+161 -58
View File
@@ -7,7 +7,9 @@ import (
"net/http"
"net/http/httptest"
"os"
"path"
"path/filepath"
"strconv"
"strings"
"sync"
"testing"
@@ -55,6 +57,21 @@ type slskdStub struct {
// noResponsesEndpoint makes /searches/{id}/responses 404, as an
// older daemon would.
noResponsesEndpoint bool
// batches makes the daemon take batch downloads, as 0.26 does.
// Without it the batch endpoint answers 400, which is what an older
// daemon's per-user route does with a batch body. batchBodies
// records each batch, and delivered is written into its destination
// under downloads when it is enqueued, keyed by file base name.
batches bool
batchBodies []map[string]any
// positions is what the queue-position endpoint answers, in order;
// the last repeats. positionAsks counts the calls.
positions []int
positionAsks int
delivered map[string]string
downloads string
}
func newSlskdStub(t *testing.T) *slskdStub {
@@ -138,6 +155,29 @@ func newSlskdStub(t *testing.T) *slskdStub {
s.paths = append(s.paths, r.URL.EscapedPath())
s.mu.Unlock()
if r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/position") {
s.mu.Lock()
idx := min(s.positionAsks, len(s.positions)-1)
s.positionAsks++
place := 0
if idx >= 0 {
place = s.positions[idx]
}
s.mu.Unlock()
writeJSON(t, w, place)
return
}
if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/batches") {
s.enqueueBatch(t, w, r)
return
}
switch r.Method {
case http.MethodPost:
var body []map[string]any
@@ -149,6 +189,13 @@ func newSlskdStub(t *testing.T) *slskdStub {
s.mu.Lock()
s.enqueued = body
s.posted = true
for _, file := range body {
name, _ := file["filename"].(string)
norm := strings.ReplaceAll(name, `\`, "/")
s.write(t, path.Base(path.Dir(norm)), path.Base(norm))
}
s.mu.Unlock()
w.WriteHeader(http.StatusCreated)
@@ -202,6 +249,89 @@ func newSlskdStub(t *testing.T) *slskdStub {
return s
}
// enqueueBatch answers the batch endpoint.
func (s *slskdStub) enqueueBatch(t *testing.T, w http.ResponseWriter, r *http.Request) {
t.Helper()
s.mu.Lock()
defer s.mu.Unlock()
if !s.batches {
w.WriteHeader(http.StatusBadRequest)
return
}
var body map[string]any
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
t.Errorf("decode batch body: %v", err)
}
s.batchBodies = append(s.batchBodies, body)
s.posted = true
files, _ := body["files"].([]any)
options, _ := body["options"].(map[string]any)
dest, _ := options["destination"].(string)
for _, f := range files {
file, _ := f.(map[string]any)
s.enqueued = append(s.enqueued, file)
name, _ := file["filename"].(string)
base := path.Base(strings.ReplaceAll(name, `\`, "/"))
s.write(t, filepath.FromSlash(dest), base)
}
w.WriteHeader(http.StatusCreated)
}
// deliver names the files that arrive once enqueued.
func (s *slskdStub) deliver(names ...string) {
s.mu.Lock()
defer s.mu.Unlock()
if s.delivered == nil {
s.delivered = map[string]string{}
}
for _, n := range names {
s.delivered[n] = "audio"
}
}
// write puts a delivered file where slskd would: under dir in the
// downloads folder, renamed name_<ticks>.ext when the name is taken, as
// slskd's default Destination.Exists does. Callers hold s.mu.
func (s *slskdStub) write(t *testing.T, dir, base string) {
t.Helper()
content, ok := s.delivered[base]
if !ok {
return
}
full := filepath.Join(s.downloads, dir)
if err := os.MkdirAll(full, 0o750); err != nil {
t.Errorf("mkdir: %v", err)
}
target := filepath.Join(full, base)
if _, err := os.Stat(target); err == nil {
ext := filepath.Ext(base)
target = filepath.Join(
full,
strings.TrimSuffix(base, ext)+"_"+strconv.FormatInt(time.Now().UnixNano(), 10)+ext,
)
}
if err := os.WriteFile(target, []byte(content), 0o600); err != nil {
t.Errorf("write: %v", err)
}
}
// reject enforces API-key auth like the real daemon.
func (s *slskdStub) reject(w http.ResponseWriter, r *http.Request) bool {
s.mu.Lock()
@@ -234,6 +364,10 @@ func newStubSlskd(t *testing.T, stub *slskdStub) (*slskd, string) {
downloads := t.TempDir()
stub.mu.Lock()
stub.downloads = downloads
stub.mu.Unlock()
p, err := newSlskd(
Config{
ID: 1,
@@ -267,6 +401,8 @@ func newStubSlskd(t *testing.T, stub *slskdStub) (*slskd, string) {
// tests about stalls and absences set their own.
s.stallAfter = time.Minute
s.absentGrace = time.Minute
s.positionPoll = time.Millisecond
s.queueCeiling = time.Hour
return s, downloads
}
@@ -471,24 +607,9 @@ func TestSlskdGrabCollectsFromDownloadsFolder(t *testing.T) {
},
}
s, downloads := newStubSlskd(t, stub)
s, _ := newStubSlskd(t, stub)
// slskd writes into <downloads>/<folder>/<file>.
folder := filepath.Join(downloads, "OK Computer")
if err := os.MkdirAll(folder, 0o750); err != nil {
t.Fatalf("mkdir: %v", err)
}
for _, name := range []string{
"01 Airbag.flac",
"02 Paranoid Android.flac",
} {
if err := os.WriteFile(
filepath.Join(folder, name), []byte("audio"), 0o600,
); err != nil {
t.Fatalf("write: %v", err)
}
}
stub.deliver("01 Airbag.flac", "02 Paranoid Android.flac")
c := Candidate{
ID: "slskd:peer:OK Computer",
@@ -547,18 +668,9 @@ func TestSlskdGrabToleratesPartialFailure(t *testing.T) {
{Filename: `\s\Album\02 B.flac`, State: "Completed, Errored"},
}}
s, downloads := newStubSlskd(t, stub)
s, _ := newStubSlskd(t, stub)
folder := filepath.Join(downloads, "Album")
if err := os.MkdirAll(folder, 0o750); err != nil {
t.Fatalf("mkdir: %v", err)
}
if err := os.WriteFile(
filepath.Join(folder, "01 A.flac"), []byte("audio"), 0o600,
); err != nil {
t.Fatalf("write: %v", err)
}
stub.deliver("01 A.flac")
c := Candidate{
Files: []CandidateFile{
@@ -643,23 +755,12 @@ func TestSlskdRequiresConfiguration(t *testing.T) {
}
}
// slskdAlbum is a two-file candidate from peer, with the files slskd
// would have written already in place under downloads.
func slskdAlbum(t *testing.T, downloads, peer string, arrived ...string) Candidate {
// slskdAlbum is a two-file candidate from peer, whose arrived files
// the stub writes where slskd would once they are enqueued.
func slskdAlbum(t *testing.T, stub *slskdStub, peer string, arrived ...string) Candidate {
t.Helper()
folder := filepath.Join(downloads, "Album")
if err := os.MkdirAll(folder, 0o750); err != nil {
t.Fatalf("mkdir: %v", err)
}
for _, name := range arrived {
if err := os.WriteFile(
filepath.Join(folder, name), []byte("audio"), 0o600,
); err != nil {
t.Fatalf("write: %v", err)
}
}
stub.deliver(arrived...)
return Candidate{
Files: []CandidateFile{
@@ -691,11 +792,11 @@ func TestSlskdGrabGivesUpOnAStalledPeer(t *testing.T) {
{ID: "t2", Filename: `\s\Album\02 B.flac`, State: "Queued, Remotely"},
}}
s, downloads := newStubSlskd(t, stub)
s, _ := newStubSlskd(t, stub)
s.stallAfter = 30 * time.Millisecond
_, err := s.Grab(
context.Background(), slskdAlbum(t, downloads, "peer"), t.TempDir(), nil,
context.Background(), slskdAlbum(t, stub, "peer"), t.TempDir(), nil,
)
if !errors.Is(err, ErrSlskdTimeout) {
t.Fatalf("error = %v, want ErrSlskdTimeout", err)
@@ -728,12 +829,12 @@ func TestSlskdGrabKeepsWhatArrivedBeforeAStall(t *testing.T) {
{ID: "t2", Filename: `\s\Album\02 B.flac`, State: "Queued, Remotely"},
}}
s, downloads := newStubSlskd(t, stub)
s, _ := newStubSlskd(t, stub)
s.stallAfter = 30 * time.Millisecond
got, err := s.Grab(
context.Background(),
slskdAlbum(t, downloads, "peer", "01 A.flac"),
slskdAlbum(t, stub, "peer", "01 A.flac"),
t.TempDir(), nil,
)
if err != nil {
@@ -779,7 +880,7 @@ func TestSlskdGrabWaitsOnATransferThatIsMoving(t *testing.T) {
},
})
s, downloads := newStubSlskd(t, stub)
s, _ := newStubSlskd(t, stub)
// A hundred polls take several times the stall window; each one
// moves a byte. The window is kept well above one poll so a
// descheduled test runner does not read as a stall.
@@ -788,7 +889,7 @@ func TestSlskdGrabWaitsOnATransferThatIsMoving(t *testing.T) {
got, err := s.Grab(
context.Background(),
slskdAlbum(t, downloads, "peer", "01 A.flac", "02 B.flac"),
slskdAlbum(t, stub, "peer", "01 A.flac", "02 B.flac"),
t.TempDir(), nil,
)
if err != nil {
@@ -814,12 +915,12 @@ func TestSlskdGrabCountsAnUnlistedFileAsFailed(t *testing.T) {
},
}}
s, downloads := newStubSlskd(t, stub)
s, _ := newStubSlskd(t, stub)
s.absentGrace = 20 * time.Millisecond
got, err := s.Grab(
context.Background(),
slskdAlbum(t, downloads, "peer", "01 A.flac"),
slskdAlbum(t, stub, "peer", "01 A.flac"),
t.TempDir(), nil,
)
if err != nil {
@@ -854,9 +955,9 @@ func TestSlskdGrabIgnoresAnEarlierAttemptsRecord(t *testing.T) {
},
}
s, downloads := newStubSlskd(t, stub)
s, _ := newStubSlskd(t, stub)
c := slskdAlbum(t, downloads, "peer", "01 A.flac")
c := slskdAlbum(t, stub, "peer", "01 A.flac")
c.Files = c.Files[:1]
got, err := s.Grab(context.Background(), c, t.TempDir(), nil)
@@ -880,12 +981,12 @@ func TestSlskdGrabCancelsTransfersWhenTheCallerGivesUp(t *testing.T) {
{ID: "t2", Filename: `\s\Album\02 B.flac`, State: "Queued, Remotely"},
}}
s, downloads := newStubSlskd(t, stub)
s, _ := newStubSlskd(t, stub)
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
defer cancel()
_, err := s.Grab(ctx, slskdAlbum(t, downloads, "peer"), t.TempDir(), nil)
_, err := s.Grab(ctx, slskdAlbum(t, stub, "peer"), t.TempDir(), nil)
if !errors.Is(err, ErrSlskdTimeout) {
t.Fatalf("error = %v, want ErrSlskdTimeout", err)
}
@@ -912,11 +1013,11 @@ func TestSlskdEscapesTheUsername(t *testing.T) {
},
}}
s, downloads := newStubSlskd(t, stub)
s, _ := newStubSlskd(t, stub)
if _, err := s.Grab(
context.Background(),
slskdAlbum(t, downloads, "dj a/b", "01 A.flac", "02 B.flac"),
slskdAlbum(t, stub, "dj a/b", "01 A.flac", "02 B.flac"),
t.TempDir(), nil,
); err != nil {
t.Fatalf("Grab: %v", err)
@@ -927,7 +1028,9 @@ func TestSlskdEscapesTheUsername(t *testing.T) {
stub.mu.Unlock()
for _, p := range paths {
if p != "/api/v0/transfers/downloads/dj%20a%2Fb" {
// The batch endpoint carries the name in its body.
if p != "/api/v0/transfers/downloads/dj%20a%2Fb" &&
p != "/api/v0/transfers/downloads/batches" {
t.Errorf("transfers call went to %s", p)
}
}
+328
View File
@@ -0,0 +1,328 @@
package download
import (
"context"
"os"
"path/filepath"
"strings"
"testing"
"time"
)
// Where slskd writes a grab's files, and how collect finds them (#274).
// slskd reads searchTimeout in whole seconds, from the last response.
func TestSlskdSearchTimeoutIsInSeconds(t *testing.T) {
t.Parallel()
cases := []struct {
wait time.Duration
want int
}{
{20 * time.Second, 18},
{200 * time.Millisecond, slskdMinSearchTimeout},
}
for _, tc := range cases {
s := &slskd{searchWait: tc.wait}
got, ok := s.searchRequest("id", "text", 2)["searchTimeout"].(int)
if !ok || got != tc.want {
t.Errorf("wait %s: searchTimeout = %v, want %d seconds", tc.wait, got, tc.want)
}
}
}
func TestIsRenamedCopy(t *testing.T) {
t.Parallel()
cases := map[string]bool{
"01 A_638912345678901234.flac": true,
"01 A.flac": false,
"01 A_.flac": false,
"01 A_v2.flac": false,
"01 A_123.mp3": false,
"01 AB_123.flac": false,
}
for name, want := range cases {
if got := isRenamedCopy(name, "01 A", ".flac"); got != want {
t.Errorf("isRenamedCopy(%q) = %v, want %v", name, got, want)
}
}
}
func succeeded(names ...string) [][]slskdTransfer {
out := make([]slskdTransfer, 0, len(names))
for i, n := range names {
out = append(out, slskdTransfer{
ID: "t" + itoa(i),
Filename: n,
State: "Completed, Succeeded",
BytesTransferred: 500,
})
}
return [][]slskdTransfer{out}
}
// On a daemon with batches, each grab writes into a folder of its own,
// collect reads from exactly there, and the folder is gone afterwards.
func TestSlskdBatchGrabUsesItsOwnFolder(t *testing.T) {
t.Parallel()
stub := newSlskdStub(t)
stub.batches = true
stub.transfers = succeeded(`\s\Album\01 A.flac`, `\s\Album\02 B.flac`)
s, downloads := newStubSlskd(t, stub)
// A same-named file in the folder a per-user enqueue would use is
// someone else's, and must not be touched.
other := filepath.Join(downloads, "Album", "01 A.flac")
if err := os.MkdirAll(filepath.Dir(other), 0o750); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(other, []byte("the user's"), 0o600); err != nil {
t.Fatal(err)
}
dst := t.TempDir()
got, err := s.Grab(
context.Background(), slskdAlbum(t, stub, "peer", "01 A.flac", "02 B.flac"), dst, nil,
)
if err != nil {
t.Fatalf("Grab: %v", err)
}
if len(got.Files) != 2 {
t.Fatalf("collected %d files, want 2", len(got.Files))
}
stub.mu.Lock()
bodies := append([]map[string]any(nil), stub.batchBodies...)
stub.mu.Unlock()
if len(bodies) != 1 {
t.Fatalf("%d batches, want 1", len(bodies))
}
if bodies[0]["username"] != "peer" {
t.Errorf("batch username = %v", bodies[0]["username"])
}
dest, _ := bodies[0]["options"].(map[string]any)["destination"].(string)
if !strings.HasPrefix(dest, slskdDestRoot+"/") {
t.Errorf("destination %q is not under %s", dest, slskdDestRoot)
}
if _, err := os.Stat(filepath.Join(downloads, filepath.FromSlash(dest))); !os.IsNotExist(err) {
t.Errorf("batch folder left behind: %v", err)
}
if data, _ := os.ReadFile(other); string(data) != "the user's" {
t.Errorf("the user's own file was taken or changed: %q", data)
}
}
// A batch's files land flat in its destination, so a two-disc rip is
// two batches, one per disc, or disc 2's "01" is renamed out of the way
// of disc 1's.
func TestSlskdBatchSplitsDiscs(t *testing.T) {
t.Parallel()
stub := newSlskdStub(t)
stub.batches = true
stub.transfers = succeeded(`\s\Wall\CD1\01 In.flac`, `\s\Wall\CD2\01 Hey You.flac`)
stub.deliver("01 In.flac", "01 Hey You.flac")
s, _ := newStubSlskd(t, stub)
dst := t.TempDir()
got, err := s.Grab(context.Background(), Candidate{
Files: []CandidateFile{
{Path: `\s\Wall\CD1\01 In.flac`, Size: 500, IsAudio: true},
{Path: `\s\Wall\CD2\01 Hey You.flac`, Size: 500, IsAudio: true},
},
Payload: map[string]string{"username": "peer"},
}, dst, nil)
if err != nil {
t.Fatalf("Grab: %v", err)
}
stub.mu.Lock()
n := len(stub.batchBodies)
stub.mu.Unlock()
if n != 2 {
t.Errorf("%d batches, want one per disc", n)
}
for _, want := range []string{
filepath.Join(dst, "CD1", "01 In.flac"),
filepath.Join(dst, "CD2", "01 Hey You.flac"),
} {
found := false
for _, f := range got.Files {
found = found || f == want
}
if !found {
t.Errorf("%s not collected; got %q", want, got.Files)
}
}
}
// An abandoned batch grab leaves nothing in slskd's folder either.
func TestSlskdBatchFailureDiscardsItsFolder(t *testing.T) {
t.Parallel()
stub := newSlskdStub(t)
stub.batches = true
stub.transfers = [][]slskdTransfer{{
{ID: "t0", Filename: `\s\Album\01 A.flac`, State: "Completed, Errored"},
{ID: "t1", Filename: `\s\Album\02 B.flac`, State: "Completed, Errored"},
}}
s, downloads := newStubSlskd(t, stub)
// The stub delivers the file, as a partial slskd left behind would.
if _, err := s.Grab(
context.Background(), slskdAlbum(t, stub, "peer", "01 A.flac"), t.TempDir(), nil,
); err == nil {
t.Fatal("Grab succeeded with every transfer failed")
}
entries, _ := os.ReadDir(filepath.Join(downloads, slskdDestRoot))
if len(entries) != 0 {
t.Errorf("%d batch folders left behind", len(entries))
}
}
// An older daemon answers the batch endpoint with 400; the grab falls
// back to the per-user enqueue, and later grabs do not ask again.
func TestSlskdFallsBackWithoutBatches(t *testing.T) {
t.Parallel()
stub := newSlskdStub(t)
stub.transfers = succeeded(`\s\Album\01 A.flac`, `\s\Album\02 B.flac`)
s, _ := newStubSlskd(t, stub)
for range 2 {
stub.mu.Lock()
stub.posted = false
stub.pollCount = 0
stub.mu.Unlock()
if _, err := s.Grab(
context.Background(), slskdAlbum(t, stub, "peer", "01 A.flac"), t.TempDir(), nil,
); err != nil {
t.Fatalf("Grab: %v", err)
}
}
stub.mu.Lock()
paths := append([]string(nil), stub.paths...)
stub.mu.Unlock()
batchCalls := 0
for _, p := range paths {
if strings.HasSuffix(p, "/batches") {
batchCalls++
}
}
if batchCalls != 1 {
t.Errorf("asked for a batch %d times, want once", batchCalls)
}
}
// Without batches, a file of the same name already in slskd's folder is
// not this grab's: slskd wrote ours beside it as name_<ticks>.ext, and
// that is the one collected.
func TestSlskdCollectsTheRenamedCopyNotTheOldFile(t *testing.T) {
t.Parallel()
stub := newSlskdStub(t)
stub.transfers = succeeded(`\s\Album\01 A.flac`, `\s\Album\02 B.flac`)
s, downloads := newStubSlskd(t, stub)
old := filepath.Join(downloads, "Album", "01 A.flac")
if err := os.MkdirAll(filepath.Dir(old), 0o750); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(old, []byte("left by an earlier attempt"), 0o600); err != nil {
t.Fatal(err)
}
dst := t.TempDir()
got, err := s.Grab(
context.Background(), slskdAlbum(t, stub, "peer", "01 A.flac", "02 B.flac"), dst, nil,
)
if err != nil {
t.Fatalf("Grab: %v", err)
}
if len(got.Files) != 2 {
t.Fatalf("collected %d files, want 2", len(got.Files))
}
data, err := os.ReadFile(filepath.Join(dst, "01 A.flac"))
if err != nil || string(data) != "audio" {
t.Errorf("collected %q, want this grab's file", data)
}
if data, _ := os.ReadFile(old); string(data) != "left by an earlier attempt" {
t.Error("the file that was already there was moved")
}
}
// And when this grab's copy never arrived, the old one is not taken in
// its place.
func TestSlskdDoesNotCollectAFileThatWasAlreadyThere(t *testing.T) {
t.Parallel()
stub := newSlskdStub(t)
stub.transfers = [][]slskdTransfer{
{
{ID: "t0", Filename: `\s\Album\01 A.flac`, State: "Completed, Errored"},
{
ID: "t1",
Filename: `\s\Album\02 B.flac`,
State: "Completed, Succeeded",
BytesTransferred: 500,
},
},
}
s, downloads := newStubSlskd(t, stub)
old := filepath.Join(downloads, "Album", "01 A.flac")
if err := os.MkdirAll(filepath.Dir(old), 0o750); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(old, []byte("stale"), 0o600); err != nil {
t.Fatal(err)
}
got, err := s.Grab(
context.Background(), slskdAlbum(t, stub, "peer", "02 B.flac"), t.TempDir(), nil,
)
if err != nil {
t.Fatalf("Grab: %v", err)
}
if len(got.Files) != 1 || filepath.Base(got.Files[0]) != "02 B.flac" {
t.Errorf("collected %q, want only 02 B.flac", got.Files)
}
}
+132
View File
@@ -0,0 +1,132 @@
package download
import (
"context"
"errors"
"strings"
"testing"
"time"
)
// A peer that has queued us is judged by where we are in its queue, not
// only by a timer (#275).
func queuedThen(polls int, final string) [][]slskdTransfer {
queued := []slskdTransfer{
{ID: "t0", Filename: `\s\Album\01 A.flac`, State: "Queued, Remotely"},
{ID: "t1", Filename: `\s\Album\02 B.flac`, State: "Queued, Remotely"},
}
out := make([][]slskdTransfer, 0, polls+1)
for range polls {
out = append(out, queued)
}
return append(out, []slskdTransfer{
{ID: "t0", Filename: `\s\Album\01 A.flac`, State: final, BytesTransferred: 500},
{ID: "t1", Filename: `\s\Album\02 B.flac`, State: final, BytesTransferred: 500},
})
}
func descending(from int) []int {
out := make([]int, 0, from)
for p := from; p >= 1; p-- {
out = append(out, p)
}
return out
}
// Two readings far back in the queue give the peer up at once, rather
// than after the stall timer.
func TestSlskdGivesUpOnALongQueue(t *testing.T) {
t.Parallel()
stub := newSlskdStub(t)
stub.transfers = queuedThen(100_000, "Completed, Succeeded")
stub.positions = []int{400}
s, _ := newStubSlskd(t, stub)
s.stallAfter = time.Hour
started := time.Now()
_, err := s.Grab(context.Background(), slskdAlbum(t, stub, "peer"), t.TempDir(), nil)
if !errors.Is(err, ErrSlskdTimeout) || !strings.Contains(err.Error(), "position 400") {
t.Fatalf("Grab = %v, want a queue-position give-up", err)
}
if time.Since(started) > 5*time.Second {
t.Error("the give-up waited on something other than the position")
}
if len(stub.cancelledURIs()) == 0 {
t.Error("the queued transfers were not cancelled")
}
}
// One far reading is not enough: slskd says the figure can be wildly
// wrong.
func TestSlskdOneBadPositionIsNotEnough(t *testing.T) {
t.Parallel()
stub := newSlskdStub(t)
stub.transfers = queuedThen(20, "Completed, Succeeded")
stub.positions = []int{400, 3}
s, _ := newStubSlskd(t, stub)
s.positionPoll = 0
if _, err := s.Grab(
context.Background(),
slskdAlbum(t, stub, "peer", "01 A.flac", "02 B.flac"),
t.TempDir(), nil,
); err != nil {
t.Fatalf("Grab: %v", err)
}
}
// A queue that is moving is progress: the grab outlives the stall timer
// while its position improves.
func TestSlskdAMovingQueueIsProgress(t *testing.T) {
t.Parallel()
stub := newSlskdStub(t)
// Near enough to wait for, with more improving readings (45) than
// there are queued polls (30), so the queue outlives the stall timer
// (30 polls of at least 2 ms against 40 ms) while still improving,
// however slowly the machine runs the loop.
stub.transfers = queuedThen(30, "Completed, Succeeded")
stub.positions = descending(slskdMaxQueuePosition - 5)
s, _ := newStubSlskd(t, stub)
s.transferPoll = 2 * time.Millisecond
s.positionPoll = 0
s.stallAfter = 40 * time.Millisecond
if _, err := s.Grab(
context.Background(),
slskdAlbum(t, stub, "peer", "01 A.flac", "02 B.flac"),
t.TempDir(), nil,
); err != nil {
t.Fatalf("Grab: %v; a moving queue was treated as a stall", err)
}
}
// However steadily the queue moves, waiting in it has a ceiling.
func TestSlskdQueueHasACeiling(t *testing.T) {
t.Parallel()
stub := newSlskdStub(t)
stub.transfers = queuedThen(100_000, "Completed, Succeeded")
stub.positions = descending(100_000)
s, _ := newStubSlskd(t, stub)
s.stallAfter = time.Hour
s.queueCeiling = 50 * time.Millisecond
_, err := s.Grab(context.Background(), slskdAlbum(t, stub, "peer"), t.TempDir(), nil)
if !errors.Is(err, ErrSlskdTimeout) {
t.Fatalf("Grab = %v, want the queue ceiling", err)
}
}
+88 -11
View File
@@ -1,8 +1,10 @@
package explore
import (
"bytes"
"context"
"database/sql"
"database/sql/driver"
"errors"
"fmt"
"os"
@@ -283,6 +285,25 @@ func (si *SearchIndex) importCoreArtifact(ctx context.Context, path string) erro
}
merged, mergeErr := si.mergeArtifactRows(ctx, info.rows)
// Every row the artifact declares has to land. The walk partitions
// the artifact's key space, so a total short of info.rows does not
// mean the artifact was smaller than it said -- it means a predicate
// filtered rows out, and the catalog is quietly partial. Equality
// rather than a lower bound because RowsAffected counts an upsert
// that changes nothing, and a row already merged locally is counted
// again here.
//
// One reachable case, so this is not merely a tripwire: a row whose
// mbid is empty is excluded by `mbid > ?` in both encodings, and an
// artifact carrying one would otherwise import as complete.
if mergeErr == nil && merged != info.rows {
mergeErr = fmt.Errorf(
"%w: merged %d of %d rows — a row the artifact holds was not selected",
ErrArtifactUnusable, merged, info.rows,
)
}
if mergeErr == nil {
si.mergeArtifactCredits(ctx)
}
@@ -348,8 +369,12 @@ func (si *SearchIndex) analyzeIndex() {
// is an index range scan and a cancelled import leaves committed work
// behind rather than rolling it all back.
func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, error) {
// Asked once, because it is a property of the file and it decides
// how the walk's own comparisons are typed. See artifactKey.
storesText := si.artifactStoresText()
selectColumns := artifactSelectColumns(
si.artifactStoresText(), si.artifactHasTotals(),
storesText, si.artifactHasTotals(),
)
insertSQL := `
@@ -367,7 +392,7 @@ func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, e
WHERE mbid > ? AND mbid <= ?` + upsertIndexConflictSQL
var (
cursor string
cursor artifactKey
merged int
)
@@ -376,17 +401,30 @@ func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, e
return merged, err
}
upper, hasUpper, err := si.artifactBatchBound(cursor)
upper, hasUpper, err := si.artifactBatchBound(storesText, cursor)
if err != nil {
return merged, err
}
if hasUpper && bytes.Compare(upper, cursor) <= 0 {
// The predicate matched the cursor itself, so the walk can
// never advance. SQLite says nothing when a comparison is
// made between types it will not coerce - the query simply
// answers wrongly - so a mismatch here would otherwise spin
// forever behind an unmoving progress bar. Fail instead.
return merged, fmt.Errorf(
"%w: artifact walk did not advance past %x",
ErrArtifactUnusable, []byte(cursor),
)
}
var res sql.Result
if hasUpper {
res, err = si.db.ExecContext(insertRangeSQL, cursor, upper)
res, err = si.db.ExecContext(insertRangeSQL,
cursor.bind(storesText), upper.bind(storesText))
} else {
res, err = si.db.ExecContext(insertSQL, cursor)
res, err = si.db.ExecContext(insertSQL, cursor.bind(storesText))
}
if err != nil {
@@ -413,26 +451,65 @@ func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, e
}
}
// artifactKey is one MBID as the attached artifact stores it: 16 raw
// bytes in a compact artifact, the dashed 36-character form in one
// published before that storage change.
//
// It is a type with a bind method rather than a string because the
// comparison it feeds is typed, and the wrong type is silent. SQLite
// does not coerce between TEXT and BLOB and orders every blob after
// every text value, so a cursor bound as text against a byte column
// makes `mbid > ?` true of the whole table - the walk rediscovers the
// same batch bound forever, and `mbid <= ?` false of the whole table,
// so no batch merges at all. Nothing errors; the import simply never
// finishes. bind is the one place that knows which form the column is
// in, decided by artifactStoresText, which asks the artifact rather than
// trusting a version number.
type artifactKey []byte
// bind renders the key as a statement argument in the artifact's own
// encoding.
func (k artifactKey) bind(storesText bool) driver.Value {
if storesText {
return string(k)
}
// Never nil. database/sql converts a nil []byte to SQL NULL, and
// `mbid > NULL` is NULL for every row - so an unset cursor would
// agree with nothing and import nothing, which is the same silently
// empty merge this type exists to prevent, one type over.
if k == nil {
return []byte{}
}
return []byte(k)
}
// artifactBatchBound returns the MBID that ends the next batch, and
// whether one exists — no bound means the remainder is the last batch.
func (si *SearchIndex) artifactBatchBound(cursor string) (string, bool, error) {
var bound string
//
// The bound is read out of the artifact and handed back as an
// artifactKey, because it becomes the next comparison the walk makes.
func (si *SearchIndex) artifactBatchBound(
storesText bool, cursor artifactKey,
) (artifactKey, bool, error) {
var bound []byte
err := si.db.QueryRowWriter(
`SELECT mbid FROM core.explore_index
WHERE mbid > ? ORDER BY mbid LIMIT 1 OFFSET ?`,
cursor, artifactMergeBatch-1,
cursor.bind(storesText), artifactMergeBatch-1,
).Scan(&bound)
if errors.Is(err, sql.ErrNoRows) {
return "", false, nil
return nil, false, nil
}
if err != nil {
return "", false, fmt.Errorf("%w: batch bound: %w", ErrArtifactUnusable, err)
return nil, false, fmt.Errorf("%w: batch bound: %w", ErrArtifactUnusable, err)
}
return bound, true, nil
return artifactKey(bound), true, nil
}
// stampArtifactMeta records what the merge established: the catalog half
+250 -62
View File
@@ -1,6 +1,7 @@
package explore
import (
"bytes"
"context"
"database/sql"
"encoding/hex"
@@ -71,13 +72,7 @@ func writeTestArtifact(
}
}
for k, v := range meta {
if _, err := db.Exec(
`INSERT INTO artifact_meta (key, value) VALUES (?, ?)`, k, v,
); err != nil {
t.Fatalf("stamp artifact meta: %v", err)
}
}
stampArtifactMeta(t, db, meta)
for _, r := range rows {
if _, err := db.Exec(`
@@ -93,6 +88,101 @@ func writeTestArtifact(
return path
}
// compactArtifactSchema is the artifact cmd/indexexport publishes: the
// catalog's ids as 16 raw bytes, its entity types as codes, and the
// per-release-group total_tracks the exporter added after the first
// artifact was shipped.
//
// It matters that a fixture carries this encoding and not the older
// text one, because SQLite does not coerce between TEXT and BLOB and
// every comparison the importer makes against an mbid is therefore
// encoding-sensitive. writeTestArtifact above is the *other* fixture:
// it still writes the text form, which is what the first published
// artifact carries and what the importer must keep reading.
var compactArtifactSchema = []string{
`CREATE TABLE explore_index (
entity_type INTEGER NOT NULL,
mbid BLOB NOT NULL,
title TEXT NOT NULL,
artist_name TEXT NOT NULL,
artist_mbid BLOB NOT NULL,
aliases TEXT NOT NULL DEFAULT '',
popularity INTEGER NOT NULL DEFAULT 0,
listener_count INTEGER NOT NULL DEFAULT 0,
duration INTEGER NOT NULL DEFAULT 0,
caa_release_mbid BLOB NOT NULL DEFAULT x'',
release_name TEXT NOT NULL DEFAULT '',
primary_type TEXT NOT NULL DEFAULT '',
secondary_types TEXT NOT NULL DEFAULT '',
release_date TEXT NOT NULL DEFAULT '',
total_tracks INTEGER NOT NULL DEFAULT 0,
artist_type TEXT NOT NULL DEFAULT '',
country TEXT NOT NULL DEFAULT '',
disambiguation TEXT NOT NULL DEFAULT '',
sort_name TEXT NOT NULL DEFAULT '',
discog_fetched INTEGER NOT NULL DEFAULT 0,
PRIMARY KEY (mbid)
) WITHOUT ROWID`,
`CREATE TABLE artifact_meta (
key TEXT PRIMARY KEY,
value TEXT NOT NULL
)`,
}
// writeCompactTestArtifact builds the artifact the exporter publishes
// today, in its own encoding, so the importer is exercised against what
// a client actually downloads rather than against what it was written
// for.
func writeCompactTestArtifact(
t *testing.T, meta map[string]string, rows []artifactRow,
) string {
t.Helper()
path := filepath.Join(t.TempDir(), "core-index.db")
db, err := sql.Open("sqlite", "file:"+path)
if err != nil {
t.Fatalf("open artifact: %v", err)
}
defer func() { _ = db.Close() }()
for _, stmt := range compactArtifactSchema {
if _, err := db.Exec(stmt); err != nil {
t.Fatalf("create artifact schema: %v", err)
}
}
stampArtifactMeta(t, db, meta)
for _, r := range rows {
if _, err := db.Exec(`
INSERT INTO explore_index
(entity_type, mbid, title, artist_name, artist_mbid, popularity)
VALUES (?, ?, ?, ?, ?, ?)`,
entityCode(r.entityType), mbidBytes(r.mbid), r.title,
r.artistName, mbidBytes(r.artistMBID), r.popularity,
); err != nil {
t.Fatalf("insert artifact row: %v", err)
}
}
return path
}
// stampArtifactMeta writes the artifact_meta rows a fixture declares.
func stampArtifactMeta(t *testing.T, db *sql.DB, meta map[string]string) {
t.Helper()
for k, v := range meta {
if _, err := db.Exec(
`INSERT INTO artifact_meta (key, value) VALUES (?, ?)`, k, v,
); err != nil {
t.Fatalf("stamp artifact meta: %v", err)
}
}
}
// validMeta is the artifact_meta a well-formed artifact carries.
func validMeta() map[string]string {
return map[string]string{
@@ -270,6 +360,71 @@ func TestImportCoreArtifactBatchWalkCoversAllRows(t *testing.T) {
}
}
// TestImportCoreArtifactBatchWalkCoversAllRowsCompact is the batch walk
// on the encoding the exporter actually publishes.
//
// The walk positions itself by comparing the artifact's own mbid column
// against the last id it reached, and that column holds 16 raw bytes.
// SQLite does not coerce between TEXT and BLOB, and a blob sorts after
// every text value, so a cursor bound as text is a predicate that either
// matches every row or none: `mbid > ?` with an empty text key is true
// of the whole table, so
// the 100th row is always the 100th row and the bound never advances,
// while `mbid <= <text>` is false of the whole table, so no batch ever
// merges. The result is not a wrong import but an unbounded loop that
// merges nothing and never fails.
//
// Both encodings are covered on purpose. The walk was only ever tested
// against the text fixture above, which is why it shipped broken on the
// one the clients download.
func TestImportCoreArtifactBatchWalkCoversAllRowsCompact(t *testing.T) {
db := database.NewTestDB(t)
si := NewSearchIndex(db, nil, nil, testLogger())
original := artifactMergeBatch
artifactMergeBatch = 100
t.Cleanup(func() { artifactMergeBatch = original })
const total = 337
rows := make([]artifactRow, 0, total)
for i := range total {
rows = append(rows, artifactRow{
entityType: EntityRecording,
mbid: syntheticMBID(i),
title: "Song",
artistName: "Artist",
artistMBID: artA,
popularity: i,
})
}
path := writeCompactTestArtifact(t, validMeta(), rows)
if err := si.importCoreArtifact(context.Background(), path); err != nil {
t.Fatalf("importCoreArtifact: %v", err)
}
var got, top int
if err := db.QueryRowWriter(
"SELECT COUNT(*), MAX(popularity) FROM explore_index",
).Scan(&got, &top); err != nil {
t.Fatalf("count rows: %v", err)
}
if got != total {
t.Errorf("merged %d rows, want %d", got, total)
}
// A count alone would pass if the walk re-merged the same first
// batch forever, so the far end of the artifact is checked too.
if top != total-1 {
t.Errorf("highest popularity = %d, want %d", top, total-1)
}
}
func TestImportCoreArtifactRejectsBadArtifacts(t *testing.T) {
tests := []struct {
name string
@@ -454,61 +609,9 @@ func TestArtifactColumnsMatchExporter(t *testing.T) {
// the importer decides by asking the artifact, not by trusting a
// version number, and both must land identically.
func TestImportCoreArtifactAcceptsBothEncodings(t *testing.T) {
compact := filepath.Join(t.TempDir(), "core-index.db")
db, err := sql.Open("sqlite", "file:"+compact)
if err != nil {
t.Fatalf("open artifact: %v", err)
}
if _, err := db.Exec(`CREATE TABLE explore_index (
entity_type INTEGER NOT NULL,
mbid BLOB NOT NULL,
title TEXT NOT NULL,
artist_name TEXT NOT NULL,
artist_mbid BLOB NOT NULL,
aliases TEXT NOT NULL DEFAULT '',
popularity INTEGER NOT NULL DEFAULT 0,
listener_count INTEGER NOT NULL DEFAULT 0,
duration INTEGER NOT NULL DEFAULT 0,
caa_release_mbid BLOB NOT NULL DEFAULT x'',
release_name TEXT NOT NULL DEFAULT '',
primary_type TEXT NOT NULL DEFAULT '',
secondary_types TEXT NOT NULL DEFAULT '',
release_date TEXT NOT NULL DEFAULT '',
artist_type TEXT NOT NULL DEFAULT '',
country TEXT NOT NULL DEFAULT '',
disambiguation TEXT NOT NULL DEFAULT '',
sort_name TEXT NOT NULL DEFAULT '',
discog_fetched INTEGER NOT NULL DEFAULT 0,
PRIMARY KEY (mbid)
)`); err != nil {
t.Fatalf("create artifact table: %v", err)
}
if _, err := db.Exec(
`CREATE TABLE artifact_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL)`,
); err != nil {
t.Fatalf("create artifact meta: %v", err)
}
for k, v := range validMeta() {
if _, err := db.Exec(
"INSERT INTO artifact_meta (key, value) VALUES (?, ?)", k, v,
); err != nil {
t.Fatalf("write artifact meta: %v", err)
}
}
if _, err := db.Exec(`
INSERT INTO explore_index (entity_type, mbid, title, artist_name, artist_mbid, popularity)
VALUES (1, ?, 'Artist A', 'Artist A', ?, 5000)`,
mbidBytes(artA), mbidBytes(artA),
); err != nil {
t.Fatalf("write artifact row: %v", err)
}
_ = db.Close()
compact := writeCompactTestArtifact(t, validMeta(), []artifactRow{
{EntityArtist, artA, "Artist A", "Artist A", artA, 5000},
})
live := database.NewTestDB(t)
si := NewSearchIndex(live, nil, nil, testLogger())
@@ -748,3 +851,88 @@ func TestImportCoreArtifactWithoutCredits(t *testing.T) {
t.Errorf("credit refs = %d, want 0", refs)
}
}
// TestImportCoreArtifactRefusesAMergeThatLosesRows is the count guard's
// positive case.
//
// The walk's predicates partition the artifact's key space, so a merge
// that lands fewer rows than the artifact declares means a predicate
// dropped some — and the failure is a catalog that looks populated and
// is missing things nobody can name. An empty mbid is the reachable
// way to get there: `mbid > ?` is false of it in both encodings, so it
// is never selected, and nothing else in the import would notice.
func TestImportCoreArtifactRefusesAMergeThatLosesRows(t *testing.T) {
db := database.NewTestDB(t)
si := NewSearchIndex(db, nil, nil, testLogger())
path := writeCompactTestArtifact(t, validMeta(), []artifactRow{
{EntityArtist, artA, "Artist A", "Artist A", artA, 5000},
{EntityArtist, "", "Nameless", "Artist A", artA, 4000},
})
err := si.importCoreArtifact(context.Background(), path)
if err == nil {
t.Fatal("a merge that lost a row was reported as a complete import")
}
if !strings.Contains(err.Error(), "merged 1 of 2 rows") {
t.Errorf("error = %v, want it to name the shortfall", err)
}
// And the same rule as every other rejection: a failed merge must not
// leave the index claiming it has a catalog, or the real build would
// never run again.
if si.hasMeta(dumpImportDoneKey) {
t.Error("a failed import still stamped dump_import_done")
}
}
// TestArtifactKeyBindsInTheArtifactsOwnEncoding pins the one place the
// batch walk's comparison type is decided.
//
// Every wrong answer is silent, which is why it is worth pinning all
// four. SQLite does not coerce TEXT to BLOB and orders every blob after
// every text value, so a text key against a byte column makes
// `mbid > ?` true of the whole artifact - the cursor never advances and
// the walk spins forever without merging a row - while a byte key
// against a text column makes it false of the whole artifact, so every
// batch merges nothing and the import "succeeds" empty. An unset cursor
// is the same fault once more: database/sql converts a nil []byte to
// SQL NULL, and `mbid > NULL` matches no row at all.
func TestArtifactKeyBindsInTheArtifactsOwnEncoding(t *testing.T) {
raw := mbidBytes(artA)
for _, tt := range []struct {
name string
key artifactKey
want []byte
}{
{"unset", nil, []byte{}},
{"set", artifactKey(raw), raw},
} {
t.Run("bytes/"+tt.name, func(t *testing.T) {
got, ok := tt.key.bind(false).([]byte)
if !ok {
t.Fatalf("bind(false) = %T, want []byte", tt.key.bind(false))
}
if got == nil {
t.Fatal("bound to SQL NULL, which matches no row")
}
if !bytes.Equal(got, tt.want) {
t.Errorf("bind(false) = %x, want %x", got, tt.want)
}
})
}
// The dashed form is what an artifact published before the storage
// change carries, and it has to compare as text against text.
if got := artifactKey(nil).bind(true); got != "" {
t.Errorf("bind(true) on an unset cursor = %#v, want an empty string", got)
}
if got := artifactKey(artA).bind(true); got != artA {
t.Errorf("bind(true) = %#v, want %q", got, artA)
}
}
+266
View File
@@ -0,0 +1,266 @@
package explore
import (
"context"
"database/sql"
"io"
"os"
"path/filepath"
"strings"
"testing"
"yellowjacket/backend/database"
)
// Import of the artifact we actually publish, as a client imports it.
//
// Every other test here builds a fixture, and a fixture is a second
// description of the storage format that can be wrong in the same
// direction as the code reading it. That is how #258 shipped: the
// importer positioned its batch walk with a Go `string` cursor against
// the artifact's 16-byte `mbid` column, and SQLite neither coerces
// between TEXT and BLOB nor complains about the comparison — so the walk
// merged nothing and never advanced, and no install could finish its
// first index build. The fixture that guards the walk writes the old
// text encoding; the only compact fixture is one row, below the batch
// size, so the bound query never ran. Both passed throughout.
//
// So this one takes the published file and runs the client's own path
// over it — checksum, decompress, merge — and asserts that what the
// artifact holds is what the client ends up with.
//
// It skips without the path, so an ordinary test run pays nothing for
// it, and the publish job is where it is meant to run:
//
// YJ_CORE_INDEX_ARTIFACT=/tmp/core-index.db.zst \
// go test -tags indexbuild -run TestImportPublishedArtifact \
// ./backend/explore/
//
// The indexbuild tag is not incidental: that job's container has no GTK,
// and the default tag set links the app through Wails.
// publishedArtifactEnv points at the published artifact: the compressed
// core-index.db.zst, or the unpacked core-index.db.
const publishedArtifactEnv = "YJ_CORE_INDEX_ARTIFACT"
// artifactTotals is the pair this test compares across the boundary.
//
// Rows is the whole point — a merge that lands fewer of them than the
// artifact declares is a catalog that looks populated and is missing
// things nobody can name — and popularity is the half whose absence was
// reported when it happened, because it arrives only through the merge.
type artifactTotals struct {
rows int
withListen int
}
func TestImportPublishedArtifact(t *testing.T) {
published := strings.TrimSpace(os.Getenv(publishedArtifactEnv))
if published == "" {
t.Skipf("set %s=<core-index.db.zst> to import the published artifact",
publishedArtifactEnv)
}
if _, err := os.Stat(published); err != nil {
t.Fatalf("%s: %v", publishedArtifactEnv, err)
}
// A file-backed database rather than NewTestDB's in-memory one: the
// artifact is ~135MB and a million rows, which is not a thing to hold
// in RAM inside a test. YJ_HOME is how NewDB is pointed somewhere
// disposable, and going through NewDB means this is the constructor,
// the schema and the read pool the app itself opens.
//
// Nothing closes it, because nothing can: `DB` has no Close and the
// app's handles are process-lifetime by design. The directory is
// unlinked at cleanup and the file goes with it.
t.Setenv("YJ_HOME", t.TempDir())
db, err := database.NewDB(testLogger())
if err != nil {
t.Fatalf("open database: %v", err)
}
si := NewSearchIndex(db, nil, nil, testLogger())
// The checksum the publisher shipped, if it shipped one. Every
// client verifies it and refuses the artifact when it does not
// match, so a wrong one breaks Explore for everyone who has not
// already imported — and nothing else would see it, because the
// comparison is between two files only the publisher has.
if want, ok := publishedChecksum(published); ok {
got, err := fileSHA256(published)
if err != nil {
t.Fatalf("checksum the artifact: %v", err)
}
if got != want {
t.Errorf("published artifact hashes to %s, but its .sha256 says %s",
got, want)
}
}
unpacked := unpackPublishedArtifact(t, si, published)
want, err := artifactTotalsOf(unpacked)
if err != nil {
t.Fatalf("count the artifact's rows: %v", err)
}
if err := si.importCoreArtifact(context.Background(), unpacked); err != nil {
t.Fatalf("importCoreArtifact: %v", err)
}
got, err := indexTotalsOf(db)
if err != nil {
t.Fatalf("count the index's rows: %v", err)
}
if got.rows != want.rows {
t.Errorf("merged %d rows, but the artifact holds %d",
got.rows, want.rows)
}
if got.withListen != want.withListen {
t.Errorf("%d rows carry a listen count, but the artifact holds %d of them",
got.withListen, want.withListen)
}
// The FTS index is rebuilt from the table once the merge is done, and
// it is what search actually reads: a merge that lands without it
// leaves Explore silently matching nothing, which is the state #258
// produced by a different route.
var indexed int
if err := db.QueryRowWriter(
"SELECT COUNT(*) FROM explore_index_fts",
).Scan(&indexed); err != nil {
t.Fatalf("count the FTS index: %v", err)
}
if indexed != got.rows {
t.Errorf("FTS index holds %d rows against the table's %d",
indexed, got.rows)
}
// And one row read back through the app's own path, which is the
// other direction of every conversion the merge makes: a byte MBID
// out of the table, the app's dashed form, and back in as a lookup.
var raw []byte
if err := db.QueryRowWriter(`
SELECT mbid FROM explore_index
WHERE entity_type = 1 /* artist */ AND popularity > 0
ORDER BY popularity DESC LIMIT 1`).Scan(&raw); err != nil {
t.Fatalf("read a stored mbid: %v", err)
}
dashed, err := mbidFromBytes(raw)
if err != nil {
t.Fatalf("the stored mbid is not one: %v", err)
}
artist := si.LookupArtistByMBID(dashed)
if artist == nil {
t.Fatalf("the artifact's most popular artist %s does not look up", dashed)
}
if artist.Popularity == 0 {
t.Errorf("artist %s came back with no popularity", dashed)
}
}
// publishedChecksum reads the sha256 the publisher wrote beside the
// artifact, in `sha256sum` output form. A missing file is not a
// failure: it is only there when the artifact came from the publish job.
func publishedChecksum(path string) (string, bool) {
body, err := os.ReadFile(path + ".sha256")
if err != nil {
return "", false
}
sum := strings.TrimSpace(string(body))
if i := strings.IndexAny(sum, " \t"); i > 0 {
sum = sum[:i]
}
if len(sum) != 64 {
return "", false
}
return strings.ToLower(sum), true
}
// unpackPublishedArtifact returns a path to the unpacked database,
// going through the client's own decompression when it is handed the
// compressed file that is actually published.
func unpackPublishedArtifact(t *testing.T, si *SearchIndex, path string) string {
t.Helper()
if strings.HasSuffix(path, ".db") {
return path
}
// Copied into the test's own directory first: decompress writes
// beside the compressed file, and the publisher's directory is not
// this test's to write in.
staging := t.TempDir()
dst := filepath.Join(staging, coreArtifactFile)
src, err := os.Open(path)
if err != nil {
t.Fatalf("open the published artifact: %v", err)
}
defer func() { _ = src.Close() }()
out, err := os.Create(dst)
if err != nil {
t.Fatalf("create a staging copy: %v", err)
}
if _, err := io.Copy(out, src); err != nil {
t.Fatalf("copy the published artifact: %v", err)
}
if err := out.Close(); err != nil {
t.Fatalf("close the staging copy: %v", err)
}
fetcher := &artifactFetcher{si: si, stagingDir: staging}
if err := fetcher.decompress(context.Background()); err != nil {
t.Fatalf("decompress the published artifact: %v", err)
}
return fetcher.unpackedPath()
}
// artifactTotalsOf counts what an artifact file holds, read directly so
// the numbers do not depend on anything the client does.
func artifactTotalsOf(path string) (artifactTotals, error) {
db, err := sql.Open("sqlite", "file:"+path+"?mode=ro")
if err != nil {
return artifactTotals{}, err
}
defer func() { _ = db.Close() }()
var totals artifactTotals
err = db.QueryRow(`SELECT COUNT(*), COALESCE(SUM(popularity > 0), 0)
FROM explore_index`).Scan(&totals.rows, &totals.withListen)
if err != nil {
return artifactTotals{}, err
}
return totals, nil
}
// indexTotalsOf counts what the client ended up with.
func indexTotalsOf(db *database.DB) (artifactTotals, error) {
var totals artifactTotals
err := db.QueryRowWriter(`SELECT COUNT(*), COALESCE(SUM(popularity > 0), 0)
FROM explore_index`).Scan(&totals.rows, &totals.withListen)
return totals, err
}
+17 -11
View File
@@ -383,9 +383,13 @@ func (si *SearchIndex) topByPopularity(
// MusicBrainz IDs, so this never touches the library tables and asks
// one query rather than one per artist.
//
// The artists are drawn most-popular-owned-album first, so a large
// library's pool is the part of it the user is likeliest to recognise
// rather than whichever artists sort first.
// **The row is drawn at random, and that is the whole point of it.**
// Ordered by popularity it was a second leaderboard: the same handful of
// big names appeared every time the page opened, which is not what "you
// own one album by these artists" is saying. The pool is still bounded
// to `pool` artists — a 4 000-artist library does not need all of them
// ranked — but which of them, and which of their albums, is `RANDOM()`,
// so the shelf is a different sample each visit.
func (si *SearchIndex) unownedAlbumsBySinglyOwnedArtists(
ctx context.Context,
pool, limit int,
@@ -396,15 +400,17 @@ func (si *SearchIndex) unownedAlbumsBySinglyOwnedArtists(
WHERE entity_type = 2 /* release_group */
AND in_library = 0
AND artist_mbid IN (
SELECT artist_mbid FROM explore_index
WHERE entity_type = 2 /* release_group */
AND in_library = 1
AND artist_mbid != x''
GROUP BY artist_mbid
HAVING COUNT(*) = 1
ORDER BY MAX(popularity) DESC
SELECT artist_mbid FROM (
SELECT artist_mbid FROM explore_index
WHERE entity_type = 2 /* release_group */
AND in_library = 1
AND artist_mbid != x''
GROUP BY artist_mbid
HAVING COUNT(*) = 1
)
ORDER BY RANDOM()
LIMIT ?)
ORDER BY popularity DESC
ORDER BY RANDOM()
LIMIT ?`,
pool, limit,
))
+6
View File
@@ -3,6 +3,7 @@ package explore
import (
"context"
"log/slog"
"sort"
"testing"
"yellowjacket/backend/database"
@@ -202,6 +203,11 @@ func TestShelves_MoreFromOwnedNeedsExactlyOneOwnedAlbum(t *testing.T) {
titles = append(titles, album.Title)
}
// The row is a random sample, so the *set* is what is asserted and
// not the order — see `unownedAlbumsBySinglyOwnedArtists` for why
// the ordering was given up.
sort.Strings(titles)
if len(titles) != 2 || titles[0] != "Second" || titles[1] != "Third" {
t.Fatalf("albums = %v, want [Second Third]", titles)
}
+17
View File
@@ -60,6 +60,23 @@ const PHONE = { width: 424, height: 439 };
const DESKTOP = { width: 1100, height: 800 };
test.describe('background jobs on a phone', () => {
/**
* **State a spec stages is the spec's to clear.** `/__test/emit` writes
* to a store nothing resets, so the event that staged a job is the
* event that clears it — `JobStore` replaces its whole list from every
* snapshot, so `testctl` needs no special case.
*
* **Measured on #168: this does not currently outlive the page.** Every
* test gets a fresh page, and `JobStore.init()` refetches `GetJobs()`
* from a backend registry that `/__test/emit` never writes to, so the
* staged job is gone before the next spec starts. Ownership is stated
* rather than a live leak repaired — the leak needs a page that
* survives its own spec, and there is none today.
*/
test.afterEach(async ({ testctl }) => {
await testctl.emit('JobsChanged', []);
});
test('are shown in the band, without opening anything', async ({
app,
testctl,
+2 -2
View File
@@ -171,8 +171,8 @@ test.describe('search on a phone', () => {
// Attached, not visible: `wa-dialog`'s host is `display: contents`,
// so the element carrying the testid always reports hidden — what
// is visible is the native `<dialog>` inside it. That awkwardness
// is written down in CLAUDE.md and is why the assertion that this
// is really up is the role query below.
// is why the assertion that this is really up is the role query
// below.
await expect(dialog).toBeAttached();
// Named, which `getByRole` can answer and the a11y snapshot cannot
+16
View File
@@ -102,6 +102,22 @@ const collapsed = (page: Page) =>
}));
test.describe('the top bar fits the window', () => {
/**
* **State a spec stages is the spec's to clear** (#168). `/__test/emit`
* writes to a store nothing resets, and this file stages the widest job
* in the app, so it puts it back — with the same event, since the store
* replaces its whole list from every snapshot.
*
* **Measured: it does not currently outlive the page.** Every test gets
* a fresh page and `JobStore.init()` refetches `GetJobs()` from a
* backend registry `/__test/emit` never writes to, so nothing is being
* repaired here; the rule is stated because it costs one line and the
* leak would need only one spec that keeps a page alive.
*/
test.afterEach(async ({ testctl }) => {
await testctl.emit('JobsChanged', []);
});
/**
* The phone's answer, which is not "it fits" (#57).
*
@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 320 512"><!--! Font Awesome Free 7.3.1 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License) Copyright 2026 Fonticons, Inc. --><path fill="currentColor" d="M9.4 233.4c-12.5 12.5-12.5 32.8 0 45.3l192 192c12.5 12.5 32.8 12.5 45.3 0s12.5-32.8 0-45.3L77.3 256 246.6 86.6c12.5-12.5 12.5-32.8 0-45.3s-32.8-12.5-45.3 0l-192 192z"/></svg>

After

Width:  |  Height:  |  Size: 476 B

@@ -60,6 +60,7 @@ import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import { dictByName } from '@utils/binding';
import type { TrackDetails } from '@components/track-details/track-details.js';
import { showTrackDetailsForPath } from '@utils/track-details-opener.js';
import { openMusicBrainz } from '@utils/external-link';
import '@components/playlist-picker/playlist-picker.js';
import {
ICON_CAN_REQUEST,
@@ -2942,7 +2943,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
if (!track?.mbid) return;
window.open(`https://musicbrainz.org/recording/${track.mbid}`, '_blank', 'noopener');
openMusicBrainz(`/recording/${track.mbid}`);
}
/**
@@ -4,6 +4,8 @@ import { customElement, property, state, query } from 'lit/decorators.js';
import { classMap } from 'lit/directives/class-map.js';
import { designTokens } from '../../styles/tokens.css';
import { backButton } from '../../styles/back-button.css';
import { albumCardStyles } from '../../styles/album-card.css';
import '../scroll-row/scroll-row.js';
import {
LookupArtist,
BrowseReleaseGroups,
@@ -46,11 +48,8 @@ import {
libraryStatusFor,
toggleRequest,
} from '@utils/library-status';
import {
isOwned,
ownershipLabel,
unownedStyles,
} from '@utils/ownership';
import { isOwned, ownershipLabel } from '@utils/ownership';
import { openMusicBrainz } from '@utils/external-link';
import { completenessStore } from '@store/completeness-store';
import '../catalog-scope-notice/catalog-scope-notice.js';
import type { CatalogScope } from '../catalog-scope-notice/catalog-scope-notice.js';
@@ -62,6 +61,7 @@ import {
ContextMenuController,
contextMenuStyles,
isContextMenuKey,
MenuKeyboard,
} from '@utils/context-menu-controller.js';
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
@@ -187,11 +187,11 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
@state() private topReleasesExpanded = false;
private topSectionStacked = false;
private topSectionObserver?: ResizeObserver;
@state() private expandedDiscoGroups = new Set<string>();
/** Number of album cards that fit in one row of the discography grid. */
@state() private discoRowSize = 5;
private discoObserver?: ResizeObserver;
@state() private similarExpanded = false;
/** Whether the Play button's Shuffle dropdown is up. */
@state() private playMenuOpen = false;
private playMenuKeyboard = new MenuKeyboard(() => this.closePlayMenu());
private playOutsideAttached = false;
/* ── Release prefetch ── */
@@ -221,6 +221,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
@query('#context-menu')
private contextMenuPopup!: MenuSurface;
@query('.play-menu-button')
private playMenuButton?: HTMLButtonElement;
@query('#artist-play-menu')
private playMenuPanel?: HTMLElement;
@query('#playlist-submenu')
private playlistSubmenuPopup?: WaPopup;
@@ -271,7 +277,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
backButton,
exploreLinkStyles,
contextMenuStyles,
unownedStyles,
albumCardStyles,
css`
:host {
display: flex;
@@ -319,10 +325,45 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
object-fit: cover;
}
.artist-follow {
.artist-actions {
display: flex;
align-items: center;
gap: 8px;
flex-wrap: wrap;
margin-top: 10px;
}
/* The Play button and its caret are one control, so they
are one box: no gap between them, and the caret carries
the same filled appearance as the button it extends. */
.play-split {
display: inline-flex;
align-items: stretch;
}
.play-menu-button {
display: inline-flex;
align-items: center;
justify-content: center;
width: 28px;
padding: 0;
border: none;
border-left: 1px solid rgba(0, 0, 0, 0.25);
border-radius: 0 6px 6px 0;
background: var(--yj-accent, #ffd43b);
color: var(--yj-accent-fg, #000);
cursor: pointer;
}
.play-menu-button:hover {
filter: brightness(1.1);
}
.play-menu-button:focus-visible {
outline: 2px solid var(--yj-accent-text, #ffd43b);
outline-offset: 2px;
}
.artist-info {
display: flex;
flex-direction: column;
@@ -331,7 +372,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
}
.artist-title {
font-size: 24px;
font-size: 28px;
font-weight: 700;
color: var(--yj-text-primary, #fff);
white-space: nowrap;
@@ -359,6 +400,13 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
flex-wrap: wrap;
}
/* The listen count is a headline number, not metadata, so
it sits a size above the type/country line. */
.artist-listens {
font-size: var(--yj-text-lg);
color: var(--yj-text-secondary, #b3b3b3);
}
.meta-separator {
opacity: 0.4;
}
@@ -446,23 +494,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
outline-offset: -2px;
}
.artist-play-actions {
margin-top: 10px;
display: flex;
gap: 8px;
align-items: center;
flex-wrap: wrap;
}
.track-rank {
width: 24px;
text-align: right;
color: var(--yj-text-tertiary, #888);
font-size: var(--yj-text-md);
font-variant-numeric: tabular-nums;
flex-shrink: 0;
}
.track-art {
width: 32px;
height: 32px;
@@ -489,6 +520,52 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
opacity: 0.5;
}
/* Play where you own the track, the request badge where you
do not — over the artwork rather than at the end of the
row, where it was a badge beside a row you can already
double-click. */
.track-art-overlay {
position: absolute;
inset: 0;
display: flex;
align-items: center;
justify-content: center;
border-radius: 4px;
background: rgba(0, 0, 0, 0.55);
visibility: hidden;
opacity: 0;
transition: opacity 0.15s ease, visibility 0.15s ease;
}
.track-art-play {
display: flex;
align-items: center;
justify-content: center;
padding: 0;
border: none;
background: none;
color: #fff;
font-size: 14px;
cursor: pointer;
}
@media (hover: hover) and (pointer: fine) {
.track-item:hover .track-art-overlay,
.track-item:focus-within .track-art-overlay {
visibility: visible;
opacity: 1;
}
}
/* No hover means no double-click either, so the overlay is
the only route to playing a top track and must be there. */
@media not all and (hover: hover) {
.track-art-overlay {
visibility: visible;
opacity: 1;
}
}
.track-info {
flex: 1;
min-width: 0;
@@ -522,7 +599,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
.track-item library-status-indicator {
flex-shrink: 0;
}
/* ── Top section (tracks + releases side-by-side) ── */
.top-section-wrapper {
container-type: inline-size;
@@ -754,8 +830,30 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
white-space: nowrap;
}
.top-release-meta library-status-indicator {
flex-shrink: 0;
.top-release-art .album-card-badge {
position: absolute;
top: 4px;
left: 4px;
z-index: 1;
display: flex;
visibility: hidden;
opacity: 0;
transition: opacity 0.15s ease, visibility 0.15s ease;
}
@media (hover: hover) and (pointer: fine) {
.top-release-card:hover .album-card-badge,
.top-release-card:focus-within .album-card-badge {
visibility: visible;
opacity: 1;
}
}
@media not all and (hover: hover) {
.top-release-art .album-card-badge {
visibility: visible;
opacity: 1;
}
}
@@ -773,150 +871,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
margin: 0;
}
.album-grid {
display: grid;
grid-template-columns: repeat(auto-fill, 140px);
gap: 16px;
}
.album-grid.collapsed {
grid-template-rows: 1fr;
overflow: hidden;
}
.disco-toggle {
display: flex;
align-items: center;
justify-content: center;
gap: 6px;
padding: 4px 10px;
margin-top: 4px;
border: none;
border-radius: 6px;
background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.06));
color: var(--yj-text-secondary, #b3b3b3);
font-size: var(--yj-text-xs);
cursor: pointer;
transition: background 0.15s ease, color 0.15s ease;
width: 100%;
}
.disco-toggle:hover {
background: var(--yj-bg-hover, rgba(255, 255, 255, 0.1));
color: var(--yj-text-primary, #fff);
}
.disco-toggle wa-icon {
font-size: 11px;
transition: transform 0.2s ease;
}
.disco-toggle[aria-expanded='true'] wa-icon {
transform: rotate(180deg);
}
.album-card {
display: flex;
flex-direction: column;
gap: 6px;
padding: 8px;
border-radius: 8px;
cursor: pointer;
transition: background 0.15s ease;
}
.album-card:hover {
background: var(
--yj-bg-overlay,
rgba(255, 255, 255, 0.06)
);
}
.album-card:active {
transform: scale(0.97);
}
.album-art-container {
width: 100%;
aspect-ratio: 1;
border-radius: 4px;
overflow: hidden;
flex-shrink: 0;
position: relative;
background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.06));
}
.album-art-container img {
width: 100%;
height: 100%;
object-fit: cover;
display: block;
border-radius: 4px;
}
.album-art-fallback {
display: flex;
align-items: center;
justify-content: center;
width: 100%;
height: 100%;
position: absolute;
inset: 0;
}
.album-art-fallback wa-icon {
color: var(--yj-text-tertiary, #888);
font-size: 24px;
opacity: 0.5;
}
.album-title {
font-weight: 500;
color: var(--yj-text-primary, #fff);
font-size: var(--yj-text-sm);
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}
.album-meta {
display: flex;
align-items: center;
justify-content: space-between;
gap: 6px;
color: var(--yj-text-tertiary, #888);
font-size: var(--yj-text-xs);
min-height: 20px;
}
.album-meta-text {
display: flex;
align-items: center;
gap: 6px;
min-width: 0;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
}
.album-meta library-status-indicator {
flex-shrink: 0;
margin-left: auto;
}
/* ── Similar artists ── */
.similar-row {
display: grid;
grid-template-columns: repeat(auto-fill, 140px);
gap: 16px;
overflow: hidden;
}
.similar-row.collapsed {
grid-template-rows: 1fr;
overflow: hidden;
}
.similar-artist-card {
display: flex;
flex-direction: column;
@@ -926,6 +881,9 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
border-radius: 8px;
cursor: pointer;
text-align: center;
width: 120px;
box-sizing: border-box;
flex-shrink: 0;
transition: background 0.15s ease;
}
@@ -1056,7 +1014,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
this.unsubSimilarReady?.();
if (this.discogFallbackTimer) clearTimeout(this.discogFallbackTimer);
this.topSectionObserver?.disconnect();
this.discoObserver?.disconnect();
this.detachPlayOutsideClose();
}
/**
@@ -1083,17 +1041,14 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
protected override firstUpdated() {
this.observeTopSectionWidth();
this.observeDiscoWidth();
}
protected override updated() {
// Re-attach observers if elements appeared after initial render.
// Re-attach the observer if the section appeared after initial
// render.
if (!this.topSectionObserver) {
this.observeTopSectionWidth();
}
if (!this.discoObserver) {
this.observeDiscoWidth();
}
}
/**
@@ -1126,32 +1081,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
this.topSectionObserver.observe(wrapper);
}
/**
* Watch the .content width and compute how many album cards
* fit in one row of the discography grid.
* Grid uses: repeat(auto-fill, minmax(140px, 1fr)) with 16px gap
* and album-card has 8px padding on each side.
*/
private observeDiscoWidth() {
const content = this.renderRoot.querySelector('.content');
if (!content) return;
const CARD_MIN = 140;
const GAP = 16;
this.discoObserver = new ResizeObserver((entries) => {
for (const entry of entries) {
const width = entry.contentBoxSize?.[0]?.inlineSize ?? entry.contentRect.width;
const cols = Math.max(1, Math.floor((width + GAP) / (CARD_MIN + GAP)));
if (cols !== this.discoRowSize) {
this.discoRowSize = cols;
}
}
});
this.discoObserver.observe(content);
}
/* ── Data Loading ── */
private async loadAllData() {
@@ -1862,6 +1791,8 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
} catch {
// No image — letter avatar stays.
}
return undefined;
}),
);
}
@@ -1999,6 +1930,65 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
}
}
/* ── Play / Shuffle split button ── */
/**
* Open the Play button's Shuffle dropdown.
*
* `page-header`'s overflow menu one control over: the same
* `MenuKeyboard`, the same document-level outside-close, and the
* same `menu-surface`, so the phone gets the bottom sheet rather
* than a popup that Chrome 113 clips.
*/
private togglePlayMenu = (): void => {
if (this.playMenuOpen) {
this.closePlayMenu();
return;
}
this.playMenuOpen = true;
void this.updateComplete.then(() => {
if (!this.playMenuOpen) return;
this.playMenuKeyboard.open(
this.playMenuPanel ?? null,
this.playMenuButton ?? null,
);
this.attachPlayOutsideClose();
});
};
private closePlayMenu = (): void => {
if (!this.playMenuOpen) return;
this.detachPlayOutsideClose();
this.playMenuKeyboard.close();
this.playMenuOpen = false;
};
private onPlayOutsideDown = (e: Event): void => {
if (e.composedPath().includes(this.playMenuPanel as EventTarget)) return;
if (e.composedPath().includes(this.playMenuButton as EventTarget)) return;
this.closePlayMenu();
};
private attachPlayOutsideClose(): void {
if (this.playOutsideAttached) return;
this.playOutsideAttached = true;
document.addEventListener('mousedown', this.onPlayOutsideDown, true);
}
private detachPlayOutsideClose(): void {
if (!this.playOutsideAttached) return;
this.playOutsideAttached = false;
document.removeEventListener('mousedown', this.onPlayOutsideDown, true);
}
/**
* File path for one top track, resolved by recording MBID — the
* same key `localId` was set from. Works whether or not the
@@ -2240,11 +2230,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
if (!release?.mbid) return;
window.open(
`https://musicbrainz.org/release-group/${release.mbid}`,
'_blank',
'noopener',
);
openMusicBrainz(`/release-group/${release.mbid}`);
}
private onContextMenuAction(
@@ -2358,7 +2344,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
if (!track?.recordingMbid) return;
window.open(`https://musicbrainz.org/recording/${track.recordingMbid}`, '_blank', 'noopener');
openMusicBrainz(`/recording/${track.recordingMbid}`);
}
/* ── Navigation ── */
@@ -2538,10 +2524,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
: nothing}
${this.renderArtistMeta()}
${this.artist?.popularity && this.artist.popularity > 0
? html`<span class="artist-meta">${formatListenCount(this.artist.popularity)} plays on ListenBrainz</span>`
? html`<span class="artist-listens">${formatListenCount(this.artist.popularity)} plays on ListenBrainz</span>`
: nothing}
${this.renderPlayLibraryAction()}
${this.renderFollowAction()}
<div class="artist-actions">
${this.renderPlayLibraryAction()}
${this.renderFollowAction()}
</div>
</div>
</div>
<div class="content">
@@ -2572,25 +2560,53 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
if (this.ownedLocalAlbumIds().length === 0) return nothing;
return html`
<div class="artist-play-actions">
<div class="play-split">
<wa-button
size="small"
appearance="filled"
data-testid="artist-play-library"
title="Play library tracks"
@click=${() => void this.playLibraryTracks(false)}
>
<wa-icon slot="start" name="play"></wa-icon>
Play library tracks
Play
</wa-button>
<wa-button
size="small"
appearance="outlined"
data-testid="artist-shuffle-library"
@click=${() => void this.playLibraryTracks(true)}
<menu-surface
placement="bottom-start"
.active=${this.playMenuOpen}
@menu-dismiss=${this.closePlayMenu}
>
<wa-icon slot="start" name="shuffle"></wa-icon>
Shuffle
</wa-button>
<button
slot="anchor"
class="play-menu-button"
type="button"
data-testid="artist-play-menu"
aria-label="More play options"
aria-haspopup="menu"
aria-expanded=${this.playMenuOpen ? 'true' : 'false'}
aria-controls="artist-play-menu"
@click=${this.togglePlayMenu}
>
<wa-icon name="chevron-down"></wa-icon>
</button>
<div
id="artist-play-menu"
class="context-menu-panel"
role="menu"
aria-label="Play options"
>
<wa-dropdown-item
data-testid="artist-shuffle-library"
@click=${() => {
this.closePlayMenu();
void this.playLibraryTracks(true);
}}
>
<wa-icon slot="icon" name="shuffle"></wa-icon>
Shuffle
</wa-dropdown-item>
</div>
</menu-surface>
</div>
`;
}
@@ -2786,25 +2802,27 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
const request = downloadStore.requestFor(this.artistMBID);
return html`
<div class="artist-follow">
<wa-button
size="small"
appearance=${request ? 'filled' : 'outlined'}
@click=${() => void this.toggleFollow(request?.id)}
>
<!-- This was bookmark-check, which is not in
names.txt and so has rendered the missing-icon
fallback — a circled question mark — on every
followed artist since it was written. A
backtick around that name would end this
template literal, which is why there is none. -->
<wa-icon
slot="start"
name=${request ? ICON_REQUESTED : ICON_CAN_REQUEST}
></wa-icon>
${request ? 'Following' : 'Follow for new releases'}
</wa-button>
</div>
<wa-button
size="small"
appearance=${request ? 'filled' : 'outlined'}
data-testid="artist-follow"
title=${request
? 'Following this artist'
: 'Follow this artist for new releases'}
@click=${() => void this.toggleFollow(request?.id)}
>
<!-- This was bookmark-check, which is not in
names.txt and so has rendered the missing-icon
fallback — a circled question mark — on every
followed artist since it was written. A
backtick around that name would end this
template literal, which is why there is none. -->
<wa-icon
slot="start"
name=${request ? ICON_REQUESTED : ICON_CAN_REQUEST}
></wa-icon>
${request ? 'Following' : 'Follow'}
</wa-button>
`;
}
@@ -2888,16 +2906,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
this.topReleasesExpanded = !this.topReleasesExpanded;
}
private toggleDiscoGroup(type: string) {
const next = new Set(this.expandedDiscoGroups);
if (next.has(type)) {
next.delete(type);
} else {
next.add(type);
}
this.expandedDiscoGroups = next;
}
private renderTopSection() {
const hasTracks = !this.loadingTracks && this.topTracks.length > 0;
const hasReleases = !this.loadingTopReleases && this.topReleaseGroups.length > 0;
@@ -2971,6 +2979,31 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
}} />`
: html`<wa-icon name="compact-disc"></wa-icon>`;
})()}
<!-- Over the artwork, not beside the
row: play where you own it, the
request badge where you do not. -->
<div class="track-art-overlay">
${owned
? html`<button
class="track-art-play"
type="button"
aria-label=${`Play ${t.trackName}`}
@click=${(e: Event) => {
e.stopPropagation();
void this.playTrack(t);
}}
>
<wa-icon name="play"></wa-icon>
</button>`
: html`<library-status-indicator
status=${libraryStatusFor(false, t.recordingMbid)}
entity-type="track"
label=${t.trackName}
request-mbid=${t.recordingMbid}
request-artist=${t.artistName ?? ''}
size="18"
></library-status-indicator>`}
</div>
</div>
<div class="track-info">
<div class="track-title">${trackLink(t.trackName, t.releaseName, t.releaseGroupMbid ?? '', t.recordingMbid)}</div>
@@ -2979,15 +3012,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
<span class="track-listens">
${formatListenCount(t.totalListenCount)} plays
</span>
${owned
? nothing
: html`<library-status-indicator
status=${libraryStatusFor(false, t.recordingMbid)}
entity-type="track"
label=${t.trackName}
request-mbid=${t.recordingMbid}
request-artist=${t.artistName ?? ''}
></library-status-indicator>`}
</div>
`;
})}
@@ -3093,6 +3117,18 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
<div class="album-art-fallback" style="${artURL ? 'display: none' : ''}">
<wa-icon name="compact-disc"></wa-icon>
</div>
<div class="album-card-badge">
<library-status-indicator
status=${badge.status}
owned=${badge.owned}
expected=${badge.expected}
entity-type="album"
label=${rg.title}
request-mbid=${rg.releaseGroupMbid}
request-artist=${this.artist?.name ?? ''}
size="21"
></library-status-indicator>
</div>
</div>
<div class="top-release-text">
<div class="top-release-title" title="${rg.title}">
@@ -3102,18 +3138,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
<div class="top-release-meta-text">
${rg.date ? html`<span>${extractYear(rg.date)}</span>` : nothing}
</div>
${badge.status === 'in-library'
? nothing
: html`<library-status-indicator
status=${badge.status}
owned=${badge.owned}
expected=${badge.expected}
entity-type="album"
label=${rg.title}
request-mbid=${rg.releaseGroupMbid}
request-artist=${this.artist?.name ?? ''}
size="18"
></library-status-indicator>`}
</div>
</div>
</div>
@@ -3164,37 +3188,16 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
<section>
<h3 class="section-header">Discography</h3>
${groups.map(
(g) => {
const isExpanded = this.expandedDiscoGroups.has(g.type);
const rowSize = this.discoRowSize;
const showToggle = g.items.length > rowSize;
const visibleItems = isExpanded ? g.items : g.items.slice(0, rowSize);
return html`
<div class="disco-group">
<h4 class="disco-type-header">
${g.type === 'Other' ? 'Other Releases' : g.type.endsWith('s') ? g.type : `${g.type}s`}
</h4>
<div class="album-grid">
${visibleItems.map((rg) => this.renderAlbumCard(rg))}
</div>
${showToggle
? html`
<button
class="disco-toggle"
aria-expanded="${isExpanded}"
@click=${() => this.toggleDiscoGroup(g.type)}
>
${isExpanded
? 'Show less'
: `Show all ${g.items.length}`}
<wa-icon name="chevron-down"></wa-icon>
</button>
`
: nothing}
</div>
`;
},
(g) => html`
<div class="disco-group">
<h4 class="disco-type-header">
${g.type === 'Other' ? 'Other Releases' : g.type.endsWith('s') ? g.type : `${g.type}s`}
</h4>
<scroll-row>
${g.items.map((rg) => this.renderAlbumCard(rg))}
</scroll-row>
</div>
`,
)}
</section>
`;
@@ -3234,23 +3237,25 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
<div class="album-art-fallback" style="${artURL ? 'display: none' : ''}">
<wa-icon name="compact-disc"></wa-icon>
</div>
<div class="album-card-badge">
<library-status-indicator
status=${badge.status}
owned=${badge.owned}
expected=${badge.expected}
entity-type="album"
label=${rg.title}
request-mbid=${rg.mbid}
request-artist=${this.artist?.name ?? ''}
size="23"
></library-status-indicator>
</div>
</div>
<div class="album-title" title="${rg.title}">${rg.title}</div>
<div class="album-artist">${rg.artistCredit ?? ''}</div>
<div class="album-meta">
<div class="album-meta-text">
${year ? html`<span>${year}</span>` : nothing}
</div>
${badge.status === 'in-library'
? nothing
: html`<library-status-indicator
status=${badge.status}
owned=${badge.owned}
expected=${badge.expected}
entity-type="album"
label=${rg.title}
request-mbid=${rg.mbid}
request-artist=${this.artist?.name ?? ''}
></library-status-indicator>`}
</div>
</div>
`;
@@ -3266,15 +3271,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
// Cap the similar-artists list at 10 to avoid a very long list.
const maxSimilar = 10;
const artists = this.similarArtists.slice(0, maxSimilar);
const showToggle = artists.length > this.discoRowSize;
const collapsed = !this.similarExpanded && showToggle;
const visible = collapsed ? artists.slice(0, this.discoRowSize) : artists;
return html`
<section>
<h3 class="section-header">Similar Artists</h3>
<div class="similar-row ${collapsed ? 'collapsed' : ''}">
${visible.map((a) => {
<scroll-row>
${artists.map((a) => {
const imgURL = this.similarImageURLs.get(a.artistMbid);
return html`
<div
@@ -3313,21 +3315,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
</div>
`;
})}
</div>
${showToggle
? html`
<button
class="disco-toggle"
aria-expanded="${this.similarExpanded}"
@click=${() => { this.similarExpanded = !this.similarExpanded; }}
>
${this.similarExpanded
? 'Show less'
: `Show all ${artists.length}`}
<wa-icon name="chevron-down"></wa-icon>
</button>
`
: nothing}
</scroll-row>
</section>
`;
}
@@ -1,10 +1,7 @@
import { avatarBackground } from '@utils/avatar-color';
import { albumBadgeFor, libraryStatusFor } from '@utils/library-status';
import {
isOwned,
ownershipLabel,
unownedStyles,
} from '@utils/ownership';
import { isOwned, ownershipLabel } from '@utils/ownership';
import { openMusicBrainz } from '@utils/external-link';
import { completenessStore } from '@store/completeness-store';
import { downloadStore } from '@store/download-store';
import { LitElement, html, css, nothing } from 'lit';
@@ -13,6 +10,8 @@ import { classMap } from 'lit/directives/class-map.js';
import '@components/page-header/page-header';
import { designTokens } from '../../styles/tokens.css';
import { srOnly } from '../../styles/sr-only.css';
import { albumCardStyles } from '../../styles/album-card.css';
import '../scroll-row/scroll-row.js';
import { SearchLocal, SearchLyrics, GetThumbnail, GetThumbnails, GetArtistImageURL, GetArtistImagesCachedPaths, GetExploreShelves, RecordSearchClick } from '@go/explore/service.js';
import { GetFilePathsByAlbums, GetFilePathsByRecordingMBIDs } from '@go/library/library.js';
import { EventsOn } from '@runtime/runtime';
@@ -253,7 +252,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
srOnly,
exploreLinkStyles,
contextMenuStyles,
unownedStyles,
albumCardStyles,
css`
:host {
display: block;
@@ -529,21 +528,9 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
line-height: 1.5;
}
/* ── Horizontal scroll rows ── */
.horizontal-row {
display: flex;
gap: 12px;
overflow-x: auto;
padding-bottom: 4px;
scrollbar-width: none;
}
.horizontal-row::-webkit-scrollbar {
display: none;
}
/* ── Top result cards ── */
/* ── Artist cards ── */
/* Fixed width, for the reason the album card is: a range
means two cards in one row are different sizes. */
.artist-card {
display: flex;
flex-direction: column;
@@ -552,8 +539,8 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
padding: 10px;
border-radius: 8px;
cursor: pointer;
min-width: 100px;
max-width: 120px;
width: 120px;
box-sizing: border-box;
flex-shrink: 0;
text-align: center;
transition: background 0.15s ease;
@@ -624,115 +611,6 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
font-size: var(--yj-text-xs);
}
/* ── Album cards ── */
.album-card {
display: flex;
flex-direction: column;
gap: 6px;
padding: 8px;
border-radius: 8px;
cursor: pointer;
min-width: 130px;
max-width: 150px;
flex-shrink: 0;
transition: background 0.15s ease;
}
.album-card:hover {
background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.06));
}
.album-card:active {
transform: scale(0.97);
}
.album-art-container {
width: 100%;
aspect-ratio: 1;
border-radius: 4px;
overflow: hidden;
background: linear-gradient(
135deg,
var(--yj-bg-overlay, #404040) 0%,
var(--yj-bg-surface, #282828) 100%
);
display: flex;
align-items: center;
justify-content: center;
position: relative;
}
.album-art-container img {
width: 100%;
height: 100%;
object-fit: cover;
display: block;
}
.album-art-fallback {
display: flex;
align-items: center;
justify-content: center;
width: 100%;
height: 100%;
position: absolute;
inset: 0;
}
.album-art-fallback wa-icon {
color: var(--yj-text-tertiary, #888);
font-size: 24px;
opacity: 0.5;
}
.album-title {
font-weight: 500;
color: var(--yj-text-primary, #fff);
font-size: var(--yj-text-sm);
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}
.album-artist {
color: var(--yj-text-tertiary, #888);
font-size: var(--yj-text-xs);
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}
.album-meta {
display: flex;
align-items: center;
justify-content: space-between;
gap: 6px;
color: var(--yj-text-tertiary, #888);
font-size: var(--yj-text-xs);
min-height: 20px;
}
.album-meta-text {
display: flex;
align-items: center;
gap: 6px;
min-width: 0;
overflow: hidden;
}
.album-meta library-status-indicator {
flex-shrink: 0;
margin-left: auto;
}
.type-badge {
background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.08));
padding: 1px 6px;
border-radius: 3px;
font-size: 10px;
white-space: nowrap;
}
/* ── Track list ── */
.track-list {
display: flex;
@@ -753,7 +631,6 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
cursor: pointer;
}
.album-card:focus-visible,
.track-item:focus-visible {
outline: 2px solid var(--yj-accent-text, #ffd43b);
outline-offset: -2px;
@@ -1371,7 +1248,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
const entity = target.kind === 'album' ? 'release-group' : 'recording';
window.open(`https://musicbrainz.org/${entity}/${target.mbid}`, '_blank', 'noopener');
openMusicBrainz(`/${entity}/${target.mbid}`);
}
private renderExploreContextMenu() {
@@ -1662,6 +1539,8 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
} catch {
// No image — leave empty string.
}
return undefined;
}),
);
@@ -2122,7 +2001,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
${subtitle
? html`<p class="section-reason">${subtitle}</p>`
: nothing}
<div class="horizontal-row">
<scroll-row>
${artists.map((a) => {
const owned = isOwned(a);
const name = a.englishName || a.name;
@@ -2171,7 +2050,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
</div>
`;
})}
</div>
</scroll-row>
</section>
`;
}
@@ -2187,7 +2066,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
${subtitle
? html`<p class="section-reason">${subtitle}</p>`
: nothing}
<div class="horizontal-row">
<scroll-row>
${releaseGroups.map((rg) => {
const artURL = this.thumbnailCache.get(rg.mbid) || '';
const year = extractYear(rg.firstReleaseDate);
@@ -2249,6 +2128,18 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
>
<wa-icon name="compact-disc"></wa-icon>
</div>
<div class="album-card-badge">
<library-status-indicator
status=${badge.status}
owned=${badge.owned}
expected=${badge.expected}
entity-type="album"
label=${rg.title}
request-mbid=${rg.mbid}
request-artist=${rg.artistCredit ?? ''}
size="23"
></library-status-indicator>
</div>
</div>
<div class="album-title" title="${rg.title}">
${rg.title}
@@ -2256,29 +2147,18 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
<div class="album-artist">${creditLink(creditStore.credits(rg.mbid), rg.artistCredit, rg.artistMbid ?? '')}</div>
<div class="album-meta">
<div class="album-meta-text">
${year ? html`<span>${year}</span>` : nothing}
${rg.primaryType
? html`<span class="type-badge"
>${rg.primaryType}</span
>`
: nothing}
${year ? html`<span>${year}</span>` : nothing}
</div>
${badge.status === 'in-library'
? nothing
: html`<library-status-indicator
status=${badge.status}
owned=${badge.owned}
expected=${badge.expected}
entity-type="album"
label=${rg.title}
request-mbid=${rg.mbid}
request-artist=${rg.artistCredit ?? ''}
></library-status-indicator>`}
</div>
</div>
`;
})}
</div>
</scroll-row>
</section>
`;
}
+5 -12
View File
@@ -12,6 +12,7 @@ import { libraryStore } from '@store/library-store';
import { EventsOn } from '@runtime/runtime';
import { Events } from '../../events';
import '@components/page-header/page-header';
import '../scroll-row/scroll-row.js';
import { designTokens } from '../../styles/tokens.css';
import { ViewLifecycleMixin } from '../../utils/view-lifecycle';
@@ -99,16 +100,6 @@ export class HomeView extends ViewLifecycleMixin(LitElement) {
color: var(--yj-text-tertiary, #888);
}
.row {
display: grid;
grid-auto-flow: column;
grid-auto-columns: 160px;
gap: 14px;
overflow-x: auto;
padding-bottom: 6px;
scrollbar-width: thin;
}
.card {
background: none;
border: none;
@@ -117,6 +108,8 @@ export class HomeView extends ViewLifecycleMixin(LitElement) {
cursor: pointer;
color: inherit;
display: block;
width: 160px;
flex-shrink: 0;
}
.art {
@@ -336,9 +329,9 @@ export class HomeView extends ViewLifecycleMixin(LitElement) {
<span class="shelf-title">${shelf.title}</span>
</div>
<p class="shelf-sub">${shelf.subtitle}</p>
<div class="row">
<scroll-row>
${(shelf.albums ?? []).map((album) => this.renderCard(album))}
</div>
</scroll-row>
</section>
`;
}
@@ -0,0 +1,213 @@
import { LitElement, css, html } from 'lit';
import { customElement, query, state } from 'lit/decorators.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
/** How far one press moves the row — most of a screenful, not all of
* it, so the card that was at the edge stays as an anchor. */
const SCROLL_FRACTION = 0.8;
/**
* A horizontally scrolling row with arrow buttons.
*
* The shelves, the search results and (now) the artist page's
* discography and similar-artists rows are all "more than fits, scroll
* sideways". Until this existed the only way to see the rest was a
* mousewheel or a trackpad gesture, which is not an affordance — a
* mouse with no horizontal wheel simply could not reach the cards past
* the fold.
*
* It is a component rather than a rule on `.horizontal-row` for two
* reasons. The arrows are *state* — which way the row can still move —
* and that state has to be recomputed when the viewport resizes or a
* card arrives with its cover art; a stylesheet cannot do that. And
* every caller then gets the same arrows, the same reveal and the same
* keyboard labels without writing them again.
*
* **The arrows are `hidden`, not merely transparent, at the end they
* cannot move from** — a control that cannot act is worse than none,
* and an invisible one still holds a hit area and a tab stop. On a
* pointer device the pair fades in with the row's hover; where there is
* no hover they are always visible, because there is no other route to
* them there (a swipe is not an affordance a mouse-less keyboard user
* has either).
*
* The cards are light DOM children and stay in the *host's* shadow
* root, so the host's own `.album-card` / `.artist-card` styles apply
* unchanged — this component only owns the box they scroll inside.
*/
@customElement('scroll-row')
export class ScrollRow extends LitElement {
@query('.viewport') private viewport?: HTMLElement;
@state() private atStart = true;
@state() private atEnd = true;
@state() private overflowing = false;
private observer?: ResizeObserver;
static override styles = css`
:host {
display: block;
position: relative;
}
.viewport {
overflow-x: auto;
overflow-y: hidden;
scrollbar-width: none;
/* A swipe that reaches the row's end should not drag the
whole page sideways with it. */
overscroll-behavior-x: contain;
}
.viewport::-webkit-scrollbar {
display: none;
}
.track {
display: flex;
gap: 12px;
}
.arrow {
position: absolute;
top: 50%;
transform: translateY(-50%);
z-index: 2;
display: flex;
align-items: center;
justify-content: center;
width: 36px;
height: 36px;
padding: 0;
border-radius: 50%;
border: 1px solid var(--yj-border-subtle, rgba(255, 255, 255, 0.1));
background: var(--yj-bg-elevated, #343a40);
color: var(--yj-text-primary, #fff);
cursor: pointer;
opacity: 0;
transition: opacity 0.15s ease;
}
.arrow[hidden] {
display: none;
}
.arrow.prev {
left: 4px;
}
.arrow.next {
right: 4px;
}
.arrow:hover {
background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.12));
}
.arrow:focus-visible {
outline: 2px solid var(--yj-accent, #ffd43b);
outline-offset: 2px;
}
@media (hover: hover) and (pointer: fine) {
:host(:hover) .arrow,
.arrow:focus-visible {
opacity: 1;
}
}
@media not all and (hover: hover) {
.arrow {
opacity: 1;
}
}
`;
override firstUpdated(): void {
const viewport = this.viewport;
if (!viewport) return;
this.observer = new ResizeObserver(() => this.measure());
this.observer.observe(viewport);
// The track's own size is what changes when a card arrives with
// its cover art, and a ResizeObserver on the viewport alone
// never fires for that.
const track = viewport.firstElementChild;
if (track) this.observer.observe(track);
this.measure();
}
override disconnectedCallback(): void {
super.disconnectedCallback();
this.observer?.disconnect();
this.observer = undefined;
}
private measure(): void {
const viewport = this.viewport;
if (!viewport) return;
this.overflowing = viewport.scrollWidth > viewport.clientWidth + 1;
this.atStart = viewport.scrollLeft <= 1;
this.atEnd =
viewport.scrollLeft + viewport.clientWidth >=
viewport.scrollWidth - 1;
}
private onScroll = (): void => this.measure();
private scrollStep(direction: -1 | 1): void {
const viewport = this.viewport;
if (!viewport) return;
viewport.scrollBy({
left: direction * viewport.clientWidth * SCROLL_FRACTION,
behavior: 'smooth',
});
}
override render() {
const showPrev = this.overflowing && !this.atStart;
const showNext = this.overflowing && !this.atEnd;
return html`
<button
class="arrow prev"
type="button"
aria-label="Scroll left"
?hidden=${!showPrev}
@click=${() => this.scrollStep(-1)}
>
<wa-icon name="chevron-left"></wa-icon>
</button>
<div class="viewport" @scroll=${this.onScroll}>
<div class="track"><slot></slot></div>
</div>
<button
class="arrow next"
type="button"
aria-label="Scroll right"
?hidden=${!showNext}
@click=${() => this.scrollStep(1)}
>
<wa-icon name="chevron-right"></wa-icon>
</button>
`;
}
}
declare global {
interface HTMLElementTagNameMap {
'scroll-row': ScrollRow;
}
}
@@ -1,6 +1,7 @@
import { LitElement, html, css, nothing } from 'lit';
import { customElement, property } from 'lit/decorators.js';
import { designTokens } from '../../styles/tokens.css';
import '../scroll-row/scroll-row.js';
import type * as explore from '@go/explore/models.js';
import {
GetArtistImageURL,
@@ -15,7 +16,6 @@ import { albumBadgeFor, libraryStatusFor } from '../../utils/library-status';
import {
isOwned,
ownershipLabel,
unownedStyles,
type OwnableKind,
} from '../../utils/ownership';
import { completenessStore } from '../../store/completeness-store';
@@ -103,20 +103,12 @@ export class TopResultsRow extends LitElement {
static override styles = [
designTokens,
exploreLinkStyles,
unownedStyles,
css`
:host {
display: block;
margin-bottom: 16px;
}
.row {
display: flex;
gap: 12px;
overflow-x: auto;
padding-bottom: 4px;
}
.card {
flex: 0 0 auto;
width: 200px;
@@ -285,9 +277,9 @@ export class TopResultsRow extends LitElement {
return html`
<div class="section-label">Top Results</div>
<div class="row">
<scroll-row>
${this.results.map((r) => this.renderCard(r))}
</div>
</scroll-row>
`;
}
+1
View File
@@ -31,6 +31,7 @@ solid/bookmark
solid/box-open
solid/check
solid/chevron-down
solid/chevron-left
solid/chevron-right
solid/circle-check
solid/circle-exclamation
+184
View File
@@ -0,0 +1,184 @@
import { css } from 'lit';
/**
* The Explore album card, once.
*
* Two components draw one — `explore-view`'s shelves and search
* results, and `explore-artist-details`'s discography — and they had
* grown two copies of the same rules. That is how the size came apart:
* `explore-view` clamped its cards to a 130–150px range so two cards in
* one row could be different widths, and since the artwork is square
* that made them different *heights* as well. A row of covers with
* ragged bottoms is the whole complaint.
*
* So the width is a fixed `--yj-album-card-width` and the lines below
* the art each reserve their own space, which is what makes every card
* the same size no matter what a given album happens to carry —
* `album-card-size.test.ts` measures that rather than trusting it.
*
* Three rules here are the parts that changed rather than moved.
*
* **The artwork is inset in the square, not cropped to it.** The
* container was already `aspect-ratio: 1` but the image was
* `object-fit: cover`, so a non-square cover lost its edges. It is
* `contain` now and the container's own background is transparent, so
* a tall or wide cover sits in the middle of the square with the page
* showing through beside it.
*
* **The badge lives on the artwork, top-left, and only under the
* pointer.** It used to sit in the metadata line and only for the
* unowned case. It draws for every card now — an owned album's tick is
* the answer to the same question — and it is revealed by hover on a
* pointer device. Where there is no hover it is *always* visible rather
* than never, because on those devices it is the only route to its
* action: `explore-view`'s card menu carries no request item, so a
* phone with the badge hidden could not ask for an album at all.
*
* **Nothing dims an unowned card.** `unownedStyles` was removed from
* the catalog surfaces on the rule that the badge is the mark; the
* album page's *tracklist* still dims unowned rows, which is a
* different statement about a different thing.
*/
export const albumCardStyles = css`
.album-card {
width: var(--yj-album-card-width, 150px);
display: flex;
flex-direction: column;
gap: 6px;
padding: 8px;
border-radius: 8px;
box-sizing: border-box;
flex-shrink: 0;
cursor: pointer;
transition: background 0.15s ease;
}
.album-card:hover {
background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.06));
}
.album-card:active {
transform: scale(0.97);
}
.album-card:focus-visible {
outline: 2px solid var(--yj-accent-text, #ffd43b);
outline-offset: -2px;
}
.album-art-container {
position: relative;
width: 100%;
aspect-ratio: 1;
border-radius: 4px;
overflow: hidden;
background: transparent;
display: flex;
align-items: center;
justify-content: center;
}
.album-art-container img {
width: 100%;
height: 100%;
object-fit: contain;
display: block;
}
/* The placeholder is the one case that *is* a full square, so it
carries the background the container gave up. */
.album-art-fallback {
display: flex;
align-items: center;
justify-content: center;
width: 100%;
height: 100%;
position: absolute;
inset: 0;
background: linear-gradient(
135deg,
var(--yj-bg-overlay, #404040) 0%,
var(--yj-bg-surface, #282828) 100%
);
}
.album-art-fallback wa-icon {
color: var(--yj-text-tertiary, #888);
font-size: 24px;
opacity: 0.5;
}
.album-card-badge {
position: absolute;
top: 6px;
left: 6px;
z-index: 1;
display: flex;
visibility: hidden;
opacity: 0;
transition: opacity 0.15s ease, visibility 0.15s ease;
}
@media (hover: hover) and (pointer: fine) {
.album-card:hover .album-card-badge,
.album-card:focus-within .album-card-badge {
visibility: visible;
opacity: 1;
}
}
@media not all and (hover: hover) {
.album-card-badge {
visibility: visible;
opacity: 1;
}
}
.album-title {
font-weight: 500;
color: var(--yj-text-primary, #fff);
font-size: var(--yj-text-sm);
line-height: 1.3;
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}
/* Reserved even where a surface has no artist to draw, so a card
in a row is never shorter than its neighbour. */
.album-artist {
color: var(--yj-text-tertiary, #888);
font-size: var(--yj-text-xs);
line-height: 1.3;
min-height: 1.3em;
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}
.album-meta {
display: flex;
align-items: center;
justify-content: space-between;
gap: 6px;
color: var(--yj-text-tertiary, #888);
font-size: var(--yj-text-xs);
height: 20px;
}
.album-meta-text {
display: flex;
align-items: center;
gap: 6px;
min-width: 0;
overflow: hidden;
}
.type-badge {
background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.08));
padding: 1px 6px;
border-radius: 3px;
font-size: 10px;
white-space: nowrap;
}
`;
+29
View File
@@ -0,0 +1,29 @@
/**
* Opening an external page, with the destination pinned.
*
* Every external link this app opens is a MusicBrainz entity page built
* from an MBID that came from the catalog. Constructing the URL by
* string concatenation leaves the destination to whatever is in that
* string, so this parses it against the one origin the app means and
* refuses anything else — an MBID cannot change the host, and if it
* somehow did, nothing would open.
*
* It navigates through a real anchor rather than `window.open`: the
* same top-level `_blank` navigation with `noopener`, and it keeps the
* destination an ordinary link rather than an argument to a function
* whose first parameter is a URL.
*/
const MUSICBRAINZ_ORIGIN = 'https://musicbrainz.org';
export function openMusicBrainz(path: string): void {
const url = new URL(path, MUSICBRAINZ_ORIGIN);
if (url.origin !== MUSICBRAINZ_ORIGIN) return;
const link = document.createElement('a');
link.href = url.toString();
link.target = '_blank';
link.rel = 'noopener noreferrer';
link.click();
}
+12 -1
View File
@@ -10,6 +10,16 @@
* badge as the only difference. This is that rule, written once, so
* eight surfaces cannot each keep their own version of it.
*
* **The catalog's *cards* no longer dim.** A grid of dimmed covers read
* as a page that had failed to load rather than as a page of things you
* could ask for, so on Explore the mark is the badge alone — over the
* artwork, on hover, drawn for owned and unowned alike. The album
* page's *tracklist* still dims unowned rows: that is a different
* statement ("this one is not here") about a different thing, and the
* `aria-disabled` row that cannot be played is what it is for. So
* `unownedStyles` survives for that one surface and the cards simply do
* not include it.
*
* ## Ownership is a file, and `localId` is the flag that says so
*
* The album page answers "do I own this row" with `filePaths`, a map
@@ -96,7 +106,8 @@ export function ownershipLabel(
}
/**
* The dimming, shared so it cannot drift across surfaces.
* The dimming, shared so it cannot drift across surfaces — and now
* used by exactly one of them.
*
* Two things about it are load-bearing.
*
@@ -0,0 +1,174 @@
/**
* Every album card is the same size, and its artwork is a square.
*
* The size came apart because `explore-view` clamped its cards to a
* 130–150px range, so two cards in one row could be different widths —
* and since the artwork is square, different *heights* as well. A row
* of covers with ragged bottoms is what that looks like.
*
* What makes the fix hold is that the lines below the art each reserve
* their own space (`album-card.css.ts`), so an album with no year, no
* release type or a one-character title is not shorter than its
* neighbour. This measures that rather than trusting it, because the
* next component to format a card is the way it comes back.
*
* The artwork half is the other change: the container was already
* square but the image was `object-fit: cover`, so a non-square cover
* was cropped to it. It is `contain` now, and the container has no
* background of its own, so a tall cover is inset with the page
* showing through beside it.
*/
import { beforeEach, describe, expect, it } from 'vitest';
import type { LitElement } from 'lit';
import '@components/explore-view/explore-view';
import { flush, stub, resetHarness } from '@test/support/harness';
import { fixture, shadow, shadowAll, update } from '@test/support/render';
import { completenessStore } from '@store/completeness-store';
const SEARCH = 'explore.Service.SearchLocal';
const SHELVES = 'explore.Service.GetExploreShelves';
/** A 1x1 transparent gif, so the `<img>` branch renders. */
const TINY_IMAGE =
'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7';
/** Release groups chosen so every optional line is present on one and
* absent on another — that is what a size regression hides behind. */
const ALBUMS = [
{
mbid: 'rg-1',
title: 'A',
artistCredit: '',
artistMbid: 'ar-1',
primaryType: '',
firstReleaseDate: '',
popularity: 1,
listenerCount: 1,
secondaryTypes: [],
inLibrary: false,
localId: 0,
},
{
mbid: 'rg-2',
title: 'A Very Long Album Name That Will Certainly Be Truncated By The Card',
artistCredit: 'An Artist With A Long Name',
artistMbid: 'ar-2',
primaryType: 'Album',
firstReleaseDate: '1994-05-01',
popularity: 1,
listenerCount: 1,
secondaryTypes: [],
inLibrary: false,
localId: 0,
},
{
mbid: 'rg-3',
title: 'Three',
artistCredit: 'Another',
artistMbid: 'ar-3',
primaryType: 'EP',
firstReleaseDate: '2001-01-01',
popularity: 1,
listenerCount: 1,
secondaryTypes: [],
inLibrary: false,
localId: 0,
},
];
async function exploreWithAlbums(): Promise<LitElement> {
stub(SHELVES, { shelves: [], state: 'ready' });
stub(SEARCH, {
artists: [],
releaseGroups: ALBUMS,
recordings: [],
});
stub('explore.Service.GetThumbnails', Object.fromEntries(
ALBUMS.map((a) => [a.mbid, TINY_IMAGE]),
));
stub('explore.Service.GetThumbnail', TINY_IMAGE);
const el = await fixture<LitElement>('explore-view');
(el as unknown as { onViewActivate: () => void }).onViewActivate?.();
await update(el, {
results: { artists: [], releaseGroups: ALBUMS, recordings: [] },
});
await flush();
await el.updateComplete;
return el;
}
beforeEach(() => {
resetHarness();
stub('library.Library.GetAlbumsCompleteness', {});
completenessStore.invalidate();
});
describe('the album card size', () => {
it('is the same width and height for every card in a row', async () => {
const el = await exploreWithAlbums();
const cards = shadowAll(el, '.album-card');
expect(cards.length).toBe(ALBUMS.length);
const boxes = cards.map((c) => c.getBoundingClientRect());
// The first card is the reference; every other one must match it.
for (const box of boxes) {
expect(box.width).toBe(boxes[0]!.width);
expect(box.height).toBe(boxes[0]!.height);
}
// …and the reference is a real box, or the loop above is vacuous.
expect(boxes[0]!.width).toBeGreaterThan(0);
expect(boxes[0]!.height).toBeGreaterThan(0);
});
it('keeps the artwork square', async () => {
const el = await exploreWithAlbums();
for (const art of shadowAll(el, '.album-art-container')) {
const box = art.getBoundingClientRect();
expect(Math.round(box.width)).toBe(Math.round(box.height));
}
});
it('insets a non-square cover rather than cropping it', async () => {
const el = await exploreWithAlbums();
// Read from the parsed stylesheet rather than from a rendered
// `<img>`: the search path is what calls `loadThumbnails`, and
// setting `results` directly skips it, so there is no image to
// measure. The regression worth catching is the rule going back to
// `cover`, which is a stylesheet fact.
const rules = (el.shadowRoot?.adoptedStyleSheets ?? []).flatMap((sheet) =>
Array.from(sheet.cssRules).map((rule) => rule.cssText),
);
const art = rules.find(
(text) =>
text.startsWith('.album-art-container img') &&
text.includes('object-fit'),
);
expect(art, 'no object-fit rule for the cover image').toBeDefined();
expect(art).toContain('object-fit: contain');
});
it('draws the badge over the artwork, and not in the metadata line', async () => {
const el = await exploreWithAlbums();
const card = shadow(el, '.album-card')!;
const badge = card.querySelector('.album-art-container .album-card-badge');
expect(badge).not.toBeNull();
// The badge is positioned inside the art box, so its parent is the
// square rather than the row underneath it.
expect(badge?.parentElement?.classList.contains('album-art-container')).toBe(
true,
);
});
});
@@ -0,0 +1,168 @@
/**
* The artist page's header and its top tracks.
*
* Two cleanups, asserted together because they are one screen:
*
* - the Play/Shuffle pair became one split button ("Play" with the
* words on its title, Shuffle behind the caret), the Follow button
* moved onto the same line, and the name and listen count went up a
* size;
* - a top track's play/request affordance moved onto its artwork,
* where a hover reveals it, instead of a badge at the end of the
* row beside a row that already plays on a double-click.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import '@components/explore-artist-details/explore-artist-details';
import { stub, flush, emit, resetHarness } from '@test/support/harness';
import { Events } from '../../src/events';
import { fixture, shadow, shadowAll } from '@test/support/render';
const ARTIST = 'artist-0001';
const track = (name: string, localId = 0) => ({
recordingMbid: `rec-${name}`,
artistName: 'Tideline',
trackName: name,
totalListenCount: 100,
caaReleaseMbid: '',
releaseName: 'Foreshore',
releaseGroupMbid: 'rg-owned',
length: 200000,
inLibrary: localId > 0,
localId,
});
beforeEach(() => {
resetHarness();
stub('explore.Service.LookupArtist', {
mbid: ARTIST,
name: 'Tideline',
popularity: 1200,
type: 'Group',
country: 'GB',
});
stub('explore.Service.TopReleaseGroupsForArtist', []);
stub('explore.Service.TopRecordingsForArtist', [
track('Owned Song', 7),
track('Absent Song'),
]);
stub('explore.Service.SimilarArtists', []);
stub('explore.Service.PrefetchReleases', undefined);
stub('explore.Service.BrowseReleaseGroups', [
{
mbid: 'rg-owned',
title: 'Foreshore',
artistCredit: 'Tideline',
primaryType: 'Album',
inLibrary: true,
localId: 7,
},
]);
stub('library.Library.GetAlbumsCompleteness', {});
stub('download.Service.ListRequests', []);
});
async function mount(): Promise<LitElement> {
const el = await fixture<LitElement>('explore-artist-details', {
artistMBID: ARTIST,
artistName: 'Tideline',
});
await flush();
return el;
}
describe('the artist header', () => {
it('offers Play, with Shuffle behind its caret', async () => {
const el = await mount();
const play = shadow<HTMLElement>(el, '[data-testid="artist-play-library"]')!;
// The words moved to the title, which is where "Play library
// tracks" can still be read without taking the width of a button.
expect(play.textContent?.trim()).toBe('Play');
expect(play.getAttribute('title')).toBe('Play library tracks');
const menuButton = shadow(el, '[data-testid="artist-play-menu"]');
expect(menuButton).not.toBeNull();
const menu = shadow(el, '#artist-play-menu');
expect(menu?.textContent).toContain('Shuffle');
});
it('puts Follow on the same line as Play', async () => {
const el = await mount();
const actions = shadow(el, '.artist-actions')!;
expect(actions.querySelector('[data-testid="artist-play-library"]')).not.toBeNull();
const follow = actions.querySelector('[data-testid="artist-follow"]') as HTMLElement;
expect(follow).not.toBeNull();
expect(follow.textContent?.trim()).toBe('Follow');
});
it('says Following once the artist is on the request list', async () => {
const el = await mount();
// The store is a singleton and caches its list, so the change is
// announced the way the backend announces one.
stub('download.Service.ListRequests', [
{ id: 3, mbid: ARTIST, state: 'queued' },
]);
emit(Events.RequestsChanged);
await flush();
await el.updateComplete;
const follow = shadow<HTMLElement>(el, '[data-testid="artist-follow"]')!;
expect(follow.textContent?.trim()).toBe('Following');
});
it('sizes the name and the listen count above the metadata line', async () => {
const el = await mount();
const title = shadow<HTMLElement>(el, '.artist-title')!;
const listens = shadow<HTMLElement>(el, '.artist-listens')!;
const meta = shadow<HTMLElement>(el, '.artist-meta')!;
expect(listens.textContent).toContain('plays on ListenBrainz');
const titleSize = parseFloat(getComputedStyle(title).fontSize);
const listensSize = parseFloat(getComputedStyle(listens).fontSize);
const metaSize = parseFloat(getComputedStyle(meta).fontSize);
expect(titleSize).toBeGreaterThan(24);
expect(listensSize).toBeGreaterThan(metaSize);
});
});
describe('a top track’s affordance', () => {
it('plays from the artwork when it is owned', async () => {
const el = await mount();
const rows = shadowAll<HTMLElement>(el, '.track-item');
const owned = rows.find((r) => r.textContent?.includes('Owned Song'))!;
expect(owned.querySelector('.track-art-overlay .track-art-play')).not.toBeNull();
// Nothing beside the row any more.
expect(owned.querySelector(':scope > library-status-indicator')).toBeNull();
});
it('requests from the artwork when it is not', async () => {
const el = await mount();
const rows = shadowAll<HTMLElement>(el, '.track-item');
const absent = rows.find((r) => r.textContent?.includes('Absent Song'))!;
expect(
absent.querySelector('.track-art-overlay library-status-indicator'),
).not.toBeNull();
expect(absent.querySelector('.track-art-overlay .track-art-play')).toBeNull();
});
});
@@ -25,7 +25,9 @@ const ARTIST = 'artist-0001';
/** The labels of the open menu's items, trimmed. */
function menuItems(el: LitElement): string[] {
const panel = shadow(el, '.context-menu-panel');
// Scoped to the context menu: the artist page also has a Play/Shuffle
// dropdown, and its panel carries the same class.
const panel = shadow(el, '#context-menu .context-menu-panel');
if (!panel) return [];
@@ -100,7 +102,7 @@ describe('the context menu on an artist page release', () => {
await openMenuOnAlbum(el, 0);
const panel = shadow(el, '.context-menu-panel');
const panel = shadow(el, '#context-menu .context-menu-panel');
expect(panel).toBeTruthy();
// The panel is shared with the track menu, so a label that does not
@@ -172,8 +172,7 @@ describe('<player-progress-line>', () => {
* The reason this component asks `matchMedia` instead of letting a
* stylesheet hide it: a media query cannot stop a 1 Hz interval
* running for the life of every desktop session. That claim is
* load-bearing in CLAUDE.md, so it is asserted rather than
* described — the timer count, because a desktop render is empty
* load-bearing, so it is asserted rather than described — the timer count, because a desktop render is empty
* either way and so cannot tell the two apart.
*/
it('runs no interpolation timer above the breakpoint', async () => {
+131
View File
@@ -0,0 +1,131 @@
/**
* A horizontally scrolling row can be moved without a wheel.
*
* Until this existed the only way to see the cards past the fold on the
* shelves, the search results and the artist page's discography was a
* mousewheel or a trackpad gesture — which is not an affordance. A
* mouse with no horizontal wheel simply could not reach them.
*
* What is asserted here is the state that makes the arrows honest: an
* arrow is `hidden` at the end it cannot move from, because a control
* that cannot act is worse than none, and an invisible one still holds
* a hit area and a tab stop.
*/
import { beforeEach, describe, expect, it } from 'vitest';
import type { LitElement } from 'lit';
import '@components/scroll-row/scroll-row';
import { fixture } from '@test/support/render';
/** Six 100px cards in a 320px row — comfortably overflowing. */
function content(el: Element): void {
for (let i = 0; i < 6; i += 1) {
const card = document.createElement('div');
card.style.cssText = 'flex: 0 0 100px; height: 40px';
card.textContent = String(i);
el.append(card);
}
}
function arrows(el: LitElement): { prev: HTMLButtonElement; next: HTMLButtonElement } {
const root = el.shadowRoot!;
return {
prev: root.querySelector('.arrow.prev') as HTMLButtonElement,
next: root.querySelector('.arrow.next') as HTMLButtonElement,
};
}
function viewport(el: LitElement): HTMLElement {
return el.shadowRoot!.querySelector('.viewport') as HTMLElement;
}
async function row(): Promise<LitElement> {
const el = await fixture<LitElement>('scroll-row');
el.style.display = 'block';
el.style.width = '320px';
content(el);
await el.updateComplete;
// The observer reports on a later frame than a microtask drain.
await new Promise((r) => setTimeout(r, 60));
await el.updateComplete;
return el;
}
describe('<scroll-row>', () => {
beforeEach(() => {
document.body.style.margin = '0';
});
it('draws an arrow for each direction it can still move', async () => {
const el = await row();
const { prev, next } = arrows(el);
expect(prev).not.toBeNull();
expect(next).not.toBeNull();
// At the start there is nothing behind, so only the forward arrow is
// offered.
expect(prev.hasAttribute('hidden')).toBe(true);
expect(next.hasAttribute('hidden')).toBe(false);
});
it('offers the way back once the row has moved', async () => {
const el = await row();
const vp = viewport(el);
vp.scrollLeft = 120;
vp.dispatchEvent(new Event('scroll'));
await el.updateComplete;
expect(arrows(el).prev.hasAttribute('hidden')).toBe(false);
});
it('stands the forward arrow down at the end', async () => {
const el = await row();
const vp = viewport(el);
vp.scrollLeft = vp.scrollWidth;
vp.dispatchEvent(new Event('scroll'));
await el.updateComplete;
expect(arrows(el).next.hasAttribute('hidden')).toBe(true);
expect(arrows(el).prev.hasAttribute('hidden')).toBe(false);
});
it('moves the row when the arrow is pressed', async () => {
const el = await row();
const vp = viewport(el);
expect(vp.scrollLeft).toBe(0);
arrows(el).next.click();
await expect.poll(() => vp.scrollLeft).toBeGreaterThan(0);
});
it('shows nothing to scroll when the content fits', async () => {
const el = await fixture<LitElement>('scroll-row');
el.style.cssText = 'display: block; width: 320px';
const only = document.createElement('div');
only.style.cssText = 'flex: 0 0 100px; height: 40px';
only.textContent = 'one';
el.append(only);
await el.updateComplete;
await new Promise((r) => setTimeout(r, 60));
await el.updateComplete;
await new Promise((r) => requestAnimationFrame(() => r(null)));
await el.updateComplete;
const { prev, next } = arrows(el);
expect(prev.hasAttribute('hidden')).toBe(true);
expect(next.hasAttribute('hidden')).toBe(true);
});
});
@@ -1,24 +1,28 @@
/**
* Owned is plain; unowned is what gets marked.
* The catalog's cards are not dimmed; the badge is the mark.
*
* `explore-album-details` had this right for one tracklist and nothing
* else did: Explore's cards, the top-results row and the artist page's
* three card shapes all mixed owned and unowned with a small badge as
* the only difference — and drew a green tick on the *common* case,
* which is the treatment the album page's own green ticks were removed
* for.
* The rule this replaced had every unowned card dimmed *and* badged,
* which on a shelf of mostly-unowned covers read as a page that had
* failed to load rather than a page of things you could ask for. So the
* dimming is gone from the catalog surfaces and the badge carries the
* whole statement — over the artwork, on hover, drawn for owned and
* unowned alike.
*
* What is pinned here is the rule rather than any one surface, because
* the fault this replaced was eight call sites each holding their own
* version of it:
* What is still pinned here is the half that was never about dimming:
*
* - an owned thing draws **no badge at all**;
* - an unowned one is dimmed *and* says so in its accessible name,
* because dimming is a colour and cannot be the only signal;
* - ownership is a **file** (`localId`), never the catalog's
* `inLibrary` ratchet, which is a flag that happens to agree;
* - and a partly-held album says *how* partly, which is the one thing
* a tick cannot.
* - a row that cannot be played is `aria-disabled`, while a card that
* still navigates is not;
* - a partly-held album says *how* partly, which is the one thing a
* tick cannot;
* - and an unowned thing still says so in its accessible name, because
* with the dimming gone that name is the whole signal for anyone not
* seeing the badge.
*
* The album page's *tracklist* still dims unowned rows — a different
* statement about a different thing — and is covered by
* `album-request-badge-visibility.test.ts`.
*/
import { beforeEach, describe, expect, it } from 'vitest';
import { page } from 'vitest/browser';
@@ -26,7 +30,7 @@ import { page } from 'vitest/browser';
import '@components/explore-view/explore-view';
import '@components/top-results-row/top-results-row';
import { flush, stub, resetHarness } from '@test/support/harness';
import { fixture, shadow, shadowAll, update } from '@test/support/render';
import { fixture, shadow, update } from '@test/support/render';
import { completenessStore } from '@store/completeness-store';
const SEARCH = 'explore.Service.SearchLocal';
@@ -105,27 +109,44 @@ beforeEach(() => {
// absent one — which is the point, or 87% of a grid re-asks forever.
// Two tests in one file are two sessions as far as it is concerned,
// so a stale entry from the test above would otherwise decide the
// one below. Found by writing the assertion the wrong way round.
// one below.
completenessStore.invalidate();
});
describe('an owned thing is plain', () => {
it('draws no badge on an album card it has files for', async () => {
describe('an unowned card is marked by its badge alone', () => {
it('does not dim the artwork', async () => {
const el = await exploreShowing({
releaseGroups: [album('Absent', {})],
});
const art = shadow(el, '.album-card .album-art-container')!;
// The dimming was an opacity on this box. With it gone the cover is
// at full strength, and the badge is what says the card is not
// yours.
expect(getComputedStyle(art).opacity).toBe('1');
expect(shadow(el, '.album-card library-status-indicator')).not.toBeNull();
});
it('still says so in the name the browser computes', async () => {
await exploreShowing({ releaseGroups: [album('Absent', {})] });
await expect
.element(page.getByRole('button', { name: /Absent — not in your library/ }))
.toBeInTheDocument();
});
});
describe('an owned card is plain except for its badge', () => {
it('draws the in-library badge rather than nothing', async () => {
const el = await exploreShowing({
releaseGroups: [album('Held', { localId: 7 })],
});
expect(shadowAll(el, '.album-card')).toHaveLength(1);
expect(shadow(el, '.album-card library-status-indicator')).toBeNull();
});
const badge = shadow(el, '.album-card library-status-indicator');
it('draws no badge on a track row it has a file for', async () => {
const el = await exploreShowing({
recordings: [recording('Held', { localId: 9 })],
});
expect(shadowAll(el, '.track-item')).toHaveLength(1);
expect(shadow(el, '.track-item library-status-indicator')).toBeNull();
expect(badge).not.toBeNull();
expect(badge?.getAttribute('status')).toBe('in-library');
});
it('does not dim it', async () => {
@@ -139,56 +160,6 @@ describe('an owned thing is plain', () => {
});
});
describe('an unowned thing is marked', () => {
it('dims the card and keeps its request badge', async () => {
const el = await exploreShowing({
releaseGroups: [album('Absent', {})],
});
expect(shadow(el, '.album-card')?.classList.contains('unowned')).toBe(true);
expect(shadow(el, '.album-card library-status-indicator')).not.toBeNull();
});
/**
* The name is the half of this that reaches anyone not seeing the
* dimming, so it has to be the browser's own answer — a shadow-root
* query cannot compute a name, and this repo has shipped a nameless
* control three times.
*/
it('says so in the name the browser computes', async () => {
await exploreShowing({ releaseGroups: [album('Absent', {})] });
await expect
.element(page.getByRole('button', { name: /Absent — not in your library/ }))
.toBeInTheDocument();
});
/**
* A track row is `aria-disabled` and a card is not, and the
* difference is not cosmetic: activating an unowned row does nothing
* (`onRecordingRowDblClick` returns early), while a card navigates to
* the catalog page for it, which is a perfectly good thing to do with
* something you do not own.
*/
it('marks a row that cannot be played as disabled', async () => {
const el = await exploreShowing({
recordings: [recording('Absent', {})],
});
expect(shadow(el, '.track-item')?.getAttribute('aria-disabled')).toBe(
'true',
);
});
it('leaves a card that still navigates enabled', async () => {
const el = await exploreShowing({
releaseGroups: [album('Absent', {})],
});
expect(shadow(el, '.album-card')?.getAttribute('aria-disabled')).toBeNull();
});
});
/**
* The decision this issue turned on.
*
@@ -206,7 +177,9 @@ describe('ownership is a file, not a flag', () => {
});
expect(shadow(el, '.album-card')?.classList.contains('unowned')).toBe(true);
expect(shadow(el, '.album-card library-status-indicator')).not.toBeNull();
expect(
shadow(el, '.album-card library-status-indicator')?.getAttribute('status'),
).not.toBe('in-library');
});
it('does the same for a track row', async () => {
@@ -220,6 +193,26 @@ describe('ownership is a file, not a flag', () => {
});
});
describe('a track row that cannot be played is disabled', () => {
it('marks an unowned row', async () => {
const el = await exploreShowing({
recordings: [recording('Absent', {})],
});
expect(shadow(el, '.track-item')?.getAttribute('aria-disabled')).toBe(
'true',
);
});
it('leaves a card that still navigates enabled', async () => {
const el = await exploreShowing({
releaseGroups: [album('Absent', {})],
});
expect(shadow(el, '.album-card')?.getAttribute('aria-disabled')).toBeNull();
});
});
/**
* The count, which is what `#16`'s deferred third step asked for: an
* album held 2 tracks of 10 wore the same green tick as one held whole,
@@ -248,9 +241,13 @@ describe('a partly-held album says how partly', () => {
// A partly-held album is *actionable* — it has three tracks left to
// ask for — so the badge is a button, and the name has to carry the
// action and the count. Naming it after the action alone left the
// one state the ring exists for as the one state whose name did not
// mention it.
// action and the count. The badge is revealed by the card's focus
// (`:focus-within`), and `visibility: hidden` is what takes it out
// of the accessibility tree until then, so the card is focused
// first — which is exactly the route a keyboard user takes.
shadow<HTMLElement>(el, '.album-card')?.focus();
await el.updateComplete;
await expect
.element(
page.getByRole('button', {
@@ -266,7 +263,7 @@ describe('a partly-held album says how partly', () => {
* state, and a ring drawn from its absence would mark all of it
* incomplete on no evidence. That is the rule `Known` exists for.
*/
it('says nothing when the total was never declared', async () => {
it('falls back to the plain in-library badge when the total was never declared', async () => {
stub(COMPLETENESS, {
'7': { owned: 3, expected: 0, known: false, complete: false },
});
@@ -279,7 +276,9 @@ describe('a partly-held album says how partly', () => {
await flush();
await el.updateComplete;
expect(shadow(el, '.album-card library-status-indicator')).toBeNull();
expect(
shadow(el, '.album-card library-status-indicator')?.getAttribute('status'),
).toBe('in-library');
});
it('asks about the owned albums only, in one call', async () => {
@@ -329,11 +328,13 @@ describe('the top-results row follows the same rule', () => {
query: 'held',
});
// A top-result card is a mixed bag — artist, album or track — and
// its badge is a corner mark rather than the cover overlay the
// album cards grew, so an owned one stays plain.
expect(shadow(el, '.card library-status-indicator')).toBeNull();
expect(shadow(el, '.card')?.classList.contains('unowned')).toBe(false);
});
it('dims and names something it does not', async () => {
it('names something it does not own', async () => {
const el = await fixture('top-results-row', {
results: [result('Absent', 'release_group')],
query: 'absent',
@@ -349,7 +350,7 @@ describe('the top-results row follows the same rule', () => {
/**
* An artist card has never had a badge — a discography subscription
* is the artist page's Follow button, which can say what it commits
* to — so the dimming and the name are the whole signal there.
* to — so the name is the whole signal there.
*/
it('marks an unowned artist without offering a request', async () => {
const el = await fixture('top-results-row', {
+2 -2
View File
@@ -1,6 +1,6 @@
module yellowjacket
go 1.25.0
go 1.26
require (
github.com/BurntSushi/toml v1.6.0
@@ -144,7 +144,7 @@ require (
github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect
github.com/go-git/go-billy/v5 v5.9.0 // indirect
github.com/go-git/go-git/v5 v5.19.2 // indirect
github.com/go-json-experiment/json v0.0.0-20251027170946-4849db3c2f7e // indirect
github.com/go-json-experiment/json v0.0.0-20260820222146-c27c302e5fc3 // indirect
github.com/go-ole/go-ole v1.3.0 // indirect
github.com/go-resty/resty/v2 v2.17.1 // indirect
github.com/go-sql-driver/mysql v1.9.3 // indirect
+2 -2
View File
@@ -362,8 +362,8 @@ github.com/go-git/go-git/v5 v5.19.2/go.mod h1:QqCBE1EFN5ddFmrliLQ3/ntRCUjZU3EJuw
github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/go-json-experiment/json v0.0.0-20251027170946-4849db3c2f7e h1:Lf/gRkoycfOBPa42vU2bbgPurFong6zXeFtPoxholzU=
github.com/go-json-experiment/json v0.0.0-20251027170946-4849db3c2f7e/go.mod h1:uNVvRXArCGbZ508SxYYTC5v1JWoz2voff5pm25jU1Ok=
github.com/go-json-experiment/json v0.0.0-20260820222146-c27c302e5fc3 h1:UADEEmDKgfXbtnGJZ97beY5XLo9ZechG1nlU4KnRrkE=
github.com/go-json-experiment/json v0.0.0-20260820222146-c27c302e5fc3/go.mod h1:tphK2c80bpPhMOI4v6bIc2xWywPfbqi1Z06+RcrMkDg=
github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY=
+1 -2
View File
@@ -64,8 +64,7 @@ var frontendDistAssets embed.FS
// Returning early is not a degraded mode: `nativeInit` has already
// re-attached the bridge, so the recreated activity's WebView talks to
// the app that is still running, with its queue and its playback
// position intact. See CLAUDE.md, "An activity is a view onto the
// process".
// position intact.
//
// It is inert off Android, where a process has exactly one main().
var mainStarted atomic.Bool
+1 -1
View File
@@ -18,7 +18,7 @@ arch=('x86_64')
url="https://git.ljones.me/yonlu/yellowjacket"
license=('custom')
depends=('webkitgtk-6.0' 'gtk4' 'alsa-lib' 'hicolor-icon-theme')
makedepends=('go>=1.25' 'nodejs>=22' 'pnpm' 'git')
makedepends=('go>=1.26' 'nodejs>=22' 'pnpm' 'git')
options=('!lto')
# Source is overridable so the same PKGBUILD works two ways:
+62 -7
View File
@@ -82,23 +82,69 @@ targets="$({ make -pqRr 2>/dev/null || true; } |
# happened to break there, and a check that fails on reflow gets
# disabled rather than fixed.
#
# **An inline span may be hard-wrapped, and then the mention is split
# across two lines.** `make` at the end of one line and its target at
# the start of the next is one code span to Markdown and two strings to
# a per-line regex, so the target was invisible — and these docs are
# mostly hard-wrapped prose, so the wrap is what the author does not
# think about. Lines are therefore joined while the span is still open,
# which is what an odd number of backticks means.
#
# Joining re-opens the reflow trap above unless it is bounded, so it is
# bounded three ways: a fence flushes first (a fenced command is already
# whole, and joining inside one would break the line-start rule), a
# blank line flushes (CommonMark does not allow a blank line inside a
# code span, so nothing legitimate is split by one), and so does a file
# boundary. A stray odd backtick in prose therefore costs one paragraph
# of over-matching rather than the rest of the file.
#
# 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="$(printf '%s\n' "$docs" |
xargs awk '
FNR == 1 { fence = 0 }
/^```/ { fence = !fence; next }
{
rest = $0
function scan(text, rest) {
rest = text
while (match(rest, /`make [a-z][a-z0-9-]*/)) {
print substr(rest, RSTART + 6, RLENGTH - 6)
rest = substr(rest, RSTART + RLENGTH)
}
if (fence && match($0, /^make [a-z][a-z0-9-]*/)) {
print substr($0, 6, RLENGTH - 5)
}
function lineStart(text) {
if (match(text, /^make [a-z][a-z0-9-]*/)) {
print substr(text, 6, RLENGTH - 5)
}
}
function ticks(s, n, i) {
n = 0
for (i = 1; i <= length(s); i++) {
if (substr(s, i, 1) == "`") n++
}
return n
}
function flush() {
if (buf == "") return
scan(buf)
if (fence) lineStart(buf)
buf = ""
}
FNR == 1 { flush(); fence = 0 }
/^```/ { flush(); fence = !fence; next }
/^[[:space:]]*$/ { flush(); next }
{
if (fence) { scan($0); lineStart($0); next }
buf = (buf == "" ? $0 : buf " " $0)
if (ticks(buf) % 2 == 0) flush()
}
END { flush() }
' | sort -u)"
missing=""
@@ -113,7 +159,16 @@ if [ -n "$missing" ]; then
echo "skill-check: the docs name make targets that do not exist:" >&2
for t in $missing; do
echo " make $t" >&2
printf '%s\n' "$docs" | xargs grep -ln "make $t" | sed 's/^/ /' >&2
# `make <t>` on one line first, because that is where a target is
# normally named and it is the precise answer. The bare name is the
# fallback, and it exists because the parser above can now find a
# mention that *this* grep cannot: a wrapped span has `make` and its
# target on different lines. Without it a missing target reported no
# file at all, and `set -o pipefail` turned the empty grep into exit
# 123, before the line telling the author what to do.
hits="$(printf '%s\n' "$docs" | xargs grep -ln "make $t" 2>/dev/null || true)"
[ -n "$hits" ] || hits="$(printf '%s\n' "$docs" | xargs grep -ln -- "$t" 2>/dev/null || true)"
[ -n "$hits" ] && printf '%s\n' "$hits" | sed 's/^/ /' >&2
done
echo "Fix the docs, or restore the target." >&2
exit 1