docs: record the third Phase 5 pass, and read the CI log
The e2e failure two sessions could not diagnose is the container's audio clock, on both engines — 48 specs pass under Chromium and 48 under WebKit, failing the same three. Nothing in last pass's dialog, focus or role work is WebKit-specific. Also records what got in the way of knowing that: gitea_ci's job-log endpoint 404s on this build while the REST API answers fine, and the WebKit step had been skipped on every red run.
This commit is contained in:
@@ -39,8 +39,28 @@ reference, because you need them *before* the failure, not after.
|
||||
`config.toml` and DB rows — a hand-built `YJ_HOME` is a second
|
||||
description of a valid one and will drift. `make sandbox-seed` drives
|
||||
the real `AddLibrary` binding and waits for the real scan.
|
||||
- **…and a seed freezes every default it has already persisted.**
|
||||
Changing a default in `backend/config` (or `backend/tracklist`) is
|
||||
invisible against an existing seed, whose `config.toml` holds the old
|
||||
value — while CI builds its seed by running the app and therefore
|
||||
tests the *new* one. Re-seed before believing either.
|
||||
- **A `wa-dialog` is awkward to locate, in three ways.** The host is
|
||||
`display: contents`, so the element carrying your testid always
|
||||
reports hidden; the visible thing is the native `<dialog>` in its
|
||||
shadow root. The slotted content is in the *host's* shadow root, not
|
||||
in that dialog's subtree, so `toContainText` on the dialog sees only
|
||||
its chrome. And the dialog has **no accessible name** — Web Awesome
|
||||
never wires `label` to `aria-labelledby` — so
|
||||
`getByRole('dialog', {name})` matches nothing.
|
||||
- **Playwright's WebKit does not run on Arch** (Ubuntu-only libs).
|
||||
`--browser=webkit` is CI-only; local work is Chromium.
|
||||
`--browser=webkit` is CI-only; local work is Chromium. CI runs it
|
||||
with `if: !cancelled()` so a chromium failure does not silently
|
||||
skip it, which it did for two sessions.
|
||||
- **CI's `e2e` job is red for a reason that is not yours.** Three
|
||||
playback specs fail in the container on both engines (48 pass on
|
||||
each) because the position never advances there — an audio-device
|
||||
problem, not a renderer or app one. Read the per-step status before
|
||||
assuming your change did it.
|
||||
- **`make e2e` needs `SEED=default`.** Its specs assert on fixture
|
||||
content — unicode tracks, the fixture artists, a known playable file.
|
||||
Run against the `bulk` seed a measurement session left behind and 13
|
||||
@@ -66,6 +86,14 @@ reference, because you need them *before* the failure, not after.
|
||||
old behaviour. `make dev-headless` prints the esbuild error; a
|
||||
reload does not. One way to cause one is a stray backtick inside a
|
||||
comment in a `css` tagged template literal, which ends the literal.
|
||||
- **A failing CI job's log is reachable even when `gitea_ci job_logs`
|
||||
says it is not.** That endpoint 404s on this Gitea build. The REST
|
||||
API answers, with the `GITEA_TOKEN` already in the environment:
|
||||
`/api/v1/repos/yonlu/yellowjacket/actions/runs/<run>/jobs` for
|
||||
per-step status (this is how "the WebKit step was *skipped*" was
|
||||
found) and `/api/v1/repos/yonlu/yellowjacket/actions/jobs/<id>/logs`
|
||||
for the whole log. Two sessions reasoned about the e2e failure from
|
||||
the commit list because the first tool's 404 read as "out of reach".
|
||||
- **`npx tsc --noEmit` is part of the gate, and nothing else runs it.**
|
||||
CI does (`.gitea/workflows/ci.yml`), and it typechecks
|
||||
`frontend/test/` — which `make lint`, `make test`, `make ui-test` and
|
||||
|
||||
@@ -1437,3 +1437,95 @@ changed focus management, dialog modality and roles, which is exactly
|
||||
the area where the two engines differ. Treat the WebKit half as
|
||||
unverified rather than as the known audio-clock flake until someone
|
||||
reads the log.
|
||||
|
||||
## "Out of reach" is a claim about a tool, not about the information
|
||||
|
||||
Plan 007 phase 5, third pass: Settings' keyboard reach, the `?` overlay
|
||||
and the arrows, an Album column, and the CI question two sessions had
|
||||
recorded as unanswerable.
|
||||
|
||||
The generalisation the pass adds: **when a tool says it cannot get
|
||||
something, that is a fact about the tool.** `gitea_ci job_logs` returns
|
||||
a 404 on this Gitea build and says so clearly, and two sessions read
|
||||
that as "the log is out of reach from here" and reasoned from the
|
||||
commits instead. The REST API on the same server answers fine, with the
|
||||
token that was already in the environment:
|
||||
|
||||
```
|
||||
GET /api/v1/repos/{owner}/{repo}/actions/runs/{run}/jobs # per-step status
|
||||
GET /api/v1/repos/{owner}/{repo}/actions/jobs/{id}/logs # the whole log
|
||||
```
|
||||
|
||||
Ten minutes, after a session and a half of careful hedging about what
|
||||
the failure might be. The hedging was correct — it just cost more than
|
||||
checking would have.
|
||||
|
||||
What the log said, in two parts:
|
||||
|
||||
- **The failure is the container's audio clock**, on both engines. Not
|
||||
a regression in the dialog/focus/menu work, which was the live worry.
|
||||
`playback.spec`'s elapsed time and two `player-truth.spec` cases: the
|
||||
UI interpolates while the backend position stays at zero, 17–18 s
|
||||
adrift. `ci.yml` says the ALSA null plugin advances at real time; it
|
||||
was measured once and no longer does.
|
||||
- **WebKit had never run.** The step had no `if:`, so a chromium
|
||||
failure skipped it — `conclusion: skipped`, in the same JSON that
|
||||
held the answer. The previous pass's "treat the WebKit half as
|
||||
unverified" was more literally true than intended: the one place
|
||||
WebKit gets any coverage had produced no signal at all for as long
|
||||
as chromium had been red. With `if: !cancelled()` it runs, and both
|
||||
engines pass 48 and fail the same three.
|
||||
|
||||
Seven more things worth keeping:
|
||||
|
||||
- **A reproduction of the *fix* can be as invalid as one of the bug.**
|
||||
After making `←`/`→` reach the player again, the check measured zero
|
||||
`Player.Seek` calls — the same answer as the broken build, because
|
||||
nothing was playing and the dispatch records nothing with no track
|
||||
loaded. Seventh costume of this plan's most-repeated trap, and the
|
||||
first on the *after* side: "the fix did nothing" and "the probe
|
||||
cannot see anything" produce identical output.
|
||||
- **A shortcut a dialog swallows is a promise the shortcut layer
|
||||
cannot keep.** The `?` overlay was written as a toggle. It cannot
|
||||
be: `focusedControlOwnsKey` yields every unmodified key to anything
|
||||
inside an open dialog, so the second `?` never reaches the service.
|
||||
Escape closes it, as it does every dialog here. The rule that
|
||||
protects focused controls is the rule that forbids the toggle, and
|
||||
an e2e spec is what noticed.
|
||||
- **Every `wa-dialog` in this app is an unnamed dialog.** `a11y.md`
|
||||
lists them under "what is already correct" and says every one passes
|
||||
a `label` — true, and the label never reaches the accessibility tree.
|
||||
Web Awesome renders it into an `<h2 id="title">` in the same shadow
|
||||
root as the `<dialog>` and never sets `aria-labelledby`. Found by
|
||||
writing `getByRole('dialog', {name})` and getting nothing. Two more
|
||||
facts about locating one, both costing a spec run: the host is
|
||||
`display: contents` so it always reports hidden, and the slotted
|
||||
content lives in the *host's* shadow root, not in the dialog's
|
||||
subtree.
|
||||
- **A fix can be right while the finding's stated benefit is wrong.**
|
||||
`H-15` wants an Album column so the three `Tideline / Aurora Fields /
|
||||
00:06` rows can be told apart. They are duplicates of the same
|
||||
album, so they still read identically; what distinguishes them is
|
||||
the duplicate-detection feature or a path column. The column is
|
||||
still the right default for every other row. Visible only in a
|
||||
screenshot — nothing failed, and the finding's sentence would have
|
||||
been ticked off without looking.
|
||||
- **A section of controls that do nothing is worse than admitting the
|
||||
section does not exist.** `H-22` asks for a Playback/Audio section;
|
||||
`backend/config` has no output device, gapless, crossfade or replay
|
||||
gain to expose. Same judgement as "Artists cannot have a sort
|
||||
*select*" two passes ago, and the same tell: the audit describes the
|
||||
UI it wants without checking what the model carries.
|
||||
- **A default the seed has already persisted is not a default you can
|
||||
see.** Changing `tracklist.DefaultColumns` changed nothing in the
|
||||
running app, because `.dev/seeds/default.tar` carries a `config.toml`
|
||||
from before it — while CI builds its seed by running the app and
|
||||
would have exercised the *new* one. A local run and a CI run testing
|
||||
different defaults is worse than either being wrong. `make
|
||||
sandbox-seed NAME=default`.
|
||||
- **`aria-controls` has to name an element that exists**, which decides
|
||||
how a disclosure renders: `config-section`'s body is rendered
|
||||
unconditionally and toggled with `hidden` rather than added and
|
||||
removed. Nothing is paid for it — the slot's light-DOM children are
|
||||
in the DOM either way; a conditional `<slot>` only stops projecting
|
||||
them.
|
||||
|
||||
@@ -1817,6 +1817,120 @@ and removing the file is a delete-your-music button one keystroke from
|
||||
a focused row. The honest interim is to stop advertising the binding in
|
||||
Settings; that is not done yet either.
|
||||
|
||||
---
|
||||
|
||||
### Phase 5 — the third pass: Settings, the key story, the small ones, and the CI answer
|
||||
|
||||
Three independently landable pieces of item 4, plus the question that
|
||||
was supposed to be first and turned out to be answerable in ten
|
||||
minutes.
|
||||
|
||||
- **`a11y.1` / `a11y.2` / `H-22` — Settings is reachable.**
|
||||
Reproduced exactly as written: seven `config-section` headers, seven
|
||||
bare `<div @click>`s, `role` and `tabindex` null on every one, all
|
||||
collapsed. They are `<button aria-expanded aria-controls>` now, on
|
||||
the pattern `explore-artist-details` has had five of the whole time,
|
||||
and the body renders unconditionally toggled with `hidden` because
|
||||
`aria-controls` has to name an element that exists. Downloads' two
|
||||
`<div class="tab">`s are a `role=tablist` with a roving tab stop and
|
||||
Left/Right/Home/End. Libraries is first and the only expanded
|
||||
section; Search Index, configured once if ever, is second to last.
|
||||
Settings also stops advertising `tracklist.delete`.
|
||||
- **The key story, told once.** `?` opens a `wa-dialog` listing every
|
||||
binding, read from `services/shortcut-meta.ts` — moved out of
|
||||
`config-page`'s private static, so the overlay and the Settings
|
||||
editor share one table. With it, **keyboard seeking from a focused
|
||||
row**: Phase 1 gave the grid all six arrows, and no list in this app
|
||||
moves horizontally, so `←`/`→` reached nobody. A row owns the
|
||||
vertical keys only now.
|
||||
- **`H-15` and the search scope.** Album is a default track-list
|
||||
column, in Go and in the TS fallback. `smart-playlist-details` is in
|
||||
`search-store`'s scope map — checked first, as the handoff asked: it
|
||||
*does* read the term, so the fix is a scope entry rather than a
|
||||
disabled state.
|
||||
- **The CI e2e failure is answered and is not ours.** Details below.
|
||||
|
||||
#### Where the plan was wrong — the third pass
|
||||
|
||||
Eight things, and the first is the one that mattered most:
|
||||
|
||||
- **The `e2e` CI failure was readable all along, from a different
|
||||
endpoint.** `gitea_ci job_logs` 404s on this Gitea build, which two
|
||||
sessions took to mean the log was out of reach. The REST API answers
|
||||
fine: `/api/v1/repos/{owner}/{repo}/actions/runs/{run}/jobs` lists
|
||||
per-step status, and `/actions/jobs/{job_id}/logs` returns the whole
|
||||
log. Cost: ten minutes, after two sessions of "cannot check".
|
||||
- **And WebKit had never run.** The `E2E — webkit` step had no `if:`,
|
||||
so a chromium failure skipped it — `conclusion: skipped` on every
|
||||
red run. The plan's "CI also runs WebKit, treat that half as
|
||||
unverified" was truer than intended: it had produced no signal at
|
||||
all for as long as chromium had been failing. With `if:
|
||||
!cancelled()` it runs, and the answer is **48 passed on both
|
||||
engines, failing exactly the same three specs** —
|
||||
`playback.spec`'s elapsed clock and two in `player-truth.spec`. No
|
||||
WebKit-specific failure anywhere, so last pass's dialog, focus and
|
||||
role work is clean on the renderer we ship. What is red is the
|
||||
container's audio clock: the UI interpolates while the backend
|
||||
position stays at zero, 17–18 s adrift. `ci.yml`'s claim that the
|
||||
null ALSA plugin advances at real time was measured once and is no
|
||||
longer true.
|
||||
- **A reproduction of the *fix* can be as invalid as one of the bug.**
|
||||
The seek-from-a-row fix measured zero `Player.Seek` calls after it
|
||||
landed — because nothing was playing, and with no track loaded the
|
||||
dispatch records nothing on any build. Both the broken and the fixed
|
||||
build answer "no seeks", which is this plan's most-repeated trap in
|
||||
its seventh costume, this time on the *after* side.
|
||||
- **`?` cannot be a toggle, and the app is right to stop it.** The
|
||||
overlay was written to toggle; the e2e spec asserting it failed,
|
||||
because `focusedControlOwnsKey` yields every unmodified key to
|
||||
anything inside an open dialog. Escape closes it, as it does every
|
||||
dialog here. A shortcut that a dialog swallows is a promise the
|
||||
shortcut layer cannot keep, and finding that out cost one spec run.
|
||||
- **Every `wa-dialog` in this app is an unnamed dialog.** `a11y.md`'s
|
||||
"what is already correct" says all five are modal, restore focus and
|
||||
"every one of them passes a `label`" — all true, and the label never
|
||||
reaches the accessibility tree. Web Awesome renders it into an `<h2
|
||||
id="title">` in the same shadow root as the `<dialog>` and never
|
||||
points `aria-labelledby` at it, so `getByRole('dialog', {name})`
|
||||
matches nothing. Found by writing that locator. Not fixed here: it
|
||||
is eight call sites and a helper, and it should be done on purpose.
|
||||
- **`H-15`'s Album column does not do what `H-15` says it will.** The
|
||||
three `Tideline / Aurora Fields / 00:06` rows are duplicates of the
|
||||
*same album*, so with an Album column they read identically. The
|
||||
column is still the right default; what tells those rows apart is
|
||||
the duplicate-detection feature or a path column. Visible only in
|
||||
the screenshot — nothing failed.
|
||||
- **`H-22`'s Playback/Audio section cannot be built honestly yet.**
|
||||
There is no output-device, gapless, crossfade or replay-gain setting
|
||||
anywhere in `backend/config`; the whole section would be controls
|
||||
that do nothing. Same judgement as "Artists cannot have a sort
|
||||
*select*" from the first pass. The reorder shipped; the section is a
|
||||
feature, not a consistency fix.
|
||||
- **A default that a seed has already persisted needs the seed
|
||||
rebuilt.** Changing `DefaultColumns` changed nothing in the running
|
||||
app, because `.dev/seeds/default.tar` carries a `config.toml` with
|
||||
the old three columns — while CI builds its seed by running the app
|
||||
and would therefore have tested a *different* default from the one
|
||||
measured locally. `make sandbox-seed NAME=default` first.
|
||||
|
||||
#### Not done, and still worth doing (after the third pass)
|
||||
|
||||
**The album page is the one item of 4 left**: `H-13` (no Play, no
|
||||
Shuffle, no Add to queue on `explore-album-details`, and the green ✓
|
||||
badges with no legend) together with `cover-grid`'s `renderSplitGrid`,
|
||||
which is referenced only to satisfy `noUnusedLocals` and is the only
|
||||
route from the albums grid to `track-details`.
|
||||
|
||||
And two things this pass found rather than inherited:
|
||||
|
||||
- **Name the dialogs.** One helper, eight call sites.
|
||||
- **CI's e2e job cannot go green until the container's audio clock
|
||||
does.** Three specs assert on a position that does not advance
|
||||
there. Either the container gets a sink that really consumes, or
|
||||
those three specs learn to skip when it does not — but silently
|
||||
loosening a tolerance would delete the only assertions this repo has
|
||||
that the player tells the truth.
|
||||
|
||||
## Phase 6 — Explore starts the conversation
|
||||
|
||||
The only phase that adds rather than repairs.
|
||||
|
||||
@@ -377,6 +377,31 @@ demand puts the element and its `showModal()` in the same update.
|
||||
`autotag-view`'s last document keydown listener died with them; it
|
||||
existed only because its dialogs could not close themselves.
|
||||
|
||||
**None of them has an accessible name**, which is worth knowing before
|
||||
writing a locator or believing the audit's "already correct" list.
|
||||
Every call site passes `label`; Web Awesome renders it into an `<h2
|
||||
id="title">` in the same shadow root as the native `<dialog>` and never
|
||||
points `aria-labelledby` at it, so `getByRole('dialog', {name})`
|
||||
matches nothing and a screen reader announces an unnamed dialog. The
|
||||
host is also `display: contents`, so the element carrying the testid
|
||||
always reports hidden — what is visible is the `<dialog>` inside it,
|
||||
and what holds the slotted content is the *host's* shadow root, not the
|
||||
dialog's subtree.
|
||||
|
||||
**A disclosure is a button, and it says what it controls.**
|
||||
`config-section`'s header was a bare `<div @click>` with no `tabindex`,
|
||||
no `role` and no `aria-expanded`, and every section defaults to
|
||||
collapsed — so every setting in the app sat behind a control that could
|
||||
not be tabbed to (the audit's last Critical). It is a
|
||||
`<button aria-expanded aria-controls>` now, on the pattern
|
||||
`explore-artist-details` has had five of all along, and **the body
|
||||
renders unconditionally and is toggled with `hidden`**: `aria-controls`
|
||||
has to name an element that is in the DOM, and a conditional `<slot>`
|
||||
only stops projecting light-DOM children that exist either way.
|
||||
Downloads' two tabs are the same fix one page over — `role=tablist` over
|
||||
`role=tab`, one roving tab stop, Left/Right/Home/End, and a
|
||||
`role=tabpanel` whose id and `aria-labelledby` swap with the tab.
|
||||
|
||||
**A menu has a keyboard model, and it is one model.**
|
||||
`utils/context-menu-controller.ts` exports **`MenuKeyboard`** — focus
|
||||
the first item on open, Arrow/Home/End to move (wrapping, as a menu
|
||||
@@ -444,6 +469,24 @@ control that owns it (button/select/slider/checkbox, or anything inside
|
||||
an open dialog) so the unmodified single-key global bindings do not
|
||||
steal Space and the arrows.
|
||||
|
||||
**A list owns the arrows it moves on, which is the vertical ones.**
|
||||
Granting a `row`/`option`/`grid` all six took `←`/`→` away from seeking
|
||||
and gave them to nobody: `track-list`'s own handler and
|
||||
`utils/roving-rows.ts` both take Up/Down/Home/End and ignore
|
||||
Left/Right, so a focused track row produced zero `Player.Seek` calls
|
||||
against one per press from the body. Grant them back in
|
||||
`keysOwnedBy` if a list ever moves horizontally.
|
||||
|
||||
**The keys are written down in one place and shown in two.**
|
||||
`services/shortcut-meta.ts` is the label, category, scope and default
|
||||
for every action; `?` opens `<shortcuts-overlay>` and Settings edits
|
||||
the same table. It used to be a private static in `config-page`, which
|
||||
listed three of the four categories by hand — so the autotag bindings
|
||||
were written down nowhere. **The overlay is not a toggle**: a dialog
|
||||
owns every unmodified key while it is up, so a second `?` never reaches
|
||||
the service; Escape closes it. And a shifted character does not report
|
||||
Shift (`?`, not `Shift+?`) — the character already carries it.
|
||||
|
||||
Two cross-cutting pieces of that UI are worth knowing before touching
|
||||
a list or a detail view:
|
||||
|
||||
@@ -519,7 +562,11 @@ 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.
|
||||
search, the disabled box points at it. **A view that filters on the
|
||||
term belongs in that map**, detail views included —
|
||||
`smart-playlist-details` narrowed its list as you typed under a
|
||||
placeholder saying there was nothing to search here, because its
|
||||
sibling was in the map and it was not.
|
||||
|
||||
**The window's minimum is measured, not aspirational.** `MinWidth`/
|
||||
`MinHeight` are 800×600 because that is where the shell was checked to
|
||||
@@ -540,6 +587,15 @@ 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.
|
||||
|
||||
**The default columns are declared twice and must agree.**
|
||||
`tracklist.DefaultColumns` is what a fresh install persists;
|
||||
`DEFAULT_COLUMN_IDS` in `track-list/columns.ts` is what the list draws
|
||||
until the config arrives. Album is in both (a library manager with
|
||||
duplicate detection whose rows are track/artist/duration cannot tell
|
||||
its own duplicates apart) — and changing either is invisible against an
|
||||
existing `YJ_HOME`, whose `config.toml` already holds the old list, so
|
||||
`make sandbox-seed NAME=default` before believing the app.
|
||||
|
||||
**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`
|
||||
@@ -766,13 +822,32 @@ Two jobs, both in an `ubuntu:24.04` container:
|
||||
against **both** Chromium and WebKit. Playwright's Linux WebKit links
|
||||
Ubuntu 24.04 libraries that Arch does not provide, so CI is the only
|
||||
place it can run, and it is the closest available approximation of
|
||||
the WebKit2GTK renderer that ships.
|
||||
the WebKit2GTK renderer that ships. The WebKit step carries
|
||||
`if: ${{ !cancelled() }}`, without which a chromium failure skips it
|
||||
— which is how the one source of WebKit signal came to produce none
|
||||
for two sessions while the plan recorded it as "unverified".
|
||||
|
||||
**A failing job's log is readable, and `gitea_ci job_logs` is not the
|
||||
only way.** That endpoint 404s on this Gitea build; the REST API does
|
||||
not. `GET /api/v1/repos/{owner}/{repo}/actions/runs/{run}/jobs` gives
|
||||
per-step status (which is how "WebKit was skipped" was found) and
|
||||
`GET /api/v1/repos/{owner}/{repo}/actions/jobs/{job_id}/logs` returns
|
||||
the whole log, with `Authorization: token $GITEA_TOKEN`.
|
||||
|
||||
**The `e2e` job is currently red on both engines, for one reason.**
|
||||
48 specs pass under Chromium and 48 under WebKit; the same three fail
|
||||
— `playback.spec`'s elapsed clock and two in `player-truth.spec` —
|
||||
because playback does not advance in the container: the UI
|
||||
interpolates while the backend position stays at zero, 17–18 s adrift.
|
||||
It is the audio device, not the renderer and not the app. Which means
|
||||
the paragraph below was measured once and is no longer true.
|
||||
|
||||
Two things the container needs that a developer machine does not. It
|
||||
has no PulseAudio socket, so `/etc/asound.conf` makes ALSA's `null`
|
||||
plugin the default device — that plugin advances its pointer on a
|
||||
timer, so playback is consumed at real-time rate and the elapsed clock
|
||||
moves, which `e2e/specs/playback.spec.ts` asserts. And
|
||||
plugin the default device — that plugin is *supposed* to advance its
|
||||
pointer on a timer, so playback is consumed at real-time rate and the
|
||||
elapsed clock moves, which `e2e/specs/playback.spec.ts` asserts; it no
|
||||
longer does. And
|
||||
`YJ_CORE_INDEX_URL` points at a dead address so no run fetches the real
|
||||
explore artifact, matching what `scripts/seed-sandbox.sh` already does.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user