Three P1 adoption-wave findings, all of which break an unattended run.
The worktree needs make build-frontend and make testdata once before its
first push — the pre-push go-test hook embeds frontend/dist and the
fixture library and refused the push without them. The merge leg now
says to fetch origin/main in the same breath as the refresh merge: a
cached origin/main merged against the wrong base, CI went green on it,
and the merge came back 405 "behind base" — a full cycle wasted. And a
killed fetch leaves 0-byte objects in the shared store whose signature
(error: object file … is empty / unpack-objects failed) must halt the
loop for a human instead of churning, with the repair recipe written
down.
Closes#240
Adopting the six v0-era PRs surfaced two conflict-shaped cases the merge
leg handled only by luck. Behind-main branches are refused outright by
the repo's block_on_outdated_branch protection, so the leg now refreshes
every branch against origin/main before merging — which is also where a
textual conflict should surface, as diff text the loop resolves only
where it authored the hunks, otherwise abandoning the PR to a human
with a comment. And the one guard no mergeability check provides is the
push run on main after the merge: three PRs touching the same file can
merge cleanly and contradict each other, so a red main now halts the
loop instead of the tick reporting merged and moving on.
Closes#238
The scheduled backlog runs already claimed, fixed, verified and opened
PRs one issue at a time (~50 runs), but stopped at "PR open, CI green" —
every merge and every stale branch was human work, and the pipeline
shape existed only as one prompt file. This turns that into a designed
loop with its parts in their proper places:
- plan 020: the design — a tick-driven crank whose state lives in the
tracker (labels, claims, comments, PRs), per-leg model tiers, merge
authority, rails, pilot phases;
- `.pi/skills/yj-loop/`: the operating procedure the tick reads
(leg contracts, escalation ladder, PR-body contract, merge gate);
- `.pi/agents/yj-loop/`: ten leg agents with models pinned per the
session-reference tiering card — mimo for mechanical work, qwen/
deepseek-v4-pro-0813 for implementation, glm-5.3 for selection,
planning and consequences review, glm-5.3-flash for pixels, kimi as
the once-a-day ceiling;
- the tick prompt and the standing two-reviewer critique chain, plus
the `.pi/loop/` gitignore entry and the CLAUDE.md pointer.
The switch stays where the v0's was — `.pi/schedule-prompts.json`,
gitignored, live only while the loop's pi session is open. No code
changes. Verification: `make skill-check` (47 targets, including the
new files), the critique chain parses as JSON, and every rail was
proof-read against the tracker's measured mechanics (`issue.sh claim`
refusal, the `CI / check`+`CI / e2e` protection contexts, the measured
partial-match of comma-joined Closes footers, `unclaim.yml`).
Closes#236
The kernel refuses to exec a file that is open for writing anywhere in
the process, and these tests are parallel: a sibling's fork duplicates
stubYtDlp's write descriptor in the moment it is open and carries it
past our close, so the exec a moment later fails with ETXTBSY. That is
the flake seen once locally and once in CI, both times on a tree with
no Go in its diff.
Closing sooner is not available -- os.WriteFile has already closed the
file before anything execs it -- and O_CLOEXEC does not help, because
the window is between another goroutine's fork and its own exec.
syscall.ForkLock is the lock forkExec takes across that fork, so
holding it over the write means no child can exist while the
descriptor does.
Measured on the helper itself under 12 concurrent writers: 176-189 of
2400 execs refused before, 0 of 2400 after, three runs each.
Closes#146
The first-run wizard dismissed itself as a step in its own flow, so a
library appearing by any other route left a full-screen modal up over
an app that was already set up — intercepting every pointer event,
which is also what keeps Settings out of reach while it is there.
AddLibrary emits LibraryAdded whoever calls it, so one subscription
makes the dismissal follow the state the wizard exists to wait for
rather than the button being pressed. It is registered before the
initial read, or a library arriving while that call is in flight is
answered with a stale empty list; both routes out now end in one
dismiss(), which sets `finished` before asking the dialog to close
because preventClose cancels the hide otherwise.
Closes#175
Config.Save() validates the whole config, so a setter that assigned
before validating did not merely fail its own call: the rejected value
stayed in memory and failed every later save, of every unrelated
setting, silently and for the rest of the session. Nothing reached
disk, so a restart cleared it — which is what made the fault invisible
and unreportable.
The defect is precisely "assignment precedes a validation that can
reject that argument", and that predicate enumerates seven setters
rather than the whole file. Each snapshots the field and restores it on
the error path.
The remaining setters were read rather than assumed and are unchanged:
shortcuts.Config.Validate returns nil unconditionally, the bools and
SetFavoritesPlaylistID pass through no validation that inspects them,
Config.Validate does not validate Downloads at all, and SetViewVisible
refuses an unknown, non-hideable or launch-page view before assigning.
SetLibraryDirectory was already correct and is the precedent the new
comment points at: it validates a candidate before assigning, so there
is nothing to undo.
The rationale sits above the setter section rather than on Save(),
which is bound — a doc comment there renders into frontend/bindings
for an audience with no use for it.
Closes#231
Settings -> Track List Columns listed two rows both called "Track
Name", one of which could not be ticked, and a screen reader heard
"Show the Track Name column" twice with nothing to tell them apart.
They are `titleArtist` and `trackName`, and the issue left open which
way to fix it: a second label so the sort dropdown and the configurator
can say different words, or drop the row because the user cannot select
it. The code settles that. `titleArtist` is not in
`tracklist.AllColumnIDs`, so `Config.Validate()` returns `unknown
track-list column ID: "titleArtist"` -- ticking the row sends a column
set the backend rejects, `config-page` swallows the rejection into a
`console.error`, and the tick reverts. A second label would have named
a control that cannot work.
So a definition says whether it is a *choice*, and the configurator
reads `CONFIGURABLE_COLUMN_IDS` rather than `Object.keys(COLUMN_DEFS)`.
The new test reads Go's own list out of `backend/tracklist/config.go`
rather than writing it down a third time, since a third copy is the
fault one step earlier. Verified by planting: with the filter removed
all four assertions fail with the defect's own numbers.
Not fixed here, and filed as #231: `SetTrackListColumns` assigns before
it validates, so a rejected list stays in memory and `Save()` validates
the whole config -- one tick and no setting saves for the rest of the
session. That is reachable from any invalid input, not from this row.
Closes#197
`fixtures.md` told an agent the WAV fixtures scan in untitled, that
there is no "Field Recordings" artist in the Artists view, and that
this is a known open bug "pinned by TestWAVTagsAreNotReadableYet" — a
test #104 deleted, because it existed to assert the reader did not work
and failed the moment it did.
That last clause is why this is worth a diff rather than being left to
rot: the paragraph is an instruction, and it instructs the next reader
that a spec asserting the *working* behaviour is the mistake. It is the
same #104 staleness #217 removed from `queue-selection.spec.ts`, one
file over, still telling agents to put it back.
Measured against a running app rather than corrected from the issue
text — and the seed had to be rebuilt first, since the one on disk
predated #104 and would have replayed a pre-#104 scan and confirmed the
stale paragraph. On a fresh `make sandbox-seed NAME=default`, both WAVs
carry a title, an artist credit and an album: "Field Recordings" is an
ordinary artist with 2 tracks and "Test Tones" has a cover row. The
only two tracks with no album at all are `unsorted/no-tags-at-all.mp3`
and `unsorted/title-only.mp3`.
The replacement also says that prose written before #104 disagrees,
because it does, and saying nothing is how the next reader reintroduces
the claim from a source this change deliberately does not touch.
Deliberately carries no `Closes` footer. #225 covers two halves, and
the second — the same staleness in two *dated* `.planning/NOTES.md`
entries — is left alone: whether measured history gets a correcting
clause is a judgement about what that file is for, which the issue
raises on purpose and this change must not settle by auto-closing it.
The check asserts that every make target named in a doc exists, and its
scanned set was .pi/ plus CLAUDE.md. Since #50, CONTRIBUTING.md is the
document a *human* goes to for a build command, and it names 21 targets
that nothing verified; README.md names none today and is in for the same
reason. The script's own header sentence is the argument — a renamed
target sends a person off the same cliff it sends an agent off.
The file list is now one `docs` variable used twice, because the failure
message carried a second copy of it and a second list is a second thing
to forget. The `[ -d .pi ]` guard went with it: gating the whole run on
.pi/ would make the human-facing half conditional on the agent-facing
one, and an empty list is the same "nothing to scan" exit without the
coupling.
The lefthook glob is that scanned set now rather than
{Makefile,.pi/**/*.md} — #220's smaller half, and it did not fire on
CLAUDE.md either, which the script had read for months.
Verified by planting a bad target rather than by reading the diff: both
matched forms in each of the four scanned surfaces, each naming the
right file; the same two plants pass on the pre-change script; unfenced
prose still does not match; and the hook fires on a staged
CONTRIBUTING.md under the new glob where the old one skipped it. The
count is unchanged at 47 — the set is a union — so coverage is the only
thing that moved.
Closes#220
`queueSixAndOpen` filters the queue down to tracks that have an album,
because `explore-link` renders a name it cannot route as plain text and
one test clicks that name. The filter is right and unchanged; the
comment explaining it named the wrong two files.
Since #104 read a WAV's `id3 ` chunk, the two tracks under `Field
Recordings/Test Tones` are tagged, scanned and ordinary. Asked of a
seeded app rather than of the comment, exactly two tracks in the
fixture library have no album: `unsorted/no-tags-at-all.mp3` and
`unsorted/title-only.mp3`.
The clause saying which change made the old names wrong is there so the
next reader does not restore them.
Closes#217
Parse sized its buffer from the chunk header, which is four bytes read
off the file, so a truncated or malformed WAV declaring a 4 GB data
chunk in a 2 kB file got 4 GB from the allocator before the read
discovered there was nothing to put in it. The error was always right;
the allocation happened first.
io.CopyN into a bytes.Buffer is what ID3Chunk beside it has done since
#104, and needs nothing new: the reader stays an io.Reader and the
buffer grows with what actually arrives.
The regression test measures rather than asserts the error, because the
error is identical on a build that allocates the gigabyte. Measured on
the pre-fix build: 1,073,750,920 bytes of TotalAlloc for a 42-byte
container whose data chunk claimed 1 GiB.
Closes#216
Clear queue and Add queue to playlist were named by a `title` attribute
and nothing else, while the close button beside them has carried an
`aria-label` since #24. They get one too.
`title` is a name, so this is the weak-name case rather than the missing
one: it is the *last* fallback in the accname order, so any content put
inside the button later silently outranks it, and a phone has no hover
to show it. The `title`s stay — on a desktop they are also the tooltip
for an icon-only control, which is a different job.
The assertion is the part worth reading. The obvious spec — `getByRole`
by name, which is what `queue-overlay.spec.ts` already does for the
close button — is **green on the broken build**: measured against the
running pre-fix app, both buttons matched. So the second test states the
property as what it is, that the name is not the tooltip: it removes the
`title` attributes and asks again, which was 0 and 0 on main and is 1
and 1 now.
Closes#170
Since #71 the phone's "More" is a bottom sheet, and at the reference
viewport it does not fit: measured at 424x439 with the seed's eight
destinations, the body is scrollHeight 412 against clientHeight 373, so
39px is below a fold nothing announces. Where the cut lands on a row
boundary the sheet ends in a clean edge that reads as the end of the
list, which is what #207 fixed one sheet over.
The rule is that sheet's, not a second answer to the same question:
#207's two background layers move into styles/sheet-scroll.css.ts and
both sheets adopt them, with the colour left to each host as
--yj-sheet-surface. The nav sheet paints the sidebar's --yj-bg-surface
and the context sheet the menus' --yj-bg-elevated, so a shared rule that
hard-coded either would draw that seam across the other one.
The half that makes it visible is that nothing inside the sheet may
repaint the surface. These are layers on the scroller, and app-sidebar's
host carries the same grey -- in the shell its own background, in the
sheet a second opaque copy of the sheet's, over the fade. With the
fragment adopted and that rule missing, the running app measured a flat
52,58,64 to the bottom edge with 39px still below: the defect unchanged,
with every assertion about background-attachment passing. menu-surface
already meets it from the other side, where the sheet's panel is
background-color: transparent.
Closes#210
The README was two documents in one, and neither reader was served by
the other's half. It opened on a feature list, then spent its second
half on Go versions, WebKitGTK packages and `make` targets — while its
install table named a `darwin-universal.app.zip` and a
`windows-amd64.exe` that nothing has ever produced, and its header
claimed Windows and never mentioned Android, which is the one platform
with a published, self-updating channel.
So this is a correctness pass as much as a friendliness one. The README
now answers a user's questions only: what the app is, three screenshots
from the seeded fixture library so anyone can retake them, the four
formats, one install section per channel that names what is actually
published, first run, where the data lives, and pointers out. The
version-restart note is linked to the two documents that own it rather
than copied, because a copy is a second thing to keep true.
CONTRIBUTING.md takes the technical half: prerequisites, the system
libraries, the build and codegen commands, which verification tier a
change demands, the tracker workflow, the commit grammar and the style
rules. CLAUDE.md is unchanged apart from one paragraph naming the split
— it was already the deep reference both of the others point at, and
stays the only one of the three that explains why a shape is what it is.
Closes#50
Scrolling the albums grid pops art in: the cards already draw the
smallest adequate tier and are already lazy, so what was left is *when*
the request happens. The grids are virtualized, so the `<img>` — and
therefore the fetch — does not exist until the virtualizer renders its
card, which is about 1000px past the viewport, or two screens on the
reference device.
The issue asks for a larger overscan and that is not available:
`_overhang` is a hard-coded `protected` field on `BaseLayout` with no
configuration surface. So the request is issued ahead of the element
instead. `utils/image-prefetch.ts` warms a bounded window either side
of the rendered range, from `rangeChanged` rather than
`visibilityChanged` — the two report different ranges, and a window
measured from what is *visible* is spent on cards that already exist.
Cover and artist URLs are served under `Cache-Control: immutable`
(content-hashed filenames), so a prefetched image is a cache hit by the
time its card is drawn. The bytes are the browser's; what this holds is
the set of URLs asked for, capped and reported to `__yjCacheStats()`.
Measured on the bulk seed (4 988 albums), ten 2 400px jumps, covers in
the viewport with `naturalWidth === 0`: 254 of 258 blank one frame
after the jump and 214 two frames after, against 117 and 77 with the
prefetch.
Closes#65
The tab bar's fifth item opened `<app-sidebar>` in a `wa-drawer`
sliding in from the side, which is a desktop shape put on a phone: a
200px column of a 424px screen, opening away from the thumb that asked
for it, with the rest of its 400px band empty. It also had three nested
scrollers in it -- the dialog, its body, and the sidebar's own
`overflow-y: auto` host -- so which box a drag moved depended on where
the finger landed, which is the "only part of the screen scrolls under
my finger" in the report.
It is the same element with `placement="bottom"` and `without-header`,
so the surface is the sheet #60 already built rather than a second
pattern: a `wa-drawer` is a native `<dialog>` opened with `showModal()`,
which is exactly the top layer that finding rests on, so the focus
trap, Escape, tap-outside and `wa-after-hide` come along unchanged and
nothing new has to be proved about paint containment.
The sidebar is still mounted rather than re-listed as data, because the
shell's own copy is `display: none` below 600px rather than removed --
a second list drawing `nav-*` handles is the duplicate-testid failure
this component already renders conditionally to avoid. What `expanded`
means had to grow to say the host owns the *box*: `app-sidebar` writes
an inline width and caps itself at 400px, which beats any rule the host
could write, so the width, the scrolling and the mouse-only resize
handle now follow that attribute. The rows are 48px below 600px, stated
in the sidebar's own stylesheet since that is the only place it renders
there.
Measured in the running app at 424x439: the sheet is 424 wide, 373 tall
(85vh, so there is an outside to tap), rows 48px, one scroller with
`overscroll-behavior: contain`, and Settings' row reachable at the end
of it. Desktop and Compact are untouched.
Closes#71
The phone drew a grey box over the bounding rect of whatever was
tapped, which is the web view saying what it is. It is gone in one
declaration: `-webkit-tap-highlight-color` is inherited and an
inherited property crosses a shadow boundary, so `html` in index.css
reaches every shadow root in the app. Measured three roots deep,
rgba(0, 0, 0, 0.18) before and rgba(0, 0, 0, 0) after.
Removing it removes the only touch feedback several surfaces had, so
the press state is part of the same change rather than a later polish
item — with the highlight gone a held row measured the *hover* tint,
which on a phone is synthesised by the hold itself and outlives it.
The four lists' rows, the tab bar, the sidebar's destinations and the
shared context-menu item take --yj-press-overlay on :active; the cards
already had scale(0.97). The press selector carries a state class
because a row is .track-row.selected.active, so a bare :active shows
nothing on the row a phone is most likely to press. And those
surfaces' hover tints move behind (hover: hover) and (pointer: fine),
which is #68's gate applied to a tint rather than a revealed control.
user-select, the other half of the Findings, was already done: the
first rule in index.css covers the shadow roots for the same reason.
touch-action: manipulation is declined — the 300ms delay it is offered
for is already absent on a width=device-width viewport, and what it
would really change is the gesture stack tuned by measurement on a
device this session cannot measure.
Closes#54
Every track, album and artist name in the app navigates through
`utils/explore-link.ts`, and every sentence of how it does that is a
desktop compromise: the navigation is held for one double-click
interval so double-clicking the row can still play it, and the target
is a few characters of text inside a row. On touch that is a delay on
an ambiguous target, and since #63 the row's own tap claims the click
anyway -- so the link was unreachable as well as fiddly.
So below the phone breakpoint a name renders as plain text and the
row's context menu carries the destination instead: `go-to-menu.ts`
draws "Go to Artist" / "Go to Album" under exactly the condition the
link is not, using `explore-link`'s own exported routing so an untagged
entity reaches the library page by the same lookup.
Three things this leans on. Suppressing a link with no menu behind it
is not a smaller affordance but a destination the phone cannot reach,
so `keepOnPhone` is the exception for the three surfaces with no row
menu. The items are drawn for a single selection only, which is the
Play item's rule one step on. And there is no "Go to Genre", because
no row renders a genre link to lose -- that would be new navigation
rather than a replacement.
Closes#67
#204 landed first, so the ui-tier rule can name `make ui-visual-update
UI_ARGS=<path>` rather than the raw vitest invocation it needed while
the recipe swallowed its filter.
`make ui-visual` had been red on main since #27, and nothing runs it,
so four references had drifted across three unrelated merges. Two were
refreshed with #186; these are the other two.
Each recorded two changes, not one. `app-sidebar` lost Jobs (#27,
shipped) and moved its highlight from Home to Tracks; `now-playing`
gained the source line (shipped) and was playing from "a dynamic mix".
Both are singleton stores read by a case that sets nothing, so the shot
photographs whatever the case above it left behind — blessing that
would have pinned the file's own ordering into a PNG. Both cases state
their world now, and only then are the references re-recorded.
The second half of the issue asks whether this tier should gate, and
the answer is measured rather than preferred: replayed in a bare
ubuntu:24.04 container — CI's `check` image — three of the ten
baselines fail on rendering alone (`track-info` and one `page-header`
shot at ratio 0.03 against a 0.02 allowance, `seek-bar` one pixel
shorter), and the two stale ones disagree about their new height
between the machines. So CI cannot run this suite without a second,
container-recorded baseline set that every local run would then fail
against, and a pre-push hook is the same fault with the machines
swapped. It stays local and opt-in; what replaces the gate is the rule
that a change moving a component's geometry refreshes that component's
baseline in the same commit, having read the image, and never one it
did not cause. Written where a person meets it: the skill's tier doc
has the table, SKILL.md has the obligation, CLAUDE.md has the
constraint.
Deleting the baselines was the third option and is declined: this tier
has caught one thing no other could, the `<span>` that lost the UA
stylesheet's `box-sizing` and grew a badge 36→38px.
Closes#196
A test file does not get its own origin. `@vitest/browser-playwright`
opens one BrowserContext per session and runs several files in it, one
after another, so everything a component persists survives from file to
file. `track-list` restores its sort in `connectedCallback` and
`aria-tail.test.ts` activates the Title column header, so any file that
mounts a track list later in that tab opens sorted by title — where
`track-11` precedes `track-3`, which is #138's failure exactly.
Nothing about it is specific to that pair: a probe that throws when a
test starts with a non-empty `localStorage` failed 24 test-starts in one
full run (11 with the two sort keys, 8 with `cover-grid-size`, 5 with
`track-list-column-widths`) and cascaded into 248 failures. Which files
share a tab, and in what order, changes run to run, which is the whole
of why this reads as a 1-in-3 flake and passes in isolation.
The clear belongs in `setup.ts` rather than in the specs that write,
because the spec that reads is never the one that knows — and it is
safe for the same reason the leak exists: files within a session are
sequential, so it cannot wipe storage a concurrent file is using.
The spec now also states the order it asserts rather than inheriting a
default, and checks the row it is about to double-click carries the path
it expects, so a stray sort fails by naming itself instead of as an
off-by-eight file path.
Closes#138
logan