Desktop: make it obvious everywhere when I am looking at things I do not own #38

Closed
opened 2026-08-18 05:56:10 +00:00 by logan · 3 comments
Collaborator

Report

Anywhere the UI shows tracks (or albums, or anything else) I do not have, that should be immediately obvious. The inverse should be mild: owned content is the default, normal, unadorned presentation; unowned content is what gets marked. It is confusing that stuff I am exploring looks the same as stuff I have.

Findings

The rule is already stated and partially implemented — explore-album-details dims unowned rows in place and marks nothing on owned ones (the previous green-tick-per-owned-row was the opposite treatment), and the dimmed rows carry aria-disabled because dimming is a colour and cannot be the only signal.

What is inconsistent is everywhere else: Explore's cards, top-results-row, artist detail's top tracks and discography cards, and the search results all mix owned and unowned with a small badge as the only difference — and the badge itself has known accuracy problems (see the "2 of 10" issue).

Direction

Write the rule down once and apply it as a pass:

  • Owned = plain. No tick, no badge.
  • Unowned = dimmed + a non-colour signal (aria-disabled, or a consistent marker) + a request affordance where one is possible.
  • Partial (album/artist) = the count, never a bare tick.

Then audit every surface that can show catalog content: album detail, artist detail, explore search/browse/shelves, top results, home shelves, downloads. This is the umbrella for the badge-accuracy and icon-language issues.

**Report** Anywhere the UI shows tracks (or albums, or anything else) I do **not** have, that should be immediately obvious. The inverse should be mild: owned content is the default, normal, unadorned presentation; unowned content is what gets marked. It is confusing that stuff I am exploring looks the same as stuff I have. **Findings** The rule is already stated and partially implemented — `explore-album-details` dims unowned rows in place and marks nothing on owned ones (the previous green-tick-per-owned-row was the opposite treatment), and the dimmed rows carry `aria-disabled` because dimming is a colour and cannot be the only signal. What is inconsistent is everywhere else: Explore's cards, `top-results-row`, artist detail's top tracks and discography cards, and the search results all mix owned and unowned with a small badge as the only difference — and the badge itself has known accuracy problems (see the "2 of 10" issue). **Direction** Write the rule down once and apply it as a pass: - Owned = plain. No tick, no badge. - Unowned = dimmed + a non-colour signal (`aria-disabled`, or a consistent marker) + a request affordance where one is possible. - Partial (album/artist) = the count, never a bare tick. Then audit every surface that can show catalog content: album detail, artist detail, explore search/browse/shelves, top results, home shelves, downloads. This is the umbrella for the badge-accuracy and icon-language issues.
logan removed the
Status
Blocked
1
label 2026-08-19 01:39:07 +00:00
Author
Collaborator

Both blockers are closed, so dropping Status/Blocked. This is next in the roadmap's (#73) Phase 1.

What the three merged issues leave for it:

  • #16tagwriter writes the per-disc track and disc totals, so autotagging no longer erases the evidence behind "9 of 12". The badge's partial state has a real input now for anything tagged from here on.
  • #34utils/icon-language.ts is the vocabulary, and the request toggle is one glyph in two weights everywhere. The half of this issue that is "the icons are half of the signal" is done; what is left is where the signal is drawn.
  • #7 — the album page can be asked for the whole release with the unowned rows dimmed, so the treatment this issue wants — owned is plain, unowned is marked — now exists in one place and can be copied outward.

So the remaining work is the audit the Direction describes: Explore's cards, top-results-row, artist detail's top tracks and discography cards, search results, home shelves, downloads. explore-album-details's tracklist is the reference implementation — dimmed in place, aria-disabled because dimming is a colour and cannot be the only signal, and nothing at all on the owned rows.

One thing worth deciding before the sweep rather than during it: the album page answers ownership with filePaths, a map from a displayed track to a real file, filled once by a batched GetFilePathsByRecordingMBIDs — deliberately not the catalog's inLibrary flag, which is set by MBID and can be true for something with no file behind it. A card grid cannot afford that call per card, so either the grids accept the weaker flag and say so, or something batches it per screenful the way credit-store.ts coalesces.

Not claiming it — flagging that it is ready to be picked up.

