From c4e055ce51f55f51df220737a1f07a8fc32c3ede Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 00:50:53 -0400 Subject: [PATCH] docs: write down which of the two ownership columns to read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `localId` / `inLibrary` choice outlives #38 — every future catalog surface has to make it, and the code read them as an OR at eight call sites precisely because nothing said they were different kinds of thing. CLAUDE.md gets the rule and its four load-bearing details; NOTES.md gets the measurement, the card that used both answers at once, and the alternative that was rejected. --- .planning/NOTES.md | 46 ++++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 53 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/.planning/NOTES.md b/.planning/NOTES.md index e2687de..f5b106b 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -3534,3 +3534,49 @@ silent.** `artifactHasTotals` and `artifactHasCredits` are both correct and both mean a feature can ship, pass every test, and produce nothing for anybody without a single failure anywhere. Checking the *published file* is one query and is not implied by any tick in CI. + +## "Do I own this" has two answers in the schema, and one of them is a flag (2026-08-19) + +Decided while doing #38, and it outlives it because every future +catalog surface has to pick one. + +`explore_index` carries both `in_library` and `local_artist_id` / +`local_release_group_id` / `local_recording_id`. They are written by +the same pass (`collectLibraryEntities`), so on a healthy database they +agree, and the code read them as an OR — `inLibrary || localId > 0` — +at eight call sites. + +They are not the same kind of thing: + +- **`local_*_id` is a fact with an owner.** Every query that sets one + joins `audio_files`, and `pruneStaleLocalCrossReferences` clears it + with an existence test that is a file test in all three cases. It is + the same rule `explore-album-details`'s `filePaths` implements, one + layer down and computed once per scan. +- **`in_library` is a ratchet.** `upsertBatch` raises it with + `MAX(in_library, excluded.in_library)` and the prune is the only + thing that lowers it — gated on the local id being non-null, so a row + holding the flag *without* an id is a fixed point nothing can clear. + Filed as #118; it still drives search scoring, the popularity-floor + bypass and two Explore shelves, so routing the UI around it was not a + fix. + +What made the choice concrete rather than theoretical: on +`explore-artist-details` the *same card* used both. The context menu +gated Play on `localId > 0`; the badge used `inLibrary`. An album with +the flag and no local row drew a green tick saying it was in your +library, offered no Play, and — the request item being gated on *not* +owned — offered no way to ask for it either. + +The rejected alternative is worth keeping: batching a real file lookup +per screenful, the way `credit-store` coalesces. It would have answered +for **recordings** (`GetFilePathsByRecordingMBIDs`) and most of the +cards on these surfaces are release groups, so it would have made track +rows strong, left album cards exactly where they were, and cost a new +store. The batch that *was* worth adding is a different question — +`GetAlbumsCompleteness`, "how much of this album is here", which no +per-card flag can answer at all. + +The general point: **two columns that agree today are not one column.** +Which of them a new surface reads should be decided by which one has +something that can un-set it. diff --git a/CLAUDE.md b/CLAUDE.md index f4d804e..9b408d5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1658,6 +1658,59 @@ not about plumbing — it says rows may be missing from the page altogether, which nothing on screen can show. (`explore-artist-details` still uses `loading`; it has no equivalent per-row signal.) +**And that treatment is the app's, not the page's.** +`utils/ownership.ts` is the rule written once, because it was written +at eight call sites and so none of them had the whole of it: Explore's +cards, `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 +the badge on the *owned* ones was a green tick — the mark on the common +case this tracklist removed. Owned is plain and draws no badge at all; +unowned is dimmed, says so in its accessible name, and keeps its +request affordance; a partly-held album says how partly. + +Four things about it are load-bearing. + +**Ownership is `localId`, and `inLibrary` is deliberately not +consulted.** The album page answers with `filePaths`, a real file per +displayed track, and a card grid cannot afford that — but it does not +need to, because `explore_index.local_*_id` is built by +`collectLibraryEntities` from queries that every one join `audio_files` +and cleared by `pruneStaleLocalCrossReferences`, whose existence test +is a file test in all three cases. That is the same "ownership is a +file" rule computed once per scan instead of once per screenful. +`in_library` is written by the same pass, so the two agree in a healthy +database, but it is a one-way ratchet +(`MAX(in_library, excluded.in_library)`) whose only clearing pass is +gated on a non-null local id: it cannot be un-set on its own (#118). +One is a fact with an owner; the other is a flag that happens to agree. +Both `explore-view` and `explore-artist-details` additionally kept a +`libraryMBIDs` set that accumulated every MBID ever seen with the flag +and cleared it never, in views that never unmount; both are gone. + +**The two answers used to sit on one card.** +`renderReleaseMenuItems` gates Play on `release.localId > 0` while the +badge used `inLibrary`, so an album with the flag and no local row drew +a tick saying it was in your library, offered no Play, and — the +request item being gated on *not* owned — offered no way to ask for it +either. Any new surface that asks the question twice will reproduce it. + +**`aria-disabled` goes on rows and not on cards.** An unowned *row* +cannot be activated; an unowned *card* still navigates to the catalog +page for it, which is a perfectly good thing to do with something you +do not own. The accessible name carries the state either way, which is +why it is one helper and not a class. + +**The count is batched, not looked up.** `store/completeness-store.ts` +is `credit-store` one question over: `request()` is per-card and +coalesces a screenful into one `GetAlbumsCompleteness`, absence is +cached as an answer (or the albums with no totals re-ask forever), and +the whole cache is dropped on a scan, a retag or a removal rather than +aged. `library-status.ts`'s `albumBadgeFor` is where that meets +`Known`: a total that was never declared is a plain `in-library`, never +a ring at 0%. One consequence in the badge itself — a `partial` badge +is *actionable*, and a control named after its action alone dropped the +count from the one state the ring exists for, so its name is both. + **A partly-owned album draws the release, not the part.** Once the tags say nine of twelve, `buildLibraryEntry` shows the *catalog's* twelve with three dimmed, rather than the nine on disk — the missing tracks