Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ bun run build # webapp first, then cargo — the order

Tests need no service container: `test_app()` builds a `Config` over a `TempDir` and the suite drives the same router `main` serves. FFmpeg and ffprobe must be on `PATH`.

The integration suite is eighteen targets under `tests/`, one per subject — `auth`, `scanner`, `identity`, `catalog`, `subsonic_contract`, `subsonic_browse`, `subsonic_fields`, `subsonic_methods`, `native_api`, `sync`, `media`, `service`, `cli`, `scrobbling`, `enrichment`, `uploads`, `canvas`, `migrations` — nearly all of them sharing `tests/support/mod.rs`. `migrations` is the exception and deliberately so: every other target starts from `test_app()`, which builds its database from nothing, and that is exactly the path an upgrade does not take. It seeds a database at each **released** schema, boots the server on it, and reads the rows back through the services — and it is where anything else that needs a database *behind* this binary belongs, such as proving that a key change reads without migrating and copies the pair aside before it does. `autotests = false`, so each is a target only because `Cargo.toml` declares it: a new file under `tests/` is compiled by nothing until a `[[test]]` entry names it, and the count above is worth checking against that list rather than trusted. `support/mod.rs` is the exception that proves it — shared module code, declared nowhere, a target never. File a new test by the surface it exercises, and check that by what it calls rather than by what its name says.
The integration suite is nineteen targets under `tests/`, one per subject — `auth`, `scanner`, `identity`, `catalog`, `subsonic_contract`, `subsonic_browse`, `subsonic_fields`, `subsonic_methods`, `native_api`, `sync`, `media`, `service`, `cli`, `scrobbling`, `enrichment`, `credits`, `uploads`, `canvas`, `migrations` — nearly all of them sharing `tests/support/mod.rs`. `migrations` is the exception and deliberately so: every other target starts from `test_app()`, which builds its database from nothing, and that is exactly the path an upgrade does not take. It seeds a database at each **released** schema, boots the server on it, and reads the rows back through the services — and it is where anything else that needs a database *behind* this binary belongs, such as proving that a key change reads without migrating and copies the pair aside before it does. `autotests = false`, so each is a target only because `Cargo.toml` declares it: a new file under `tests/` is compiled by nothing until a `[[test]]` entry names it, and the count above is worth checking against that list rather than trusted. `support/mod.rs` is the exception that proves it — shared module code, declared nowhere, a target never. File a new test by the surface it exercises, and check that by what it calls rather than by what its name says.

`cargo build` works without a client build — `build.rs` embeds a placeholder page when `webapp/dist` is absent. A real `vite build` overwrites it.

