the rule in CLAUDE.md, the measurement and the rejected alternative in NOTES.md
The decision this turned on
The issue asked whether the grids accept the catalog's inLibrary flag
and say so in the copy, or whether something batches a real file lookup
per screenful the way credit-store.ts coalesces.
Neither: the grids take localId, which is already on every model. collectLibraryEntities builds local_artist_id / local_release_group_id / local_recording_id from queries that every
one join audio_files, and pruneStaleLocalCrossReferences clears them
with an existence test that is a file test in all three cases — the same
"ownership is a file" rule the album page's filePaths implements,
computed once per scan instead of once per screenful.
inLibrary is written by the same pass, so in a healthy database the
two agree. What differs is that it is a one-way ratchet
(in_library = 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. One of the two is a fact with an owner; the other is a flag
that happens to agree with it.
A batched GetFilePathsByRecordingMBIDs would also only reach half the
problem: it answers for recordings, and most of the cards on these
surfaces are release groups.
Two bugs this found
A card answered "do I own this" twice and got two answers. On explore-artist-details, renderReleaseMenuItems gates Play on release.localId > 0 while the badge and albumTarget.owned used libraryMBIDs.has(mbid) || rg.inLibrary || localId > 0. An album with
the flag and no local row therefore drew a green tick saying it was in
your library, offered no Play, and — the request item being gated on !release.owned — offered no way to ask for it either.
A partial badge that can act had no count in its name. A control is
named after what activating it does, so an actionable badge said
"Request album X"; partial is actionable (three tracks left to ask
for), which made the one state the ring exists for the one state whose
name did not mention it. library-status-indicator's own documentation
already argues the count is the point — "a ring says some to a
sighted user and nothing to anyone else" — and that does not stop
applying because the badge became clickable.
What the rule is now, in utils/ownership.ts
Owned is plain. No badge at all on an owned card or row, which is
what explore-album-details already did with its tracklist and what
its green ticks were removed for.
Unowned is dimmed and says so. Text to --yj-text-secondary
rather than an opacity multiplier, because theme-contrast.test.ts
checks the ramp and not a multiplier on top of it; artwork takes the
opacity, since a cover is not text.
aria-disabled goes on rows and deliberately 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 in
both cases.
Partial says how partial, from completenessStore — shaped like credit-store: request() is per-card and coalesces a screenful into
one call, absence is cached as an answer, and the cache is dropped on
a scan, a retag or a removal rather than aged.
Both libraryMBIDs sets are deleted. They accumulated every MBID ever
seen with inLibrary and cleared it never, in views that never unmount.
Audited and unchanged
home-view, downloads-view, cover-grid, artist-details and genre-details cannot show catalog content — none of them reads inLibrary or localId at all — so everything on them is owned and
"owned is plain" is already what they do. explore-album-details's
header badge stays: that page is about one entity and the badge is its
answer, rather than a mark on one of many.
Verification actually run
make generate # clean
make bindings-check # frontend/bindings is current
make lint # 0 issues, all three tag sets
make test # all three passes green
make ui-test # 871 passed (76 files), up from 855
frontend/node_modules/.bin/tsc --noEmit # clean
make skill-check # 47 targets, all present
unowned-everywhere.test.ts is 15 new specs covering the rule rather
than any one surface. The name assertions go through page.getByRole(...) — the browser's own answer — because a shadow-root
query cannot compute a name, and this repo has shipped a nameless
control three times.
One thing the tests found on their own: completenessStore is a
singleton that caches the absent answer (deliberately, or 87% of a
grid re-asks forever), so two specs in one file are two sessions as far
as it is concerned. beforeEach invalidates it, with the reason
written down.
Not done here
The roadmap pairs #17 ("demote the release-version selector") with this
as "same page, same pass". It is a different page — explore-album-details
rather than the card surfaces — and is left for its own change.
Filed on the way: #118.in_library can never be cleared on a row
that has no local id — MAX(in_library, excluded) only raises it and
the prune is gated on the id being present, so the pair is a fixed
point. Nothing in the tree produces one today, but the flag still
drives search scoring, the popularity-floor bypass and two Explore
shelves, so routing the UI around it (which is all this PR does) is not
a fix. Reproduction is in the issue.
Releases are manual now (#115), so merging this cuts no version.
Closes #38 — the Phase 1 item the roadmap (#73) puts after #16, #7 and
#34, all merged.
## Commits
| commit | issue | what |
| --- | --- | --- |
| `4bf59b4` feat(library): answer album completeness for a screenful in one query | #38 | `GetAlbumsCompleteness(albumIDs)` — one query for a grid, so a card can say "9 of 12" |
| `41c41a8` feat(explore): carry the local row id on a top result | #38 | `TopResult` was the one projection shipping `inLibrary` with no `LocalID` |
| `19c68d7` fix(ui): keep the count in a partial badge that can act | #38 | an actionable `partial` badge was named "Request album X" and dropped the count |
| `88fc50a` feat(explore): mark what is not owned, everywhere it can be shown | #38 | the sweep, and `utils/ownership.ts` — the rule written once |
| `10eca35` fix(explore): gate playback on the same answer the row is drawn from | #38 | two play paths still accepted `inLibrary` |
| `c4e055c` docs: write down which of the two ownership columns to read | #38 | the rule in CLAUDE.md, the measurement and the rejected alternative in NOTES.md |
## The decision this turned on
The issue asked whether the grids accept the catalog's `inLibrary` flag
and say so in the copy, or whether something batches a real file lookup
per screenful the way `credit-store.ts` coalesces.
**Neither: the grids take `localId`, which is already on every model.**
`collectLibraryEntities` builds `local_artist_id` /
`local_release_group_id` / `local_recording_id` from queries that every
one join `audio_files`, and `pruneStaleLocalCrossReferences` clears them
with an existence test that is a file test in all three cases — the same
"ownership is a file" rule the album page's `filePaths` implements,
computed once per scan instead of once per screenful.
`inLibrary` is written by the same pass, so in a healthy database the
two agree. What differs is that it is a one-way ratchet
(`in_library = 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. One of the two is a fact with an owner; the other is a flag
that happens to agree with it.
A batched `GetFilePathsByRecordingMBIDs` would also only reach half the
problem: it answers for **recordings**, and most of the cards on these
surfaces are release groups.
## Two bugs this found
**A card answered "do I own this" twice and got two answers.** On
`explore-artist-details`, `renderReleaseMenuItems` gates Play on
`release.localId > 0` while the badge and `albumTarget.owned` used
`libraryMBIDs.has(mbid) || rg.inLibrary || localId > 0`. An album with
the flag and no local row therefore drew a **green tick saying it was in
your library**, offered no Play, and — the request item being gated on
`!release.owned` — offered no way to ask for it either.
**A partial badge that can act had no count in its name.** A control is
named after what activating it does, so an actionable badge said
"Request album X"; `partial` is actionable (three tracks left to ask
for), which made the one state the ring exists for the one state whose
name did not mention it. `library-status-indicator`'s own documentation
already argues the count is the point — "a ring says *some* to a
sighted user and nothing to anyone else" — and that does not stop
applying because the badge became clickable.
## What the rule is now, in `utils/ownership.ts`
- **Owned is plain.** No badge at all on an owned card or row, which is
what `explore-album-details` already did with its tracklist and what
its green ticks were removed for.
- **Unowned is dimmed and says so.** Text to `--yj-text-secondary`
rather than an `opacity` multiplier, because `theme-contrast.test.ts`
checks the ramp and not a multiplier on top of it; artwork takes the
opacity, since a cover is not text.
- **`aria-disabled` goes on rows and deliberately 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 in
both cases.
- **Partial says how partial**, from `completenessStore` — shaped like
`credit-store`: `request()` is per-card and coalesces a screenful into
one call, absence is cached as an answer, and the cache is dropped on
a scan, a retag or a removal rather than aged.
Both `libraryMBIDs` sets are deleted. They accumulated every MBID ever
seen with `inLibrary` and cleared it never, in views that never unmount.
## Audited and unchanged
`home-view`, `downloads-view`, `cover-grid`, `artist-details` and
`genre-details` cannot show catalog content — none of them reads
`inLibrary` or `localId` at all — so everything on them is owned and
"owned is plain" is already what they do. `explore-album-details`'s
header badge stays: that page is about one entity and the badge is its
answer, rather than a mark on one of many.
## Verification actually run
```
make generate # clean
make bindings-check # frontend/bindings is current
make lint # 0 issues, all three tag sets
make test # all three passes green
make ui-test # 871 passed (76 files), up from 855
frontend/node_modules/.bin/tsc --noEmit # clean
make skill-check # 47 targets, all present
```
`unowned-everywhere.test.ts` is 15 new specs covering the rule rather
than any one surface. The name assertions go through
`page.getByRole(...)` — the browser's own answer — because a shadow-root
query cannot compute a name, and this repo has shipped a nameless
control three times.
One thing the tests found on their own: `completenessStore` is a
singleton that caches the *absent* answer (deliberately, or 87% of a
grid re-asks forever), so two specs in one file are two sessions as far
as it is concerned. `beforeEach` invalidates it, with the reason
written down.
## Not done here
The roadmap pairs #17 ("demote the release-version selector") with this
as "same page, same pass". It is a different page — `explore-album-details`
rather than the card surfaces — and is left for its own change.
**Filed on the way: #118.** `in_library` can never be cleared on a row
that has no local id — `MAX(in_library, excluded)` only raises it and
the prune is gated on the id being present, so the pair is a fixed
point. Nothing in the tree produces one today, but the flag still
drives search scoring, the popularity-floor bypass and two Explore
shelves, so routing the UI around it (which is all this PR does) is not
a fix. Reproduction is in the issue.
Releases are manual now (#115), so merging this cuts no version.
Closes #38
A card grid has to know how much of an album is here — an album held 2
tracks of 10 wearing the same green tick as one held whole is the
complaint the badge-accuracy work was filed about — and
`GetAlbumCompleteness` is one query per album, which is fifty round
trips for a grid of fifty.
`GetAlbumsCompleteness` is the same question over a slice. It is two
grouping levels rather than the single-album form's correlated
subqueries, because a correlated subquery in the FROM clause is not
something SQLite will reliably do, and because the slice may only be
spelled once or sqlc expands it twice with independently numbered
placeholders.
An album with no files is absent from the result rather than zeroed:
"I have none of this" and "I have no idea" are the third state `Known`
exists to keep apart.
The test that matters is that the two spellings never disagree — they
are genuinely different SQL, so the risk is a drift in meaning (a
disc's total counted once per file, a duplicate counted twice) rather
than a typo.
`TopResult` was the one projection here that shipped `inLibrary` and no
local id, so the top-results cards had no choice but to read the weaker
flag. Every sibling model — `MBArtist`, `MBReleaseGroup`, `MBRecording`
— already carries `LocalID`, and the candidate builders had the value
in hand at every construction site.
`LocalID` is set and cleared by a test against `audio_files`, so it
means "there is something of mine here". `InLibrary` is written by the
same pass but is a one-way ratchet the prune can only clear alongside a
local id; it stays for scoring, which is where an approximate answer is
fine.
A control is named after what activating it does, so an actionable
badge said "Request album X" — and `partial` is actionable, because an
album you hold nine of twelve tracks of has three left to ask for.
That made the one state the ring exists for the one state whose name
did not mention it.
The argument the `partial` branch already carries does not stop
applying because the badge became clickable: a ring says "some" to a
sighted user and nothing to anyone else. The name is now the action and
the count.
`explore-album-details` had the rule right for one tracklist and
nothing else did: 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 drew a green tick on the *common* case —
which is the treatment that tracklist's own green ticks were removed
for.
`utils/ownership.ts` is the rule written once, so eight call sites
stop each holding their own version:
- owned is plain, and draws no badge at all;
- unowned is dimmed *and* says so in its accessible name, because
dimming is a colour and cannot be the only signal;
- a partly-held album says how partly.
**Ownership is a file, and `localId` is the flag that says so.** The
album page answers with `filePaths`, a real file per displayed track; a
card grid cannot afford that and does not need to, because
`local_*_id` is built by queries that all join `audio_files` and
cleared by a prune whose existence test is a file test in every case.
`inLibrary` 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)`)
whose only clearing pass is gated on a non-null local id, so it cannot
be un-set on its own.
Where they already diverged was the client. Both `explore-view` and
`explore-artist-details` kept a `libraryMBIDs` set that accumulated
every MBID ever seen with `inLibrary` and cleared it never, in views
that never unmount. Both are deleted.
And one card answered the question twice and got two answers:
`renderReleaseMenuItems` gates Play on `localId > 0` while the badge
and `albumTarget.owned` 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.
The count comes from `completenessStore`, shaped like `credit-store`:
`request()` is per-card and coalesces a screenful into one
`GetAlbumsCompleteness`, absence is cached as an answer, and the whole
cache is dropped on a scan, a retag or a removal rather than aged.
`aria-disabled` goes on rows that cannot be activated and deliberately
not on cards: 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.
Audited and unchanged: `home-view`, `downloads-view`, `cover-grid`,
`artist-details` and `genre-details` cannot show catalog content, so
everything on them is owned and "owned is plain" is already what they
do. The album page's own header badge stays, because that page is about
one entity and the badge is its answer rather than a mark on one of
many.
Closes#38
Two play paths still accepted `inLibrary`, so a row drawn dimmed and
`aria-disabled` by the new rule would still attempt to play and fail
with "this track could not be found in your library" — the disagreement
this pass exists to remove, one layer down from the badge.
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.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #38 — the Phase 1 item the roadmap (#73) puts after #16, #7 and
#34, all merged.
Commits
4bf59b4feat(library): answer album completeness for a screenful in one queryGetAlbumsCompleteness(albumIDs)— one query for a grid, so a card can say "9 of 12"41c41a8feat(explore): carry the local row id on a top resultTopResultwas the one projection shippinginLibrarywith noLocalID19c68d7fix(ui): keep the count in a partial badge that can actpartialbadge was named "Request album X" and dropped the count88fc50afeat(explore): mark what is not owned, everywhere it can be shownutils/ownership.ts— the rule written once10eca35fix(explore): gate playback on the same answer the row is drawn frominLibraryc4e055cdocs: write down which of the two ownership columns to readThe decision this turned on
The issue asked whether the grids accept the catalog's
inLibraryflagand say so in the copy, or whether something batches a real file lookup
per screenful the way
credit-store.tscoalesces.Neither: the grids take
localId, which is already on every model.collectLibraryEntitiesbuildslocal_artist_id/local_release_group_id/local_recording_idfrom queries that everyone join
audio_files, andpruneStaleLocalCrossReferencesclears themwith an existence test that is a file test in all three cases — the same
"ownership is a file" rule the album page's
filePathsimplements,computed once per scan instead of once per screenful.
inLibraryis written by the same pass, so in a healthy database thetwo agree. What differs is that it is a one-way ratchet
(
in_library = MAX(in_library, excluded.in_library)) whose onlyclearing pass is gated on a non-null
local_*_id. It cannot be un-seton its own. One of the two is a fact with an owner; the other is a flag
that happens to agree with it.
A batched
GetFilePathsByRecordingMBIDswould also only reach half theproblem: it answers for recordings, and most of the cards on these
surfaces are release groups.
Two bugs this found
A card answered "do I own this" twice and got two answers. On
explore-artist-details,renderReleaseMenuItemsgates Play onrelease.localId > 0while the badge andalbumTarget.ownedusedlibraryMBIDs.has(mbid) || rg.inLibrary || localId > 0. An album withthe flag and no local row therefore drew a green tick saying it was in
your library, offered no Play, and — the request item being gated on
!release.owned— offered no way to ask for it either.A partial badge that can act had no count in its name. A control is
named after what activating it does, so an actionable badge said
"Request album X";
partialis actionable (three tracks left to askfor), which made the one state the ring exists for the one state whose
name did not mention it.
library-status-indicator's own documentationalready argues the count is the point — "a ring says some to a
sighted user and nothing to anyone else" — and that does not stop
applying because the badge became clickable.
What the rule is now, in
utils/ownership.tswhat
explore-album-detailsalready did with its tracklist and whatits green ticks were removed for.
--yj-text-secondaryrather than an
opacitymultiplier, becausetheme-contrast.test.tschecks the ramp and not a multiplier on top of it; artwork takes the
opacity, since a cover is not text.
aria-disabledgoes on rows and deliberately not on cards. Anunowned 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 in
both cases.
completenessStore— shaped likecredit-store:request()is per-card and coalesces a screenful intoone call, absence is cached as an answer, and the cache is dropped on
a scan, a retag or a removal rather than aged.
Both
libraryMBIDssets are deleted. They accumulated every MBID everseen with
inLibraryand cleared it never, in views that never unmount.Audited and unchanged
home-view,downloads-view,cover-grid,artist-detailsandgenre-detailscannot show catalog content — none of them readsinLibraryorlocalIdat all — so everything on them is owned and"owned is plain" is already what they do.
explore-album-details'sheader badge stays: that page is about one entity and the badge is its
answer, rather than a mark on one of many.
Verification actually run
unowned-everywhere.test.tsis 15 new specs covering the rule ratherthan any one surface. The name assertions go through
page.getByRole(...)— the browser's own answer — because a shadow-rootquery cannot compute a name, and this repo has shipped a nameless
control three times.
One thing the tests found on their own:
completenessStoreis asingleton that caches the absent answer (deliberately, or 87% of a
grid re-asks forever), so two specs in one file are two sessions as far
as it is concerned.
beforeEachinvalidates it, with the reasonwritten down.
Not done here
The roadmap pairs #17 ("demote the release-version selector") with this
as "same page, same pass". It is a different page —
explore-album-detailsrather than the card surfaces — and is left for its own change.
Filed on the way: #118.
in_librarycan never be cleared on a rowthat has no local id —
MAX(in_library, excluded)only raises it andthe prune is gated on the id being present, so the pair is a fixed
point. Nothing in the tree produces one today, but the flag still
drives search scoring, the popularity-floor bypass and two Explore
shelves, so routing the UI around it (which is all this PR does) is not
a fix. Reproduction is in the issue.
Releases are manual now (#115), so merging this cuts no version.
Closes #38