diff --git a/.planning/audits/2026-08-11-ui/hands-on.md b/.planning/audits/2026-08-11-ui/hands-on.md index 08edb1b..e768072 100644 --- a/.planning/audits/2026-08-11-ui/hands-on.md +++ b/.planning/audits/2026-08-11-ui/hands-on.md @@ -13,7 +13,7 @@ reviews. Nothing was changed. Findings below are numbered `H-n` (hands-on) and cross-reference the static reports where they overlap. The reconciliation plan built from -all four files is `.planning/plans/pending/007-ui-reconciliation.md`. +all four files is `.planning/plans/completed/007-ui-reconciliation.md`. --- diff --git a/.planning/plans/pending/012-api-call-audit.md b/.planning/plans/completed/012-api-call-audit.md similarity index 98% rename from .planning/plans/pending/012-api-call-audit.md rename to .planning/plans/completed/012-api-call-audit.md index cf65f80..92f972a 100644 --- a/.planning/plans/pending/012-api-call-audit.md +++ b/.planning/plans/completed/012-api-call-audit.md @@ -1,5 +1,7 @@ # 012 — What we ask the network for, and what we already had +> **Completed.** Findings 1, 2 and 4 shipped. Finding 3 — the bound-but-uncalled methods — is now **#86**. + **Status:** all four findings fixed. Lint (3 configs), Go tests (3 configs), `tsc` and 752 Vitest tests pass; **not driven against the real app**, so the numbers below are read off the code, not measured. diff --git a/.planning/plans/active/015-android-release-pipeline.md b/.planning/plans/completed/015-android-release-pipeline.md similarity index 99% rename from .planning/plans/active/015-android-release-pipeline.md rename to .planning/plans/completed/015-android-release-pipeline.md index e26ed59..7098c25 100644 --- a/.planning/plans/active/015-android-release-pipeline.md +++ b/.planning/plans/completed/015-android-release-pipeline.md @@ -1,5 +1,7 @@ # 015 — Android release pipeline +> **Completed.** The pipeline ships a signed APK from CI on every `v*` tag; `docs/android-release.md` is its operating document. + Ship an Android APK from CI on every version tag, published to the Gitea generic package registry so Obtainium can poll a plain URL. diff --git a/.planning/plans/active/015-multi-artist-credits.md b/.planning/plans/completed/015-multi-artist-credits.md similarity index 98% rename from .planning/plans/active/015-multi-artist-credits.md rename to .planning/plans/completed/015-multi-artist-credits.md index 7ac3c23..a00477e 100644 --- a/.planning/plans/active/015-multi-artist-credits.md +++ b/.planning/plans/completed/015-multi-artist-credits.md @@ -1,5 +1,7 @@ # 015 — Multi-artist credits, navigable +> **Completed.** Phases 1, 2 and 4 shipped. Running the ingest against the real dump and publishing an artifact that carries credits is **#88**; Phase 3 (`file_artists`) is **#89**, blocked on it. + ## The problem A track credited to more than one artist has exactly one navigable diff --git a/.planning/plans/pending/016-android-feature-parity.md b/.planning/plans/completed/016-android-feature-parity.md similarity index 99% rename from .planning/plans/pending/016-android-feature-parity.md rename to .planning/plans/completed/016-android-feature-parity.md index 4a3fbf2..e34fd57 100644 --- a/.planning/plans/pending/016-android-feature-parity.md +++ b/.planning/plans/completed/016-android-feature-parity.md @@ -1,5 +1,7 @@ # 016 — What Android parity would actually take +> **Completed.** Sections A, B1, B2 and B4 shipped. B3, writing tags on the device, is now **#87**; the device-found UI faults are #51–#72, sequenced by #73. + > **Status: all of section A is done.** A1–A3 landed with "let the app > reach the user's music"; A4 (MediaSession, transport notification, > audio focus) landed with "survive the screen locking". The direction diff --git a/.planning/autotag.md b/.planning/plans/completed/autotag-v1.3.md similarity index 97% rename from .planning/autotag.md rename to .planning/plans/completed/autotag-v1.3.md index 268ab99..b046b51 100644 --- a/.planning/autotag.md +++ b/.planning/plans/completed/autotag-v1.3.md @@ -1,5 +1,7 @@ # Autotag (v1.3) — MusicBrainz Autotagger +> **Historical record.** Phases 008–010 shipped, and the scoring engine was subsequently overhauled (`recommend.go`, `rank.go`, `mixedbag.go`), which makes the 011/012 sections below stale in their details. What is actually left is **#90** (auto-accept and entry points) and **#91** (settings, and a way back from the dismissed file-write warning). + The MusicBrainz autotagger, collectively **v1.3**. Builds on the explore-browser API client + cache foundation. Five sequential phases (008–012), each depending on the prior one. | Phase | Title | Status | diff --git a/.planning/plans/pending/010-owned-album-catalog-offline.md b/.planning/plans/pending/010-owned-album-catalog-offline.md deleted file mode 100644 index 90d0252..0000000 --- a/.planning/plans/pending/010-owned-album-catalog-offline.md +++ /dev/null @@ -1,195 +0,0 @@ -# 010 — Owned albums, offline - -**Status:** not started — and **much smaller than when it was written** -**Branch:** none yet -**Created:** 2026-08-13 -**Depends on:** nothing -**Related:** the `AlbumReleasesFailed` fix that prompted it, and the -tag-derived completeness that landed after it (same session) - ---- - -## What already shipped, and what it leaves - -The common case is solved without this plan. `GetAlbumCompleteness` -reads the "5/12" denominator off the files' own tags — persisted to -`release_group_recordings.total_tracks`, having been extracted at every -scan since forever and discarded — and an album that is **MBID-matched -and complete** now opens with **no catalog call at all**. Identity from -the MBID, tracklist from the tags; those were the two things the browse -was being spent on. - -So the set this plan still has to serve is not "albums you own a track -of". It is: - -- albums that are genuinely **incomplete** (the catalog is the only way - to say *which* tracks are missing — tags give the count, not the - names), and -- albums whose tags **never declared a total**, where completeness is - unknowable locally and the catalog is the only source. - -On a well-tagged library that is a small minority, which changes the -economics below considerably: the run is shorter, and the rate limiter -contention that dominates this design is proportionally less severe. -Re-measure before building — the answer may now be "the prefetch is -enough". - ---- - -## The problem - -Opening an album detail page for an album **you already own** hits -MusicBrainz. Every time it is not in the response cache, which for most -of a library is every time, because nothing warms that cache except a -capped prefetch on the artist page. - -The user's framing: *this is a classic example of an album we should -have had locally.* - -## Why we do not have it, despite the discography backfill - -`BackfillLibraryDiscographies` / `EnsureArtistDiscography` -(`backend/explore/searchindex.go:301`, `:397`) do less than the name -suggests. Per artist, `indexOneArtist` fetches: - -- `fetchTopReleaseGroups` — capped at `indexMaxRGs` (50) -- `fetchTopRecordings` — capped at `indexMaxRecs` (200) - -and writes them as **flat `explore_index` rows**. There is no release -group → tracklist relation anywhere in the index, and no release-level -rows at all. `explore_index` recordings carry `caa_release_mbid` and -`release_name`, which name the release used for cover art — not a -tracklist. - -So "we have full discographies for library artists" means *we know -which albums the artist made, offline*. It has never meant we know -what is on any of them. - -The only store of release-level catalog data in the app is `http_cache` -under `mb:browse:releases:` (90-day TTL, `musicbrainz.go:27`), -populated **only** by a live `BrowseReleases` with -`Includes: ["recordings", "media"]` at `MaxLimit` — the most expensive -call the app makes to MusicBrainz. It is warmed by exactly one thing: -`PrefetchReleases` (`explore.go:746`), capped at 8, called only when an -artist page renders. - -An album opened from the library grid therefore always browses live. - -## What to build - -**A post-scan backfill that warms the release cache for release groups -that are owned but not known-complete** — bounded, resumable, and -shaped exactly like `BackfillLibraryDiscographies`, which is the proven -pattern for this in the codebase. - -The scoping rule is the user's and it is the right one: not "every -album by every artist in the library" (50 release groups per artist, -mostly never opened) but albums with owned tracks — narrowed further, -now, to the ones a local answer cannot already cover. The query gains -one clause: skip release groups whose `GetAlbumCompleteness` reports -`complete`. - -Sketch: - -1. A query for release groups with ≥1 owned track and no warm release - cache entry. `release_groups.mbid` is the key; the owned-track join - is `audio_files → recordings → release_group_recordings`, the same - shape `unenrichedLibraryArtistMBIDs` already uses one table over. -2. Order by owned-track count descending, so the albums the user has - most of are warmed first — same reasoning as the discography - backfill's ordering, same benefit if a run is cut short. -3. Run through `releasesSF`, so it never double-fetches a release group - an interactive open is already handling. -4. Bound a run (`discogBackfillMaxPerRun` has a value to copy) and make - it resumable: the resume marker is the response cache itself — - `BrowseReleasesCached` already answers "is this one done", so unlike - the discography path this needs **no new flag column**. -5. Trigger it where `BackfillLibraryDiscographies` is triggered, and - register it with `jobs` so it has progress, pause and cancel like - every other long-running operation. - -### The rate limiter is the whole design constraint - -> **Update (2026-08-13): the priority half is built, and the sentence -> below is wrong on a detail.** `e.mb` runs on `mbSearchLimiter` -> (`NewRateLimiterBurst(3, 1)`); the 1 req/s `NewRateLimiter()` cited -> here is the *artist image* limiter. Both are shared and both were -> FIFO. `RateLimiter.WithBackgroundLane` + `WithBackgroundPriority(ctx)` -> now make a marked caller yield to interactive work and pace at 1/s, -> and `jobs.KindCatalogEnrich` + `startBackfillJob` give the existing -> backfills progress and cancel. **"Do not start until the priority -> question has an answer" is satisfied** — mark this backfill's context -> and register it the way `BackfillLibraryDiscographies` now is. -> `PrefetchReleases`' cap of 8 is still unrevisited. - -One shared `NewRateLimiter()` at 1 req/s (`explore.go:84`) serves this, -`PrefetchReleases`, and every interactive browse. A backfill over a -few thousand owned albums is *hours* of wall clock at that rate — which -is fine for a background job, and not fine if it starves the album page -the user is looking at right now. - -That is the real work in this plan, and it is not the query: - -- Interactive browses need to **jump the queue**. Today they cannot; - there is one limiter and it is FIFO. -- `PrefetchReleases`' cap of 8 was sized when nothing else competed for - the limiter. Revisit it in the same change. -- The 60 s fallback the `AlbumReleasesFailed` fix installed is sized - for today's contention. If a backfill can queue behind it, that - number is wrong again — which is an argument for priority, not for a - bigger number. - -Do not start the query until the priority question has an answer. - -## The alternative that was considered and rejected - -**Project release-group tracklists in the dump build and ship them in -the artifact.** The data is there: `canonical_musicbrainz_data.csv` -carries `release_mbid` *and* `recording_mbid` -(`dumpcatalog.go:520`), and `release_to_rg` already maps release → -release group. It is derivable from bytes the index build already -streams, with no new API surface at all, and it would work offline on -first launch with no per-user backfill. - -It is rejected **for this plan** because the artifact is built -centrally and is byte-identical for every user, so "albums the user -owns a track of" cannot be a filter on it. Shipping tracklists for the -whole catalog means per-recording rows against a ~900 MB artifact -budget (~426 B/row measured), and gating on a popularity floor means it -is absent for exactly the obscure albums a local backfill would have -covered. - -Worse than absent, in fact — and this is the argument that actually -kills it. The floor is not one number over artists; it is a **per -artist track budget** (`dumpcatalog.go:58-89`): 50 tracks for a tier-A -artist, 25 for tier B, 12 for tier C. A projected tracklist would -therefore be *whichever* of an album's tracks survived that budget, -with nothing marking the rest as absent — so the album page would count -owned against a truncated denominator and render "Play 7 of 9" for a -twelve-track album. That is a confident lie, where the honest states -this plan's alternative produces (complete / incomplete / unknown) are -at worst silent. - -Note that `markLibraryArtists` (`dumpcatalog.go:246`) already grants -every library artist full coverage — 500 tracks, 100 release groups — -by reading the local library, so the per-user tailoring this option -supposedly cannot have does exist in code. It is a no-op in the CI -build (empty library), and reaching it means a **local** dump build: -the ~205 GB, half-a-day download the entire artifact design exists to -avoid. Whoever finds that function next should read this paragraph -before getting excited about it. - -Worth revisiting if the artifact ever gains per-user tailoring, or if a -measurement shows the row count is smaller than feared. Note it also -yields the *canonical* tracklist rather than MusicBrainz's full version -list, so the versions dropdown would still browse live when opened. - -## Done when - -- Opening an owned album that has never been opened before renders its - catalog tracklist with no network call, after one backfill run. -- An interactive browse issued while the backfill is running is not - delayed by it. -- The backfill appears in the jobs indicator, and can be paused and - cancelled there. -- A second run after a completed one does approximately nothing. diff --git a/CLAUDE.md b/CLAUDE.md index d5634d1..57c6f60 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,16 +6,85 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co YellowJacket is a cross-platform desktop music player built with Go (backend) and TypeScript/Lit (frontend), using the Wails framework to bridge them. It supports MP3, FLAC, OGG Vorbis, and WAV playback. +## Issues + +**The tracker is the source of truth for what is wanted and what is +already being worked on**, and it is shared with a collaborator who +cannot see this session. `scripts/issue.sh` is the whole interface to +it (`list`, `mine`, `search`, `show`, `new`, `claim`, `unclaim`, +`comment`, `close`, `label`, `depends`, `labels`); it needs a +`GITEA_TOKEN` with `write:issue`. + +**Search the tracker before starting any work, and claim what you +find.** Fifty-odd issues make that a real lookup rather than a +formality. `./scripts/issue.sh search ` covers open and closed — +closed matters, because "that was fixed three weeks ago" is the +cheapest possible answer. + +**Claiming happens before the first edit, not before the commit.** The +whole point is that the collaborator can see the work is taken *while +it is being done*, so `claim` sets the assignee, applies +`Status/In Progress` and posts a comment naming the branch and the +approach — all three, or none. It refuses outright if somebody else +already holds it, and that refusal is the feature: talk to them rather +than working around it. + +**If no issue covers the work, open one first.** The issue exists +before the branch does. That is what makes the tracker a description +of the project rather than a description of the past. + +**Findings get filed.** A bug tripped over while doing something else +is an issue with a reproduction, not a sentence in a chat message +nobody can search. So is a piece of work deliberately not done — the +issue is where "we decided not to, and here is why" survives. + +Four conventions are already established and are not up for +reinvention: + +- **The labels are a taxonomy**, not tags: `Kind/*`, `Area/*`, + `Priority/*`, `Platform/*`, plus `Reviewed/Confirmed` (the code was + read and the defect confirmed) and the `Status/*` family. `Status/*` + and `Reviewed/*` are **exclusive scopes** — one of each at most, so + applying a second replaces the first. +- **#73 is the roadmap.** It states the order the backlog should be + worked in and the soft relations that are not expressible as + blockers. Picking work off the open list by eye when a meta issue + states the sequence is how the sequence stops meaning anything. +- **Hard blockers are real Gitea dependencies**, which render on the + issue itself, and the blocked issue carries `Status/Blocked`. +- **A PR body carries a commit-to-issue table, the verification + actually run, and a `Closes` list** — PR #83 is the shape. + +**And the `Closes` list does not reliably close anything.** #83 listed +ten and five of them stayed open, shipped in `main`, for a fortnight. +So closing is a step you take and check, not a keyword you trust: +`./scripts/issue.sh close ` after the merge, with a comment naming +the commit that shipped it. `close` also drops `Status/In Progress`, +because a claim outlives the work if nothing takes the label off. + ## Planning -Active and historical plans live in `.planning/`: +`.planning/` is **design documents and measured history**, not a queue +— the queue is the tracker, and a plan file that describes work nobody +has started is a second, staler answer to "what are we doing next". -- `.planning/NOTES.md` — gotchas, deferred items, open architecture questions, the "we already considered and rejected" list. -- `.planning/plans/active/` — work currently in progress (read first). -- `.planning/plans/pending/` — sequenced future work. -- `.planning/plans/completed/` — one concise recap per shipped milestone. +- `.planning/NOTES.md` — gotchas, measured facts, open architecture + questions, and the "we already considered and rejected" list. Dated, + because several are properties of someone else's server. **This is + where a decision reached on an issue gets written down** when it + outlives the issue. +- `.planning/plans/completed/` — one recap per shipped milestone, kept + for the arguments in it. Where a plan shipped incompletely, its + header says which issue carries the remainder. +- `.planning/audits/` — the read-only audits that produced the + reconciliation plans. Historical evidence; not a backlog. +- `.planning/plans/active/` — a multi-phase design document for work + **in flight**, linked from the issue that tracks it. Empty is the + normal state. There is no `pending/`: a plan nobody is executing is + an issue. -Numbering is sequential and stable across status moves (a plan keeps its `NNN-` prefix as it migrates between `pending → active → completed`). Abandoned plans are deleted; paused work stays in `pending/`. +Numbering is sequential and stable across status moves (a plan keeps +its `NNN-` prefix). Abandoned plans are deleted. ## Commands @@ -2066,6 +2135,17 @@ branch** (`enable_push: false`, an empty push whitelist, and `CI / check*` the pre-receive hook. This file said otherwise for a long time. Tags are *not* protected, which is what lets `release.yml` push one. +**A branch answers a claimed issue** — see "Issues" above. The commit +grammar is unchanged and is load-bearing for a different reason +(semantic-release reads it), so the issue number lives in the branch +name and the PR body rather than in the commit subject. + +**A batch of small fixes can be one PR**, which is what #83 did: eight +branches preserved as merges under one integration branch, so +authorship survives and the batch lands as one release rather than +eight. The cost is that its `Closes` list has to be checked afterwards +— it half-worked. + Pre-commit runs vet, lint, codegen check, and frontend typecheck in parallel. Pre-push runs the full test suite. ## CI