Expand All @@ -60,7 +60,7 @@ The integration suite is eighteen targets under `tests/`, one per subject — `a
- **404 blurs everything.** A resource that is missing, and one that belongs to another account, answer identically. `ServiceError::Forbidden` maps onto 404 for that reason. Never confirm existence to someone not entitled to it.
- **SQLite discipline.** WAL, foreign keys, `busy_timeout`. Every mutation takes the process-wide writer gate (`db.writer_guard()`); libraries may scan concurrently but never open independent writers. `NULL` sorts first in SQLite — order explicitly with `NULLS LAST` wherever a missing tag would otherwise jump the queue.
- **Migrations are immutable once merged.** They are embedded at compile time and checksummed, so editing an applied migration makes the server refuse to start. Schema changes are new dated files under `migrations-v2/`.
- **Identifiers and time.** Public ids are UUIDs; timestamps are Unix epoch milliseconds. Album and artist ids are *derived* UUIDs (version 8, RFC 9562 §5.8): the identity engine in `src/pid.rs` hashes the tags that name the entity, under the configurable spec `WAVEFLOW_PID_ALBUM`/`WAVEFLOW_PID_ARTIST`, so the same files answer the same ids on a fresh install. Track ids stay drawn at random — six tables cascade off them, and a scan matches a track by path, then by content hash, then by the relocation hint in `track.pid`. That hint is the track spec evaluated over the file's tags, and it stays a hint: it is asked only when the first two have nothing to say — a file both moved and re-encoded — and it moves nothing unless it names exactly one track whose file has genuinely disappeared. Changing `WAVEFLOW_PID_TRACK` drops every stored hint on the next boot: a hint written under one spec must never be compared against a lookup computed under another, because two specs can evaluate to the same string and the hint would then hand a new file an unrelated track's identity. The next ordinary scan writes them back from its skip path, so clearing them costs no rescan — and a track spec change still re-identifies nothing. Changing a spec re-identifies everything it governs: the next boot notices, schedules a full rescan of every library, and album favourites do not survive it.
- **Identifiers and time.** Public ids are UUIDs; timestamps are Unix epoch milliseconds. Album and artist ids are *derived* UUIDs (version 8, RFC 9562 §5.8): the identity engine in `src/pid.rs` hashes the tags that name the entity, under the configurable spec `WAVEFLOW_PID_ALBUM`/`WAVEFLOW_PID_ARTIST`, so the same files answer the same ids on a fresh install. Track ids stay drawn at random — six tables cascade off them, and a scan matches a track by path, then by content hash, then by the relocation hint in `track.pid`. That hint is the track spec evaluated over the file's tags, and it stays a hint: it is asked only when the first two have nothing to say — a file both moved and re-encoded — and it moves nothing unless it names exactly one track whose file has genuinely disappeared. Changing `WAVEFLOW_PID_TRACK` drops every stored hint on the next boot: a hint written under one spec must never be compared against a lookup computed under another, because two specs can evaluate to the same string and the hint would then hand a new file an unrelated track's identity. The next ordinary scan writes them back from its skip path, so clearing them costs no rescan — and a track spec change still re-identifies nothing. Changing a spec re-identifies everything it governs: the next boot notices, schedules a full rescan of every library, and album favourites do not survive it. A credit decision (RFC-015) is narrower: splitting a joined credit such as `Zedd, Alessia Cara` re-identifies only the artists it names. The album display string and the track hint read the tag mapper's names, before any decision, so no album or track id moves, and favourites on the joined artist are carried onto its parts through the journal.
- **Credentials.** Web passwords use Argon2id. Access, refresh, API and OAuth codes are stored as SHA-256 hashes only. The dedicated Subsonic password is encrypted with ChaCha20-Poly1305 under the instance key. Back up `data/waveflow.db` and `data/instance.key` together — encrypted values cannot be recovered with one without the other.
- **Browser playback uses stream tickets** (`src/stream_ticket.rs`). `<audio src>` cannot send an `Authorization` header, so the client exchanges its session for an AEAD-sealed `(user, track, expiry)` and plays from `/api/v2/stream/{ticket}`. Access is re-checked on every redemption, so a ticket cannot outlive the membership that justified it. The TTL is an hour rather than seconds because the browser reuses that URL for every range request while seeking.
- **Native clients use Authorization Code + PKCE** (`src/oauth.rs`). S256 only; redirect targets restricted to loopback, https or a reverse-domain private scheme. A code is spent by its first presentation whatever the outcome.
Expand Down
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,7 @@ path = "tests/scrobbling.rs"
[[test]]
name = "enrichment"
path = "tests/enrichment.rs"

[[test]]
name = "credits"
path = "tests/credits.rs"
16 changes: 10 additions & 6 deletions docs/rfcs/RFC-015-artist-credit-splitting.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
# RFC-015 — Splitting a joined artist credit, and a page for what needs a decision

- **Status**: Proposed
- **Implemented by**: in progress, in the order of "The order of the work"
below: [#315](https://github.com/InstaZDLL/waveflow-server/pull/315) (this
RFC). The *Status* field above never flips in this project; this line is the
one that says what is running.
- **Implemented by**: [#315](https://github.com/InstaZDLL/waveflow-server/pull/315)
(this RFC) and [#316](https://github.com/InstaZDLL/waveflow-server/pull/316)
(steps 2 to 4 together: the decision table and `track_credit`, the second
step in the scan, applying and undoing a decision, favourites and ratings
through the journal, the "Needs attention" routes and track categories, the
web client's page and the artist page's "Split into N artists"). The
*Status* field above never flips in this project; this line is the one that
says what is running.
- **Date**: 2026-10-24
- **Revised**: 2026-10-24, after review on #315. The server keeps each
track's credits as the tags give them (`track_credit`), and one function
Expand Down Expand Up @@ -416,7 +420,7 @@ its category at once. Only available tracks are counted.
| `missing_track_number` | effective track number `NULL` or `<= 0` |
| `missing_cover` | neither the track nor its album has artwork |
| `album_year_conflict` | its album's available tracks carry more than one non-zero effective year |
| `compilation_no_album_artist` | flagged a compilation, and the file names no album artist (it was filed under `Various Artists` by the fallback) |
| `compilation_no_album_artist` | its album is flagged a compilation and its album artist is `Various Artists`. That is the fallback for a compilation whose files name no album artist, and the server cannot tell it from a file that writes those words itself, so both are listed |
| `duplicate_track_number` | another track of the same album has the same `COALESCE(disc, 1)` and the same number `> 0` |
| `track_number_gap` | on its disc, `MAX - MIN + 1 > COUNT(DISTINCT number)` |
| `probable_duplicate` | same normalised title and same credited artists, durations chained within 2 seconds. This reuses `waveflow_core::metadata::name_match::normalize_name` and `waveflow_core::inventory::chain_by_duration`, both public at the pinned revision |
Expand Down Expand Up @@ -451,7 +455,7 @@ caller is not an owner or manager of, and for an id not in it.
| --- | --- |
| `GET /libraries/{id}/attention` | `[{ category, count }]` for `split_credits` and each track category, in display order. |
| `GET /libraries/{id}/attention/credits` | The candidates of decision 8, sorted as stated, with their fragments. |
| `GET /libraries/{id}/attention/tracks?category=&offset=&limit=` | The tracks in one category, as the song items `GET /libraries/{id}/tracks` already returns. `probable_duplicate` adds a group key to each item, so a client can draw the groups. `422` for an unknown category. |
| `GET /libraries/{id}/attention/tracks?category=&offset=&limit=` | The tracks in one category, as the song items `GET /libraries/{id}/tracks` already returns. `probable_duplicate` adds `duplicate_group`, numbered from one, to each item, so a client can draw the groups. `422` for an unknown category. |
| `GET /libraries/{id}/credit-decisions` | The decisions, newest first: id, credit, decision, parts, who and when. |
| `POST /libraries/{id}/credit-decisions` | `{ "artist_id": …, "decision": "split" \| "keep" }`. The artist must be a candidate. A split takes its fragments as decision 2 computes them now, and the server stores them. `201` with the decision. `409` when the credit already has one (remove it first). `422` for an artist that is not a candidate. |
| `DELETE /libraries/{id}/credit-decisions/{decision_id}` | Decision 7. `204`. |
Expand Down
64 changes: 64 additions & 0 deletions migrations-v2/20261024020000_artist_credit_decision.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
-- Splitting a joined artist credit, by decision (RFC-015).
--
-- A comma is not a separator the scanner may cut on by itself: `Tyler, The
-- Creator` is one artist and `Zedd, Alessia Cara` is two, and nothing in the
-- tags says which. So a person decides, once per library, and the decision is
-- a row every scan reads back — which is what the desktop's own list never
-- had, and why its splits came undone whenever a file changed.

-- One decision per credit and library. The key is the credit folded the way
-- the artist identity folds a name (lower case, then `pid::str_clear`), so it
-- names exactly the spellings that make one artist row, and it survives a
-- change of `WAVEFLOW_PID_ARTIST` that an artist id would not.
--
-- `parts` holds the fragments as they were shown and accepted, so a recorded
-- split keeps its meaning if the fragment rule ever changes. `decided_by` is an
-- account id without a foreign key, as `artist_image_choice.set_by` is: an
-- account removed later does not take the library's decision with it.
CREATE TABLE artist_credit_decision (
id TEXT PRIMARY KEY NOT NULL,
library_id TEXT NOT NULL REFERENCES library(id) ON DELETE CASCADE,
credit_key TEXT NOT NULL CHECK (length(credit_key) > 0),
credit TEXT NOT NULL CHECK (length(trim(credit)) > 0),
decision TEXT NOT NULL CHECK (decision IN ('split', 'keep')),
parts TEXT CHECK (
(decision = 'split' AND json_type(parts) = 'array')
OR (decision = 'keep' AND parts IS NULL)),
decided_by TEXT NOT NULL,
decided_at INTEGER NOT NULL,
UNIQUE (library_id, credit_key)
) STRICT;

-- The credits as the tag mapper produced them, before any decision: what a
-- decision is applied to, and what an undo goes back to. The participants
-- became a function of this table and of the decisions, so taking or removing
-- a decision needs no file read, and two credits sharing a part (`A, B` and
-- `A, C`) keep the file's order whichever is undone first.
--
-- Positions are per role, across sub-roles, as in `track_participant`: the
-- performers of one track are numbered in one sequence whatever their
-- instruments.
CREATE TABLE track_credit (
track_id TEXT NOT NULL,
library_id TEXT NOT NULL,
role TEXT NOT NULL CHECK (length(trim(role)) > 0),
sub_role TEXT NOT NULL DEFAULT '',
position INTEGER NOT NULL CHECK (position >= 0),
name TEXT NOT NULL CHECK (length(trim(name)) > 0),
sort_name TEXT,
PRIMARY KEY (track_id, role, position),
FOREIGN KEY (track_id, library_id)
REFERENCES track(id, library_id) ON DELETE CASCADE
) STRICT;

-- No decision exists yet, so the credits are what the participants already
-- say. The copy loses only a later credit naming the same artist as an earlier
-- one, which the participants never held and which the computation would drop
-- again by keeping the first occurrence; the next scan of that file writes it
-- back. Under a correction the copied `artist` rows are the corrected list
-- rather than the file's, which nothing reads while the correction stands, and
-- removing a correction re-reads the file. So no rescan is scheduled.
INSERT INTO track_credit (track_id, library_id, role, sub_role, position, name, sort_name)
SELECT tp.track_id, tp.library_id, tp.role, tp.sub_role, tp.position, ar.name, tp.sort_name
FROM track_participant tp
JOIN artist ar ON ar.id = tp.artist_id;
Loading
Loading