Both blockers are closed, so dropping `Status/Blocked`. This is next in the roadmap's (#73) Phase 1. What the three merged issues leave for it: - **#16** — `tagwriter` writes the per-disc track and disc totals, so autotagging no longer erases the evidence behind "9 of 12". The badge's `partial` state has a real input now for anything tagged from here on. - **#34** — `utils/icon-language.ts` is the vocabulary, and the request toggle is one glyph in two weights everywhere. The half of this issue that is "the icons are half of the signal" is done; what is left is *where* the signal is drawn. - **#7** — the album page can be asked for the whole release with the unowned rows dimmed, so the treatment this issue wants — owned is plain, unowned is marked — now exists in one place and can be copied outward. So the remaining work is the audit the Direction describes: Explore's cards, `top-results-row`, artist detail's top tracks and discography cards, search results, home shelves, downloads. `explore-album-details`'s tracklist is the reference implementation — dimmed in place, `aria-disabled` because dimming is a colour and cannot be the only signal, and nothing at all on the owned rows. One thing worth deciding before the sweep rather than during it: the album page answers ownership with **`filePaths`**, a map from a displayed track to a real file, filled once by a batched `GetFilePathsByRecordingMBIDs` — deliberately not the catalog's `inLibrary` flag, which is set by MBID and can be true for something with no file behind it. A card grid cannot afford that call per card, so either the grids accept the weaker flag and say so, or something batches it per screenful the way `credit-store.ts` coalesces. Not claiming it — flagging that it is ready to be picked up.
logan self-assigned this 2026-08-19 04:24:08 +00:00
logan added the
Status
In Progress
label 2026-08-19 04:24:10 +00:00
Author
Collaborator

Claiming this. Branch: feat/38-ownership-visibility.

The decision, before the sweep

The question was 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 the file-backed answer
and is already on every model at no cost.
Measured in the code:

  • collectLibraryEntities builds every local_artist_id /
    local_release_group_id / local_recording_id from a query that
    joins audio_files — the same "ownership is a file" rule the album
    page's filePaths implements — and pruneStaleLocalCrossReferences
    clears them with an exists test that is a file test in all three
    cases, with the comment saying why: the version that tested the
    metadata table left 129 rows in a real catalog claiming to be owned
    by files that were gone.
  • in_library is written by the same pass, so in the database today
    the two agree. What differs is that in_library is a one-way ratchet
    (in_library = MAX(in_library, excluded.in_library) in
    upsertIndexConflictSQL) whose only clearing pass is gated on
    local_*_id IS NOT NULL. 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.

So a batched call buys nothing the models do not already carry. It
would also only reach half the problem: GetFilePathsByRecordingMBIDs
answers for recordings, and most of the cards on the surfaces this
issue names are release groups, for which there is no batch — the
equivalent is GetAlbumCompleteness, per album. Batching would make
track rows strong and leave album cards exactly where they are, at the
price of a new coalescing store.

Where they diverge today, which is the client

Both explore-view and explore-artist-details keep a libraryMBIDs
Set that accumulates every MBID ever seen with inLibrary and is
never cleared. explore-view is a cached primary view — it never
unmounts — so a rescan that removes files leaves its cards claiming
ownership for the life of the session. Those sets go.

The bug that makes this concrete

On explore-artist-details, one card answers "do I own this" twice and
gets two answers. renderReleaseMenuItems gates playback on
release.localId > 0, per the rule already written above it; the badge
and albumTarget.owned use this.libraryMBIDs.has(mbid) || rg.inLibrary || localId > 0. An album with inLibrary and no local id
therefore draws a green tick saying it is in your library, offers
no Play, and — because the request item is gated on !release.owned
offers no way to ask for it either. A card that claims to be owned and
can do nothing at all.

What the pass does

  1. The rule written once, utils/ownership.ts: isOwned() (a
    local id, nothing else), the sentence unowned things say, and the
    dimming as a shared css fragment so eight surfaces cannot drift.
  2. Owned becomes plain. No badge at all on an owned card or row,
    which is what explore-album-details already does with its
    tracklist and what the green ticks were removed for.
  3. Unowned is dimmed in place, with aria-disabled and a name that
    says so — dimming is a colour and cannot be the only signal. Text
    dims to --yj-text-secondary rather than opacity, because the
    contrast table is what guarantees it still clears 4.5:1.
  4. Partial gets its count on album cards, which is the Direction's
    third bullet and #16's deferred step 3. This is the one part that
    needs a batch, and it is a different one: a new
    GetAlbumsCompleteness(albumIDs) — one query for a screenful,
    asked only for cards that have a local album id, coalesced per frame
    in a store shaped like credit-store. Without it an album you hold
    2 of 10 tracks of wears a bare tick on the cards page, which is #16's
    original complaint one surface over.

home-view, downloads-view, cover-grid, artist-details and
genre-details are audited and unchanged: none of them can show
catalog content, so everything on them is owned and "owned = plain" is
already what they do. That is a finding, not an omission.

