The shelves, the top results and the artist page's discography and
similar-artists rows scrolled sideways only by a horizontal wheel or a
trackpad, so a plain mouse could not reach anything past the fold.
<scroll-row> wraps each of them with previous/next arrows that are
hidden at the end they cannot move from, revealed on hover where there
is hover, and always shown where there is not.
The album card is defined once, in albumCardStyles, instead of twice.
explore-view clamped its cards to 130-150px, and with square artwork
that made cards in one row different heights as well as widths. The
width is fixed now and each line under the art reserves its own space.
Covers are inset rather than cropped (contain, not cover).
The ownership badge sits over the artwork for owned and unowned alike,
and catalog cards no longer dim: a grid of dimmed covers read as a page
that had failed to load. The album page's tracklist still dims unowned
rows, which says something different about something different.
Every MusicBrainz link now goes through openMusicBrainz, which pins the
origin, including the one on the album page that still called
window.open directly. solid/chevron-left is bundled for the left arrow;
without it the arrow drew the missing-icon fallback.
Closes#264
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
"More from artists you own one album by" drew its artists and their
albums most-popular first, so it was a second leaderboard: the same
handful of big names every time the page opened, which is not what the
shelf is saying. The pool is still bounded, but which artists and
which albums fill it is RANDOM(), so each visit is a different sample.
The test asserts the set rather than the order.
Refs #264
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT
Both branches rewrite CLAUDE.md. Mine appended a note about #258 to the
"the catalog stores ids as bytes" section; theirs cuts the file from
4,046 lines to 376 and deletes those write-ups, on the grounds that each
one already exists as a comment beside the code it describes.
Resolved in theirs. Nothing is lost by it: the refactor already states
both halves of the note as broad engineering rules, and cites #258 —
"Encodings are converted at one boundary … a Go `string` cursor against
a BLOB column made the catalog merge loop forever while merging nothing"
and "A loop that must make progress checks that it did … a merge that
lands fewer rows than it read". The detail stays in `artifactKey`'s doc
comment and in `publishedartifact_test.go`'s, which is where the new
file says this class belongs.
CLAUDE.md had grown to 4,046 lines, ~3,300 of them per-component
write-ups: why a breakpoint is 500px, why a cap is a quarter, what a
spec once missed. Every session loaded all of it, and the rules that
apply to every change were buried among decisions that apply to one.
Those write-ups were already duplicated as comments beside the code
they describe -- every issue number they cite also appears in a code
comment -- so they are deleted rather than moved. What is left is the
tracker workflow, the commands, the verification tiers and a set of
broad engineering rules distilled from them, each pointing at where
its example lives. Three declined decisions recorded nowhere else go
to NOTES.md, and three code comments that cited CLAUDE.md sections by
name no longer do.
Closes#256
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Nothing should be published until the code that imports it on a user's
machine has imported it here. The exporter and the importer are two
descriptions of one storage format, and every other tier tests the
importer against a *fixture* rather than against the file being shipped —
a second description free to be wrong in the same direction as the code
reading it.
That is how #258 reached everyone: the importer positioned its batch walk
with a Go `string` cursor against this file's 16-byte `mbid` column, and
SQLite neither coerces between TEXT and BLOB nor complains about the
comparison, so the walk merged no rows and never advanced. The fixture
guarding that walk writes the old text encoding, and the only compact
fixture is one row, below the batch size, so the bound query never ran.
Both were green throughout, and no install could finish a first index
build.
`TestImportPublishedArtifact` takes the published file and runs the
client's own path over it — checksum, decompress, merge — and asserts
that what the artifact holds is what the client ends up with: the row
count, the rows carrying a listen count, an FTS index in step with the
table, and one row read back through the app's own MBID conversions. It
skips without `YJ_CORE_INDEX_ARTIFACT`, so an ordinary run pays nothing.
It needs no Wails, which is why the step runs it under the indexbuild
tag: that container has no GTK. Measured on the current artifact, 64.8 MB
compressed: 39 seconds including the decompress.
Verified against the pre-fix comparison behaviour, the step goes red in
about 3 seconds — the strictly-advancing guard fails the import with a
named reason rather than the day-long spin it used to produce.
Refs #258
The walk's predicates partition the artifact's key space, so a merge that
ends with fewer rows than the artifact declares does not mean the artifact
was smaller than it said — it means a predicate filtered rows out, and
the catalog is quietly partial while reporting complete.
Equality rather than a lower bound: RowsAffected counts an upsert that
changes nothing, and a row already merged locally is counted again here.
One reachable case, so this is not merely a tripwire. A row whose mbid is
empty is excluded by `mbid > ?` in both encodings, so an artifact
carrying one imports as a success with a row missing — which is the shape
#258 had, one cause over. The test covers exactly that artifact.
Refs #258
The prebuilt catalog never merged. `mergeArtifactRows` positions itself
with `WHERE mbid > ? ORDER BY mbid LIMIT 1 OFFSET ?` against the
attached artifact, and it bound that cursor as a Go `string` while
`cmd/indexexport` publishes `explore_index.mbid` as 16 raw bytes — the
storage change that took the table from 677 MB to 389 MB.
SQLite does not coerce between TEXT and BLOB and orders every blob after
every text value, so against a byte column the predicate was not wrong
but unconditional: `mbid > <text>` matched the whole artifact, so the
bound the walk looked up was the same row every time and the cursor
never advanced, and `mbid <= <text>` matched nothing, so no batch
merged. No error, no rows, no state change — a fresh install sat at "0
of 1,077,893 rows" burning a core indefinitely, which is what it did
here for a day, while Explore showed only the rows the library scan and
the lazy artist enrichment had produced and popularity for none of the
catalog.
The cursor is now an `artifactKey`, typed to the encoding
`artifactStoresText` reports for the file it is attached to, so the
comparison is made in the same type as the column it is made against.
Two things guard the class rather than the instance: a nil key binds as
an empty value instead of SQL NULL, because `mbid > NULL` agrees with
nothing and would import nothing just as silently; and the walk returns
an error when its bound does not strictly advance, because the failure
here is silence and the next one should be a failed job with a reason.
It was never caught because the fixture that guards the walk writes the
old text encoding, and the only compact fixture is a single row — below
`artifactMergeBatch`, so the bound query never ran at all. The walk is
now covered on both encodings, across several batch boundaries.
Closes#258
Both specs stage a job through `/__test/emit` and neither cleared it.
Nothing resets those stores, so the spec that staged it is the one that
should put it back, and `JobsChanged` with `[]` is the whole cleanup --
`JobStore` replaces its list from every snapshot, so `testctl` needs no
special case.
**The leak as reported did not reproduce, and that is worth recording
rather than quietly fixing.** Measured with a temporary probe: a
positive control confirmed a staged job really does move the shell at a
phone width (`job-band` renders a row, `.main-panel`'s top goes 0 to
55), and the very next page had no job at all. The reason is that
every test gets a fresh page and `JobStore.init()` refetches `GetJobs()`
from a backend registry `/__test/emit` never writes to -- it calls
`events.Deliver`, which touches frontends and no state. So the state
cannot cross a spec boundary as described, and the 55px offset the
draft assertion saw in that suite run has another cause that is not in
evidence.
The cleanup stays, because it costs a line and the leak would need only
one spec that keeps a page alive, and the comments say what was
measured rather than asserting the mechanism.
The durable half is the rule, now in the harness reference: measure
against the element next to you, not an absolute coordinate. An
absolute number in a shell measurement is also a claim about everything
above it -- `contentTop === 0` asserts "and no background job is
running", which that spec could not arrange.
Closes#168
Gitea leaves `github.event.before` empty on a `pull_request`, so the
Commit messages step fell through to bare `make commit-check`, which
lints `git log -1` -- the tip alone. Every other commit the branch
would bring was first examined by *main's* post-merge run, so a green
PR stopped being true after the merge, and it happened twice: PR #245
merged a 75-char subject its own CI never saw.
The PR's base is the stand-in. `base.sha..head` lints the PR's own
commits because base advances on main, so the commits the branch shares
with it stay reachable from it and drop out of the range.
Both payload fields are handed to the shell rather than chosen in an
expression: `github.event.issue.number` in unclaim.yml is this repo's
proof that payload fields resolve, and `github.event` is the webhook
body unmarshalled into a map, so `pull_request.base.sha` comes from
Gitea's own `PRBranchInfo.Sha`. The shell then falls back to today's
behaviour for a dispatch run, an all-zeros push, or a base commit the
clone does not have -- so the worst case is the fix not taking effect
rather than a broken job.
Verified locally against the report's own evidence: at 68e7edb8 the old
invocation passes ("HEAD is well-formed") while the range catches
a3b5b437 at 75 chars, which is what main's post-merge run did. All
four event shapes were exercised against the new snippet. The
end-to-end proof is the next PR with an over-length commit that is not
its tip.
Closes#254
`scripts/skill-check.sh` matched one regex against one line, so a
mention the file hard-wraps -- `` `make `` at the end of one line and
the target at the start of the next -- was invisible to it. These docs
are mostly hard-wrapped prose, so the wrap is what the author does not
think about, and `CONTRIBUTING.md:80` is already that shape.
Lines are now joined while the inline span is still open, which an odd
number of backticks means. The fence and line-start halves are
untouched: a fenced command is already whole, and joining inside one
would break the rule that made this awk rather than a grep. Joining is
bounded three ways -- a fence, a blank line (CommonMark allows no blank
line inside a code span) and a file boundary -- so a stray backtick
costs one paragraph of over-matching rather than the rest of the file.
The reporting loop needed the other half of the same fix: it named the
offending file with `grep -ln "make $t"`, which cannot see a wrapped
mention either, so a target the new parser found reported no file at
all and `set -o pipefail` turned the empty grep into exit 123 before
the line telling the author what to do. It falls back to the bare
name.
Verified by planting the report's own wrapped `make
no-such-wrapped-target` into `CONTRIBUTING.md`: the old script reports
48 targets and exits 0, the new one names the target and the file and
exits 1. Plant removed afterwards.
Closes#228
Both branches add to the same two registries, so the conflicts are
between the two fixes rather than with main:
- backend/app.go: both register a janitor job. Both are registered.
- backend/maintenance/sweeps.go: both append a job at the end of the
file. Both are kept, each with its own closing tail.
- backend/maintenance/maintenance_test.go: both append a test. Both are
kept as separate functions.
- backend/library/library.go: #249's orphan-path lyrics delete was
written against the loop variable before #250 renamed it, so its
`audioFile.ID` no longer exists in that function. Adapted to `f.ID`.
Closes#248Closes#249
frontend/bindings is generated by wails3 and is not covered by the
codegen pre-commit hook, so the new bound method went out without it and
make bindings-check failed on the PR.
Three unbounded or stale surfaces, each small on its own:
- lyrics_index rows were never pruned on track removal, so the FTS index
grew forever. Delete the entry where the library search FTS entry is
already deleted, on the orphan and RemoveFromLibrary paths.
- search_clicks had no ceiling; age out ranking rows after a retention
window via a daily janitor job.
- queue.source_* kept a "Playing from X" label after its playlist was
deleted. Drop the source when the queue's own playlist goes, wired
through a playlist-service hook like Library.SetRemovalHooks.
Closes#249
artist_metadata was classified Cache/Swept but had no sweep and no
DELETE anywhere, so long-lived entity data (no TTL by design) grew for
the life of the install. Sweep rows whose MBID is neither a library
artist nor holding cached artwork, and register the job with the
janitor.
Closes#248
pruneEmptyEntities deleted empty albums but never the cover_art rows
they referenced, so removing the last track of an album leaked the row
and its files forever — the janitor's covers sweep computes its live set
from cover_art.file_path, which keeps the orphaned row's files exempt.
Extract sweepOrphanedCoverArt/removeCoverArtFiles as one implementation
and run it from pruneEmptyEntities (scan orphan path and
RemoveFromLibrary) and RemoveLibrary alike.
Closes#247
The incremental scan's orphan cleanup and RemoveFromLibrary deleted
audio_files rows without first filling the playlist phantom columns, so
a track removed from the library folder outside YellowJacket (or
removed from the library) became a permanently empty playlist row that
nothing could re-link — the same bug #183 fixed on the full-rescan and
retire paths, on the two paths it missed.
Add a scoped PreservePlaylistPhantomsForFiles and run it in the same
transaction as the deletes on both paths.
Closes#246
Replace play_history with listening_events — one row per track exit,
kind (complete/play/skip) plus raw position/duration — and add
skip_count/last_skipped to audio_files beside play_count/last_played.
The classifier that writes these lands later (plan 021); this is the
schema it records into.
Also drop the dead queue.source_playlist_id column and remove the stale
references to the squashed migration chain in download_*.sql and
tagging_items.sql, declaring the missing download-request indexes inline.
Retiring a stale audio_files dropped every playlist entry to an empty
row: ON DELETE SET NULL ran before the phantom_* columns were filled,
whereas the manual rescan path populates them first. Run the same
phantom population inside the retire transaction, before the drop, only
when audio_files is among the tables going, so
ResolvePhantomTracksAfterScan can re-link the entries.
Closes#183
Two #31-tick findings that would strand an unattended run. The qwen
worker hit its weekly 429 mid-tick; the obvious fallback
deepseek/deepseek-v4-pro is wrong because the deepseek provider has no
models (only catalog overrides) and fails silently — the model lives on
the go gateway as go/deepseek-v4-pro, with go/glm-5.3-flash the next
rung. And the async subagent runner has died without persisting a
session, so legs launch in the foreground and a dead worker is recovered
by completing, never re-implementing.
Closes#243
Four pages that list tracks — Tracks, genre, artist, and both playlist
views — had no way to start the whole list, or had a broken one. One
shared helper (utils/play-all.ts) now owns what "shuffle this
collection" means: SetQueue's shuffleStart only picks a random first
track when shuffle mode is already on, it does not turn it on, so the
mode is toggled before the queue is set. Each host passes an honest
queue Source (#14): anything that builds a queue names what it built it
from, so "Playing from" stops lying.
Two behaviour changes ride along, both flagged: smart-playlist-details'
Shuffle was a live no-op (shuffleStart without enabling mode played
track 1 in order) and is fixed; playlist-details' Play all drops its
shuffleStart:true, so with shuffle mode already on it now starts at the
first row instead of a random one — the album page's existing
semantics.
Verified: make ui-test (1147, incl. a case that fails when the
smart-playlist fix is reverted), npx tsc --noEmit, make e2e (255,
incl. new play-all and header-fit specs), make lint, make test,
make bindings-check, make css-check; artist header read from
screenshots at 424/320/900 (the pair wraps below the name on a phone).
Closes#31
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
yonlu
logan