Soulseek downloads: transfer lifecycle, candidate shape, search recall, per-peer concurrency #267

Closed
yonlu wants to merge 0 commits from fix/263-slskd-transfer-lifecycle into main
Owner

The Soulseek download path, from an audit of backend/download/, as one batch because every commit edits provider_slskd.go.

  • Transfers (#263): a failed or stalled peer no longer fails the download, ties up the single slskd transfer slot for up to six hours, or leaves transfers running in slskd.
  • Candidates (#270): a multi-disc rip is scored as one album; single-track requests can be served; completeness counts tracks rather than files.
  • Search (#271): two queries instead of one, stated search options, a state-only poll, and durations scored.
  • Concurrency (#272): one grab per peer, several peers at once, and no two grabs writing into the same slskd folder.
commit issue what it does
fc012122 #263 Gives up after 10 minutes with no bytes moving. Counts a file slskd never lists as failed after a 30 s grace. Ignores terminal records that existed before the enqueue. Cancels and removes live transfers in slskd on every non-success exit. Path-escapes usernames.
0a33b9d6 #263 grab falls through to the next auto-acceptable candidate: at most 3 attempts; on Soulseek a failed peer rules out every folder it offered; no fallback for a hand pick. Also fixes auto-pick grabbing ranked[0] while the veto had judged the first candidate inside the guardrails, so an oversized copy was downloaded anyway.
7fbfd9c1 #266 TestScan_StoresOnlyCoverTiers skips without fixtures instead of failing; it blocked pre-push in a fresh worktree.
f81a9509 #270 Groups CD1/Disc 2 folders under their album and reads the disc from the folder. Keeps the disc folders in staging, where flattened they overwrote each other. Sets a one-file floor for recording requests. Counts completeness in aligned tracks rather than files.
5e3ac8fb #271 Runs a normalised second query alongside the first (edition qualifiers, a leading -, "Various Artists") and merges by peer and folder. States the search options. Polls state only and fetches responses once, falling back for an older daemon. Carries per-file length as LengthMillis and scores DurationFit, which takes 0.15 of title fit's weight when timed. Drops the phantom freeUploadSlots. Regenerates bindings.
9710c114 #272 A per-(provider, peer) lock is taken before any slot, so one slow peer no longer serialises every Soulseek download, and the slskd default goes 1 → 3 (it now counts peers; help text rewritten). Grabs whose local slskd folders overlap (two peers' "Greatest Hits", any two rips' "CD1") take a package-level per-path lock in sorted order, because collect finds files by name.

Verification

  • go test -race -count=2 ./backend/download/ passes, and the pre-push hook ran go test -race ./... and make ui-test.
  • New tests:
    • Transfers: a stalled peer is abandoned and cancelled with ?remove=true; a stall after partial arrival keeps what arrived; a transfer that is moving is never abandoned; an unlisted file counts as failed; a stale record is ignored; a cancelled caller still cancels in slskd; usernames are escaped.
    • Fallback: reaches the working copy, respects the guardrails, is bounded at 3, and does not happen for a hand pick; auto-pick takes the best eligible candidate.
    • Candidates: disc-folder parsing, AlbumDir, disc grouping, disc folders kept in staging, a two-disc album aligning completely, the one-file floor for a track request, coverage counted in tracks.
    • Concurrency: one peer is asked for one thing at a time with the daemon allowing three; four different peers run up to the cap; a held folder makes a grab wait without enqueueing; the keyed lock frees its entries and honours its context.
    • Search: the query ladder table, both queries run and merged with options stated and no includeResponses poll, the older-daemon fallback, duration agreement, lengths separating a studio album from a live one, and an unknown duration leaving the score unchanged.
  • golangci-lint reports 0 issues (default and dev tags; run with GOTOOLCHAIN=go1.26.0, see #265). tsc --noEmit passes against the regenerated bindings.

Not verified

  • None of this was run against a live slskd. It rests on my reading of slskd's API: the DELETE …/{id}?remove=true cancel, the GET /searches/{id}/responses endpoint (which has a fallback), and the search option names (searchTimeout, responseLimit, fileLimit, filterResponses, minimumResponseFileCount, maximumPeerQueueLength). ASP.NET ignores unknown body fields, so a wrong option name is inert rather than an error. A first real download, a real stall and one search with slskd's own log open would settle all of it.
  • A file left in slskd's folder by an earlier failed attempt can still be collected by a later grab of a same-named folder if that grab's own copy of the file never arrives. The folder lock stops concurrent grabs mixing; it does not clean up after failed ones, because deleting by name in slskd's directory could take a file the user downloaded by hand.
  • Two concurrent searches per request are within what ordinary Soulseek clients do, but they double our search volume on the network.

Closes

🤖 Generated with Claude Code

https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT

The Soulseek download path, from an audit of `backend/download/`, as one batch because every commit edits `provider_slskd.go`. - **Transfers** (#263): a failed or stalled peer no longer fails the download, ties up the single slskd transfer slot for up to six hours, or leaves transfers running in slskd. - **Candidates** (#270): a multi-disc rip is scored as one album; single-track requests can be served; completeness counts tracks rather than files. - **Search** (#271): two queries instead of one, stated search options, a state-only poll, and durations scored. - **Concurrency** (#272): one grab per peer, several peers at once, and no two grabs writing into the same slskd folder. | commit | issue | what it does | | --- | --- | --- | | `fc012122` | #263 | Gives up after 10 minutes with no bytes moving. Counts a file slskd never lists as failed after a 30 s grace. Ignores terminal records that existed before the enqueue. Cancels and removes live transfers in slskd on every non-success exit. Path-escapes usernames. | | `0a33b9d6` | #263 | `grab` falls through to the next auto-acceptable candidate: at most 3 attempts; on Soulseek a failed peer rules out every folder it offered; no fallback for a hand pick. Also fixes auto-pick grabbing `ranked[0]` while the veto had judged the first candidate *inside* the guardrails, so an oversized copy was downloaded anyway. | | `7fbfd9c1` | #266 | `TestScan_StoresOnlyCoverTiers` skips without fixtures instead of failing; it blocked pre-push in a fresh worktree. | | `f81a9509` | #270 | Groups `CD1`/`Disc 2` folders under their album and reads the disc from the folder. Keeps the disc folders in staging, where flattened they overwrote each other. Sets a one-file floor for recording requests. Counts completeness in aligned tracks rather than files. | | `5e3ac8fb` | #271 | Runs a normalised second query alongside the first (edition qualifiers, a leading `-`, "Various Artists") and merges by peer and folder. States the search options. Polls state only and fetches responses once, falling back for an older daemon. Carries per-file `length` as `LengthMillis` and scores `DurationFit`, which takes 0.15 of title fit's weight when timed. Drops the phantom `freeUploadSlots`. Regenerates bindings. | | `9710c114` | #272 | A per-(provider, peer) lock is taken before any slot, so one slow peer no longer serialises every Soulseek download, and the slskd default goes 1 → 3 (it now counts peers; help text rewritten). Grabs whose local slskd folders overlap (two peers' "Greatest Hits", any two rips' "CD1") take a package-level per-path lock in sorted order, because collect finds files by name. | ## Verification - `go test -race -count=2 ./backend/download/` passes, and the pre-push hook ran `go test -race ./...` and `make ui-test`. - New tests: - **Transfers:** a stalled peer is abandoned and cancelled with `?remove=true`; a stall after partial arrival keeps what arrived; a transfer that is moving is never abandoned; an unlisted file counts as failed; a stale record is ignored; a cancelled caller still cancels in slskd; usernames are escaped. - **Fallback:** reaches the working copy, respects the guardrails, is bounded at 3, and does not happen for a hand pick; auto-pick takes the best *eligible* candidate. - **Candidates:** disc-folder parsing, `AlbumDir`, disc grouping, disc folders kept in staging, a two-disc album aligning completely, the one-file floor for a track request, coverage counted in tracks. - **Concurrency:** one peer is asked for one thing at a time with the daemon allowing three; four different peers run up to the cap; a held folder makes a grab wait without enqueueing; the keyed lock frees its entries and honours its context. - **Search:** the query ladder table, both queries run and merged with options stated and no `includeResponses` poll, the older-daemon fallback, duration agreement, lengths separating a studio album from a live one, and an unknown duration leaving the score unchanged. - `golangci-lint` reports 0 issues (default and `dev` tags; run with `GOTOOLCHAIN=go1.26.0`, see #265). `tsc --noEmit` passes against the regenerated bindings. ## Not verified - **None of this was run against a live slskd.** It rests on my reading of slskd's API: the `DELETE …/{id}?remove=true` cancel, the `GET /searches/{id}/responses` endpoint (which has a fallback), and the search option names (`searchTimeout`, `responseLimit`, `fileLimit`, `filterResponses`, `minimumResponseFileCount`, `maximumPeerQueueLength`). ASP.NET ignores unknown body fields, so a wrong option name is inert rather than an error. A first real download, a real stall and one search with slskd's own log open would settle all of it. - **A file left in slskd's folder by an earlier failed attempt** can still be collected by a later grab of a same-named folder if that grab's own copy of the file never arrives. The folder lock stops concurrent grabs mixing; it does not clean up after failed ones, because deleting by name in slskd's directory could take a file the user downloaded by hand. - **Two concurrent searches per request** are within what ordinary Soulseek clients do, but they double our search volume on the network. ## Closes - #263 - #266 - #270 - #271 - #272 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
yonlu added 3 commits 2026-09-26 21:04:03 +00:00
awaitTransfers waited on every requested file reaching a terminal state
with no bound but the caller's six-hour context.  slskd's transfer
limit is one, so a peer that queued us and never sent a byte held every
other Soulseek download behind it for the whole six hours.  A grab now
gives up after ten minutes with no bytes moving; a folder that stalls
on its last tracks goes forward with what arrived, as a partial failure
always has.

Three smaller faults on the same path:

- A file slskd never lists (refused at enqueue) could never reach a
  terminal state, so the wait could not end.  It counts as failed after
  a short grace period.
- A terminal record left by an earlier attempt at the same file from the
  same peer was read as this attempt's answer on the first poll.  The
  ids already terminal before enqueue are ignored.
- Giving up, for any reason, left slskd downloading for a request nobody
  was waiting on.  The live transfers are cancelled and removed there,
  on a context of their own so a cancelled caller still sends it.

Usernames are now path-escaped; they may carry spaces and slashes.

Refs #263

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
A failed transfer failed the whole download.  On Soulseek the usual
failure is one peer being offline or refusing, and a popular album has
several other peers offering the same folder; the ranked list that
names them was already held in m.results and nothing walked it.

grab now loops: when a candidate's transfer fails, or delivers too
little of the album to import, the next candidate is tried in its
place, up to three in all.  Three rules keep that honest:

- Only a candidate auto-pick would itself have accepted is offered, so
  a second choice clears the same match, quality and guardrail gates
  as the first.
- On Soulseek the failure is the peer's, so every folder that peer
  offered is skipped with it; elsewhere only the failed release is.
- A candidate the user picked by hand does not fall back.  They chose
  that copy, and quietly substituting another is a decision they did
  not make.

The same change fixes auto-pick grabbing the wrong candidate.
AutoPickVeto judges the best candidate inside the user's guardrails,
but Start and Attempt then grabbed ranked[0] -- so when the overall best
was over the size ceiling, the veto passed on the strength of the
second and the first was downloaded anyway: the one copy the user had
said not to take unattended.  autoPick returns the candidate the veto
actually judged.

Closes #263

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
test(library): skip the cover-tier scan test when fixtures are absent
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 4m23s
CI / e2e (pull_request) Canceled after 0s
7fbfd9c105
TestScan_StoresOnlyCoverTiers built the fixture path by hand, so in a
tree where make testdata had not run it failed on a missing covers
directory, where every other fixture test skips via testfixtures.Load.
In a fresh worktree that failure blocked the pre-push hook for every
branch.

Closes #266

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
yonlu added 2 commits 2026-09-26 21:12:59 +00:00
fix(download): score a multi-disc rip as one album, and count tracks
CI / check (push) Skipped
CI / e2e (push) Skipped
f81a950916
Three faults in how candidates are shaped and scored, one commit because
they meet in the same completeness number.

Multi-disc albums were split in two.  Soulseek shares them as
Album/CD1 and Album/CD2, and candidates were grouped by the immediate
parent, so each disc became its own candidate titled "CD1": about half
complete, with an album title that could not match.  Such a release
essentially never cleared auto-pick.  AlbumDir groups a disc folder
under its parent, ParsePath takes the disc number from the folder (a
disc in the filename still wins), and collect keeps the disc folders in
staging, where flattened, disc 2's "01 Intro.flac" overwrote disc 1's.

A single-track request could never be served from Soulseek.  A track
search matches one file per folder, and the two-file floor that screens
out noise for an album screened out every result.  A recording request
takes one.

Completeness counted files.  Ten files against a ten-track album scored
full marks whether or not they were its tracks, and title fit is the
mean over the files that did align, so a folder where three titles
matched read as near-perfect on both.  Coverage is now counted in
aligned tracks, with the file count still setting the penalty for
extras.

Closes #270

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
fix(download): search Soulseek more than once, and read file lengths
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Canceled after 0s
CI / e2e (pull_request) Canceled after 0s
5e3ac8fb1b
The slskd search asked one question and ignored part of the answer.

Two queries.  Soulseek matches every term against a file's full path,
so every extra word is a filter, and several filter wrongly: an edition
qualifier from the catalog title that no one puts in a folder name, a
term with a leading "-", which Soulseek reads as an exclusion, and
"Various Artists", which is in no one's path.  When a normalised form of
the request differs, it runs alongside the original and the candidates
are merged by peer and folder.  Concurrently, not as a fallback: the
manager gives a provider one search budget, and a Soulseek search spends
most of it waiting.  A query the user typed is searched as written.

Stated options.  The search carried only its id and text, so slskd's
own defaults for its timeout and response limits applied.  Its timeout
is now set inside our wait, the limits are well above a popular album,
and slskd drops folders below the file floor and peers with a queue we
would not reach today.

A state-only poll.  Every one-second poll re-sent every response; the
responses are now fetched once at the end, falling back to the old
includeResponses form for a daemon without that endpoint.

Durations.  slskd reports each file's length and it was discarded.  It
is now carried as CandidateFile.LengthMillis and scored against the
expected tracks as DurationFit, which takes 0.15 of title fit's weight
when at least half the aligned pairs are timed: a title says which song
a file claims to be, a length says whether it is that recording.
Without lengths the score is exactly the previous formula.

freeUploadSlots is removed from the response type; slskd sends
hasFreeUploadSlot and nothing by that name.

Closes #271

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
yonlu changed title from fix(download): fall back past a failed or stalled Soulseek peer (#263) to fix(download): the Soulseek path: transfers, candidates and search (#263, #270, #271) 2026-09-26 21:14:32 +00:00
yonlu added 1 commit 2026-09-26 21:33:20 +00:00
feat(download): one grab per Soulseek peer, several peers at once
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Failing after 2m51s
CI / e2e (pull_request) Skipped
9710c11476
slskd was capped at one transfer per daemon, on the grounds that
Soulseek peers punish clients that ask for too much. That politeness is
per peer: two different users do not compete for anyone's upload slot.
So one slow peer serialised every other Soulseek download behind it.

The manager now takes a per-(provider, peer) lock before any slot, so a
grab waiting on a busy peer does not hold a provider slot another peer
could use, and the slskd default rises to 3, which now counts peers.
The help text says so.

Running grabs at once exposed the folder collision: slskd names a
download's directory after the remote leaf folder, so two peers'
"Greatest Hits" (or any two rips' "CD1") share one directory, and
collect finds files by name there. Grabs whose local folders overlap
now take a package-level lock per folder, in sorted order, keyed on the
full path because two clients can share one daemon.

Closes #272

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
yonlu changed title from fix(download): the Soulseek path: transfers, candidates and search (#263, #270, #271) to Soulseek downloads: transfer lifecycle, candidate shape, search recall, per-peer concurrency 2026-09-26 21:33:21 +00:00
yonlu closed this pull request 2026-09-27 01:44:09 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.