Claiming this. Branch: `feat/38-ownership-visibility`. ## The decision, before the sweep The question was 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 the file-backed answer and is already on every model at no cost.** Measured in the code: - `collectLibraryEntities` builds every `local_artist_id` / `local_release_group_id` / `local_recording_id` from a query that joins `audio_files` — the same "ownership is a file" rule the album page's `filePaths` implements — and `pruneStaleLocalCrossReferences` clears them with an `exists` test that is a file test in all three cases, with the comment saying why: the version that tested the metadata table left 129 rows in a real catalog claiming to be owned by files that were gone. - `in_library` is written by the *same* pass, so in the database today the two agree. What differs is that `in_library` is a one-way ratchet (`in_library = MAX(in_library, excluded.in_library)` in `upsertIndexConflictSQL`) whose only clearing pass is gated on `local_*_id IS NOT NULL`. 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. So a batched call buys nothing the models do not already carry. It would also only reach half the problem: `GetFilePathsByRecordingMBIDs` answers for **recordings**, and most of the cards on the surfaces this issue names are release groups, for which there is no batch — the equivalent is `GetAlbumCompleteness`, per album. Batching would make track rows strong and leave album cards exactly where they are, at the price of a new coalescing store. ## Where they diverge today, which is the client Both `explore-view` and `explore-artist-details` keep a `libraryMBIDs` `Set` that accumulates every MBID ever seen with `inLibrary` and is never cleared. `explore-view` is a **cached primary view** — it never unmounts — so a rescan that removes files leaves its cards claiming ownership for the life of the session. Those sets go. ## The bug that makes this concrete On `explore-artist-details`, one card answers "do I own this" twice and gets two answers. `renderReleaseMenuItems` gates playback on `release.localId > 0`, per the rule already written above it; the badge and `albumTarget.owned` use `this.libraryMBIDs.has(mbid) || rg.inLibrary || localId > 0`. An album with `inLibrary` and no local id therefore draws a **green tick saying it is in your library**, offers no Play, and — because the request item is gated on `!release.owned` — offers no way to ask for it either. A card that claims to be owned and can do nothing at all. ## What the pass does 1. **The rule written once**, `utils/ownership.ts`: `isOwned()` (a local id, nothing else), the sentence unowned things say, and the dimming as a shared `css` fragment so eight surfaces cannot drift. 2. **Owned becomes plain.** No badge at all on an owned card or row, which is what `explore-album-details` already does with its tracklist and what the green ticks were removed for. 3. **Unowned is dimmed in place**, with `aria-disabled` and a name that says so — dimming is a colour and cannot be the only signal. Text dims to `--yj-text-secondary` rather than `opacity`, because the contrast table is what guarantees it still clears 4.5:1. 4. **Partial gets its count on album cards**, which is the Direction's third bullet and #16's deferred step 3. This is the one part that needs a batch, and it is a *different* one: a new `GetAlbumsCompleteness(albumIDs)` — one query for a screenful, asked only for cards that have a local album id, coalesced per frame in a store shaped like `credit-store`. Without it an album you hold 2 of 10 tracks of wears a bare tick on the cards page, which is #16's original complaint one surface over. `home-view`, `downloads-view`, `cover-grid`, `artist-details` and `genre-details` are audited and unchanged: none of them can show catalog content, so everything on them is owned and "owned = plain" is already what they do. That is a finding, not an omission.
Author
Collaborator

PR #117 is up. CI was green on both jobs (check 3m, e2e 6m
including WebKit) at 10eca35; the docs commit c4e055c is queued
behind it.

The decision, as implemented: the grids take localId — set and
cleared by a file test — and never inLibrary. The full argument is in
the claim comment above and is now written down in CLAUDE.md and
.planning/NOTES.md, since every future catalog surface has to make
the same choice.

Two things found on the way and fixed here: a card on
explore-artist-details that answered ownership twice and got two
answers (green tick, no Play, and no way to request it either), and an
actionable partial badge whose name dropped the count — the one state
the ring exists for.

One filed rather than fixed: #118, in_library can never be
cleared on a row with no local id. Routing the UI around the flag is
not a fix, because it still drives search scoring, the popularity-floor
bypass and two Explore shelves.

Not done here: #17 ("demote the release-version selector"), which
#73 pairs with this as "same page, same pass". It is a different page —
explore-album-details rather than the card surfaces — so it is left
for its own change rather than folded in.

PR #117 is up. CI was green on both jobs (`check` 3m, `e2e` 6m including WebKit) at `10eca35`; the docs commit `c4e055c` is queued behind it. **The decision, as implemented:** the grids take `localId` — set and cleared by a file test — and never `inLibrary`. The full argument is in the claim comment above and is now written down in CLAUDE.md and `.planning/NOTES.md`, since every future catalog surface has to make the same choice. Two things found on the way and fixed here: a card on `explore-artist-details` that answered ownership twice and got two answers (green tick, no Play, and no way to request it either), and an actionable `partial` badge whose name dropped the count — the one state the ring exists for. One filed rather than fixed: **#118**, `in_library` can never be cleared on a row with no local id. Routing the UI around the flag is not a fix, because it still drives search scoring, the popularity-floor bypass and two Explore shelves. Not done here: **#17** ("demote the release-version selector"), which #73 pairs with this as "same page, same pass". It is a different page — `explore-album-details` rather than the card surfaces — so it is left for its own change rather than folded in.
logan closed this issue 2026-08-19 05:11:43 +00:00
gitea-actions bot removed the
Status
In Progress
label 2026-08-19 05:11:54 +00:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Reference: yonlu/yellowjacket#38