docs: open plan 009 and record what phase 1 found

The plan's own framing was wrong in a way worth keeping: the badge was not waiting on the download client, which had largely landed already — it was waiting on somebody looking at a state nothing produced.
2026-08-13 14:14:39 -04:00
parent 48f7795687
commit 979c6e83ed
2 changed files with 283 additions and 0 deletions
@@ -2196,3 +2196,71 @@ Six more things worth keeping:
  hook* is what raises it — and a spec that spends the shared database
  has to give it back, since the 90 specs share one backend in file
  order.
 ## A state nothing produces is a state nobody has checked
 Plan 009 phase 1: `library-status-indicator`'s third state, wired.
 The generalisation: **an enum whose last value is never constructed is
 not unfinished, it is wrong** — because everything around it has been
 written, reviewed and tested against the two values that do occur, and
 the code reads as complete from every angle except the one that
 produces the third. `LibraryStatus` has had `queued` since it was
 written: styled amber, given an hourglass, given the sentence "… is
 queued for download". All eight call sites were a two-way ternary. So
 an album on the request list rendered a plus and announced "is not in
 your library" — on the same page, forty pixels from a filled button
 reading "Wanted".
 Nothing was going to find that. `make ui-test` and `make e2e` both
 covered the badge; both asserted the states it produced. 007 phase 6
 had rewritten this exact component, and the note it left behind
 ("when the download-client integration lands…") was itself the reason
 nobody looked: it names a *future* condition for work that was already
 possible, since `backend/download` was 16 541 lines and 20 bound
 methods on the day it was written. **A written-down reason not to look
 ages worse than the code it is about.**
 Six more things worth keeping:
 - **The second bug was in the screenshot of the first.** The "Wanted"
  button rendered a question mark — the missing-icon fallback —
  because `bookmark-check` is Font Awesome **Pro** and has never been
  bundled. `offline-icons.spec.ts` asserts `__yjIconMisses` is empty
  and passed the whole time: no spec had ever put the app in a state
  where an album was requested. The bundled-icon design anticipated
  exactly this ("twenty call sites compute their icon name from
  state") and the *sweep* still could not see it, because a sweep only
  sees the states it visits. Seventh regression in five plans that
  only a PNG has caught, and the first found in a PNG taken of a
  different bug.
 - **A property that does not change does not re-render a child.**
  `top-results-row` reads the request list, and its host handing back
  the same `results` array means Lit stops at the property — the row
  keeps its old badges while the store holds the right answer. The
  virtualizer rule (`requestUpdate()` on host state) one level milder,
  and the same fix: subscribe where the state is *read*.
 - **A spec that gives state back has to be run twice to know it did.**
  The `afterAll` cleanup called `callBinding`, which goes through
  `window.__yjEvents` — installed by the `app` fixture, not by a bare
  `browser.newPage()`. It threw where nothing was watching, left the
  request behind and failed the *next* run with a stale `queued`. One
  run proves the assertions; the second proves the teardown.
 - **A freshly launched app cannot search its own catalog for ~40 s.**
  The core artifact merge has to land (`core artifact: merge complete`
  in `.dev/app.log`), and until it does Explore's search returns
  nothing at all — *including for rows staged directly into
  `explore_index` a moment earlier*, which makes it look like the
  staging failed. Cost a cycle here reading as a failure of the neuter
  the run was under. Budget 60 s, or wait for the log line.
 - **The neuter has to be per line, not per feature.** Two fixes landed
  together and each got its own one-line neuter, which is what made
  the two failures distinguishable: one spec reported the wrong badge
  status, the other reported `["bookmark-check"]`. Neutered together
  they would both have failed and either could have been decorative.
 - **The fix is where the rule is, and the rule was in eight places.**
  Every one of the eight sites was individually reasonable; the third
  state was missing from all of them because no site owns the
  question. Same shape as `getCoverUrl()`, `track-index.ts` and
  `page-header` — when a rule is written per call site, the call sites
  do not disagree, they are all incomplete in the same way.
@@ -0,0 +1,215 @@
 # 009 — The badge that cannot act, and the state it already had
 **Status:** active — Phase 1 shipped; Phase 2 is a decision, not written yet.
 **Branch:** main
 **Created:** 2026-08-13
 **Follows:** 008-the-last-audit
 ## Problem
 007 phase 6 turned `library-status-indicator` from a `<button>` that did
 nothing into a `role="img"` badge, on the rule that **a control which
 cannot act is worse than none**, and wrote down what would change the
 answer: *"when the download-client integration lands, the right change
 is to make it a `<button>` again with a handler."*
 Two things about that are wrong, and both were found by reading the code
 and then the running app rather than the note.
 **The download client has largely already landed.** `backend/download`
 is 16 541 lines: a durable request model with four entity types
 (`artist` / `release-group` / `release` / `recording`, `request.go`), a
 reconciler, a staging importer, six provider adapters, 20 bound methods,
 `downloads-view`, the `download-picker` dialog, and a working **"Want
 this"** toggle on `explore-album-details`. What has not landed is the
 badge.
 **And the badge is not merely inert — it is wrong.** `LibraryStatus`
 declares, styles and labels a third state, `queued` ("… is queued for
 download"). **Zero of the eight call sites ever produce it**
 (`explore-view:1839,1877`, `explore-artist-details:2116,2228,2323`,
 `explore-album-details:1641,2283`, `top-results-row:258` — every one is
 a two-way ternary). So an album the user has *already requested*
 displays a plus and says it is not in their library.
 ### Reproduced, 2026-08-13, before anything was written
 Against `SEED=default` with the real 900 000-row catalog:
 `AddRequest({mbid: e51c54ea…, entity: 'release-group'})` for *GOLDEN* by
 Jung Kook, then Explore → search "GOLDEN":
 ```
 status  not-in-library
 icon    plus
 aria    Album "GOLDEN" is not in your library
 ```
 and on the album's **own detail page**, forty pixels apart in the same
 screenshot: the button reads **"Wanted"** (filled) and the badge beside
 the title reads **plus / "is not in your library"**. One component,
 two surfaces, opposite answers. This is the header-badge-contradicting-
 Settings failure again, and again only a PNG showed it.
 The same PNG showed a second one, which is why it is in this plan:
 **`bookmark-check` is not a bundled icon.** `window.__yjIconMisses`
 reports exactly `["bookmark-check"]`, so the "Wanted" button renders the
 fallback question-mark glyph. `e2e/specs/offline-icons.spec.ts` asserts
 that array is empty and passes, because no spec has ever put the app in
 a state where an album is requested — precisely the "twenty call sites
 compute their icon name from state" case `names.txt` exists for.
 ## Ordering principle
 By **what is a fact and what is a decision**.
 Phase 1 is a bug: the badge contradicts the app's own state, and fixing
 it needs no interaction design at all. It also produces the evidence
 Phase 2 needs — once the badge can say "requested", whether it must also
 *become* requestable is a question that can be looked at rather than
 assumed.
 Phase 2 is a decision made before any code, in the shape 008 phase 4
 used, because one 20 px circle would otherwise mean three different
 commitments: on an artist card a **discography subscription**
 (`scope: 'future'`, `Expands()`, never satisfied), on an album a
 release-group request, on a track row a recording request.
 Phase 3 is whatever Phase 2 leaves. **"Album only" is a legitimate
 outcome** and shrinks this plan rather than inventing work for it.
 ---
 ## Phase 1 — the badge tells the truth
 **Ships:**
 - `utils/library-status.ts` — one definition of the rule, since the
  reason all eight sites are two-state is that the rule is written at
  all eight. Owning something outranks wanting it, so `in-library` wins
  over `queued`.
 - The eight call sites using it.
 - `explore-view` gaining the `downloadStore` subscription both detail
  views already have (`init()` + `subscribe()`), through
  `view-lifecycle` — it is a **cached primary view**, so a raw
  `connectedCallback` subscription would live for the session.
 - `bookmark-check` in `src/icons/names.txt`, and an e2e case that
  reaches the state that exposes it.
 **The badge stays `role="img"`.** Telling the truth is not acting.
 **Watch for:** `downloadStore.init()` fetches providers, descriptors,
 downloads *and* requests, so this warms a singleton on a page that
 previously did not construct it — "a store with no subscriber fetches
 nothing" cuts the other way here, and the cost belongs in the note.
 ### Phase 1 — what actually shipped
 Three landings. `make ui-test` 677 → **685**; `make e2e` 90 → **92**.
 - **The rule, written once.** `utils/library-status.ts`, the eight call
  sites, and `explore-view`'s subscription.
 - **The Pro icon.** `regular/bookmark` / `solid/bookmark`, vendored.
 - **`e2e/specs/requested-badge.spec.ts`**, which is also the first spec
  that reaches the state the icon sweep needed.
 Pinned by `library-status.test.ts` (8) and `requested-badge.spec.ts`
 (2). Both e2e cases were watched failing on the pre-fix build by
 neutering one line each — the badge reported `not-in-library` where
 `queued` was expected, and the sweep returned `["bookmark-check"]`.
 #### Where the plan was wrong — Phase 1
 Six things, and the first is the plan's own framing.
 - **"When the download client lands" had already half happened, and
  the note that said otherwise was written before it.** 007 phase 6
  left a condition ("make it a button *with* a handler") that reads as
  future work; `backend/download` was 16 541 lines and 20 bound methods
  at the time it was written. The badge was not waiting on the download
  client. It was waiting on somebody looking.
 - **The bug was one layer below the one in the plan.** The plan says
  the badge cannot act. What the reproduction says is that it could not
  even *report* — three states declared, two produced, at eight sites
  none of which knew about the third. "A control that cannot act" and
  "a control that is wrong" are different faults and only the second
  one is a lie.
 - **The second bug was in the screenshot of the first.** The "Wanted"
  button rendered a question mark, which is the missing-icon fallback:
  `bookmark-check` is a **Pro** name. It has been that way for as long
  as anything could be requested, and `offline-icons.spec.ts` — which
  exists to assert exactly this — passed throughout, because it never
  reached a state where an album was requested. Seventh regression in
  five plans that only a PNG has caught, and the first one caught in a
  PNG taken of a *different* bug.
 - **A sibling component does not hear its host re-render.**
  `top-results-row` takes `results` as a property; `explore-view`
  re-rendering hands back the same array, so Lit stops at the property
  and the row keeps its old badges. Same shape as the virtualizer rule
  one level milder, and the fix is the same: subscribe where the state
  is read.
 - **The cleanup ran on a page that could not run it.** `afterAll` used
  `callBinding`, which goes through `window.__yjEvents` — installed by
  the `app` fixture and not by `browser.newPage()`. It threw where
  nothing was watching, left the request behind, and failed the *next*
  run of the same spec with a stale `queued`. A spec that gives state
  back has to be checked by running it twice, which is what found this.
 - **A freshly launched app cannot search its own catalog for ~40 s.**
  The core artifact merge (`core artifact: merge complete` in
  `.dev/app.log`) has to land first, and until it does Explore's search
  returns nothing — *including for rows staged directly into
  `explore_index` a moment earlier*, which is what makes it look like a
  staging bug. It cost a cycle here reading as a failure of the neuter
  it was run under.
 ---
 ## Phase 2 — what a badge click means, per entity
 *(decision, before code — not started)*
 What Phase 1 leaves for it, now as observations rather than guesses:
 - On the album page the badge and the "Want this" button now say the
  same thing twice, four centimetres apart. That is an argument for the
  badge being **read-only there** and clickable only where there is no
  button — or for the button going.
 - A requested album shows an amber hourglass while every track in its
  tracklist shows a plus, which is correct per the rule and reads as
  busy. Worth deciding whether a track inside a requested album should
  render *nothing* rather than a plus.
 - An artist badge would mean a discography subscription, which is the
  heaviest commitment in the download subsystem behind the smallest
  control in the app.
 ---
 ## Phase 3 — the button
 *(scope depends on Phase 2)*
 ---
 ## Deliberately not in this plan
 - **Deleting the file from disk** (008 phase 4's explicit sequel). Not
  refused — mis-ordered. 008's own notes record that the *reversible*
  option shipped with **nothing implementing its reversibility**:
  `excluded_paths` has no management surface, and "a full rescan clears
  it" is the escape hatch. Shipping an irreversible delete beside a
  reversible one that cannot yet be undone is backwards, and the
  platform trash is a new cross-platform dependency besides.
 - **`a11y.20`, deriving `_itemSize` from a measured row.** Real and
  confirmed in code — `.track-row` is `height: 33px; contain: strict`
  with a `rem` font size, so text scales and the box does not, across
  four lists (33 / 49 / 45 / 45 px). It waits because its only honest
  verification does not exist yet: both surviving comments
  (`track-list.ts:349`, `queue-panel.ts:179`) say a wrong `_itemSize`
  desynchronises the **native scrollbar at 20k+ rows**, and `make perf`
  has no scroll-fidelity row. That measurement is its own first phase
  and belongs to a plan that is about it.
 ## First step
 Phase 1, and within it the helper rather than the call sites — the
 reproduction above is already the failing case, and the point of the
 helper is that there is one place for the next state to be added.