docs: record the first Phase 5 pass and what it corrected
Two reproductions in this pass were read before Lit had rendered, so both reported the same answer on the broken build and the fixed one - the third costume of this plan's most-repeated trap, and the first time it has appeared in a reproduction rather than a measurement. Also records that the audit's symptom for H-11 outlives its mechanism, that fixing H-7's arithmetic does not remove every clipped Duration, and that two e2e specs spend backend state they never give back.
This commit is contained in:
@@ -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 `<page-header>` 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.
|
||||
|
||||
@@ -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.**
|
||||
`<page-header>` 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
|
||||
|
||||
Reference in New Issue
Block a user