diff --git a/.pi/skills/yellowjacket-dev/SKILL.md b/.pi/skills/yellowjacket-dev/SKILL.md index 7d8cc04..3f8e265 100644 --- a/.pi/skills/yellowjacket-dev/SKILL.md +++ b/.pi/skills/yellowjacket-dev/SKILL.md @@ -18,7 +18,7 @@ here has disappeared. ## Read this part before you fail -Seven things cost a cycle each the first time. They are here, not in a +Nine things cost a cycle each the first time. They are here, not in a reference, because you need them *before* the failure, not after. - **Time out every binding call.** A bound Go method called with wrong @@ -46,6 +46,16 @@ reference, because you need them *before* the failure, not after. Run against the `bulk` seed a measurement session left behind and 13 of 36 fail, in a list that reads exactly like a regression in whatever you are holding. `make dev-headless SEED=default` first. +- **…and the suite spends state it cannot always give back.** + `view-lifecycle.spec.ts` **skips an autotag album** on every run, out + of the eleven the seed has, and does not put it back — so around the + eleventh consecutive run against one app it starts failing on an + empty queue. Restart between runs (`make dev-stop && make + dev-headless SEED=default`) when a spec starts failing that you have + not touched, and *before* believing a failure at all. Backend state + outlives the page: shuffle used to be left on the same way, which + failed `playback.spec` on the next run — that one is fixed, the + autotag one is inherent. - **A frontend edit is not live until you restart the app.** Vite updates the module, but an already-registered custom element class cannot be re-registered, so a running page keeps the old one and your diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 98eb563..b7fcf04 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -1259,3 +1259,73 @@ And three on measuring, all of which produced a wrong number first: sidebar click: one highlighted card, `aria-selected="true"` on the right one, in both card grids. Check `viewActive` before believing a view did not repaint. + +## A reproduction read too early is a fix applied to nothing + +Plan 007 phase 5, first pass: the track list's arithmetic, a window +minimum the layout can actually hold, and one page header for nine +views. Three items, one inherited one-liner, and the pass's own +contribution to this plan's longest-running theme — *a number that +cannot move is not evidence* — which appeared twice more here, both +times in a **reproduction** rather than in a measurement. + +`config-page`'s rename bug is real: the library name's click bubbles to +the document handler that closes the rename editor, so it opens and +closes it in the same click. But the probe that "reproduced" it read +`.edit-input` synchronously after a synthetic `.click()`, and Lit +renders on a microtask — so it reported "not editing" on the broken +build *and* on the fixed one. The fix looked like it had done nothing, +which nearly bought a second, unnecessary fix; the real check needed one +`await`. The same shape then failed an e2e spec of mine, which captured +a header count immediately after a navigation and got `null`, making +every assertion after it vacuous. + +Seven things worth keeping: + +- **A screenshot disagreeing with a number is the useful signal, not a + puzzle to explain away.** A viewport ladder reported the top bar + overflowing by 0 px at every width while the PNG showed the job + indicator cut off at the edge. The badge was `display: none` — no job + was running by then — so nothing was overflowing and the "bug" was a + rendering of the app working. Two numbers that must agree are worth + more than one number you have to be sceptical about, and a picture + counts as one of the two. +- **An audit's symptom can outlive its mechanism.** `H-11` says the app + title "wraps into the nav" below 700 px. It does — but the title + block is 80 px tall inside a 64 px bar at *every* width, including + 1440; what changes at 780 px is the *subtitle* taking a second line. + Fixing the visible half is a breakpoint on the subtitle. The + permanent 16 px was never the finding and is still there. +- **Fixing the stated cause does not always remove the stated + symptom.** With the 40 px arithmetic fixed, Duration still reads + "Durat…" at 800 px — because the column is at its 50 px floor and the + *label* no longer fits, while the values do. Same screenshot, + different mechanism. Worth writing down, or the next reader + reasonably concludes the arithmetic fix did not land. +- **Count the copies before calling something a duplicate pair.** The + audit names Albums and Tracks as the two views with a sort toolbar; + `playlist-view` had a third copy of the same twenty lines. The fix + was worth 1.5× what the finding implied. +- **What a model carries decides what a control can offer.** + "Artists and Genres have no sort control" is one finding and two + different fixes: genres have a track count to sort by, and + `library.Artist` carries nothing countable at all. A select with one + option is a control that does nothing, so that view gets a label and + a direction button. +- **Backend state outlives the page, and a spec that spends it fails + the *next* run.** `view-lifecycle.spec.ts` toggled shuffle and never + toggled it back, so a second `make e2e` against the same app failed + `playback.spec`'s shuffle assertion — in a list that reads exactly + like a regression in the change you are holding, which is what I + assumed for half an hour. Stashing the phase's source changes and + re-running the same specs is what proved it pre-existing; the same + file also skips an autotag album per run, out of eleven, which is + inherent and now in the skill. **Restart the app before believing an + e2e failure you did not cause.** +- **A header that appears with the data is the layout problem it was + meant to fix.** The first version of `` rendered only + once a view had loaded, and showed "0 playlists" while loading. Both + were caught by reading a screenshot rather than by any assertion. The + header now renders during load and omits the count until there is an + answer — `null` meaning "no answer yet", which is a different thing + from zero and has to be a different value. diff --git a/.planning/plans/active/007-ui-reconciliation.md b/.planning/plans/active/007-ui-reconciliation.md index 62bcd49..a1e08a6 100644 --- a/.planning/plans/active/007-ui-reconciliation.md +++ b/.planning/plans/active/007-ui-reconciliation.md @@ -1557,6 +1557,134 @@ views; an e2e spec asserting no horizontal overflow on any row at 1440×900, 1024×768 and the new minimum; and a manual pass with a screen reader on the four surfaces that gained live regions. +### Phase 5 — the first pass: the arithmetic, the minimum, and the header + +Items 1 and 2 of the proposed four, plus one of the seven inherited +items. Each landed with its reproduction watched failing first, in the +running app rather than in a test. + +- **`H-7` — the track list fits its container.** Reproduced exactly as + the audit says: `scrollWidth 1280` against `clientWidth 1240` on the + header row and all 31 track rows, and the 40 px is precisely + `24 + 2×8`. `computeDefaultWidths`, `normalizeWidths` and + `onColResizeMove` now share one `availableColumnWidth`, and the two + constants behind it are read by `colBoundaryPositions` and the grid + template too — they were written out separately in four places, + which is how they came to disagree. **0 of 31 rows overflow after, + at every viewport tested.** +- **`H-11` — the minimum is a size the layout supports.** Reproduced: + at 700×480 the sidebar's eleven items need 406 px of a 352 px pane, + and Settings rendered at y=420–454 against a pane ending at y=416 — + outside its own box, clipped, unreachable. The pane scrolls now, the + sidebar collapses to icons below 900 px (its `.collapsed` mode + existed and only a manual drag had ever reached it), the subtitle + hides at the same breakpoint, and `MinWidth`/`MinHeight` are + **800×600**, chosen by walking a ladder of nine viewports and + reading what broke where rather than by picking a round number. +- **`H-19` and `H-10` — one page header, nine views.** + `` is title, count, sort and actions; Artists and + Genres gain the sort they never had, four views gain a heading, and + five gain a count. The header search box keeps its slot everywhere, + names its scope in the placeholder and in the header's own line, and + is disabled with a reason where it cannot serve. +- **The Settings rename one-liner** (found in Phase 3, routed here): + the library name's click bubbled to `config-page`'s document + handler, which exists to close the rename editor, so it opened and + closed it in the same click. + +#### Where the plan was wrong — the first pass + +Seven things, and two of them are about how the finding was checked +rather than about the finding: + +- **The two reproductions I wrote first were both invalid, in the same + way, and one of them nearly shipped a fix for nothing.** Reading the + DOM synchronously after a synthetic `.click()` reports the state + *before* Lit renders — so the Settings rename probe returned "not + editing" both before and after the fix. The bug is real (verified + properly: `false` before, `true` after, with an await), but for + twenty minutes the evidence for it was a number that could not move. + Same trap as this plan's `0 ms` view-open and its 150 ms debounce, + in a third costume, and now also in an e2e spec that read a count + before the view had one. +- **`job-indicator` is `display: none` when no job is running**, which + looks exactly like a control squeezed out of an overflowing header. + A ladder of viewport measurements said the top bar overflowed by + 0 px at every size while a screenshot plainly showed the badge cut + off at the right edge; the badge was simply *absent* by then, + because the index build had finished. The contradiction between the + number and the picture was the useful signal, and chasing it saved + fixing a layout that was not broken. +- **`H-11`'s "the app title wraps into the nav" is a subtitle + problem, and it is not new at 700 px.** The title block is 80 px + tall inside a 64 px bar at *every* width — it merely stops being + visible about there, when the subtitle takes a second line and it + becomes 98 px. Hiding the subtitle under the breakpoint fixes the + visible half; the 16 px of permanent overflow is cosmetic and + untouched. +- **`H-7` is not the only reason Duration looks clipped.** With the + arithmetic fixed, at 800 px the column is at its 50 px floor and the + *label* still ellipsises to "Durat…", because the saved widths are + scaled proportionally from whatever size they were set at. The + values fit; the heading does not. Different mechanism, same + screenshot, and worth knowing before someone "fixes" the arithmetic + again. +- **The sort toolbar existed three times, not twice.** The audit names + Albums and Tracks as the two views with sort controls; `playlist-view` + has a third copy of the same twenty lines. All three are now the + header's. +- **Artists cannot have a sort *select*.** `library.Artist` carries a + name, an MBID and three image URLs — nothing countable — so "the two + missing sort controls" is really one control and one direction + toggle. The header renders a label instead of a select with a single + option in it. +- **Two e2e specs were spending state they never gave back**, which is + not in any audit and cost most of an hour to attribute. + `view-lifecycle.spec.ts` toggled shuffle and left it on, so the + *second* `make e2e` against the same app failed `playback.spec`'s + shuffle assertion — a failure that reads exactly like a regression + in whatever you are holding, and which I first assumed was mine. + Stashing the phase's source changes and re-running proved it + pre-existing. Shuffle is restored now; the same file also skips an + autotag album per run out of the eleven the seed has, which is + inherent and is now in the skill instead. + +#### Not done, and still worth doing (after the first pass) + +Items 3 and 4 of the four, in that order: the dialogs, the context +menu's keyboard model and the ARIA tail as **one** pass (they are one +focus/semantics story, and splitting them is how two focus traps get +built); then the smaller items — landing on Home, the Home card's +missing-art placeholder, the album page's primary action and its +unexplained ✓ badges, the `?` overlay, Settings reordered with a +Playback section, and an Album column in the track list. + +Five of the seven inherited items remain: `cover-grid`'s dead album +dropdown (`perf.p2` — still a missing feature, not housekeeping), the +context menu's keyboard model, `tracklist.delete` (which needs a +"remove from library" that does not exist, and a decision about what +it removes), keyboard seeking from a focused track row, and the +header search box on `smart-playlist-details` — which is a *detail* +view and so was outside this pass's nine primary ones. + +One finding of my own, not fixed: **the Home shelves' cards render a +missing cover as nothing at all** (`H-9`), which is plainly visible in +any Home screenshot now that the page has a header above it. + +And two things about CI, neither mine and both pre-existing on +`main` at `9e92721`: + +- **`player-truth.spec.ts` fails in the CI container**, on the elapsed + clock (17 s and 11 s adrift, against a tolerance of 1) and + intermittently on the auto-advance skip. It passed 18 h earlier and + failed on a **docs-only** commit, so it is the container's audio + clock rather than a regression. All 44 specs pass locally, twice in + a row against one app. +- **`CLAUDE.md`'s claim that commitlint enforces the commit format in + CI is stale** — there is no config and no workflow running it — as + is the implication that semantic-release runs. Left alone pending a + decision: wire them up, or stop saying it. + --- ## Phase 6 — Explore starts the conversation diff --git a/CLAUDE.md b/CLAUDE.md index 56ca177..1b004b6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -405,6 +405,54 @@ result would silently reorder a queue — and because `cover-grid`'s drag cache stores them per album. A `libraryID` of 0 means "every library", matching an unset library filter. +**A view says what it is, in one component.** `` +(`components/page-header/`) is title, count, sort and actions, and the +nine primary views use it rather than each writing its own — which is +how four came to have a heading and four not, and how the same sort +toolbar came to exist three times with different bugs. Three rules in +it are load-bearing. **An empty `heading` is a mode, not a missing +value**: `cover-grid` and `track-list` are also embedded in the artist +and genre pages, which have a heading already, so there they keep the +count and the sort and drop the title rather than growing a second +arrangement. **The header renders while the view loads** — a heading +that arrives with the data is the shifting layout it exists to stop — +and the *count* is `null` until there is an answer, because "0 albums" +that corrects itself a moment later is worse than saying nothing. And +**the header asks for a sort, it does not perform one**: the host owns +the field, the direction and their persistence, so the control cannot +disagree with the list. + +**The header search box is view-scoped, and now says so.** It sits in +the app header and reads as global; typing `tide` on Playlists answered +"No playlists match your search" with three *Tideline* tracks in the +library. The scope was always real — the fix is that the placeholder +names it ("Search albums"), the page header repeats it ("Showing +artists matching ‘tide’"), and `search-store` holds the one map of +what each view searches. It also **keeps its slot everywhere and is +disabled** where it cannot serve, rather than being hidden: its +appearing and disappearing is what moved the library filter and the job +indicator on every navigation. On Explore, which has its own catalog +search, the disabled box points at it. + +**The window's minimum is measured, not aspirational.** `MinWidth`/ +`MinHeight` are 800×600 because that is where the shell was checked to +still work: below ~780 the header subtitle wraps and pushes the title +out of the 4em bar, and below ~600 tall the eleven sidebar items stop +fitting at once. The old 512×384 promised a size at which Settings and +Jobs were unreachable — the sidebar clipped them with `overflow: +hidden` and nothing scrolled. The sidebar scrolls now, and collapses to +its (long-existing, previously drag-only) icon mode below 900px, which +is the same breakpoint that hides the subtitle. + +**A row's columns must fit the row.** `track-list`'s +`computeDefaultWidths` shared out the host's whole `clientWidth` while +every row spends 24px on the favourite column and 2×8px on its own +padding first, so the grid was always exactly 40px too wide and the +last column was clipped at every size. Both numbers are constants read +by the three places that need them (the default widths, the +normaliser, and the resize handles' positions), because they were +written out separately and that is how they came to disagree. + **Event-driven communication**: Backend emits events via Wails runtime; frontend stores subscribe to them. Event names are constants in `backend/events/`. `frontend/src/events.ts` is **generated** from `backend/events/events.go`